Ensure second and subsequent auth cookies are random. Spotted by George

2007-11-30  Ray Strode  <rstrode@redhat.com>

	Ensure second and subsequent auth cookies are
	random.  Spotted by George Lebl.

	* common/gdm-common.[ch] (gdm_generate_random_bytes):
	open /dev/urandom and read size bytes from it.
	(_fd_is_character_device): check if fd points to char
        device
	(_read_bytes): the usual loop until all bytes are read
	helper function for read().

	* daemon/gdm-display-access-file.c
	(generate_random_bytes): remove function
	(gdm_display_access_file_add_display):
	use gdm_generate_random_bytes instead of
	_generate_random_bytes

svn path=/trunk/; revision=5534

4 files changed, 138 insertions, 21 deletions

diff --git a/ChangeLog b/ChangeLog
index 07254eab..1228910a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,21 @@
+2007-11-30  Ray Strode  <rstrode@redhat.com>
+
+	Ensure second and subsequent auth cookies are
+	random.  Spotted by George Lebl.
+
+	* common/gdm-common.[ch] (gdm_generate_random_bytes):
+	open /dev/urandom and read size bytes from it.
+	(_fd_is_character_device): check if fd points to char
+        device
+	(_read_bytes): the usual loop until all bytes are read
+	helper function for read().
+
+	* daemon/gdm-display-access-file.c
+	(generate_random_bytes): remove function
+	(gdm_display_access_file_add_display):
+	use gdm_generate_random_bytes instead of 
+	_generate_random_bytes
+
 2007-11-19  Ray Strode  <rstrode@redhat.com>
 
 	Fix make distcheck
diff --git a/common/gdm-common.c b/common/gdm-common.c
index f6075f37..014c464e 100644
--- a/common/gdm-common.c
+++ b/common/gdm-common.c
@@ -272,3 +272,111 @@ gdm_string_hex_decode (const GString *source,
 
         return retval;
 }
+
+static gboolean
+_fd_is_character_device (int fd)
+{
+        struct stat file_info;
+
+        if (fstat (fd, &file_info) < 0) {
+                return FALSE;
+        }
+
+        return S_ISCHR (file_info.st_mode);
+}
+
+static gboolean
+_read_bytes (int      fd,
+             char    *bytes,
+             gsize    number_of_bytes,
+             GError **error)
+{
+        size_t bytes_left_to_read;
+        size_t total_bytes_read = 0;
+        gboolean premature_eof;
+
+        premature_eof = FALSE;
+        do {
+                size_t bytes_read = 0;
+
+                bytes_read = read (fd, ((guchar *) bytes) + total_bytes_read,
+                                   bytes_left_to_read);
+
+                if (bytes_read > 0) {
+                        total_bytes_read += bytes_read;
+                        bytes_left_to_read -= bytes_read;
+                } else if (bytes_read == 0) {
+                        premature_eof = TRUE;
+                        break;
+                } else if ((errno != EINTR)) {
+                        break;
+                }
+        } while (bytes_left_to_read > 0);
+
+        if (premature_eof) {
+                g_set_error (error,
+                             G_FILE_ERROR,
+                             g_file_error_from_errno (ENODATA),
+                             g_strerror (ENODATA));
+
+                return FALSE;
+        } else if (bytes_left_to_read > 0) {
+                g_set_error (error,
+                             G_FILE_ERROR,
+                             g_file_error_from_errno (errno),
+                             g_strerror (errno));
+                return FALSE;
+        }
+
+        return TRUE;
+}
+
+/**
+ * Pulls a requested number of bytes from /dev/urandom
+ *
+ * @param size number of bytes to pull
+ * @param error error if read fails
+ * @returns The requested number of random bytes or #NULL if fail
+ */
+
+char *
+gdm_generate_random_bytes (gsize    size,
+                           GError **error)
+{
+        int fd;
+        char *bytes;
+        GError *read_error;
+
+        /* We don't use the g_rand_* glib apis because they don't document
+         * how much entropy they are seeded with, and it might be less
+         * than the passed in size.
+         */
+
+        fd = open ("/dev/urandom", O_RDONLY);
+
+        if (fd < 0) {
+                g_set_error (error,
+                             G_FILE_ERROR,
+                             g_file_error_from_errno (errno),
+                             "%s", g_strerror (errno));
+                return NULL;
+        }
+
+        if (!_fd_is_character_device (fd)) {
+                g_set_error (error,
+                             G_FILE_ERROR,
+                             g_file_error_from_errno (ENODEV),
+                             _("/dev/urandom is not a character device"));
+                return NULL;
+        }
+
+        bytes = g_malloc (size);
+        read_error = NULL;
+        if (!_read_bytes (fd, bytes, size, &read_error)) {
+                g_propagate_error (error, read_error);
+                g_free (bytes);
+                return NULL;
+        }
+
+        return bytes;
+}
diff --git a/common/gdm-common.h b/common/gdm-common.h
index d281b9ab..a1aea54e 100644
--- a/common/gdm-common.h
+++ b/common/gdm-common.h
@@ -41,6 +41,8 @@ gboolean       gdm_string_hex_decode     (const GString *source,
                                           int           *end_return,
                                           GString       *dest,
                                           int            insert_at);
+char          *gdm_generate_random_bytes (gsize          size,
+                                          GError       **error);
 
 G_END_DECLS
 
diff --git a/daemon/gdm-display-access-file.c b/daemon/gdm-display-access-file.c
index 0cc0edb3..08875682 100644
--- a/daemon/gdm-display-access-file.c
+++ b/daemon/gdm-display-access-file.c
@@ -36,6 +36,7 @@
 #include <X11/Xauth.h>
 
 #include "gdm-display-access-file.h"
+#include "gdm-common.h"
 
 struct _GdmDisplayAccessFilePrivate
 {
@@ -298,25 +299,6 @@ gdm_display_access_file_open (GdmDisplayAccessFile  *file,
         return TRUE;
 }
 
-static char *
-_generate_random_bytes (gsize size)
-{
-        char *bytes;
-        int i;
-
-        bytes = g_malloc (size);
-
-        for (i = 0; i < size; i++) {
-                guint8 byte;
-
-                byte = (guint8) g_random_int_range (0, G_MAXUINT8);
-
-                bytes[i] = (char) byte;
-        }
-
-        return bytes;
-}
-
 static void
 _get_auth_info_for_display (GdmDisplayAccessFile *file,
                             GdmDisplay           *display,
@@ -364,10 +346,17 @@ gdm_display_access_file_add_display (GdmDisplayAccessFile  *file,
         g_return_val_if_fail (file->priv->path != NULL, FALSE);
         g_return_val_if_fail (cookie != NULL, FALSE);
 
-        *cookie = _generate_random_bytes (GDM_DISPLAY_ACCESS_COOKIE_SIZE);
+        add_error = NULL;
+        *cookie = gdm_generate_random_bytes (GDM_DISPLAY_ACCESS_COOKIE_SIZE,
+                                             &add_error);
+
+        if (*cookie == NULL) {
+                g_propagate_error (error, add_error);
+                return FALSE;
+        }
+
         *cookie_size = GDM_DISPLAY_ACCESS_COOKIE_SIZE;
 
-        add_error = NULL;
         display_added = gdm_display_access_file_add_display_with_cookie (file, display,
                                                                          *cookie,
                                                                          *cookie_size,