2003-02-23 Havoc Pennington <hp@pobox.com>

	* 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

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  <hp@pobox.com>
+
+	* 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  <hp@pobox.com>
 
 	* 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
@@ -181,19 +181,6 @@ _dbus_verbose_real (const char *format,
 }
 
 /**
- * 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.
  *
  * @todo should cover more errnos, specifically those
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 (&timestamp, 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, &timestamp, &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 <stdio.h>
+
+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 <dbus/dbus-macros.h>
 #include <dbus/dbus-errors.h>
+#include <dbus/dbus-string.h>
 
 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
@@ -931,6 +931,45 @@ _dbus_string_parse_int (const DBusString *str,
 }
 
 /**
+ * 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
  * integer is parsed and stored in value_return. Return parameters are
@@ -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,