Merge pull request #240 from Nitrokey/remove-pin-storage

Replace FIDO2 PIN storage with its hash

fido2/ctap.c

@@ -25,11 +25,11 @@
 #include "extensions.h"
 
 #include "device.h"
+#include "data_migration.h"
 
 uint8_t PIN_TOKEN[PIN_TOKEN_SIZE];
 uint8_t KEY_AGREEMENT_PUB[64];
 static uint8_t KEY_AGREEMENT_PRIV[32];
-static uint8_t PIN_CODE_HASH[32];
 static int8_t PIN_BOOT_ATTEMPTS_LEFT = PIN_BOOT_ATTEMPTS;
 
 AuthenticatorState STATE;
@@ -1286,11 +1286,13 @@ uint8_t ctap_update_pin_if_verified(uint8_t * pinEnc, int len, uint8_t * platfor
     uint8_t hmac[32];
     int ret;
 
+//    Validate incoming data packet len
     if (len < 64)
     {
         return CTAP1_ERR_OTHER;
     }
 
+//    Validate device's state
     if (ctap_is_pin_set())  // Check first, prevent SCA
     {
         if (ctap_device_locked())
@@ -1303,6 +1305,7 @@ uint8_t ctap_update_pin_if_verified(uint8_t * pinEnc, int len, uint8_t * platfor
         }
     }
 
+//    calculate shared_secret
     crypto_ecc256_shared_secret(platform_pubkey, KEY_AGREEMENT_PRIV, shared_secret);
 
     crypto_sha256_init();
@@ -1325,6 +1328,7 @@ uint8_t ctap_update_pin_if_verified(uint8_t * pinEnc, int len, uint8_t * platfor
         return CTAP2_ERR_PIN_AUTH_INVALID;
     }
 
+//     decrypt new PIN with shared secret
     crypto_aes256_init(shared_secret, NULL);
 
     while((len & 0xf) != 0) // round up to nearest  AES block size multiple
@@ -1334,7 +1338,7 @@ uint8_t ctap_update_pin_if_verified(uint8_t * pinEnc, int len, uint8_t * platfor
 
     crypto_aes256_decrypt(pinEnc, len);
 
-
+//      validate new PIN (length)
 
     ret = trailing_zeros(pinEnc, NEW_PIN_ENC_MIN_SIZE - 1);
     ret = NEW_PIN_ENC_MIN_SIZE  - ret;
@@ -1350,6 +1354,8 @@ uint8_t ctap_update_pin_if_verified(uint8_t * pinEnc, int len, uint8_t * platfor
         dump_hex1(TAG_CP, pinEnc, ret);
     }
 
+//    validate device's state, decrypt and compare pinHashEnc (user provided current PIN hash) with stored PIN_CODE_HASH
+
     if (ctap_is_pin_set())
     {
         if (ctap_device_locked())
@@ -1362,7 +1368,14 @@ uint8_t ctap_update_pin_if_verified(uint8_t * pinEnc, int len, uint8_t * platfor
         }
         crypto_aes256_reset_iv(NULL);
         crypto_aes256_decrypt(pinHashEnc, 16);
-        if (memcmp(pinHashEnc, PIN_CODE_HASH, 16) != 0)
+
+        uint8_t pinHashEncSalted[32];
+        crypto_sha256_init();
+        crypto_sha256_update(pinHashEnc, 16);
+        crypto_sha256_update(STATE.PIN_SALT, sizeof(STATE.PIN_SALT));
+        crypto_sha256_final(pinHashEncSalted);
+
+        if (memcmp(pinHashEncSalted, STATE.PIN_CODE_HASH, 16) != 0)
         {
             ctap_reset_key_agreement();
             ctap_decrement_pin_attempts();
@@ -1378,6 +1391,7 @@ uint8_t ctap_update_pin_if_verified(uint8_t * pinEnc, int len, uint8_t * platfor
         }
     }
 
+//      set new PIN (update and store PIN_CODE_HASH)
     ctap_update_pin(pinEnc, ret);
 
     return 0;
@@ -1397,12 +1411,16 @@ uint8_t ctap_add_pin_if_verified(uint8_t * pinTokenEnc, uint8_t * platform_pubke
 
     crypto_aes256_decrypt(pinHashEnc, 16);
 
-
+    uint8_t pinHashEncSalted[32];
-    if (memcmp(pinHashEnc, PIN_CODE_HASH, 16) != 0)
+    crypto_sha256_init();
+    crypto_sha256_update(pinHashEnc, 16);
+    crypto_sha256_update(STATE.PIN_SALT, sizeof(STATE.PIN_SALT));
+    crypto_sha256_final(pinHashEncSalted);
+    if (memcmp(pinHashEncSalted, STATE.PIN_CODE_HASH, 16) != 0)
     {
         printf2(TAG_ERR,"Pin does not match!\n");
         printf2(TAG_ERR,"platform-pin-hash: "); dump_hex1(TAG_ERR, pinHashEnc, 16);
-        printf2(TAG_ERR,"authentic-pin-hash: "); dump_hex1(TAG_ERR, PIN_CODE_HASH, 16);
+        printf2(TAG_ERR,"authentic-pin-hash: "); dump_hex1(TAG_ERR, STATE.PIN_CODE_HASH, 16);
         printf2(TAG_ERR,"shared-secret: "); dump_hex1(TAG_ERR, shared_secret, 32);
         printf2(TAG_ERR,"platform-pubkey: "); dump_hex1(TAG_ERR, platform_pubkey, 64);
         printf2(TAG_ERR,"device-pubkey: "); dump_hex1(TAG_ERR, KEY_AGREEMENT_PUB, 64);
@@ -1710,8 +1728,18 @@ static void ctap_state_init()
     STATE.remaining_tries = PIN_LOCKOUT_ATTEMPTS;
     STATE.is_pin_set = 0;
     STATE.rk_stored = 0;
+    STATE.data_version = STATE_VERSION;
 
     ctap_reset_rk();
+
+    if (ctap_generate_rng(STATE.PIN_SALT, sizeof(STATE.PIN_SALT)) != 1) {
+        printf2(TAG_ERR, "Error, rng failed\n");
+        exit(1);
+    }
+
+    printf1(TAG_STOR, "Generated PIN SALT: ");
+    dump_hex1(TAG_STOR, STATE.PIN_SALT, sizeof STATE.PIN_SALT);
+
 }
 
 void ctap_init()
@@ -1744,14 +1772,12 @@ void ctap_init()
         }
     }
 
+    do_migration_if_required(&STATE);
+
     crypto_load_master_secret(STATE.key_space);
 
     if (ctap_is_pin_set())
     {
-        printf1(TAG_STOR,"pin code: \"%s\"\n", STATE.pin_code);
-        crypto_sha256_init();
-        crypto_sha256_update(STATE.pin_code, STATE.pin_code_length);
-        crypto_sha256_final(PIN_CODE_HASH);
         printf1(TAG_STOR, "attempts_left: %d\n", STATE.remaining_tries);
     }
     else
@@ -1783,34 +1809,38 @@ uint8_t ctap_is_pin_set()
     return STATE.is_pin_set == 1;
 }
 
-uint8_t ctap_pin_matches(uint8_t * pin, int len)
+/**
-{
+ * Set new PIN, by updating PIN hash. Save state.
-    return memcmp(pin, STATE.pin_code, len) == 0;
+ * Globals: STATE
-}
+ * @param pin new PIN (raw)
-
+ * @param len pin array length
-
+ */
 void ctap_update_pin(uint8_t * pin, int len)
 {
-    if (len > NEW_PIN_ENC_MIN_SIZE || len < 4)
+    if (len >= NEW_PIN_ENC_MIN_SIZE || len < 4)
     {
         printf2(TAG_ERR, "Update pin fail length\n");
         exit(1);
     }
-    memset(STATE.pin_code, 0, NEW_PIN_ENC_MIN_SIZE);
-    memmove(STATE.pin_code, pin, len);
-    STATE.pin_code_length = len;
-    STATE.pin_code[NEW_PIN_ENC_MIN_SIZE - 1] = 0;
 
     crypto_sha256_init();
-    crypto_sha256_update(STATE.pin_code, len);
+    crypto_sha256_update(pin, len);
-    crypto_sha256_final(PIN_CODE_HASH);
+    uint8_t intermediateHash[32];
+    crypto_sha256_final(intermediateHash);
+
+    crypto_sha256_init();
+    crypto_sha256_update(intermediateHash, 16);
+    memset(intermediateHash, 0, sizeof(intermediateHash));
+    crypto_sha256_update(STATE.PIN_SALT, sizeof(STATE.PIN_SALT));
+    crypto_sha256_final(STATE.PIN_CODE_HASH);
 
     STATE.is_pin_set = 1;
 
     authenticator_write_state(&STATE, 1);
     authenticator_write_state(&STATE, 0);
 
-    printf1(TAG_CTAP, "New pin set: %s\n", STATE.pin_code);
+    printf1(TAG_CTAP, "New pin set: %s [%d]\n", pin, len);
+    dump_hex1(TAG_ERR, STATE.PIN_CODE_HASH, sizeof(STATE.PIN_CODE_HASH));
 }
 
 uint8_t ctap_decrement_pin_attempts()
@@ -1827,9 +1857,7 @@ uint8_t ctap_decrement_pin_attempts()
 
         if (ctap_device_locked())
         {
-            memset(PIN_TOKEN,0,sizeof(PIN_TOKEN));
+            lock_device_permanently();
-            memset(PIN_CODE_HASH,0,sizeof(PIN_CODE_HASH));
-            printf1(TAG_CP, "Device locked!\n");
         }
     }
     else
@@ -1985,8 +2013,17 @@ void ctap_reset()
     }
 
     ctap_reset_state();
-    memset(PIN_CODE_HASH,0,sizeof(PIN_CODE_HASH));
     ctap_reset_key_agreement();
 
     crypto_load_master_secret(STATE.key_space);
 }
+
+void lock_device_permanently() {
+    memset(PIN_TOKEN, 0, sizeof(PIN_TOKEN));
+    memset(STATE.PIN_CODE_HASH, 0, sizeof(STATE.PIN_CODE_HASH));
+
+    printf1(TAG_CP, "Device locked!\n");
+
+    authenticator_write_state(&STATE, 0);
+    authenticator_write_state(&STATE, 1);
+}

fido2/ctap.h

@@ -359,5 +359,6 @@ uint16_t ctap_key_len(uint8_t index);
 extern uint8_t PIN_TOKEN[PIN_TOKEN_SIZE];
 extern uint8_t KEY_AGREEMENT_PUB[64];
 
+void lock_device_permanently();
 
 #endif

fido2/data_migration.c

@@ -0,0 +1,91 @@
+// Copyright 2019 SoloKeys Developers
+//
+// Licensed under the Apache License, Version 2.0, <LICENSE-APACHE or
+// http://apache.org/licenses/LICENSE-2.0> or the MIT license <LICENSE-MIT or
+// http://opensource.org/licenses/MIT>, at your option. This file may not be
+// copied, modified, or distributed except according to those terms.
+
+#include "data_migration.h"
+#include "log.h"
+#include "device.h"
+#include "crypto.h"
+
+// TODO move from macro to function/assert for better readability?
+#define check(x) assert(state_prev_0xff->x == state_tmp_ptr->x);
+#define check_buf(x) assert(memcmp(state_prev_0xff->x, state_tmp_ptr->x, sizeof(state_tmp_ptr->x)) == 0);
+
+bool migrate_from_FF_to_01(AuthenticatorState_0xFF* state_prev_0xff, AuthenticatorState_0x01* state_tmp_ptr){
+    // Calculate PIN hash, and replace PIN raw storage with it; add version to structure
+    // other ingredients do not change
+    if (state_tmp_ptr->data_version != 0xFF)
+        return false;
+
+    static_assert(sizeof(AuthenticatorState_0xFF) <= sizeof(AuthenticatorState_0x01), "New state structure is smaller, than current one, which is not handled");
+
+    if (ctap_generate_rng(state_tmp_ptr->PIN_SALT, sizeof(state_tmp_ptr->PIN_SALT)) != 1) {
+        printf2(TAG_ERR, "Error, rng failed\n");
+        return false;
+    }
+    if (state_prev_0xff->is_pin_set){
+        crypto_sha256_init();
+        crypto_sha256_update(state_prev_0xff->pin_code, state_prev_0xff->pin_code_length);
+        uint8_t intermediateHash[32];
+        crypto_sha256_final(intermediateHash);
+
+        crypto_sha256_init();
+        crypto_sha256_update(intermediateHash, 16);
+        memset(intermediateHash, 0, sizeof(intermediateHash));
+        crypto_sha256_update(state_tmp_ptr->PIN_SALT, sizeof(state_tmp_ptr->PIN_SALT));
+        crypto_sha256_final(state_tmp_ptr->PIN_CODE_HASH);
+    }
+
+    assert(state_tmp_ptr->_reserved == state_prev_0xff->pin_code_length);
+    state_tmp_ptr->_reserved = 0xFF;
+    state_tmp_ptr->data_version = 1;
+
+    check(is_initialized);
+    check(is_pin_set);
+    check(remaining_tries);
+    check(rk_stored);
+    check_buf(key_lens);
+    check_buf(key_space);
+    assert(state_tmp_ptr->data_version != 0xFF);
+
+    return true;
+}
+
+void save_migrated_state(AuthenticatorState *state_tmp_ptr) {
+    memmove(&STATE, state_tmp_ptr, sizeof(AuthenticatorState));
+    authenticator_write_state(state_tmp_ptr, 0);
+    authenticator_write_state(state_tmp_ptr, 1);
+}
+
+void do_migration_if_required(AuthenticatorState* state_current){
+    // Currently handles only state structures with the same size, or bigger
+    // FIXME rework to raw buffers with fixed size to allow state structure size decrease
+    if(!state_current->is_initialized)
+        return;
+
+    AuthenticatorState state_tmp;
+    AuthenticatorState state_previous;
+    authenticator_read_state(&state_previous);
+    authenticator_read_state(&state_tmp);
+    if(state_current->data_version == 0xFF){
+        printf2(TAG_ERR, "Running migration\n");
+        bool success = migrate_from_FF_to_01((AuthenticatorState_0xFF *) &state_previous, &state_tmp);
+        if (!success){
+            printf2(TAG_ERR, "Failed migration from 0xFF to 1\n");
+            // FIXME discuss migration failure behavior
+            goto return_cleanup;
+        }
+        dump_hex1(TAG_ERR, (void*)&state_tmp, sizeof(state_tmp));
+        dump_hex1(TAG_ERR, (void*)&state_previous, sizeof(state_previous));
+        save_migrated_state(&state_tmp);
+    }
+
+    assert(state_current->data_version == STATE_VERSION);
+
+    return_cleanup:
+    memset(&state_tmp, 0, sizeof(AuthenticatorState));
+    memset(&state_previous, 0, sizeof(AuthenticatorState));
+}

fido2/data_migration.h

@@ -0,0 +1,15 @@
+// Copyright 2019 SoloKeys Developers
+//
+// Licensed under the Apache License, Version 2.0, <LICENSE-APACHE or
+// http://apache.org/licenses/LICENSE-2.0> or the MIT license <LICENSE-MIT or
+// http://opensource.org/licenses/MIT>, at your option. This file may not be
+// copied, modified, or distributed except according to those terms.
+
+#ifndef FIDO2_PR_DATA_MIGRATION_H
+#define FIDO2_PR_DATA_MIGRATION_H
+
+#include "storage.h"
+
+void do_migration_if_required(AuthenticatorState* state_current);
+
+#endif //FIDO2_PR_DATA_MIGRATION_H

fido2/extensions/wallet.c

@@ -95,7 +95,7 @@ int8_t wallet_pin(uint8_t subcmd, uint8_t * pinAuth, uint8_t * arg1, uint8_t * a
             if (ret != 0)
                 return ret;
 
-            printf1(TAG_WALLET,"Success.  Pin = %s\n", STATE.pin_code);
+//            printf1(TAG_WALLET,"Success.  Pin = %s\n", STATE.pin_code);
 
             break;
         case CP_cmdChangePin:

fido2/storage.h

@@ -11,6 +11,9 @@
 
 #define KEY_SPACE_BYTES     128
 #define MAX_KEYS            (1)
+#define PIN_SALT_LEN        (32)
+#define STATE_VERSION        (1)
+
 
 #define BACKUP_MARKER       0x5A
 #define INITIALIZED_MARKER  0xA5
@@ -19,20 +22,40 @@
 #define ERR_KEY_SPACE_TAKEN (-2)
 #define ERR_KEY_SPACE_EMPTY (-2)
 
+typedef struct
+{
+  // Pin information
+  uint8_t is_initialized;
+  uint8_t is_pin_set;
+  uint8_t pin_code[NEW_PIN_ENC_MIN_SIZE];
+  int pin_code_length;
+  int8_t remaining_tries;
+
+  uint16_t rk_stored;
+
+  uint16_t key_lens[MAX_KEYS];
+  uint8_t key_space[KEY_SPACE_BYTES];
+} AuthenticatorState_0xFF;
+
+
 typedef struct
 {
     // Pin information
     uint8_t is_initialized;
     uint8_t is_pin_set;
-    uint8_t pin_code[NEW_PIN_ENC_MIN_SIZE];
+    uint8_t PIN_CODE_HASH[32];
-    int pin_code_length;
+    uint8_t PIN_SALT[PIN_SALT_LEN];
+    int _reserved;
     int8_t remaining_tries;
 
     uint16_t rk_stored;
 
     uint16_t key_lens[MAX_KEYS];
     uint8_t key_space[KEY_SPACE_BYTES];
-} AuthenticatorState;
+    uint8_t data_version;
+} AuthenticatorState_0x01;
+
+typedef AuthenticatorState_0x01 AuthenticatorState;
 
 
 typedef struct

pc/device.c

@@ -633,6 +633,12 @@ int device_is_nfc()
     return 0;
 }
 
+
+void request_from_nfc(bool request_active)
+{
+  
+}
+
 void device_set_clock_rate(DEVICE_CLOCK_RATE param)
 {
 

targets/stm32l432/build/application.mk

@@ -10,6 +10,7 @@ SRC += $(DRIVER_LIBS) $(USB_LIB)
 SRC += ../../fido2/apdu.c ../../fido2/util.c ../../fido2/u2f.c ../../fido2/test_power.c
 SRC += ../../fido2/stubs.c ../../fido2/log.c  ../../fido2/ctaphid.c  ../../fido2/ctap.c
 SRC += ../../fido2/ctap_parse.c ../../fido2/main.c
+SRC += ../../fido2/data_migration.c
 SRC += ../../fido2/extensions/extensions.c ../../fido2/extensions/solo.c
 SRC += ../../fido2/extensions/wallet.c