Use hal_error_t as suggested by Rob.

4 files changed, 101 insertions, 71 deletions

diff --git a/hal.h b/hal.h
index 776acac..04dab48 100644
--- a/hal.h
+++ b/hal.h
@@ -139,6 +139,9 @@
   DEFINE_HAL_ERROR(HAL_ERROR_RPC_PACKET_OVERFLOW,       "RPC packet overflow")                          \
   DEFINE_HAL_ERROR(HAL_ERROR_RPC_BAD_FUNCTION,          "Bad RPC function number")                      \
   DEFINE_HAL_ERROR(HAL_ERROR_KEY_NAME_TOO_LONG,         "Key name too long")                            \
+  DEFINE_HAL_ERROR(HAL_ERROR_MASTERKEY_NOT_SET,         "Master key (Key Encryption Key) not set")      \
+  DEFINE_HAL_ERROR(HAL_ERROR_MASTERKEY_FAIL,            "Master key generic failure")                   \
+  DEFINE_HAL_ERROR(HAL_ERROR_MASTERKEY_BAD_LENGTH,      "Master key of unacceptable length")            \
   END_OF_HAL_ERROR_LIST
 
 /* Marker to forestall silly line continuation errors */
diff --git a/ks_flash.c b/ks_flash.c
index 036e72e..159df5b 100644
--- a/ks_flash.c
+++ b/ks_flash.c
@@ -324,11 +324,11 @@ hal_error_t hal_ks_get_kek(uint8_t *kek,
                       (kek_max < bitsToBytes(256)) ? bitsToBytes(192) :
                       bitsToBytes(256));
 
-  if (masterkey_volatile_read(kek, len) == HSM_MASTERKEY_SET) {
+  if (masterkey_volatile_read(kek, len) == LIBHAL_OK) {
       *kek_len = len;
       return LIBHAL_OK;
   }
-  if (masterkey_flash_read(kek, len) == HSM_MASTERKEY_SET) {
+  if (masterkey_flash_read(kek, len) == LIBHAL_OK) {
       *kek_len = len;
       return LIBHAL_OK;
   }
diff --git a/masterkey.c b/masterkey.c
index cc9a62f..6425515 100644
--- a/masterkey.c
+++ b/masterkey.c
@@ -32,6 +32,23 @@
  * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
+/*
+ * Code to load the Master key (Key Encryption Key) from either the volatile MKM
+ * (by asking the FPGA to provide it, using the mkmif) or from the last sector in
+ * the keystore flash.
+ *
+ * Storing the master key in flash is a pretty Bad Idea, but since the Alpha board
+ * doesn't have a battery mounted (only pin headers for attaching one), it might
+ * help in non-production use where one doesn't have tamper protection anyways.
+ *
+ * For production use on the Alpha, one option is to have the Master Key on paper
+ * and enter it into volatile RAM after each power on.
+ *
+ * In both volatile memory and flash, the data is stored as a 32 bit status to
+ * know if the memory is initialized or not, followed by 32 bytes (256 bits) of
+ * Master Key.
+ */
+
 #define HAL_OK CMIS_HAL_OK
 #include "stm-init.h"
 #include "stm-keystore.h"
@@ -49,11 +66,18 @@ static int volatile_init = 0, flash_init = 0;
 static const hal_core_t *core = NULL;
 
 #define MKM_VOLATILE_STATUS_ADDRESS	0
-#define SCLK_DIV			0x20
+#define MKM_VOLATILE_SCLK_DIV		0x20
 #define MKM_FLASH_STATUS_ADDRESS	(KEYSTORE_SECTOR_SIZE * (KEYSTORE_NUM_SECTORS - 1))
-#define MKM_FLASH_STATUS_IN_USE		0x0000ffff
 #define KEK_LENGTH (256 / 8)
 
+/* Match uninitialized flash for the "not set" value.
+ * Leave some bits at 1 for the "set" value to allow
+ * for adding more values later, if needed.
+ */
+#define MKM_STATUS_NOT_SET		0xffffffff
+#define MKM_STATUS_SET			0x0000ffff
+#define MKM_STATUS_ERASED		0x00000000
+
 
 hal_error_t masterkey_volatile_init()
 {
@@ -66,17 +90,21 @@ hal_error_t masterkey_volatile_init()
 	}
 
 	err =
-	    hal_mkmif_set_clockspeed(core, SCLK_DIV) ||
+	    hal_mkmif_set_clockspeed(core, MKM_VOLATILE_SCLK_DIV) ||
 	    hal_mkmif_init(core) ||
 	    hal_mkmif_read_word(core, MKM_VOLATILE_STATUS_ADDRESS, &status);
 
 	if (err != LIBHAL_OK) return err;
 
-	if (status != HSM_MASTERKEY_SET &&
-	    status != HSM_MASTERKEY_NOT_SET) {
-	    if ((err = hal_mkmif_write_word(core, MKM_VOLATILE_STATUS_ADDRESS, HSM_MASTERKEY_NOT_SET)) != LIBHAL_OK) {
-		return err;
-	    }
+	if (status != MKM_STATUS_SET && status != MKM_STATUS_NOT_SET) {
+	    /* XXX Something is a bit fishy here. If we just write the status word, it reads back wrong sometimes,
+	     * while if we write the full buf too it is consistently right afterwards.
+	     */
+	    uint8_t buf[KEK_LENGTH] = {0};
+	    err =
+		hal_mkmif_write(core, MKM_VOLATILE_STATUS_ADDRESS + 4, buf, sizeof(buf)) ||
+		hal_mkmif_write_word(core, MKM_VOLATILE_STATUS_ADDRESS, MKM_STATUS_NOT_SET);
+	    if (err != LIBHAL_OK) return err;
 	}
 
 	volatile_init = 1;
@@ -84,71 +112,68 @@ hal_error_t masterkey_volatile_init()
     return LIBHAL_OK;
 }
 
-masterkey_status_t masterkey_volatile_read(uint8_t *buf, size_t len)
+hal_error_t masterkey_volatile_read(uint8_t *buf, size_t len)
 {
     hal_error_t err;
     uint32_t status;
 
+    if (len && len != KEK_LENGTH) return HAL_ERROR_MASTERKEY_BAD_LENGTH;
+
     err =
 	masterkey_volatile_init() ||
 	hal_mkmif_read_word(core, MKM_VOLATILE_STATUS_ADDRESS, &status);
 
-    if (err != LIBHAL_OK) return HSM_MASTERKEY_FAIL;
+    if (err != LIBHAL_OK) return err;
 
-    if (buf != NULL && status == HSM_MASTERKEY_SET) {
-	if ((err = hal_mkmif_read(core, MKM_VOLATILE_STATUS_ADDRESS + 4, buf, len)) == LIBHAL_OK) {
-	    return HSM_MASTERKEY_SET;
+    if (buf != NULL && len) {
+	/* Don't return the random bytes in the RAM memory in case it isn't initialized.
+	 * Or maybe we should fill the buffer with proper random data in that case... hmm.
+	 */
+	if (status == MKM_STATUS_SET) {
+	    if ((err = hal_mkmif_read(core, MKM_VOLATILE_STATUS_ADDRESS + 4, buf, len)) != LIBHAL_OK) {
+		return err;
+	    }
+	} else {
+	    memset(buf, 0x0, len);
 	}
     }
 
-    if (status == HSM_MASTERKEY_SET || status == HSM_MASTERKEY_NOT_SET) {
-	return (masterkey_status_t) status;
-    }
+    if (status == MKM_STATUS_SET) return LIBHAL_OK;
+    if (status == MKM_STATUS_NOT_SET) return HAL_ERROR_MASTERKEY_NOT_SET;
 
-    return HSM_MASTERKEY_FAIL;
+    return HAL_ERROR_MASTERKEY_FAIL;
 }
 
-masterkey_status_t masterkey_volatile_write(uint8_t *buf, size_t len)
+hal_error_t masterkey_volatile_write(uint8_t *buf, size_t len)
 {
     hal_error_t err;
 
-    if (len != KEK_LENGTH) return HSM_MASTERKEY_FAIL;
-    if (! buf) return HSM_MASTERKEY_FAIL;
+    if (len != KEK_LENGTH) return HAL_ERROR_MASTERKEY_BAD_LENGTH;
+    if (! buf) return HAL_ERROR_MASTERKEY_FAIL;
 
     err =
 	masterkey_volatile_init() ||
 	hal_mkmif_write(core, MKM_VOLATILE_STATUS_ADDRESS + 4, buf, len) ||
-	hal_mkmif_write_word(core, MKM_VOLATILE_STATUS_ADDRESS, HSM_MASTERKEY_SET);
-
-    if (err != LIBHAL_OK) return HSM_MASTERKEY_FAIL;
+	hal_mkmif_write_word(core, MKM_VOLATILE_STATUS_ADDRESS, MKM_STATUS_SET);
 
-    return HSM_MASTERKEY_OK;
+    return err;
 }
 
-masterkey_status_t masterkey_volatile_erase(size_t len)
+hal_error_t masterkey_volatile_erase(size_t len)
 {
     uint8_t buf[KEK_LENGTH] = {0};
     hal_error_t err;
 
-    if (len != KEK_LENGTH) return HSM_MASTERKEY_FAIL;
+    if (len != KEK_LENGTH) return HAL_ERROR_MASTERKEY_BAD_LENGTH;
 
     err =
 	masterkey_volatile_init() ||
 	hal_mkmif_write(core, MKM_VOLATILE_STATUS_ADDRESS + 4, buf, sizeof(buf)) ||
-	hal_mkmif_write_word(core, MKM_VOLATILE_STATUS_ADDRESS, HSM_MASTERKEY_NOT_SET);
+	hal_mkmif_write_word(core, MKM_VOLATILE_STATUS_ADDRESS, MKM_STATUS_NOT_SET);
 
-    if (err != LIBHAL_OK) return HSM_MASTERKEY_FAIL;
-
-    return HSM_MASTERKEY_OK;
+    return err;
 }
 
-/* Storing the master key in flash is a pretty Bad Idea, but since the Alpha board
- * doesn't have a battery mounted (only pin headers for attaching one), it might
- * help in non-production use where one doesn't have tamper protection anyways.
- *
- * For production use on the Alpha, one option is to have the Master Key on paper
- * and enter it into volatile RAM after each power on.
- */
 hal_error_t masterkey_flash_init()
 {
     if (! flash_init) {
@@ -158,61 +183,70 @@ hal_error_t masterkey_flash_init()
     return LIBHAL_OK;
 }
 
-masterkey_status_t masterkey_flash_read(uint8_t *buf, size_t len)
+hal_error_t masterkey_flash_read(uint8_t *buf, size_t len)
 {
     uint8_t page[KEYSTORE_PAGE_SIZE];
     uint32_t *status = (uint32_t *) page;
 
-    if (len > sizeof(page) - 4) return HSM_MASTERKEY_FAIL;
+    if (len && len != KEK_LENGTH) return HAL_ERROR_MASTERKEY_BAD_LENGTH;
 
-    if (masterkey_flash_init() != LIBHAL_OK) return HSM_MASTERKEY_FAIL;
+    if (masterkey_flash_init() != LIBHAL_OK) return HAL_ERROR_MASTERKEY_FAIL;
 
     if (! keystore_read_data(MKM_FLASH_STATUS_ADDRESS, page, sizeof(page))) {
 	memset(page, 0, sizeof(page));
-	return HSM_MASTERKEY_FAIL;
+	return HAL_ERROR_MASTERKEY_FAIL;
     }
 
-    if (buf != NULL && len) memcpy(buf, page + 4, len);
+    if (buf != NULL && len) {
+	/* Don't return what's in the flash memory in case it isn't initialized.
+	 * Or maybe we should fill the buffer with proper random data in that case... hmm.
+	 */
+	if (*status == MKM_STATUS_SET) {
+	    memcpy(buf, page + 4, len);
+	} else {
+	    memset(buf, 0x0, len);
+	}
+    }
 
     memset(page + 4, 0, sizeof(page) - 4);
 
-    if (*status == MKM_FLASH_STATUS_IN_USE) return HSM_MASTERKEY_SET;
-    if (*status == 0x0 || *status == 0xffffffff) return HSM_MASTERKEY_NOT_SET;
+    if (*status == MKM_STATUS_SET) return LIBHAL_OK;
+    if (*status == MKM_STATUS_ERASED || *status == MKM_STATUS_NOT_SET) return HAL_ERROR_MASTERKEY_NOT_SET;
 
-    return HSM_MASTERKEY_FAIL;
+    return HAL_ERROR_MASTERKEY_FAIL;
 }
 
-masterkey_status_t masterkey_flash_write(uint8_t *buf, size_t len)
+hal_error_t masterkey_flash_write(uint8_t *buf, size_t len)
 {
     uint8_t page[KEYSTORE_PAGE_SIZE] = {0xff};
     uint32_t *status = (uint32_t *) page;
     int res;
 
-    if (len > sizeof(page) - 4) return HSM_MASTERKEY_FAIL;
-    if (buf == NULL) return HSM_MASTERKEY_FAIL;
+    if (len != KEK_LENGTH) return HAL_ERROR_MASTERKEY_BAD_LENGTH;
+    if (buf == NULL) return HAL_ERROR_MASTERKEY_FAIL;
 
-    if (masterkey_flash_init() != LIBHAL_OK) return HSM_MASTERKEY_FAIL;
+    if (masterkey_flash_init() != LIBHAL_OK) return HAL_ERROR_MASTERKEY_FAIL;
 
-    *status = MKM_FLASH_STATUS_IN_USE;
+    *status = MKM_STATUS_SET;
     memcpy(page + 4, buf, len);
 
     res = keystore_write_data(MKM_FLASH_STATUS_ADDRESS, page, sizeof(page));
     memset(page, 0, sizeof(page));
     if (res != 1) {
-	return HSM_MASTERKEY_FAIL;
+	return HAL_ERROR_MASTERKEY_FAIL;
     }
 
-    return HSM_MASTERKEY_OK;
+    return LIBHAL_OK;
 }
 
-masterkey_status_t masterkey_flash_erase(size_t len)
+hal_error_t masterkey_flash_erase(size_t len)
 {
-    if (len > KEYSTORE_PAGE_SIZE - 4) return HSM_MASTERKEY_FAIL;
+    if (len != KEK_LENGTH) return HAL_ERROR_MASTERKEY_BAD_LENGTH;
 
     if (keystore_erase_sectors(MKM_FLASH_STATUS_ADDRESS / KEYSTORE_SECTOR_SIZE,
 			       MKM_FLASH_STATUS_ADDRESS / KEYSTORE_SECTOR_SIZE) != 1) {
-	return HSM_MASTERKEY_FAIL;
+	return HAL_ERROR_MASTERKEY_FAIL;
     }
 
-    return HSM_MASTERKEY_OK;
+    return LIBHAL_OK;
 }
diff --git a/masterkey.h b/masterkey.h
index 8b47409..1d5e7a5 100644
--- a/masterkey.h
+++ b/masterkey.h
@@ -35,19 +35,12 @@
 #ifndef __STM32_HSM_MASTERKEY_H
 #define __STM32_HSM_MASTERKEY_H
 
-typedef enum {
-    HSM_MASTERKEY_FAIL,
-    HSM_MASTERKEY_OK,
-    HSM_MASTERKEY_SET,
-    HSM_MASTERKEY_NOT_SET,
-} masterkey_status_t;
+extern hal_error_t masterkey_volatile_read(uint8_t *buf, size_t len);
+extern hal_error_t masterkey_volatile_write(uint8_t *buf, size_t len);
+extern hal_error_t masterkey_volatile_erase(size_t len);
 
-extern masterkey_status_t masterkey_volatile_read(uint8_t *buf, size_t len);
-extern masterkey_status_t masterkey_volatile_write(uint8_t *buf, size_t len);
-extern masterkey_status_t masterkey_volatile_erase(size_t len);
-
-extern masterkey_status_t masterkey_flash_read(uint8_t *buf, size_t len);
-extern masterkey_status_t masterkey_flash_write(uint8_t *buf, size_t len);
-extern masterkey_status_t masterkey_flash_erase(size_t len);
+extern hal_error_t masterkey_flash_read(uint8_t *buf, size_t len);
+extern hal_error_t masterkey_flash_write(uint8_t *buf, size_t len);
+extern hal_error_t masterkey_flash_erase(size_t len);
 
 #endif /* __STM32_HSM_MASTERKEY_H */