From 241971fac0fc52efc87ed5753a01d18b0672d900 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Wed, 2 Oct 2024 16:44:04 +0200 Subject: [PATCH] gpgsm: Implement a cache for the KEYINFO queries. * sm/gpgsm.h (struct keyinfo_cache_item_s): New. (struct server_control_s): Add keyinfo_cache and keyinfo_cache_valid. * sm/call-agent.c (keyinfo_cache_disabled): New flag. (release_a_keyinfo_cache): New. (gpgsm_flush_keyinfo_cache): New. (struct keyinfo_status_parm_s): New. (keyinfo_status_cb): Implement a fill mode. (gpgsm_agent_keyinfo): Implement a cache. * sm/server.c (reset_notify): Flush the cache. * sm/gpgsm.c (gpgsm_deinit_default_ctrl): Ditto. -- In almost all cases we have just a few private keys in the agent and thus it is better to fetch them early. This does not work in a restricted connection but we take care and disable the cache in this case. This cache gives a a minor speed up. GnuPG-bug-id: 7308 --- sm/call-agent.c | 158 ++++++++++++++++++++++++++++++++++++++++++------ sm/gpgsm.c | 1 + sm/gpgsm.h | 14 +++++ sm/server.c | 1 + 4 files changed, 154 insertions(+), 20 deletions(-) diff --git a/sm/call-agent.c b/sm/call-agent.c index 60a076b27..a0211fc41 100644 --- a/sm/call-agent.c +++ b/sm/call-agent.c @@ -104,6 +104,9 @@ static istrusted_cache_t istrusted_cache; static int istrusted_cache_valid; static int istrusted_cache_disabled; +/* Flag indicating that we can't use the keyinfo cache at all. The + * actual cache is stored in CTRL. */ +static int keyinfo_cache_disabled; @@ -127,6 +130,33 @@ flush_istrusted_cache (void) } +/* Release all items in *CACHEP and set CACHEP to NULL */ +static void +release_a_keyinfo_cache (keyinfo_cache_item_t *cachep) +{ + keyinfo_cache_item_t mycache; + + /* First unlink the cache to be npth safe. */ + mycache = *cachep; + *cachep = NULL; + + while (mycache) + { + keyinfo_cache_item_t next = mycache->next; + xfree (mycache); + mycache = next; + } +} + + +/* Flush the keyinfo cache for the session CTRL. */ +void +gpgsm_flush_keyinfo_cache (ctrl_t ctrl) +{ + ctrl->keyinfo_cache_valid = 0; + release_a_keyinfo_cache (&ctrl->keyinfo_cache); +} + /* Print a warning if the server's version number is less than our version number. Returns an error code on a connection problem. */ static gpg_error_t @@ -1007,9 +1037,10 @@ gpgsm_agent_istrusted (ctrl_t ctrl, ksba_cert_t cert, const char *hexfpr, if (rc) { if (gpg_err_code (rc) != GPG_ERR_FORBIDDEN) - log_error ("filling istrusted cache failed: %s\n", + log_info ("filling istrusted cache failed: %s\n", gpg_strerror (rc)); istrusted_cache_disabled = 1; + flush_istrusted_cache (); rc = 0; /* Fallback to single requests. */ } else @@ -1347,42 +1378,79 @@ gpgsm_agent_send_nop (ctrl_t ctrl) +struct keyinfo_status_parm_s +{ + char *serialno; + int fill_mode; /* True if we want to fill the cache. */ + keyinfo_cache_item_t cache; +}; + static gpg_error_t keyinfo_status_cb (void *opaque, const char *line) { - char **serialno = opaque; - const char *s, *s2; + struct keyinfo_status_parm_s *parm = opaque; + const char *s0, *s, *s2; - if ((s = has_leading_keyword (line, "KEYINFO")) && !*serialno) + if ((s0 = has_leading_keyword (line, "KEYINFO")) + && (!parm->serialno || parm->fill_mode)) { - s = strchr (s, ' '); + s = strchr (s0, ' '); + xfree (parm->serialno); + parm->serialno = NULL; if (s && s[1] == 'T' && s[2] == ' ' && s[3]) { s += 3; s2 = strchr (s, ' '); if ( s2 > s ) { - *serialno = xtrymalloc ((s2 - s)+1); - if (*serialno) + parm->serialno = xtrymalloc ((s2 - s)+1); + if (parm->serialno) { - memcpy (*serialno, s, s2 - s); - (*serialno)[s2 - s] = 0; + memcpy (parm->serialno, s, s2 - s); + parm->serialno[s2 - s] = 0; } } } + + if (parm->fill_mode && *s0) + { + keyinfo_cache_item_t ci; + size_t n; + + n = s? (s - s0) : strlen (s0); + ci = xtrymalloc (sizeof *ci + n); + if (!ci) + return gpg_error_from_syserror (); + memcpy (ci->hexgrip, s0, n); + ci->hexgrip[n] = 0; + ci->serialno = parm->serialno; + parm->serialno = NULL; + ci->next = parm->cache; + parm->cache = ci; + } + } return 0; } + /* Return the serial number for a secret key. If the returned serial - number is NULL, the key is not stored on a smartcard. Caller needs - to free R_SERIALNO. */ + * number is NULL, the key is not stored on a smartcard. Caller needs + * to free R_SERIALNO. + * + * Take care: The cache is currently only used in the key listing and + * it should not interfere with import or creation of new keys because + * we assume that is done by another process. However we assume that + * in server mode the key listing is not directly followed by an import + * and another key listing. + */ gpg_error_t gpgsm_agent_keyinfo (ctrl_t ctrl, const char *hexkeygrip, char **r_serialno) { gpg_error_t err; char line[ASSUAN_LINELENGTH]; - char *serialno = NULL; + keyinfo_cache_item_t ci; + struct keyinfo_status_parm_s parm = { NULL }; *r_serialno = NULL; @@ -1393,20 +1461,70 @@ gpgsm_agent_keyinfo (ctrl_t ctrl, const char *hexkeygrip, char **r_serialno) if (!hexkeygrip || strlen (hexkeygrip) != 40) return gpg_error (GPG_ERR_INV_VALUE); - snprintf (line, DIM(line), "KEYINFO %s", hexkeygrip); + /* First try to fill the cache. */ + if (!keyinfo_cache_disabled && !ctrl->keyinfo_cache_valid) + { + parm.fill_mode = 1; + err = assuan_transact (agent_ctx, "KEYINFO --list", + NULL, NULL, NULL, NULL, + keyinfo_status_cb, &parm); + if (err) + { + if (gpg_err_code (err) != GPG_ERR_FORBIDDEN) + log_error ("filling keyinfo cache failed: %s\n", + gpg_strerror (err)); + keyinfo_cache_disabled = 1; + release_a_keyinfo_cache (&parm.cache); + err = 0; /* Fallback to single requests. */ + } + else + { + ctrl->keyinfo_cache_valid = 1; + ctrl->keyinfo_cache = parm.cache; + parm.cache = NULL; + } + } - err = assuan_transact (agent_ctx, line, NULL, NULL, NULL, NULL, - keyinfo_status_cb, &serialno); - if (!err && serialno) + /* Then consult the cache or send a query */ + if (ctrl->keyinfo_cache_valid) + { + for (ci = ctrl->keyinfo_cache; ci; ci = ci->next) + if (!strcmp (hexkeygrip, ci->hexgrip)) + break; + if (ci) + { + xfree (parm.serialno); + parm.serialno = NULL; + err = 0; + if (ci->serialno) + { + parm.serialno = xtrystrdup (ci->serialno); + if (!parm.serialno) + err = gpg_error_from_syserror (); + } + } + else + err = gpg_error (GPG_ERR_NOT_FOUND); + } + else + { + snprintf (line, DIM(line), "KEYINFO %s", hexkeygrip); + parm.fill_mode = 0; + err = assuan_transact (agent_ctx, line, NULL, NULL, NULL, NULL, + keyinfo_status_cb, &parm); + } + + if (!err && parm.serialno) { /* Sanity check for bad characters. */ - if (strpbrk (serialno, ":\n\r")) - err = GPG_ERR_INV_VALUE; + if (strpbrk (parm.serialno, ":\n\r")) + err = gpg_error (GPG_ERR_INV_VALUE); } + if (err) - xfree (serialno); + xfree (parm.serialno); else - *r_serialno = serialno; + *r_serialno = parm.serialno; return err; } diff --git a/sm/gpgsm.c b/sm/gpgsm.c index ac80fadde..c108da58c 100644 --- a/sm/gpgsm.c +++ b/sm/gpgsm.c @@ -2389,6 +2389,7 @@ gpgsm_deinit_default_ctrl (ctrl_t ctrl) unsigned int n; gpgsm_keydb_deinit_session_data (ctrl); + gpgsm_flush_keyinfo_cache (ctrl); xfree (ctrl->revocation_reason); ctrl->revocation_reason = NULL; n = 0; diff --git a/sm/gpgsm.h b/sm/gpgsm.h index 36d2fdc9a..142e7bb94 100644 --- a/sm/gpgsm.h +++ b/sm/gpgsm.h @@ -241,6 +241,15 @@ struct cert_cache_item_s }; typedef struct cert_cache_item_s *cert_cache_item_t; +/* On object used to keep a KEYINFO data from the agent. */ +struct keyinfo_cache_item_s +{ + struct keyinfo_cache_item_s *next; + char *serialno; /* Malloced serialnumber of a card. */ + char hexgrip[1]; /* The keygrip in hexformat. */ +}; +typedef struct keyinfo_cache_item_s *keyinfo_cache_item_t; + /* Session control object. This object is passed down to most functions. Note that the default values for it are set by @@ -299,6 +308,10 @@ struct server_control_s /* The cache used to find the parent cert. */ cert_cache_item_t parent_cert_cache; + + /* Cache of recently gathered KEYINFO data. */ + keyinfo_cache_item_t keyinfo_cache; + int keyinfo_cache_valid; }; @@ -497,6 +510,7 @@ gpg_error_t gpgsm_qualified_consent (ctrl_t ctrl, ksba_cert_t cert); gpg_error_t gpgsm_not_qualified_warning (ctrl_t ctrl, ksba_cert_t cert); /*-- call-agent.c --*/ +void gpgsm_flush_keyinfo_cache (ctrl_t ctrl); int gpgsm_agent_pksign (ctrl_t ctrl, const char *keygrip, const char *desc, unsigned char *digest, size_t digestlen, diff --git a/sm/server.c b/sm/server.c index f00b70d38..cb3fae24d 100644 --- a/sm/server.c +++ b/sm/server.c @@ -327,6 +327,7 @@ reset_notify (assuan_context_t ctx, char *line) (void) line; + gpgsm_flush_keyinfo_cache (ctrl); gpgsm_release_certlist (ctrl->server_local->recplist); gpgsm_release_certlist (ctrl->server_local->signerlist); ctrl->server_local->recplist = NULL;