[PATCH setup 2/3] Restructure how we try keys in order for signature checking

Jon Turney jon.turney@dronecode.org.uk
Sat Feb 22 14:17:00 GMT 2020


Restructure how we try keys in order for signature checking, so we can
log which key signature was made by
---
 crypto.cc | 97 +++++++++++++++++++++++++++++--------------------------
 1 file changed, 51 insertions(+), 46 deletions(-)

diff --git a/crypto.cc b/crypto.cc
index 5a10e16..3720f01 100644
--- a/crypto.cc
+++ b/crypto.cc
@@ -431,17 +431,23 @@ add_key_from_sexpr (gcry_sexp_t key)
 bool
 verify_ini_file_sig (io_stream *ini_file, io_stream *ini_sig_file, HWND owner)
 {
-  /*  DSA public key in s-expr format.  */
-  gcry_sexp_t dsa_key;
-
   /*  Data returned from packet walker.  */
   struct sig_data sigdat;
 
-  /*  Vector of extra keys to use.  */
-  std::vector<gcry_sexp_t> keys_to_try;
+  /*  Vector of keys to use.  */
+  struct key_info
+  {
+    key_info(std::string _name, bool _builtin, gcry_sexp_t _key, bool _owned=FALSE) :
+      name(_name), builtin(_builtin), key(_key), owned(_owned)
+    {
+    }
 
-  /*  Vector of cached extra keys from last run.  */
-  static std::vector<gcry_sexp_t> input_keys;
+    std::string name;
+    bool builtin;  // if TRUE, we don't need to retain this key with add_key_from_sexpr()
+    gcry_sexp_t key;
+    bool owned;    // if TRUE, we own this key and should use gcry_sexp_release() on it
+  };
+  std::vector<struct key_info> keys_to_try;
 
   /*  Overall status of signature.  */
   bool sig_ok = false;
@@ -454,11 +460,16 @@ verify_ini_file_sig (io_stream *ini_file, io_stream *ini_sig_file, HWND owner)
   gcry_check_version (NULL);
 
   /* So first build the built-in key.  */
+  gcry_sexp_t dsa_key;
   rv = gcry_sexp_new (&dsa_key, cygwin_pubkey_sexpr, strlen (cygwin_pubkey_sexpr), 1);
   if (rv != GPG_ERR_NO_ERROR)
     {
       ERRKIND (owner, IDS_CRYPTO_ERROR, rv, "while creating pubkey s-expr.");
     }
+  else
+    {
+      keys_to_try.push_back (key_info("cygwin", TRUE, dsa_key));
+    }
 
 #if CRYPTODEBUGGING
   char sexprbuf[GPG_KEY_SEXPR_BUF_SIZE];
@@ -466,6 +477,10 @@ verify_ini_file_sig (io_stream *ini_file, io_stream *ini_sig_file, HWND owner)
   msg ("key:%d\n'%s'", n, sexprbuf);
 #endif /* CRYPTODEBUGGING */
 
+
+  /*  Vector of cached extra keys from last run.  */
+  static std::vector<gcry_sexp_t> input_keys;
+
   /* Next we should extract the keys from the extrakeys user
   setting, and flush it; we'll only return them to it if they
   get used.  OTOH, should we do this at all?  The user settings
@@ -504,6 +519,15 @@ verify_ini_file_sig (io_stream *ini_file, io_stream *ini_sig_file, HWND owner)
 	}
     }
 
+  // We only use the untrusted keys if told to.
+  if (KeepUntrustedKeysOption || UntrustedKeysOption)
+    for (std::vector<gcry_sexp_t>::const_iterator it = input_keys.begin ();
+         it < input_keys.end ();
+         ++it)
+      {
+        keys_to_try.push_back (key_info ("saved key", FALSE, *it, FALSE));
+      }
+
   /* Next, there may have been command-line options. */
   std::vector<std::string> SexprExtraKeyStrings = SexprExtraKeyOption;
   for (std::vector<std::string>::const_iterator it
@@ -527,7 +551,7 @@ verify_ini_file_sig (io_stream *ini_file, io_stream *ini_sig_file, HWND owner)
 	  ExtraKeysSetting::instance().add_key (sexprbuf);
 	  msg ("key2:%d\n'%s'", n, sexprbuf);
 #endif /* CRYPTODEBUGGING */
-	  keys_to_try.push_back (dsa_key2);
+	  keys_to_try.push_back (key_info ("from command-line option --sexpr-pubkey", FALSE, dsa_key2));
 	}
       else
 	{
@@ -562,7 +586,7 @@ verify_ini_file_sig (io_stream *ini_file, io_stream *ini_sig_file, HWND owner)
 	      ExtraKeysSetting::instance().add_key (sexprbuf);
 	      msg ("key3:%d\n'%s'", n, sexprbuf);
 #endif /* CRYPTODEBUGGING */
-	      keys_to_try.push_back (kdat.keys.back ());
+	      keys_to_try.push_back (key_info ("from command-line option --pubkey", FALSE, kdat.keys.back ()));
 	      kdat.keys.pop_back ();
 	    }
 	}
@@ -626,40 +650,22 @@ verify_ini_file_sig (io_stream *ini_file, io_stream *ini_sig_file, HWND owner)
       msg ("hash:%d\n'%s'", n, sexprbuf);
 #endif /* CRYPTODEBUGGING */
 
-      // Well, we're actually there!  Try it against the main key.
-      rv = gcry_pk_verify (dsa_sig, dsa_hash, dsa_key);
-      // If not that, try any supplied on the commandline.
-      if (rv != GPG_ERR_NO_ERROR)
-	{
-	  std::vector<gcry_sexp_t>::iterator it;
-	  for (it = keys_to_try.begin (); it < keys_to_try.end (); ++it)
-	    {
-	      MESSAGE ("Testing a key to try\n");
-	      rv = gcry_pk_verify (dsa_sig, dsa_hash, *it);
-	      if (rv != GPG_ERR_NO_ERROR)
-		continue;
-	      // Found it!  This key gets kept!
-	      add_key_from_sexpr (*it);
-	      break;
-	    }
-
-	  // We only use the untrusted keys if told to.
-	  it = ((rv != GPG_ERR_NO_ERROR) 
-	    && (KeepUntrustedKeysOption || UntrustedKeysOption))
-		? input_keys.begin ()
-		: input_keys.end ();
-	  for ( ; it < input_keys.end (); ++it)
-	    {
-	      MESSAGE ("Testing an input key\n");
-	      rv = gcry_pk_verify (dsa_sig, dsa_hash, *it);
-	      if (rv != GPG_ERR_NO_ERROR)
-		continue;
-	      // Found it!  This key gets kept!
-	      add_key_from_sexpr (*it);
-	      break;
-	    }
-	}
-
+      // Well, we're actually there!
+      // Try it against each key in turn
+
+      std::vector<key_info>::iterator it;
+      for (it = keys_to_try.begin (); it < keys_to_try.end (); ++it)
+        {
+          MESSAGE ("Trying key %s\n", it->name.c_str());
+          rv = gcry_pk_verify (dsa_sig, dsa_hash, it->key);
+          if (rv != GPG_ERR_NO_ERROR)
+            continue;
+          // Found it!  This key gets kept!
+          LogBabblePrintf("Valid signature by key %s", it->name.c_str());
+          if (!it->builtin)
+            add_key_from_sexpr (it->key);
+          break;
+        }
       sig_ok = (rv == GPG_ERR_NO_ERROR);
 
 #if CRYPTODEBUGGING
@@ -676,7 +682,6 @@ verify_ini_file_sig (io_stream *ini_file, io_stream *ini_sig_file, HWND owner)
     }
 
   // Discard the temp data then.
-  gcry_sexp_release (dsa_key);
   if (sigdat.dsa_mpi_r)
     gcry_mpi_release (sigdat.dsa_mpi_r);
   if (sigdat.dsa_mpi_s)
@@ -685,10 +690,10 @@ verify_ini_file_sig (io_stream *ini_file, io_stream *ini_sig_file, HWND owner)
     gcry_md_close (sigdat.md);
   while (keys_to_try.size ())
     {
-      gcry_sexp_release (keys_to_try.back ());
+      if (keys_to_try.back ().owned)
+        gcry_sexp_release (keys_to_try.back ().key);
       keys_to_try.pop_back ();
     }
 
   return sig_ok;
 }
-
-- 
2.21.0



More information about the Cygwin-apps mailing list