gpg: Improve speed of secret key listing.

* agent/command.c (cmd_keyinfo): Factor some code out to ...
(get_keyinfo_on_cards): ... new.
(cmd_havekey): Add --list mode.
* g10/gpg.h (struct server_control_s): Add new caching vars.
* g10/gpg.c (gpg_deinit_default_ctrl): Release cache.
* g10/call-agent.c (agent_probe_any_secret_key): Init and try to use
the keygrip cache.
(agent_genkey): Clear the cache.
(agent_import_key): Ditto.

* g10/keylist.c (list_all, list_one): Pass ctrl to
agent_probe_any_secret_key.
* g10/getkey.c (lookup): Ditto.
--

With this change we first ask the agent for a list of all secret
keygrips and use that list instead of asking the agent for each public
key.  Speeds up my "gpg -K" with a lot of secret and public keys by
more than 25%.

Signed-off-by: Werner Koch <wk@gnupg.org>

agent/command.c

@@ -1,7 +1,7 @@
 /* command.c - gpg-agent command handler
  * Copyright (C) 2001-2011 Free Software Foundation, Inc.
  * Copyright (C) 2001-2013 Werner Koch
- * Copyright (C) 2015 g10 Code GmbH.
+ * Copyright (C) 2015-2021 g10 Code GmbH.
  *
  * This file is part of GnuPG.
  *
@@ -442,6 +442,34 @@ leave_cmd (assuan_context_t ctx, gpg_error_t err)
 }
 
 
+/* Take the keyinfo for cards from our local cache.  Actually this
+ * cache could be a global one but then we would need to employ
+ * reference counting.  */
+struct card_key_info_s *
+get_keyinfo_on_cards (ctrl_t ctrl)
+{
+  struct card_key_info_s *keyinfo_on_cards;
+
+  if (ctrl->server_local->last_card_keyinfo.ki
+      && ctrl->server_local->last_card_keyinfo.eventno == eventcounter.card
+      && (ctrl->server_local->last_card_keyinfo.maybe_key_change
+          == eventcounter.maybe_key_change))
+    {
+      keyinfo_on_cards = ctrl->server_local->last_card_keyinfo.ki;
+    }
+  else if (!agent_card_keyinfo (ctrl, NULL, 0, &keyinfo_on_cards))
+    {
+      agent_card_free_keyinfo (ctrl->server_local->last_card_keyinfo.ki);
+      ctrl->server_local->last_card_keyinfo.ki = keyinfo_on_cards;
+      ctrl->server_local->last_card_keyinfo.eventno = eventcounter.card;
+      ctrl->server_local->last_card_keyinfo.maybe_key_change
+        = eventcounter.maybe_key_change;
+    }
+
+  return keyinfo_on_cards;
+}
+
+
 
 static const char hlp_geteventcounter[] =
   "GETEVENTCOUNTER\n"
@@ -602,22 +630,46 @@ cmd_marktrusted (assuan_context_t ctx, char *line)
 
 static const char hlp_havekey[] =
   "HAVEKEY <hexstrings_with_keygrips>\n"
+  "HAVEKEY --list[=<limit>]\n"
   "\n"
   "Return success if at least one of the secret keys with the given\n"
-  "keygrips is available.";
+  "keygrips is available.  With --list return all availabale keygrips\n"
+  "as binary data; with <limit> bail out at this number of keygrips";
 static gpg_error_t
 cmd_havekey (assuan_context_t ctx, char *line)
 {
+  ctrl_t ctrl;
   gpg_error_t err;
-  unsigned char buf[20];
+  unsigned char grip[20];
+  char *p;
+  int list_mode;  /* Less than 0 for no limit.  */
+  int counter;
+  char *dirname;
+  gnupg_dir_t dir;
+  gnupg_dirent_t dir_entry;
+  char hexgrip[41];
+  struct card_key_info_s *keyinfo_on_cards, *l;
 
+  if (has_option_name (line, "--list"))
+    {
+      if ((p = option_value (line, "--list")))
+	list_mode = atoi (p);
+      else
+	list_mode = -1;
+    }
+  else
+    list_mode = 0;
+
+
+  if (!list_mode)
+    {
       do
         {
-      err = parse_keygrip (ctx, line, buf);
+          err = parse_keygrip (ctx, line, grip);
           if (err)
             return err;
 
-      if (!agent_key_available (buf))
+          if (!agent_key_available (grip))
             return 0; /* Found.  */
 
           while (*line && *line != ' ' && *line != '\t')
@@ -628,10 +680,84 @@ cmd_havekey (assuan_context_t ctx, char *line)
       while (*line);
 
       /* No leave_cmd() here because errors are expected and would clutter
-     the log.  */
+       * the log.  */
       return gpg_error (GPG_ERR_NO_SECKEY);
     }
 
+  /* List mode.  */
+  dir = NULL;
+  dirname = NULL;
+  ctrl = assuan_get_pointer (ctx);
+
+  if (ctrl->restricted)
+    {
+      err = gpg_error (GPG_ERR_FORBIDDEN);
+      goto leave;
+    }
+
+  dirname = make_filename_try (gnupg_homedir (),
+                               GNUPG_PRIVATE_KEYS_DIR, NULL);
+  if (!dirname)
+    {
+      err = gpg_error_from_syserror ();
+      goto leave;
+    }
+  dir = gnupg_opendir (dirname);
+  if (!dir)
+    {
+      err = gpg_error_from_syserror ();
+      goto leave;
+    }
+
+  counter = 0;
+  while ((dir_entry = gnupg_readdir (dir)))
+    {
+      if (strlen (dir_entry->d_name) != 44
+          || strcmp (dir_entry->d_name + 40, ".key"))
+        continue;
+      strncpy (hexgrip, dir_entry->d_name, 40);
+      hexgrip[40] = 0;
+
+      if ( hex2bin (hexgrip, grip, 20) < 0 )
+        continue; /* Bad hex string.  */
+
+      if (list_mode > 0 && ++counter > list_mode)
+        {
+          err = gpg_error (GPG_ERR_TRUNCATED);
+          goto leave;
+        }
+
+      err = assuan_send_data (ctx, grip, 20);
+      if (err)
+        goto leave;
+    }
+
+  /* And now the keys from the current cards.  If they already got a
+   * stub, they are listed twice but we don't care.  */
+  keyinfo_on_cards = get_keyinfo_on_cards (ctrl);
+  for (l = keyinfo_on_cards; l; l = l->next)
+    {
+      if ( hex2bin (l->keygrip, grip, 20) < 0 )
+        continue; /* Bad hex string.  */
+
+      if (list_mode > 0 && ++counter > list_mode)
+        {
+          err = gpg_error (GPG_ERR_TRUNCATED);
+          goto leave;
+        }
+
+      err = assuan_send_data (ctx, grip, 20);
+      if (err)
+        goto leave;
+    }
+  err = 0;
+
+ leave:
+  gnupg_closedir (dir);
+  xfree (dirname);
+  return leave_cmd (ctx, err);
+}
+
 
 static const char hlp_sigkey[] =
   "SIGKEY <hexstring_with_keygrip>\n"
@@ -1423,24 +1549,7 @@ cmd_keyinfo (assuan_context_t ctx, char *line)
   if (opt_with_ssh || list_mode == 2)
     cf = ssh_open_control_file ();
 
-  /* Take the keyinfo for cards from our local cache.  Actually this
+  keyinfo_on_cards = get_keyinfo_on_cards (ctrl);
-   * cache could be a global one but then we would need to employ
-   * reference counting. */
-  if (ctrl->server_local->last_card_keyinfo.ki
-      && ctrl->server_local->last_card_keyinfo.eventno == eventcounter.card
-      && (ctrl->server_local->last_card_keyinfo.maybe_key_change
-          == eventcounter.maybe_key_change))
-    {
-      keyinfo_on_cards = ctrl->server_local->last_card_keyinfo.ki;
-    }
-  else if (!agent_card_keyinfo (ctrl, NULL, 0, &keyinfo_on_cards))
-    {
-      agent_card_free_keyinfo (ctrl->server_local->last_card_keyinfo.ki);
-      ctrl->server_local->last_card_keyinfo.ki = keyinfo_on_cards;
-      ctrl->server_local->last_card_keyinfo.eventno = eventcounter.card;
-      ctrl->server_local->last_card_keyinfo.maybe_key_change
-        = eventcounter.maybe_key_change;
-    }
 
   if (list_mode == 2)
     {

g10/call-agent.c

@@ -2236,13 +2236,50 @@ agent_probe_any_secret_key (ctrl_t ctrl, kbnode_t keyblock)
   char line[ASSUAN_LINELENGTH];
   char *p;
   kbnode_t kbctx, node;
-  int nkeys;
+  int nkeys;  /* (always zero in secret_keygrips mode)  */
   unsigned char grip[KEYGRIP_LEN];
+  const unsigned char *s;
+  unsigned int n;
 
   err = start_agent (ctrl, 0);
   if (err)
     return err;
 
+  /* If we have not yet issued a "HAVEKEY --list" do that now.  We use
+   * a more or less arbitray limit of 1000 keys.  */
+  if (ctrl && !ctrl->secret_keygrips && !ctrl->no_more_secret_keygrips)
+    {
+      membuf_t data;
+
+      init_membuf (&data, 4096);
+      err = assuan_transact (agent_ctx, "HAVEKEY --list=1000",
+                             put_membuf_cb, &data,
+                             NULL, NULL, NULL, NULL);
+      if (err)
+        xfree (get_membuf (&data, NULL));
+      else
+        {
+          ctrl->secret_keygrips = get_membuf (&data,
+                                              &ctrl->secret_keygrips_len);
+          if (!ctrl->secret_keygrips)
+            err = gpg_error_from_syserror ();
+          if ((ctrl->secret_keygrips_len % 20))
+            {
+              err = gpg_error (GPG_ERR_INV_DATA);
+              xfree (ctrl->secret_keygrips);
+              ctrl->secret_keygrips = NULL;
+            }
+        }
+      if (err)
+        {
+          log_info ("problem with fast path key listing: %s - ignored\n",
+                    gpg_strerror (err));
+          err = 0;
+        }
+      /* We want to do this only once.  */
+      ctrl->no_more_secret_keygrips = 1;
+    }
+
   err = gpg_error (GPG_ERR_NO_SECKEY); /* Just in case no key was
                                           found in KEYBLOCK.  */
   p = stpcpy (line, "HAVEKEY");
@@ -2251,6 +2288,24 @@ agent_probe_any_secret_key (ctrl_t ctrl, kbnode_t keyblock)
         || node->pkt->pkttype == PKT_PUBLIC_SUBKEY
         || node->pkt->pkttype == PKT_SECRET_KEY
         || node->pkt->pkttype == PKT_SECRET_SUBKEY)
+      {
+        if (ctrl && ctrl->secret_keygrips)
+          {
+            /* We got an array with all secret keygrips.  Check this.  */
+            err = keygrip_from_pk (node->pkt->pkt.public_key, grip);
+            if (err)
+              return err;
+            for (s=ctrl->secret_keygrips, n = 0;
+                 n < ctrl->secret_keygrips_len;
+                 s += 20, n += 20)
+              {
+                if (!memcmp (s, grip, 20))
+                  return 0;
+              }
+            err = gpg_error (GPG_ERR_NO_SECKEY);
+            /* Keep on looping over the keyblock.  Never bump nkeys.  */
+          }
+        else
           {
             if (nkeys && ((p - line) + 41) > (ASSUAN_LINELENGTH - 2))
               {
@@ -2270,6 +2325,7 @@ agent_probe_any_secret_key (ctrl_t ctrl, kbnode_t keyblock)
             p += 40;
             nkeys++;
           }
+      }
 
   if (!err && nkeys)
     err = assuan_transact (agent_ctx, line,
@@ -2419,6 +2475,14 @@ agent_genkey (ctrl_t ctrl, char **cache_nonce_addr, char **passwd_nonce_addr,
     return err;
   dfltparm.ctx = agent_ctx;
 
+  /* Do not use our cache of secret keygrips anymore - this command
+   * would otherwise requiring to update that cache.  */
+  if (ctrl && ctrl->secret_keygrips)
+    {
+      xfree (ctrl->secret_keygrips);
+      ctrl->secret_keygrips = 0;
+    }
+
   if (timestamp)
     {
       strcpy (timestamparg, " --timestamp=");
@@ -2876,6 +2940,14 @@ agent_import_key (ctrl_t ctrl, const char *desc, char **cache_nonce_addr,
     return err;
   dfltparm.ctx = agent_ctx;
 
+  /* Do not use our cache of secret keygrips anymore - this command
+   * would otherwise requiring to update that cache.  */
+  if (ctrl && ctrl->secret_keygrips)
+    {
+      xfree (ctrl->secret_keygrips);
+      ctrl->secret_keygrips = 0;
+    }
+
   if (timestamp)
     {
       strcpy (timestamparg, " --timestamp=");

g10/getkey.c

@@ -3916,7 +3916,7 @@ lookup (ctrl_t ctrl, getkey_ctx_t ctx, int want_secret,
 
       if (want_secret)
 	{
-	  rc = agent_probe_any_secret_key (NULL, keyblock);
+	  rc = agent_probe_any_secret_key (ctrl, keyblock);
 	  if (gpg_err_code(rc) == GPG_ERR_NO_SECKEY)
 	    goto skip; /* No secret key available.  */
 	  if (rc)

g10/gpg.c

@@ -2314,6 +2314,8 @@ gpg_deinit_default_ctrl (ctrl_t ctrl)
 
   keydb_release (ctrl->cached_getkey_kdb);
   gpg_keyboxd_deinit_session_data (ctrl);
+  xfree (ctrl->secret_keygrips);
+  ctrl->secret_keygrips = NULL;
 }
 
 

g10/gpg.h

@@ -110,6 +110,14 @@ struct server_control_s
 
   /* This is used to cache a key data base handle.  */
   KEYDB_HANDLE cached_getkey_kdb;
+
+  /* Cached results from HAVEKEY --list.  They are used if the pointer
+   * is not NULL.  The length gives the length in bytes and is a
+   * multiple of 20.  If the no_more flag is set the list shall not
+   * anymore be refreshed even if it has been freed and NULLed. */
+  unsigned char *secret_keygrips;
+  size_t secret_keygrips_len;
+  int no_more_secret_keygrips;
 };
 
 

g10/keylist.c

@@ -553,7 +553,7 @@ list_all (ctrl_t ctrl, int secret, int mark_secret)
 	}
 
       if (secret || mark_secret)
-        any_secret = !agent_probe_any_secret_key (NULL, keyblock);
+        any_secret = !agent_probe_any_secret_key (ctrl, keyblock);
       else
         any_secret = 0;
 
@@ -645,7 +645,7 @@ list_one (ctrl_t ctrl, strlist_t names, int secret, int mark_secret)
       if (secret)
         any_secret = 1;
       else if (mark_secret)
-        any_secret = !agent_probe_any_secret_key (NULL, keyblock);
+        any_secret = !agent_probe_any_secret_key (ctrl, keyblock);
       else
         any_secret = 0;