scd: Factor out pinpad_prompt, clean-up for app-openpgp.

* scd/app-dinsig.c (verify_pin): Use pinpad_prompt for pinpad.
* scd/app-nks.c (verify_pin): Likewise.
* scd/app-openpgp.c (verify_a_chv): Factor in the SYNC_CHV handling
originally in verify_chv2 and do_sign.  Use pinpad_prompt for pinpad.
(verify_chv2): Factor out the SYNC_CHV handling into verify_a_chv.
(verify_chv3, do_change_pin): Use pinpad_prompt for pinpad.
(do_sign): Factor out the SYNC_CHV handling.
* scd/app-sc-hsm.c (verify_pin): Use pinpad_prompt for pinpad.
* scd/command.c (pinpad_prompt): New.
(askpin): Factor out to pinpad_prompt.
(askpin): Factor out pinpad handling into pinpad_prompt.
* scd/scdaemon.h (pinpad_prompt): New.

--

GnuPG-bug-id: 8353
Signed-off-by: NIIBE Yutaka <[email protected]>

6 files changed, 151 insertions, 150 deletions

diff --git a/scd/app-dinsig.c b/scd/app-dinsig.c
index 7a0af8d12..02dd7d6ba 100644
--- a/scd/app-dinsig.c
+++ b/scd/app-dinsig.c
@@ -295,8 +295,8 @@ verify_pin (app_t app, ctrl_t ctrl)
   if (!opt.disable_pinpad
       && !iso7816_check_pinpad (app_get_slot (app), ISO7816_VERIFY, &pininfo) )
     {
-      rc = askpin (ctrl, _("||Please enter your PIN at the reader's pinpad"),
-                   NULL);
+      rc = pinpad_prompt (ctrl,
+                          _("||Please enter your PIN at the reader's pinpad"));
       if (rc)
         {
           log_info (_("PIN callback returned error: %s\n"),
@@ -305,7 +305,7 @@ verify_pin (app_t app, ctrl_t ctrl)
         }
       rc = iso7816_verify_kp (app_get_slot (app), 0x81, &pininfo);
       /* Dismiss the prompt. */
-      askpin (ctrl, NULL, NULL);
+      pinpad_prompt (ctrl, NULL);
     }
   else  /* No Pinpad.  */
     {
diff --git a/scd/app-nks.c b/scd/app-nks.c
index 924272ddb..711b5364e 100644
--- a/scd/app-nks.c
+++ b/scd/app-nks.c
@@ -1654,7 +1654,7 @@ verify_pin (app_t app, ctrl_t ctrl, int pwid, const char *desc)
       && !iso7816_check_pinpad (app_get_slot (app), ISO7816_VERIFY, &pininfo) )
     {
       prompt = make_prompt (app, remaining, desc, extrapromptline);
-      rc = askpin (ctrl, prompt, NULL);
+      rc = pinpad_prompt (ctrl, prompt);
       xfree (prompt);
       if (rc)
         {
@@ -1664,7 +1664,7 @@ verify_pin (app_t app, ctrl_t ctrl, int pwid, const char *desc)
         }
 
       rc = iso7816_verify_kp (app_get_slot (app), pwid, &pininfo);
-      askpin (ctrl, NULL, NULL);  /* Dismiss the prompt. */
+      pinpad_prompt (ctrl, NULL);  /* Dismiss the prompt. */
     }
   else
     {
diff --git a/scd/app-openpgp.c b/scd/app-openpgp.c
index 882108035..6b5008b09 100644
--- a/scd/app-openpgp.c
+++ b/scd/app-openpgp.c
@@ -2763,26 +2763,26 @@ build_enter_pin_prompt (app_t app, int chvno, const char *firstline,
    that PIN shall be used for another verify operation).  The caller
    needs to free this value.  If the function returns with success and
    NULL is stored at PINVALUE, the caller should take this as an
-   indication that the pinpad has been used.
+   indication that the pinpad has been used.  If SYNC_CHV is set, care
+   about other CHV.
    */
 static gpg_error_t
 verify_a_chv (app_t app, ctrl_t ctrl,
               int chvno, unsigned long sigcount,
-              char **r_pinvalue, size_t *r_pinlen)
+              int sync_chv)
 {
   int rc = 0;
   char *prompt;
   pininfo_t pininfo;
   int minlen = 6;
   int remaining;
-  char *pin = NULL;
   const char *firstline = _("||Please unlock the card");
+  char *pinvalue = NULL;
+  size_t pinlen = 0;
+  char *pin = NULL;
 
   log_assert (chvno == 1 || chvno == 2);
 
-  *r_pinvalue = NULL;
-  *r_pinlen = 0;
-
   rc = build_enter_pin_prompt (app,
                                1, /* Use 1, even if CHVNO==2.  */
                                firstline, sigcount, &prompt, &remaining);
@@ -2806,6 +2806,7 @@ verify_a_chv (app_t app, ctrl_t ctrl,
           app->app_local->flags.def_chv2 = 0;
         }
       return rc;
+      /* Note: ignore SYNC_CHV for this case.  */
     }
 
   memset (&pininfo, 0, sizeof pininfo);
@@ -2818,67 +2819,116 @@ verify_a_chv (app_t app, ctrl_t ctrl,
     {
       /* The reader supports the verify command through the pinpad.
        * In this case we do not utilize the PIN cache because by using
-       * a pinpad the PIN can't have been cached.
-       * Note that the askpin appends a text to the prompt telling the
-       * user to use the pinpad. */
-      rc = askpin (ctrl, prompt, NULL);
+       * a pinpad the PIN can't have been cached.  Note that the
+       * pinpad_prompt appends a text to the prompt telling the user
+       * to use the pinpad. */
+      rc = pinpad_prompt (ctrl, prompt);
       xfree (prompt);
       prompt = NULL;
       if (rc)
         {
-          log_info (_("PIN callback returned error: %s\n"),
+          log_info (_("pinpad_prompt returned error: %s\n"),
                     gpg_strerror (rc));
           return rc;
         }
       rc = iso7816_verify_kp (app_get_slot (app), 0x80+chvno, &pininfo);
       /* Dismiss the prompt. */
-      askpin (ctrl, NULL, NULL);
+      pinpad_prompt (ctrl, NULL);
+      return rc;
+      /* Note: SYNC_CHV can't be done because we don't have PINVALUE.  */
     }
+
+  /* The reader has no pinpad or we don't want to use it.  If we
+   * have at least the standard 3 remaining tries we first try to
+   * get the PIN from the cache.  With less remaining tries it is
+   * better to let the user know about failed attempts (which
+   * might be due to a bug in the PIN cache handling). */
+  if (remaining >= 3 && pin_from_cache (app, ctrl, chvno, &pin))
+    rc = 0;
   else
+    rc = askpin (ctrl, prompt, &pin);
+  xfree (prompt);
+  prompt = NULL;
+  if (rc)
     {
-      /* The reader has no pinpad or we don't want to use it.  If we
-       * have at least the standard 3 remaining tries we first try to
-       * get the PIN from the cache.  With less remaining tries it is
-       * better to let the user know about failed attempts (which
-       * might be due to a bug in the PIN cache handling). */
-      if (remaining >= 3 && pin_from_cache (app, ctrl, chvno, &pin))
-        rc = 0;
-      else
-        rc = askpin (ctrl, prompt, &pin);
-      xfree (prompt);
-      prompt = NULL;
-      if (rc)
-        {
-          log_info (_("PIN callback returned error: %s\n"),
-                    gpg_strerror (rc));
-          return rc;
-        }
-
-      if (strlen (pin) < minlen)
-        {
-          log_error (_("PIN for CHV%d is too short;"
-                       " minimum length is %d\n"), chvno, minlen);
-          wipe_and_free_string (pin);
-          return gpg_error (GPG_ERR_BAD_PIN);
-        }
+      log_info (_("askpin returned error: %s\n"), gpg_strerror (rc));
+      return rc;
+    }
 
-      rc = pin2hash_if_kdf (app, chvno, pin, r_pinvalue, r_pinlen);
-      if (!rc)
-        rc = iso7816_verify (app_get_slot (app),
-                             0x80 + chvno, *r_pinvalue, *r_pinlen);
-      if (!rc)
-        cache_pin (app, ctrl, chvno, pin);
+  if (strlen (pin) < minlen)
+    {
+      log_error (_("PIN for CHV%d is too short;"
+                   " minimum length is %d\n"), chvno, minlen);
+      wipe_and_free_string (pin);
+      return gpg_error (GPG_ERR_BAD_PIN);
     }
 
-  wipe_and_free_string (pin);
+  rc = pin2hash_if_kdf (app, chvno, pin, &pinvalue, &pinlen);
+  if (!rc)
+    rc = iso7816_verify (app_get_slot (app),
+                         0x80 + chvno, pinvalue, pinlen);
+  if (!rc)
+    cache_pin (app, ctrl, chvno, pin);
+
   if (rc)
     {
       log_error (_("verify CHV%d failed: %s\n"), chvno, gpg_strerror (rc));
-      xfree (*r_pinvalue);
-      *r_pinvalue = NULL;
-      *r_pinlen = 0;
       flush_cache_after_error (app);
     }
+  else if (sync_chv)
+    {
+      if (chvno == 1)
+        {
+          app->did_chv1 = 1;
+          /* For cards with versions < 2 we want to keep CHV1 and CHV2 in
+             sync, thus we verify CHV2 here using the given PIN.  Cards
+             with version2 to not have the need for a separate CHV2 and
+             internally use just one.  Obviously we can't do that if the
+             pinpad has been used. */
+          if (!app->did_chv2 && pinvalue && !app->app_local->extcap.is_v2)
+            {
+              rc = iso7816_verify (app_get_slot (app), 0x82, pinvalue, pinlen);
+              if (gpg_err_code (rc) == GPG_ERR_BAD_PIN)
+                rc = gpg_error (GPG_ERR_PIN_NOT_SYNCED);
+              if (rc)
+                {
+                  log_error (_("verify CHV%d failed: %s\n"), 2, gpg_strerror (rc));
+                  flush_cache_after_error (app);
+                }
+              else
+                {
+                  app->did_chv2 = 1;
+                  cache_pin (app, ctrl, 2, pin);
+                }
+            }
+        }
+      else if (chvno == 2)
+        {
+          app->did_chv2 = 1;
+          if (!app->did_chv1 && !app->force_chv1 && pinvalue && !opt.pcsc_shared)
+            {
+              /* For convenience we verify CHV1 here too.  We do this only if
+                 the card is not configured to require a verification before
+                 each CHV1 controlled operation (force_chv1) and if we are not
+                 using the pinpad (PINVALUE == NULL). */
+              rc = iso7816_verify (app_get_slot (app), 0x81, pinvalue, pinlen);
+              if (gpg_err_code (rc) == GPG_ERR_BAD_PIN)
+                rc = gpg_error (GPG_ERR_PIN_NOT_SYNCED);
+              if (rc)
+                {
+                  log_error (_("verify CHV%d failed: %s\n"), 1, gpg_strerror (rc));
+                  flush_cache_after_error (app);
+                }
+              else
+                {
+                  app->did_chv1 = 1;
+                  cache_pin (app, ctrl, 1, pin);
+                }
+            }
+        }
+    }
+  wipe_and_free_string (pin);
+  wipe_and_free (pinvalue, pinlen);
 
   return rc;
 }
@@ -2890,51 +2940,23 @@ static gpg_error_t
 verify_chv2 (app_t app, ctrl_t ctrl)
 {
   int rc;
-  char *pinvalue;
-  size_t pinlen;
 
   if (app->did_chv2)
     return 0;  /* We already verified CHV2.  */
 
   if (app->app_local->pk[1].key || app->app_local->pk[2].key)
     {
-      rc = verify_a_chv (app, ctrl, 2, 0, &pinvalue, &pinlen);
+      rc = verify_a_chv (app, ctrl, 2, 0, 1);
       if (rc)
         return rc;
-      app->did_chv2 = 1;
-
-      if (!app->did_chv1 && !app->force_chv1 && pinvalue && !opt.pcsc_shared)
-        {
-          /* For convenience we verify CHV1 here too.  We do this only if
-             the card is not configured to require a verification before
-             each CHV1 controlled operation (force_chv1) and if we are not
-             using the pinpad (PINVALUE == NULL). */
-          rc = iso7816_verify (app_get_slot (app), 0x81, pinvalue, pinlen);
-          if (gpg_err_code (rc) == GPG_ERR_BAD_PIN)
-            rc = gpg_error (GPG_ERR_PIN_NOT_SYNCED);
-          if (rc)
-            {
-              log_error (_("verify CHV%d failed: %s\n"), 1, gpg_strerror (rc));
-              flush_cache_after_error (app);
-            }
-          else
-            {
-              app->did_chv1 = 1;
-              /* Note that we are not able to cache the CHV 1 here because
-               * it is possible that due to the use of a KDF-DO PINVALUE
-               * has the hashed binary PIN of length PINLEN.  */
-            }
-        }
     }
   else
     {
-      rc = verify_a_chv (app, ctrl, 1, 0, &pinvalue, &pinlen);
+      rc = verify_a_chv (app, ctrl, 1, 0, 0);
       if (rc)
         return rc;
     }
 
-  wipe_and_free (pinvalue, pinlen);
-
   return rc;
 }
 
@@ -2988,18 +3010,18 @@ verify_chv3 (app_t app, ctrl_t ctrl)
           && !check_pinpad_request (app, &pininfo, 1))
         {
           /* The reader supports the verify command through the pinpad. */
-          rc = askpin (ctrl, prompt, NULL);
+          rc = pinpad_prompt (ctrl, prompt);
           xfree (prompt);
           prompt = NULL;
           if (rc)
             {
-              log_info (_("PIN callback returned error: %s\n"),
+              log_info (_("pinpad_prompt returned error: %s\n"),
                         gpg_strerror (rc));
               return rc;
             }
           rc = iso7816_verify_kp (app_get_slot (app), 0x83, &pininfo);
           /* Dismiss the prompt. */
-          askpin (ctrl, NULL, NULL);
+          pinpad_prompt (ctrl, NULL);
         }
       else
         {
@@ -3015,7 +3037,7 @@ verify_chv3 (app_t app, ctrl_t ctrl)
           prompt = NULL;
           if (rc)
             {
-              log_info (_("PIN callback returned error: %s\n"),
+              log_info (_("askpin returned error: %s\n"),
                         gpg_strerror (rc));
               return rc;
             }
@@ -3193,7 +3215,7 @@ do_setattr (app_t app, ctrl_t ctrl, const char *name,
               rc = askpin (ctrl, prompt, &oldpinvalue);
               if (rc)
                 {
-                  log_info (_("PIN callback returned error: %s\n"),
+                  log_info (_("askpin returned error: %s\n"),
                             gpg_strerror (rc));
                   return rc;
                 }
@@ -3504,7 +3526,7 @@ do_change_pin (app_t app, ctrl_t ctrl,  const char *chvnostr,
               prompt = NULL;
               if (rc)
                 {
-                  log_info (_("PIN callback returned error: %s\n"),
+                  log_info (_("askpin returned error: %s\n"),
                             gpg_strerror (rc));
                   goto leave;
                 }
@@ -3544,7 +3566,7 @@ do_change_pin (app_t app, ctrl_t ctrl,  const char *chvnostr,
           prompt = NULL;
           if (rc)
             {
-              log_info (_("PIN callback returned error: %s\n"),
+              log_info (_("askpin returned error: %s\n"),
                         gpg_strerror (rc));
               goto leave;
             }
@@ -3719,19 +3741,19 @@ do_change_pin (app_t app, ctrl_t ctrl,  const char *chvnostr,
           if (rc)
             goto leave;
 
-          rc = askpin (ctrl, prompt, NULL);
+          rc = pinpad_prompt (ctrl, prompt);
           xfree (prompt);
           prompt = NULL;
           if (rc)
             {
-              log_info (_("PIN callback returned error: %s\n"),
+              log_info (_("pinpad_prompt returned error: %s\n"),
                         gpg_strerror (rc));
               goto leave;
             }
           rc = iso7816_change_reference_data_kp (app_get_slot (app),
                                                  0x80 + chvno, 0,
                                                  &pininfo);
-          askpin (ctrl, NULL, NULL); /* Dismiss the prompt. */
+          pinpad_prompt (ctrl, NULL); /* Dismiss the prompt. */
         }
       else
         {
@@ -5503,39 +5525,11 @@ do_sign (app_t app, ctrl_t ctrl, const char *keyidstr, int hashalgo,
   /* Check CHV if needed.  */
   if (!app->did_chv1 || app->force_chv1)
     {
-      char *pinvalue;
-      size_t pinlen;
-
-      rc = verify_a_chv (app, ctrl, 1, sigcount, &pinvalue, &pinlen);
+      rc = verify_a_chv (app, ctrl, 1, sigcount, 1);
       if (rc)
         return rc;
-
-      app->did_chv1 = 1;
-
-      /* For cards with versions < 2 we want to keep CHV1 and CHV2 in
-         sync, thus we verify CHV2 here using the given PIN.  Cards
-         with version2 to not have the need for a separate CHV2 and
-         internally use just one.  Obviously we can't do that if the
-         pinpad has been used. */
-      if (!app->did_chv2 && pinvalue && !app->app_local->extcap.is_v2)
-        {
-          rc = iso7816_verify (app_get_slot (app), 0x82, pinvalue, pinlen);
-          if (gpg_err_code (rc) == GPG_ERR_BAD_PIN)
-            rc = gpg_error (GPG_ERR_PIN_NOT_SYNCED);
-          if (rc)
-            {
-              log_error (_("verify CHV%d failed: %s\n"), 2, gpg_strerror (rc));
-              wipe_and_free (pinvalue, pinlen);
-              flush_cache_after_error (app);
-              return rc;
-            }
-          app->did_chv2 = 1;
-          cache_pin (app, ctrl, 2, pinvalue);
-        }
-      wipe_and_free (pinvalue, pinlen);
     }
 
-
   if (app->app_local->cardcap.ext_lc_le
       && app->app_local->keyattr[0].key_type == KEY_TYPE_RSA
       && app->app_local->keyattr[0].rsa.n_bits > RSA_SMALL_SIZE_OP)
diff --git a/scd/app-sc-hsm.c b/scd/app-sc-hsm.c
index 35239b0ac..43cfb028c 100644
--- a/scd/app-sc-hsm.c
+++ b/scd/app-sc-hsm.c
@@ -1725,7 +1725,7 @@ verify_pin (app_t app, ctrl_t ctrl)
   if (!opt.disable_pinpad
       && !iso7816_check_pinpad (app_get_slot (app), ISO7816_VERIFY, &pininfo) )
     {
-      err = askpin (ctrl, prompt, NULL);
+      err = pinpad_prompt (ctrl, prompt);
       if (err)
         {
           log_info ("PIN callback returned error: %s\n", gpg_strerror (err));
@@ -1733,7 +1733,7 @@ verify_pin (app_t app, ctrl_t ctrl)
         }
 
       err = iso7816_verify_kp (app_get_slot (app), 0x81, &pininfo);
-      askpin (ctrl, NULL, NULL);  /* Dismiss the prompt. */
+      pinpad_prompt (ctrl, NULL);  /* Dismiss the prompt. */
     }
   else
     {
diff --git a/scd/command.c b/scd/command.c
index 739db2f7f..1f1fc3953 100644
--- a/scd/command.c
+++ b/scd/command.c
@@ -973,10 +973,8 @@ cmd_setdata (assuan_context_t ctx, char *line)
   return 0;
 }
 
-
-
 gpg_error_t
-askpin (ctrl_t ctrl, const char *info, char **retstr)
+pinpad_prompt (ctrl_t ctrl, const char *info)
 {
   assuan_context_t ctx = ctrl->server_local->assuan_ctx;
   char *command;
@@ -984,32 +982,39 @@ askpin (ctrl_t ctrl, const char *info, char **retstr)
   unsigned char *value;
   size_t valuelen;
 
-  if (!retstr)
+  /* We prompt for pinpad entry.  To make sure that the popup has
+     been show we use an inquire and not just a status message.
+     We ignore any value returned.  */
+  if (info)
     {
-      /* We prompt for pinpad entry.  To make sure that the popup has
-         been show we use an inquire and not just a status message.
-         We ignore any value returned.  */
-      if (info)
-        {
-          if (DBG_IPC)
-            log_debug ("prompting for pinpad entry '%s'\n", info);
-          rc = gpgrt_asprintf (&command, "POPUPPINPADPROMPT %s", info);
-          if (rc < 0)
-            return gpg_error (gpg_err_code_from_errno (errno));
-          rc = assuan_inquire (ctx, command, &value, &valuelen, MAXLEN_PIN);
-          xfree (command);
-        }
-      else
-        {
-          if (DBG_IPC)
-            log_debug ("dismiss pinpad entry prompt\n");
-          rc = assuan_inquire (ctx, "DISMISSPINPADPROMPT",
-                               &value, &valuelen, MAXLEN_PIN);
-        }
-      if (!rc)
-        xfree (value);
-      return rc;
+      if (DBG_IPC)
+        log_debug ("prompting for pinpad entry '%s'\n", info);
+      rc = gpgrt_asprintf (&command, "POPUPPINPADPROMPT %s", info);
+      if (rc < 0)
+        return gpg_error (gpg_err_code_from_errno (errno));
+      rc = assuan_inquire (ctx, command, &value, &valuelen, MAXLEN_PIN);
+      xfree (command);
+    }
+  else
+    {
+      if (DBG_IPC)
+        log_debug ("dismiss pinpad entry prompt\n");
+      rc = assuan_inquire (ctx, "DISMISSPINPADPROMPT",
+                           &value, &valuelen, MAXLEN_PIN);
     }
+  if (!rc)
+    xfree (value);
+  return rc;
+}
+
+gpg_error_t
+askpin (ctrl_t ctrl, const char *info, char **retstr)
+{
+  assuan_context_t ctx = ctrl->server_local->assuan_ctx;
+  char *command;
+  int rc;
+  unsigned char *value;
+  size_t valuelen;
 
   *retstr = NULL;
   if (DBG_IPC)
diff --git a/scd/scdaemon.h b/scd/scdaemon.h
index f27f5fc61..2b3c4d2c3 100644
--- a/scd/scdaemon.h
+++ b/scd/scdaemon.h
@@ -165,6 +165,8 @@ void popup_prompt (void *opaque, int on);
 
 gpg_error_t askpin (ctrl_t ctrl, const char *info, char **retstr);
 
+gpg_error_t pinpad_prompt (ctrl_t ctrl, const char *info);
+
 /* Take care: this function assumes that CARD is locked.  */
 void send_client_notifications (card_t card, int removal);