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

[Hans de Goede]

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.

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

Regards,

Hans