[Qemu-devel] [PATCH 0/5] ATAPI pass through v2

[Qemu-devel] [PATCH 0/5] ATAPI pass through v2

[Bique Alexandre]

Hi,

I updated my patch according to your previous comments.

Changes from my previous version:
 - split the big patch in 5 patches.
 - not exporting any private structure
 - switched to SG_IO and brdv_aio_ioctl()
 - not including linux/cdrom.h or linux/bsg.h
 - got some stuff like defines and request_sense structure from linux/cdrom.h

Thanks.
-- 
Alexandre Bique

Re: [Qemu-devel] [PATCH 0/5] ATAPI pass through v2

[Stuart Brady]

On Wed, Jul 01, 2009 at 07:31:53PM +0100, Bique Alexandre wrote:
> I updated my patch according to your previous comments.
> 
> Changes from my previous version:
>  - split the big patch in 5 patches.
>  - not exporting any private structure
>  - switched to SG_IO and brdv_aio_ioctl()
>  - not including linux/cdrom.h or linux/bsg.h
>  - got some stuff like defines and request_sense structure from linux/cdrom.h

Forgive my ignorance, but does ATAPI passthrough have any security 
implications that should be documented?

I expect that running qemu as root counts as a 'bad idea' (I gather
that commands are filtered when running as a regular user), but even so,
I wonder if guests should be prevented from performing firmware updates?

Obviously, the same questions would apply for SCSI passthrough...

Regards,
-- 
Stuart Brady

Re: [Qemu-devel] [PATCH 0/5] ATAPI pass through v2

[Alexandre Bique]

Hi Stuart Brady,

On Tue, Jul 7, 2009 at 8:03 PM, Stuart Brady<sdbrady@ntlworld.com> wrote:
> On Wed, Jul 01, 2009 at 07:31:53PM +0100, Bique Alexandre wrote:
>> I updated my patch according to your previous comments.
>>
>> Changes from my previous version:
>>  - split the big patch in 5 patches.
>>  - not exporting any private structure
>>  - switched to SG_IO and brdv_aio_ioctl()
>>  - not including linux/cdrom.h or linux/bsg.h
>>  - got some stuff like defines and request_sense structure from linux/cdrom.h
>
> Forgive my ignorance, but does ATAPI passthrough have any security
> implications that should be documented?

The patch doesn't introduce any resource allocation so it will be
difficult to bomb qemu with the ATAPI pass through code.
There is one command to update the firmware of the device. This one is blocked.
The security issue is the same as giving the device (+rw) to a user on
the system.

> I expect that running qemu as root counts as a 'bad idea' (I gather
> that commands are filtered when running as a regular user), but even so,
> I wonder if guests should be prevented from performing firmware updates?
Yeps.

> Obviously, the same questions would apply for SCSI passthrough...

Regards,

-- 
Alexandre Bique

Re: [Qemu-devel] [PATCH 0/5] ATAPI pass through v2

[Paul Brook]

> > I expect that running qemu as root counts as a 'bad idea' (I gather
> > that commands are filtered when running as a regular user), but even so,
> > I wonder if guests should be prevented from performing firmware updates?
>
> Yeps.

I disagree.  Upgrading the firmware from within the guest sounds like a 
legitimate use,  especially given the proliferation of proprietary windows-
only upgrade utilities.

Paul

Re: [Qemu-devel] [PATCH 0/5] ATAPI pass through v2

[Alexandre Bique]

On Tue, Jul 7, 2009 at 10:44 PM, Paul Brook<paul@codesourcery.com> wrote:
>> > I expect that running qemu as root counts as a 'bad idea' (I gather
>> > that commands are filtered when running as a regular user), but even so,
>> > I wonder if guests should be prevented from performing firmware updates?
>>
>> Yeps.
>
> I disagree.  Upgrading the firmware from within the guest sounds like a
> legitimate use,  especially given the proliferation of proprietary windows-
> only upgrade utilities.

Maybe we could agree on a switch to forward/block this command ?

-- 
Alexandre Bique

Re: [Qemu-devel] [PATCH 0/5] ATAPI pass through v2

[Anthony Liguori]

Alexandre Bique wrote:
> On Tue, Jul 7, 2009 at 10:44 PM, Paul Brook<paul@codesourcery.com> wrote:
>   
>>>> I expect that running qemu as root counts as a 'bad idea' (I gather
>>>> that commands are filtered when running as a regular user), but even so,
>>>> I wonder if guests should be prevented from performing firmware updates?
>>>>         
>>> Yeps.
>>>       
>> I disagree.  Upgrading the firmware from within the guest sounds like a
>> legitimate use,  especially given the proliferation of proprietary windows-
>> only upgrade utilities.
>>     
>
> Maybe we could agree on a switch to forward/block this command ?
>   

No need for a switch IMHO.  If a user is doing pass through, they ought 
to expect that the guest has direct access to the device.

The fact that we can prevent this with ATAPI is a special case.  When we 
eventually merge in VT-d based PCI pass through, we would not be able to 
enforce this sort of switch.  That would result in confusion on the part 
of users.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/5] ATAPI pass through v2

[Stuart Brady]

On Tue, Jul 07, 2009 at 06:01:05PM -0500, Anthony Liguori wrote:
> 
> No need for a switch IMHO.  If a user is doing pass through, they ought 
> to expect that the guest has direct access to the device.

Of course, if passthrough were to be done by default, the user might be
forgiven for being unaware of this...
-- 
Stuart Brady

Re: [Qemu-devel] [PATCH 0/5] ATAPI pass through v2

[Ian Jackson]

Anthony Liguori writes ("Re: [Qemu-devel] [PATCH 0/5] ATAPI pass through v2"):
> No need for a switch IMHO.  If a user is doing pass through, they ought 
> to expect that the guest has direct access to the device.

The firmware of an IDE device can usually take over complete the
control of the host, if it chooses to and knows how.  So upgrading the
firmware on the device is a lot more serious than just being able to
break the device.  It would allow the guest to escape containment.

So this definitely needs to be disabled by default.

Anthony Liguori writes ("Re: [Qemu-devel] [PATCH 0/5] ATAPI pass through v2"):
> One should never rely on QEMU to enforce any security policy.  That's 
> the job of the OS.

I disagree entirely.  The qemu process inevitably has access to an
enormous amount of stuff that the guest shouldn't have, and in most
cases users don't even run it as a different user.

Or are you suggesting qemu should always be run in a chroot ?  Or a VM
perhaps ?  qemu only safe run under Xen PV ?  I don't think the KVM
guys are really going to like that as a security policy ...

> I'm sure something like SELinux can be used to prevent a root QEMU 
> process from doing a firmware upgrade.

*boggle*  You're not serious, are you ?

Ian.

Re: [Qemu-devel] [PATCH 0/5] ATAPI pass through v2

[Avi Kivity]

On 07/08/2009 07:09 PM, Ian Jackson wrote:
>> I'm sure something like SELinux can be used to prevent a root QEMU
>> process from doing a firmware upgrade.
>>      
>
> *boggle*  You're not serious, are you ?
>    

selinux can prevent anything.  In fact, I'm sure it does.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 0/5] ATAPI pass through v2

[Carl-Daniel Hailfinger]

On 08.07.2009 18:38, Avi Kivity wrote:
> On 07/08/2009 07:09 PM, Ian Jackson wrote:
>>> I'm sure something like SELinux can be used to prevent a root QEMU
>>> process from doing a firmware upgrade.
>>>      
>>
>> *boggle*  You're not serious, are you ?    
>
> selinux can prevent anything.  In fact, I'm sure it does.

I doubt SELinux has a builtin ATAPI command filter which knows all
_undocumented_ firmware upgrade commands. In fact, there are some ATAPI
devices which abuse existing and documented-as-harmless ATAPI commands
(which are regularly used for CD burning) for firmware upgrades.

Unless SELinux knows every single firmware upgrade mechanism for every
ATAPI device ever released (including special hacked RPC1 firmware
etc.), the only way to prevent firmware upgrades is to disable ATAPI
command passthrough. It's like wanting to secure a completely unpatched
Windows server by placing it behind a Linux firewall. You can hope, but
nobody is going to vouch for the security of that Windows machine.

So yes, SELinux can probably prevent firmware upgrades, but only by
disabling raw ATAPI access completely. In that case, the ATAPI
passthrough is pointless.

Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/

Re: [Qemu-devel] [PATCH 0/5] ATAPI pass through v2

[Anthony Liguori]

Carl-Daniel Hailfinger wrote:
> On 08.07.2009 18:38, Avi Kivity wrote:
>   
>> On 07/08/2009 07:09 PM, Ian Jackson wrote:
>>     
>>>> I'm sure something like SELinux can be used to prevent a root QEMU
>>>> process from doing a firmware upgrade.
>>>>      
>>>>         
>>> *boggle*  You're not serious, are you ?    
>>>       
>> selinux can prevent anything.  In fact, I'm sure it does.
>>     
>
> I doubt SELinux has a builtin ATAPI command filter which knows all
> _undocumented_ firmware upgrade commands. In fact, there are some ATAPI
> devices which abuse existing and documented-as-harmless ATAPI commands
> (which are regularly used for CD burning) for firmware upgrades.
>   

So then we can't prevent it either which means the whole discussion is 
moot :-)

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/5] ATAPI pass through v2

[Avi Kivity]

On 07/08/2009 08:28 PM, Carl-Daniel Hailfinger wrote:
> On 08.07.2009 18:38, Avi Kivity wrote:
>    
>> On 07/08/2009 07:09 PM, Ian Jackson wrote:
>>      
>>>> I'm sure something like SELinux can be used to prevent a root QEMU
>>>> process from doing a firmware upgrade.
>>>>
>>>>          
>>> *boggle*  You're not serious, are you ?
>>>        
>> selinux can prevent anything.  In fact, I'm sure it does.
>>      
>
> I doubt SELinux has a builtin ATAPI command filter which knows all
> _undocumented_ firmware upgrade commands. In fact, there are some ATAPI
> devices which abuse existing and documented-as-harmless ATAPI commands
> (which are regularly used for CD burning) for firmware upgrades.
>    

Come on, it was a joke (though these days you can actually work on a 
machine with selinux enabled).

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [PATCH 0/5] ATAPI pass through v2

[Anthony Liguori]

Ian Jackson wrote:
> Anthony Liguori writes ("Re: [Qemu-devel] [PATCH 0/5] ATAPI pass through v2"):
>   
>> No need for a switch IMHO.  If a user is doing pass through, they ought 
>> to expect that the guest has direct access to the device.
>>     
>
> The firmware of an IDE device can usually take over complete the
> control of the host, if it chooses to and knows how.  So upgrading the
> firmware on the device is a lot more serious than just being able to
> break the device.  It would allow the guest to escape containment.
>
> So this definitely needs to be disabled by default.
>   

Pass through == escape containment.

That's generally true (even with VT-d).  There's all sorts of bad things 
you can do generating interrupt storms or physically bricking hardware.

> I disagree entirely.  The qemu process inevitably has access to an
> enormous amount of stuff that the guest shouldn't have, and in most
> cases users don't even run it as a different user.
>
> Or are you suggesting qemu should always be run in a chroot ?

As a lesser privileged user or as root heavily restricted by SELinux, yes.

>   Or a VM
> perhaps ?  qemu only safe run under Xen PV ?  I don't think the KVM
> guys are really going to like that as a security policy ...
>   

It's about layers of security.  If you design your security assuming 
that QEMU is safe, you're taking a much bigger risk than if you assume 
QEMU is hostile.

>> I'm sure something like SELinux can be used to prevent a root QEMU 
>> process from doing a firmware upgrade.
>>     
>
> *boggle*  You're not serious, are you ?
>   

Yes, I'm actually a fan of SELinux in the context of a dedicated 
virtualization system.


Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/5] ATAPI pass through v2

[Vincent Hanquez]

On Wed, Jul 08, 2009 at 12:20:59PM -0500, Anthony Liguori wrote:
>>> I'm sure something like SELinux can be used to prevent a root QEMU  
>>> process from doing a firmware upgrade.
>>>     
>>
>> *boggle*  You're not serious, are you ?
>>   
>
> Yes, I'm actually a fan of SELinux in the context of a dedicated  
> virtualization system.

do you really expect to put a SCSI packet inspector (to detect firmware update
for example) in a SELinux layer ?

-- 
Vincent

Re: [Qemu-devel] [PATCH 0/5] ATAPI pass through v2

[Anthony Liguori]

Vincent Hanquez wrote:
> On Wed, Jul 08, 2009 at 12:20:59PM -0500, Anthony Liguori wrote:
>   
>>>> I'm sure something like SELinux can be used to prevent a root QEMU  
>>>> process from doing a firmware upgrade.
>>>>     
>>>>         
>>> *boggle*  You're not serious, are you ?
>>>   
>>>       
>> Yes, I'm actually a fan of SELinux in the context of a dedicated  
>> virtualization system.
>>     
>
> do you really expect to put a SCSI packet inspector (to detect firmware update
> for example) in a SELinux layer ?
>   

SELinux uses LSM to provide security hooks for enforcement so if there 
isn't already, one would add an LSM hook in the Linux ATAPI driver for 
firmware updates.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/5] ATAPI pass through v2

[Anthony Liguori]

Stuart Brady wrote:
> On Wed, Jul 01, 2009 at 07:31:53PM +0100, Bique Alexandre wrote:
>   
>> I updated my patch according to your previous comments.
>>
>> Changes from my previous version:
>>  - split the big patch in 5 patches.
>>  - not exporting any private structure
>>  - switched to SG_IO and brdv_aio_ioctl()
>>  - not including linux/cdrom.h or linux/bsg.h
>>  - got some stuff like defines and request_sense structure from linux/cdrom.h
>>     
>
> Forgive my ignorance, but does ATAPI passthrough have any security 
> implications that should be documented?
>
> I expect that running qemu as root counts as a 'bad idea' (I gather
> that commands are filtered when running as a regular user), but even so,
> I wonder if guests should be prevented from performing firmware updates?
>   

One should never rely on QEMU to enforce any security policy.  That's 
the job of the OS.

I'm sure something like SELinux can be used to prevent a root QEMU 
process from doing a firmware upgrade.

Regards,

Anthony Liguori