[PATCH] agent: API change of agent_key_from_file

NIIBE Yutaka gniibe at fsij.org
Tue Mar 4 04:06:00 CET 2014


On 2014-02-24 at 13:03 +0900, NIIBE Yutaka wrote:
> 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.

The patch is revised.  I changed comments for new API, too.

    agent: API change of agent_key_from_file.
    
    * agent/findkey.c (agent_key_from_file): Always return S-expression.
    * agent/command.c (cmd_passwd): Distinguish by SHADOW_INFO.
    (cmd_export_key): Likewise.  Free SHADOW_INFO.
    (cmd_keytocard): Likewise.  Release S_SKEY.
    * agent/pkdecrypt.c (agent_pkdecrypt): Likewise.
    * agent/pksign.c (agent_pksign_do): Likewise.  Use the S-expression to
    know the key type.
    
    Signed-off-by: NIIBE Yutaka <gniibe at fsij.org>

OK to push?

diff --git a/agent/command.c b/agent/command.c
index 4fa40d9..d1e53cd 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,15 +2164,13 @@ 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
-         function, thus it will return this error directly.  */
+      /* Key is on a smartcard.  */
       err = gpg_error (GPG_ERR_UNUSABLE_SECKEY);
       goto leave;
     }
@@ -2241,6 +2240,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 +2260,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 +2305,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);
+      gcry_sexp_release (s_skey);
+      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..7b24c55 100644
--- a/agent/findkey.c
+++ b/agent/findkey.c
@@ -537,9 +537,9 @@ read_key_file (const unsigned char *grip, gcry_sexp_t *result)
 

 /* Return the secret key as an S-Exp in RESULT after locating it using
-   the GRIP.  Stores NULL at RESULT if the operation shall be diverted
-   to a token; in this case an allocated S-expression with the
-   shadow_info part from the file is stored at SHADOW_INFO.
+   the GRIP.  If the operation shall be diverted to a token, an
+   allocated S-expression with the shadow_info part from the file is
+   stored at SHADOW_INFO; if not NULL will be stored at SHADOW_INFO.
    CACHE_MODE defines now the cache shall be used.  DESC_TEXT may be
    set to present a custom description for the pinentry.  LOOKUP_TTL
    is an optional function to convey a TTL to the cache manager; we do
@@ -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