Merge pull request #182 from solokeys/nfc_adjustments

Nfc adjustments

CHANGELOG.md

@@ -33,3 +33,8 @@ WARNING: This update may break previous registrations! This is because we fixed
 - Adds hmac-secret extension support. This extension is used for generating 32 or 64 byte symmetric keys using parameters from the platform and secrets on the authenticator. It's used by Windows Hello - - for offline authentication.
 - Fix bug in FIDO auth, where setting the pin requires all previous registrations to use pin. Only UV bit needs to be cleared.
 - Slightly change serial emulation USB descriptor to make it less abused by Linux Modem Manager.
+
+## [2.2.0] - 2019-04-17
+
+- Fixes the ordering of keys encoded in CBOR maps to be canonical ordering. They previously were not ordered in any particular way and caused issues for Chrome. #170
+- Fixes CTAP2 implementation to accept credential IDs created by the CTAP1 implementation. So registering with U2F and later authenticating with FIDO2 should work.

STABLE_VERSION

@@ -1 +1 @@
-2.1.0
+2.2.0

fido2/cose_key.h

@@ -16,7 +16,7 @@
 #define COSE_KEY_KTY_EC2        2
 #define COSE_KEY_CRV_P256       1
 
-
-#define COSE_ALG_ES256              -7
+#define COSE_ALG_ES256            -7
+#define COSE_ALG_ECDH_ES_HKDF_256 -25
 
 #endif

fido2/ctap.c

@@ -466,12 +466,11 @@ static int ctap_make_auth_data(struct rpId * rp, CborEncoder * map, uint8_t * au
     count = auth_data_update_count(&authData->head);
 
     device_set_status(CTAPHID_STATUS_UPNEEDED);
-  // if NFC - not need to click a button
-    int but = 1;
-    if(!device_is_nfc())
-    {
-        but = ctap_user_presence_test();
-    }
+
+    int but;
+
+    but = ctap_user_presence_test();
+
 
     if (!but)
     {
@@ -702,6 +701,14 @@ uint8_t ctap_make_credential(CborEncoder * encoder, uint8_t * request, int lengt
         printf2(TAG_ERR,"error, parse_make_credential failed\n");
         return ret;
     }
+    if (MC.pinAuthEmpty)
+    {
+        if (!ctap_user_presence_test())
+        {
+                return CTAP2_ERR_OPERATION_DENIED;
+        }
+        return ctap_is_pin_set() == 1 ? CTAP2_ERR_PIN_INVALID : CTAP2_ERR_PIN_NOT_SET;
+    }
     if ((MC.paramsParsed & MC_requiredMask) != MC_requiredMask)
     {
         printf2(TAG_ERR,"error, required parameter(s) for makeCredential are missing\n");
@@ -1133,6 +1140,14 @@ uint8_t ctap_get_assertion(CborEncoder * encoder, uint8_t * request, int length)
         return ret;
     }
 
+    if (GA.pinAuthEmpty)
+    {
+        if (!ctap_user_presence_test())
+        {
+                return CTAP2_ERR_OPERATION_DENIED;
+        }
+        return ctap_is_pin_set() == 1 ? CTAP2_ERR_PIN_INVALID : CTAP2_ERR_PIN_NOT_SET;
+    }
     if (GA.pinAuthPresent)
     {
         ret = verify_pin_auth(GA.pinAuth, GA.clientDataHash);
@@ -1460,7 +1475,7 @@ uint8_t ctap_client_pin(CborEncoder * encoder, uint8_t * request, int length)
 
             ret = cbor_encode_int(&map, RESP_keyAgreement);
             check_ret(ret);
-            ret = ctap_add_cose_key(&map, KEY_AGREEMENT_PUB, KEY_AGREEMENT_PUB+32, PUB_KEY_CRED_PUB_KEY, COSE_ALG_ES256);
+            ret = ctap_add_cose_key(&map, KEY_AGREEMENT_PUB, KEY_AGREEMENT_PUB+32, PUB_KEY_CRED_PUB_KEY, COSE_ALG_ECDH_ES_HKDF_256);
             check_retr(ret);
 
             break;

fido2/ctap.h

@@ -243,6 +243,11 @@ typedef struct
 
     uint8_t pinAuth[16];
     uint8_t pinAuthPresent;
+    // pinAuthEmpty is true iff an empty bytestring was provided as pinAuth.
+    // This is exclusive with |pinAuthPresent|. It exists because an empty
+    // pinAuth is a special signal to block for touch. See
+    // https://fidoalliance.org/specs/fido-v2.0-ps-20190130/fido-client-to-authenticator-protocol-v2.0-ps-20190130.html#using-pinToken-in-authenticatorMakeCredential
+    uint8_t pinAuthEmpty;
     int pinProtocol;
     CTAP_extensions extensions;
 
@@ -266,6 +271,11 @@ typedef struct
 
     uint8_t pinAuth[16];
     uint8_t pinAuthPresent;
+    // pinAuthEmpty is true iff an empty bytestring was provided as pinAuth.
+    // This is exclusive with |pinAuthPresent|. It exists because an empty
+    // pinAuth is a special signal to block for touch. See
+    // https://fidoalliance.org/specs/fido-v2.0-ps-20190130/fido-client-to-authenticator-protocol-v2.0-ps-20190130.html#using-pinToken-in-authenticatorGetAssertion
+    uint8_t pinAuthEmpty;
     int pinProtocol;
 
     CTAP_credentialDescriptor creds[ALLOW_LIST_MAX_SIZE];

fido2/ctap_parse.c

@@ -823,14 +823,22 @@ uint8_t ctap_parse_make_credential(CTAP_makeCredential * MC, CborEncoder * encod
                 ret = parse_options(&map, &MC->credInfo.rk, &MC->uv, &MC->up);
                 check_retr(ret);
                 break;
-            case MC_pinAuth:
+            case MC_pinAuth: {
                 printf1(TAG_MC,"CTAP_pinAuth\n");
 
+                size_t pinSize;
+                if (cbor_value_get_type(&map) == CborByteStringType &&
+                    cbor_value_get_string_length(&map, &pinSize) == CborNoError &&
+                    pinSize == 0)
+                {
+                    MC->pinAuthEmpty = 1;
+                    break;
+                }
+
                 ret = parse_fixed_byte_string(&map, MC->pinAuth, 16);
                 if (CTAP1_ERR_INVALID_LENGTH != ret)    // damn microsoft
                 {
                     check_retr(ret);
-
                 }
                 else
                 {
@@ -838,6 +846,7 @@ uint8_t ctap_parse_make_credential(CTAP_makeCredential * MC, CborEncoder * encod
                 }
                 MC->pinAuthPresent = 1;
                 break;
+            }
             case MC_pinProtocol:
                 printf1(TAG_MC,"CTAP_pinProtocol\n");
                 if (cbor_value_get_type(&map) == CborIntegerType)
@@ -1055,9 +1064,18 @@ uint8_t ctap_parse_get_assertion(CTAP_getAssertion * GA, uint8_t * request, int
                 ret = parse_options(&map, &GA->rk, &GA->uv, &GA->up);
                 check_retr(ret);
                 break;
-            case GA_pinAuth:
+            case GA_pinAuth: {
                 printf1(TAG_GA,"CTAP_pinAuth\n");
 
+                size_t pinSize;
+                if (cbor_value_get_type(&map) == CborByteStringType &&
+                    cbor_value_get_string_length(&map, &pinSize) == CborNoError &&
+                    pinSize == 0)
+                {
+                    GA->pinAuthEmpty = 1;
+                    break;
+                }
+
                 ret = parse_fixed_byte_string(&map, GA->pinAuth, 16);
                 if (CTAP1_ERR_INVALID_LENGTH != ret)    // damn microsoft
                 {
@@ -1073,6 +1091,7 @@ uint8_t ctap_parse_get_assertion(CTAP_getAssertion * GA, uint8_t * request, int
                 GA->pinAuthPresent = 1;
 
                 break;
+            }
             case GA_pinProtocol:
                 printf1(TAG_GA,"CTAP_pinProtocol\n");
                 if (cbor_value_get_type(&map) == CborIntegerType)

targets/stm32l432/build/application.mk

@@ -81,4 +81,4 @@ cbor:
 	cd ../../tinycbor/ && make clean
 	cd ../../tinycbor/ && make CC="$(CC)" AR=$(AR) \
 LDFLAGS="$(LDFLAGS_LIB)" \
-CFLAGS="$(CFLAGS)"
+CFLAGS="$(CFLAGS) -Os"

targets/stm32l432/src/device.c

@@ -111,13 +111,13 @@ void device_init()
 {
 
     hw_init(LOW_FREQUENCY);
-    isLowFreq = 0;
 
     haveNFC = nfc_init();
 
     if (haveNFC)
     {
         printf1(TAG_NFC, "Have NFC\r\n");
+        isLowFreq = 1;
     }
     else
     {
@@ -460,6 +460,10 @@ static int handle_packets()
 int ctap_user_presence_test()
 {
     int ret;
+    if (device_is_nfc())
+    {
+        return 1;
+    }
 #if SKIP_BUTTON_CHECK_WITH_DELAY
     int i=500;
     while(i--)

targets/stm32l432/src/nfc.c

@@ -496,9 +496,9 @@ void nfc_process_iblock(uint8_t * buf, int len)
 			// WTX_on(WTX_TIME_DEFAULT);
             // SystemClock_Config_LF32();
             // delay(300);
-            device_set_clock_rate(DEVICE_LOW_POWER_FAST);;
+            if (device_is_nfc()) device_set_clock_rate(DEVICE_LOW_POWER_FAST);;
 			u2f_request_nfc(&buf[1], len, &ctap_resp);
-            device_set_clock_rate(DEVICE_LOW_POWER_IDLE);;
+            if (device_is_nfc())  device_set_clock_rate(DEVICE_LOW_POWER_IDLE);;
 			// if (!WTX_off())
 			// 	return;