From 2f38c959212d98e2194139daa9120fda37415b4f Mon Sep 17 00:00:00 2001 From: Havoc Pennington Date: Mon, 24 Feb 2003 02:24:13 +0000 Subject: 2003-02-23 Havoc Pennington * dbus/dbus-keyring.c: finish most of this implementation and simple unit test * dbus/dbus-errors.c (dbus_set_error_const, dbus_set_error): make these barf if the error isn't cleared to NULL * dbus/dbus-sysdeps.c (_dbus_delete_file): set error on failure (_dbus_create_directory): new function * dbus/dbus-errors.c (dbus_set_error): fix warning * dbus/dbus-string.c (_dbus_string_hex_encode): new function (_dbus_string_hex_decode): new function (test_hex_roundtrip): test code * dbus/dbus-sha.c (_dbus_sha_compute): use dbus_string_hex_encode * dbus/dbus-md5.c (_dbus_md5_compute): use dbus_string_hex_encode * dbus/dbus-sysdeps.c (_dbus_string_save_to_file): make this use the save-to-temp/rename trick to atomically write the new file (_dbus_string_parse_uint): new function --- ChangeLog | 25 +++ dbus/Makefile.am | 2 + dbus/dbus-errors.c | 12 +- dbus/dbus-internals.c | 13 -- dbus/dbus-keyring.c | 601 +++++++++++++++++++++++++++++++++++++++++++++++--- dbus/dbus-keyring.h | 3 +- dbus/dbus-md5.c | 28 +-- dbus/dbus-sha.c | 28 +-- dbus/dbus-string.c | 329 +++++++++++++++++++++++---- dbus/dbus-string.h | 12 + dbus/dbus-sysdeps.c | 208 ++++++++++++++++- dbus/dbus-sysdeps.h | 2 + dbus/dbus-test.c | 4 + dbus/dbus-test.h | 1 + 14 files changed, 1123 insertions(+), 145 deletions(-) diff --git a/ChangeLog b/ChangeLog index 4d4b2f09..5a29d1a9 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,28 @@ +2003-02-23 Havoc Pennington + + * dbus/dbus-keyring.c: finish most of this implementation and + simple unit test + + * dbus/dbus-errors.c (dbus_set_error_const, dbus_set_error): make + these barf if the error isn't cleared to NULL + + * dbus/dbus-sysdeps.c (_dbus_delete_file): set error on failure + (_dbus_create_directory): new function + + * dbus/dbus-errors.c (dbus_set_error): fix warning + + * dbus/dbus-string.c (_dbus_string_hex_encode): new function + (_dbus_string_hex_decode): new function + (test_hex_roundtrip): test code + + * dbus/dbus-sha.c (_dbus_sha_compute): use dbus_string_hex_encode + + * dbus/dbus-md5.c (_dbus_md5_compute): use dbus_string_hex_encode + + * dbus/dbus-sysdeps.c (_dbus_string_save_to_file): make this use + the save-to-temp/rename trick to atomically write the new file + (_dbus_string_parse_uint): new function + 2003-02-22 Havoc Pennington * test/Makefile.am (dist-hook): fix dist for test/data/sha-1 diff --git a/dbus/Makefile.am b/dbus/Makefile.am index e3d4f628..eb7f5c93 100644 --- a/dbus/Makefile.am +++ b/dbus/Makefile.am @@ -30,6 +30,8 @@ libdbus_1_la_SOURCES= \ dbus-connection.c \ dbus-connection-internal.h \ dbus-errors.c \ + dbus-keyring.c \ + dbus-keyring.h \ dbus-md5.c \ dbus-md5.h \ dbus-memory.c \ diff --git a/dbus/dbus-errors.c b/dbus/dbus-errors.c index a469f7e6..3aca9f10 100644 --- a/dbus/dbus-errors.c +++ b/dbus/dbus-errors.c @@ -196,6 +196,10 @@ dbus_set_error_const (DBusError *error, if (error == NULL) return; + + /* it's a bug to pile up errors */ + _dbus_assert (error->name == NULL); + _dbus_assert (error->message == NULL); real = (DBusRealError *)error; @@ -225,13 +229,17 @@ dbus_set_error (DBusError *error, ...) { DBusRealError *real; - va_list args, args2; + va_list args; int message_length; char *message; char c; if (error == NULL) return; + + /* it's a bug to pile up errors */ + _dbus_assert (error->name == NULL); + _dbus_assert (error->message == NULL); va_start (args, format); /* Measure the message length */ @@ -248,7 +256,7 @@ dbus_set_error (DBusError *error, } va_start (args, format); - vsprintf (message, format, args2); + vsprintf (message, format, args); va_end (args); real = (DBusRealError *)error; diff --git a/dbus/dbus-internals.c b/dbus/dbus-internals.c index d51f5a97..f85ad18b 100644 --- a/dbus/dbus-internals.c +++ b/dbus/dbus-internals.c @@ -180,19 +180,6 @@ _dbus_verbose_real (const char *format, va_end (args); } -/** - * A wrapper around strerror() because some platforms - * may be lame and not have strerror(). - * - * @param error_number errno. - * @returns error description. - */ -const char* -_dbus_strerror (int error_number) -{ - return strerror (error_number); -} - /** * Converts a UNIX errno into a DBusResultCode. * diff --git a/dbus/dbus-keyring.c b/dbus/dbus-keyring.c index 0bc7ab9a..ea20c9c4 100644 --- a/dbus/dbus-keyring.c +++ b/dbus/dbus-keyring.c @@ -58,19 +58,26 @@ * clocks can change or be wrong, but we make a best effort to only * use keys for a short time. */ -#define NEW_KEY_TIMEOUT (60*5) +#define NEW_KEY_TIMEOUT_SECONDS (60*5) /** - * The time after which we drop a key from the secrets file + * The time after which we drop a key from the secrets file. + * The EXPIRE_KEYS_TIMEOUT_SECONDS - NEW_KEY_TIMEOUT_SECONDS is the minimum + * time window a client has to complete authentication. */ -#define EXPIRE_KEYS_TIMEOUT (NEW_KEY_TIMEOUT + (60*2)) +#define EXPIRE_KEYS_TIMEOUT_SECONDS (NEW_KEY_TIMEOUT_SECONDS + (60*2)) +/** + * The maximum amount of time a key can be in the future. + */ +#define MAX_TIME_TRAVEL_SECONDS (60*5) typedef struct { dbus_int32_t id; /**< identifier used to refer to the key */ - unsigned long creation_time; /**< when the key was generated, - * as unix timestamp - */ + long creation_time; /**< when the key was generated, + * as unix timestamp. signed long + * matches struct timeval. + */ DBusString secret; /**< the actual key */ @@ -101,13 +108,13 @@ _dbus_keyring_new (void) if (keyring == NULL) goto out_0; - if (!_dbus_string_init (&keyring->directory)) + if (!_dbus_string_init (&keyring->directory, _DBUS_INT_MAX)) goto out_1; - if (!_dbus_string_init (&keyring->filename)) + if (!_dbus_string_init (&keyring->filename, _DBUS_INT_MAX)) goto out_2; - if (!_dbus_string_init (&keyring->filename_lock)) + if (!_dbus_string_init (&keyring->filename_lock, _DBUS_INT_MAX)) goto out_3; keyring->refcount = 1; @@ -161,9 +168,9 @@ free_keys (DBusKey *keys, */ /** Maximum number of timeouts waiting for lock before we decide it's stale */ -#define MAX_LOCK_TIMEOUTS 6 +#define MAX_LOCK_TIMEOUTS 16 /** Length of each timeout while waiting for a lock */ -#define LOCK_TIMEOUT 500 +#define LOCK_TIMEOUT_MILLISECONDS 250 static dbus_bool_t _dbus_keyring_lock (DBusKeyring *keyring) @@ -180,25 +187,38 @@ _dbus_keyring_lock (DBusKeyring *keyring) &error)) break; - _dbus_verbose ("Did not get lock file: %s\n", - error.message); + _dbus_verbose ("Did not get lock file, sleeping %d milliseconds (%s)\n", + LOCK_TIMEOUT_MILLISECONDS, error.message); dbus_error_free (&error); - _dbus_sleep_milliseconds (LOCK_TIMEOUT); + _dbus_sleep_milliseconds (LOCK_TIMEOUT_MILLISECONDS); ++n_timeouts; } if (n_timeouts == MAX_LOCK_TIMEOUTS) { - _dbus_verbose ("Lock file timed out, assuming stale\n"); + DBusError error; + + _dbus_verbose ("Lock file timed out %d times, assuming stale\n", + n_timeouts); - _dbus_delete_file (&keyring->filename_lock); + dbus_error_init (&error); + + if (!_dbus_delete_file (&keyring->filename_lock, &error)) + { + _dbus_verbose ("Couldn't delete old lock file: %s\n", + error.message); + dbus_error_free (&error); + return FALSE; + } if (!_dbus_create_file_exclusively (&keyring->filename_lock, - NULL)) + &error)) { - _dbus_verbose ("Couldn't create lock file after trying to delete the stale one, giving up\n"); + _dbus_verbose ("Couldn't create lock file after deleting stale one: %s\n", + error.message); + dbus_error_free (&error); return FALSE; } } @@ -209,17 +229,153 @@ _dbus_keyring_lock (DBusKeyring *keyring) static void _dbus_keyring_unlock (DBusKeyring *keyring) { - if (!_dbus_delete_file (&keyring->filename_lock)) - _dbus_warn ("Failed to delete lock file\n"); + DBusError error; + dbus_error_init (&error); + if (!_dbus_delete_file (&keyring->filename_lock, &error)) + { + _dbus_warn ("Failed to delete lock file: %s\n", + error.message); + dbus_error_free (&error); + } +} + +static DBusKey* +find_key_by_id (DBusKey *keys, + int n_keys, + int id) +{ + int i; + + i = 0; + while (i < n_keys) + { + if (keys[i].id == id) + return &keys[i]; + + ++i; + } + + return NULL; +} + +static dbus_bool_t +add_new_key (DBusKey **keys_p, + int *n_keys_p, + DBusError *error) +{ + DBusKey *new; + DBusString bytes; + int id; + unsigned long timestamp; + const unsigned char *s; + dbus_bool_t retval; + DBusKey *keys; + int n_keys; + + if (!_dbus_string_init (&bytes, _DBUS_INT_MAX)) + { + dbus_set_error (error, DBUS_ERROR_NO_MEMORY, + "No memory to generate new secret key"); + return FALSE; + } + + keys = *keys_p; + n_keys = *n_keys_p; + retval = FALSE; + + /* Generate an integer ID and then the actual key. */ + retry: + + if (!_dbus_generate_random_bytes (&bytes, 4)) + { + dbus_set_error (error, DBUS_ERROR_NO_MEMORY, + "No memory to generate new secret key"); + goto out; + } + + _dbus_string_get_const_data (&bytes, (const char**) &s); + + id = s[0] | (s[1] << 8) | (s[2] << 16) | (s[3] << 24); + if (id < 0) + id = - id; + _dbus_assert (id >= 0); + + if (find_key_by_id (keys, n_keys, id) != NULL) + { + _dbus_string_set_length (&bytes, 0); + _dbus_verbose ("Key ID %d already existed, trying another one\n", + id); + goto retry; + } + + _dbus_verbose ("Creating key with ID %d\n", id); + +#define KEY_LENGTH_BYTES 24 + _dbus_string_set_length (&bytes, 0); + if (!_dbus_generate_random_bytes (&bytes, KEY_LENGTH_BYTES)) + { + dbus_set_error (error, DBUS_ERROR_NO_MEMORY, + "No memory to generate new secret key"); + goto out; + } + + new = dbus_realloc (keys, sizeof (DBusKey) * (n_keys + 1)); + if (new == NULL) + { + dbus_set_error (error, DBUS_ERROR_NO_MEMORY, + "No memory to reallocate secret key list"); + goto out; + } + + keys = new; + n_keys += 1; + + if (!_dbus_string_init (&keys[n_keys-1].secret, + _DBUS_INT_MAX)) + { + n_keys -= 1; /* we don't want to free the one we didn't init */ + dbus_set_error (error, DBUS_ERROR_NO_MEMORY, + "No memory to store secret key"); + goto out; + } + + _dbus_get_current_time (×tamp, NULL); + + keys[n_keys-1].id = id; + keys[n_keys-1].creation_time = timestamp; + if (!_dbus_string_move (&bytes, 0, + &keys[n_keys-1].secret, + 0)) + { + dbus_set_error (error, DBUS_ERROR_NO_MEMORY, + "No memory to store secret key"); + goto out; + } + + retval = TRUE; + + out: + if (retval) + { + *n_keys_p = n_keys; + *keys_p = keys; + } + + _dbus_string_free (&bytes); + return retval; } /** * Reloads the keyring file, optionally adds one new key to the file, - * removes all expired keys from the file, then resaves the file. - * Stores the keys from the file in keyring->keys. + * removes all expired keys from the file iff a key was added, then + * resaves the file. Stores the keys from the file in keyring->keys. + * Note that the file is only resaved (written to) if a key is added, + * this means that only servers ever write to the file and need to + * lock it, which avoids a lot of lock contention at login time and + * such. * * @param keyring the keyring - * @param add_new #TRUE to add a new key to the file before resave + * @param add_new #TRUE to add a new key to the file, expire keys, and resave * @param error return location for errors * @returns #FALSE on failure */ @@ -228,8 +384,247 @@ _dbus_keyring_reload (DBusKeyring *keyring, dbus_bool_t add_new, DBusError *error) { - /* FIXME */ + DBusString contents; + DBusString line; + DBusResultCode result; + dbus_bool_t retval; + dbus_bool_t have_lock; + DBusKey *keys; + int n_keys; + int i; + long now; + + if (!_dbus_string_init (&contents, _DBUS_INT_MAX)) + { + dbus_set_error (error, DBUS_ERROR_NO_MEMORY, + "No memory to reload keyring"); + return FALSE; + } + + if (!_dbus_string_init (&line, _DBUS_INT_MAX)) + { + dbus_set_error (error, DBUS_ERROR_NO_MEMORY, + "No memory to reload keyring"); + _dbus_string_free (&contents); + return FALSE; + } + + keys = NULL; + n_keys = 0; + retval = FALSE; + have_lock = FALSE; + + _dbus_get_current_time (&now, NULL); + + if (add_new) + { + if (!_dbus_keyring_lock (keyring)) + { + dbus_set_error (error, DBUS_ERROR_FAILED, + "Could not lock keyring file to add to it"); + goto out; + } + + have_lock = TRUE; + } + + result = _dbus_file_get_contents (&contents, + &keyring->filename); + + if (result != DBUS_RESULT_SUCCESS) + { + _dbus_verbose ("Failed to load keyring file: %s\n", + dbus_result_to_string (result)); + /* continue with empty keyring file, so we recreate it */ + } + + if (!_dbus_string_validate_ascii (&contents, 0, + _dbus_string_get_length (&contents))) + { + _dbus_warn ("Secret keyring file contains non-ASCII! Ignoring existing contents\n"); + _dbus_string_set_length (&contents, 0); + } + + while (_dbus_string_pop_line (&contents, &line)) + { + int next; + long val; + int id; + long timestamp; + int len; + DBusKey *new; + + next = 0; + if (!_dbus_string_parse_int (&line, 0, &val, &next)) + { + _dbus_verbose ("could not parse secret key ID at start of line\n"); + continue; + } + + if (val > _DBUS_INT_MAX || val < 0) + { + _dbus_verbose ("invalid secret key ID at start of line\n"); + continue; + } + + id = val; + + _dbus_string_skip_blank (&line, next, &next); + + if (!_dbus_string_parse_int (&line, next, ×tamp, &next)) + { + _dbus_verbose ("could not parse secret key timestamp\n"); + continue; + } + if (timestamp < 0 || + (now + MAX_TIME_TRAVEL_SECONDS) < timestamp || + (now - EXPIRE_KEYS_TIMEOUT_SECONDS) > timestamp) + { + _dbus_verbose ("dropping/ignoring %ld-seconds old key with timestamp %ld as current time is %ld\n", + now - timestamp, timestamp, now); + continue; + } + + _dbus_string_skip_blank (&line, next, &next); + + len = _dbus_string_get_length (&line); + + if ((len - next) == 0) + { + _dbus_verbose ("no secret key after ID and timestamp\n"); + continue; + } + + /* We have all three parts */ + new = dbus_realloc (keys, sizeof (DBusKey) * (n_keys + 1)); + if (new == NULL) + { + dbus_set_error (error, DBUS_ERROR_NO_MEMORY, + "No memory to reallocate secret key list"); + goto out; + } + + keys = new; + n_keys += 1; + + if (!_dbus_string_init (&keys[n_keys-1].secret, + _DBUS_INT_MAX)) + { + n_keys -= 1; /* we don't want to free the one we didn't init */ + dbus_set_error (error, DBUS_ERROR_NO_MEMORY, + "No memory to store secret key"); + goto out; + } + + keys[n_keys-1].id = id; + keys[n_keys-1].creation_time = timestamp; + if (!_dbus_string_hex_decode (&line, next, + &keys[n_keys-1].secret, + 0)) + { + dbus_set_error (error, DBUS_ERROR_NO_MEMORY, + "No memory to store secret key or invalid hex encoding"); + goto out; + } + } + + _dbus_verbose ("Successfully loaded %d existing keys\n", + n_keys); + + if (add_new) + { + if (!add_new_key (&keys, &n_keys, error)) + { + _dbus_verbose ("Failed to generate new key: %s\n", + error ? "(unknown)" : error->message); + goto out; + } + + _dbus_string_set_length (&contents, 0); + + i = 0; + while (i < n_keys) + { + if (!_dbus_string_append_int (&contents, + keys[i].id)) + goto nomem; + + if (!_dbus_string_append_byte (&contents, ' ')) + goto nomem; + + if (!_dbus_string_append_int (&contents, + keys[i].creation_time)) + goto nomem; + + if (!_dbus_string_append_byte (&contents, ' ')) + goto nomem; + + if (!_dbus_string_hex_encode (&keys[i].secret, 0, + &contents, + _dbus_string_get_length (&contents))) + goto nomem; + + if (!_dbus_string_append_byte (&contents, '\n')) + goto nomem; + + ++i; + continue; + + nomem: + dbus_set_error (error, DBUS_ERROR_NO_MEMORY, + "No memory to save secret keyring"); + goto out; + } + + result = _dbus_string_save_to_file (&contents, &keyring->filename); + if (result != DBUS_RESULT_SUCCESS) + { + dbus_set_error (error, DBUS_ERROR_FAILED, + "Failed to save keyring file: %s", + dbus_result_to_string (result)); + goto out; + } + } + + dbus_free (keyring->keys); + keyring->keys = keys; + keyring->n_keys = n_keys; + keys = NULL; + n_keys = 0; + + retval = TRUE; + + out: + if (have_lock) + _dbus_keyring_unlock (keyring); + + if (! ((retval == TRUE && (error == NULL || error->name == NULL)) || + (retval == FALSE && (error == NULL || error->name != NULL)))) + { + if (error && error->name) + _dbus_verbose ("error is %s: %s\n", error->name, error->message); + _dbus_warn ("returning %d but error pointer %p name %s\n", + retval, error, error->name ? error->name : "(none)"); + _dbus_assert_not_reached ("didn't handle errors properly"); + } + + if (keys != NULL) + { + i = 0; + while (i < n_keys) + { + _dbus_string_free (&keys[i].secret); + ++i; + } + + dbus_free (keys); + } + + _dbus_string_free (&contents); + _dbus_string_free (&line); + + return retval; } /** @} */ /* end of internals */ @@ -291,7 +686,7 @@ _dbus_keyring_new_homedir (const DBusString *username, DBusKeyring *keyring; dbus_bool_t error_set; DBusString dotdir; - DBusString lock_extension; + DBusError tmp_error; keyring = NULL; error_set = FALSE; @@ -300,7 +695,6 @@ _dbus_keyring_new_homedir (const DBusString *username, return FALSE; _dbus_string_init_const (&dotdir, ".dbus-keyrings"); - _dbus_string_init_const (&lock_extension, ".lock"); if (username == NULL) { @@ -355,10 +749,30 @@ _dbus_keyring_new_homedir (const DBusString *username, &keyring->filename_lock, 0)) goto failed; - if (!_dbus_concat_dir_and_file (&keyring->filename_lock, - &lock_extension)) + if (!_dbus_string_append (&keyring->filename_lock, ".lock")) goto failed; + dbus_error_init (&tmp_error); + if (!_dbus_keyring_reload (keyring, FALSE, &tmp_error)) + { + _dbus_verbose ("didn't load an existing keyring: %s\n", + tmp_error.message); + dbus_error_free (&tmp_error); + } + + /* We don't fail fatally if we can't create the directory, + * but the keyring will probably always be empty + * unless someone else manages to create it + */ + dbus_error_init (&tmp_error); + if (!_dbus_create_directory (&keyring->directory, + &tmp_error)) + { + _dbus_verbose ("Creating keyring directory: %s\n", + tmp_error.message); + dbus_error_free (&tmp_error); + } + return keyring; failed: @@ -385,7 +799,7 @@ _dbus_keyring_new_homedir (const DBusString *username, dbus_bool_t _dbus_keyring_validate_context (const DBusString *context) { - if (_dbus_string_length (context) == 0) + if (_dbus_string_get_length (context) == 0) { _dbus_verbose ("context is zero-length\n"); return FALSE; @@ -436,7 +850,10 @@ find_recent_key (DBusKeyring *keyring) { DBusKey *key = &keyring->keys[i]; - if (tv_sec - NEW_KEY_TIMEOUT < key->creation_time) + _dbus_verbose ("Key %d is %ld seconds old\n", + i, tv_sec - key->creation_time); + + if ((tv_sec - NEW_KEY_TIMEOUT_SECONDS) < key->creation_time) return key; ++i; @@ -458,7 +875,7 @@ find_recent_key (DBusKeyring *keyring) */ int _dbus_keyring_get_best_key (DBusKeyring *keyring, - DBusError **error) + DBusError *error) { DBusKey *key; @@ -487,3 +904,125 @@ _dbus_keyring_get_best_key (DBusKeyring *keyring, /** @} */ /* end of exposed API */ +#ifdef DBUS_BUILD_TESTS +#include "dbus-test.h" +#include + +dbus_bool_t +_dbus_keyring_test (void) +{ + DBusString context; + DBusKeyring *ring1; + DBusKeyring *ring2; + int id; + DBusError error; + int i; + + ring1 = NULL; + ring2 = NULL; + + /* Context validation */ + + _dbus_string_init_const (&context, "foo"); + _dbus_assert (_dbus_keyring_validate_context (&context)); + _dbus_string_init_const (&context, "org_freedesktop_blah"); + _dbus_assert (_dbus_keyring_validate_context (&context)); + + _dbus_string_init_const (&context, ""); + _dbus_assert (!_dbus_keyring_validate_context (&context)); + _dbus_string_init_const (&context, ".foo"); + _dbus_assert (!_dbus_keyring_validate_context (&context)); + _dbus_string_init_const (&context, "bar.foo"); + _dbus_assert (!_dbus_keyring_validate_context (&context)); + _dbus_string_init_const (&context, "bar/foo"); + _dbus_assert (!_dbus_keyring_validate_context (&context)); + _dbus_string_init_const (&context, "bar\\foo"); + _dbus_assert (!_dbus_keyring_validate_context (&context)); + _dbus_string_init_const (&context, "foo\xfa\xf0"); + _dbus_assert (!_dbus_keyring_validate_context (&context)); + _dbus_string_init_const (&context, "foo\x80"); + _dbus_assert (!_dbus_keyring_validate_context (&context)); + _dbus_string_init_const (&context, "foo\x7f"); + _dbus_assert (_dbus_keyring_validate_context (&context)); + + if (!_dbus_string_init (&context, _DBUS_INT_MAX)) + _dbus_assert_not_reached ("no memory"); + if (!_dbus_string_append_byte (&context, '\0')) + _dbus_assert_not_reached ("no memory"); + _dbus_assert (!_dbus_keyring_validate_context (&context)); + _dbus_string_free (&context); + + /* Now verify that if we create a key in keyring 1, + * it is properly loaded in keyring 2 + */ + + _dbus_string_init_const (&context, "org_freedesktop_dbus_testsuite"); + dbus_error_init (&error); + ring1 = _dbus_keyring_new_homedir (NULL, &context, + &error); + _dbus_assert (ring1); + _dbus_assert (error.name == NULL); + + id = _dbus_keyring_get_best_key (ring1, &error); + if (id < 0) + { + fprintf (stderr, "Could not load keyring: %s\n", error.message); + dbus_error_free (&error); + goto failure; + } + + ring2 = _dbus_keyring_new_homedir (NULL, &context, &error); + _dbus_assert (ring2); + _dbus_assert (error.name == NULL); + + if (ring1->n_keys != ring2->n_keys) + { + fprintf (stderr, "Different number of keys in keyrings\n"); + goto failure; + } + + /* We guarantee we load and save keeping keys in a fixed + * order + */ + i = 0; + while (i < ring1->n_keys) + { + if (ring1->keys[i].id != ring2->keys[i].id) + { + fprintf (stderr, "Keyring 1 has first key ID %d and keyring 2 has %d\n", + ring1->keys[i].id, ring2->keys[i].id); + goto failure; + } + + if (ring1->keys[i].creation_time != ring2->keys[i].creation_time) + { + fprintf (stderr, "Keyring 1 has first key time %ld and keyring 2 has %ld\n", + ring1->keys[i].creation_time, ring2->keys[i].creation_time); + goto failure; + } + + if (!_dbus_string_equal (&ring1->keys[i].secret, + &ring2->keys[i].secret)) + { + fprintf (stderr, "Keyrings 1 and 2 have different secrets for same ID/timestamp\n"); + goto failure; + } + + ++i; + } + + printf (" %d keys in test\n", ring1->n_keys); + + return TRUE; + + failure: + if (ring1) + _dbus_keyring_unref (ring1); + if (ring2) + _dbus_keyring_unref (ring2); + + return FALSE; +} + +#endif /* DBUS_BUILD_TESTS */ + diff --git a/dbus/dbus-keyring.h b/dbus/dbus-keyring.h index 7ff4fdc4..28826aed 100644 --- a/dbus/dbus-keyring.h +++ b/dbus/dbus-keyring.h @@ -25,6 +25,7 @@ #include #include +#include DBUS_BEGIN_DECLS; @@ -37,7 +38,7 @@ void _dbus_keyring_ref (DBusKeyring *keyring); void _dbus_keyring_unref (DBusKeyring *keyring); dbus_bool_t _dbus_keyring_validate_context (const DBusString *context); int _dbus_keyring_get_best_key (DBusKeyring *keyring, - DBusError **error); + DBusError *error); dbus_bool_t _dbus_keyring_create_challenge (DBusString *challenge); dbus_bool_t _dbus_keyring_compute_response (DBusKeyring *keyring, int key_id, diff --git a/dbus/dbus-md5.c b/dbus/dbus-md5.c index 1d1c451f..b246b355 100644 --- a/dbus/dbus-md5.c +++ b/dbus/dbus-md5.c @@ -470,13 +470,6 @@ _dbus_md5_compute (const DBusString *data, { DBusMD5Context context; DBusString digest; - const char hexdigits[16] = { - '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', - 'a', 'b', 'c', 'd', 'e', 'f' - }; - unsigned char *p; - unsigned char *end; - int orig_len; _dbus_md5_init (&context); @@ -485,32 +478,17 @@ _dbus_md5_compute (const DBusString *data, if (!_dbus_string_init (&digest, _DBUS_INT_MAX)) return FALSE; - orig_len = _dbus_string_get_length (ascii_output); - if (!_dbus_md5_final (&context, &digest)) goto error; - _dbus_string_get_const_data (&digest, (const char **) &p); - end = p + 16; - - while (p != end) - { - if (!_dbus_string_append_byte (ascii_output, - hexdigits[(*p >> 4)])) - goto error; - - if (!_dbus_string_append_byte (ascii_output, - hexdigits[(*p & 0x0f)])) - goto error; - - ++p; - } + if (!_dbus_string_hex_encode (&digest, 0, ascii_output, + _dbus_string_get_length (ascii_output))) + goto error; return TRUE; error: _dbus_string_free (&digest); - _dbus_string_set_length (ascii_output, orig_len); return FALSE; } diff --git a/dbus/dbus-sha.c b/dbus/dbus-sha.c index d0a8e4fb..2035a475 100644 --- a/dbus/dbus-sha.c +++ b/dbus/dbus-sha.c @@ -484,13 +484,6 @@ _dbus_sha_compute (const DBusString *data, { DBusSHAContext context; DBusString digest; - const char hexdigits[16] = { - '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', - 'a', 'b', 'c', 'd', 'e', 'f' - }; - unsigned char *p; - unsigned char *end; - int orig_len; _dbus_sha_init (&context); @@ -499,32 +492,17 @@ _dbus_sha_compute (const DBusString *data, if (!_dbus_string_init (&digest, _DBUS_INT_MAX)) return FALSE; - orig_len = _dbus_string_get_length (ascii_output); - if (!_dbus_sha_final (&context, &digest)) goto error; - _dbus_string_get_const_data (&digest, (const char **) &p); - end = p + 20; - - while (p != end) - { - if (!_dbus_string_append_byte (ascii_output, - hexdigits[(*p >> 4)])) - goto error; - - if (!_dbus_string_append_byte (ascii_output, - hexdigits[(*p & 0x0f)])) - goto error; - - ++p; - } + if (!_dbus_string_hex_encode (&digest, 0, ascii_output, + _dbus_string_get_length (ascii_output))) + goto error; return TRUE; error: _dbus_string_free (&digest); - _dbus_string_set_length (ascii_output, orig_len); return FALSE; } diff --git a/dbus/dbus-string.c b/dbus/dbus-string.c index 7d8cfb8c..181a10cf 100644 --- a/dbus/dbus-string.c +++ b/dbus/dbus-string.c @@ -1961,7 +1961,6 @@ _dbus_string_base64_encode (const DBusString *source, return TRUE; } - /** * Decodes a string from Base64, as documented in RFC 2045. * @@ -2067,9 +2066,203 @@ _dbus_string_base64_decode (const DBusString *source, } /** - * Checks that the given range of the string - * is valid ASCII. If the given range is not contained - * in the string, returns #FALSE. + * Encodes a string in hex, the way MD5 and SHA-1 are usually + * encoded. (Each byte is two hex digits.) + * + * @param source the string to encode + * @param start byte index to start encoding + * @param dest string where encoded data should be placed + * @param insert_at where to place encoded data + * @returns #TRUE if encoding was successful, #FALSE if no memory etc. + */ +dbus_bool_t +_dbus_string_hex_encode (const DBusString *source, + int start, + DBusString *dest, + int insert_at) +{ + DBusString result; + const char hexdigits[16] = { + '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', + 'a', 'b', 'c', 'd', 'e', 'f' + }; + const unsigned char *p; + const unsigned char *end; + dbus_bool_t retval; + + _dbus_assert (start <= _dbus_string_get_length (source)); + + if (!_dbus_string_init (&result, _DBUS_INT_MAX)) + return FALSE; + + retval = FALSE; + + _dbus_string_get_const_data (source, (const char**) &p); + end = p + _dbus_string_get_length (source); + p += start; + + while (p != end) + { + if (!_dbus_string_append_byte (&result, + hexdigits[(*p >> 4)])) + goto out; + + if (!_dbus_string_append_byte (&result, + hexdigits[(*p & 0x0f)])) + goto out; + + ++p; + } + + if (!_dbus_string_move (&result, 0, dest, insert_at)) + goto out; + + retval = TRUE; + + out: + _dbus_string_free (&result); + return retval; +} + +/** + * Decodes a string from hex encoding. + * + * @param source the string to decode + * @param start byte index to start decode + * @param dest string where decoded data should be placed + * @param insert_at where to place decoded data + * @returns #TRUE if decoding was successful, #FALSE if no memory etc. + */ +dbus_bool_t +_dbus_string_hex_decode (const DBusString *source, + int start, + DBusString *dest, + int insert_at) +{ + DBusString result; + const unsigned char *p; + const unsigned char *end; + dbus_bool_t retval; + dbus_bool_t high_bits; + + _dbus_assert (start <= _dbus_string_get_length (source)); + + if (!_dbus_string_init (&result, _DBUS_INT_MAX)) + return FALSE; + + retval = FALSE; + + high_bits = TRUE; + _dbus_string_get_const_data (source, (const char**) &p); + end = p + _dbus_string_get_length (source); + p += start; + + while (p != end) + { + unsigned int val; + + switch (*p) + { + case '0': + val = 0; + break; + case '1': + val = 1; + break; + case '2': + val = 2; + break; + case '3': + val = 3; + break; + case '4': + val = 4; + break; + case '5': + val = 5; + break; + case '6': + val = 6; + break; + case '7': + val = 7; + break; + case '8': + val = 8; + break; + case '9': + val = 9; + break; + case 'a': + case 'A': + val = 10; + break; + case 'b': + case 'B': + val = 11; + break; + case 'c': + case 'C': + val = 12; + break; + case 'd': + case 'D': + val = 13; + break; + case 'e': + case 'E': + val = 14; + break; + case 'f': + case 'F': + val = 15; + break; + default: + val = 0; + _dbus_verbose ("invalid character '%c' in hex encoded text\n", + *p); + goto out; + } + + if (high_bits) + { + if (!_dbus_string_append_byte (&result, + val << 4)) + goto out; + } + else + { + int len; + unsigned char b; + + len = _dbus_string_get_length (&result); + + b = _dbus_string_get_byte (&result, len - 1); + + b |= val; + + _dbus_string_set_byte (&result, len - 1, b); + } + + high_bits = !high_bits; + + ++p; + } + + if (!_dbus_string_move (&result, 0, dest, insert_at)) + goto out; + + retval = TRUE; + + out: + _dbus_string_free (&result); + return retval; +} + +/** + * Checks that the given range of the string is valid ASCII with no + * nul bytes. If the given range is not contained in the string, + * returns #FALSE. * * @param str the string * @param start first byte index to check @@ -2250,6 +2443,97 @@ test_base64_roundtrip (const unsigned char *data, _dbus_string_free (&decoded); } +static void +test_hex_roundtrip (const unsigned char *data, + int len) +{ + DBusString orig; + DBusString encoded; + DBusString decoded; + + if (len < 0) + len = strlen (data); + + if (!_dbus_string_init (&orig, _DBUS_INT_MAX)) + _dbus_assert_not_reached ("could not init string"); + + if (!_dbus_string_init (&encoded, _DBUS_INT_MAX)) + _dbus_assert_not_reached ("could not init string"); + + if (!_dbus_string_init (&decoded, _DBUS_INT_MAX)) + _dbus_assert_not_reached ("could not init string"); + + if (!_dbus_string_append_len (&orig, data, len)) + _dbus_assert_not_reached ("couldn't append orig data"); + + if (!_dbus_string_hex_encode (&orig, 0, &encoded, 0)) + _dbus_assert_not_reached ("could not encode"); + + if (!_dbus_string_hex_decode (&encoded, 0, &decoded, 0)) + _dbus_assert_not_reached ("could not decode"); + + if (!_dbus_string_equal (&orig, &decoded)) + { + const char *s; + + printf ("Original string %d bytes encoded %d bytes decoded %d bytes\n", + _dbus_string_get_length (&orig), + _dbus_string_get_length (&encoded), + _dbus_string_get_length (&decoded)); + printf ("Original: %s\n", data); + _dbus_string_get_const_data (&decoded, &s); + printf ("Decoded: %s\n", s); + _dbus_assert_not_reached ("original string not the same as string decoded from base64"); + } + + _dbus_string_free (&orig); + _dbus_string_free (&encoded); + _dbus_string_free (&decoded); +} + +typedef void (* TestRoundtripFunc) (const unsigned char *data, + int len); +static void +test_roundtrips (TestRoundtripFunc func) +{ + (* func) ("Hello this is a string\n", -1); + (* func) ("Hello this is a string\n1", -1); + (* func) ("Hello this is a string\n12", -1); + (* func) ("Hello this is a string\n123", -1); + (* func) ("Hello this is a string\n1234", -1); + (* func) ("Hello this is a string\n12345", -1); + (* func) ("", 0); + (* func) ("1", 1); + (* func) ("12", 2); + (* func) ("123", 3); + (* func) ("1234", 4); + (* func) ("12345", 5); + (* func) ("", 1); + (* func) ("1", 2); + (* func) ("12", 3); + (* func) ("123", 4); + (* func) ("1234", 5); + (* func) ("12345", 6); + { + unsigned char buf[512]; + int i; + + i = 0; + while (i < _DBUS_N_ELEMENTS (buf)) + { + buf[i] = i; + ++i; + } + i = 0; + while (i < _DBUS_N_ELEMENTS (buf)) + { + (* func) (buf, i); + ++i; + } + } +} + + /** * @ingroup DBusStringInternals * Unit test for DBusString. @@ -2618,40 +2902,9 @@ _dbus_string_test (void) _dbus_string_free (&str); - /* Base 64 */ - test_base64_roundtrip ("Hello this is a string\n", -1); - test_base64_roundtrip ("Hello this is a string\n1", -1); - test_base64_roundtrip ("Hello this is a string\n12", -1); - test_base64_roundtrip ("Hello this is a string\n123", -1); - test_base64_roundtrip ("Hello this is a string\n1234", -1); - test_base64_roundtrip ("Hello this is a string\n12345", -1); - test_base64_roundtrip ("", 0); - test_base64_roundtrip ("1", 1); - test_base64_roundtrip ("12", 2); - test_base64_roundtrip ("123", 3); - test_base64_roundtrip ("1234", 4); - test_base64_roundtrip ("12345", 5); - test_base64_roundtrip ("", 1); - test_base64_roundtrip ("1", 2); - test_base64_roundtrip ("12", 3); - test_base64_roundtrip ("123", 4); - test_base64_roundtrip ("1234", 5); - test_base64_roundtrip ("12345", 6); - { - unsigned char buf[512]; - i = 0; - while (i < _DBUS_N_ELEMENTS (buf)) - { - buf[i] = i; - ++i; - } - i = 0; - while (i < _DBUS_N_ELEMENTS (buf)) - { - test_base64_roundtrip (buf, i); - ++i; - } - } + /* Base 64 and Hex encoding */ + test_roundtrips (test_base64_roundtrip); + test_roundtrips (test_hex_roundtrip); return TRUE; } diff --git a/dbus/dbus-string.h b/dbus/dbus-string.h index 4fef652a..25cdd4dd 100644 --- a/dbus/dbus-string.h +++ b/dbus/dbus-string.h @@ -147,6 +147,10 @@ dbus_bool_t _dbus_string_parse_int (const DBusString *str, int start, long *value_return, int *end_return); +dbus_bool_t _dbus_string_parse_uint (const DBusString *str, + int start, + unsigned long *value_return, + int *end_return); dbus_bool_t _dbus_string_parse_double (const DBusString *str, int start, double *value, @@ -199,6 +203,14 @@ dbus_bool_t _dbus_string_base64_decode (const DBusString *source, int start, DBusString *dest, int insert_at); +dbus_bool_t _dbus_string_hex_encode (const DBusString *source, + int start, + DBusString *dest, + int insert_at); +dbus_bool_t _dbus_string_hex_decode (const DBusString *source, + int start, + DBusString *dest, + int insert_at); dbus_bool_t _dbus_string_validate_ascii (const DBusString *str, int start, diff --git a/dbus/dbus-sysdeps.c b/dbus/dbus-sysdeps.c index 8c0567ed..4d610a4d 100644 --- a/dbus/dbus-sysdeps.c +++ b/dbus/dbus-sysdeps.c @@ -930,6 +930,45 @@ _dbus_string_parse_int (const DBusString *str, return TRUE; } +/** + * Parses an unsigned integer contained in a DBusString. Either return + * parameter may be #NULL if you aren't interested in it. The integer + * is parsed and stored in value_return. Return parameters are not + * initialized if the function returns #FALSE. + * + * @param str the string + * @param start the byte index of the start of the integer + * @param value_return return location of the integer value or #NULL + * @param end_return return location of the end of the integer, or #NULL + * @returns #TRUE on success + */ +dbus_bool_t +_dbus_string_parse_uint (const DBusString *str, + int start, + unsigned long *value_return, + int *end_return) +{ + unsigned long v; + const char *p; + char *end; + + _dbus_string_get_const_data_len (str, &p, start, + _dbus_string_get_length (str) - start); + + end = NULL; + errno = 0; + v = strtoul (p, &end, 0); + if (end == NULL || end == p || errno != 0) + return FALSE; + + if (value_return) + *value_return = v; + if (end_return) + *end_return = start + (end - p); + + return TRUE; +} + /** * Parses a floating point number contained in a DBusString. Either * return parameter may be #NULL if you aren't interested in it. The @@ -1623,8 +1662,39 @@ _dbus_file_get_contents (DBusString *str, } } +static dbus_bool_t +append_unique_chars (DBusString *str) +{ + static const char letters[] = + "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"; + int i; + int len; + +#define N_UNIQUE_CHARS 8 + + if (!_dbus_generate_random_bytes (str, N_UNIQUE_CHARS)) + return FALSE; + + len = _dbus_string_get_length (str); + i = len - N_UNIQUE_CHARS; + while (i < len) + { + _dbus_string_set_byte (str, i, + letters[_dbus_string_get_byte (str, i) % + (sizeof (letters) - 1)]); + + ++i; + } + + _dbus_assert (_dbus_string_validate_ascii (str, len - N_UNIQUE_CHARS, + N_UNIQUE_CHARS)); + + return TRUE; +} + /** - * Writes a string out to a file. + * Writes a string out to a file. If the file exists, + * it will be atomically overwritten by the new data. * * @param str the string to write out * @param filename the file to save string to @@ -1637,15 +1707,41 @@ _dbus_string_save_to_file (const DBusString *str, int fd; int bytes_to_write; const char *filename_c; + DBusString tmp_filename; + const char *tmp_filename_c; int total; + DBusResultCode result; + dbus_bool_t need_unlink; + + fd = -1; + result = DBUS_RESULT_FAILED; + need_unlink = FALSE; + + if (!_dbus_string_init (&tmp_filename, _DBUS_INT_MAX)) + return DBUS_RESULT_NO_MEMORY; - _dbus_string_get_const_data (filename, &filename_c); + if (!_dbus_string_copy (filename, 0, &tmp_filename, 0)) + return DBUS_RESULT_NO_MEMORY; - fd = open (filename_c, O_WRONLY | O_BINARY | O_EXCL | O_CREAT, + if (!_dbus_string_append (&tmp_filename, ".")) + return DBUS_RESULT_NO_MEMORY; + + if (!append_unique_chars (&tmp_filename)) + return DBUS_RESULT_NO_MEMORY; + + _dbus_string_get_const_data (filename, &filename_c); + _dbus_string_get_const_data (&tmp_filename, &tmp_filename_c); + + fd = open (tmp_filename_c, O_WRONLY | O_BINARY | O_EXCL | O_CREAT, 0600); if (fd < 0) - return _dbus_result_from_errno (errno); + { + result = _dbus_result_from_errno (errno); + goto out; + } + need_unlink = TRUE; + total = 0; bytes_to_write = _dbus_string_get_length (str); @@ -1665,15 +1761,45 @@ _dbus_string_save_to_file (const DBusString *str, _dbus_verbose ("write() failed: %s", _dbus_strerror (errno)); - close (fd); - return result; + goto out; } total += bytes_written; } - close (fd); - return DBUS_RESULT_SUCCESS; + if (close (fd) < 0) + { + _dbus_verbose ("close() failed: %s\n", _dbus_strerror (errno)); + goto out; + } + + fd = -1; + + if (rename (tmp_filename_c, filename_c) < 0) + { + _dbus_verbose ("rename() failed: %s\n", _dbus_strerror (errno)); + goto out; + } + + need_unlink = FALSE; + + result = DBUS_RESULT_SUCCESS; + + out: + /* close first, then unlink, to prevent ".nfs34234235" garbage + * files + */ + + if (fd >= 0) + close (fd); + + if (need_unlink && unlink (tmp_filename_c) < 0) + _dbus_verbose ("Failed to unlink temp file %s: %s\n", + tmp_filename_c, _dbus_strerror (errno)); + + _dbus_string_free (&tmp_filename); + + return result; } /** Creates the given file, failing if the file already exists. @@ -1733,7 +1859,42 @@ _dbus_delete_file (const DBusString *filename, _dbus_string_get_const_data (filename, &filename_c); if (unlink (filename_c) < 0) - return FALSE; + { + dbus_set_error (error, DBUS_ERROR_FAILED, + "Failed to delete file %s: %s\n", + filename_c, _dbus_strerror (errno)); + return FALSE; + } + else + return TRUE; +} + +/** + * Creates a directory; succeeds if the directory + * is created or already existed. + * + * @param filename directory filename + * @param error initialized error object + * @returns #TRUE on success + */ +dbus_bool_t +_dbus_create_directory (const DBusString *filename, + DBusError *error) +{ + const char *filename_c; + + _dbus_string_get_const_data (filename, &filename_c); + + if (mkdir (filename_c, 0700) < 0) + { + if (errno == EEXIST) + return TRUE; + + dbus_set_error (error, DBUS_ERROR_FAILED, + "Failed to create directory %s: %s\n", + filename_c, _dbus_strerror (errno)); + return FALSE; + } else return TRUE; } @@ -1907,6 +2068,8 @@ _dbus_generate_random_bytes (DBusString *str, int i; /* fall back to pseudorandom */ + _dbus_verbose ("Falling back to pseudorandom for %d bytes\n", + n_bytes); _dbus_get_current_time (NULL, &tv_usec); srand (tv_usec); @@ -1915,7 +2078,7 @@ _dbus_generate_random_bytes (DBusString *str, while (i < n_bytes) { double r; - int b; + unsigned int b; r = rand (); b = (r / (double) RAND_MAX) * 255.0; @@ -1933,6 +2096,9 @@ _dbus_generate_random_bytes (DBusString *str, if (_dbus_read (fd, str, n_bytes) != n_bytes) goto failed; + _dbus_verbose ("Read %d bytes from /dev/urandom\n", + n_bytes); + close (fd); return TRUE; @@ -1948,6 +2114,9 @@ _dbus_generate_random_bytes (DBusString *str, /** * A wrapper around strerror() * + * @todo get rid of this function, it's the same as + * _dbus_strerror(). + * * @param errnum the errno * @returns an error message (never #NULL) */ @@ -1963,6 +2132,25 @@ _dbus_errno_to_string (int errnum) return msg; } +/** + * A wrapper around strerror() because some platforms + * may be lame and not have strerror(). + * + * @param error_number errno. + * @returns error description. + */ +const char* +_dbus_strerror (int error_number) +{ + const char *msg; + + msg = strerror (error_number); + if (msg == NULL) + msg = "unknown"; + + return msg; +} + /* Avoids a danger in threaded situations (calling close() * on a file descriptor twice, and another thread has * re-opened it since the first close) diff --git a/dbus/dbus-sysdeps.h b/dbus/dbus-sysdeps.h index b14833eb..fb8362e2 100644 --- a/dbus/dbus-sysdeps.h +++ b/dbus/dbus-sysdeps.h @@ -147,6 +147,8 @@ dbus_bool_t _dbus_create_file_exclusively (const DBusString *filename, DBusError *error); dbus_bool_t _dbus_delete_file (const DBusString *filename, DBusError *error); +dbus_bool_t _dbus_create_directory (const DBusString *filename, + DBusError *error); dbus_bool_t _dbus_concat_dir_and_file (DBusString *dir, const DBusString *next_component); diff --git a/dbus/dbus-test.c b/dbus/dbus-test.c index 05de8c75..b1f8ac1d 100644 --- a/dbus/dbus-test.c +++ b/dbus/dbus-test.c @@ -59,6 +59,10 @@ dbus_internal_do_not_use_run_tests (const char *test_data_dir) if (!_dbus_string_test ()) die ("strings"); + printf ("%s: running keyring tests\n", "dbus-test"); + if (!_dbus_keyring_test ()) + die ("keyring"); + printf ("%s: running md5 tests\n", "dbus-test"); if (!_dbus_md5_test ()) die ("md5"); diff --git a/dbus/dbus-test.h b/dbus/dbus-test.h index 1ea30dc9..b5e38480 100644 --- a/dbus/dbus-test.h +++ b/dbus/dbus-test.h @@ -45,6 +45,7 @@ dbus_bool_t _dbus_message_test (const char *test_data_dir); dbus_bool_t _dbus_auth_test (const char *test_data_dir); dbus_bool_t _dbus_md5_test (void); dbus_bool_t _dbus_sha_test (const char *test_data_dir); +dbus_bool_t _dbus_keyring_test (void); void dbus_internal_do_not_use_run_tests (const char *test_data_dir); dbus_bool_t dbus_internal_do_not_use_try_message_file (const DBusString *filename, -- cgit