Re: Fix filter string behaviour to be more intuitive before lots of people start depending on it

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 06/04/2012 04:28 PM, David Jaša wrote:
Hans de Goede píše v Po 04. 06. 2012 v 14:55 +0200:
Hi,

On 06/04/2012 01:17 PM, David Jaša wrote:

Hans de Goede píše v Pá 01. 06. 2012 v 20:45 +0200:
More importantly then the "incentive pro did it this way" argument though,
is that it is a safe default from security pov. These filter strings are
used in more then one place:

IIRC, they were quite confusing too, generating their number of false
bugs. I see this as a opportunity to get things right before we start to
depend on "wrong" behavior.

1) In the client to decide which devices to auto-redirect when new devices
      are hotplugged
2) In the client to specify (already plugged in) devices to redirect on
      client connect (*)
3) On the qemu cmdline to specify which redirected devices will be accepted

*) Not yet available, to be implemented soon.

For 2 and 3 deny by default clearly is the right policy,
WRT 2: agreed. I'd question 3 though. The best of "default deny" is USB
redirection disabled altogether. I can imagine that when there is no
"sane default" here, we'll end up with administrators going for the
shortest string that will do the job. "-1,-1,-1,-1,1". Effectively doing
exactly the opposite you want to achieve.
The default qemu behavior of course is no redirection, only if you specify
usbredir devices on the qemu cmdline you get usbredir. Then once you enable
usbredir devices, the default is to allow any devices, but once you
specify a filter, then, and only then does "-1,-1,-1,-1,0" always get
appended. So that if the admin requests filtering and he forgets to close
with the filter with a catch all rule, the behavior will be to deny anything
not explicitly allowed.
Iff there is indeed used deny-all filter by default AND qemu will spit a
message when no whitelist filter is set (something like "USB redirection
is set but no devices are allowed to pass by the filter: %s" %
FILTER_STIRNG), I'm fine with that.

   for 2 having a
redirect everything that does not match policy means redirecting all
devices the client has on client connect, not good. For 3 an allow by
defautl policy is simple not an option from a security pov.


        2. there is a "soft default" that enables everything but HID
           devices (aka 3,-1,-1,-1,0|-1,-1,-1,-1,1)
        3. when user specifies a custom filter string, it
                 * replaces string from #2
                 * is based on top of #1

All combined together cause unexpected behaviours like the one described
in https://bugzilla.redhat.com/show_bug.cgi?id=823541 .

The most straightforward fix seems to get rid of "soft default" #2 and
use its value in "hard default" #1. That way, when user specifies
--spice-usbredir-filter="11,-1,-1,-1,0", the effective filter string
will be what user will expect:
"3,-1,-1,-1,0|11,-1,-1,-1,0|-1,-1,-1,-1,1"

instead of current behaviour where such string will become
"11,-1,-1,-1,0|-1,-1,-1,-1,0", effectively block-all "-1,-1,-1,-1,0"

Hans, do you think this is doable in a short time frame? If it is not
done this way soon, people will constantly ask about that...
Doable yes, desirable no. Having a default policy is one thing, but
mandatory adding (in a very hidden way) "11,-1,-1,-1,0" to any
filter is just plain wrong, as it will surprise the user.

I might have been unclear here. Effective filter when one doesn't
specify one is: "3,-1,-1,-1,0|-1,-1,-1,-1,1"
Correct.

When user specifies --spice-usbredir-filter="11,-1,-1,-1,0", the
effective filter will be "-1,-1,-1,-1,0" (!)
Correct.

This does exactly what you
question - it in a "very hidden way" removes default values from any
filter, IMO exactly as "plain wrong" as what you say.
It does not remove default values, it overrides the default with
a user specified value, which is perfectly normal behavior for
user setable things.

What you are asking for is a -background option for a program
which by default has red background and then when a user specifies
"-background blue" he gets a purple background!

No, I'm thinking in sets here:

Current "soft default" filter is like this:

+--------------------+
|  -1,-1,-1,-1,1     |
|                    |
|  +--------------+  |
|  | 3,-1,-1,-1,0 |  |
|  +--------------+  |
+--------------------+

If I add there "11,-1,-1,-1,Z", I expect it to touch just newly-defined

I agree with Hans.

Default values are for cases where the user does not specifies specific values/options.
If a user specifies a value/option that's what being used.

A user specified filter is not added to the default filter, but overrides it, becoming the effective filter.

If you specify 11,-1-1-1,Z then that's your filter.

To prevent accidents Hans added the block-everything rule at the end.

That makes the filter   11,-1,-1,-1,Z|-1,-1,-1,-1,0

You can set the filter to be 11,-1,-1,-1,Z| 3,-1,-1,-1,0|-1,-1,-1,-1,1 if you like to.

Regards,
    Uri.

specific set "11,-1,-1,-1", not non-intersecting "-3,-1,-1,-1,X" nor
universal "-1,-1,-1,-1,Y" sets:

+-------------------------------------------------+
|  -1,-1,-1,-1,UNCHANGED                          |
|                             +---------------+   |
|  +----------------------+   | 11,-1,-1,-1,Z |   |
|  | 3,-1,-1,-1,UNCHANGED |   +---------------+   |
|  +----------------------+                       |
+-------------------------------------------------+

Touching them is counter-intuitive.

In your colour analogy it's like saying "hey I want blue square in
top-left corner" and you'll get whole background blue instead of the
rest staying red.

David


_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]