buf fix of password change
authorNIIBE Yutaka <gniibe@fsij.org>
Thu, 12 May 2011 02:04:14 +0000 (11:04 +0900)
committerNIIBE Yutaka <gniibe@fsij.org>
Thu, 12 May 2011 02:04:14 +0000 (11:04 +0900)
ChangeLog
NEWS
README
src/ac.c
src/openpgp-do.c
src/openpgp.c

index 9c50ef6..c703c1c 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,9 @@
 2011-05-12  NIIBE Yutaka  <gniibe@fsij.org>
 
+       * src/ac.c (verify_user_0): New.
+       (verify_pso_cds, verify_admin_0): Use verify_user_0.
+       * src/openpgp.c (cmd_change_password): Use verify_user_0.
+
        * src/random.c (get_salt): Rename from get_random.
        (random_bytes_get, random_bytes_free): It's 16-byte.
 
diff --git a/NEWS b/NEWS
index a074857..86d9aad 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -16,13 +16,16 @@ At the initialization of the card, Gnuk becomes compatible mode by
 setting PW3.  Without setting PW3, it becomes "admin-less" mode
 by setting PW1.
 
-** Important bug fix.
-Gnuk (<= 0.11) has a bug which makes possible for attacker to guess
+** Important two bug fixes.
+Gnuk (<= 0.11) had a bug which makes possible for attacker to guess
 admin password easily.  When admin password is not set (the default
 value of factory setting), failure of VERIFY doesn't increment error
 counter in older versions.  Observing no increment of error counter,
 attacker could know that admin password is the one of factory setting.
 
+Gnuk (<= 0.11) had a bug which makes possible for attacker to change
+user password without knowing original password.
+
 ** tool/gnuk_put_binary.py now uses pyscard.
 Instead of PyUSB, it uses Python binding of PC/SC.  PyUSB version is
 still available as tool/gnuk_put_binary_libusb.py.
diff --git a/README b/README
index 25da9da..b32efc0 100644 (file)
--- a/README
+++ b/README
@@ -352,7 +352,7 @@ Connect your board to USB port of your PC.  And invoke gnuk_put_binary.py:
 
 If you use fixed serial number in the file 'GNUK_SERIAL_NUMBER', you can do:
 
-  $ EMAIL=<YOUR-EMAIL-ADDRESS> ../tool/gnuk_put_binary.py -s ../GNUK_SERIAL_NUMBER 
+  $ EMAIL=<YOUR-EMAIL-ADDRESS> ../tool/gnuk_put_binary.py -s ../GNUK_SERIAL_NUMBER
   Writing serial number
   ...
 
index dfb8bef..f6300d5 100644 (file)
--- a/src/ac.c
+++ b/src/ac.c
@@ -56,34 +56,67 @@ ac_reset_other (void)
   auth_status &= ~AC_OTHER_AUTHORIZED;
 }
 
-/*
- * Verify for "Perform Security Operation : Compute Digital Signature"
- */
 int
-verify_pso_cds (const uint8_t *pw, int pw_len)
+verify_user_0 (const uint8_t *pw, int buf_len, int pw_len_known,
+              const uint8_t *ks_pw1)
 {
+  int pw_len;
   int r;
   uint8_t keystring[KEYSTRING_MD_SIZE];
 
   if (gpg_pw_locked (PW_ERR_PW1))
     return 0;
 
-  DEBUG_INFO ("verify_pso_cds\r\n");
-  DEBUG_BYTE (pw_len);
+  if (ks_pw1 == NULL)
+    {
+      pw_len = strlen (OPENPGP_CARD_INITIAL_PW1);
+      if ((pw_len_known >= 0 && pw_len_known != pw_len)
+         || buf_len < pw_len
+         || strncmp ((const char *)pw, OPENPGP_CARD_INITIAL_PW1, pw_len))
+       goto failure;
+      else
+       goto success;
+    }
+  else
+    pw_len = ks_pw1[0];
 
-  sha1 (pw, pw_len, keystring);
-  if ((r = gpg_do_load_prvkey (GPG_KEY_FOR_SIGNING, BY_USER, keystring)) < 0)
+  if ((pw_len_known >= 0 && pw_len_known != pw_len)
+      || buf_len < pw_len)
     {
+    failure:
       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;
   else if (r == 0)
-    /* No key is available.  Fail even if password can match.  */
-    return -1;
+    if (memcmp (ks_pw1+1, keystring, KEYSTRING_MD_SIZE) != 0)
+      goto failure;
 
+ success:
   gpg_pw_reset_err_counter (PW_ERR_PW1);
-  auth_status |= AC_PSO_CDS_AUTHORIZED;
-  return 1;
+  return pw_len;
+}
+
+/*
+ * Verify for "Perform Security Operation : Compute Digital Signature"
+ */
+int
+verify_pso_cds (const uint8_t *pw, int pw_len)
+{
+  const uint8_t *ks_pw1 = gpg_do_read_simple (NR_DO_KEYSTRING_PW1);
+  int r;
+
+  DEBUG_INFO ("verify_pso_cds\r\n");
+  DEBUG_BYTE (pw_len);
+
+  r = verify_user_0 (pw, pw_len, pw_len, ks_pw1);
+  if (r > 0)
+    auth_status |= AC_PSO_CDS_AUTHORIZED;
+  return r;
 }
 
 int
@@ -196,43 +229,21 @@ verify_admin_0 (const uint8_t *pw, int buf_len, int pw_len_known)
       const uint8_t *ks_pw1 = gpg_do_read_simple (NR_DO_KEYSTRING_PW1);
 
       if (ks_pw1 != NULL)
-       {                       /* empty PW3, but PW1 exists */
-         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)
-           {
-           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_pw1;
-         else if (r == 0)
-           {
-             if (memcmp (ks_pw1+1, keystring, KEYSTRING_MD_SIZE) != 0)
-               goto failure_pw1;
-           }
-
-         admin_authorized = BY_USER;
-         gpg_pw_reset_err_counter (PW_ERR_PW1);
-         return pw_len;
+       {         /* empty PW3, but PW1 exists */
+         int r = verify_user_0 (pw, buf_len, pw_len_known, ks_pw1);
+
+         if (r > 0)
+           admin_authorized = BY_USER;
+
+         return r;
        }
 
       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
+       * For the case of empty PW3 (with empty PW1), pass phrase
+       * should be OPENPGP_CARD_INITIAL_PW3
        */
       pw_len = strlen (OPENPGP_CARD_INITIAL_PW3);
       if ((pw_len_known >=0 && pw_len_known != pw_len)
@@ -252,9 +263,9 @@ gpg_set_pw3 (const uint8_t *newpw, int newpw_len)
   uint32_t random;
 
   ks[0] = newpw_len;
-  random = get_random ();
+  random = get_salt ();
   memcpy (&ks[1], &random, sizeof (random));
-  random = get_random ();
+  random = get_salt ();
   memcpy (&ks[5], &random, sizeof (random));
   ks[9] = 0x60;                        /* 65536 iterations */
 
index 58dc75c..aa36a7b 100644 (file)
@@ -716,7 +716,7 @@ gpg_do_write_prvkey (enum kind_of_key kk, const uint8_t *key_data, int key_len,
 
   memcpy (kdi.data, key_data, KEY_CONTENT_LEN);
   kdi.check = calc_check32 (key_data, KEY_CONTENT_LEN);
-  kdi.random = get_random ();
+  kdi.random = get_salt ();
   memcpy (kdi.magic, GNUK_MAGIC, KEY_MAGIC_LEN);
 
   dek = random_bytes_get (); /* 16-byte random bytes */
index f9ad601..7c5e58a 100644 (file)
@@ -302,24 +302,24 @@ cmd_change_password (void)
 
   if (who == BY_USER)                  /* PW1 */
     {
-      const uint8_t *pk = gpg_do_read_simple (NR_DO_KEYSTRING_PW1);
+      const uint8_t *ks_pw1 = gpg_do_read_simple (NR_DO_KEYSTRING_PW1);
 
-      if (pk == NULL)
-       {
-         if (len < (int)strlen (OPENPGP_CARD_INITIAL_PW1))
-           {
-             DEBUG_INFO ("permission denied.\r\n");
-             GPG_SECURITY_FAILURE ();
-             return;
-           }
+      pw_len = verify_user_0 (pw, len, -1, ks_pw1);
 
-         pw_len = strlen (OPENPGP_CARD_INITIAL_PW1);
-         newpw = pw + pw_len;
-         newpw_len = len - pw_len;
+      if (pw_len < 0)
+       {
+         DEBUG_INFO ("permission denied.\r\n");
+         GPG_SECURITY_FAILURE ();
+         return;
+       }
+      else if (pw_len == 0)
+       {
+         DEBUG_INFO ("blocked.\r\n");
+         GPG_SECURITY_AUTH_BLOCKED ();
+         return;
        }
       else
        {
-         pw_len = pk[0];
          newpw = pw + pw_len;
          newpw_len = len - pw_len;
        }
@@ -368,7 +368,6 @@ cmd_change_password (void)
       gpg_do_write_simple (NR_DO_KEYSTRING_PW1, new_ks0, KEYSTRING_SIZE_PW1);
       ac_reset_pso_cds ();
       ac_reset_other ();
-      gpg_pw_reset_err_counter (PW_ERR_PW1);
       DEBUG_INFO ("Changed DO_KEYSTRING_PW1.\r\n");
       GPG_SUCCESS ();
     }
@@ -377,7 +376,6 @@ cmd_change_password (void)
       gpg_do_write_simple (NR_DO_KEYSTRING_PW1, new_ks0, 1);
       ac_reset_pso_cds ();
       ac_reset_other ();
-      gpg_pw_reset_err_counter (PW_ERR_PW1);
       DEBUG_INFO ("Changed length of DO_KEYSTRING_PW1.\r\n");
       GPG_SUCCESS ();
     }
@@ -385,7 +383,6 @@ cmd_change_password (void)
     {
       DEBUG_INFO ("done.\r\n");
       ac_reset_admin ();
-      gpg_pw_reset_err_counter (PW_ERR_PW3);
       GPG_SUCCESS ();
     }
 }