[Spice-devel] Fix filter string behaviour to be more intuitive before lots of people start depending on it

[David Jaša]

Hello,

Hans de Goede píše v Pá 01. 06. 2012 v 20:45 +0200:
> Hi,
> 
> On 05/31/2012 03:34 PM, David Jaša wrote:
> > SSIA
> >
> > The current "grandfathered" filter format behaves counter-intuitively.
> > Let me describe it based on my experiments:
> >       1. there is a "hard default" that disables all devices (aka
> >          -1,-1,-1,-1,0), it must be always overwritten if you wish to
> >          auto-share anything. This is not advertised anywhere
> 
> AFAIK this is the same behavior as the filter for incentive pro, the
> filter format is based on that of incentive pro, so the things like
> the filter-editor tool and old filters can be reused.
> 
> 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.

>  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"

When user specifies --spice-usbredir-filter="11,-1,-1,-1,0", the
effective filter will be "-1,-1,-1,-1,0" (!) 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.

David

> And as discussed above I believe that an allow by default if nothing
> matches the given filter is a bad idea!
> 
> Regards,
> 
> Hans
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel

-- 

David Jaša, RHCE

SPICE QE based in Brno
GPG Key:     22C33E24 
Fingerprint: 513A 060B D1B4 2A72 7F0D 0278 B125 CD00 22C3 3E24