agent: Prepend the description to a PIN prompt.

* agent/divert-scd.c (has_percent0A_suffix): New.
(getpin_cb): Prepend DESC_TEXT to the prompt.
* agent/findkey.c (modify_description): Rename to ...
(agent_modify_description): this.  MAke global.  Add kludge to remove
empty parentheses from the end.
(agent_key_from_file, agent_delete_key): Adjust for above change.
* agent/pksign.c (agent_pksign_do): Modify DESC_TEXT also when
diverting to a card.
--

Now that we have support for multiple tokens, it is important to show
information on which key has been requested.  Without that it may
happen that the PIN for a wrong card is accidentally entered.

The texts are a bit ugly, because they talk about "passphrase" but
later about entering a PIN.

A quick hack would be to s/passphrase/PIN/ in the description but that
is complicated due to i18n.  Another solution might be never to talk
about PINs in the description but always about "passphrase: and only
use "PIN" or "passphrase" on the left of the entry field.

4 files changed, 96 insertions, 19 deletions

diff --git a/agent/agent.h b/agent/agent.h
index 22a4d431b..e98a24699 100644
--- a/agent/agent.h
+++ b/agent/agent.h
@@ -381,6 +381,8 @@ gpg_error_t ssh_search_control_file (ssh_control_file_t cf,
 void start_command_handler_ssh (ctrl_t, gnupg_fd_t);
 
 /*-- findkey.c --*/
+gpg_error_t agent_modify_description (const char *in, const char *comment,
+                                      const gcry_sexp_t key, char **result);
 int agent_write_private_key (const unsigned char *grip,
                              const void *buffer, size_t length, int force);
 gpg_error_t agent_key_from_file (ctrl_t ctrl,
diff --git a/agent/divert-scd.c b/agent/divert-scd.c
index 5ffb7ea54..316440495 100644
--- a/agent/divert-scd.c
+++ b/agent/divert-scd.c
@@ -157,6 +157,18 @@ encode_md_for_card (const unsigned char *digest, size_t digestlen, int algo,
 }
 
 
+/* Return true if STRING ends in "%0A". */
+static int
+has_percent0A_suffix (const char *string)
+{
+  size_t n;
+
+  return (string
+          && (n = strlen (string)) >= 3
+          && !strcmp (string + n - 3, "%0A"));
+}
+
+
 /* Callback used to ask for the PIN which should be set into BUF.  The
    buf has been allocated by the caller and is of size MAXBUF which
    includes the terminating null.  The function should return an UTF-8
@@ -246,7 +258,7 @@ getpin_cb (void *opaque, const char *desc_text, const char *info,
         {
           if (info)
             {
-              char *desc;
+              char *desc, *desc2;
 
               if ( asprintf (&desc,
                              L_("%s%%0A%%0AUse the reader's pinpad for input."),
@@ -254,12 +266,22 @@ getpin_cb (void *opaque, const char *desc_text, const char *info,
                 rc = gpg_error_from_syserror ();
               else
                 {
-                  rc = agent_popup_message_start (ctrl, desc, NULL);
+                  /* Prepend DESC_TEXT to INFO.  */
+                  if (desc_text)
+                    desc2 = strconcat (desc_text,
+                                       has_percent0A_suffix (desc_text)
+                                       ? "%0A" : "%0A%0A",
+                                       desc, NULL);
+                  else
+                    desc2 = NULL;
+                  rc = agent_popup_message_start (ctrl,
+                                                  desc2? desc2:desc, NULL);
+                  xfree (desc2);
                   xfree (desc);
                 }
             }
           else
-            rc = agent_popup_message_start (ctrl, NULL, NULL);
+            rc = agent_popup_message_start (ctrl, desc_text, NULL);
         }
       else
         rc = gpg_error (GPG_ERR_INV_VALUE);
@@ -280,7 +302,19 @@ getpin_cb (void *opaque, const char *desc_text, const char *info,
 
   if (any_flags)
     {
-      rc = agent_askpin (ctrl, info, prompt, again_text, pi, NULL, 0);
+      {
+        char *desc2;
+
+        if (desc_text)
+          desc2 = strconcat (desc_text,
+                             has_percent0A_suffix (desc_text)
+                             ? "%0A" : "%0A%0A",
+                             info, NULL);
+        else
+          desc2 = NULL;
+        rc = agent_askpin (ctrl, desc2, prompt, again_text, pi, NULL, 0);
+        xfree (desc2);
+      }
       again_text = NULL;
       if (!rc && newpin)
         {
@@ -319,14 +353,24 @@ getpin_cb (void *opaque, const char *desc_text, const char *info,
     }
   else
     {
-      char *desc;
+      char *desc, *desc2;
+
       if ( asprintf (&desc,
                      L_("Please enter the PIN%s%s%s to unlock the card"),
                      info? " (":"",
                      info? info:"",
                      info? ")":"") < 0)
         desc = NULL;
-      rc = agent_askpin (ctrl, desc?desc:info, prompt, NULL, pi, NULL, 0);
+      if (desc_text)
+        desc2 = strconcat (desc_text,
+                           has_percent0A_suffix (desc_text)
+                           ? "%0A" : "%0A%0A",
+                           desc, NULL);
+      else
+        desc2 = NULL;
+      rc = agent_askpin (ctrl, desc2? desc2 : desc? desc : info,
+                         prompt, NULL, pi, NULL, 0);
+      xfree (desc2);
       xfree (desc);
     }
 
diff --git a/agent/findkey.c b/agent/findkey.c
index 698f765cb..ac74fa98f 100644
--- a/agent/findkey.c
+++ b/agent/findkey.c
@@ -321,9 +321,9 @@ try_unprotect_cb (struct pin_entry_info_s *pi)
    The functions returns 0 on success or an error code.  On success a
    newly allocated string is stored at the address of RESULT.
  */
-static gpg_error_t
-modify_description (const char *in, const char *comment, const gcry_sexp_t key,
-                    char **result)
+gpg_error_t
+agent_modify_description (const char *in, const char *comment,
+                          const gcry_sexp_t key, char **result)
 {
   size_t comment_length;
   size_t in_len;
@@ -332,12 +332,19 @@ modify_description (const char *in, const char *comment, const gcry_sexp_t key,
   size_t i;
   int special, pass;
   char *ssh_fpr = NULL;
+  char *p;
+
+  *result = NULL;
+
+  if (!comment)
+    comment = "";
 
   comment_length = strlen (comment);
   in_len  = strlen (in);
 
   /* First pass calculates the length, second pass does the actual
      copying.  */
+  /* FIXME: This can be simplified by using es_fopenmem.  */
   out = NULL;
   out_len = 0;
   for (pass=0; pass < 2; pass++)
@@ -427,8 +434,23 @@ modify_description (const char *in, const char *comment, const gcry_sexp_t key,
     }
 
   *out = 0;
-  assert (*result + out_len == out);
+  log_assert (*result + out_len == out);
   xfree (ssh_fpr);
+
+  /* The ssh prompt may sometimes end in
+   *    "...%0A  ()"
+   * The empty parentheses doesn't look very good.  We use this hack
+   * here to remove them as well as the indentation spaces. */
+  p = *result;
+  i = strlen (p);
+  if (i > 2 && !strcmp (p + i - 2, "()"))
+    {
+      p += i - 2;
+      *p-- = 0;
+      while (p > *result && spacep (p))
+        *p-- = 0;
+    }
+
   return 0;
 }
 
@@ -874,8 +896,8 @@ agent_key_from_file (ctrl_t ctrl, const char *cache_nonce,
 
         desc_text_final = NULL;
 	if (desc_text)
-          rc = modify_description (desc_text, comment? comment:"", s_skey,
-                                   &desc_text_final);
+          rc = agent_modify_description (desc_text, comment, s_skey,
+                                         &desc_text_final);
         gcry_free (comment);
 
 	if (!rc)
@@ -1453,8 +1475,8 @@ agent_delete_key (ctrl_t ctrl, const char *desc_text,
           }
 
           if (desc_text)
-            err = modify_description (desc_text, comment? comment:"", s_skey,
-                                      &desc_text_final);
+            err = agent_modify_description (desc_text, comment, s_skey,
+                                            &desc_text_final);
           if (err)
             goto leave;
 
diff --git a/agent/pksign.c b/agent/pksign.c
index 4a5daedb0..3b2fcc4f5 100644
--- a/agent/pksign.c
+++ b/agent/pksign.c
@@ -285,7 +285,8 @@ agent_pksign_do (ctrl_t ctrl, const char *cache_nonce,
                  cache_mode_t cache_mode, lookup_ttl_t lookup_ttl,
                  const void *overridedata, size_t overridedatalen)
 {
-  gcry_sexp_t s_skey = NULL, s_sig = NULL;
+  gcry_sexp_t s_skey = NULL;
+  gcry_sexp_t s_sig  = NULL;
   gcry_sexp_t s_hash = NULL;
   gcry_sexp_t s_pkey = NULL;
   unsigned char *shadow_info = NULL;
@@ -346,10 +347,18 @@ agent_pksign_do (ctrl_t ctrl, const char *cache_nonce,
             is_ECDSA = 1;
         }
 
-      rc = divert_pksign (ctrl, desc_text,
-                          data, datalen,
-                          ctrl->digest.algo,
-                          shadow_info, &buf, &len);
+      {
+        char *desc2 = NULL;
+
+        if (desc_text)
+          agent_modify_description (desc_text, NULL, s_skey, &desc2);
+
+        rc = divert_pksign (ctrl, desc2? desc2 : desc_text,
+                            data, datalen,
+                            ctrl->digest.algo,
+                            shadow_info, &buf, &len);
+        xfree (desc2);
+      }
       if (rc)
         {
           log_error ("smartcard signing failed: %s\n", gpg_strerror (rc));