Bug#700422: wdm shouldn't use /dev/mem
control: tag -1 +pending
Hi, Borislav
On Tue, Feb 12, 2013 at 07:39:19PM +0100, Borislav Petkov wrote:
> Ok, thanks. I've tested it lightly here by building a debian package
> using debian/rules and applying the patch by hand (couldn't get it to
> apply the patch automatically with "debian/rules patch" because the
> patch target was missing there... yadda yadda) ...
In wdm, changes inside debian/ dir are done directly and changes involving
upstream sources (everything outside debian/) are done through quilt
patches. They are automatically applied by dpkg-build* scripts on package
build and can be {un,}applied on demand by quilt pop/push (some tweaks may
be needed first). I am attaching a patch with the commit that included your
changes in my personal git repo, part as a normal change and part as a new
quilt patch.
> ... long story short, ping me if there's an official package available
> so that I can test it too, before it enters the distro repos.
I have prepared a personal package and put it in my personal Debian repo
under http://people.debian.org/~agmartin/debian-store/misc. Changes file is
signed with my Debian gpg key, available from the Debian keyring.
That repo is reachable through apt line
deb http://people.debian.org/~agmartin/debian-store/misc/ ./
and is also signed with my debian gpg key. Note that there is other stuff
there (mostly personal backports for other packages).
I am currently testing that package in this box and seems to work well in
the normal use needed here, so I will soon commit my changes to the wdm
collab-maint git repo.
In the meantime I am tagging this bug as pending.
Thanks a lot for your feedback.
Regards,
--
Agustin
>From dfc7f1d4fcba1f02d2d4fe8506b4366ad697e078 Mon Sep 17 00:00:00 2001
From: Agustin Martin Domingo <agmartin@debian.org>
Date: Tue, 12 Feb 2013 18:33:14 +0100
Subject: [PATCH] 08_do_not_use_dev_mem.patch: Use /dev/urandom instead of
/dev/mem (#700422)
Patch by Borislav Petkov:
wdm still uses /dev/mem in genauth.c to generate a tmp key and it
shouldn't. The kernel currently allows userspace to read < 640K of
/dev/mem for compatibility reasons with X. The modern way of getting
two random longs is /dev/urandom and I've a patch below which
converts wdm to do that.
---
debian/man/wdm.1x | 2 +-
debian/patches/08_do_not_use_dev_mem.patch | 108 ++++++++++++++++++++++++++++
debian/patches/series | 1 +
3 files changed, 110 insertions(+), 1 deletion(-)
create mode 100644 debian/patches/08_do_not_use_dev_mem.patch
diff --git a/debian/man/wdm.1x b/debian/man/wdm.1x
index 5f468d5..968acf2 100644
--- a/debian/man/wdm.1x
+++ b/debian/man/wdm.1x
@@ -202,7 +202,7 @@ to pass on to the \fIXsetup\fP,
.IP \fBDisplayManager.randomFile\fP
A file to checksum to generate the seed of authorization keys.
This should be a file that changes frequently.
-The default is \fI/dev/mem\fP.
+The default is \fI/dev/urandom\fP.
.IP \fBDisplayManager.greeterLib\fP
On systems that support a dynamically-loadable greeter library, the
name of the library. The default is
diff --git a/debian/patches/08_do_not_use_dev_mem.patch b/debian/patches/08_do_not_use_dev_mem.patch
new file mode 100644
index 0000000..489bc54
--- /dev/null
+++ b/debian/patches/08_do_not_use_dev_mem.patch
@@ -0,0 +1,108 @@
+Author: Borislav Petkov <bp@alien8.de>
+Description: Use /dev/urandom instead of /dev/mem
+Bug-Debian: http://bugs.debian.org/700422
+
+Borislav Petkov:
+
+wdm still uses /dev/mem in genauth.c to generate a tmp key and it
+shouldn't. The kernel currently allows userspace to read < 640K of
+/dev/mem for compatibility reasons with X. The modern way of getting
+two random longs is /dev/urandom and I've a patch below which
+converts wdm to do that.
+
+Index: wdm/doc/wdm.man.in
+===================================================================
+--- wdm.orig/doc/wdm.man.in 2013-02-12 18:26:27.879201302 +0100
++++ wdm/doc/wdm.man.in 2013-02-12 18:26:27.863202561 +0100
+@@ -202,7 +202,7 @@
+ .IP \fBDisplayManager.randomFile\fP
+ A file to checksum to generate the seed of authorization keys.
+ This should be a file that changes frequently.
+-The default is \fI/dev/mem\fP.
++The default is \fI/dev/urandom\fP.
+ .IP \fBDisplayManager.greeterLib\fP
+ On systems that support a dynamically-loadable greeter library, the
+ name of the library. The default is
+Index: wdm/src/wdm/genauth.c
+===================================================================
+--- wdm.orig/src/wdm/genauth.c 2013-02-12 18:26:27.879201302 +0100
++++ wdm/src/wdm/genauth.c 2013-02-12 18:26:27.875201665 +0100
+@@ -71,37 +71,26 @@
+
+ #if !defined(ARC4_RANDOM) && !defined(DEV_RANDOM)
+ static int
+-sumFile (char *name, long sum[2])
++sumFile (char *name, long sum[], unsigned n)
+ {
+- long buf[1024*2];
+ int cnt;
+ int fd;
+- int loops;
+- int reads;
+- int i;
+- int ret_status = 0;
++ int ret_status = 1;
+
+ fd = open (name, O_RDONLY);
+ if (fd < 0) {
+ WDMError("Cannot open randomFile \"%s\", errno = %d\n", name, errno);
+ return 0;
+ }
+-#ifdef FRAGILE_DEV_MEM
+- if (strcmp(name, "/dev/mem") == 0) lseek (fd, (off_t) 0x100000, SEEK_SET);
+-#endif
+- reads = FILE_LIMIT;
+- sum[0] = 0;
+- sum[1] = 0;
+- while ((cnt = read (fd, (char *) buf, sizeof (buf))) > 0 && --reads > 0) {
+- loops = cnt / (2 * sizeof (long));
+- for (i = 0; i < loops; i+= 2) {
+- sum[0] += buf[i];
+- sum[1] += buf[i+1];
+- ret_status = 1;
+- }
+- }
+- if (cnt < 0)
++
++ memset(sum, 0, n);
++
++ cnt = read(fd, (char *) sum, sizeof(long) * n);
++ if (cnt < 0) {
+ WDMError("Cannot read randomFile \"%s\", errno = %d\n", name, errno);
++ ret_status = 0;
++ }
++
+ close (fd);
+ return ret_status;
+ }
+@@ -139,7 +128,7 @@
+ long sum[2];
+ unsigned char tmpkey[8];
+
+- if (!sumFile (randomFile, sum)) {
++ if (!sumFile (randomFile, sum, 2)) {
+ sum[0] = time ((Time_t *) 0);
+ sum[1] = time ((Time_t *) 0);
+ }
+@@ -244,7 +233,7 @@
+ localkey[0] = 1;
+ }
+ #else
+- if (!sumFile (randomFile, localkey)) {
++ if (!sumFile (randomFile, localkey, 2)) {
+ localkey[0] = 1; /* To keep from continually calling sumFile() */
+ }
+ #endif
+Index: wdm/src/wdm/resource.c
+===================================================================
+--- wdm.orig/src/wdm/resource.c 2013-02-12 18:26:27.879201302 +0100
++++ wdm/src/wdm/resource.c 2013-02-12 18:26:27.875201665 +0100
+@@ -156,7 +156,7 @@
+ #define DEF_ACCESS_FILE_PL ""
+ #endif
+ #ifndef DEF_RANDOM_FILE
+-#define DEF_RANDOM_FILE "/dev/mem"
++#define DEF_RANDOM_FILE "/dev/urandom"
+ #endif
+ #ifndef DEF_GREETER_LIB
+ #define DEF_GREETER_LIB "/X11/lib/X11/xdm/libXdmGreet.so"
diff --git a/debian/patches/series b/debian/patches/series
index 44218a4..e2dd557 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -5,3 +5,4 @@
05_french_translation_name.patch
06_login_composite.patch
07_ld_no_add_needed.patch
+08_do_not_use_dev_mem.patch
--
1.7.10.4
Reply to: