gen-key with wrong passphrase

NIIBE Yutaka gniibe at fsij.org
Wed Oct 7 09:34:50 CEST 2015


On 10/07/2015 03:53 AM, Werner Koch wrote:
> Right.  I used -1 to make the functions similar to strcmp - but that is
> not really required.
> 
>> I'm not sure how we fix this.  Shall we change the callback
>> pininfo->check_cb return type from int to gpg_error_t?
> 
> That is a good idea.  I am not sure what error to return.  The new
> GPG_ERR_FALSE seems to be best but we may need to also change the
> callers to return a more descriptive error message.

I had thought that it were big change, but it is not, actually.

I'm afraid GPG_ERR_FALSE is too new.  For the pininfo->check_cb case,
I think that we can simply use GPG_ERR_BAD_PASSPHRASE, because the
semantics is clear and this is independent usage.

Here is a fix.  Built successfully, confirmed that the regression
is fixed.

OK to push?

diff --git a/agent/agent.h b/agent/agent.h
index b3e8470..6e24df4 100644
--- a/agent/agent.h
+++ b/agent/agent.h
@@ -257,7 +257,8 @@ struct pin_entry_info_s
   int with_qualitybar; /* Set if the quality bar should be displayed.  */
   int with_repeat;  /* Request repetition of the passphrase.  */
   int repeat_okay;  /* Repetition worked. */
-  int (*check_cb)(struct pin_entry_info_s *); /* CB used to check the PIN */
+  gpg_error_t (*check_cb)(struct pin_entry_info_s *); /* CB used to check
+                                                         the PIN */
   void *check_cb_arg;  /* optional argument which might be of use in the CB */
   const char *cb_errtext; /* used by the cb to display a specific error */
   size_t max_length;   /* Allocated length of the buffer PIN. */
@@ -402,11 +403,11 @@ void initialize_module_call_pinentry (void);
 void agent_query_dump_state (void);
 void agent_reset_query (ctrl_t ctrl);
 int pinentry_active_p (ctrl_t ctrl, int waitseconds);
-int agent_askpin (ctrl_t ctrl,
-                  const char *desc_text, const char *prompt_text,
-                  const char *inital_errtext,
-                  struct pin_entry_info_s *pininfo,
-                  const char *keyinfo, cache_mode_t cache_mode);
+gpg_error_t agent_askpin (ctrl_t ctrl,
+                          const char *desc_text, const char *prompt_text,
+                          const char *inital_errtext,
+                          struct pin_entry_info_s *pininfo,
+                          const char *keyinfo, cache_mode_t cache_mode);
 int agent_get_passphrase (ctrl_t ctrl, char **retpass,
                           const char *desc, const char *prompt,
                           const char *errtext, int with_qualitybar,
diff --git a/agent/call-pinentry.c b/agent/call-pinentry.c
index 0140387..8b22aea 100644
--- a/agent/call-pinentry.c
+++ b/agent/call-pinentry.c
@@ -127,8 +127,8 @@ agent_reset_query (ctrl_t ctrl)
    disconnect that pinentry - we do this after the unlock so that a
    stalled pinentry does not block other threads.  Fixme: We should
    have a timeout in Assuan for the disconnect operation. */
-static int
-unlock_pinentry (int rc)
+static gpg_error_t
+unlock_pinentry (gpg_error_t rc)
 {
   assuan_context_t ctx = entry_ctx;
   int err;
@@ -229,7 +229,7 @@ getinfo_pid_cb (void *opaque, const void *buffer, size_t length)
    that this function must always be used to aquire the lock for the
    pinentry - we will serialize _all_ pinentry calls.
  */
-static int
+static gpg_error_t
 start_pinentry (ctrl_t ctrl)
 {
   int rc = 0;
@@ -709,7 +709,7 @@ inq_quality (void *opaque, const char *line)


 /* Helper for agent_askpin and agent_get_passphrase.  */
-static int
+static gpg_error_t
 setup_qualitybar (ctrl_t ctrl)
 {
   int rc;
@@ -801,14 +801,14 @@ pinentry_status_cb (void *opaque, const char *line)
    number here and repeat it as long as we have invalid formed
    numbers.  KEYINFO and CACHE_MODE are used to tell pinentry something
    about the key. */
-int
+gpg_error_t
 agent_askpin (ctrl_t ctrl,
               const char *desc_text, const char *prompt_text,
               const char *initial_errtext,
               struct pin_entry_info_s *pininfo,
               const char *keyinfo, cache_mode_t cache_mode)
 {
-  int rc;
+  gpg_error_t rc;
   char line[ASSUAN_LINELENGTH];
   struct entry_parm_s parm;
   const char *errtext = NULL;
@@ -829,10 +829,10 @@ agent_askpin (ctrl_t ctrl,
 	  size_t size;

 	  *pininfo->pin = 0; /* Reset the PIN. */
-	  rc = pinentry_loopback(ctrl, "PASSPHRASE", &passphrase, &size,
-		  pininfo->max_length);
-	  if (rc)
-	    return rc;
+          rc = pinentry_loopback (ctrl, "PASSPHRASE", &passphrase, &size,
+                                  pininfo->max_length);
+          if (rc)
+            return rc;

 	  memcpy(&pininfo->pin, passphrase, size);
 	  xfree(passphrase);
@@ -1006,7 +1006,7 @@ agent_askpin (ctrl_t ctrl,
           /* More checks by utilizing the optional callback. */
           pininfo->cb_errtext = NULL;
           rc = pininfo->check_cb (pininfo);
-          if (rc == -1 && pininfo->cb_errtext)
+          if (gpg_err_code (rc) == GPG_ERR_BAD_PASSPHRASE && pininfo->cb_errtext)
             errtext = pininfo->cb_errtext;
           else if (gpg_err_code (rc) == GPG_ERR_BAD_PASSPHRASE
                    || gpg_err_code (rc) == GPG_ERR_BAD_PIN)
diff --git a/agent/command-ssh.c b/agent/command-ssh.c
index 0aa0098..6144ae1 100644
--- a/agent/command-ssh.c
+++ b/agent/command-ssh.c
@@ -3040,14 +3040,14 @@ ssh_key_to_protected_buffer (gcry_sexp_t key, const char *passphrase,

 /* Callback function to compare the first entered PIN with the one
    currently being entered. */
-static int
+static gpg_error_t
 reenter_compare_cb (struct pin_entry_info_s *pi)
 {
   const char *pin1 = pi->check_cb_arg;

   if (!strcmp (pin1, pi->pin))
     return 0; /* okay */
-  return -1;
+  return gpg_error (GPG_ERR_BAD_PASSPHRASE);
 }


@@ -3133,7 +3133,7 @@ ssh_identity_register (ctrl_t ctrl, ssh_key_type_spec_t *spec,
   if (*pi->pin && !pi->repeat_okay)
     {
       err = agent_askpin (ctrl, description2, NULL, NULL, pi2, NULL, 0);
-      if (err == -1)
+      if (gpg_err_code (err) == GPG_ERR_BAD_PASSPHRASE)
 	{ /* The re-entered one did not match and the user did not
 	     hit cancel. */
 	  initial_errtext = L_("does not match - try again");
diff --git a/agent/cvt-openpgp.c b/agent/cvt-openpgp.c
index fb5a473..0b9ecf0 100644
--- a/agent/cvt-openpgp.c
+++ b/agent/cvt-openpgp.c
@@ -657,7 +657,7 @@ do_unprotect (const char *passphrase,

 /* Callback function to try the unprotection from the passphrase query
    code.  */
-static int
+static gpg_error_t
 try_do_unprotect_cb (struct pin_entry_info_s *pi)
 {
   gpg_error_t err;
diff --git a/agent/findkey.c b/agent/findkey.c
index c49c37a..af61e7e 100644
--- a/agent/findkey.c
+++ b/agent/findkey.c
@@ -111,7 +111,7 @@ agent_write_private_key (const unsigned char *grip,

 /* Callback function to try the unprotection from the passphrase query
    code. */
-static int
+static gpg_error_t
 try_unprotect_cb (struct pin_entry_info_s *pi)
 {
   struct try_unprotect_arg_s *arg = pi->check_cb_arg;
diff --git a/agent/genkey.c b/agent/genkey.c
index e8195c2..b780c50 100644
--- a/agent/genkey.c
+++ b/agent/genkey.c
@@ -326,14 +326,14 @@ check_passphrase_constraints (ctrl_t ctrl, const char *pw,

 /* Callback function to compare the first entered PIN with the one
    currently being entered. */
-static int
+static gpg_error_t
 reenter_compare_cb (struct pin_entry_info_s *pi)
 {
   const char *pin1 = pi->check_cb_arg;

   if (!strcmp (pin1, pi->pin))
     return 0; /* okay */
-  return -1;
+  return gpg_error (GPG_ERR_BAD_PASSPHRASE);
 }


@@ -410,7 +410,7 @@ agent_ask_new_passphrase (ctrl_t ctrl, const char *prompt,
       if (*pi->pin && !pi->repeat_okay)
         {
           err = agent_askpin (ctrl, text2, NULL, NULL, pi2, NULL, 0);
-          if (err == -1)
+          if (gpg_err_code (err) == GPG_ERR_BAD_PASSPHRASE)
             { /* The re-entered one did not match and the user did not
                  hit cancel. */
               initial_errtext = xtrystrdup (L_("does not match - try again"));
-- 



More information about the Gnupg-devel mailing list