scd:openpgp: Allow auto-changing of the key attributes in genkey.

* scd/app-openpgp.c (struct app_local_s): Add field keyalgo.
(parse_algorithm_attribute): Store the new keyalgo field.
(change_keyattr): Change info message.
(change_keyattr_from_string): Rewrite to also accept a keyref and a
keyalgo string.
(do_genkey): Change the keyattr if a keyalgo string is given.
* scd/command.c (cmd_genkey): Add option --algo.
--

Having this feature makes it easier to use OpenPGP cards in a similar
way to other cards.  Note that the explicit changing via SETATTR is
still supported.

Signed-off-by: Werner Koch <[email protected]>
(cherry picked from commit d7d75da50543bc7259c5a6e6367b58cbca7f1b7b)
(cherry picked from commit b349adc5c0d00d2fc405a45bd078f1580b5610cc)

3 files changed, 215 insertions, 69 deletions

diff --git a/common/openpgpdefs.h b/common/openpgpdefs.h
index f7ea0b52c..e50c94e9b 100644
--- a/common/openpgpdefs.h
+++ b/common/openpgpdefs.h
@@ -226,5 +226,8 @@ gpg_error_t compute_openpgp_fpr_ecc (int keyversion,
                                      unsigned char *result,
                                      unsigned int *r_resultlen);
 
+/*-- openpgp-oid.c --*/
+enum gcry_pk_algos map_openpgp_pk_to_gcry (pubkey_algo_t algo);
+
 
 #endif /*GNUPG_COMMON_OPENPGPDEFS_H*/
diff --git a/scd/app-openpgp.c b/scd/app-openpgp.c
index e7288ef2d..1e282fac1 100644
--- a/scd/app-openpgp.c
+++ b/scd/app-openpgp.c
@@ -223,6 +223,7 @@ struct app_local_s {
    struct
    {
     key_type_t key_type;
+    const char *keyalgo;         /* Algorithm in standard string format.  */
     union {
       struct {
         unsigned int n_bits;     /* Size of the modulus in bits.  The rest
@@ -257,6 +258,7 @@ static gpg_error_t change_keyattr_from_string
                            (app_t app,
                             gpg_error_t (*pincb)(void*, const char *, char **),
                             void *pincb_arg,
+                            const char *keyref, const char *keyalgo,
                             const void *value, size_t valuelen);
 
 
@@ -2612,7 +2614,8 @@ do_setattr (app_t app, ctrl_t ctrl, const char *name,
     return gpg_error (GPG_ERR_NOT_SUPPORTED); /* Not yet supported.  */
 
   if (table[idx].special == 3)
-    return change_keyattr_from_string (app, pincb, pincb_arg, value, valuelen);
+    return change_keyattr_from_string (app, pincb, pincb_arg,
+                                       NULL, NULL, value, valuelen);
 
   switch (table[idx].need_chv)
     {
@@ -3389,9 +3392,10 @@ change_keyattr (app_t app, int keyno, const unsigned char *buf, size_t buflen,
   /* Change the attribute.  */
   err = iso7816_put_data (app_get_slot (app), 0, 0xC1+keyno, buf, buflen);
   if (err)
-    log_error ("error changing key attribute (key=%d)\n", keyno+1);
+    log_error ("error changing key attribute of OPENPGP.%d\n",
+               keyno+1);
   else
-    log_info ("key attribute changed (key=%d)\n", keyno+1);
+    log_info ("key attribute of OPENPGP.%d changed\n", keyno+1);
   flush_cache (app);
   err = parse_algorithm_attribute (app, keyno);
   app->did_chv1 = 0;
@@ -3451,51 +3455,138 @@ change_rsa_keyattr (app_t app, int keyno, unsigned int nbits,
 
 
 /* Helper to process an setattr command for name KEY-ATTR.
-   In (VALUE,VALUELEN), it expects following string:
-        RSA: "--force <key> <algo> rsa<nbits>"
-        ECC: "--force <key> <algo> <curvename>"
-  */
+ *
+ * If KEYREF and KEYALGO are NULL (VALUE,VALUELEN) are expected to
+ * contain one of the following strings:
+ *       RSA: "--force <key> <algo> rsa<nbits>"
+ *       ECC: "--force <key> <algo> <curvename>"
+ *
+ * If KEYREF and KEYALGO is given the key attribute for KEYREF are
+ * changed to what is described by KEYALGO (e.g. "rsa3072", "rsa2048",
+ * or "ed25519").
+ */
 static gpg_error_t
 change_keyattr_from_string (app_t app,
                             gpg_error_t (*pincb)(void*, const char *, char **),
                             void *pincb_arg,
+                            const char *keyref, const char *keyalgo,
                             const void *value, size_t valuelen)
 {
   gpg_error_t err = 0;
-  char *string;
+  char *string = NULL;
   int key, keyno, algo;
-  int n = 0;
+  unsigned int nbits = 0;
+  const char *oidstr = NULL; /* OID of the curve.  */
+  char *endp;
+
+  if (keyref && keyalgo && *keyref && *keyalgo)
+    {
+      if (!ascii_strcasecmp (keyref, "OPENPGP.1"))
+        keyno = 0;
+      else if (!ascii_strcasecmp (keyref, "OPENPGP.2"))
+        keyno = 1;
+      else if (!ascii_strcasecmp (keyref, "OPENPGP.3"))
+        keyno = 2;
+      else
+        {
+          err = gpg_error (GPG_ERR_INV_ID);
+          goto leave;
+        }
 
-  /* VALUE is expected to be a string but not guaranteed to be
-     terminated.  Thus copy it to an allocated buffer first. */
-  string = xtrymalloc (valuelen+1);
-  if (!string)
-    return gpg_error_from_syserror ();
-  memcpy (string, value, valuelen);
-  string[valuelen] = 0;
+      if (!strncmp (keyalgo, "rsa", 3) && digitp (keyalgo+3))
+        {
+          errno = 0;
+          nbits = strtoul (keyalgo+3, &endp, 10);
+          if (errno || *endp)
+            {
+              err = gpg_error (GPG_ERR_INV_DATA);
+              goto leave;
+            }
+          algo = PUBKEY_ALGO_RSA;
+        }
+      else if ((!strncmp (keyalgo, "dsa", 3) || !strncmp (keyalgo, "elg", 3))
+               && digitp (keyalgo+3))
+        {
+          err = gpg_error (GPG_ERR_PUBKEY_ALGO);
+          goto leave;
+        }
+      else
+        {
+          nbits = 0;
+          oidstr = openpgp_curve_to_oid (keyalgo, NULL, &algo);
+          if (!oidstr)
+            {
+              err = gpg_error (GPG_ERR_INV_DATA);
+              goto leave;
+            }
+          if (!algo)
+            algo = keyno == 1? PUBKEY_ALGO_ECDH : PUBKEY_ALGO_ECDSA;
+        }
 
-  /* Because this function deletes the key we require the string
-     "--force" in the data to make clear that something serious might
-     happen.  */
-  sscanf (string, "--force %d %d %n", &key, &algo, &n);
-  if (n < 12)
+    }
+  else if (!keyref && !keyalgo && value)
+    {
+      int n;
+
+      /* VALUE is expected to be a string but not guaranteed to be
+       * terminated.  Thus copy it to an allocated buffer first. */
+      string = xtrymalloc (valuelen+1);
+      if (!string)
+        {
+          err = gpg_error_from_syserror ();
+          goto leave;
+        }
+      memcpy (string, value, valuelen);
+      string[valuelen] = 0;
+
+      /* Because this function deletes the key we require the string
+       * "--force" in the data to make clear that something serious
+       * might happen.  */
+      n = 0;
+      sscanf (string, "--force %d %d %n", &key, &algo, &n);
+      if (n < 12)
+        {
+          err = gpg_error (GPG_ERR_INV_DATA);
+          goto leave;
+        }
+      keyno = key - 1;
+      if (algo == PUBKEY_ALGO_RSA)
+        {
+          errno = 0;
+          nbits = strtoul (string+n+3, NULL, 10);
+          if (errno)
+            {
+              err = gpg_error (GPG_ERR_INV_DATA);
+              goto leave;
+            }
+        }
+      else if (algo == PUBKEY_ALGO_ECDH || algo == PUBKEY_ALGO_ECDSA
+               || algo == PUBKEY_ALGO_EDDSA)
+        {
+          oidstr = openpgp_curve_to_oid (string+n, NULL, NULL);
+          if (!oidstr)
+            {
+              err = gpg_error (GPG_ERR_INV_DATA);
+              goto leave;
+            }
+        }
+      else
+        {
+          err = gpg_error (GPG_ERR_PUBKEY_ALGO);
+          goto leave;
+        }
+    }
+  else
     {
-      err = gpg_error (GPG_ERR_INV_DATA);
+      err = gpg_error (GPG_ERR_INV_ARG);
       goto leave;
     }
 
-  keyno = key - 1;
   if (keyno < 0 || keyno > 2)
     err = gpg_error (GPG_ERR_INV_ID);
   else if (algo == PUBKEY_ALGO_RSA)
     {
-      unsigned int nbits;
-
-      errno = 0;
-      nbits = strtoul (string+n+3, NULL, 10);
-      if (errno)
-        err = gpg_error (GPG_ERR_INV_DATA);
-      else if (nbits < 1024)
+      if (nbits < 1024)
         err = gpg_error (GPG_ERR_TOO_SHORT);
       else if (nbits > 4096)
         err = gpg_error (GPG_ERR_TOO_LARGE);
@@ -3505,18 +3596,23 @@ change_keyattr_from_string (app_t app,
   else if (algo == PUBKEY_ALGO_ECDH || algo == PUBKEY_ALGO_ECDSA
            || algo == PUBKEY_ALGO_EDDSA)
     {
-      const char *oidstr;
       gcry_mpi_t oid;
       const unsigned char *oidbuf;
       size_t oid_len;
+      unsigned int n;
+
+      /* Check that the requested algo matches the properties of the
+       * key slot.  */
+      if (keyno == 1 && algo != PUBKEY_ALGO_ECDH)
+        err = gpg_error (GPG_ERR_WRONG_PUBKEY_ALGO);
+      else if (keyno != 1 && algo == PUBKEY_ALGO_ECDH)
+        err = gpg_error (GPG_ERR_WRONG_PUBKEY_ALGO);
+      else
+        err = 0;
+      if (err)
+        goto leave;
 
-      oidstr = openpgp_curve_to_oid (string+n, NULL, NULL);
-      if (!oidstr)
-        {
-          err = gpg_error (GPG_ERR_INV_DATA);
-          goto leave;
-        }
-
+      /* Convert the OID string to an OpenPGP formatted OID.  */
       err = openpgp_oid_from_str (oidstr, &oid);
       if (err)
         goto leave;
@@ -3524,7 +3620,14 @@ change_keyattr_from_string (app_t app,
       oidbuf = gcry_mpi_get_opaque (oid, &n);
       oid_len = (n+7)/8;
 
-      /* We have enough room at STRING.  */
+      /* Create the template.  */
+      xfree (string);
+      string = xtrymalloc (1 + oid_len);
+      if (!string)
+        {
+          err = gpg_error_from_syserror ();
+          goto leave;
+        }
       string[0] = algo;
       memcpy (string+1, oidbuf+1, oid_len-1);
       err = change_keyattr (app, keyno, string, oid_len, pincb, pincb_arg);
@@ -4204,26 +4307,31 @@ do_writekey (app_t app, ctrl_t ctrl,
 
 /* Handle the GENKEY command. */
 static gpg_error_t
-do_genkey (app_t app, ctrl_t ctrl,  const char *keynostr, const char *keytype,
+do_genkey (app_t app, ctrl_t ctrl,  const char *keyref, const char *keyalgo,
            unsigned int flags, time_t createtime,
            gpg_error_t (*pincb)(void*, const char *, char **),
            void *pincb_arg)
 {
   gpg_error_t err;
   char numbuf[30];
+  const char *keynostr;
   unsigned char *buffer = NULL;
   const unsigned char *keydata;
   size_t buflen, keydatalen;
   u32 created_at;
-  int keyno = atoi (keynostr) - 1;
-  int force = (flags & 1);
+  int keyno;
+  int force = !!(flags & APP_GENKEY_FLAG_FORCE);
   time_t start_at;
   int exmode = 0;
   int le_value = 256; /* Use legacy value. */
 
-  (void)keytype;  /* Ignored for OpenPGP cards.  */
+  /* Strip the OpenPGP prefix which is for historical reasons optional.  */
+  keynostr = keyref;
+  if (!ascii_strncasecmp (keynostr, "OPENPGP.", 8))
+    keynostr += 8;
 
-  if (keyno < 0 || keyno > 2)
+  keyno = atoi (keynostr) - 1;
+  if (!digitp (keynostr) || keyno < 0 || keyno > 2)
     return gpg_error (GPG_ERR_INV_ID);
 
   /* We flush the cache to increase the traffic before a key
@@ -4241,6 +4349,19 @@ do_genkey (app_t app, ctrl_t ctrl,  const char *keynostr, const char *keytype,
   if (err)
     return err;
 
+  if (keyalgo && app->app_local->keyattr[keyno].keyalgo
+      && strcmp (keyalgo, app->app_local->keyattr[keyno].keyalgo))
+    {
+      /* Specific algorithm requested which is not the currently
+       * configured algorithm.  Change it.  */
+      log_info ("openpgp: changing key attribute from %s to %s\n",
+                 app->app_local->keyattr[keyno].keyalgo, keyalgo);
+      err = change_keyattr_from_string (app, pincb, pincb_arg,
+                                        keyref, keyalgo, NULL, 0);
+      if (err)
+        return err;
+    }
+
   if (app->app_local->keyattr[keyno].key_type == KEY_TYPE_RSA)
     {
       unsigned int keybits = app->app_local->keyattr[keyno].rsa.n_bits;
@@ -5164,7 +5285,7 @@ parse_historical (struct app_local_s *apploc,
 
 /*
  * Check if the OID in an DER encoding is available by GnuPG/libgcrypt,
- * and return the curve name.  Return NULL if not available.
+ * and return the canonical curve name.  Return NULL if not available.
  * The constant string is not allocated dynamically, never free it.
  */
 static const char *
@@ -5208,9 +5329,12 @@ parse_algorithm_attribute (app_t app, int keyno)
   size_t buflen;
   void *relptr;
   const char desc[3][5] = {"sign", "encr", "auth"};
+  enum gcry_pk_algos galgo;
+  unsigned int nbits;
+  const char *curve;
   gpg_error_t err = 0;
 
-  assert (keyno >=0 && keyno <= 2);
+  log_assert (keyno >=0 && keyno <= 2);
 
   app->app_local->keyattr[keyno].key_type = KEY_TYPE_RSA;
   app->app_local->keyattr[keyno].rsa.n_bits = 0;
@@ -5230,6 +5354,11 @@ parse_algorithm_attribute (app_t app, int keyno)
 
   if (opt.verbose)
     log_info ("Key-Attr-%s ..: ", desc[keyno]);
+
+  galgo = map_openpgp_pk_to_gcry (*buffer);
+  nbits = 0;
+  curve = NULL;
+
   if (*buffer == PUBKEY_ALGO_RSA && (buflen == 5 || buflen == 6))
     {
       app->app_local->keyattr[keyno].rsa.n_bits = (buffer[1]<<8 | buffer[2]);
@@ -5244,6 +5373,7 @@ parse_algorithm_attribute (app_t app, int keyno)
                                                      buffer[5] == 3? RSA_CRT_N :
                                                      RSA_UNKNOWN_FMT);
 
+      nbits = app->app_local->keyattr[keyno].rsa.n_bits;
       if (opt.verbose)
         log_printf
           ("RSA, n=%u, e=%u, fmt=%s\n",
@@ -5257,7 +5387,6 @@ parse_algorithm_attribute (app_t app, int keyno)
   else if (*buffer == PUBKEY_ALGO_ECDH || *buffer == PUBKEY_ALGO_ECDSA
            || *buffer == PUBKEY_ALGO_EDDSA)
     {
-      const char *curve;
       int oidlen = buflen - 1;
 
       app->app_local->keyattr[keyno].ecc.flags = 0;
@@ -5309,6 +5438,13 @@ parse_algorithm_attribute (app_t app, int keyno)
   else if (opt.verbose)
     log_printhex (buffer, buflen, "");
 
+  app->app_local->keyattr[keyno].keyalgo
+    = get_keyalgo_string (galgo, nbits, curve);
+
+  if (opt.verbose)
+    log_info ("Key-Algo-%s ..: %s\n",
+              desc[keyno], app->app_local->keyattr[keyno].keyalgo);
+
   xfree (relptr);
   return err;
 }
diff --git a/scd/command.c b/scd/command.c
index ec6989a4d..e3a27ae28 100644
--- a/scd/command.c
+++ b/scd/command.c
@@ -1083,10 +1083,10 @@ cmd_writekey (assuan_context_t ctx, char *line)
 
 
 static const char hlp_genkey[] =
-  "GENKEY [--force] [--timestamp=<isodate>] <no>\n"
+  "GENKEY [--force] [--timestamp=<isodate>] [--algo=ALGO] <keyref>\n"
   "\n"
-  "Generate a key on-card identified by NO, which is application\n"
-  "specific.  Return values are application specific.  For OpenPGP\n"
+  "Generate a key on-card identified by <keyref>, which is application\n"
+  "specific.  Return values are also application specific.  For OpenPGP\n"
   "cards 3 status lines are returned:\n"
   "\n"
   "  S KEY-FPR  <hexstring>\n"
@@ -1105,16 +1105,21 @@ static const char hlp_genkey[] =
   "value.  The value needs to be in ISO Format; e.g.\n"
   "\"--timestamp=20030316T120000\" and after 1970-01-01 00:00:00.\n"
   "\n"
+  "The option --algo can be used to request creation using a specific\n"
+  "algorithm.  The possible algorithms are card dependent.\n"
+  "\n"
   "The public part of the key can also later be retrieved using the\n"
   "READKEY command.";
 static gpg_error_t
 cmd_genkey (assuan_context_t ctx, char *line)
 {
   ctrl_t ctrl = assuan_get_pointer (ctx);
-  int rc;
-  char *save_line;
+  gpg_error_t err;
+  char *keyref_buffer = NULL;
+  char *keyref;
   int force;
   const char *s;
+  char *opt_algo = NULL;
   time_t timestamp;
 
   force = has_option (line, "--force");
@@ -1130,39 +1135,41 @@ cmd_genkey (assuan_context_t ctx, char *line)
   else
     timestamp = 0;
 
+  err = get_option_value (line, "--algo", &opt_algo);
+  if (err)
+    goto leave;
 
   line = skip_options (line);
   if (!*line)
-    {
-      rc = set_error (GPG_ERR_ASS_PARAMETER, "no key number given");
-      goto leave;
-    }
-  save_line = line;
+    return set_error (GPG_ERR_ASS_PARAMETER, "no key number given");
+  keyref = line;
   while (*line && !spacep (line))
     line++;
   *line = 0;
 
-  if ((rc = open_card (ctrl)))
+  if ((err = open_card (ctrl)))
     goto leave;
 
   if (!ctrl->app_ctx)
     {
-      rc = gpg_error (GPG_ERR_UNSUPPORTED_OPERATION);
+      err = gpg_error (GPG_ERR_UNSUPPORTED_OPERATION);
       goto leave;
     }
 
-  {
-    char *tmp = xtrystrdup (save_line);
-    if (!tmp)
-      return gpg_error_from_syserror ();
-    rc = app_genkey (ctrl->app_ctx, ctrl, tmp, NULL,
-                     force? APP_GENKEY_FLAG_FORCE : 0,
-                     timestamp, pin_cb, ctx);
-    xfree (tmp);
-  }
+  keyref = keyref_buffer = xtrystrdup (keyref);
+  if (!keyref)
+    {
+      err = gpg_error_from_syserror ();
+      goto leave;
+    }
+  err = app_genkey (ctrl->app_ctx, ctrl, keyref, opt_algo,
+                    force? APP_GENKEY_FLAG_FORCE : 0,
+                    timestamp, pin_cb, ctx);
 
  leave:
-  return rc;
+  xfree (keyref_buffer);
+  xfree (opt_algo);
+  return err;
 }