FS#22337 - [udev] bad usb key group

Attached to Project: Arch Linux
Opened by Sébastien Luttringer (seblu) - Thursday, 06 January 2011, 01:31 GMT
Last edited by Tom Gundersen (tomegun) - Tuesday, 30 August 2011, 23:25 GMT
Task Type Bug Report
Category Packages: Core
Status Closed
Assigned To Tom Gundersen (tomegun)
Architecture All
Severity Medium
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 1
Private No

Details

If i understand right, every usb key should be right/write by group storage. cf: https://wiki.archlinux.org/index.php/Group.

One of my usb key, kingston data have the bad group.

$ ll /dev/disk/by-id
lrwxrwxrwx 1 root root 9 2011-01-06 01:50 usb-Kingston_DataTraveler_2.0_5B870F0000D4-0:0 -> ../../sdb
lrwxrwxrwx 1 root root 10 2011-01-06 01:50 usb-Kingston_DataTraveler_2.0_5B870F0000D4-0:0-part1 -> ../../sdb1
$ ll /dev/sdb*
brw-rw---- 1 root disk 8, 16 2011-01-06 01:50 /dev/sdb
brw-rw---- 1 root disk 8, 17 2011-01-06 01:50 /dev/sdb1

So, rights is set by udev rule file /lib/udev/rules.d/81-arch.rules which match on scsi_level to select usb key

$ grep 'GROUP="storage"' 81-arch.rules
KERNEL=="sd*", ATTRS{scsi_level}=="3", ATTRS{type}=="0", GROUP="storage"
KERNEL=="sd*", ATTRS{scsi_level}=="5", GROUP="storage"
KERNEL=="sd*", ATTRS{scsi_level}=="3", ATTRS{type}=="7", GROUP="storage"
KERNEL=="hd*", ENV{ID_TYPE}=="generic", GROUP="storage"

but this key seems to have scsi_level =0. has you can see with lssci or via sysfs.

$ lsscsi -l 12:0:0:0
[12:0:0:0] disk Kingston DataTraveler 2.0 PMAP /dev/sdb
state=running queue_depth=1 scsi_level=0 type=0 device_blocked=0 timeout=30

We need to patch this rule file to better match usb key. My first plan was to add my scsi_level in file but, i look how debian handle do and to be honnest, it's really more sexy. In debian usb key have group floppy.

$ grep 'GROUP="floppy"' /lib/udev/rules.d/*
91-permissions.rules:SUBSYSTEM=="block", ATTRS{removable}=="1", GROUP="floppy"
91-permissions.rules:SUBSYSTEM=="block", SUBSYSTEMS=="usb|ieee1394|mmc|pcmcia", GROUP="floppy"
91-permissions.rules:KERNEL=="cbm", GROUP="floppy"

So, I propose to doing the same matching.
This task depends upon

Closed by  Tom Gundersen (tomegun)
Tuesday, 30 August 2011, 23:25 GMT
Reason for closing:  Won't fix
Comment by Matthias Dienstbier (fs4000) - Thursday, 06 January 2011, 04:25 GMT
I also noticed that, some but not all USB mass storage devices have scsi_level 0 and thus the group disk instead of storage. As far as I remember some time in the past (more than a year ago) everything was ok.
Comment by Sébastien Luttringer (seblu) - Tuesday, 25 January 2011, 15:20 GMT
this is a patch which fix issue for my kingston key.
   patch.v1 (0.4 KiB)
Comment by Sébastien Luttringer (seblu) - Tuesday, 14 June 2011, 20:25 GMT
this is a patch against current 81-arch.rules. It just add a better way of detection but a smarter cleaning of this file should be done in future.
Comment by Tom Gundersen (tomegun) - Saturday, 18 June 2011, 17:33 GMT
I have been intending to look into this more closely. We should not be doing this (all the permission things in 81-arch.rules). If it is a valid thing to do, then we should push it upstream. If we cannot push it upstream then it should just go away.

I don't know enough to do anything about this atm though, I will try to study it to get a better understanding.
Comment by Tom Gundersen (tomegun) - Saturday, 18 June 2011, 17:40 GMT
I had a look at the link <https://wiki.archlinux.org/index.php/Group>, and I think the definition of "storage" means that probably that group should just be removed. If I understand the udev guys correctly, it is simply not possible to reliably classify removable devices from non-removable ones.

Again, I need to look into the "correct" way to do this.
Comment by Sébastien Luttringer (seblu) - Saturday, 18 June 2011, 17:52 GMT
Not everything can be upstream, especially things like user/group which is not common to distro.

Maybe you're right about concept of storage group, but we need to keep in mind, some users needs to mount opticale and/or removable devices without be root.

About classification of removable devices i don't understand, kernel export un parameter via sysfs to specify about removability as you can see in rules:
+SUBSYSTEM=="block", ATTRS{removable}=="1", GROUP="storage"

I look into debian before sending this patch and they consider storage as floppy.


Comment by Tom Gundersen (tomegun) - Saturday, 18 June 2011, 17:55 GMT
The correct way to deal with this is to use udisks, we should remove all the "storage" stuff from our udev package (in the long run at least).
Comment by Tom Gundersen (tomegun) - Saturday, 18 June 2011, 18:05 GMT
@seblu: Sorry, I didn't read your reply before posting, so here comes a more detailed answer:

I agree we need to keep our custom groups or group renaming (which is done in the PKGBUILD file). So we will always rename "tape" to "storage", but I don't think it is correct to add these devices to the storage group.

The "removable" attribute, is not reliable as far as I understand. If it _is_ reliable this should be taken advantage of by udev upstream (there was just a discussion on the udev ml about something related to this, and the conclusion was AFAICT that classifying this is impossible in general).

Any reason that users can not use udisks? If I understand correctly, this is exactly the problem udisks is meant to solve.

PS: in my opinion suse is the distro to look to for how to deal with udev, they have a very KISS approach that i like a lot.
Comment by Sébastien Luttringer (seblu) - Sunday, 19 June 2011, 00:42 GMT
udisks cannot be a solution. it's a tools which can be used to mount partition of devices (a great successor to pmount imho). But have rights on /dev files is not just to be able to mount devices.

For example, you plug an usb key and you want to partition it. fdisk or gdisk, not udisks.
Another one, i plug and external disk (eSATA) and i directly run virtualbox on it.
I often do this two things.


To resume my current point of view. There is
disk - Which are devices where you should not have write access as user (security reason). This is like given a root access to my system fs.
storage - external plugged storage devices ("power" user and me want to be able to write on usb key / external sata quietly)
optical - external plugged cdrom/dvd/blueray disk (i want to be able to write on my cd quietly)
floppy - external floppy disk (i want to be able to write on my floppy quietly)

I remember that pmount make a difference between "removable" and not removable devices with a poor pattern matching on devices name.
udisks have in its rights database the 2 actions

$ pkaction|grep udisks|grep -- -mount
org.freedesktop.udisks.filesystem-mount
org.freedesktop.udisks.filesystem-mount-system-internal

do udisks make difference between system internal (our disk group) and mount (storage group). I didn't look into the code, but they probably have a way to check this.

about reliability of sysfs "removable", as far i know, it's a hint like many others to know if a devices is removable. But I'm technically very light, saying it.

I will look how suse do. Thanks.
Comment by Tom Gundersen (tomegun) - Wednesday, 17 August 2011, 01:53 GMT
@Seblu: I'm interested to merge something like your patch. I'm happy with relying on the "removable" attribute to be correct. However, I think the rules you proposed may be too vague. That is, there is stuff which is removable which should not be in the storage group, e.g., cdrom's. Any thoughts?

PS
Check out the newest 81-arch.rules from svn, it is now much smaller than before.
Comment by Sébastien Luttringer (seblu) - Saturday, 27 August 2011, 17:54 GMT
@tom,

i tested with your last version of 81 (http://projects.archlinux.org/svntogit/packages.git/tree/udev/trunk/81-arch.rules) and your new 20 version

usb key are no more in storage group when you set storage with 20. I tried with 80 and it was fine. So 20 is a little too low.

About removable arg, i must need more test with eSATA, i believe it doesn't work.
Comment by Tom Gundersen (tomegun) - Tuesday, 30 August 2011, 23:24 GMT
After some more discussion on ML / irc, I was convinced that the "storage" group should go away. I will not remove the group altogeher (at least not for the time being), but the rules that assigned devices to this group will be removed from the next udev release. This is in line with the recommendations from upstream.

The main reason is security, by giving group access to devices you would allow remote users accessing local devices, which the local user might not expect. In general, there should never be any reason to put users in groups to access hardware, tools such as policykit, consolekit and udisks should hand out the required permissions when needed.

The original use-case from seblu was to be able to partition and format devices such as usb keys. I just installed gnome-disk-utility to have a look at how this works, and I can confirm that I could both partition and format my SD card, so this should not be a problem.

I'll close as won't fix, and remove all the related rules in the next udev release.

Thanks for taking the time to report anyway, and for helping me figure out what should be done.

Loading...