From 02dce8c0cc57deb2095a9b06aeb8f4dea34eef7e Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Tue, 27 Mar 2018 08:40:58 +0200 Subject: [PATCH] agent: Make the request origin a part of the cache items. * agent/cache.c (agent_put_cache): Add arg 'ctrl' and change all callers to pass it. (agent_get_cache): Ditto. * agent/cache.c (struct cache_items_s): Add field 'restricted'. (housekeeping): Adjust debug output. (agent_flush_cache): Ditto. (agent_put_cache): Ditto. Take RESTRICTED into account. (agent_get_cache): Ditto. -- If requests are coming from different sources they should not share the same cache. This way we make sure that a Pinentry pops up for a remote request to a key we have already used locally. GnuPG-bug-id: 3858 Signed-off-by: Werner Koch --- agent/agent.h | 4 ++-- agent/cache.c | 38 ++++++++++++++++++++++---------------- agent/command-ssh.c | 2 +- agent/command.c | 31 ++++++++++++++++--------------- agent/cvt-openpgp.c | 2 +- agent/findkey.c | 8 ++++---- agent/genkey.c | 6 +++--- agent/protect-tool.c | 3 ++- 8 files changed, 51 insertions(+), 43 deletions(-) diff --git a/agent/agent.h b/agent/agent.h index 743b76595..cf50d9280 100644 --- a/agent/agent.h +++ b/agent/agent.h @@ -452,9 +452,9 @@ void initialize_module_cache (void); void deinitialize_module_cache (void); void agent_cache_housekeeping (void); void agent_flush_cache (void); -int agent_put_cache (const char *key, cache_mode_t cache_mode, +int agent_put_cache (ctrl_t ctrl, const char *key, cache_mode_t cache_mode, const char *data, int ttl); -char *agent_get_cache (const char *key, cache_mode_t cache_mode); +char *agent_get_cache (ctrl_t ctrl, const char *key, cache_mode_t cache_mode); void agent_store_cache_hit (const char *key); diff --git a/agent/cache.c b/agent/cache.c index ed5c97cd2..238b6e214 100644 --- a/agent/cache.c +++ b/agent/cache.c @@ -58,6 +58,7 @@ struct cache_item_s { int ttl; /* max. lifetime given in seconds, -1 one means infinite */ struct secret_data_s *pw; cache_mode_t cache_mode; + int restricted; /* The value of ctrl->restricted is part of the key. */ char key[1]; }; @@ -202,8 +203,8 @@ housekeeping (void) if (r->pw && r->ttl >= 0 && r->accessed + r->ttl < current) { if (DBG_CACHE) - log_debug (" expired '%s' (%ds after last access)\n", - r->key, r->ttl); + log_debug (" expired '%s'.%d (%ds after last access)\n", + r->key, r->restricted, r->ttl); release_data (r->pw); r->pw = NULL; r->accessed = current; @@ -224,8 +225,8 @@ housekeeping (void) if (r->pw && r->created + maxttl < current) { if (DBG_CACHE) - log_debug (" expired '%s' (%lus after creation)\n", - r->key, opt.max_cache_ttl); + log_debug (" expired '%s'.%d (%lus after creation)\n", + r->key, r->restricted, opt.max_cache_ttl); release_data (r->pw); r->pw = NULL; r->accessed = current; @@ -233,15 +234,15 @@ housekeeping (void) } /* Third, make sure that we don't have too many items in the list. - Expire old and unused entries after 30 minutes */ + * Expire old and unused entries after 30 minutes. */ for (rprev=NULL, r=thecache; r; ) { if (!r->pw && r->ttl >= 0 && r->accessed + 60*30 < current) { ITEM r2 = r->next; if (DBG_CACHE) - log_debug (" removed '%s' (mode %d) (slot not used for 30m)\n", - r->key, r->cache_mode); + log_debug (" removed '%s'.%d (mode %d) (slot not used for 30m)\n", + r->key, r->restricted, r->cache_mode); xfree (r); if (!rprev) thecache = r2; @@ -296,7 +297,7 @@ agent_flush_cache (void) if (r->pw) { if (DBG_CACHE) - log_debug (" flushing '%s'\n", r->key); + log_debug (" flushing '%s'.%d\n", r->key, r->restricted); release_data (r->pw); r->pw = NULL; r->accessed = 0; @@ -326,20 +327,21 @@ cache_mode_equal (cache_mode_t a, cache_mode_t b) 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, +agent_put_cache (ctrl_t ctrl, const char *key, cache_mode_t cache_mode, const char *data, int ttl) { gpg_error_t err = 0; ITEM r; int res; + int restricted = ctrl? ctrl->restricted : -1; res = npth_mutex_lock (&cache_lock); if (res) log_fatal ("failed to acquire cache mutex: %s\n", strerror (res)); if (DBG_CACHE) - log_debug ("agent_put_cache '%s' (mode %d) requested ttl=%d\n", - key, cache_mode, ttl); + log_debug ("agent_put_cache '%s'.%d (mode %d) requested ttl=%d\n", + key, restricted, cache_mode, ttl); housekeeping (); if (!ttl) @@ -358,6 +360,7 @@ agent_put_cache (const char *key, cache_mode_t cache_mode, if (((cache_mode != CACHE_MODE_USER && cache_mode != CACHE_MODE_NONCE) || cache_mode_equal (r->cache_mode, cache_mode)) + && r->restricted == restricted && !strcmp (r->key, key)) break; } @@ -386,6 +389,7 @@ agent_put_cache (const char *key, cache_mode_t cache_mode, else { strcpy (r->key, key); + r->restricted = restricted; r->created = r->accessed = gnupg_get_time (); r->ttl = ttl; r->cache_mode = cache_mode; @@ -415,13 +419,14 @@ agent_put_cache (const char *key, cache_mode_t cache_mode, make use of CACHE_MODE except for CACHE_MODE_NONCE and CACHE_MODE_USER. */ char * -agent_get_cache (const char *key, cache_mode_t cache_mode) +agent_get_cache (ctrl_t ctrl, const char *key, cache_mode_t cache_mode) { gpg_error_t err; ITEM r; char *value = NULL; int res; int last_stored = 0; + int restricted = ctrl? ctrl->restricted : -1; if (cache_mode == CACHE_MODE_IGNORE) return NULL; @@ -439,8 +444,8 @@ agent_get_cache (const char *key, cache_mode_t cache_mode) } if (DBG_CACHE) - log_debug ("agent_get_cache '%s' (mode %d)%s ...\n", - key, cache_mode, + log_debug ("agent_get_cache '%s'.%d (mode %d)%s ...\n", + key, ctrl->restricted, cache_mode, last_stored? " (stored cache key)":""); housekeeping (); @@ -450,6 +455,7 @@ agent_get_cache (const char *key, cache_mode_t cache_mode) && ((cache_mode != CACHE_MODE_USER && cache_mode != CACHE_MODE_NONCE) || cache_mode_equal (r->cache_mode, cache_mode)) + && r->restricted == restricted && !strcmp (r->key, key)) { /* Note: To avoid races KEY may not be accessed anymore below. */ @@ -472,8 +478,8 @@ agent_get_cache (const char *key, cache_mode_t cache_mode) { xfree (value); value = NULL; - log_error ("retrieving cache entry '%s' failed: %s\n", - key, gpg_strerror (err)); + log_error ("retrieving cache entry '%s'.%d failed: %s\n", + key, restricted, gpg_strerror (err)); } break; } diff --git a/agent/command-ssh.c b/agent/command-ssh.c index e0b723839..517231a8c 100644 --- a/agent/command-ssh.c +++ b/agent/command-ssh.c @@ -3140,7 +3140,7 @@ ssh_identity_register (ctrl_t ctrl, ssh_key_type_spec_t *spec, goto out; /* Cache this passphrase. */ - err = agent_put_cache (key_grip, CACHE_MODE_SSH, pi->pin, ttl); + err = agent_put_cache (ctrl, key_grip, CACHE_MODE_SSH, pi->pin, ttl); if (err) goto out; diff --git a/agent/command.c b/agent/command.c index 8bb9b6a70..f2d0389c7 100644 --- a/agent/command.c +++ b/agent/command.c @@ -199,14 +199,14 @@ clear_nonce_cache (ctrl_t ctrl) { if (ctrl->server_local->last_cache_nonce) { - agent_put_cache (ctrl->server_local->last_cache_nonce, + agent_put_cache (ctrl, ctrl->server_local->last_cache_nonce, CACHE_MODE_NONCE, NULL, 0); xfree (ctrl->server_local->last_cache_nonce); ctrl->server_local->last_cache_nonce = NULL; } if (ctrl->server_local->last_passwd_nonce) { - agent_put_cache (ctrl->server_local->last_passwd_nonce, + agent_put_cache (ctrl, ctrl->server_local->last_passwd_nonce, CACHE_MODE_NONCE, NULL, 0); xfree (ctrl->server_local->last_passwd_nonce); ctrl->server_local->last_passwd_nonce = NULL; @@ -930,7 +930,7 @@ cmd_genkey (assuan_context_t ctx, char *line) } else if (passwd_nonce) - newpasswd = agent_get_cache (passwd_nonce, CACHE_MODE_NONCE); + newpasswd = agent_get_cache (ctrl, passwd_nonce, CACHE_MODE_NONCE); rc = agent_genkey (ctrl, cache_nonce, (char*)value, valuelen, no_protection, newpasswd, opt_preset, &outbuf); @@ -1179,7 +1179,7 @@ do_one_keyinfo (ctrl_t ctrl, const unsigned char *grip, assuan_context_t ctx, /* Here we have a little race by doing the cache check separately from the retrieval function. Given that the cache flag is only a hint, it should not really matter. */ - pw = agent_get_cache (hexgrip, CACHE_MODE_NORMAL); + pw = agent_get_cache (ctrl, hexgrip, CACHE_MODE_NORMAL); cached = pw ? "1" : "-"; xfree (pw); @@ -1484,7 +1484,7 @@ cmd_get_passphrase (assuan_context_t ctx, char *line) if (!strcmp (desc, "X")) desc = NULL; - pw = cacheid ? agent_get_cache (cacheid, CACHE_MODE_USER) : NULL; + pw = cacheid ? agent_get_cache (ctrl, cacheid, CACHE_MODE_USER) : NULL; if (pw) { rc = send_back_passphrase (ctx, opt_data, pw); @@ -1551,7 +1551,7 @@ cmd_get_passphrase (assuan_context_t ctx, char *line) if (!rc) { if (cacheid) - agent_put_cache (cacheid, CACHE_MODE_USER, response, 0); + agent_put_cache (ctrl, cacheid, CACHE_MODE_USER, response, 0); rc = send_back_passphrase (ctx, opt_data, response); } xfree (response); @@ -1593,7 +1593,8 @@ cmd_clear_passphrase (assuan_context_t ctx, char *line) if (!*cacheid || strlen (cacheid) > 50) return set_error (GPG_ERR_ASS_PARAMETER, "invalid length of cacheID"); - agent_put_cache (cacheid, opt_normal ? CACHE_MODE_NORMAL : CACHE_MODE_USER, + agent_put_cache (ctrl, cacheid, + opt_normal ? CACHE_MODE_NORMAL : CACHE_MODE_USER, NULL, 0); agent_clear_passphrase (ctrl, cacheid, @@ -1770,7 +1771,7 @@ cmd_passwd (assuan_context_t ctx, char *line) passwd_nonce = bin2hex (buf, 12, NULL); } if (passwd_nonce - && !agent_put_cache (passwd_nonce, CACHE_MODE_NONCE, + && !agent_put_cache (ctrl, passwd_nonce, CACHE_MODE_NONCE, passphrase, CACHE_TTL_NONCE)) { assuan_write_status (ctx, "PASSWD_NONCE", passwd_nonce); @@ -1785,7 +1786,7 @@ cmd_passwd (assuan_context_t ctx, char *line) char *newpass = NULL; if (passwd_nonce) - newpass = agent_get_cache (passwd_nonce, CACHE_MODE_NONCE); + newpass = agent_get_cache (ctrl, passwd_nonce, CACHE_MODE_NONCE); err = agent_protect_and_store (ctrl, s_skey, &newpass); if (!err && passphrase) { @@ -1800,7 +1801,7 @@ cmd_passwd (assuan_context_t ctx, char *line) cache_nonce = bin2hex (buf, 12, NULL); } if (cache_nonce - && !agent_put_cache (cache_nonce, CACHE_MODE_NONCE, + && !agent_put_cache (ctrl, cache_nonce, CACHE_MODE_NONCE, passphrase, CACHE_TTL_NONCE)) { assuan_write_status (ctx, "CACHE_NONCE", cache_nonce); @@ -1820,7 +1821,7 @@ cmd_passwd (assuan_context_t ctx, char *line) passwd_nonce = bin2hex (buf, 12, NULL); } if (passwd_nonce - && !agent_put_cache (passwd_nonce, CACHE_MODE_NONCE, + && !agent_put_cache (ctrl, passwd_nonce, CACHE_MODE_NONCE, newpass, CACHE_TTL_NONCE)) { assuan_write_status (ctx, "PASSWD_NONCE", passwd_nonce); @@ -1834,7 +1835,7 @@ cmd_passwd (assuan_context_t ctx, char *line) { char hexgrip[40+1]; bin2hex(grip, 20, hexgrip); - err = agent_put_cache (hexgrip, CACHE_MODE_ANY, newpass, + err = agent_put_cache (ctrl, hexgrip, CACHE_MODE_ANY, newpass, ctrl->cache_ttl_opt_preset); } xfree (newpass); @@ -1939,7 +1940,7 @@ cmd_preset_passphrase (assuan_context_t ctx, char *line) if (!rc) { - rc = agent_put_cache (grip_clear, CACHE_MODE_ANY, passphrase, ttl); + rc = agent_put_cache (ctrl, grip_clear, CACHE_MODE_ANY, passphrase, ttl); if (opt_inquire) xfree (passphrase); } @@ -2174,7 +2175,7 @@ cmd_import_key (assuan_context_t ctx, char *line) cache_nonce = bin2hex (buf, 12, NULL); } if (cache_nonce - && !agent_put_cache (cache_nonce, CACHE_MODE_NONCE, + && !agent_put_cache (ctrl, cache_nonce, CACHE_MODE_NONCE, passphrase, CACHE_TTL_NONCE)) assuan_write_status (ctx, "CACHE_NONCE", cache_nonce); } @@ -2336,7 +2337,7 @@ cmd_export_key (assuan_context_t ctx, char *line) cache_nonce = bin2hex (buf, 12, NULL); } if (cache_nonce - && !agent_put_cache (cache_nonce, CACHE_MODE_NONCE, + && !agent_put_cache (ctrl, cache_nonce, CACHE_MODE_NONCE, passphrase, CACHE_TTL_NONCE)) { assuan_write_status (ctx, "CACHE_NONCE", cache_nonce); diff --git a/agent/cvt-openpgp.c b/agent/cvt-openpgp.c index ee1222113..bf05174fa 100644 --- a/agent/cvt-openpgp.c +++ b/agent/cvt-openpgp.c @@ -951,7 +951,7 @@ convert_from_openpgp_main (ctrl_t ctrl, gcry_sexp_t s_pgp, int dontcare_exist, { char *cache_value; - cache_value = agent_get_cache (cache_nonce, CACHE_MODE_NONCE); + cache_value = agent_get_cache (ctrl, cache_nonce, CACHE_MODE_NONCE); if (cache_value) { if (strlen (cache_value) < pi->max_length) diff --git a/agent/findkey.c b/agent/findkey.c index e3e9a123f..78c3b1a47 100644 --- a/agent/findkey.c +++ b/agent/findkey.c @@ -511,7 +511,7 @@ unprotect (ctrl_t ctrl, const char *cache_nonce, const char *desc_text, { char *pw; - pw = agent_get_cache (cache_nonce, CACHE_MODE_NONCE); + pw = agent_get_cache (ctrl, cache_nonce, CACHE_MODE_NONCE); if (pw) { rc = agent_unprotect (ctrl, *keybuf, pw, NULL, &result, &resultlen); @@ -536,7 +536,7 @@ unprotect (ctrl_t ctrl, const char *cache_nonce, const char *desc_text, char *pw; retry: - pw = agent_get_cache (hexgrip, cache_mode); + pw = agent_get_cache (ctrl, hexgrip, cache_mode); if (pw) { rc = agent_unprotect (ctrl, *keybuf, pw, NULL, &result, &resultlen); @@ -574,7 +574,7 @@ unprotect (ctrl_t ctrl, const char *cache_nonce, const char *desc_text, We can often avoid the passphrase entry in the second step. We do this only in normal mode, so not to interfere with unrelated cache entries. */ - pw = agent_get_cache (NULL, cache_mode); + pw = agent_get_cache (ctrl, NULL, cache_mode); if (pw) { rc = agent_unprotect (ctrl, *keybuf, pw, NULL, @@ -670,7 +670,7 @@ unprotect (ctrl_t ctrl, const char *cache_nonce, const char *desc_text, else { /* Passphrase is fine. */ - agent_put_cache (hexgrip, cache_mode, pi->pin, + agent_put_cache (ctrl, hexgrip, cache_mode, pi->pin, lookup_ttl? lookup_ttl (hexgrip) : 0); agent_store_cache_hit (hexgrip); if (r_passphrase && *pi->pin) diff --git a/agent/genkey.c b/agent/genkey.c index a3e37ee3a..d5c80d0aa 100644 --- a/agent/genkey.c +++ b/agent/genkey.c @@ -468,7 +468,7 @@ agent_genkey (ctrl_t ctrl, const char *cache_nonce, passphrase = NULL; else { - passphrase_buffer = agent_get_cache (cache_nonce, CACHE_MODE_NONCE); + passphrase_buffer = agent_get_cache (ctrl, cache_nonce, CACHE_MODE_NONCE); passphrase = passphrase_buffer; } @@ -528,7 +528,7 @@ agent_genkey (ctrl_t ctrl, const char *cache_nonce, } if (cache_nonce && !no_protection - && !agent_put_cache (cache_nonce, CACHE_MODE_NONCE, + && !agent_put_cache (ctrl, cache_nonce, CACHE_MODE_NONCE, passphrase, ctrl->cache_ttl_opt_preset)) agent_write_status (ctrl, "CACHE_NONCE", cache_nonce, NULL); if (preset && !no_protection) @@ -538,7 +538,7 @@ agent_genkey (ctrl_t ctrl, const char *cache_nonce, if (gcry_pk_get_keygrip (s_private, grip)) { bin2hex(grip, 20, hexgrip); - rc = agent_put_cache (hexgrip, CACHE_MODE_ANY, passphrase, + rc = agent_put_cache (ctrl, hexgrip, CACHE_MODE_ANY, passphrase, ctrl->cache_ttl_opt_preset); } } diff --git a/agent/protect-tool.c b/agent/protect-tool.c index a193e4969..ec7b47695 100644 --- a/agent/protect-tool.c +++ b/agent/protect-tool.c @@ -749,8 +749,9 @@ agent_key_available (const unsigned char *grip) } char * -agent_get_cache (const char *key, cache_mode_t cache_mode) +agent_get_cache (ctrl_t ctrl, const char *key, cache_mode_t cache_mode) { + (void)ctrl; (void)key; (void)cache_mode; return NULL;