Commit f385324d authored by Simon McVittie's avatar Simon McVittie

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=90414Reviewed-by: Ralf Habacker's avatarRalf Habacker <ralf.habacker@freenet.de>
[smcv: document @error]
Signed-off-by: default avatarSimon McVittie <simon.mcvittie@collabora.co.uk>
parent 49646211
...@@ -766,7 +766,8 @@ bus_context_new (const DBusString *config_file, ...@@ -766,7 +766,8 @@ bus_context_new (const DBusString *config_file,
} }
context->refcount = 1; 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)) if (!_dbus_string_copy_data (config_file, &context->config_file))
{ {
......
...@@ -4436,15 +4436,24 @@ _dbus_connection_peer_filter_unlocked_no_update (DBusConnection *connection, ...@@ -4436,15 +4436,24 @@ _dbus_connection_peer_filter_unlocked_no_update (DBusConnection *connection,
"GetMachineId")) "GetMachineId"))
{ {
DBusString uuid; DBusString uuid;
DBusError error = DBUS_ERROR_INIT;
ret = dbus_message_new_method_return (message);
if (ret == NULL) if (!_dbus_string_init (&uuid))
goto out; goto out;
_dbus_string_init (&uuid); if (_dbus_get_local_machine_uuid_encoded (&uuid, &error))
if (_dbus_get_local_machine_uuid_encoded (&uuid))
{ {
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, if (dbus_message_append_args (ret,
DBUS_TYPE_STRING, &v_STRING, DBUS_TYPE_STRING, &v_STRING,
DBUS_TYPE_INVALID)) DBUS_TYPE_INVALID))
...@@ -4452,6 +4461,23 @@ _dbus_connection_peer_filter_unlocked_no_update (DBusConnection *connection, ...@@ -4452,6 +4461,23 @@ _dbus_connection_peer_filter_unlocked_no_update (DBusConnection *connection,
sent = _dbus_connection_send_unlocked_no_update (connection, ret, NULL); 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); _dbus_string_free (&uuid);
} }
else else
......
...@@ -646,9 +646,12 @@ _dbus_string_array_contains (const char **array, ...@@ -646,9 +646,12 @@ _dbus_string_array_contains (const char **array,
* there's some text about it in the spec that should also change. * there's some text about it in the spec that should also change.
* *
* @param uuid the uuid to initialize * @param uuid the uuid to initialize
* @param error location to store reason for failure
* @returns #TRUE on success
*/ */
void dbus_bool_t
_dbus_generate_uuid (DBusGUID *uuid) _dbus_generate_uuid (DBusGUID *uuid,
DBusError *error)
{ {
long now; long now;
...@@ -660,6 +663,7 @@ _dbus_generate_uuid (DBusGUID *uuid) ...@@ -660,6 +663,7 @@ _dbus_generate_uuid (DBusGUID *uuid)
uuid->as_uint32s[DBUS_UUID_LENGTH_WORDS - 1] = DBUS_UINT32_TO_BE (now); 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); _dbus_generate_random_bytes_buffer (uuid->as_bytes, DBUS_UUID_LENGTH_BYTES - 4);
return TRUE;
} }
/** /**
...@@ -841,7 +845,10 @@ _dbus_read_uuid_file (const DBusString *filename, ...@@ -841,7 +845,10 @@ _dbus_read_uuid_file (const DBusString *filename,
else else
{ {
dbus_error_free (&read_error); 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); return _dbus_write_uuid_file (filename, uuid, error);
} }
} }
...@@ -858,22 +865,27 @@ static DBusGUID machine_uuid; ...@@ -858,22 +865,27 @@ static DBusGUID machine_uuid;
* machine is reconfigured or its hardware is modified. * machine is reconfigured or its hardware is modified.
* *
* @param uuid_str string to append hex-encoded machine uuid to * @param uuid_str string to append hex-encoded machine uuid to
* @returns #FALSE if no memory * @param error location to store reason for failure
* @returns #TRUE if successful
*/ */
dbus_bool_t 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)) if (!_DBUS_LOCK (machine_uuid))
return FALSE; {
_DBUS_SET_OOM (error);
return FALSE;
}
if (machine_uuid_initialized_generation != _dbus_current_generation) 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, if (!_dbus_read_local_machine_uuid (&machine_uuid, FALSE,
&error)) &local_error))
{ {
#ifndef DBUS_ENABLE_EMBEDDED_TESTS #ifndef DBUS_ENABLE_EMBEDDED_TESTS
/* For the test suite, we may not be installed so just continue silently /* For the test suite, we may not be installed so just continue silently
...@@ -882,16 +894,23 @@ _dbus_get_local_machine_uuid_encoded (DBusString *uuid_str) ...@@ -882,16 +894,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" _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", "See the manual page for dbus-uuidgen to correct this issue.\n",
error.message); local_error.message);
#endif #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); _DBUS_UNLOCK (machine_uuid);
......
...@@ -389,8 +389,9 @@ union DBusGUID ...@@ -389,8 +389,9 @@ union DBusGUID
char as_bytes[DBUS_UUID_LENGTH_BYTES]; /**< guid as 16 single-byte values */ char as_bytes[DBUS_UUID_LENGTH_BYTES]; /**< guid as 16 single-byte values */
}; };
DBUS_PRIVATE_EXPORT DBUS_PRIVATE_EXPORT _DBUS_GNUC_WARN_UNUSED_RESULT
void _dbus_generate_uuid (DBusGUID *uuid); dbus_bool_t _dbus_generate_uuid (DBusGUID *uuid,
DBusError *error);
DBUS_PRIVATE_EXPORT DBUS_PRIVATE_EXPORT
dbus_bool_t _dbus_uuid_encode (const DBusGUID *uuid, dbus_bool_t _dbus_uuid_encode (const DBusGUID *uuid,
DBusString *encoded); DBusString *encoded);
...@@ -403,7 +404,8 @@ dbus_bool_t _dbus_write_uuid_file (const DBusString *filename, ...@@ -403,7 +404,8 @@ dbus_bool_t _dbus_write_uuid_file (const DBusString *filename,
const DBusGUID *uuid, const DBusGUID *uuid,
DBusError *error); 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_PASTE2(a, b) a ## b
#define _DBUS_PASTE(a, b) _DBUS_PASTE2 (a, b) #define _DBUS_PASTE(a, b) _DBUS_PASTE2 (a, b)
......
...@@ -80,7 +80,11 @@ dbus_get_local_machine_id (void) ...@@ -80,7 +80,11 @@ dbus_get_local_machine_id (void)
if (!_dbus_string_init (&uuid)) if (!_dbus_string_init (&uuid))
return NULL; 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_steal_data (&uuid, &s))
{ {
_dbus_string_free (&uuid); _dbus_string_free (&uuid);
......
...@@ -137,7 +137,8 @@ _dbus_server_init_base (DBusServer *server, ...@@ -137,7 +137,8 @@ _dbus_server_init_base (DBusServer *server,
return FALSE; 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)) if (!_dbus_uuid_encode (&server->guid, &server->guid_hex))
goto oom; goto oom;
...@@ -167,6 +168,7 @@ _dbus_server_init_base (DBusServer *server, ...@@ -167,6 +168,7 @@ _dbus_server_init_base (DBusServer *server,
oom: oom:
_DBUS_SET_OOM (error); _DBUS_SET_OOM (error);
failed:
_dbus_rmutex_free_at_location (&server->mutex); _dbus_rmutex_free_at_location (&server->mutex);
server->mutex = NULL; server->mutex = NULL;
if (server->watches) if (server->watches)
......
...@@ -3742,9 +3742,8 @@ _dbus_get_autolaunch_address (const char *scope, ...@@ -3742,9 +3742,8 @@ _dbus_get_autolaunch_address (const char *scope,
return FALSE; 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; goto out;
} }
...@@ -3844,7 +3843,10 @@ _dbus_read_local_machine_uuid (DBusGUID *machine_id, ...@@ -3844,7 +3843,10 @@ _dbus_read_local_machine_uuid (DBusGUID *machine_id,
/* if none found, try to make a new one */ /* if none found, try to make a new one */
dbus_error_free (error); dbus_error_free (error);
_dbus_string_init_const (&filename, DBUS_MACHINE_UUID_FILE); _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); return _dbus_write_uuid_file (&filename, machine_id, error);
} }
......
...@@ -111,20 +111,20 @@ dbus_internal_do_not_use_get_uuid (const char *filename, ...@@ -111,20 +111,20 @@ 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 * @param uuid_p out param to return the uuid
* @returns #FALSE if no memory * @param error location to store reason for failure
* @returns #TRUE on success
*/ */
dbus_bool_t dbus_bool_t
dbus_internal_do_not_use_create_uuid (char **uuid_p) _dbus_create_uuid (char **uuid_p,
DBusError *error)
{ {
DBusGUID uuid; DBusGUID uuid;
_dbus_generate_uuid (&uuid); if (!_dbus_generate_uuid (&uuid, error))
return return_uuid (&uuid, uuid_p, NULL); return FALSE;
return return_uuid (&uuid, uuid_p, error);
} }
/** @} */ /** @} */
...@@ -41,8 +41,8 @@ dbus_bool_t dbus_internal_do_not_use_ensure_uuid (const char *filename, ...@@ -41,8 +41,8 @@ dbus_bool_t dbus_internal_do_not_use_ensure_uuid (const char *filename,
char **uuid_p, char **uuid_p,
DBusError *error); DBusError *error);
DBUS_PRIVATE_EXPORT 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 DBUS_END_DECLS
......
...@@ -137,15 +137,12 @@ main (int argc, char *argv[]) ...@@ -137,15 +137,12 @@ main (int argc, char *argv[])
else else
{ {
char *uuid; char *uuid;
if (dbus_internal_do_not_use_create_uuid (&uuid))
if (_dbus_create_uuid (&uuid, &error))
{ {
printf ("%s\n", uuid); printf ("%s\n", uuid);
dbus_free (uuid); dbus_free (uuid);
} }
else
{
dbus_set_error (&error, DBUS_ERROR_NO_MEMORY, "No memory");
}
} }
if (dbus_error_is_set (&error)) if (dbus_error_is_set (&error))
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment