improve key store handling (for overriding)

diff --git a/ChangeLog b/ChangeLog
index c2d323c..aafff6e 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,16 @@
+2013-10-24  Niibe Yutaka  <gniibe@fsij.org>
+
+       * src/gnuk.ld.in (.gnuk_flash): Three pages for three keys.
+       * src/flash.c (FLASH_KEYSTORE_SIZE): Likewise.
+       (flash_keystore_release): Remove.
+       (flash_key_fill_zero_as_released)
+       (flash_check_all_other_keys_released, flash_key_release): New.
+       (flash_init, flash_key_alloc): New method to handle free space.
+       * src/openpgp-do.c (fetch_four_bytes): New.
+       (gpg_do_load_prvkey, gpg_do_delete_prvkey, gpg_do_public_key): Use
+       fetch_four_bytes.
+       (gpg_do_delete_prvkey): Call flash_key_release.
+
 2013-10-23  Niibe Yutaka  <gniibe@fsij.org>
 
        * test/features/010_setup_passphrase.feature

diff --git a/src/flash.c b/src/flash.c
index 850b870..efcb4c9 100644 (file)
--- a/src/flash.c
+++ b/src/flash.c
@@ -57,19 +57,18 @@
  * _data_pool
  *        <two pages>
  * _keystore_pool
- *         1.5-KiB Key store (512-byte (p, q and N) key-store * 3)
+ *         three flash pages for keystore (single: 512-byte (p, q and N))
  */
 #define KEY_SIZE       512     /* P, Q and N */
 
 #define FLASH_DATA_POOL_HEADER_SIZE    2
 #define FLASH_DATA_POOL_SIZE           (FLASH_PAGE_SIZE*2)
-#define FLASH_KEYSTORE_SIZE            (KEY_SIZE*3)
+#define FLASH_KEYSTORE_SIZE            (FLASH_PAGE_SIZE*3)
 
 static const uint8_t *data_pool;
 extern uint8_t _keystore_pool;
 
 static uint8_t *last_p;
-static const uint8_t *keystore;
 
 /* The first halfword is generation for the data page (little endian) */
 const uint8_t const flash_data[4] __attribute__ ((section (".gnuk_data"))) = {
@@ -82,7 +81,6 @@ extern uint8_t _data_pool;
 const uint8_t *
 flash_init (void)
 {
-  const uint8_t *p;
   uint16_t gen0, gen1;
   uint16_t *gen0_p = (uint16_t *)&_data_pool;
   uint16_t *gen1_p = (uint16_t *)(&_data_pool + FLASH_PAGE_SIZE);
@@ -99,13 +97,6 @@ flash_init (void)
   else
     data_pool = &_data_pool;
 
-  /* Seek empty keystore */
-  p = &_keystore_pool;
-  while (*p != 0xff || *(p+1) != 0xff)
-    p += KEY_SIZE;
-
-  keystore = p;
-
   return data_pool + FLASH_DATA_POOL_HEADER_SIZE;
 }
 
@@ -277,16 +268,31 @@ flash_do_release (const uint8_t *do_data)
     flash_warning ("fill-zero tag_nr failure");
 }
 
+
 uint8_t *
 flash_key_alloc (void)
 {
-  uint8_t *k = (uint8_t *)keystore;
+  uint8_t *k;
+  int i; 
+
+  /* Seek empty keystore.  */
+  k = &_keystore_pool;
+  while (k < &_keystore_pool + FLASH_KEYSTORE_SIZE)
+    {
+      const uint32_t *p = (const uint32_t *)k;
 
-  if ((k - &_keystore_pool) >= FLASH_KEYSTORE_SIZE)
-    return NULL;
+      for (i = 0; i < KEY_SIZE/4; i++)
+       if (p[i] != 0xffffffff)
+         break;
 
-  keystore += KEY_SIZE;
-  return k;
+      if (i == KEY_SIZE/4)     /* Yes, it's empty.  */
+       return k;
+
+      k += KEY_SIZE;
+    }
+
+  /* Should not happen as we have enough space, but just in case.  */
+  return NULL;
 }
 
 int
@@ -317,16 +323,44 @@ flash_key_write (uint8_t *key_addr, const uint8_t *key_data,
   return 0;
 }
 
+static int
+flash_check_all_other_keys_released (const uint8_t *key_addr)
+{
+  uint32_t start = (uint32_t)key_addr & ~(FLASH_PAGE_SIZE - 1);
+  const uint32_t *p = (const uint32_t *)start;
+
+  while (p < (const uint32_t *)(start + FLASH_PAGE_SIZE))
+    if (p == (const uint32_t *)key_addr)
+      p += KEY_SIZE/4;
+    else
+      if (*p)
+       return 0;
+      else
+       p++;
+
+  return 1;
+}
+
+static void
+flash_key_fill_zero_as_released (uint8_t *key_addr)
+{
+  int i;
+  uint32_t addr = (uint32_t)key_addr;
+
+  for (i = 0; i < KEY_SIZE/2; i++)
+    flash_program_halfword (addr + i*2, 0);
+}
+
 void
-flash_keystore_release (void)
+flash_key_release (uint8_t *key_addr)
 {
-  flash_erase_page ((uint32_t)&_keystore_pool);
-#if FLASH_KEYSTORE_SIZE > FLASH_PAGE_SIZE
-  flash_erase_page ((uint32_t)&_keystore_pool + FLASH_PAGE_SIZE);
-#endif
-  keystore = &_keystore_pool;
+  if (flash_check_all_other_keys_released (key_addr))
+    flash_erase_page (((uint32_t)key_addr & ~(FLASH_PAGE_SIZE - 1)));
+  else
+    flash_key_fill_zero_as_released (key_addr);
 }
 
+
 void
 flash_clear_halfword (uint32_t addr)
 {

diff --git a/src/gnuk.h b/src/gnuk.h
index 060db71..8fdb3a4 100644 (file)
--- a/src/gnuk.h
+++ b/src/gnuk.h
@@ -111,9 +111,9 @@ extern const uint8_t *flash_init (void);
 extern void flash_do_release (const uint8_t *);
 extern const uint8_t *flash_do_write (uint8_t nr, const uint8_t *data, int len);
 extern uint8_t *flash_key_alloc (void);
+extern void flash_key_release (uint8_t *);
 extern int flash_key_write (uint8_t *key_addr, const uint8_t *key_data,
                            const uint8_t *pubkey, int pubkey_len);
-extern void flash_keystore_release (void);
 extern void flash_set_data_pool_last (const uint8_t *p);
 extern void flash_clear_halfword (uint32_t addr);
 extern void flash_increment_counter (uint8_t counter_tag_nr);

diff --git a/src/gnuk.ld.in b/src/gnuk.ld.in
index a215a6f..c8376b1 100644 (file)
--- a/src/gnuk.ld.in
+++ b/src/gnuk.ld.in
@@ -167,7 +167,11 @@ SECTIONS
         . = ALIGN(@FLASH_PAGE_SIZE@);
         . += @FLASH_PAGE_SIZE@;
         _keystore_pool = .;
-        . += 512*3;
+        . += 512;
+        . = ALIGN(@FLASH_PAGE_SIZE@);
+        . += 512;
+        . = ALIGN(@FLASH_PAGE_SIZE@);
+        . += 512;
         . = ALIGN(@FLASH_PAGE_SIZE@);
         _updatekey_store = .;
         . += 1024;

diff --git a/src/openpgp-do.c b/src/openpgp-do.c
index 5b9c52e..037071b 100644 (file)
--- a/src/openpgp-do.c
+++ b/src/openpgp-do.c
@@ -35,6 +35,16 @@
 #include "polarssl/config.h"
 #include "polarssl/aes.h"
 
+/* Handles possible unaligned access.  */
+static uint32_t
+fetch_four_bytes (const void *addr)
+{
+  const uint8_t *p = (const uint8_t *)addr;
+
+  return p[0] | (p[1] << 8) | (p[2] << 16) | (p[3] << 24);
+}
+
+
 #define PASSWORD_ERRORS_MAX 3  /* >= errors, it will be locked */
 static const uint8_t *pw_err_counter_p[3];
 
@@ -705,7 +715,7 @@ gpg_do_load_prvkey (enum kind_of_key kk, int who, const uint8_t *keystring)
   if (do_data == NULL)
     return 0;
 
-  key_addr = *(const uint8_t **)&(do_data)[1]; /* Possible unaligned access */
+  key_addr = (const uint8_t *)fetch_four_bytes (&do_data[1]);
   memcpy (kdi.data, key_addr, KEY_CONTENT_LEN);
   iv = do_data+5;
   memcpy (kdi.checksum, iv + INITIAL_VECTOR_SIZE, DATA_ENCRYPTION_KEY_SIZE);
@@ -734,10 +744,13 @@ gpg_do_delete_prvkey (enum kind_of_key kk)
 {
   uint8_t nr = get_do_ptr_nr_for_kk (kk);
   const uint8_t *do_data = do_ptr[nr - NR_DO__FIRST__];
+  uint8_t *key_addr;
 
   if (do_data == NULL)
     return;
 
+  key_addr = (uint8_t *)fetch_four_bytes (&do_data[1]);
+  flash_key_release (key_addr);
   do_ptr[nr - NR_DO__FIRST__] = NULL;
   flash_do_release (do_data);
 
@@ -758,8 +771,6 @@ gpg_do_delete_prvkey (enum kind_of_key kk)
 
   if (--num_prv_keys == 0)
     {
-      flash_keystore_release ();
-
       /* Delete PW1 and RC if any.  */
       gpg_do_write_simple (NR_DO_KEYSTRING_PW1, NULL, 0);
       gpg_do_write_simple (NR_DO_KEYSTRING_RC, NULL, 0);
@@ -1591,7 +1602,7 @@ gpg_do_public_key (uint8_t kk_byte)
       return;
     }
 
-  key_addr = *(const uint8_t **)&do_data[1];
+  key_addr = (const uint8_t *)fetch_four_bytes (&do_data[1]);
 
   res_p = res_APDU;