Bug 133098

Should be for RHEL4 surely not RHEL3

Alan, wouldn't any RHEL4 exposure to this also occur on RHEL3?

I briefly examined sg_new_write() and sg_ioctl() for the SG_IO
case (in drivers/scsi/sg.c), and both 2.6 and RHEL3 seem to use
"read_only" similarly (but I'm not sure where the alleged bug is).

CAN-2004-0813

Well, I agree with Alan.  This is for RHEL4 not RHEL3.  Not only that,
but it appears to be fixed in upstream 2.6 kernels at this point.

So, to answer Ernie's question about where the bug is.  In RHEL3, you
grant access to a block device by giving permissions to the
/dev/{hd,sd,sr,etc.} files.  You grant access to SG_IO ioctls by
setting permissions on /dev/sg files.  So, even if the SG_IO routines
didn't check anything before allowing a user to blank the device or
overwrite a firmware, the user still had to be able to open the
/dev/sg* file associated with the device, not just the normal device file.

When Jens moved the SG_IO functionality out of the scsi layer and into
the block layer, the SG_IO ioctls now became valid ioctls on all block
devices.  So permissions that used to apply only to regular block
device access (/dev/{hd,sd,sr,etc}) now applied to the SG_IO ioctls as
well and this had unintended side effects.   <--- The Bug

To resolve the issue, the 2.6 kernel now implements a read-only vs.
write-required filter on SG_IO packets that compares the requested
operation against a table and determines if the command is allowed
(see verify_command in drivers/block/scsi_ioctl.c in current 2.6 kernels).

So, looks like this was a short lived security issue that only existed
in the 2.6 kernel series for a short while, and doesn't effect RHEL3
at all.

Alan, Ernie, any disagreement on that conclusion?

Doug, I concur, and have just verified that /dev/sg* files on
a RHEL3 U3 installation all have 660 (uid "root", gid "disk"),
and thus unprivileged users would not be able to open any of
these character special files in order to perform any ioctls.

Doug/Alan, should this bug be closed now?

I believe so.  Alan?

Actually our pam setup will give console users write access to
/dev/sg* afaics ....

It isnt supposed to. If our pam_console rules are wrong please open a
pam bug about that. I agree this can closed.

To summarize my understanding:

In RHEL /dev/sg* are 660 root.disk and altered to be owned by the
console user at login.  So there is no case where we only issue read
access and don't want write access.  Therefore this particular kernel
issue isn't going to affect RHEL users, closing.

(The bigger issue outside the scope of this bug is that we want to be
able to give the console user write access to a cdrom without them
being able to flash firmware etc).

Disagree entirely. The bug allows every user to bypass file
permissions, it also allows them to load modified firmware that
bypasses system security and it allows any local user exploit to
trivially get CAP_SYS_RAWIO and/or destroy hardware.

One trivial end user mistake, one app run by that user that should not
have been and game over send computer back to Dell.

cdrecord supports setuid operation and has been audited for it, its
also got a fairly good record and having reviewed the code seems far
more secure than allowing all users to own the computer.

Re-opening. 

Alan: Are you wanting the /dev/sg* permissions modified or are you
wanting a kernel change?  If it's the /dev/sg* permissions, then this
bug needs reassigned to whomever owns the pam stuff and another bug
should be opened on cdrecord et al to get them whipped into shape for
suid operation so that user's don't suddenly stop being able to burn
CDs when they get the relevant security update.

Note that there has been considerable patches added after the 2.6.8 upstream
patch to correct this, and SUSE had CAN-2004-1190 also issues for "firmware
overwrites /dev/cdrom".  For many details see 
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=300162

The /dev/sg* permissions would be modified by pam_console only if they are
linked by udev (or kudzu) as /dev/cdrom* or /dev/cdwriter*

The same applies to /dev/sd* and /dev/hd*

Doug, if I follow your remarks in comment #5 correctly, this bug should be
against either pam_console or RHEL-4 kernel, but not RHEL-3 kernel.  Is that
still your opinion?

I followed the recent thread (flamewar?) on the linux-scsi mailing list. 
Upstream seems to be working toward a per-device /sysfs-based solution to move
the policy decision about which SCSI commands are allowed by SG_IO ioctls out of
the kernel.  This is not anywhere near finished yet, though.  In the meantime,
the verify_command function in drivers/block/scsi_ioctl.c is already in the
RHEL-4 kernel.

So what exactly is the bug?

Chip

Relevant (old) thread here

http://groups-beta.google.com/group/linux.kernel/browse_frm/thread/5cfe44b11c8a99c5/ed58b3d4b1cfa39b?q=scsi_ioctl+firmware#ed58b3d4b1cfa39b

You could probably update pam to make cdwrite devices owned by root.cdburner,
create the new cdburner group, and make cdrecord sgid cdburner.

Created attachment 135302 [details]
Possible fix

OK, this might do the trick.  It's similar to my original patch, the main
difference being that it adds a switch (dev_type) statement and for the common
bootable device, TYPE_DISK, TYPE_WORM, TYPE_ROM, TYPE_MOD, it activates the
command filtering and in the event of a blocked opcode it warns exactly once
for each opcode (and if you are root it warns but then allows the opcode
anyway).  This way, non-bootable devices are not broken nor affected, bootable
device are protected plus we will have the ability to have people run things as
root to get past the limitations and should we need to train this, we will know
the opcode we need to train from the block messages.

I only worked on the sg file access because A) the block layer doesn't have
SG_IO ioctl capability and B) the scsi layers SCSI_IOCTL_SEND_CMD is root only,
so that only leaves the sg.c file as a possible point of weakness.

I still contend this has the possibility of breaking a *lot* of customer
setups.  Things like assigning raid device files to group wheel and making them
rw for wheel, then having admins run hardware raid management utilities without
being root, stuff like that.  But, at least by limiting the restrictions to
bootable devices we are limiting the overall amount of breakage.

Created attachment 135398 [details]
Slightly cleaned up and more readable version of same patch

This makes the previous patch slightly more readable and the logic in the
actual command tests a little easier to follow.  This is the patch I actually
submitted internally for review/possible inclusion in the kernel.  As noted in
my internal email, I expect this patch to break some setups.  For example, late
model Plextor CD burners will require the user to su to root before running
cdrecord or else they won't work.  Additionally, it's possible some raid device
management software will now need to be run as root where it used to work just
by giving administrators read/write permissions to the raid management device. 
However, there is nothing that should fail to work if the user has run the
command with root privileges.  So, in the event that breakage does occur, the
usage of sudo or something like console-helper to run the command as root will
work around the problem.

Product Management has reviewed and declined this request.  You may appeal this
decision by reopening this request. 

Indeed. I'll reopen this until either its fixed or someone fires me. Its a
disgrace and those involved should be embarrased.

I've filed bug 232096 against cdrecord and I'm moving this bug to be against
pam.  cdrecord can use consolehelper to let the local user execute the command
as root.  pam_console should not allow local users access to the /dev/sg devices.

I've marked bug 232096 as depending on this bug.

Can I do strange things to other SG devices with cdrecord being executed as root?
Can cdrecord be used to gain root access?

We will probably need to add a few patches to our cdrecord.  We've not fixed
flaws that could gain root in the past since it wasn't suid root.  I'll find the
list and note what we'll need in bug 232096.

AFAIK a user can't do anything out of the ordinary to non cd writers with cdrecord.

Can you make consolehelper start the program as sgid <something> instead of suid
root?  If so, you could still do the "make cdburner sg devices owned by group
cdburner and group writable" and use consolehelper to set the program to sgid
cdburner, which avoids any possible root compromise holes in cdrecord.

And no, cdrecord won't futz with other devices willingly (although I don't know
if it can be tricked).

Yes, this makes more sense.  Thanks Doug.

That's somewhat significantly more work, as you need something that tracks CD
burners and hotplug and knows to change the ownership to the CD groups - we
don't have any of that code (that code only exists for pam_console, and only
changes to the user). Unless I'm forgetting what pam_console can do.

cdrecord is designed to run setuid. not setgid. Its internal code is tested for
setuid not setgid cases and it handles setuidness internally and explicitly already.

Yeah root is the *wrong* user but group might need a tweak to cdrecord if unlucky. 

Doug is right that cdrecord won't futz with other devices, if it is in say
"setgid disk" category then it'll still only talk to CD devices. It's actually
very careful because on many OS's it has to run setuid root to do anything at all.

Note that when a command is run through consolehelper it is not 'setuid root' it
is run with both real and effective uids as root and this is certainly not what
we want. We would need to patch userhelper to add support for running program
with only effective id set to root.

Hmm, that won't work.  How hard is it going to be to set cdrecord to be setuid
root then, and restrict execution to only the local users?

Unfortunately consolehelper doesn't have this option. But there is another
option - add a PAM authentication patch to the (setuid root) cdrecord.

How much work would that be?  I've not done a lot of pam development.

Created attachment 150938 [details]
Simple PAM support for cdrecord

Actually it is pretty simple. The patch (untested) should provide that - it
will call PAM if the cdrecord is called with euid != uid && euid == 0.

Created attachment 150940 [details]
PAM configuration for cdrecord (/etc/pam.d/cdrecord)

DEV ACK. Pls lets make this one go away... correctly! Tomas Thanks!

One question - what about users who modified their console.perms - should there
be a %post script in the PAM package which would remove the /dev/cdwriter* from
the file using sed?

It can be potentially dangerous to mess with user modified files, also .rpmnew
file will be created so it is questionable whether it is necessary. On the other
hand the security problem will be unresolved if the /dev/cdwriter* is not
removed from console.perms.

As long as we note the change in the errata text, this won't be an issue.  We
should let users with modified console.perms files fix them if they're
interested in this flaw.

what about:
isoinfo
isodump
isovfy
isodebug
cdrdao
devdump
readcd
skel

Patch with pam and make them suid also?

cdrecord and cdrdao need cd writer write access.  all the others in your list
should be  fine with just read access (last I knew, they don't use raw scsi
commands to talk to the device, just block access).  So, being that they only
need read access, I don't know if you'll have to go all the way to suid root or
some such, or whether permissions could be set to simply allow read access for them.

readcd also can use /dev/sg stuff I believe.

The others just need read access via the normal device file

Yes but even for the read access to /dev/sg you have to make the readcd suid
because /dev/sg nodes cannot be readable for all.

Are cdrdao and readcd actually written so they drop priviledges if they are made
suid?

An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on the solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHSA-2007-0465.html