[PATCH] drivers/block/scsi_ioctl.c, Video DVD playback support

[PATCH] drivers/block/scsi_ioctl.c, Video DVD playback support

[Elias da Silva]

[-- Attachment #1: Type: text/plain, Size: 242 bytes --]

Moin.

Attached patch fixes a problem of reading Video DVDs
through the cdrom_ioctl interface. VMware is among
the prominent victims.

The bug was introduced in kernel version 2.6.8 in the
function verify_command().

Regards,

Elias da Silva

[-- Attachment #2: linux-2.6.10-dvd.patch --]
[-- Type: text/x-diff, Size: 1906 bytes --]

--- linux-2.6.10/drivers/block/scsi_ioctl.c	2004-12-24 22:35:40.000000000 +0100
+++ linux-2.6.10-dvd/drivers/block/scsi_ioctl.c	2005-01-22 02:31:28.223951296 +0100
@@ -159,6 +159,11 @@
 		safe_for_read(GPCMD_SEEK),
 		safe_for_read(GPCMD_STOP_PLAY_SCAN),
 
+                /* Video DVD playback support */
+		safe_for_read(GPCMD_SET_STREAMING),
+		safe_for_read(GPCMD_SEND_KEY),
+                /* safe_for_read(0xe9), missing this opcode definition */
+
 		/* Basic writing commands */
 		safe_for_write(WRITE_6),
 		safe_for_write(WRITE_10),
@@ -179,13 +184,11 @@
 		safe_for_write(GPCMD_RESERVE_RZONE_TRACK),
 		safe_for_write(GPCMD_SEND_DVD_STRUCTURE),
 		safe_for_write(GPCMD_SEND_EVENT),
-		safe_for_write(GPCMD_SEND_KEY),
 		safe_for_write(GPCMD_SEND_OPC),
 		safe_for_write(GPCMD_SEND_CUE_SHEET),
 		safe_for_write(GPCMD_SET_SPEED),
 		safe_for_write(GPCMD_PREVENT_ALLOW_MEDIUM_REMOVAL),
 		safe_for_write(GPCMD_LOAD_UNLOAD),
-		safe_for_write(GPCMD_SET_STREAMING),
 	};
 	unsigned char type = cmd_type[cmd[0]];
 
@@ -194,13 +197,11 @@
 		return 0;
 
 	/* Write-safe commands just require a writable open.. */
-	if (type & CMD_WRITE_SAFE) {
-		if (file->f_mode & FMODE_WRITE)
-			return 0;
-	}
+	if ((type & CMD_WRITE_SAFE) && (file->f_mode & FMODE_WRITE))
+		return 0;
 
-	if (!(type & CMD_WARNED)) {
-		cmd_type[cmd[0]] = CMD_WARNED;
+	if (!type) {
+		type = cmd_type[cmd[0]] = CMD_WARNED;
 		printk(KERN_WARNING "scsi: unknown opcode 0x%02x\n", cmd[0]);
 	}
 
@@ -208,7 +209,14 @@
 	if (capable(CAP_SYS_RAWIO))
 		return 0;
 
-	/* Otherwise fail it with an "Operation not permitted" */
+        if (!(type & CMD_WARNED))
+        {
+          cmd_type[cmd[0]] |= CMD_WARNED;
+          printk(KERN_WARNING "scsi: opcode 0x%02x write/rawio"
+                 " permission denied\n", cmd[0]);
+        }
+
+        /* Otherwise fail it with an "Operation not permitted" */
 	return -EPERM;
 }
 

Re: [PATCH] drivers/block/scsi_ioctl.c, Video DVD playback support

[Jens Axboe]

On Sat, Jan 22 2005, Elias da Silva wrote:
> Moin.
> 
> Attached patch fixes a problem of reading Video DVDs
> through the cdrom_ioctl interface. VMware is among
> the prominent victims.
> 
> The bug was introduced in kernel version 2.6.8 in the
> function verify_command().

It's not a bug, add write permission to the device for the user using
the drive.

-- 
Jens Axboe

Re: [PATCH] drivers/block/scsi_ioctl.c, Video DVD playback support

[Elias da Silva]

> On Sat, Jan 22 2005, Elias da Silva wrote:
> > Attached patch fixes a problem of reading Video DVDs
> > through the cdrom_ioctl interface. VMware is among
> > the prominent victims.
> > 
> > The bug was introduced in kernel version 2.6.8 in the
> > function verify_command().
> 
> It's not a bug, add write permission to the device for the user using
> the drive.
Hi.

The device already has write permission for the user using the
drive... and this is not the point.

The point is
	a. the user (program) wants to read a protected DVD,

	b. the user has permission to read the device,

	c. since kernel 2.6.8 the user can't use his right to read a
	DVD media, because according to verify_command() he is forced
	to open the device with RW mode instead of RONLY.

This is true for protected media because of the authentication
process needed between "host" 	and DVD device. IMHO,
the classification of the opcodes

	a. GPCMD_SEND_KEY and
	b. GPCMD_SET_STREAMING

as only "save for write" is wrong.

Furthermore, if you use
	a. cdrom_ioctl (..., DVD_AUTH,...) instead of
	b. cdrom_ioctl (..., CDROM_SEND_PACKET,...)
	-> scsi_cmd_ioctl()-> sg_io()-> verify_command()

the same authentication procedure works as expected on a
RONLY opened device!

Please rethink your decisions introduced with verify_command()
and make for example VMware work _again_ with Video DVDs.

Regards,

Elias

Re: [PATCH] drivers/block/scsi_ioctl.c, Video DVD playback support

[Jens Axboe]

On Mon, Jan 24 2005, Elias da Silva wrote:
> > On Sat, Jan 22 2005, Elias da Silva wrote:
> > > Attached patch fixes a problem of reading Video DVDs
> > > through the cdrom_ioctl interface. VMware is among
> > > the prominent victims.
> > > 
> > > The bug was introduced in kernel version 2.6.8 in the
> > > function verify_command().
> > 
> > It's not a bug, add write permission to the device for the user using
> > the drive.
> Hi.
> 
> The device already has write permission for the user using the
> drive... and this is not the point.
> 
> The point is
> 	a. the user (program) wants to read a protected DVD,
> 
> 	b. the user has permission to read the device,
> 
> 	c. since kernel 2.6.8 the user can't use his right to read a
> 	DVD media, because according to verify_command() he is forced
> 	to open the device with RW mode instead of RONLY.

Right, it's an unfortunate side effect of the command table.

> This is true for protected media because of the authentication
> process needed between "host" 	and DVD device. IMHO,
> the classification of the opcodes
> 
> 	a. GPCMD_SEND_KEY and
> 	b. GPCMD_SET_STREAMING
> 
> as only "save for write" is wrong.

You need to explain why you think that is so, since this is the core of
the argument. The only thing I can say is that perhaps SEND_KEY should
even be root only, since it has a fairly large scope. It's really a
device exclusive type situation, where is a user has exclusive access to
the device it would be ok to issue a SEND_KEY but otherwise not. It's
not clearly a read vs write thing. It's impossible to shoe-horn a more
complicated permission model on top of something as silly as read vs
write.

> Furthermore, if you use
> 	a. cdrom_ioctl (..., DVD_AUTH,...) instead of
> 	b. cdrom_ioctl (..., CDROM_SEND_PACKET,...)
> 	-> scsi_cmd_ioctl()-> sg_io()-> verify_command()
> 
> the same authentication procedure works as expected on a
> RONLY opened device!

DVD_AUTH by-passes scsi_ioctl.c, so yes.

> Please rethink your decisions introduced with verify_command()
> and make for example VMware work _again_ with Video DVDs.

It's not my decisions. The problem is that it is policy, and it has to
be restrictive to be safe.

-- 
Jens Axboe

Re: [PATCH] drivers/block/scsi_ioctl.c, Video DVD playback support

[Elias da Silva]

On Monday 24 January 2005 21:39, you wrote:
[snip]
: > This is true for protected media because of the authentication
: > process needed between "host" 	and DVD device. IMHO,
: > the classification of the opcodes
: > 
: > 	a. GPCMD_SEND_KEY and
: > 	b. GPCMD_SET_STREAMING
: > 
: > as only "save for write" is wrong.
: 
: You need to explain why you think that is so, since this is the core of
: the argument. The only thing I can say is that perhaps SEND_KEY should
: even be root only, since it has a fairly large scope.

This is exactly the point: if the kernel wants to be safe, the
authentication procedure should be totally implemented in the kernel
and be protected against further changes via "alternative" ways...
but it isn't now and probably won't be although it could be.

On the other hand I don't think it is a good idea to let only
root read a video DVD an a Linux system.

[snip]
: > Furthermore, if you use
: > 	a. cdrom_ioctl (..., DVD_AUTH,...) instead of
: > 	b. cdrom_ioctl (..., CDROM_SEND_PACKET,...)
: > 	-> scsi_cmd_ioctl()-> sg_io()-> verify_command()
: > 
: > the same authentication procedure works as expected on a
: > RONLY opened device!
: 
: DVD_AUTH by-passes scsi_ioctl.c, so yes.
[snip]
:The problem is that it is policy, and it has to
: be restrictive to be safe.

Yes, and with this "alternative" way to to things in the kernel
a user can complete circumvent the restrictive policy now
implemented in verify_command(). So you are securing
the backdoor and leaving the frontdoor completely open, right?

Now if safety is top priority, than

  a. the authentication must be implemented by the kernel and

  b. there should be no other way to access the device and
	completely circumvent the policy enforcement.

or

we both agree, that in a situation where a user has exclusive
access to the device it would be OK to issue a SEND_KEY, even
if he uses RONLY mode for access.

It is as you wrote a silly situation but the current implementation
don't augment security but instead prevents "innocent" software
to continue to run.

Regards,

Elias

Re: [PATCH] drivers/block/scsi_ioctl.c, Video DVD playback support

[Alan Cox]

On Llu, 2005-01-24 at 22:10, Elias da Silva wrote:
> This is exactly the point: if the kernel wants to be safe, the
> authentication procedure should be totally implemented in the kernel
> and be protected against further changes via "alternative" ways...
> but it isn't now and probably won't be although it could be.

It provides the DVD_AUTH ioctls to handle this. Why are you banging raw
commands at hardware when there is an abstraction for it ?

Someone did actually have a demo of a small fs that allowed you to
fiddle with the status but possibly the code could get smarter for
"exclusive user of media". I'm not sure if that is worth it.

Re: [PATCH] drivers/block/scsi_ioctl.c, Video DVD playback support

[Jens Axboe]

On Tue, Jan 25 2005, Alan Cox wrote:
> Someone did actually have a demo of a small fs that allowed you to
> fiddle with the status but possibly the code could get smarter for
> "exclusive user of media". I'm not sure if that is worth it.

I think it is worth it, on the grounds that this is a never ending farce
on what commands should be what type and for what device. At least the
fs would allow people to easily customize for their setup.

-- 
Jens Axboe

Re: [PATCH] drivers/block/scsi_ioctl.c, Video DVD playback support

[Elias da Silva]

On Tuesday 25 January 2005 01:01, you wrote:
[snip]
: > This is exactly the point: if the kernel wants to be safe, the
: > authentication procedure should be totally implemented in the kernel
: > and be protected against further changes via "alternative" ways...
: > but it isn't now and probably won't be although it could be.
: 
: It provides the DVD_AUTH ioctls to handle this. Why are you banging raw
: commands at hardware when there is an abstraction for it ?
Hello.

This is the way VMware and probably other comparable emulators
access the devices.

Yes, sometimes you have to risk broken software in favor of augmented
security, but so far we only have broken software.

: Someone did actually have a demo of a small fs that allowed you to
: fiddle with the status but possibly the code could get smarter for
: "exclusive user of media". I'm not sure if that is worth it.

Do you have the name of the fs and/or the name of author?

Do we have a clear understanding that this fs would only
be a benefit if *All* the different ways to access the device would
use the same policy enforcement and consistently allow or
disallow certain operations regardless of the access method?

Regards,

Elias

Re: [PATCH] drivers/block/scsi_ioctl.c, Video DVD playback support

[Alan Cox]

On Maw, 2005-01-25 at 09:29, Elias da Silva wrote:
> On Tuesday 25 January 2005 01:01, you wrote:
> Yes, sometimes you have to risk broken software in favor of augmented
> security, but so far we only have broken software.

Well let me see in 2.6.5 if you could read open the block device at all
you could erase the drive firmware. I think we've significantly improved
security actually.

Re: [PATCH] drivers/block/scsi_ioctl.c, Video DVD playback support

[Elias da Silva]

On Tuesday 25 January 2005 13:44, you wrote:
: On Maw, 2005-01-25 at 09:29, Elias da Silva wrote:
: > On Tuesday 25 January 2005 01:01, you wrote:
: > Yes, sometimes you have to risk broken software in favor of augmented
: > security, but so far we only have broken software.
: 
: Well let me see in 2.6.5 if you could read open the block device at all
: you could erase the drive firmware. I think we've significantly improved
: security actually.

Alan, please don't let us loose focus!

I'm talking about  the classification of the opcodes
     a. GPCMD_SEND_KEY and
     b. GPCMD_SET_STREAMING

as only "save for write" in scsi_ioctl.c:verify_command()
since kernel version 2.6.8.

The intended security improvements of this restriction can be
completed circumvented by using
	a. cdrom_ioctl (..., DVD_AUTH,...) instead of
	b. cdrom_ioctl (..., CDROM_SEND_PACKET,...)

so the result is as described:

"no security improvements at the cost of broken software".

The changes looked random to me and I would like to see
a clear concept, which would drive the necessary changes for
improved security and stability.
I'm putting my finger on some loose ends below drivers/cdrom,
drivers/ide and drivers/block.

Regards,

Elias

Re: [PATCH] drivers/block/scsi_ioctl.c, Video DVD playback support

[Jens Axboe]

On Tue, Jan 25 2005, Elias da Silva wrote:
> : Someone did actually have a demo of a small fs that allowed you to
> : fiddle with the status but possibly the code could get smarter for
> : "exclusive user of media". I'm not sure if that is worth it.
> 
> Do you have the name of the fs and/or the name of author?

If I'm not mistaken, Peter Jones has posted a few iterations of such an
fs some months ago.

> Do we have a clear understanding that this fs would only
> be a benefit if *All* the different ways to access the device would
> use the same policy enforcement and consistently allow or
> disallow certain operations regardless of the access method?

The command restriction table _only_ works through the SG_IO path, which
does include CDROM_SEND_PACKET as well since it is layered on top of
SG_IO. It doesn't control various driver ioctl exported interfaces, they
would need to add a callback to verify_command() for permission checks.

-- 
Jens Axboe

Re: [PATCH] drivers/block/scsi_ioctl.c, Video DVD playback support

[Elias da Silva]

On Tuesday 25 January 2005 13:45, you wrote:
[snip]
: If I'm not mistaken, Peter Jones has posted a few iterations of such an
: fs some months ago.

Thank you. I will check this...

: > Do we have a clear understanding that this fs would only
: > be a benefit if *All* the different ways to access the device would
: > use the same policy enforcement and consistently allow or
: > disallow certain operations regardless of the access method?
: 
: The command restriction table _only_ works through the SG_IO path, which
: does include CDROM_SEND_PACKET as well since it is layered on top of
: SG_IO. It doesn't control various driver ioctl exported interfaces, they
: would need to add a callback to verify_command() for permission checks.

Hmm... what exactly does that mean? Who is ment by "_they_ would need..."?

I don't want to take responsability for the reaction of the xine/mplayer/etc.
community the day their software stops playing video DVDs on Linux ;-)

Regards,

Elias

Re: [PATCH] drivers/block/scsi_ioctl.c, Video DVD playback support

[Jens Axboe]

On Tue, Jan 25 2005, Elias da Silva wrote:
> On Tuesday 25 January 2005 13:45, you wrote:
> [snip]
> : If I'm not mistaken, Peter Jones has posted a few iterations of such an
> : fs some months ago.
> 
> Thank you. I will check this...
> 
> : > Do we have a clear understanding that this fs would only
> : > be a benefit if *All* the different ways to access the device would
> : > use the same policy enforcement and consistently allow or
> : > disallow certain operations regardless of the access method?
> : 
> : The command restriction table _only_ works through the SG_IO path, which
> : does include CDROM_SEND_PACKET as well since it is layered on top of
> : SG_IO. It doesn't control various driver ioctl exported interfaces, they
> : would need to add a callback to verify_command() for permission checks.
> 
> Hmm... what exactly does that mean? Who is ment by "_they_ would need..."?

It refers back to 'various driver ioctl' earlier, so it refers to the
driver itself.

-- 
Jens Axboe

Re: [PATCH] drivers/block/scsi_ioctl.c, Video DVD playback support

[Elias da Silva]

On Tuesday 25 January 2005 17:21, you wrote:
: > : The command restriction table _only_ works through the SG_IO path, which
: > : does include CDROM_SEND_PACKET as well since it is layered on top of
: > : SG_IO. It doesn't control various driver ioctl exported interfaces, they
: > : would need to add a callback to verify_command() for permission checks.
: > 
: > Hmm... what exactly does that mean? Who is ment by "_they_ would need..."?
: 
: It refers back to 'various driver ioctl' earlier, so it refers to the
: driver itself.

Ouch... Jens, the various driver won't change by themselves!

Who is responsible, aka what is the name of the person(s)?
Are they reading this thread? Is this going to happen
(the driver changes) or is this only a wast of time?

Regards,

Elias