pw err counter fix

ChangeLog

@@ -1,3 +1,8 @@
+2011-05-12  NIIBE Yutaka  <gniibe@fsij.org>
+
+	* src/ac.c (verify_admin_0): Use PW_ERR_PW1 counter when
+	authenticated by PW1.
+
 2011-05-11  NIIBE Yutaka  <gniibe@fsij.org>
 
 	* src/ac.c (verify_pso_cds, verify_other): Fail (with no counter

NEWS

@@ -5,8 +5,8 @@ Gnuk NEWS - User visible changes
   Released 2011-05-1X, by NIIBE Yutaka
 
 ** Admin-less mode is supported.
-The OpenPGP card specification assumes existence of a security
+The OpenPGP card specification assumes existence of a security officer
-officer, who has privilege to manage the card.  On the other hand,
+(admin), who has privilege to manage the card.  On the other hand,
 many use cases of Gnuk are admin == user.
 
 Thus, Gnuk now supports "admin-less" mode.  In this mode, user can get
@@ -17,12 +17,11 @@ setting PW3.  Without setting PW3, it becomes "admin-less" mode
 by setting PW1.
 
 ** Important bug fix.
-Gnuk (<= 0.11) has a severe bug which makes possible for attacker to
+Gnuk (<= 0.11) has a bug which makes possible for attacker to guess
-guess admin password easily.  When admin password is not set (the
+admin password easily.  When admin password is not set (the default
-default value of factory setting), failure of VERIFY doesn't increment
+value of factory setting), failure of VERIFY doesn't increment error
-error counter in older versions.  Observing no increment of error
+counter in older versions.  Observing no increment of error counter,
-counter, attacker could know that admin password is the one of factory
+attacker could know that admin password is the one of factory setting.
-setting.
 
 ** tool/gnuk_put_binary.py now uses pyscard.
 Instead of PyUSB, it uses Python binding of PC/SC.  PyUSB version is

src/ac.c

@@ -161,9 +161,6 @@ verify_admin_0 (const uint8_t *pw, int buf_len, int pw_len_known)
   const uint8_t *pw3_keystring;
   int pw_len;
 
-  if (gpg_pw_locked (PW_ERR_PW3))
-    return 0;
-
   pw3_keystring = gpg_do_read_simple (NR_DO_KEYSTRING_PW3);
   if (pw3_keystring != NULL)
     {
@@ -171,6 +168,9 @@ verify_admin_0 (const uint8_t *pw, int buf_len, int pw_len_known)
       uint8_t md[KEYSTRING_MD_SIZE];
       const uint8_t *salt;
 
+      if (gpg_pw_locked (PW_ERR_PW3))
+	return 0;
+
       pw_len = pw3_keystring[0];
       if ((pw_len_known >= 0 && pw_len_known != pw_len) || pw_len < buf_len)
 	goto failure;
@@ -187,8 +187,7 @@ verify_admin_0 (const uint8_t *pw, int buf_len, int pw_len_known)
 	}
 
       admin_authorized = BY_ADMIN;
-    success:
+    success:		       /* OK, the user is now authenticated */
-      /* OK, the user is now authenticated */
       gpg_pw_reset_err_counter (PW_ERR_PW3);
       return pw_len;
     }
@@ -201,23 +200,36 @@ verify_admin_0 (const uint8_t *pw, int buf_len, int pw_len_known)
 	  int r;
 	  uint8_t keystring[KEYSTRING_MD_SIZE];
 
+	  if (gpg_pw_locked (PW_ERR_PW1))
+	    return 0;
+
 	  pw_len = ks_pw1[0];
 	  if ((pw_len_known >= 0 && pw_len_known != pw_len)
 	      || buf_len < pw_len)
-	    goto failure;
+	    {
+	    failure_pw1:
+	      gpg_pw_increment_err_counter (PW_ERR_PW1);
+	      return -1;
+	    }
 
 	  sha1 (pw, pw_len, keystring);
 	  if ((r = gpg_do_load_prvkey (GPG_KEY_FOR_SIGNING, BY_USER, keystring))
 	      < 0)
-	    goto failure;
+	    goto failure_pw1;
-	  else if (r > 0)
+	  else if (r == 0)
 	    {
+	      if (memcmp (ks_pw1+1, keystring, KEYSTRING_MD_SIZE) != 0)
+		goto failure_pw1;
+	    }
+
 	  admin_authorized = BY_USER;
-	      goto success; 
+	  gpg_pw_reset_err_counter (PW_ERR_PW1);
-	    }
+	  return pw_len;
-	  /* if r == 0 (no signing key), then fall through */
 	}
 
+      if (gpg_pw_locked (PW_ERR_PW3))
+	return 0;
+
       /*
        * For the case of empty PW3 (with empty PW1 or no signing key yet),
        * pass phrase should be OPENPGP_CARD_INITIAL_PW3