From 6461a1a958e02404c06fae35890f924f8d016cf8 Mon Sep 17 00:00:00 2001 From: Stuart Schechter Date: Wed, 30 Oct 2019 08:30:06 +0900 Subject: [PATCH 1/2] document the ctap_atomic_count amount parameter --- targets/stm32l432/src/device.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/targets/stm32l432/src/device.c b/targets/stm32l432/src/device.c index 532736b0..85a0ba19 100644 --- a/targets/stm32l432/src/device.c +++ b/targets/stm32l432/src/device.c @@ -201,9 +201,9 @@ int solo_is_locked(){ /** device_migrate * Depending on version of device, migrates: - * * Moves attestation certificate to data segment. + * * Moves attestation certificate to data segment. * * Creates locked variable and stores in data segment. - * + * * Once in place, this allows all devices to accept same firmware, * rather than using "hacker" and "secure" builds. */ @@ -507,6 +507,8 @@ void authenticator_write_state(AuthenticatorState * a, int backup) } #if !defined(IS_BOOTLOADER) +// amounts <= 256 increment the counter +// amounts > 256 set the counter uint32_t ctap_atomic_count(uint32_t amount) { int offset = 0; From a01d6f4b35d9e2dc01032a26243783330965ab16 Mon Sep 17 00:00:00 2001 From: Stuart Schechter Date: Wed, 30 Oct 2019 08:33:56 +0900 Subject: [PATCH 2/2] correct CTAPHID_LOADKEY counter_replacement doc verify all four bytes of version document why version must be 0 --- fido2/ctaphid.c | 40 ++++++++++++++++++++++++++++++---------- fido2/device.h | 6 +++++- 2 files changed, 35 insertions(+), 11 deletions(-) diff --git a/fido2/ctaphid.c b/fido2/ctaphid.c index 9cee0625..fb01c0cc 100644 --- a/fido2/ctaphid.c +++ b/fido2/ctaphid.c @@ -762,9 +762,23 @@ uint8_t ctaphid_custom_command(int len, CTAP_RESPONSE * ctap_resp, CTAPHID_WRITE * Load external key. Useful for enabling backups. * bytes: 4 4 96 * payload: version [maj rev patch RFU]| counter_replacement (BE) | master_key | - * - * Counter should be increased by a large amount, e.g. (0x10000000) - * to outdo any previously lost/broken keys. + * + * The version should be set to 0 until future versions of this interface + * are released. + * + * The counter_replacement value will be used to set the counter that must be + * monotonically-increasing in each execution of the FIDO protocol. + * If the value is <= 256, it will be added to the current counter. + * If the value is > 256, it will replace the current counter. + * Alas, we can't know if the user has previously loaded this key into another + * SoloKey token, what the initial counter value was on that key, and how many + * times it was incremented. + * So, we suggest the convention of setting the value to + * (seconds since Jan 1, 2019 GMT) / 3 + * If this convention is followed, and this should ensure a SoloKey + * always has a higher counter than the one it replaces, so long as that + * SoloKey also used the same convention and did was not used to authenticate + * at a rate greater than once every 3 seconds. */ printf1(TAG_HID,"CTAPHID_LOADKEY\n"); if (len != 104) @@ -773,18 +787,24 @@ uint8_t ctaphid_custom_command(int len, CTAP_RESPONSE * ctap_resp, CTAPHID_WRITE ctaphid_send_error(wb->cid, CTAP1_ERR_INVALID_LENGTH); return 1; } - param = ctap_buffer[0] << 16; - param |= ctap_buffer[1] << 8; - param |= ctap_buffer[2] << 0; + // Store the current operation version into param + param = (ctap_buffer[0] << 24) | + (ctap_buffer[1] << 16) | + (ctap_buffer[2] << 8) | + (ctap_buffer[3] << 0); + // This code is written for version 0 of the load key operation, + // and we cannot know if future versions of the operation will be + // compatible with it. So, reject any version other than 0 as + // unsupported if (param != 0){ ctaphid_send_error(wb->cid, CTAP2_ERR_UNSUPPORTED_OPTION); return 1; } // Ask for THREE button presses - if (ctap_user_presence_test(8000) > 0) - if (ctap_user_presence_test(2000) > 0) - if (ctap_user_presence_test(2000) > 0) + if (ctap_user_presence_test(CTAP_USER_PRESENCE_TEST_MS_TO_WAIT_ONE_BUTTON_PRESS) > 0) + if (ctap_user_presence_test(CTAP_USER_PRESENCE_TEST_MS_TO_WAIT_SUBSEQUENT_BUTTON_PRESSES) > 0) + if (ctap_user_presence_test(CTAP_USER_PRESENCE_TEST_MS_TO_WAIT_SUBSEQUENT_BUTTON_PRESSES) > 0) { ctap_load_external_keys(ctap_buffer + 8); param = ctap_buffer[7]; @@ -808,4 +828,4 @@ uint8_t ctaphid_custom_command(int len, CTAP_RESPONSE * ctap_resp, CTAPHID_WRITE } return 0; -} \ No newline at end of file +} diff --git a/fido2/device.h b/fido2/device.h index 46923236..1354a466 100644 --- a/fido2/device.h +++ b/fido2/device.h @@ -54,7 +54,11 @@ int device_is_button_pressed(); // Test for user presence // Return 2 for disabled, 1 for user is present, 0 user not present, -1 if cancel is requested. -int ctap_user_presence_test(uint32_t delay); +int ctap_user_presence_test(uint32_t ms_to_wait_for_user_consent); + +// Suggested delays for user presence test +#define CTAP_USER_PRESENCE_TEST_MS_TO_WAIT_ONE_BUTTON_PRESS 8000 +#define CTAP_USER_PRESENCE_TEST_MS_TO_WAIT_SUBSEQUENT_BUTTON_PRESSES 2000 // Generate @num bytes of random numbers to @dest // return 1 if success, error otherwise