diff --git a/agent/ChangeLog b/agent/ChangeLog index 59d1b31f2..db77fe014 100644 --- a/agent/ChangeLog +++ b/agent/ChangeLog @@ -1,3 +1,21 @@ +2010-09-02 Werner Koch + + * cache.c (new_data): Change arg and callers to use a string and + explicity return an error code. We never used raw binary data and + thus it is easier to use a string. Adjust callers. + (initialize_module_cache, deinitialize_module_cache): New. + (new_data): Encrypt the cached data. + (struct cache_item_s): Remove field LOCKCOUNT. Change all users + accordingly. + (agent_unlock_cache_entry): Remove. + (agent_get_cache): Return an allocated string and remove CACHE_ID. + * genkey.c (agent_genkey): Remove cache marker stuff. + * findkey.c (unprotect): Ditto. + * cvt-openpgp.c (convert_openpgp): Ditto. + * command.c (cmd_get_passphrase): Ditto. + * gpg-agent.c (main, cleanup): Initialize and deinitialize the + cache module. + 2010-09-01 Werner Koch * call-pinentry.c (start_pinentry): Disable pinentry logging. diff --git a/agent/agent.h b/agent/agent.h index e3e46abc9..6c2e7c65e 100644 --- a/agent/agent.h +++ b/agent/agent.h @@ -264,12 +264,12 @@ void agent_popup_message_stop (ctrl_t ctrl); /*-- cache.c --*/ +void initialize_module_cache (void); +void deinitialize_module_cache (void); void agent_flush_cache (void); int agent_put_cache (const char *key, cache_mode_t cache_mode, const char *data, int ttl); -const char *agent_get_cache (const char *key, cache_mode_t cache_mode, - void **cache_id); -void agent_unlock_cache_entry (void **cache_id); +char *agent_get_cache (const char *key, cache_mode_t cache_mode); /*-- pksign.c --*/ diff --git a/agent/cache.c b/agent/cache.c index c96087a36..19f250217 100644 --- a/agent/cache.c +++ b/agent/cache.c @@ -24,13 +24,31 @@ #include #include #include +#include #include "agent.h" +/* The size of the encryption key in bytes. */ +#define ENCRYPTION_KEYSIZE (128/8) + +/* A mutex used to protect the encryption. This is required because + we use one context to do all encryption and decryption. */ +static pth_mutex_t encryption_lock; +/* The encryption context. This is the only place where the + encryption key for all cached entries is available. It would nice + to keep this (or just the key) in some hardware device, for example + a TPM. Libgcrypt could be extended to provide such a service. + With the current scheme it is easy to retrieve the cached entries + if access to Libgcrypt's memory is available. The encryption + merely avoids grepping for clear texts in the memory. Nevertheless + the encryption provides the necessary infrastructure to make it + more secure. */ +static gcry_cipher_hd_t encryption_handle; + + struct secret_data_s { - int totallen; /* this includes the padding */ - int datalen; /* actual data length */ - char data[1]; + int totallen; /* This includes the padding and space for AESWRAP. */ + char data[1]; /* A string. */ }; typedef struct cache_item_s *ITEM; @@ -39,47 +57,116 @@ struct cache_item_s { time_t created; time_t accessed; int ttl; /* max. lifetime given in seconds, -1 one means infinite */ - int lockcount; struct secret_data_s *pw; cache_mode_t cache_mode; char key[1]; }; - +/* The cache himself. */ static ITEM thecache; +/* This function must be called once to initialize this module. It + has to be done before a second thread is spawned. */ +void +initialize_module_cache (void) +{ + static int initialized; + gpg_error_t err; + void *key; + + if (!pth_mutex_init (&encryption_lock)) + err = gpg_error_from_syserror (); + else + err = gcry_cipher_open (&encryption_handle, GCRY_CIPHER_AES128, + GCRY_CIPHER_MODE_AESWRAP, GCRY_CIPHER_SECURE); + if (!err) + { + key = gcry_random_bytes (ENCRYPTION_KEYSIZE, GCRY_STRONG_RANDOM); + if (!key) + err = gpg_error_from_syserror (); + else + { + err = gcry_cipher_setkey (encryption_handle, key, ENCRYPTION_KEYSIZE); + xfree (key); + } + } + if (err) + log_fatal ("error initializing cache encryption context: %s\n", + gpg_strerror (err)); + initialized = 1; +} + + +void +deinitialize_module_cache (void) +{ + gcry_cipher_close (encryption_handle); + encryption_handle = NULL; +} + + static void release_data (struct secret_data_s *data) { xfree (data); } -static struct secret_data_s * -new_data (const void *data, size_t length) +static gpg_error_t +new_data (const char *string, struct secret_data_s **r_data) { - struct secret_data_s *d; + gpg_error_t err; + struct secret_data_s *d, *d_enc; + size_t length; int total; + + *r_data = NULL; - /* we pad the data to 32 bytes so that it get more complicated + if (!encryption_handle) + return gpg_error (GPG_ERR_NOT_INITIALIZED); + + length = strlen (string) + 1; + + /* We pad the data to 32 bytes so that it get more complicated finding something out by watching allocation patterns. This is - usally not possible but we better assume nothing about our - secure storage provider*/ - total = length + 32 - (length % 32); + usally not possible but we better assume nothing about our secure + storage provider. To support the AESWRAP mode we need to add 8 + extra bytes as well. */ + total = (length + 8) + 32 - ((length+8) % 32); - d = gcry_malloc_secure (sizeof *d + total - 1); - if (d) + d = xtrymalloc_secure (sizeof *d + total - 1); + if (!d) + return gpg_error_from_syserror (); + memcpy (d->data, string, length); + + d_enc = xtrymalloc (sizeof *d_enc + total - 1); + if (!d_enc) { - d->totallen = total; - d->datalen = length; - memcpy (d->data, data, length); + err = gpg_error_from_syserror (); + xfree (d); + return err; } - return d; + + d_enc->totallen = total; + if (!pth_mutex_acquire (&encryption_lock, 0, NULL)) + log_fatal ("failed to acquire cache encryption mutex\n"); + err = gcry_cipher_encrypt (encryption_handle, d_enc->data, total, + d->data, total - 8); + xfree (d); + if (!pth_mutex_release (&encryption_lock)) + log_fatal ("failed to release cache encryption mutex\n"); + if (err) + { + xfree (d_enc); + return err; + } + *r_data = d_enc; + return 0; } -/* check whether there are items to expire */ +/* Check whether there are items to expire. */ static void housekeeping (void) { @@ -89,8 +176,7 @@ housekeeping (void) /* First expire the actual data */ for (r=thecache; r; r = r->next) { - if (!r->lockcount && r->pw - && r->ttl >= 0 && r->accessed + r->ttl < current) + if (r->pw && r->ttl >= 0 && r->accessed + r->ttl < current) { if (DBG_CACHE) log_debug (" expired `%s' (%ds after last access)\n", @@ -112,7 +198,7 @@ housekeeping (void) case CACHE_MODE_SSH: maxttl = opt.max_cache_ttl_ssh; break; default: maxttl = opt.max_cache_ttl; break; } - if (!r->lockcount && r->pw && r->created + maxttl < current) + if (r->pw && r->created + maxttl < current) { if (DBG_CACHE) log_debug (" expired `%s' (%lus after creation)\n", @@ -129,28 +215,16 @@ housekeeping (void) { if (!r->pw && r->ttl >= 0 && r->accessed + 60*30 < current) { - if (r->lockcount) - { - log_error ("can't remove unused cache entry `%s' (mode %d) due to" - " lockcount=%d\n", - r->key, r->cache_mode, r->lockcount); - r->accessed += 60*10; /* next error message in 10 minutes */ - rprev = r; - r = r->next; - } + ITEM r2 = r->next; + if (DBG_CACHE) + log_debug (" removed `%s' (mode %d) (slot not used for 30m)\n", + r->key, r->cache_mode); + xfree (r); + if (!rprev) + thecache = r2; else - { - ITEM r2 = r->next; - if (DBG_CACHE) - log_debug (" removed `%s' (mode %d) (slot not used for 30m)\n", - r->key, r->cache_mode); - xfree (r); - if (!rprev) - thecache = r2; - else - rprev->next = r2; - r = r2; - } + rprev->next = r2; + r = r2; } else { @@ -171,7 +245,7 @@ agent_flush_cache (void) for (r=thecache; r; r = r->next) { - if (!r->lockcount && r->pw) + if (r->pw) { if (DBG_CACHE) log_debug (" flushing `%s'\n", r->key); @@ -179,28 +253,22 @@ agent_flush_cache (void) r->pw = NULL; r->accessed = 0; } - else if (r->lockcount && r->pw) - { - if (DBG_CACHE) - log_debug (" marked `%s' for flushing\n", r->key); - r->accessed = 0; - r->ttl = 0; - } } } -/* Store DATA of length DATALEN in the cache under KEY and mark it - with a maximum lifetime of TTL seconds. If there is already data - under this key, it will be replaced. Using a DATA of NULL deletes - the entry. A TTL of 0 is replaced by the default TTL and a TTL of - -1 set infinite timeout. CACHE_MODE is stored with the cache entry +/* Store the string DATA in the cache under KEY and mark it with a + maximum lifetime of TTL seconds. If there is already data under + this key, it will be replaced. Using a DATA of NULL deletes the + entry. A TTL of 0 is replaced by the default TTL and a TTL of -1 + set infinite timeout. CACHE_MODE is stored with the cache entry and used to select different timeouts. */ int agent_put_cache (const char *key, cache_mode_t cache_mode, const char *data, int ttl) { + gpg_error_t err = 0; ITEM r; if (DBG_CACHE) @@ -221,15 +289,14 @@ agent_put_cache (const char *key, cache_mode_t cache_mode, for (r=thecache; r; r = r->next) { - if (!r->lockcount - && ((cache_mode != CACHE_MODE_USER - && cache_mode != CACHE_MODE_NONCE) - || r->cache_mode == cache_mode) + if (((cache_mode != CACHE_MODE_USER + && cache_mode != CACHE_MODE_NONCE) + || r->cache_mode == cache_mode) && !strcmp (r->key, key)) break; } - if (r) - { /* replace */ + if (r) /* Replace. */ + { if (r->pw) { release_data (r->pw); @@ -240,46 +307,47 @@ agent_put_cache (const char *key, cache_mode_t cache_mode, r->created = r->accessed = gnupg_get_time (); r->ttl = ttl; r->cache_mode = cache_mode; - r->pw = new_data (data, strlen (data)+1); - if (!r->pw) - log_error ("out of core while allocating new cache item\n"); + err = new_data (data, &r->pw); + if (err) + log_error ("error replacing cache item: %s\n", gpg_strerror (err)); } } - else if (data) - { /* simply insert */ + else if (data) /* Insert. */ + { r = xtrycalloc (1, sizeof *r + strlen (key)); if (!r) - log_error ("out of core while allocating new cache control\n"); + err = gpg_error_from_syserror (); else { strcpy (r->key, key); r->created = r->accessed = gnupg_get_time (); r->ttl = ttl; r->cache_mode = cache_mode; - r->pw = new_data (data, strlen (data)+1); - if (!r->pw) - { - log_error ("out of core while allocating new cache item\n"); - xfree (r); - } + err = new_data (data, &r->pw); + if (err) + xfree (r); else { r->next = thecache; thecache = r; } } + if (err) + log_error ("error inserting cache item: %s\n", gpg_strerror (err)); } - return 0; + return err; } /* Try to find an item in the cache. Note that we currently don't make use of CACHE_MODE except for CACHE_MODE_NONCE and CACHE_MODE_USER. */ -const char * -agent_get_cache (const char *key, cache_mode_t cache_mode, void **cache_id) +char * +agent_get_cache (const char *key, cache_mode_t cache_mode) { + gpg_error_t err; ITEM r; + char *value = NULL; if (cache_mode == CACHE_MODE_IGNORE) return NULL; @@ -288,67 +356,46 @@ agent_get_cache (const char *key, cache_mode_t cache_mode, void **cache_id) log_debug ("agent_get_cache `%s' (mode %d) ...\n", key, cache_mode); housekeeping (); - /* first try to find one with no locks - this is an updated cache - entry: We might have entries with a lockcount and without a - lockcount. */ for (r=thecache; r; r = r->next) { - if (!r->lockcount && r->pw + if (r->pw && ((cache_mode != CACHE_MODE_USER && cache_mode != CACHE_MODE_NONCE) || r->cache_mode == cache_mode) && !strcmp (r->key, key)) { - /* put_cache does only put strings into the cache, so we - don't need the lengths */ r->accessed = gnupg_get_time (); if (DBG_CACHE) log_debug ("... hit\n"); - r->lockcount++; - *cache_id = r; - return r->pw->data; - } - } - /* again, but this time get even one with a lockcount set */ - for (r=thecache; r; r = r->next) - { - if (r->pw - && ((cache_mode != CACHE_MODE_USER - && cache_mode != CACHE_MODE_NONCE) - || r->cache_mode == cache_mode) - && !strcmp (r->key, key)) - { - r->accessed = gnupg_get_time (); - if (DBG_CACHE) - log_debug ("... hit (locked)\n"); - r->lockcount++; - *cache_id = r; - return r->pw->data; + if (r->pw->totallen < 32) + err = gpg_error (GPG_ERR_INV_LENGTH); + else if (!encryption_handle) + err = gpg_error (GPG_ERR_NOT_INITIALIZED); + else if (!(value = xtrymalloc_secure (r->pw->totallen - 8))) + err = gpg_error_from_syserror (); + else + { + if (!pth_mutex_acquire (&encryption_lock, 0, NULL)) + log_fatal ("failed to acquire cache encryption mutex\n"); + err = gcry_cipher_decrypt (encryption_handle, + value, r->pw->totallen - 8, + r->pw->data, r->pw->totallen); + if (!pth_mutex_release (&encryption_lock)) + log_fatal ("failed to release cache encryption mutex\n"); + } + if (err) + { + xfree (value); + value = NULL; + log_error ("retrieving cache entry `%s' failed: %s\n", + key, gpg_strerror (err)); + } + return value; } } if (DBG_CACHE) log_debug ("... miss\n"); - *cache_id = NULL; return NULL; } - -void -agent_unlock_cache_entry (void **cache_id) -{ - ITEM r; - - for (r=thecache; r; r = r->next) - { - if (r == *cache_id) - { - if (!r->lockcount) - log_error ("trying to unlock non-locked cache entry `%s'\n", - r->key); - else - r->lockcount--; - return; - } - } -} diff --git a/agent/command.c b/agent/command.c index 1446b9090..a8827e5de 100644 --- a/agent/command.c +++ b/agent/command.c @@ -1069,12 +1069,11 @@ cmd_get_passphrase (assuan_context_t ctx, char *line) { ctrl_t ctrl = assuan_get_pointer (ctx); int rc; - const char *pw; + char *pw; char *response; char *cacheid = NULL, *desc = NULL, *prompt = NULL, *errtext = NULL; const char *desc2 = _("Please re-enter this passphrase"); char *p; - void *cache_marker; int opt_data, opt_check, opt_no_ask, opt_qualbar; int opt_repeat = 0; char *repeat_errtext = NULL; @@ -1135,12 +1134,11 @@ cmd_get_passphrase (assuan_context_t ctx, char *line) if (!strcmp (desc, "X")) desc = NULL; - pw = cacheid ? agent_get_cache (cacheid, CACHE_MODE_NORMAL, &cache_marker) - : NULL; + pw = cacheid ? agent_get_cache (cacheid, CACHE_MODE_NORMAL) : NULL; if (pw) { rc = send_back_passphrase (ctx, opt_data, pw); - agent_unlock_cache_entry (&cache_marker); + xfree (pw); } else if (opt_no_ask) rc = gpg_error (GPG_ERR_NO_DATA); diff --git a/agent/cvt-openpgp.c b/agent/cvt-openpgp.c index a1678ea16..fee93e27d 100644 --- a/agent/cvt-openpgp.c +++ b/agent/cvt-openpgp.c @@ -766,16 +766,14 @@ convert_openpgp (ctrl_t ctrl, gcry_sexp_t s_pgp, err = gpg_error (GPG_ERR_BAD_PASSPHRASE); if (cache_nonce) { - void *cache_marker = NULL; - const char *cache_value; + char *cache_value; - cache_value = agent_get_cache (cache_nonce, CACHE_MODE_NONCE, - &cache_marker); + cache_value = agent_get_cache (cache_nonce, CACHE_MODE_NONCE); if (cache_value) { if (strlen (cache_value) < pi->max_length) strcpy (pi->pin, cache_value); - agent_unlock_cache_entry (&cache_marker); + xfree (cache_value); } if (*pi->pin) err = try_do_unprotect_cb (pi); diff --git a/agent/findkey.c b/agent/findkey.c index 5f98d59d6..02aea24e5 100644 --- a/agent/findkey.c +++ b/agent/findkey.c @@ -291,14 +291,13 @@ unprotect (ctrl_t ctrl, const char *cache_nonce, const char *desc_text, /* Initially try to get it using a cache nonce. */ if (cache_nonce) { - void *cache_marker; - const char *pw; + char *pw; - pw = agent_get_cache (cache_nonce, CACHE_MODE_NONCE, &cache_marker); + pw = agent_get_cache (cache_nonce, CACHE_MODE_NONCE); if (pw) { rc = agent_unprotect (*keybuf, pw, NULL, &result, &resultlen); - agent_unlock_cache_entry (&cache_marker); + xfree (pw); if (!rc) { xfree (*keybuf); @@ -312,15 +311,14 @@ unprotect (ctrl_t ctrl, const char *cache_nonce, const char *desc_text, unprotect it, we fall back to ask the user */ if (cache_mode != CACHE_MODE_IGNORE) { - void *cache_marker; - const char *pw; + char *pw; retry: - pw = agent_get_cache (hexgrip, cache_mode, &cache_marker); + pw = agent_get_cache (hexgrip, cache_mode); if (pw) { rc = agent_unprotect (*keybuf, pw, NULL, &result, &resultlen); - agent_unlock_cache_entry (&cache_marker); + xfree (pw); if (!rc) { xfree (*keybuf); diff --git a/agent/genkey.c b/agent/genkey.c index f46974e77..0a35643e5 100644 --- a/agent/genkey.c +++ b/agent/genkey.c @@ -359,7 +359,7 @@ agent_genkey (ctrl_t ctrl, const char *cache_nonce, membuf_t *outbuf) { gcry_sexp_t s_keyparam, s_key, s_private, s_public; - char *passphrase = NULL; + char *passphrase; int rc; size_t len; char *buf; @@ -372,21 +372,7 @@ agent_genkey (ctrl_t ctrl, const char *cache_nonce, } /* Get the passphrase now, cause key generation may take a while. */ - if (cache_nonce) - { - void *cache_marker = NULL; - const char *cache_value; - - cache_value = agent_get_cache (cache_nonce, CACHE_MODE_NONCE, - &cache_marker); - if (cache_value) - { - passphrase = xtrymalloc_secure (strlen (cache_value)+1); - if (passphrase) - strcpy (passphrase, cache_value); - agent_unlock_cache_entry (&cache_marker); - } - } + passphrase = cache_nonce? agent_get_cache (cache_nonce, CACHE_MODE_NONCE):NULL; if (passphrase) rc = 0; else diff --git a/agent/gpg-agent.c b/agent/gpg-agent.c index 71b0274bb..0e64939f5 100644 --- a/agent/gpg-agent.c +++ b/agent/gpg-agent.c @@ -440,6 +440,7 @@ remove_socket (char *name) static void cleanup (void) { + deinitialize_module_cache (); remove_socket (socket_name); remove_socket (socket_name_ssh); } @@ -842,6 +843,7 @@ main (int argc, char **argv ) exit (1); } + initialize_module_cache (); initialize_module_call_pinentry (); initialize_module_call_scd (); initialize_module_trustlist (); diff --git a/common/ChangeLog b/common/ChangeLog index 62fc480dd..35dd2d8a4 100644 --- a/common/ChangeLog +++ b/common/ChangeLog @@ -1,3 +1,7 @@ +2010-09-02 Werner Koch + + * util.h (GPG_ERR_NOT_INITIALIZED): Define if not defined. + 2010-09-01 Marcus Brinkmann * estream.c (_es_set_std_fd): Disable debug output. diff --git a/common/util.h b/common/util.h index fdea333b5..341df2531 100644 --- a/common/util.h +++ b/common/util.h @@ -30,6 +30,10 @@ #ifndef GPG_ERR_LIMIT_REACHED #define GPG_ERR_LIMIT_REACHED 183 #endif +#ifndef GPG_ERR_NOT_INITIALIZED +#define GPG_ERR_NOT_INITIALIZED 184 +#endif + /* Hash function used with libksba. */ #define HASH_FNC ((void (*)(void *, const void*,size_t))gcry_md_write)