[PATCH] agent: API change of agent_key_from_file

NIIBE Yutaka gniibe at fsij.org
Mon Feb 24 05:03:33 CET 2014


Hello,

I'd like to update the smartcard support of ECC in the master branch.

I found that current master doesn't work well for signing with ECC key
by smartcard (it worked in 2013, IIRC).  Perhaps, it is due to some
interface changes of libgcrypt.  Specifically, gpg-agent fails in the
function agent_public_key_from_file, when it will find required
parameters are not found.  In the file, it's S-expression and it's
like:

  (shadowed-private-key
   (ecc (curve "secp256k1")
        (q <PUBLIC_KEY>)
        (shadowed t1-v1 (<CARD_INFO> "OPENPGP.1"))))

and the function function agent_public_key_from_file tries to fill
paramerters of "pabgnq" (while only the parameter "q" is available
directly).

I think that we need some clean up here.

Reading the code which calls agent_public_key_from_file
(agent_pksign_do), all we need to know is which key type.

My proposal fix is to change the semantics of agent_key_from_file a
bit.  Currently, callers of agent_key_from_file know that private key
is on smartcard by examining S_SKEY.  I'd like to change this function
to return the content of S_SKEY always (even if the private key is on
smartcard), so that we can check the key type later.  We can examine
the SHADOW_INFO to know it's on smartcard or not.

Here's the patch.  Any comments are appreciated.

diff --git a/agent/command.c b/agent/command.c
index 4fa40d9..c2f3752 100644
--- a/agent/command.c
+++ b/agent/command.c
@@ -1667,7 +1667,7 @@ cmd_passwd (assuan_context_t ctx, char *line)
                              &s_skey, &passphrase);
   if (err)
     ;
-  else if (!s_skey)
+  else if (shadow_info)
     {
       log_error ("changing a smartcard PIN is not yet supported\n");
       err = gpg_error (GPG_ERR_NOT_IMPLEMENTED);
@@ -2126,6 +2126,7 @@ cmd_export_key (assuan_context_t ctx, char *line)
   int openpgp;
   char *cache_nonce;
   char *passphrase = NULL;
+  unsigned char *shadow_info = NULL;
 
   openpgp = has_option (line, "--openpgp");
   cache_nonce = option_value (line, "--cache-nonce");
@@ -2163,11 +2164,11 @@ cmd_export_key (assuan_context_t ctx, char *line)
   /* Get the key from the file.  With the openpgp flag we also ask for
      the passphrase so that we can use it to re-encrypt it.  */
   err = agent_key_from_file (ctrl, NULL, ctrl->server_local->keydesc, grip,
-                             NULL, CACHE_MODE_IGNORE, NULL, &s_skey,
+                             &shadow_info, CACHE_MODE_IGNORE, NULL, &s_skey,
                              openpgp ? &passphrase : NULL);
   if (err)
     goto leave;
-  if (!s_skey)
+  if (shadow_info)
     {
       /* Key is on a smartcard.  Actually we should not see this here
          because we do not pass a shadow_info variable to the above
@@ -2241,6 +2242,7 @@ cmd_export_key (assuan_context_t ctx, char *line)
   gcry_sexp_release (s_skey);
   xfree (ctrl->server_local->keydesc);
   ctrl->server_local->keydesc = NULL;
+  xfree (shadow_info);
 
   return leave_cmd (ctx, err);
 }
@@ -2260,7 +2262,7 @@ cmd_keytocard (assuan_context_t ctx, char *line)
   unsigned char *keydata;
   size_t keydatalen, timestamplen;
   const char *serialno, *timestamp_str, *id;
-  unsigned char *shadow_info;
+  unsigned char *shadow_info = NULL;
   unsigned char *shdkey;
   time_t timestamp;
 
@@ -2305,12 +2307,20 @@ cmd_keytocard (assuan_context_t ctx, char *line)
     return gpg_error (GPG_ERR_INV_VALUE);
 
   err = agent_key_from_file (ctrl, NULL, ctrl->server_local->keydesc, grip,
-                             NULL, CACHE_MODE_IGNORE, NULL, &s_skey, NULL);
+                             &shadow_info, CACHE_MODE_IGNORE, NULL,
+                             &s_skey, NULL);
   if (err)
-    return err;
-  if (!s_skey)
-    /* Key is on a smartcard already.  */
-    return gpg_error (GPG_ERR_UNUSABLE_SECKEY);
+    {
+      xfree (shadow_info);
+      return err;
+    }
+  if (shadow_info)
+    {
+      /* Key is on a smartcard already.  */
+      xfree (shadow_info);
+      shadow_info = NULL;
+      return gpg_error (GPG_ERR_UNUSABLE_SECKEY);
+    }
 
   keydatalen =  gcry_sexp_sprint (s_skey, GCRYSEXP_FMT_CANON, NULL, 0);
   keydata = xtrymalloc_secure (keydatalen + 30);
diff --git a/agent/findkey.c b/agent/findkey.c
index 6464b02..7d8c41a 100644
--- a/agent/findkey.c
+++ b/agent/findkey.c
@@ -562,7 +562,6 @@ agent_key_from_file (ctrl_t ctrl, const char *cache_nonce,
   unsigned char *buf;
   size_t len, buflen, erroff;
   gcry_sexp_t s_skey;
-  int got_shadow_info = 0;
 
   *result = NULL;
   if (shadow_info)
@@ -638,7 +637,6 @@ agent_key_from_file (ctrl_t ctrl, const char *cache_nonce,
                 {
                   memcpy (*shadow_info, s, n);
                   rc = 0;
-                  got_shadow_info = 1;
                 }
             }
           if (rc)
@@ -654,7 +652,7 @@ agent_key_from_file (ctrl_t ctrl, const char *cache_nonce,
     }
   gcry_sexp_release (s_skey);
   s_skey = NULL;
-  if (rc || got_shadow_info)
+  if (rc)
     {
       xfree (buf);
       if (r_passphrase)
diff --git a/agent/pkdecrypt.c b/agent/pkdecrypt.c
index 9924d6d..14aa78f 100644
--- a/agent/pkdecrypt.c
+++ b/agent/pkdecrypt.c
@@ -79,7 +79,7 @@ agent_pkdecrypt (ctrl_t ctrl, const char *desc_text,
       goto leave;
     }
 
-  if (!s_skey)
+  if (shadow_info)
     { /* divert operation to the smartcard */
 
       if (!gcry_sexp_canon_len (ciphertext, ciphertextlen, NULL, NULL))
diff --git a/agent/pksign.c b/agent/pksign.c
index 4d0a240..0886150 100644
--- a/agent/pksign.c
+++ b/agent/pksign.c
@@ -299,31 +299,20 @@ agent_pksign_do (ctrl_t ctrl, const char *cache_nonce,
       goto leave;
     }
 
-  if (!s_skey)
+  if (shadow_info)
     {
       /* Divert operation to the smartcard */
-      gcry_sexp_t s_pkey, l;
-      const char *name;
       size_t len;
       unsigned char *buf = NULL;
+      int key_type;
       int is_RSA = 0;
       int is_ECDSA = 0;
 
-      /* Check keytype by public key */
-      rc = agent_public_key_from_file (ctrl, ctrl->keygrip, &s_pkey);
-      if (rc)
-        {
-          log_error ("failed to read the public key\n");
-          goto leave;
-        }
-      l = gcry_sexp_cadr (s_pkey);
-      name = gcry_sexp_nth_data (l, 0, &len);
-      if (len == 3 && !memcmp (name, "rsa", 3))
+      key_type = agent_is_dsa_key (s_skey);
+      if (key_type == 0)
         is_RSA = 1;
-      else if (len == 5 && !memcmp (name, "ecdsa", 5))
+      else if (key_type == GCRY_PK_ECDSA)
         is_ECDSA = 1;
-      gcry_sexp_release (l);
-      gcry_sexp_release (s_pkey);
 
       rc = divert_pksign (ctrl,
                           ctrl->digest.value,
-- 





More information about the Gnupg-devel mailing list