new password management

ChangeLog

@@ -1,5 +1,19 @@
 2010-11-30  NIIBE Yutaka  <gniibe@fsij.org>
 
+	New private key management.
+	* src/ac.c (ac_reset_pso_cds, ac_reset_pso_other): Call
+	gpg_do_clear_prvkey.
+	(verify_pso_other): load private keys here.
+	* src/openpgp-do.c (kd): Keydata for Signing, Decryption, and
+	Authentication.
+	(gpg_do_load_prvkey, gpg_do_write_prvkey): Use kd[].
+	(gpg_do_clear_prvkey): New function.
+	* src/openpgp.c (cmd_pso, cmd_internal_authenticate): Use new API
+	of rsa_sign and rsa_decrypt.
+	* src/call-rsa.c (rsa_sign): New argument KD.
+	(rsa_decrypt): Likewise.
+
+	Don't use malloc/free in C library.
 	* src/stdlib.h (malloc, free): Use chHeapAlloc and chHeapFree.
 
 2010-11-26  NIIBE Yutaka  <gniibe@fsij.org>

NEWS

@@ -9,6 +9,8 @@ ST-Link part (with STM32F103C8T6) of STM8S Discovery board is now supported.
 
 ** Digital signing for SHA224/SHA256/SHA384/SHA512 digestInfo is now possible.
 
+** Fixes for password management.
+
 ** More improved USB-CCID/ICCD implementation.
 Gnuk works better with GPG's in-stock protocol stack.  Still, 
 changes are needed for GPG (scd/ccid-driver.c) to support the case

README

@@ -1,7 +1,7 @@
 Gnuk - software for GPG USB Token
 
-							    Version 0.4
+							    Version 0.5
-							     2010-11-09
+							     2010-12-XX
 						           Niibe Yutaka
 				      Free Software Initiative of Japan
 
@@ -26,7 +26,7 @@ USB Token by Gnuk everywhere.
 Release notes
 =============
 
-This is fifth release of Gnuk.  While it works well for specific
+This is sixth release of Gnuk.  While it works well for specific
 usages, it is still experimental.
 
 Tested features are:
@@ -51,12 +51,12 @@ Tested features are:
 
 	* INTERNAL AUTHENTICATE
 
+	* Changing value of password status bytes (0x00C4).
+
 It is known not-working well:
 
 	* Key import multiple times
 
-	* Changing value of password status bytes (0x00C4).
-
 	* For some version of kernel and libccid, --enable-debug can't
           work well.  Please disable DEBUG option if it doesn't work well.
 
@@ -68,10 +68,14 @@ Not (yet) supported feature(s):
 Targets
 =======
 
-We use Olimex STM32-H103 board.  DFU support is added, it's mainly for
+We use Olimex STM32-H103 board.  We also use STM32 part of STM8S
-CQ STARM and STBee Mini but those targets are not tested extensively.
+Discovery Kit.
-That's because we don't have a Free Software tool to write through
+
-DFU.
+With DfuSe support, CQ STARM and STBee Mini are also our targets.  But
+those target with DfuSe are basically not for normal use but for
+experiment, because it would be impossible for DfuSe to disable read
+from flash.  For real use, kill DfuSe and enable read protect using
+JTAG debugger.
 
 I think that it could run on Olimex STM32-P103, or STBee too.
 Besides, we are porting it to STM32 Primer 2.

src/ac.c

@@ -44,15 +44,15 @@ ac_check_status (uint8_t ac_flag)
 void
 ac_reset_pso_cds (void)
 {
+  gpg_do_clear_prvkey (GPG_KEY_FOR_SIGNING);
   auth_status &= ~AC_PSO_CDS_AUTHORIZED;
 }
 
-uint8_t pw1_keystring[KEYSTRING_SIZE_PW1];
-
 void
 ac_reset_pso_other (void)
 {
-  memset (pw1_keystring, 0, KEYSTRING_SIZE_PW1);
+  gpg_do_clear_prvkey (GPG_KEY_FOR_DECRYPTION);
+  gpg_do_clear_prvkey (GPG_KEY_FOR_AUTHENTICATION);
   auth_status &= ~AC_PSO_OTHER_AUTHORIZED;
 }
 
@@ -89,31 +89,35 @@ int
 verify_pso_other (const uint8_t *pw, int pw_len)
 {
   const uint8_t *ks_pw1;
+  uint8_t pw1_keystring[KEYSTRING_SIZE_PW1];
 
   DEBUG_INFO ("verify_pso_other\r\n");
 
   if (gpg_passwd_locked (PW_ERR_PW1))
     return 0;
 
-  /*
-   * We check only the length of password string here.
-   * Real check is defered to decrypt/authenticate routines.
-   */
   ks_pw1 = gpg_do_read_simple (NR_DO_KEYSTRING_PW1);
   if ((ks_pw1 == NULL && pw_len == strlen (OPENPGP_CARD_INITIAL_PW1))
       || (ks_pw1 != NULL && pw_len == ks_pw1[0]))
     {				/* No problem */
-      /*
-       * We don't call gpg_reset_pw_err_counters here, because
-       * password may be wrong.
-       */
       pw1_keystring[0] = pw_len;
       sha1 (pw, pw_len, pw1_keystring+1);
+      if (gpg_do_load_prvkey (GPG_KEY_FOR_DECRYPTION, BY_USER,
+			      pw1_keystring + 1) < 0)
+	goto error;
+
+      if (gpg_do_load_prvkey (GPG_KEY_FOR_AUTHENTICATION, BY_USER,
+			      pw1_keystring + 1) < 0)
+	goto error;
+
+      /* Reset counter as it's success now */
+      gpg_reset_pw_err_counter (PW_ERR_PW1);
       auth_status |= AC_PSO_OTHER_AUTHORIZED;
       return 1;
     }
   else
     {
+    error:
       gpg_increment_pw_err_counter (PW_ERR_PW1);
       return 0;
     }

src/call-rsa.c

@@ -33,7 +33,8 @@
 static rsa_context rsa_ctx;
 
 int
-rsa_sign (const uint8_t *raw_message, uint8_t *output, int msg_len)
+rsa_sign (const uint8_t *raw_message, uint8_t *output, int msg_len,
+	  struct key_data *kd)
 {
   mpi P1, Q1, H;
   int r;
@@ -43,8 +44,8 @@ rsa_sign (const uint8_t *raw_message, uint8_t *output, int msg_len)
 
   rsa_ctx.len = 2048 / 8;
   mpi_read_string (&rsa_ctx.E, 16, "10001");
-  mpi_read_binary (&rsa_ctx.P, &kd.data[0], rsa_ctx.len / 2);
+  mpi_read_binary (&rsa_ctx.P, &kd->data[0], rsa_ctx.len / 2);
-  mpi_read_binary (&rsa_ctx.Q, &kd.data[128], rsa_ctx.len / 2);
+  mpi_read_binary (&rsa_ctx.Q, &kd->data[128], rsa_ctx.len / 2);
   mpi_mul_mpi (&rsa_ctx.N, &rsa_ctx.P, &rsa_ctx.Q);
   mpi_sub_int (&P1, &rsa_ctx.P, 1);
   mpi_sub_int (&Q1, &rsa_ctx.Q, 1);
@@ -114,7 +115,8 @@ modulus_free (const uint8_t *p)
 }
 
 int
-rsa_decrypt (const uint8_t *input, uint8_t *output, int msg_len)
+rsa_decrypt (const uint8_t *input, uint8_t *output, int msg_len,
+	     struct key_data *kd)
 {
   mpi P1, Q1, H;
   int r;
@@ -130,8 +132,8 @@ rsa_decrypt (const uint8_t *input, uint8_t *output, int msg_len)
   DEBUG_WORD (msg_len);
 
   mpi_read_string (&rsa_ctx.E, 16, "10001");
-  mpi_read_binary (&rsa_ctx.P, &kd.data[0], 2048 / 8 / 2);
+  mpi_read_binary (&rsa_ctx.P, &kd->data[0], 2048 / 8 / 2);
-  mpi_read_binary (&rsa_ctx.Q, &kd.data[128], 2048 / 8 / 2);
+  mpi_read_binary (&rsa_ctx.Q, &kd->data[128], 2048 / 8 / 2);
   mpi_mul_mpi (&rsa_ctx.N, &rsa_ctx.P, &rsa_ctx.Q);
   mpi_sub_int (&P1, &rsa_ctx.P, 1);
   mpi_sub_int (&Q1, &rsa_ctx.Q, 1);

src/gnuk.h

@@ -81,7 +81,7 @@ extern void gpg_do_public_key (uint8_t kk_byte);
 
 
 enum kind_of_key {
-  GPG_KEY_FOR_SIGNING,
+  GPG_KEY_FOR_SIGNING = 0,
   GPG_KEY_FOR_DECRYPTION,
   GPG_KEY_FOR_AUTHENTICATION,
 };
@@ -140,6 +140,7 @@ extern int flash_key_write (uint8_t *key_addr, const uint8_t *key_data, const ui
 #define KEYSTRING_SIZE_PW3 (KEYSTRING_PASSLEN_SIZE+KEYSTRING_SALT_SIZE \
   				+KEYSTRING_ITER_SIZE+KEYSTRING_MD_SIZE)
 
+extern void gpg_do_clear_prvkey (enum kind_of_key kk);
 extern int gpg_do_load_prvkey (enum kind_of_key kk, int who, const uint8_t *keystring);
 extern int gpg_do_chks_prvkey (enum kind_of_key kk,
 			       int who_old, const uint8_t *old_ks,
@@ -148,7 +149,7 @@ extern int gpg_do_chks_prvkey (enum kind_of_key kk,
 extern int gpg_change_keystring (int who_old, const uint8_t *old_ks,
 				 int who_new, const uint8_t *new_ks);
 
-extern struct key_data kd;
+extern struct key_data kd[3];
 
 #ifdef DEBUG
 #define DEBUG_INFO(msg)	    put_string (msg)
@@ -164,10 +165,10 @@ extern struct key_data kd;
 #define DEBUG_BINARY(s,len)
 #endif
 
-extern int rsa_sign (const uint8_t *, uint8_t *, int);
+extern int rsa_sign (const uint8_t *, uint8_t *, int, struct key_data *);
 extern const uint8_t *modulus_calc (const uint8_t *, int);
 extern void modulus_free (const uint8_t *);
-extern int rsa_decrypt (const uint8_t *input, uint8_t *output, int msg_len);
+extern int rsa_decrypt (const uint8_t *, uint8_t *, int, struct key_data *);
 
 extern int gpg_do_write_prvkey (enum kind_of_key kk, const uint8_t *key_data, int key_len, const uint8_t *keystring);
 

src/openpgp-do.c

@@ -528,7 +528,8 @@ encrypt (const uint8_t *key_str, uint8_t *data, int len)
   aes_crypt_cfb128 (&aes, AES_ENCRYPT, len, &iv_offset, iv, data, data);
 }
 
-struct key_data kd;
+/* Signing, Decryption, and Authentication */
+struct key_data kd[3];
 
 static void
 decrypt (const uint8_t *key_str, uint8_t *data, int len)
@@ -561,6 +562,12 @@ get_do_ptr_nr_for_kk (enum kind_of_key kk)
   return NR_DO_PRVKEY_SIG;
 }
 
+void
+gpg_do_clear_prvkey (enum kind_of_key kk)
+{
+  memset ((void *)&kd[kk], 0, sizeof (struct key_data));
+}
+
 /*
  * Return  1 on success,
  *         0 if none,
@@ -578,13 +585,13 @@ gpg_do_load_prvkey (enum kind_of_key kk, int who, const uint8_t *keystring)
     return 0;
 
   key_addr = *(uint8_t **)&(do_data)[1];
-  memcpy (kd.data, key_addr, KEY_CONTENT_LEN);
+  memcpy (kd[kk].data, key_addr, KEY_CONTENT_LEN);
-  memcpy (((uint8_t *)&kd.check), do_data+5, ADDITIONAL_DATA_SIZE);
+  memcpy (((uint8_t *)&kd[kk].check), do_data+5, ADDITIONAL_DATA_SIZE);
   memcpy (dek, do_data+5+16*who, DATA_ENCRYPTION_KEY_SIZE);
 
   decrypt (keystring, dek, DATA_ENCRYPTION_KEY_SIZE);
-  decrypt (dek, (uint8_t *)&kd, sizeof (struct key_data));
+  decrypt (dek, (uint8_t *)&kd[kk], sizeof (struct key_data));
-  if (memcmp (kd.magic, GNUK_MAGIC, KEY_MAGIC_LEN) != 0)
+  if (memcmp (kd[kk].magic, GNUK_MAGIC, KEY_MAGIC_LEN) != 0)
     {
       DEBUG_INFO ("gpg_do_load_prvkey failed.\r\n");
       return -1;
@@ -653,10 +660,10 @@ gpg_do_write_prvkey (enum kind_of_key kk, const uint8_t *key_data, int key_len,
   DEBUG_INFO ("key_addr: ");
   DEBUG_WORD ((uint32_t)key_addr);
 
-  memcpy (kd.data, key_data, KEY_CONTENT_LEN);
+  memcpy (kd[kk].data, key_data, KEY_CONTENT_LEN);
-  kd.check = calc_check32 (key_data, KEY_CONTENT_LEN);
+  kd[kk].check = calc_check32 (key_data, KEY_CONTENT_LEN);
-  kd.random = get_random ();
+  kd[kk].random = get_random ();
-  memcpy (kd.magic, GNUK_MAGIC, KEY_MAGIC_LEN);
+  memcpy (kd[kk].magic, GNUK_MAGIC, KEY_MAGIC_LEN);
 
   if (do_data)			/* We have old prvkey */
     {
@@ -684,9 +691,9 @@ gpg_do_write_prvkey (enum kind_of_key kk, const uint8_t *key_data, int key_len,
       ks_rc = gpg_do_read_simple (NR_DO_KEYSTRING_RC);
     }
 
-  encrypt (dek, (uint8_t *)&kd, sizeof (struct key_data));
+  encrypt (dek, (uint8_t *)&kd[kk], sizeof (struct key_data));
 
-  r = flash_key_write (key_addr, kd.data, modulus);
+  r = flash_key_write (key_addr, kd[kk].data, modulus);
   modulus_free (modulus);
 
   if (r < 0)
@@ -698,9 +705,13 @@ gpg_do_write_prvkey (enum kind_of_key kk, const uint8_t *key_data, int key_len,
     }
 
   pd->key_addr = key_addr;
-  memcpy (pd->crm_encrypted, (uint8_t *)&kd.check, ADDITIONAL_DATA_SIZE);
+  memcpy (pd->crm_encrypted, (uint8_t *)&kd[kk].check, ADDITIONAL_DATA_SIZE);
 
+  if (kk == GPG_KEY_FOR_SIGNING)
     ac_reset_pso_cds ();
+  else
+    ac_reset_pso_other ();
+
   if (ks_pw1)
     encrypt (ks_pw1+1, pd->dek_encrypted_1, DATA_ENCRYPTION_KEY_SIZE);
   else

src/openpgp.c

@@ -258,6 +258,7 @@ cmd_change_password (void)
     {
       gpg_do_write_simple (NR_DO_KEYSTRING_PW1, new_ks0, KEYSTRING_SIZE_PW1);
       ac_reset_pso_cds ();
+      ac_reset_pso_other ();
       gpg_reset_pw_err_counter (PW_ERR_PW1);
       DEBUG_INFO ("Changed DO_KEYSTRING_PW1.\r\n");
       GPG_SUCCESS ();
@@ -266,6 +267,7 @@ cmd_change_password (void)
     {
       gpg_do_write_simple (NR_DO_KEYSTRING_PW1, new_ks0, 1);
       ac_reset_pso_cds ();
+      ac_reset_pso_other ();
       gpg_reset_pw_err_counter (PW_ERR_PW1);
       DEBUG_INFO ("Changed length of DO_KEYSTRING_PW1.\r\n");
       GPG_SUCCESS ();
@@ -344,6 +346,7 @@ cmd_reset_user_password (void)
 	  DEBUG_INFO ("done (no prvkey).\r\n");
 	  gpg_do_write_simple (NR_DO_KEYSTRING_PW1, new_ks0, KEYSTRING_SIZE_PW1);
 	  ac_reset_pso_cds ();
+	  ac_reset_pso_other ();
 	  gpg_reset_pw_err_counter (PW_ERR_RC);
 	  gpg_reset_pw_err_counter (PW_ERR_PW1);
 	  GPG_SUCCESS ();
@@ -352,6 +355,7 @@ cmd_reset_user_password (void)
 	{
 	  DEBUG_INFO ("done.\r\n");
 	  ac_reset_pso_cds ();
+	  ac_reset_pso_other ();
 	  gpg_reset_pw_err_counter (PW_ERR_RC);
 	  gpg_reset_pw_err_counter (PW_ERR_PW1);
 	  GPG_SUCCESS ();
@@ -388,6 +392,7 @@ cmd_reset_user_password (void)
 	  DEBUG_INFO ("done (no privkey).\r\n");
 	  gpg_do_write_simple (NR_DO_KEYSTRING_PW1, new_ks0, KEYSTRING_SIZE_PW1);
 	  ac_reset_pso_cds ();
+	  ac_reset_pso_other ();
 	  gpg_reset_pw_err_counter (PW_ERR_PW1);
 	  GPG_SUCCESS ();
 	}
@@ -395,6 +400,7 @@ cmd_reset_user_password (void)
 	{
 	  DEBUG_INFO ("done.\r\n");
 	  ac_reset_pso_cds ();
+	  ac_reset_pso_other ();
 	  gpg_reset_pw_err_counter (PW_ERR_PW1);
 	  GPG_SUCCESS ();
 	}
@@ -575,7 +581,8 @@ cmd_pso (void)
 	{
 	  DEBUG_SHORT (len);  /* Should be cmd_APDU_size - 8 [- 1] */
 
-	  r = rsa_sign (&cmd_APDU[data_start], res_APDU, len);
+	  r = rsa_sign (&cmd_APDU[data_start], res_APDU, len,
+			&kd[GPG_KEY_FOR_SIGNING]);
 	  if (r < 0)
 	    {
 	      ac_reset_pso_cds ();
@@ -602,23 +609,11 @@ cmd_pso (void)
 	  return;
 	}
 
-      if ((r = gpg_do_load_prvkey (GPG_KEY_FOR_DECRYPTION, BY_USER,
-				   pw1_keystring + 1)) < 0)
-	{
-	  gpg_increment_pw_err_counter (PW_ERR_PW1);
-	  GPG_SECURITY_FAILURE ();
-	  return;
-	}
-      else
-	/* Reset counter as it's success now */
-	gpg_reset_pw_err_counter (PW_ERR_PW1);
-
-      ac_reset_pso_other ();
-
       /* Skip padding 0x00 */
       data_start++;
       len--;
-      r = rsa_decrypt (&cmd_APDU[data_start], res_APDU, len);
+      r = rsa_decrypt (&cmd_APDU[data_start], res_APDU, len,
+		       &kd[GPG_KEY_FOR_DECRYPTION]);
       if (r < 0)
 	GPG_ERROR ();
     }
@@ -661,20 +656,8 @@ cmd_internal_authenticate (void)
 	  return;
 	}
 
-      if ((r = gpg_do_load_prvkey (GPG_KEY_FOR_AUTHENTICATION, BY_USER,
+      r = rsa_sign (&cmd_APDU[data_start], res_APDU, len,
-				   pw1_keystring + 1)) < 0)
+		    &kd[GPG_KEY_FOR_AUTHENTICATION]);
-	{
-	  gpg_increment_pw_err_counter (PW_ERR_PW1);
-	  GPG_SECURITY_FAILURE ();
-	  return;
-	}
-      else
-	/* Reset counter as it's success now */
-	gpg_reset_pw_err_counter (PW_ERR_PW1);
-
-      ac_reset_pso_other ();
-
-      r = rsa_sign (&cmd_APDU[data_start], res_APDU, len);
       if (r < 0)
 	GPG_ERROR ();
     }