gpg: Improve passphrase caching.
authorWerner Koch <wk@gnupg.org>
Wed, 17 Sep 2014 13:12:08 +0000 (15:12 +0200)
committerWerner Koch <wk@gnupg.org>
Wed, 17 Sep 2014 13:12:08 +0000 (15:12 +0200)
* agent/cache.c (last_stored_cache_key): New.
(agent_get_cache): Allow NULL for KEY.
(agent_store_cache_hit): New.
* agent/findkey.c (unprotect): Call new function and try to use the
last stored key.

* g10/revoke.c (create_revocation): Add arg CACHE_NONCE and pass to
make_keysig_packet.
(gen_standard_revoke): Add arg CACHE_NONCE and pass to
create_revocation.
* g10/keygen.c (do_generate_keypair): Call gen_standard_revoke with
cache nonce.
--

This patch adds two features:

1. The key for the last passphrase successfully used for unprotecting
a key is stored away.  On a cache miss the stored away passphrase is
tried as well.  This helps for the common GPG use case of having a
signing and encryption (sub)key with the same passphrase.  See the
code for more comments.

2. The now auto-generated revocation certificate does not anymore
popup a passphrase prompt.  Thus for standard key generation the
passphrase needs to be given only once (well, two with the
confirmation).

agent/agent.h
agent/cache.c
agent/findkey.c
g10/keygen.c
g10/main.h
g10/revoke.c

index 4ed8c7f..a420bae 100644 (file)
@@ -364,6 +364,7 @@ void agent_flush_cache (void);
 int agent_put_cache (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);
+void agent_store_cache_hit (const char *key);
 
 
 /*-- pksign.c --*/
index d4deaeb..49402e4 100644 (file)
@@ -65,6 +65,9 @@ struct cache_item_s {
 /* The cache himself.  */
 static ITEM thecache;
 
+/* NULL or the last cache key stored by agent_store_cache_hit.  */
+static char *last_stored_cache_key;
+
 
 /* This function must be called once to initialize this module. It
    has to be done before a second thread is spawned.  */
@@ -388,12 +391,24 @@ agent_get_cache (const char *key, cache_mode_t cache_mode)
   ITEM r;
   char *value = NULL;
   int res;
+  int last_stored = 0;
 
   if (cache_mode == CACHE_MODE_IGNORE)
     return NULL;
 
+  if (!key)
+    {
+      key = last_stored_cache_key;
+      if (!key)
+        return NULL;
+      last_stored = 1;
+    }
+
+
   if (DBG_CACHE)
-    log_debug ("agent_get_cache '%s' (mode %d) ...\n", key, cache_mode);
+    log_debug ("agent_get_cache '%s' (mode %d)%s ...\n",
+               key, cache_mode,
+               last_stored? " (stored cache key)":"");
   housekeeping ();
 
   for (r=thecache; r; r = r->next)
@@ -404,6 +419,7 @@ agent_get_cache (const char *key, cache_mode_t cache_mode)
               || r->cache_mode == cache_mode)
           && !strcmp (r->key, key))
         {
+          /* Note: To avoid races KEY may not be accessed anymore below.  */
           r->accessed = gnupg_get_time ();
           if (DBG_CACHE)
             log_debug ("... hit\n");
@@ -442,3 +458,14 @@ agent_get_cache (const char *key, cache_mode_t cache_mode)
 
   return NULL;
 }
+
+
+/* Store the key for the last successful cache hit.  That value is
+   used by agent_get_cache if the requested KEY is given as NULL.
+   NULL may be used to remove that key. */
+void
+agent_store_cache_hit (const char *key)
+{
+  xfree (last_stored_cache_key);
+  last_stored_cache_key = key? xtrystrdup (key) : NULL;
+}
index 5ff263e..fbe3031 100644 (file)
@@ -372,6 +372,8 @@ unprotect (ctrl_t ctrl, const char *cache_nonce, const char *desc_text,
           rc = agent_unprotect (ctrl, *keybuf, pw, NULL, &result, &resultlen);
           if (!rc)
             {
+              if (cache_mode == CACHE_MODE_NORMAL)
+                agent_store_cache_hit (hexgrip);
               if (r_passphrase)
                 *r_passphrase = pw;
               else
@@ -383,6 +385,45 @@ unprotect (ctrl_t ctrl, const char *cache_nonce, const char *desc_text,
           xfree (pw);
           rc  = 0;
         }
+      else if (cache_mode == CACHE_MODE_NORMAL)
+        {
+          /* The standard use of GPG keys is to have a signing and an
+             encryption subkey.  Commonly both use the same
+             passphrase.  We try to help the user to enter the
+             passphrase only once by silently trying the last
+             correctly entered passphrase.  Checking one additional
+             passphrase should be acceptable; despite the S2K
+             introduced delays. The assumed workflow is:
+
+               1. Read encrypted message in a MUA and thus enter a
+                  passphrase for the encryption subkey.
+
+               2. Reply to that mail with an encrypted and signed
+                  mail, thus entering the passphrase for the signing
+                  subkey.
+
+             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);
+          if (pw)
+            {
+              rc = agent_unprotect (ctrl, *keybuf, pw, NULL,
+                                    &result, &resultlen);
+              if (!rc)
+                {
+                  if (r_passphrase)
+                    *r_passphrase = pw;
+                  else
+                    xfree (pw);
+                  xfree (*keybuf);
+                  *keybuf = result;
+                  return 0;
+                }
+              xfree (pw);
+              rc  = 0;
+            }
+        }
 
       /* If the pinentry is currently in use, we wait up to 60 seconds
          for it to close and check the cache again.  This solves a common
@@ -460,6 +501,7 @@ unprotect (ctrl_t ctrl, const char *cache_nonce, const char *desc_text,
         {
           agent_put_cache (hexgrip, cache_mode, pi->pin,
                            lookup_ttl? lookup_ttl (hexgrip) : 0);
+          agent_store_cache_hit (hexgrip);
           if (r_passphrase && *pi->pin)
             *r_passphrase = xtrystrdup (pi->pin);
         }
index 92337bb..4ae34bf 100644 (file)
@@ -4115,7 +4115,7 @@ do_generate_keypair (struct para_data_s *para,
           update_ownertrust (pk, ((get_ownertrust (pk) & ~TRUST_MASK)
                                   | TRUST_ULTIMATE ));
 
-          gen_standard_revoke (pk);
+          gen_standard_revoke (pk, cache_nonce);
 
           if (!opt.batch)
             {
index 4eb1b5f..44c4478 100644 (file)
@@ -333,7 +333,7 @@ int enarmor_file( const char *fname );
 
 /*-- revoke.c --*/
 struct revocation_reason_info;
-int gen_standard_revoke (PKT_public_key *psk);
+int gen_standard_revoke (PKT_public_key *psk, const char *cache_nonce);
 int gen_revoke( const char *uname );
 int gen_desig_revoke( const char *uname, strlist_t locusr);
 int revocation_reason_build_cb( PKT_signature *sig, void *opaque );
index 67f62e5..019c62c 100644 (file)
@@ -443,7 +443,8 @@ create_revocation (const char *filename,
                    struct revocation_reason_info *reason,
                    PKT_public_key *psk,
                    kbnode_t keyblock,
-                   const char *leadintext, int suffix)
+                   const char *leadintext, int suffix,
+                   const char *cache_nonce)
 {
   int rc;
   iobuf_t out = NULL;
@@ -466,7 +467,7 @@ create_revocation (const char *filename,
   rc = make_keysig_packet (&sig, psk, NULL, NULL, psk, 0x20, 0,
                            opt.force_v4_certs? 4:0,
                            0, 0,
-                           revocation_reason_build_cb, reason, NULL);
+                           revocation_reason_build_cb, reason, cache_nonce);
   if (rc)
     {
       log_error (_("make_keysig_packet failed: %s\n"), g10_errstr (rc));
@@ -511,9 +512,10 @@ create_revocation (const char *filename,
    by gpg's interactive key generation function.  The certificate is
    stored at a dedicated place in a slightly modified form to avoid an
    accidental import.  PSK is the primary key; a corresponding secret
-   key must be available.  */
+   key must be available.  CACHE_NONCE is optional but can be used to
+   help gpg-agent to avoid an extra passphrase prompt. */
 int
-gen_standard_revoke (PKT_public_key *psk)
+gen_standard_revoke (PKT_public_key *psk, const char *cache_nonce)
 {
   int rc;
   estream_t memfp;
@@ -573,7 +575,7 @@ gen_standard_revoke (PKT_public_key *psk)
 
   reason.code = 0x00; /* No particular reason.  */
   reason.desc = NULL;
-  rc = create_revocation (fname, &reason, psk, NULL, leadin, 3);
+  rc = create_revocation (fname, &reason, psk, NULL, leadin, 3, cache_nonce);
   xfree (leadin);
   xfree (fname);
 
@@ -662,7 +664,7 @@ gen_revoke (const char *uname)
   if (!opt.armor)
     tty_printf (_("ASCII armored output forced.\n"));
 
-  rc = create_revocation (NULL, reason, psk, keyblock, NULL, 0);
+  rc = create_revocation (NULL, reason, psk, keyblock, NULL, 0, NULL);
   if (rc)
     goto leave;