scd: Simplify the app_readkey parameters.

* scd/app-help.c (app_help_pubkey_from_cert): New.
* scd/command.c (cmd_readkey): Refactor to use that new function and
handle the --advanced flag only here.
* scd/app.c (app_readkey): Remove parm advanced.
* scd/app-common.h (struct app_ctx_s): Remove parm advanced from the
readkey member.
* scd/app-nks.c (do_readkey): Adjust for removed parm.
* scd/app-piv.c (do_readkey): Ditto.
* scd/app-openpgp.c (do_readkey): Ditto.

Signed-off-by: Werner Koch <[email protected]>

7 files changed, 93 insertions, 117 deletions

diff --git a/scd/app-common.h b/scd/app-common.h
index e9d167ecc..97274a7cb 100644
--- a/scd/app-common.h
+++ b/scd/app-common.h
@@ -71,7 +71,7 @@ struct app_ctx_s {
     gpg_error_t (*learn_status) (app_t app, ctrl_t ctrl, unsigned int flags);
     gpg_error_t (*readcert) (app_t app, const char *certid,
                      unsigned char **cert, size_t *certlen);
-    gpg_error_t (*readkey) (app_t app, int advanced, const char *certid,
+    gpg_error_t (*readkey) (app_t app, const char *certid,
                     unsigned char **pk, size_t *pklen);
     gpg_error_t (*getattr) (app_t app, ctrl_t ctrl, const char *name);
     gpg_error_t (*setattr) (app_t app, const char *name,
@@ -123,6 +123,8 @@ struct app_ctx_s {
 /*-- app-help.c --*/
 unsigned int app_help_count_bits (const unsigned char *a, size_t len);
 gpg_error_t app_help_get_keygrip_string (ksba_cert_t cert, char *hexkeygrip);
+gpg_error_t app_help_pubkey_from_cert (const void *cert, size_t certlen,
+                                       unsigned char **r_pk, size_t *r_pklen);
 size_t app_help_read_length_of_cert (int slot, int fid, size_t *r_certoff);
 
 
@@ -144,7 +146,7 @@ gpg_error_t app_write_learn_status (app_t app, ctrl_t ctrl,
                                     unsigned int flags);
 gpg_error_t app_readcert (app_t app, ctrl_t ctrl, const char *certid,
                   unsigned char **cert, size_t *certlen);
-gpg_error_t app_readkey (app_t app, ctrl_t ctrl, int advanced,
+gpg_error_t app_readkey (app_t app, ctrl_t ctrl,
                  const char *keyid, unsigned char **pk, size_t *pklen);
 gpg_error_t app_getattr (app_t app, ctrl_t ctrl, const char *name);
 gpg_error_t app_setattr (app_t app, ctrl_t ctrl, const char *name,
diff --git a/scd/app-help.c b/scd/app-help.c
index 842a73d5a..f0f551c55 100644
--- a/scd/app-help.c
+++ b/scd/app-help.c
@@ -29,9 +29,9 @@
 #include "../common/tlv.h"
 
 
-/* Count the number of bits, assuming the A represents an unsigned big
-   integer of length LEN bytes.  If A is NULL a length of 0 is
-   returned. */
+/* Count the number of bits, assuming that A represents an unsigned
+ * big integer of length LEN bytes.  If A is NULL a length of 0 is
+ * returned. */
 unsigned int
 app_help_count_bits (const unsigned char *a, size_t len)
 {
@@ -87,6 +87,45 @@ app_help_get_keygrip_string (ksba_cert_t cert, char *hexkeygrip)
 }
 
 
+gpg_error_t
+app_help_pubkey_from_cert (const void *cert, size_t certlen,
+                           unsigned char **r_pk, size_t *r_pklen)
+{
+  gpg_error_t err;
+  ksba_cert_t kc;
+  unsigned char *pk;
+  size_t pklen;
+
+  *r_pk = NULL;
+  *r_pklen = 0;
+
+  err = ksba_cert_new (&kc);
+  if (err)
+    return err;
+
+  err = ksba_cert_init_from_mem (kc, cert, certlen);
+  if (err)
+    goto leave;
+
+  pk = ksba_cert_get_public_key (kc);
+  if (!pk)
+    {
+      err = gpg_error (GPG_ERR_NO_PUBKEY);
+      goto leave;
+    }
+  pklen = gcry_sexp_canon_len (pk, 0, NULL, &err);
+
+ leave:
+  if (!err)
+    {
+      *r_pk = pk;
+      *r_pklen = pklen;
+    }
+  else
+    ksba_free (pk);
+  ksba_cert_release (kc);
+  return err;
+}
 
 /* Given the SLOT and the File ID FID, return the length of the
    certificate contained in that file. Returns 0 if the file does not
diff --git a/scd/app-nks.c b/scd/app-nks.c
index 0f38e7cd0..40c941616 100644
--- a/scd/app-nks.c
+++ b/scd/app-nks.c
@@ -618,17 +618,13 @@ do_readcert (app_t app, const char *certid,
    certificate parsing code in commands.c:cmd_readkey.  For internal
    use PK and PKLEN may be NULL to just check for an existing key.  */
 static gpg_error_t
-do_readkey (app_t app, int advanced, const char *keyid,
-            unsigned char **pk, size_t *pklen)
+do_readkey (app_t app, const char *keyid, unsigned char **pk, size_t *pklen)
 {
   gpg_error_t err;
   unsigned char *buffer[2];
   size_t buflen[2];
   unsigned short path[1] = { 0x4500 };
 
-  if (advanced)
-    return GPG_ERR_NOT_SUPPORTED;
-
   /* We use a generic name to retrieve PK.AUT.IFD-SPK.  */
   if (!strcmp (keyid, "$IFDAUTHKEY") && app->app_local->nks_version >= 3)
     ;
@@ -702,7 +698,7 @@ do_writekey (app_t app, ctrl_t ctrl,
   else
     return gpg_error (GPG_ERR_INV_ID);
 
-  if (!force && !do_readkey (app, 0, keyid, NULL, NULL))
+  if (!force && !do_readkey (app, keyid, NULL, NULL))
     return gpg_error (GPG_ERR_EEXIST);
 
   /* Parse the S-expression.  */
diff --git a/scd/app-openpgp.c b/scd/app-openpgp.c
index b6ead6478..de8a7742b 100644
--- a/scd/app-openpgp.c
+++ b/scd/app-openpgp.c
@@ -1875,10 +1875,8 @@ do_learn_status (app_t app, ctrl_t ctrl, unsigned int flags)
    buffer. On error PK and PKLEN are not changed and an error code is
    returned.  */
 static gpg_error_t
-do_readkey (app_t app, int advanced, const char *keyid,
-            unsigned char **pk, size_t *pklen)
+do_readkey (app_t app, const char *keyid, unsigned char **pk, size_t *pklen)
 {
-#if GNUPG_MAJOR_VERSION > 1
   gpg_error_t err;
   int keyno;
   unsigned char *buf;
@@ -1900,45 +1898,17 @@ do_readkey (app_t app, int advanced, const char *keyid,
   if (!buf)
     return gpg_error (GPG_ERR_NO_PUBKEY);
 
-  if (advanced)
+  *pklen = app->app_local->pk[keyno].keylen;
+  *pk = xtrymalloc (*pklen);
+  if (!*pk)
     {
-      gcry_sexp_t s_key;
-
-      err = gcry_sexp_new (&s_key, buf, app->app_local->pk[keyno].keylen, 0);
-      if (err)
-        return err;
-
-      *pklen = gcry_sexp_sprint (s_key, GCRYSEXP_FMT_ADVANCED, NULL, 0);
-      *pk = xtrymalloc (*pklen);
-      if (!*pk)
-        {
-          err = gpg_error_from_syserror ();
-          *pklen = 0;
-          return err;
-        }
-
-      gcry_sexp_sprint (s_key, GCRYSEXP_FMT_ADVANCED, *pk, *pklen);
-      gcry_sexp_release (s_key);
-      /* Decrement for trailing '\0' */
-      *pklen = *pklen - 1;
-    }
-  else
-    {
-      *pklen = app->app_local->pk[keyno].keylen;
-      *pk = xtrymalloc (*pklen);
-      if (!*pk)
-        {
-          err = gpg_error_from_syserror ();
-          *pklen = 0;
-          return err;
-        }
-      memcpy (*pk, buf, *pklen);
+      err = gpg_error_from_syserror ();
+      *pklen = 0;
+      return err;
     }
+  memcpy (*pk, buf, *pklen);
 
   return 0;
-#else
-  return gpg_error (GPG_ERR_NOT_IMPLEMENTED);
-#endif
 }
 
 /* Read the standard certificate of an OpenPGP v2 card.  It is
diff --git a/scd/app-piv.c b/scd/app-piv.c
index 704a498c6..495e26a63 100644
--- a/scd/app-piv.c
+++ b/scd/app-piv.c
@@ -1418,14 +1418,14 @@ do_readcert (app_t app, const char *certid,
  * application has been performed.  This is because we return a cached
  * result from key generation.  If no cached result is available, the
  * error GPG_ERR_UNSUPPORTED_OPERATION is returned so that the higher
- * layer can then to get the key by reading the matching certificate.
+ * layer can then get the key by reading the matching certificate.
  * On success a canonical encoded s-expression with the public key is
  * stored at (R_PK,R_PKLEN); the caller must release that buffer.  On
  * error R_PK and R_PKLEN are not changed and an error code is
  * returned.
  */
 static gpg_error_t
-do_readkey (app_t app, int advanced, const char *keyrefstr,
+do_readkey (app_t app, const char *keyrefstr,
             unsigned char **r_pk, size_t *r_pklen)
 {
   gpg_error_t err;
@@ -1471,19 +1471,6 @@ do_readkey (app_t app, int advanced, const char *keyrefstr,
   err = make_canon_sexp (s_pkey, &pk, &pklen);
   if (err)
     goto leave;
-  if (advanced)
-    {
-      /* FIXME: How ugly - we should move that to command.c */
-      char *p = canon_sexp_to_string (pk, pklen);
-      if (!p)
-        {
-          err = gpg_error_from_syserror ();
-          goto leave;
-        }
-      xfree (pk);
-      pk = p;
-      pklen = strlen (pk);
-    }
 
   *r_pk = pk;
   pk = NULL;
diff --git a/scd/app.c b/scd/app.c
index 8d4d1c032..ada1eb963 100644
--- a/scd/app.c
+++ b/scd/app.c
@@ -692,7 +692,7 @@ app_readcert (app_t app, ctrl_t ctrl, const char *certid,
 
    This function might not be supported by all applications.  */
 gpg_error_t
-app_readkey (app_t app, ctrl_t ctrl, int advanced, const char *keyid,
+app_readkey (app_t app, ctrl_t ctrl, const char *keyid,
              unsigned char **pk, size_t *pklen)
 {
   gpg_error_t err;
@@ -711,7 +711,7 @@ app_readkey (app_t app, ctrl_t ctrl, int advanced, const char *keyid,
   err = lock_app (app, ctrl);
   if (err)
     return err;
-  err= app->fnc.readkey (app, advanced, keyid, pk, pklen);
+  err= app->fnc.readkey (app, keyid, pk, pklen);
   unlock_app (app);
   return err;
 }
diff --git a/scd/command.c b/scd/command.c
index b4098e465..5b2ca6c29 100644
--- a/scd/command.c
+++ b/scd/command.c
@@ -516,11 +516,8 @@ cmd_readkey (assuan_context_t ctx, char *line)
   int rc;
   int advanced = 0;
   unsigned char *cert = NULL;
-  size_t ncert, n;
-  ksba_cert_t kc = NULL;
-  ksba_sexp_t p = NULL;
-  unsigned char *pk;
-  size_t pklen;
+  unsigned char *pk = NULL;
+  size_t ncert, pklen;
 
   if ((rc = open_card (ctrl)))
     return rc;
@@ -529,83 +526,68 @@ cmd_readkey (assuan_context_t ctx, char *line)
     advanced = 1;
 
   line = skip_options (line);
-
   line = xstrdup (line); /* Need a copy of the line. */
+
   /* If the application supports the READKEY function we use that.
      Otherwise we use the old way by extracting it from the
      certificate.  */
-  rc = app_readkey (ctrl->app_ctx, ctrl, advanced, line, &pk, &pklen);
+  rc = app_readkey (ctrl->app_ctx, ctrl, line, &pk, &pklen);
   if (!rc)
-    { /* Yeah, got that key - send it back.  */
-      rc = assuan_send_data (ctx, pk, pklen);
-      xfree (pk);
-      xfree (line);
-      line = NULL;
-      goto leave;
-    }
-
-  if (gpg_err_code (rc) != GPG_ERR_UNSUPPORTED_OPERATION
-      && gpg_err_code (rc) != GPG_ERR_NOT_FOUND)
-    log_error ("app_readkey failed: %s\n", gpg_strerror (rc));
-  else
+    ; /* Okay, got that key.  */
+  else if (gpg_err_code (rc) == GPG_ERR_UNSUPPORTED_OPERATION
+           || gpg_err_code (rc) == GPG_ERR_NOT_FOUND)
     {
+      /* Fall back to certificate reading.  */
       rc = app_readcert (ctrl->app_ctx, ctrl, line, &cert, &ncert);
       if (rc)
-        log_error ("app_readcert failed: %s\n", gpg_strerror (rc));
-    }
-  xfree (line);
-  line = NULL;
-  if (rc)
-    goto leave;
-
-  rc = ksba_cert_new (&kc);
-  if (rc)
-    goto leave;
-
-  rc = ksba_cert_init_from_mem (kc, cert, ncert);
-  if (rc)
-    {
-      log_error ("failed to parse the certificate: %s\n", gpg_strerror (rc));
-      goto leave;
+        {
+          log_error ("app_readcert failed: %s\n", gpg_strerror (rc));
+          goto leave;
+        }
+      rc = app_help_pubkey_from_cert (cert, ncert, &pk, &pklen);
+      if (rc)
+        {
+          log_error ("failed to parse the certificate: %s\n",
+                     gpg_strerror (rc));
+          goto leave;
+        }
     }
-
-  p = ksba_cert_get_public_key (kc);
-  if (!p)
+  else
     {
-      rc = gpg_error (GPG_ERR_NO_PUBKEY);
+      log_error ("app_readkey failed: %s\n", gpg_strerror (rc));
       goto leave;
     }
-  n = gcry_sexp_canon_len (p, 0, NULL, NULL);
 
   if (advanced)
     {
       gcry_sexp_t s_key;
+      unsigned char *pkadv;
+      size_t pkadvlen;
 
-      rc = gcry_sexp_new (&s_key, (void*)p, n, 0);
+      rc = gcry_sexp_new (&s_key, pk, pklen, 0);
       if (rc)
         goto leave;
 
-      pklen = gcry_sexp_sprint (s_key, GCRYSEXP_FMT_ADVANCED, NULL, 0);
-      pk = xtrymalloc (pklen);
-      if (!pk)
+      pkadvlen = gcry_sexp_sprint (s_key, GCRYSEXP_FMT_ADVANCED, NULL, 0);
+      pkadv = xtrymalloc (pkadvlen);
+      if (!pkadv)
         {
           rc = gpg_error_from_syserror ();
           goto leave;
         }
-      log_assert (pklen);
+      log_assert (pkadvlen);
 
-      gcry_sexp_sprint (s_key, GCRYSEXP_FMT_ADVANCED, pk, pklen);
+      gcry_sexp_sprint (s_key, GCRYSEXP_FMT_ADVANCED, pkadv, pkadvlen);
       gcry_sexp_release (s_key);
       /* (One less to adjust for the trailing '\0') */
-      rc = assuan_send_data (ctx, pk, pklen-1);
-      xfree (pk);
+      rc = assuan_send_data (ctx, pkadv, pkadvlen-1);
+      xfree (pkadv);
     }
   else
-    rc = assuan_send_data (ctx, p, n);
+    rc = assuan_send_data (ctx, pk, pklen);
 
  leave:
-  xfree (p);
-  ksba_cert_release (kc);
+  xfree (pk);
   xfree (cert);
   return rc;
 }