Attachment 115717 Details for Bug 90414 – Make UUID generation failable

[patch] Make UUID generation failable

0004-Make-UUID-generation-failable.patch (text/plain), 11.41 KB, created by Simon McVittie on 2015-05-12 11:52:55 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2015-05-12 11:52:55 UTC

Size: 11.41 KB

patch

obsolete

>From 1d14348e705474cd0632cb52810a906a8fbdbad9 Mon Sep 17 00:00:00 2001
>From: Simon McVittie <simon.mcvittie@collabora.co.uk>
>Date: Tue, 12 May 2015 11:35:04 +0100
>Subject: [PATCH 4/5] Make UUID generation failable
>
>Previously, this would always succeed, but might use
>weak random numbers in rare failure cases. I don't think
>these UUIDs are security-sensitive, but if they're generated
>by a PRNG as weak as rand() (<= 32 bits of entropy), we
>certainly can't claim that they're universally unique.
>
>Bug: https://bugs.freedesktop.org/show_bug.cgi?id=90414
>---
> bus/bus.c                |  3 ++-
> dbus/dbus-connection.c   | 38 ++++++++++++++++++++++++++++++++------
> dbus/dbus-internals.c    | 40 ++++++++++++++++++++++++++++------------
> dbus/dbus-internals.h    |  8 +++++---
> dbus/dbus-misc.c         |  6 +++++-
> dbus/dbus-server.c       |  4 +++-
> dbus/dbus-sysdeps-unix.c |  8 +++++---
> dbus/dbus-uuidgen.c      | 15 +++++++--------
> dbus/dbus-uuidgen.h      |  4 ++--
> tools/dbus-uuidgen.c     |  7 ++-----
> 10 files changed, 91 insertions(+), 42 deletions(-)
>
>diff --git a/bus/bus.c b/bus/bus.c
>index 68de1b2..1aa893b 100644
>--- a/bus/bus.c
>+++ b/bus/bus.c
>@@ -766,7 +766,8 @@ bus_context_new (const DBusString *config_file,
>     }
>   context->refcount = 1;
> 
>-  _dbus_generate_uuid (&context->uuid);
>+  if (!_dbus_generate_uuid (&context->uuid, error))
>+    goto failed;
> 
>   if (!_dbus_string_copy_data (config_file, &context->config_file))
>     {
>diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c
>index 8952b75..97512fb 100644
>--- a/dbus/dbus-connection.c
>+++ b/dbus/dbus-connection.c
>@@ -4436,15 +4436,24 @@ _dbus_connection_peer_filter_unlocked_no_update (DBusConnection *connection,
>                                         "GetMachineId"))
>     {
>       DBusString uuid;
>-      
>-      ret = dbus_message_new_method_return (message);
>-      if (ret == NULL)
>+      DBusError error = DBUS_ERROR_INIT;
>+
>+      if (!_dbus_string_init (&uuid))
>         goto out;
> 
>-      _dbus_string_init (&uuid);
>-      if (_dbus_get_local_machine_uuid_encoded (&uuid))
>+      if (_dbus_get_local_machine_uuid_encoded (&uuid, &error))
>         {
>-          const char *v_STRING = _dbus_string_get_const_data (&uuid);
>+          const char *v_STRING;
>+
>+          ret = dbus_message_new_method_return (message);
>+
>+          if (ret == NULL)
>+            {
>+              _dbus_string_free (&uuid);
>+              goto out;
>+            }
>+
>+          v_STRING = _dbus_string_get_const_data (&uuid);
>           if (dbus_message_append_args (ret,
>                                         DBUS_TYPE_STRING, &v_STRING,
>                                         DBUS_TYPE_INVALID))
>@@ -4452,6 +4461,23 @@ _dbus_connection_peer_filter_unlocked_no_update (DBusConnection *connection,
>               sent = _dbus_connection_send_unlocked_no_update (connection, ret, NULL);
>             }
>         }
>+      else if (dbus_error_has_name (&error, DBUS_ERROR_NO_MEMORY))
>+        {
>+          dbus_error_free (&error);
>+          goto out;
>+        }
>+      else
>+        {
>+          ret = dbus_message_new_error (message, error.name, error.message);
>+          dbus_error_free (&error);
>+
>+          if (ret == NULL)
>+            goto out;
>+
>+          sent = _dbus_connection_send_unlocked_no_update (connection, ret,
>+                                                           NULL);
>+        }
>+
>       _dbus_string_free (&uuid);
>     }
>   else
>diff --git a/dbus/dbus-internals.c b/dbus/dbus-internals.c
>index c7f81e5..d988d59 100644
>--- a/dbus/dbus-internals.c
>+++ b/dbus/dbus-internals.c
>@@ -647,8 +647,9 @@ _dbus_string_array_contains (const char **array,
>  *
>  * @param uuid the uuid to initialize
>  */
>-void
>-_dbus_generate_uuid (DBusGUID *uuid)
>+dbus_bool_t
>+_dbus_generate_uuid (DBusGUID  *uuid,
>+                     DBusError *error)
> {
>   long now;
> 
>@@ -660,6 +661,7 @@ _dbus_generate_uuid (DBusGUID *uuid)
>   uuid->as_uint32s[DBUS_UUID_LENGTH_WORDS - 1] = DBUS_UINT32_TO_BE (now);
>   
>   _dbus_generate_random_bytes_buffer (uuid->as_bytes, DBUS_UUID_LENGTH_BYTES - 4);
>+  return TRUE;
> }
> 
> /**
>@@ -841,7 +843,10 @@ _dbus_read_uuid_file (const DBusString *filename,
>   else
>     {
>       dbus_error_free (&read_error);
>-      _dbus_generate_uuid (uuid);
>+
>+      if (!_dbus_generate_uuid (uuid, error))
>+        return FALSE;
>+
>       return _dbus_write_uuid_file (filename, uuid, error);
>     }
> }
>@@ -861,19 +866,23 @@ static DBusGUID machine_uuid;
>  * @returns #FALSE if no memory
>  */
> dbus_bool_t
>-_dbus_get_local_machine_uuid_encoded (DBusString *uuid_str)
>+_dbus_get_local_machine_uuid_encoded (DBusString *uuid_str,
>+                                      DBusError  *error)
> {
>-  dbus_bool_t ok;
>+  dbus_bool_t ok = TRUE;
>   
>   if (!_DBUS_LOCK (machine_uuid))
>-    return FALSE;
>+    {
>+      _DBUS_SET_OOM (error);
>+      return FALSE;
>+    }
> 
>   if (machine_uuid_initialized_generation != _dbus_current_generation)
>     {
>-      DBusError error = DBUS_ERROR_INIT;
>+      DBusError local_error = DBUS_ERROR_INIT;
> 
>       if (!_dbus_read_local_machine_uuid (&machine_uuid, FALSE,
>-                                          &error))
>+                                          &local_error))
>         {          
> #ifndef DBUS_ENABLE_EMBEDDED_TESTS
>           /* For the test suite, we may not be installed so just continue silently
>@@ -882,16 +891,23 @@ _dbus_get_local_machine_uuid_encoded (DBusString *uuid_str)
>            */
>           _dbus_warn_check_failed ("D-Bus library appears to be incorrectly set up; failed to read machine uuid: %s\n"
>                                    "See the manual page for dbus-uuidgen to correct this issue.\n",
>-                                   error.message);
>+                                   local_error.message);
> #endif
>           
>-          dbus_error_free (&error);
>+          dbus_error_free (&local_error);
>           
>-          _dbus_generate_uuid (&machine_uuid);
>+          ok = _dbus_generate_uuid (&machine_uuid, error);
>         }
>     }
> 
>-  ok = _dbus_uuid_encode (&machine_uuid, uuid_str);
>+  if (ok)
>+    {
>+      if (!_dbus_uuid_encode (&machine_uuid, uuid_str))
>+        {
>+          ok = FALSE;
>+          _DBUS_SET_OOM (error);
>+        }
>+    }
> 
>   _DBUS_UNLOCK (machine_uuid);
> 
>diff --git a/dbus/dbus-internals.h b/dbus/dbus-internals.h
>index 6368538..a0d69c2 100644
>--- a/dbus/dbus-internals.h
>+++ b/dbus/dbus-internals.h
>@@ -392,8 +392,9 @@ union DBusGUID
>   char as_bytes[DBUS_UUID_LENGTH_BYTES];                /**< guid as 16 single-byte values */
> };
> 
>-DBUS_PRIVATE_EXPORT
>-void        _dbus_generate_uuid  (DBusGUID         *uuid);
>+DBUS_PRIVATE_EXPORT _DBUS_GNUC_WARN_UNUSED_RESULT
>+dbus_bool_t _dbus_generate_uuid  (DBusGUID         *uuid,
>+                                  DBusError        *error);
> DBUS_PRIVATE_EXPORT
> dbus_bool_t _dbus_uuid_encode    (const DBusGUID   *uuid,
>                                   DBusString       *encoded);
>@@ -406,7 +407,8 @@ dbus_bool_t _dbus_write_uuid_file (const DBusString *filename,
>                                    const DBusGUID   *uuid,
>                                    DBusError        *error);
> 
>-dbus_bool_t _dbus_get_local_machine_uuid_encoded (DBusString *uuid_str);
>+dbus_bool_t _dbus_get_local_machine_uuid_encoded (DBusString *uuid_str,
>+                                                  DBusError  *error);
> 
> #define _DBUS_PASTE2(a, b) a ## b
> #define _DBUS_PASTE(a, b) _DBUS_PASTE2 (a, b)
>diff --git a/dbus/dbus-misc.c b/dbus/dbus-misc.c
>index 6ca30f2..a2da562 100644
>--- a/dbus/dbus-misc.c
>+++ b/dbus/dbus-misc.c
>@@ -80,7 +80,11 @@ dbus_get_local_machine_id (void)
>   if (!_dbus_string_init (&uuid))
>     return NULL;
> 
>-  if (!_dbus_get_local_machine_uuid_encoded (&uuid) ||
>+  /* The documentation says dbus_get_local_machine_id() only fails on OOM;
>+   * this can actually also fail if the D-Bus installation is faulty
>+   * (no UUID) *and* reading a new random UUID fails, but we have no way
>+   * to report that */
>+  if (!_dbus_get_local_machine_uuid_encoded (&uuid, NULL) ||
>       !_dbus_string_steal_data (&uuid, &s))
>     {
>       _dbus_string_free (&uuid);
>diff --git a/dbus/dbus-server.c b/dbus/dbus-server.c
>index d062eb7..12c793a 100644
>--- a/dbus/dbus-server.c
>+++ b/dbus/dbus-server.c
>@@ -136,7 +136,8 @@ _dbus_server_init_base (DBusServer             *server,
>       return FALSE;
>     }
> 
>-  _dbus_generate_uuid (&server->guid);
>+  if (!_dbus_generate_uuid (&server->guid, error))
>+    goto failed;
> 
>   if (!_dbus_uuid_encode (&server->guid, &server->guid_hex))
>     goto oom;
>@@ -166,6 +167,7 @@ _dbus_server_init_base (DBusServer             *server,
> 
>  oom:
>   _DBUS_SET_OOM (error);
>+ failed:
>   _dbus_rmutex_free_at_location (&server->mutex);
>   server->mutex = NULL;
>   if (server->watches)
>diff --git a/dbus/dbus-sysdeps-unix.c b/dbus/dbus-sysdeps-unix.c
>index ab7efc2..e8178c8 100644
>--- a/dbus/dbus-sysdeps-unix.c
>+++ b/dbus/dbus-sysdeps-unix.c
>@@ -3734,9 +3734,8 @@ _dbus_get_autolaunch_address (const char *scope,
>       return FALSE;
>     }
> 
>-  if (!_dbus_get_local_machine_uuid_encoded (&uuid))
>+  if (!_dbus_get_local_machine_uuid_encoded (&uuid, error))
>     {
>-      _DBUS_SET_OOM (error);
>       goto out;
>     }
> 
>@@ -3836,7 +3835,10 @@ _dbus_read_local_machine_uuid (DBusGUID   *machine_id,
>   /* if none found, try to make a new one */
>   dbus_error_free (error);
>   _dbus_string_init_const (&filename, DBUS_MACHINE_UUID_FILE);
>-  _dbus_generate_uuid (machine_id);
>+
>+  if (!_dbus_generate_uuid (machine_id, error))
>+    return FALSE;
>+
>   return _dbus_write_uuid_file (&filename, machine_id, error);
> }
> 
>diff --git a/dbus/dbus-uuidgen.c b/dbus/dbus-uuidgen.c
>index 6d7c0ae..2b0f85b 100644
>--- a/dbus/dbus-uuidgen.c
>+++ b/dbus/dbus-uuidgen.c
>@@ -111,20 +111,19 @@ dbus_internal_do_not_use_get_uuid (const char *filename,
> }
> 
> /**
>- * For use by the dbus-uuidgen binary ONLY, do not call this.
>- * We can and will change this function without modifying
>- * the libdbus soname.
>- *
>  * @param uuid_p out param to return the uuid
>- * @returns #FALSE if no memory
>+ * @returns #FALSE on error
>  */
> dbus_bool_t
>-dbus_internal_do_not_use_create_uuid (char      **uuid_p)
>+_dbus_create_uuid (char      **uuid_p,
>+                   DBusError  *error)
> {
>   DBusGUID uuid;
> 
>-  _dbus_generate_uuid (&uuid);
>-  return return_uuid (&uuid, uuid_p, NULL);
>+  if (!_dbus_generate_uuid (&uuid, error))
>+    return FALSE;
>+
>+  return return_uuid (&uuid, uuid_p, error);
> }
> 
> /** @} */
>diff --git a/dbus/dbus-uuidgen.h b/dbus/dbus-uuidgen.h
>index 9d12f2b..5e4a948 100644
>--- a/dbus/dbus-uuidgen.h
>+++ b/dbus/dbus-uuidgen.h
>@@ -41,8 +41,8 @@ dbus_bool_t dbus_internal_do_not_use_ensure_uuid (const char *filename,
>                                                   char      **uuid_p,
>                                                   DBusError  *error);
> DBUS_PRIVATE_EXPORT
>-dbus_bool_t dbus_internal_do_not_use_create_uuid (char      **uuid_p);
>-
>+dbus_bool_t _dbus_create_uuid                    (char      **uuid_p,
>+                                                  DBusError  *error);
> 
> DBUS_END_DECLS
> 
>diff --git a/tools/dbus-uuidgen.c b/tools/dbus-uuidgen.c
>index c8ba1cf..03ce553 100644
>--- a/tools/dbus-uuidgen.c
>+++ b/tools/dbus-uuidgen.c
>@@ -137,15 +137,12 @@ main (int argc, char *argv[])
>   else
>     {
>       char *uuid;
>-      if (dbus_internal_do_not_use_create_uuid (&uuid))
>+
>+      if (_dbus_create_uuid (&uuid, &error))
>         {
>           printf ("%s\n", uuid);
>           dbus_free (uuid);
>         }
>-      else
>-        {
>-          dbus_set_error (&error, DBUS_ERROR_NO_MEMORY, "No memory");
>-        }
>     }
> 
>   if (dbus_error_is_set (&error))
>-- 
>2.1.4
>

Actions: View | Splinter Review

Attachments on bug 90414: 115714 | 115715 | 115716 | 115717 | 115718

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.