[svn] GnuPG - r5413 - in trunk: agent common
svn author wk
cvs at cvs.gnupg.org
Thu Sep 2 12:46:24 CEST 2010
Author: wk
Date: 2010-09-02 12:46:23 +0200 (Thu, 02 Sep 2010)
New Revision: 5413
Modified:
trunk/agent/ChangeLog
trunk/agent/agent.h
trunk/agent/cache.c
trunk/agent/command.c
trunk/agent/cvt-openpgp.c
trunk/agent/findkey.c
trunk/agent/genkey.c
trunk/agent/gpg-agent.c
trunk/common/ChangeLog
trunk/common/util.h
Log:
Obscure the cached passphrases.
Modified: trunk/agent/ChangeLog
===================================================================
--- trunk/agent/ChangeLog 2010-09-01 12:49:05 UTC (rev 5412)
+++ trunk/agent/ChangeLog 2010-09-02 10:46:23 UTC (rev 5413)
@@ -1,3 +1,21 @@
+2010-09-02 Werner Koch <wk at g10code.com>
+
+ * 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 <wk at g10code.com>
* call-pinentry.c (start_pinentry): Disable pinentry logging.
Modified: trunk/common/ChangeLog
===================================================================
--- trunk/common/ChangeLog 2010-09-01 12:49:05 UTC (rev 5412)
+++ trunk/common/ChangeLog 2010-09-02 10:46:23 UTC (rev 5413)
@@ -1,3 +1,7 @@
+2010-09-02 Werner Koch <wk at g10code.com>
+
+ * util.h (GPG_ERR_NOT_INITIALIZED): Define if not defined.
+
2010-09-01 Marcus Brinkmann <marcus at g10code.de>
* estream.c (_es_set_std_fd): Disable debug output.
Modified: trunk/agent/agent.h
===================================================================
--- trunk/agent/agent.h 2010-09-01 12:49:05 UTC (rev 5412)
+++ trunk/agent/agent.h 2010-09-02 10:46:23 UTC (rev 5413)
@@ -264,12 +264,12 @@
/*-- 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 --*/
Modified: trunk/agent/cache.c
===================================================================
--- trunk/agent/cache.c 2010-09-01 12:49:05 UTC (rev 5412)
+++ trunk/agent/cache.c 2010-09-02 10:46:23 UTC (rev 5413)
@@ -24,13 +24,31 @@
#include <string.h>
#include <time.h>
#include <assert.h>
+#include <pth.h>
#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 @@
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 @@
/* 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 @@
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 @@
{
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 @@
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 @@
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 @@
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 @@
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 @@
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;
+ 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;
}
}
- /* 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 (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;
- }
- }
-}
Modified: trunk/agent/command.c
===================================================================
--- trunk/agent/command.c 2010-09-01 12:49:05 UTC (rev 5412)
+++ trunk/agent/command.c 2010-09-02 10:46:23 UTC (rev 5413)
@@ -1069,12 +1069,11 @@
{
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 @@
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);
Modified: trunk/agent/cvt-openpgp.c
===================================================================
--- trunk/agent/cvt-openpgp.c 2010-09-01 12:49:05 UTC (rev 5412)
+++ trunk/agent/cvt-openpgp.c 2010-09-02 10:46:23 UTC (rev 5413)
@@ -766,16 +766,14 @@
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);
Modified: trunk/agent/findkey.c
===================================================================
--- trunk/agent/findkey.c 2010-09-01 12:49:05 UTC (rev 5412)
+++ trunk/agent/findkey.c 2010-09-02 10:46:23 UTC (rev 5413)
@@ -291,14 +291,13 @@
/* 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 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);
Modified: trunk/agent/genkey.c
===================================================================
--- trunk/agent/genkey.c 2010-09-01 12:49:05 UTC (rev 5412)
+++ trunk/agent/genkey.c 2010-09-02 10:46:23 UTC (rev 5413)
@@ -359,7 +359,7 @@
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 @@
}
/* 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
Modified: trunk/agent/gpg-agent.c
===================================================================
--- trunk/agent/gpg-agent.c 2010-09-01 12:49:05 UTC (rev 5412)
+++ trunk/agent/gpg-agent.c 2010-09-02 10:46:23 UTC (rev 5413)
@@ -440,6 +440,7 @@
static void
cleanup (void)
{
+ deinitialize_module_cache ();
remove_socket (socket_name);
remove_socket (socket_name_ssh);
}
@@ -842,6 +843,7 @@
exit (1);
}
+ initialize_module_cache ();
initialize_module_call_pinentry ();
initialize_module_call_scd ();
initialize_module_trustlist ();
Modified: trunk/common/util.h
===================================================================
--- trunk/common/util.h 2010-09-01 12:49:05 UTC (rev 5412)
+++ trunk/common/util.h 2010-09-02 10:46:23 UTC (rev 5413)
@@ -30,7 +30,11 @@
#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)
More information about the Gnupg-commits
mailing list