Bug#695504: eject: Avoid setuid to root for dmcrypt-get-device
Forgot to attach.
Thanks,
Bhushan
From: Bhushan Jain
Sent: Sunday, February 03, 2013 3:59 PM
To: Frank Lichtenheld; 695...@bugs.debian.org
Subject: RE: Bug#695504: eject: Avoid setuid to root for dmcrypt-get-device
Hi Frank,
Sorry for not looking into this earlier.
Attached is a new patch on latest eject-2.1.5+deb1+cvs20081104-13 version.
Please let me know if you find any issues with this one.
Thanks,
Bhushan
From: Frank Lichtenheld [fr...@lichtenheld.de]
Sent: Sunday, December 23, 2012 3:19 PM
To: Bhushan Jain; 695...@bugs.debian.org
Subject: Re: Bug#695504: eject: Avoid setuid to root for dmcrypt-get-device
Control: tags -1 = moreinfo
On Mon, Dec 10, 2012 at 12:16:46AM +, Bhushan Jain wrote:
Hi Frank,
Attached is the patch integrating dmcrypt-get-device functionality with code
in eject.c.
Please let me know if you have any comments or suggestions on the patch.
I have created this patch over the latest eject_2.1.5+deb1+cvs20081104-12
version.
Hi.
I finally managed to test your patch but it didn't work for me. Two reasons:
1) a simple coding error:
+ sprintf(filename,/sys/block/%s/dm/name,ent-d_name);
+ file = fopen(filename,r);
+ if(fgets(name, sizeof(name), file) == 0)
+ {
[...]
+ }
+ FCLOSE(file);
+/* Read the major:minor number for given device */
+ if(!strcmp(name,dev + devmapperlen))
This is never true. The string read by fgets (i.e. name) contains a trailing
newline.
dev does not. So the two strings never match.
+ {
2) but more importantly reading the major:minor from dev also doesn't work.
+ sprintf(filename,/sys/block/%s/dev,ent-d_name);
+ file = fopen(filename,r);
At least on my system this file always contains 254:0, not the major:minor of
the underlying device (which should be something like 8:XX usually). Does
this really work on your system?
The underlying device seems to be available in the /sys/block/dm-X/slaves/
folder though. Maybe we could read it from there?
Cheers,
Frank
eject-2.1.5+deb1+cvs20081104-13-no-dmcrypt-get-device.diff.gz
Description: eject-2.1.5+deb1+cvs20081104-13-no-dmcrypt-get-device.diff.gz
Bug#695504: eject: Avoid setuid to root for dmcrypt-get-device
Hi Frank,
Sorry for not looking into this earlier.
Attached is a new patch on latest eject-2.1.5+deb1+cvs20081104-13 version.
Please let me know if you find any issues with this one.
Thanks,
Bhushan
From: Frank Lichtenheld [fr...@lichtenheld.de]
Sent: Sunday, December 23, 2012 3:19 PM
To: Bhushan Jain; 695...@bugs.debian.org
Subject: Re: Bug#695504: eject: Avoid setuid to root for dmcrypt-get-device
Control: tags -1 = moreinfo
On Mon, Dec 10, 2012 at 12:16:46AM +, Bhushan Jain wrote:
Hi Frank,
Attached is the patch integrating dmcrypt-get-device functionality with code
in eject.c.
Please let me know if you have any comments or suggestions on the patch.
I have created this patch over the latest eject_2.1.5+deb1+cvs20081104-12
version.
Hi.
I finally managed to test your patch but it didn't work for me. Two reasons:
1) a simple coding error:
+ sprintf(filename,/sys/block/%s/dm/name,ent-d_name);
+ file = fopen(filename,r);
+ if(fgets(name, sizeof(name), file) == 0)
+ {
[...]
+ }
+ FCLOSE(file);
+/* Read the major:minor number for given device */
+ if(!strcmp(name,dev + devmapperlen))
This is never true. The string read by fgets (i.e. name) contains a trailing
newline.
dev does not. So the two strings never match.
+ {
2) but more importantly reading the major:minor from dev also doesn't work.
+ sprintf(filename,/sys/block/%s/dev,ent-d_name);
+ file = fopen(filename,r);
At least on my system this file always contains 254:0, not the major:minor of
the underlying device (which should be something like 8:XX usually). Does
this really work on your system?
The underlying device seems to be available in the /sys/block/dm-X/slaves/
folder though. Maybe we could read it from there?
Cheers,
Frank
--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#695504: eject: Avoid setuid to root for dmcrypt-get-device
Control: tags -1 = moreinfo
On Mon, Dec 10, 2012 at 12:16:46AM +, Bhushan Jain wrote:
Hi Frank,
Attached is the patch integrating dmcrypt-get-device functionality with code
in eject.c.
Please let me know if you have any comments or suggestions on the patch.
I have created this patch over the latest eject_2.1.5+deb1+cvs20081104-12
version.
Hi.
I finally managed to test your patch but it didn't work for me. Two reasons:
1) a simple coding error:
+ sprintf(filename,/sys/block/%s/dm/name,ent-d_name);
+ file = fopen(filename,r);
+ if(fgets(name, sizeof(name), file) == 0)
+ {
[...]
+ }
+ FCLOSE(file);
+/* Read the major:minor number for given device */
+ if(!strcmp(name,dev + devmapperlen))
This is never true. The string read by fgets (i.e. name) contains a trailing
newline.
dev does not. So the two strings never match.
+ {
2) but more importantly reading the major:minor from dev also doesn't work.
+ sprintf(filename,/sys/block/%s/dev,ent-d_name);
+ file = fopen(filename,r);
At least on my system this file always contains 254:0, not the major:minor of
the underlying device (which should be something like 8:XX usually). Does
this really work on your system?
The underlying device seems to be available in the /sys/block/dm-X/slaves/
folder though. Maybe we could read it from there?
Cheers,
Frank
--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#695504: eject: Avoid setuid to root for dmcrypt-get-device
2012/12/10 Bhushan Jain bpj...@cs.stonybrook.edu Hi Frank, Attached is the patch integrating dmcrypt-get-device functionality with code in eject.c. Please let me know if you have any comments or suggestions on the patch. I have created this patch over the latest eject_2.1.5+deb1+cvs20081104-12 version. Thanks, looks good. I will try to find some time to test it in the next days. Cheers, Frank -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#695504: eject: Avoid setuid to root for dmcrypt-get-device
Package: eject
Version: 2.1.5+deb1+cvs20081104-11
Severity: wishlist
Dear Maintainer,
I noticed that the only reason dmcrypt-get-device (from eject package) needs
setuid privilege is to read the major:minor numbers (unless I have missed
something).
A lot of distributions are trying to avoid use of the setuid bit because it can
potentially introduce a privilege escalation attack vector.
I think the same thing could be accomplished by reading the major:minor device
numbers through a sys file, and then eliminate the need for dmcrypt-get-device
to be setuid-to-root.
The major:minor numbers are available in the file /sys/block/dm-*/dev and the
corresponding device name can be confirmed from file /sys/block/dm-*/dm/name.
Martin Pitt - the author of dmcrypt-get-device.c - suggested that I should send
the patch here and you could help integrate and comment on the patch.
Attached is the patch for dmcrypt-get-device.c.
-- System Information:
Debian Release: wheezy/sid
APT prefers quantal-updates
APT policy: (500, 'quantal-updates'), (500, 'quantal-security'), (500,
'quantal'), (100, 'quantal-backports')
Architecture: i386 (i686)
Kernel: Linux 3.5.0-17-generic (SMP w/1 CPU core)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Versions of packages eject depends on:
ii libc6 2.15-0ubuntu20
ii libdevmapper1.02.1 2:1.02.74-4ubuntu1
eject recommends no packages.
Versions of packages eject suggests:
pn cdtool none
pn setcd none
-- no debconf information
--- original-dmcrypt-get-device.c 2012-12-09 05:29:50.792388997 -0500
+++ dmcrypt-get-device.c 2012-12-09 05:38:00.512388998 -0500
@@ -15,20 +15,22 @@
* parsing is done with normal user privileges afterwards.
*/
-#include libdevmapper.h
#include stdio.h
#include string.h
#include sys/stat.h
#include unistd.h
+#include dirent.h
+#include stdlib.h
+#include linux/dm-ioctl.h
int
main (int argc, char** argv)
{
-struct dm_task *dmt = NULL;
-struct dm_info dmi;
-char *target_type = NULL, *params = NULL;
-uint64_t start, length;
-void *next = NULL;
+DIR *dir;
+FILE *file;
+struct dirent *ent;
+char filename[4096];
+char name[DM_NAME_LEN];
struct stat st;
unsigned major, minor;
@@ -49,37 +51,35 @@
if (stat (argv[1], st) || !S_ISBLK(st.st_mode))
return 1;
-/* Request device info */
-if (!(dmt = dm_task_create(DM_DEVICE_TABLE)))
-return 1;
-if (!dm_task_set_name(dmt, argv[1]))
-return 1;
-if (!dm_task_run(dmt))
-return 1;
-
-/* Drop all privileges */
-setgid(getgid());
-setuid(getuid());
-
-if (!dm_task_get_info(dmt, dmi))
-return 1;
-
-/* Get underlying physical device */
-next = dm_get_next_target(dmt, next, start, length,
-target_type, params);
-
-/* verify validity and that it is a dmcrypt device */
-if (!target_type || strcmp(target_type, crypt) || next)
-return 1;
-
-/* params has the format: cipher key offset major:minor unknown */
-length = sscanf(params, %*s %*s %*s %u:%u %*s, major, minor);
-if (length != 2)
-return 1;
+if ((dir = opendir(/sys/block)) == NULL)
+{
+ perror(Unable to open directory);
+ return 1;
+}
-/* Success */
-printf (%u:%u\n, major, minor);
-
-return 0;
+while ((ent = readdir(dir)) != NULL)
+{
+const char* dmname = dm-;
+/* Check for the device name */
+ if(!strncmp(ent-d_name, dmname, strlen(dmname)))
+ {
+ sprintf(filename,/sys/block/%s/dm/name,ent-d_name);
+ file = fopen(filename,r);
+ fscanf(file,%s,name);
+ fclose(file);
+/* Read the major:minor number for given device */
+ if(!strcmp(name,argv[1] + devmapdirlen))
+ {
+ sprintf(filename,/sys/block/%s/dev,ent-d_name);
+ file = fopen(filename,r);
+ fscanf(file,%u:%u,major,minor);
+ /* Success */
+printf (%u:%u\n, major, minor);
+ fclose(file);
+return 0;
+ }
+ }
+}
+/* Not found */
+return 1;
}
-
Bug#695504: eject: Avoid setuid to root for dmcrypt-get-device
2012/12/9 Bhushan Jain bpj...@cs.stonybrook.edu Dear Maintainer, I noticed that the only reason dmcrypt-get-device (from eject package) needs setuid privilege is to read the major:minor numbers (unless I have missed something). A lot of distributions are trying to avoid use of the setuid bit because it can potentially introduce a privilege escalation attack vector. I think the same thing could be accomplished by reading the major:minor device numbers through a sys file, and then eliminate the need for dmcrypt-get-device to be setuid-to-root. The major:minor numbers are available in the file /sys/block/dm-*/dev and the corresponding device name can be confirmed from file /sys/block/dm-*/dm/name. Martin Pitt - the author of dmcrypt-get-device.c - suggested that I should send the patch here and you could help integrate and comment on the patch. Attached is the patch for dmcrypt-get-device.c. Thanks, looks reasonable (although I don't have a setup currently to test it). Questions and comments: * Since the only reason this code is in a separate binary is the setuid bit, the code should probably be integrated directly into eject. Would you be willing to extend your patch to do that? * Do you know if that code is dependent on a recent kernel version or if it works for older ones, too? (e.g. Debian stable) Cheers, -- Frank Lichtenheld dj...@debian.org -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#695504: eject: Avoid setuid to root for dmcrypt-get-device
From: flichtenhel...@gmail.com [flichtenhel...@gmail.com] on behalf of Frank Lichtenheld [dj...@debian.org] Sent: Sunday, December 09, 2012 9:16 AM To: Bhushan Jain; 695...@bugs.debian.org Subject: Re: Bug#695504: eject: Avoid setuid to root for dmcrypt-get-device 2012/12/9 Bhushan Jain bpj...@cs.stonybrook.edu Dear Maintainer, I noticed that the only reason dmcrypt-get-device (from eject package) needs setuid privilege is to read the major:minor numbers (unless I have missed something). A lot of distributions are trying to avoid use of the setuid bit because it can potentially introduce a privilege escalation attack vector. I think the same thing could be accomplished by reading the major:minor device numbers through a sys file, and then eliminate the need for dmcrypt-get-device to be setuid-to-root. The major:minor numbers are available in the file /sys/block/dm-*/dev and the corresponding device name can be confirmed from file /sys/block/dm-*/dm/name. Martin Pitt - the author of dmcrypt-get-device.c - suggested that I should send the patch here and you could help integrate and comment on the patch. Attached is the patch for dmcrypt-get-device.c. Thanks, looks reasonable (although I don't have a setup currently to test it). Questions and comments: * Since the only reason this code is in a separate binary is the setuid bit, the code should probably be integrated directly into eject. Would you be willing to extend your patch to do that? Sure. I will send a patch for eject package soon. * Do you know if that code is dependent on a recent kernel version or if it works for older ones, too? (e.g. Debian stable) It is not dependent on recent kernel version. Infact, I have confirmed that it works on kernel version 2.6.35-22 as well. Cheers, -- Frank Lichtenheld dj...@debian.org Thanks, Bhushan Jain -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#695504: eject: Avoid setuid to root for dmcrypt-get-device
Hi Frank, Attached is the patch integrating dmcrypt-get-device functionality with code in eject.c. Please let me know if you have any comments or suggestions on the patch. I have created this patch over the latest eject_2.1.5+deb1+cvs20081104-12 version. Thanks, Bhushan Jain From: Bhushan Jain Sent: Sunday, December 09, 2012 4:51 PM To: Frank Lichtenheld; 695...@bugs.debian.org Subject: RE: Bug#695504: eject: Avoid setuid to root for dmcrypt-get-device From: flichtenhel...@gmail.com [flichtenhel...@gmail.com] on behalf of Frank Lichtenheld [dj...@debian.org] Sent: Sunday, December 09, 2012 9:16 AM To: Bhushan Jain; 695...@bugs.debian.org Subject: Re: Bug#695504: eject: Avoid setuid to root for dmcrypt-get-device 2012/12/9 Bhushan Jain bpj...@cs.stonybrook.edu Dear Maintainer, I noticed that the only reason dmcrypt-get-device (from eject package) needs setuid privilege is to read the major:minor numbers (unless I have missed something). A lot of distributions are trying to avoid use of the setuid bit because it can potentially introduce a privilege escalation attack vector. I think the same thing could be accomplished by reading the major:minor device numbers through a sys file, and then eliminate the need for dmcrypt-get-device to be setuid-to-root. The major:minor numbers are available in the file /sys/block/dm-*/dev and the corresponding device name can be confirmed from file /sys/block/dm-*/dm/name. Martin Pitt - the author of dmcrypt-get-device.c - suggested that I should send the patch here and you could help integrate and comment on the patch. Attached is the patch for dmcrypt-get-device.c. Thanks, looks reasonable (although I don't have a setup currently to test it). Questions and comments: * Since the only reason this code is in a separate binary is the setuid bit, the code should probably be integrated directly into eject. Would you be willing to extend your patch to do that? Sure. I will send a patch for eject package soon. * Do you know if that code is dependent on a recent kernel version or if it works for older ones, too? (e.g. Debian stable) It is not dependent on recent kernel version. Infact, I have confirmed that it works on kernel version 2.6.35-22 as well. Cheers, -- Frank Lichtenheld dj...@debian.org Thanks, Bhushan Jain eject_2.1.5+deb1+cvs20081104-12-no-dmcrypt-get-device.diff.gz Description: eject_2.1.5+deb1+cvs20081104-12-no-dmcrypt-get-device.diff.gz
