Bug 130290 - fstab-sync makes all entries suid,dev
fstab-sync makes all entries suid,dev
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: hal (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Ray Strode [halfline]
: Security
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2004-08-18 16:47 EDT by Pekka Pietikäinen
Modified: 2007-11-30 17:10 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2004-08-19 16:23:07 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
lshal output (57.79 KB, text/plain)
2004-08-19 06:03 EDT, Pekka Pietikäinen
no flags Details
tree /sys (53.76 KB, text/plain)
2004-08-19 06:04 EDT, Pekka Pietikäinen
no flags Details

  None (edit)
Description Pekka Pietikäinen 2004-08-18 16:47:47 EDT
Description of problem:

Enter magic floppy/cdrom containing ext2 filesystem and some harmless
executables:

[pp@the ~]$ mount /media/floppy/
[pp@the ~]$ cd /media/floppy/
[pp@the floppy]$ ls -l
total 533
drwx------  2 root root  12288 Aug 18 23:29 lost+found
-rwsr-xr-x  1 root root 529492 Aug 18 23:33 sh
[pp@the floppy]$ ./sh
Stand-alone shell (version 3.7)
> touch /0wned
> ls -l /0wned
-rw-rw-r--  1 root pp 0 Aug 18 23:43 /0wned
>

Looking at fstab-sync.c there seem to be quite a few alternatives, all
of them containing dev,suid. Not good (tm).
Comment 1 John (J5) Palmieri 2004-08-18 17:11:52 EDT
Reasigned to Ray Strode and cc'ed David.  This should go upstream.
Comment 2 Bill Nottingham 2004-08-18 17:46:28 EDT
Hm. This is more or less in line with previous behavior, though.
Comment 3 John (J5) Palmieri 2004-08-18 18:03:00 EDT
I'm patching hal right now to take out the suid and dev flags as a
stopgap solution.  Should I go ahead with this?
Comment 4 David Zeuthen 2004-08-18 18:07:56 EDT
Makes sense, I'll fix it upstream. Thanks.
Comment 5 Bill Nottingham 2004-08-18 18:11:10 EDT
Well, if you just take out suid/dev, it should revert to the defaults.

Which is 'suid,dev'. :)

Comment 6 John (J5) Palmieri 2004-08-18 18:17:50 EDT
They are all added with "user" which implies "nosuid" and "nodev".  At
least that is what it says in the mount man page.
Comment 7 Bill Nottingham 2004-08-18 23:35:15 EDT
Carry on, don't mind me then. :)

Any particular reason you're using 'user' and not 'owner'?
Comment 8 John (J5) Palmieri 2004-08-19 00:22:14 EDT
What are the implications for dynamicly added devices?  As I
understand it console permissions are only set when a user logs in. 
What happens when udev creates a device node after the user has logged
in?  Otherwise owner seems like the correct flag to use here.
Comment 9 Bill Nottingham 2004-08-19 00:26:58 EDT
udev has/had a helper to do this. I suppose I should make sure it's
still there and working.
Comment 10 David Zeuthen 2004-08-19 05:29:35 EDT
We don't want to use owner because then we'll need to give permission
to the device node and that gives the user the ability to read or
destroy all data through the device node (if the partition has an ext3
filesystem some files may be unreadable for the user if mounted;
that's a feature). 

We do, however, want to get the console user to own the device node
when it represents an optical drive (so he can eject(1) it and burn
discs with to it). That's bug #130094 and bug #130095.
Comment 11 Pekka Pietikäinen 2004-08-19 05:49:04 EDT
Indeed. In fact, with the current stuff entries for idedisk
and scsidisk were created for partitions which are unmounted
more or less on purpose (in my case an old /boot partition 
I don't use for anything, and idedisk is a LVM thingy that isn't
mountable). Plugging in a digital camera (basically a USB card reader,
/dev/sdb) didn't cause an entry to be added at all

I think the stuff should be pretty conservative on what devices
to create fstab entries for by default and if the admin wants
something else then that should be enabled manually. 
Comment 12 David Zeuthen 2004-08-19 05:59:59 EDT
Right, /boot should probably be blacklisted and fstab-sync shouldn't
add entries for devices with no filesystem on; we have a filesystem
prober in hal, so the properties volumes.fstype and
volume.is_filesystem should be honoured. I'll be fixing all these
issues today in upstream (I'm the upstream maintainer) and hope to
push out a new package tonight.

Btw, care to attach the output of lshal and 'tree /sys' for your
system, just so I can doublecheck everything is set correctly for LVM
systems (I haven't tested on those yet).

Thanks,
David
Comment 13 Pekka Pietikäinen 2004-08-19 06:03:35 EDT
Created attachment 102871 [details]
lshal output
Comment 14 Pekka Pietikäinen 2004-08-19 06:04:08 EDT
Created attachment 102872 [details]
tree /sys
Comment 15 David Zeuthen 2004-08-19 06:23:47 EDT
Thanks, that's very useful. 

> Plugging in a digital camera (basically a USB card reader,
> (/dev/sdb) didn't cause an entry to be added at all

That's a bug, this should really work. We even try to look at the main
block device (/dev/sdb) for the odd media that doesn't have partition
tables. Does it hal detect it properly, can you see it in
hal-device-manager? Please attach the relevant /sys and lshal snippets.

Much thanks,
David

Comment 16 Joe Orton 2004-08-19 09:33:49 EDT
Any objections to clearing the "Security Sensitive" flag on this bug
so it can be viewed publically?  Pekka?
Comment 17 Pekka Pietikäinen 2004-08-19 09:35:53 EDT
No problem here, the fixed version got reported on the rawhide report
anyway so the problem is "out" :-)
Comment 18 Joe Orton 2004-08-19 09:38:25 EDT
OK, thanks.
Comment 19 Pekka Pietikäinen 2004-08-19 11:23:17 EDT
Camera problem reported as new bug #130355. hal-device-manager sees
the camera just fine.
Comment 20 Bill Nottingham 2004-08-19 15:21:34 EDT
Are you sure you just want to restrict 'owner' to a optical drive?

Seems flash such as a USB key or camera media may also want this flag.
Comment 21 David Zeuthen 2004-08-19 15:37:11 EDT
No no, I'm saying don't use 'owner' at all, always use 'user'. Why
should we use 'owner' at all?
Comment 22 David Zeuthen 2004-08-19 16:23:07 EDT
I'm closing this bug as this is fixed both upstream and in Rawhide.
Thanks.

Note You need to log in before you can comment on or make changes to this bug.