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 <wk@gnupg.org>
This commit is contained in:
Werner Koch 2018-03-27 08:40:58 +02:00
parent eb68c2d3d1
commit 02dce8c0cc
No known key found for this signature in database
GPG Key ID: E3FDFF218E45B72B
8 changed files with 51 additions and 43 deletions

View File

@ -452,9 +452,9 @@ void initialize_module_cache (void);
void deinitialize_module_cache (void); void deinitialize_module_cache (void);
void agent_cache_housekeeping (void); void agent_cache_housekeeping (void);
void agent_flush_cache (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); 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); void agent_store_cache_hit (const char *key);

View File

@ -58,6 +58,7 @@ struct cache_item_s {
int ttl; /* max. lifetime given in seconds, -1 one means infinite */ int ttl; /* max. lifetime given in seconds, -1 one means infinite */
struct secret_data_s *pw; struct secret_data_s *pw;
cache_mode_t cache_mode; cache_mode_t cache_mode;
int restricted; /* The value of ctrl->restricted is part of the key. */
char key[1]; char key[1];
}; };
@ -202,8 +203,8 @@ housekeeping (void)
if (r->pw && r->ttl >= 0 && r->accessed + r->ttl < current) if (r->pw && r->ttl >= 0 && r->accessed + r->ttl < current)
{ {
if (DBG_CACHE) if (DBG_CACHE)
log_debug (" expired '%s' (%ds after last access)\n", log_debug (" expired '%s'.%d (%ds after last access)\n",
r->key, r->ttl); r->key, r->restricted, r->ttl);
release_data (r->pw); release_data (r->pw);
r->pw = NULL; r->pw = NULL;
r->accessed = current; r->accessed = current;
@ -224,8 +225,8 @@ housekeeping (void)
if (r->pw && r->created + maxttl < current) if (r->pw && r->created + maxttl < current)
{ {
if (DBG_CACHE) if (DBG_CACHE)
log_debug (" expired '%s' (%lus after creation)\n", log_debug (" expired '%s'.%d (%lus after creation)\n",
r->key, opt.max_cache_ttl); r->key, r->restricted, opt.max_cache_ttl);
release_data (r->pw); release_data (r->pw);
r->pw = NULL; r->pw = NULL;
r->accessed = current; r->accessed = current;
@ -233,15 +234,15 @@ housekeeping (void)
} }
/* Third, make sure that we don't have too many items in the list. /* 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; ) for (rprev=NULL, r=thecache; r; )
{ {
if (!r->pw && r->ttl >= 0 && r->accessed + 60*30 < current) if (!r->pw && r->ttl >= 0 && r->accessed + 60*30 < current)
{ {
ITEM r2 = r->next; ITEM r2 = r->next;
if (DBG_CACHE) if (DBG_CACHE)
log_debug (" removed '%s' (mode %d) (slot not used for 30m)\n", log_debug (" removed '%s'.%d (mode %d) (slot not used for 30m)\n",
r->key, r->cache_mode); r->key, r->restricted, r->cache_mode);
xfree (r); xfree (r);
if (!rprev) if (!rprev)
thecache = r2; thecache = r2;
@ -296,7 +297,7 @@ agent_flush_cache (void)
if (r->pw) if (r->pw)
{ {
if (DBG_CACHE) if (DBG_CACHE)
log_debug (" flushing '%s'\n", r->key); log_debug (" flushing '%s'.%d\n", r->key, r->restricted);
release_data (r->pw); release_data (r->pw);
r->pw = NULL; r->pw = NULL;
r->accessed = 0; 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 set infinite timeout. CACHE_MODE is stored with the cache entry
and used to select different timeouts. */ and used to select different timeouts. */
int 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) const char *data, int ttl)
{ {
gpg_error_t err = 0; gpg_error_t err = 0;
ITEM r; ITEM r;
int res; int res;
int restricted = ctrl? ctrl->restricted : -1;
res = npth_mutex_lock (&cache_lock); res = npth_mutex_lock (&cache_lock);
if (res) if (res)
log_fatal ("failed to acquire cache mutex: %s\n", strerror (res)); log_fatal ("failed to acquire cache mutex: %s\n", strerror (res));
if (DBG_CACHE) if (DBG_CACHE)
log_debug ("agent_put_cache '%s' (mode %d) requested ttl=%d\n", log_debug ("agent_put_cache '%s'.%d (mode %d) requested ttl=%d\n",
key, cache_mode, ttl); key, restricted, cache_mode, ttl);
housekeeping (); housekeeping ();
if (!ttl) if (!ttl)
@ -358,6 +360,7 @@ agent_put_cache (const char *key, cache_mode_t cache_mode,
if (((cache_mode != CACHE_MODE_USER if (((cache_mode != CACHE_MODE_USER
&& cache_mode != CACHE_MODE_NONCE) && cache_mode != CACHE_MODE_NONCE)
|| cache_mode_equal (r->cache_mode, cache_mode)) || cache_mode_equal (r->cache_mode, cache_mode))
&& r->restricted == restricted
&& !strcmp (r->key, key)) && !strcmp (r->key, key))
break; break;
} }
@ -386,6 +389,7 @@ agent_put_cache (const char *key, cache_mode_t cache_mode,
else else
{ {
strcpy (r->key, key); strcpy (r->key, key);
r->restricted = restricted;
r->created = r->accessed = gnupg_get_time (); r->created = r->accessed = gnupg_get_time ();
r->ttl = ttl; r->ttl = ttl;
r->cache_mode = cache_mode; 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 make use of CACHE_MODE except for CACHE_MODE_NONCE and
CACHE_MODE_USER. */ CACHE_MODE_USER. */
char * 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; gpg_error_t err;
ITEM r; ITEM r;
char *value = NULL; char *value = NULL;
int res; int res;
int last_stored = 0; int last_stored = 0;
int restricted = ctrl? ctrl->restricted : -1;
if (cache_mode == CACHE_MODE_IGNORE) if (cache_mode == CACHE_MODE_IGNORE)
return NULL; return NULL;
@ -439,8 +444,8 @@ agent_get_cache (const char *key, cache_mode_t cache_mode)
} }
if (DBG_CACHE) if (DBG_CACHE)
log_debug ("agent_get_cache '%s' (mode %d)%s ...\n", log_debug ("agent_get_cache '%s'.%d (mode %d)%s ...\n",
key, cache_mode, key, ctrl->restricted, cache_mode,
last_stored? " (stored cache key)":""); last_stored? " (stored cache key)":"");
housekeeping (); 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_USER
&& cache_mode != CACHE_MODE_NONCE) && cache_mode != CACHE_MODE_NONCE)
|| cache_mode_equal (r->cache_mode, cache_mode)) || cache_mode_equal (r->cache_mode, cache_mode))
&& r->restricted == restricted
&& !strcmp (r->key, key)) && !strcmp (r->key, key))
{ {
/* Note: To avoid races KEY may not be accessed anymore below. */ /* 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); xfree (value);
value = NULL; value = NULL;
log_error ("retrieving cache entry '%s' failed: %s\n", log_error ("retrieving cache entry '%s'.%d failed: %s\n",
key, gpg_strerror (err)); key, restricted, gpg_strerror (err));
} }
break; break;
} }

View File

@ -3140,7 +3140,7 @@ ssh_identity_register (ctrl_t ctrl, ssh_key_type_spec_t *spec,
goto out; goto out;
/* Cache this passphrase. */ /* 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) if (err)
goto out; goto out;

View File

@ -199,14 +199,14 @@ clear_nonce_cache (ctrl_t ctrl)
{ {
if (ctrl->server_local->last_cache_nonce) 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); CACHE_MODE_NONCE, NULL, 0);
xfree (ctrl->server_local->last_cache_nonce); xfree (ctrl->server_local->last_cache_nonce);
ctrl->server_local->last_cache_nonce = NULL; ctrl->server_local->last_cache_nonce = NULL;
} }
if (ctrl->server_local->last_passwd_nonce) 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); CACHE_MODE_NONCE, NULL, 0);
xfree (ctrl->server_local->last_passwd_nonce); xfree (ctrl->server_local->last_passwd_nonce);
ctrl->server_local->last_passwd_nonce = NULL; ctrl->server_local->last_passwd_nonce = NULL;
@ -930,7 +930,7 @@ cmd_genkey (assuan_context_t ctx, char *line)
} }
else if (passwd_nonce) 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, rc = agent_genkey (ctrl, cache_nonce, (char*)value, valuelen, no_protection,
newpasswd, opt_preset, &outbuf); 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 /* Here we have a little race by doing the cache check separately
from the retrieval function. Given that the cache flag is only a from the retrieval function. Given that the cache flag is only a
hint, it should not really matter. */ 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" : "-"; cached = pw ? "1" : "-";
xfree (pw); xfree (pw);
@ -1484,7 +1484,7 @@ cmd_get_passphrase (assuan_context_t ctx, char *line)
if (!strcmp (desc, "X")) if (!strcmp (desc, "X"))
desc = NULL; 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) if (pw)
{ {
rc = send_back_passphrase (ctx, opt_data, 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 (!rc)
{ {
if (cacheid) 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); rc = send_back_passphrase (ctx, opt_data, response);
} }
xfree (response); xfree (response);
@ -1593,7 +1593,8 @@ cmd_clear_passphrase (assuan_context_t ctx, char *line)
if (!*cacheid || strlen (cacheid) > 50) if (!*cacheid || strlen (cacheid) > 50)
return set_error (GPG_ERR_ASS_PARAMETER, "invalid length of cacheID"); 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); NULL, 0);
agent_clear_passphrase (ctrl, cacheid, agent_clear_passphrase (ctrl, cacheid,
@ -1770,7 +1771,7 @@ cmd_passwd (assuan_context_t ctx, char *line)
passwd_nonce = bin2hex (buf, 12, NULL); passwd_nonce = bin2hex (buf, 12, NULL);
} }
if (passwd_nonce if (passwd_nonce
&& !agent_put_cache (passwd_nonce, CACHE_MODE_NONCE, && !agent_put_cache (ctrl, passwd_nonce, CACHE_MODE_NONCE,
passphrase, CACHE_TTL_NONCE)) passphrase, CACHE_TTL_NONCE))
{ {
assuan_write_status (ctx, "PASSWD_NONCE", passwd_nonce); assuan_write_status (ctx, "PASSWD_NONCE", passwd_nonce);
@ -1785,7 +1786,7 @@ cmd_passwd (assuan_context_t ctx, char *line)
char *newpass = NULL; char *newpass = NULL;
if (passwd_nonce) 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); err = agent_protect_and_store (ctrl, s_skey, &newpass);
if (!err && passphrase) if (!err && passphrase)
{ {
@ -1800,7 +1801,7 @@ cmd_passwd (assuan_context_t ctx, char *line)
cache_nonce = bin2hex (buf, 12, NULL); cache_nonce = bin2hex (buf, 12, NULL);
} }
if (cache_nonce if (cache_nonce
&& !agent_put_cache (cache_nonce, CACHE_MODE_NONCE, && !agent_put_cache (ctrl, cache_nonce, CACHE_MODE_NONCE,
passphrase, CACHE_TTL_NONCE)) passphrase, CACHE_TTL_NONCE))
{ {
assuan_write_status (ctx, "CACHE_NONCE", cache_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); passwd_nonce = bin2hex (buf, 12, NULL);
} }
if (passwd_nonce if (passwd_nonce
&& !agent_put_cache (passwd_nonce, CACHE_MODE_NONCE, && !agent_put_cache (ctrl, passwd_nonce, CACHE_MODE_NONCE,
newpass, CACHE_TTL_NONCE)) newpass, CACHE_TTL_NONCE))
{ {
assuan_write_status (ctx, "PASSWD_NONCE", passwd_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]; char hexgrip[40+1];
bin2hex(grip, 20, hexgrip); 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); ctrl->cache_ttl_opt_preset);
} }
xfree (newpass); xfree (newpass);
@ -1939,7 +1940,7 @@ cmd_preset_passphrase (assuan_context_t ctx, char *line)
if (!rc) 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) if (opt_inquire)
xfree (passphrase); xfree (passphrase);
} }
@ -2174,7 +2175,7 @@ cmd_import_key (assuan_context_t ctx, char *line)
cache_nonce = bin2hex (buf, 12, NULL); cache_nonce = bin2hex (buf, 12, NULL);
} }
if (cache_nonce if (cache_nonce
&& !agent_put_cache (cache_nonce, CACHE_MODE_NONCE, && !agent_put_cache (ctrl, cache_nonce, CACHE_MODE_NONCE,
passphrase, CACHE_TTL_NONCE)) passphrase, CACHE_TTL_NONCE))
assuan_write_status (ctx, "CACHE_NONCE", cache_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); cache_nonce = bin2hex (buf, 12, NULL);
} }
if (cache_nonce if (cache_nonce
&& !agent_put_cache (cache_nonce, CACHE_MODE_NONCE, && !agent_put_cache (ctrl, cache_nonce, CACHE_MODE_NONCE,
passphrase, CACHE_TTL_NONCE)) passphrase, CACHE_TTL_NONCE))
{ {
assuan_write_status (ctx, "CACHE_NONCE", cache_nonce); assuan_write_status (ctx, "CACHE_NONCE", cache_nonce);

View File

@ -951,7 +951,7 @@ convert_from_openpgp_main (ctrl_t ctrl, gcry_sexp_t s_pgp, int dontcare_exist,
{ {
char *cache_value; 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 (cache_value)
{ {
if (strlen (cache_value) < pi->max_length) if (strlen (cache_value) < pi->max_length)

View File

@ -511,7 +511,7 @@ unprotect (ctrl_t ctrl, const char *cache_nonce, const char *desc_text,
{ {
char *pw; char *pw;
pw = agent_get_cache (cache_nonce, CACHE_MODE_NONCE); pw = agent_get_cache (ctrl, cache_nonce, CACHE_MODE_NONCE);
if (pw) if (pw)
{ {
rc = agent_unprotect (ctrl, *keybuf, pw, NULL, &result, &resultlen); 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; char *pw;
retry: retry:
pw = agent_get_cache (hexgrip, cache_mode); pw = agent_get_cache (ctrl, hexgrip, cache_mode);
if (pw) if (pw)
{ {
rc = agent_unprotect (ctrl, *keybuf, pw, NULL, &result, &resultlen); 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 We can often avoid the passphrase entry in the second
step. We do this only in normal mode, so not to step. We do this only in normal mode, so not to
interfere with unrelated cache entries. */ interfere with unrelated cache entries. */
pw = agent_get_cache (NULL, cache_mode); pw = agent_get_cache (ctrl, NULL, cache_mode);
if (pw) if (pw)
{ {
rc = agent_unprotect (ctrl, *keybuf, pw, NULL, rc = agent_unprotect (ctrl, *keybuf, pw, NULL,
@ -670,7 +670,7 @@ unprotect (ctrl_t ctrl, const char *cache_nonce, const char *desc_text,
else else
{ {
/* Passphrase is fine. */ /* 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); lookup_ttl? lookup_ttl (hexgrip) : 0);
agent_store_cache_hit (hexgrip); agent_store_cache_hit (hexgrip);
if (r_passphrase && *pi->pin) if (r_passphrase && *pi->pin)

View File

@ -468,7 +468,7 @@ agent_genkey (ctrl_t ctrl, const char *cache_nonce,
passphrase = NULL; passphrase = NULL;
else 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; passphrase = passphrase_buffer;
} }
@ -528,7 +528,7 @@ agent_genkey (ctrl_t ctrl, const char *cache_nonce,
} }
if (cache_nonce if (cache_nonce
&& !no_protection && !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)) passphrase, ctrl->cache_ttl_opt_preset))
agent_write_status (ctrl, "CACHE_NONCE", cache_nonce, NULL); agent_write_status (ctrl, "CACHE_NONCE", cache_nonce, NULL);
if (preset && !no_protection) 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)) if (gcry_pk_get_keygrip (s_private, grip))
{ {
bin2hex(grip, 20, hexgrip); 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); ctrl->cache_ttl_opt_preset);
} }
} }

View File

@ -749,8 +749,9 @@ agent_key_available (const unsigned char *grip)
} }
char * 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)key;
(void)cache_mode; (void)cache_mode;
return NULL; return NULL;