From 821880a8d6d999afc16161e83fd0b51fda0687ab Mon Sep 17 00:00:00 2001 From: Conor Patrick Date: Wed, 20 Mar 2019 15:45:10 -0400 Subject: [PATCH 01/20] parse extension info in MC --- fido2/ctap.c | 21 +++++++++++++--- fido2/ctap.h | 6 +++++ fido2/ctap_parse.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 86 insertions(+), 4 deletions(-) diff --git a/fido2/ctap.c b/fido2/ctap.c index f110ef3..ea14e62 100644 --- a/fido2/ctap.c +++ b/fido2/ctap.c @@ -69,6 +69,8 @@ uint8_t verify_pin_auth(uint8_t * pinAuth, uint8_t * clientDataHash) } + + uint8_t ctap_get_info(CborEncoder * encoder) { int ret; @@ -77,16 +79,14 @@ uint8_t ctap_get_info(CborEncoder * encoder) CborEncoder options; CborEncoder pins; - const int number_of_versions = 2; - - ret = cbor_encoder_create_map(encoder, &map, 5); + ret = cbor_encoder_create_map(encoder, &map, 6); check_ret(ret); { ret = cbor_encode_uint(&map, RESP_versions); // versions key check_ret(ret); { - ret = cbor_encoder_create_array(&map, &array, number_of_versions); + ret = cbor_encoder_create_array(&map, &array, 2); check_ret(ret); { ret = cbor_encode_text_stringz(&array, "U2F_V2"); @@ -98,6 +98,19 @@ uint8_t ctap_get_info(CborEncoder * encoder) check_ret(ret); } + ret = cbor_encode_uint(&map, RESP_extensions); + check_ret(ret); + { + ret = cbor_encoder_create_array(&map, &array, 1); + check_ret(ret); + { + ret = cbor_encode_text_stringz(&array, "hmac-secret"); + check_ret(ret); + } + ret = cbor_encoder_close_container(&map, &array); + check_ret(ret); + } + ret = cbor_encode_uint(&map, RESP_aaguid); check_ret(ret); { diff --git a/fido2/ctap.h b/fido2/ctap.h index a3a8783..797df60 100644 --- a/fido2/ctap.h +++ b/fido2/ctap.h @@ -181,6 +181,11 @@ struct rpId uint8_t name[RP_NAME_LIMIT]; }; +typedef struct +{ + uint8_t hmac_secret; +} CTAP_extensions; + typedef struct { uint32_t paramsParsed; @@ -201,6 +206,7 @@ typedef struct uint8_t pinAuth[16]; uint8_t pinAuthPresent; int pinProtocol; + CTAP_extensions extensions; } CTAP_makeCredential; diff --git a/fido2/ctap_parse.c b/fido2/ctap_parse.c index c432e70..43287e1 100644 --- a/fido2/ctap_parse.c +++ b/fido2/ctap_parse.c @@ -556,6 +556,67 @@ uint8_t parse_options(CborValue * val, uint8_t * rk, uint8_t * uv, uint8_t * up) return 0; } +uint8_t ctap_parse_extensions(CTAP_extensions * ext, CborValue * val) +{ + CborValue map; + size_t sz, map_length; + uint8_t key[16]; + uint8_t ret; + int i; + bool b; + + if (cbor_value_get_type(val) != CborMapType) + { + printf2(TAG_ERR,"error, wrong type\n"); + return CTAP2_ERR_INVALID_CBOR_TYPE; + } + + ret = cbor_value_enter_container(val, &map); + check_ret(ret); + + ret = cbor_value_get_map_length(val, &map_length); + check_ret(ret); + + for (i = 0; i < map_length; i++) + { + if (cbor_value_get_type(&map) != CborTextStringType) + { + printf2(TAG_ERR,"Error, expecting text string type for options map key, got %s\n", cbor_value_get_type_string(&map)); + return CTAP2_ERR_INVALID_CBOR_TYPE; + } + sz = sizeof(key); + ret = cbor_value_copy_text_string(&map, key, &sz, NULL); + + if (ret == CborErrorOutOfMemory) + { + printf2(TAG_ERR,"Error, rp map key is too large. Ignoring.\n"); + cbor_value_advance(&map); + cbor_value_advance(&map); + continue; + } + check_ret(ret); + key[sizeof(key) - 1] = 0; + + ret = cbor_value_advance(&map); + check_ret(ret); + + if (cbor_value_get_type(&map) == CborBooleanType) + { + if (strncmp(key, "hmac-secret",11) == 0) + { + ret = cbor_value_get_boolean(&map, &b); + check_ret(ret); + ext->hmac_secret = b; + printf1(TAG_CTAP, "set hmac-secret to %d\r\n", b); + } + } + + ret = cbor_value_advance(&map); + check_ret(ret); + } + return 0; +} + uint8_t ctap_parse_make_credential(CTAP_makeCredential * MC, CborEncoder * encoder, uint8_t * request, int length) { int ret; @@ -665,6 +726,8 @@ uint8_t ctap_parse_make_credential(CTAP_makeCredential * MC, CborEncoder * encod { return CTAP2_ERR_INVALID_CBOR_TYPE; } + ret = ctap_parse_extensions(&MC->extensions, &map); + check_retr(ret); break; case MC_options: From 60988101676bfecccfcc97696de29c9483b9e6d9 Mon Sep 17 00:00:00 2001 From: Conor Patrick Date: Wed, 20 Mar 2019 15:45:35 -0400 Subject: [PATCH 02/20] start to test hmac-secret --- tools/ctap_test.py | 252 +++++++++++++++++++++++++-------------------- 1 file changed, 138 insertions(+), 114 deletions(-) diff --git a/tools/ctap_test.py b/tools/ctap_test.py index 5695f27..bd7b915 100755 --- a/tools/ctap_test.py +++ b/tools/ctap_test.py @@ -153,6 +153,43 @@ class Tester: elif data[0] != err: raise ValueError("Unexpected error: %02x" % data[0]) + def testFunc(self, func, test, *args, **kwargs): + with Test(test): + res = None + expectedError = kwargs.get("expectedError", None) + otherArgs = kwargs.get("other", {}) + try: + res = func(*args, **otherArgs) + if expectedError != CtapError.ERR.SUCCESS: + raise RuntimeError("Expected error to occur for test: %s" % test) + except CtapError as e: + if expectedError is not None: + if e.code != expectedError: + raise RuntimeError( + "Got error code 0x%x, expected %x" % (e.code, expectedError) + ) + else: + print(e) + return res + + def testReset(self,): + print("Resetting Authenticator...") + self.ctap.reset() + + def testMC(self, test, *args, **kwargs): + return self.testFunc(self.ctap.make_credential, test, *args, **kwargs) + + def testGA(self, test, *args, **kwargs): + return self.testFunc(self.ctap.get_assertion, test, *args, **kwargs) + + def testCP(self, test, *args, **kwargs): + return self.testFunc(self.ctap.client_pin, test, *args, **kwargs) + + def testPP(self, test, *args, **kwargs): + return self.testFunc( + self.client.pin_protocol.get_pin_token, test, *args, **kwargs + ) + def test_long_ping(self): amt = 1000 pingdata = os.urandom(amt) @@ -723,6 +760,30 @@ class Tester: except CtapError as e: print("Warning, reset failed: ", e) + def test_extensions(self,): + creds = [] + exclude_list = [] + rp = {"id": self.host, "name": "ExaRP"} + user = {"id": b"usee_od", "name": "AB User"} + challenge = "Y2hhbGxlbmdl" + pin_protocol = 1 + key_params = [{"type": "public-key", "alg": ES256.ALGORITHM}] + cdh = b"123456789abcdef0123456789abcdef0" + + with Test("Get info has hmac-secret"): + info = self.ctap.get_info() + assert "hmac-secret" in info.extensions + + self.testMC( + "Send MC with hmac-secret ext set to true, expect SUCCESS", + cdh, + rp, + user, + key_params, + expectedError=CtapError.ERR.SUCCESS, + other={"extensions": {"hmac-secret": True}}, + ) + def test_fido2_other(self,): creds = [] @@ -738,46 +799,6 @@ class Tester: key_params = [{"type": "public-key", "alg": ES256.ALGORITHM}] cdh = b"123456789abcdef0123456789abcdef0" - def testFunc(func, test, *args, **kwargs): - with Test(test): - res = None - expectedError = kwargs.get("expectedError", None) - otherArgs = kwargs.get("other", {}) - try: - res = func(*args, **otherArgs) - if expectedError != CtapError.ERR.SUCCESS: - raise RuntimeError( - "Expected error to occur for test: %s" % test - ) - except CtapError as e: - if expectedError is not None: - if e.code != expectedError: - raise RuntimeError( - "Got error code 0x%x, expected %x" - % (e.code, expectedError) - ) - else: - print(e) - return res - - def testReset(): - print("Resetting Authenticator...") - self.ctap.reset() - - def testMC(test, *args, **kwargs): - return testFunc(self.ctap.make_credential, test, *args, **kwargs) - - def testGA(test, *args, **kwargs): - return testFunc(self.ctap.get_assertion, test, *args, **kwargs) - - def testCP(test, *args, **kwargs): - return testFunc(self.ctap.client_pin, test, *args, **kwargs) - - def testPP(test, *args, **kwargs): - return testFunc( - self.client.pin_protocol.get_pin_token, test, *args, **kwargs - ) - def reboot(): if self.is_sim: print("Sending restart command...") @@ -788,7 +809,7 @@ class Tester: input() self.find_device() - testReset() + self.testReset() with Test("Get info"): info = self.ctap.get_info() @@ -804,7 +825,7 @@ class Tester: for x in info.options: assert info.options[x] in [True, False] - prev_reg = testMC( + prev_reg = self.testMC( "Send MC request, expect success", cdh, rp, @@ -826,7 +847,7 @@ class Tester: } ] - prev_auth = testGA( + prev_auth = self.testGA( "Send GA request, expect success", rp["id"], cdh, @@ -847,7 +868,7 @@ class Tester: assert prev_auth.user == None assert prev_auth.number_of_credentials == None - testGA( + self.testGA( "Send GA request with empty allow_list, expect NO_CREDENTIALS", rp["id"], cdh, @@ -860,7 +881,7 @@ class Tester: badid[len(badid) // 2] = badid[len(badid) // 2] ^ 1 badid = bytes(badid) - testGA( + self.testGA( "Send GA request with corrupt credId in allow_list, expect NO_CREDENTIALS", rp["id"], cdh, @@ -868,7 +889,7 @@ class Tester: expectedError=CtapError.ERR.NO_CREDENTIALS, ) - testMC( + self.testMC( "Send MC request with missing clientDataHash, expect error", None, rp, @@ -877,7 +898,7 @@ class Tester: expectedError=CtapError.ERR.MISSING_PARAMETER, ) - testMC( + self.testMC( "Send MC request with integer for clientDataHash, expect error", 5, rp, @@ -885,7 +906,7 @@ class Tester: key_params, ) - testMC( + self.testMC( "Send MC request with missing user, expect error", cdh, rp, @@ -894,7 +915,7 @@ class Tester: expectedError=CtapError.ERR.MISSING_PARAMETER, ) - testMC( + self.testMC( "Send MC request with bytearray user, expect error", cdh, rp, @@ -902,7 +923,7 @@ class Tester: key_params, ) - testMC( + self.testMC( "Send MC request with missing RP, expect error", cdh, None, @@ -911,7 +932,7 @@ class Tester: expectedError=CtapError.ERR.MISSING_PARAMETER, ) - testMC( + self.testMC( "Send MC request with bytearray RP, expect error", cdh, b"1234abcd", @@ -919,7 +940,7 @@ class Tester: key_params, ) - testMC( + self.testMC( "Send MC request with missing pubKeyCredParams, expect error", cdh, rp, @@ -928,7 +949,7 @@ class Tester: expectedError=CtapError.ERR.MISSING_PARAMETER, ) - testMC( + self.testMC( "Send MC request with incorrect pubKeyCredParams, expect error", cdh, rp, @@ -936,7 +957,7 @@ class Tester: b"2356", ) - testMC( + self.testMC( "Send MC request with incorrect excludeList, expect error", cdh, rp, @@ -945,7 +966,7 @@ class Tester: other={"exclude_list": 8}, ) - testMC( + self.testMC( "Send MC request with incorrect extensions, expect error", cdh, rp, @@ -954,7 +975,7 @@ class Tester: other={"extensions": 8}, ) - testMC( + self.testMC( "Send MC request with incorrect options, expect error", cdh, rp, @@ -963,7 +984,7 @@ class Tester: other={"options": 8}, ) - testMC( + self.testMC( "Send MC request with bad RP.name", cdh, {"id": self.host, "name": 8, "icon": "icon"}, @@ -971,7 +992,7 @@ class Tester: key_params, ) - testMC( + self.testMC( "Send MC request with bad RP.id", cdh, {"id": 8, "name": "name", "icon": "icon"}, @@ -979,7 +1000,7 @@ class Tester: key_params, ) - testMC( + self.testMC( "Send MC request with bad RP.icon", cdh, {"id": self.host, "name": "name", "icon": 8}, @@ -987,7 +1008,7 @@ class Tester: key_params, ) - testMC( + self.testMC( "Send MC request with bad user.name", cdh, rp, @@ -995,7 +1016,7 @@ class Tester: key_params, ) - testMC( + self.testMC( "Send MC request with bad user.id", cdh, rp, @@ -1003,7 +1024,7 @@ class Tester: key_params, ) - testMC( + self.testMC( "Send MC request with bad user.displayName", cdh, rp, @@ -1011,7 +1032,7 @@ class Tester: key_params, ) - testMC( + self.testMC( "Send MC request with bad user.icon", cdh, rp, @@ -1019,7 +1040,7 @@ class Tester: key_params, ) - testMC( + self.testMC( "Send MC request with non-map pubKeyCredParams item", cdh, rp, @@ -1027,7 +1048,7 @@ class Tester: ["wrong"], ) - testMC( + self.testMC( "Send MC request with pubKeyCredParams item missing type field", cdh, rp, @@ -1036,7 +1057,7 @@ class Tester: expectedError=CtapError.ERR.MISSING_PARAMETER, ) - testMC( + self.testMC( "Send MC request with pubKeyCredParams item with bad type field", cdh, rp, @@ -1044,7 +1065,7 @@ class Tester: [{"alg": ES256.ALGORITHM, "type": b"public-key"}], ) - testMC( + self.testMC( "Send MC request with pubKeyCredParams item missing alg", cdh, rp, @@ -1053,7 +1074,7 @@ class Tester: expectedError=CtapError.ERR.MISSING_PARAMETER, ) - testMC( + self.testMC( "Send MC request with pubKeyCredParams item with bad alg", cdh, rp, @@ -1061,7 +1082,7 @@ class Tester: [{"alg": "7", "type": "public-key"}], ) - testMC( + self.testMC( "Send MC request with pubKeyCredParams item with bogus alg, expect UNSUPPORTED_ALGORITHM", cdh, rp, @@ -1070,7 +1091,7 @@ class Tester: expectedError=CtapError.ERR.UNSUPPORTED_ALGORITHM, ) - testMC( + self.testMC( "Send MC request with pubKeyCredParams item with bogus type, expect UNSUPPORTED_ALGORITHM", cdh, rp, @@ -1079,7 +1100,7 @@ class Tester: expectedError=CtapError.ERR.UNSUPPORTED_ALGORITHM, ) - testMC( + self.testMC( "Send MC request with excludeList item with bogus type, expect SUCCESS", cdh, rp, @@ -1089,7 +1110,7 @@ class Tester: other={"exclude_list": [{"id": b"1234", "type": "rot13"}]}, ) - testMC( + self.testMC( "Send MC request with excludeList with bad item, expect error", cdh, rp, @@ -1098,7 +1119,7 @@ class Tester: other={"exclude_list": ["1234"]}, ) - testMC( + self.testMC( "Send MC request with excludeList with item missing type field, expect error", cdh, rp, @@ -1107,7 +1128,7 @@ class Tester: other={"exclude_list": [{"id": b"1234"}]}, ) - testMC( + self.testMC( "Send MC request with excludeList with item missing id field, expect error", cdh, rp, @@ -1116,7 +1137,7 @@ class Tester: other={"exclude_list": [{"type": "public-key"}]}, ) - testMC( + self.testMC( "Send MC request with excludeList with item containing bad id field, expect error", cdh, rp, @@ -1125,7 +1146,7 @@ class Tester: other={"exclude_list": [{"type": "public-key", "id": "1234"}]}, ) - testMC( + self.testMC( "Send MC request with excludeList with item containing bad type field, expect error", cdh, rp, @@ -1134,7 +1155,7 @@ class Tester: other={"exclude_list": [{"type": b"public-key", "id": b"1234"}]}, ) - testMC( + self.testMC( "Send MC request with excludeList containing previous registration, expect CREDENTIAL_EXCLUDED", cdh, rp, @@ -1151,7 +1172,7 @@ class Tester: expectedError=CtapError.ERR.CREDENTIAL_EXCLUDED, ) - testMC( + self.testMC( "Send MC request with unknown option, expect SUCCESS", cdh, rp, @@ -1163,7 +1184,7 @@ class Tester: if "uv" in info.options: if info.options["uv"]: - testMC( + self.testMC( "Send MC request with uv set to true, expect SUCCESS", cdh, rp, @@ -1174,7 +1195,7 @@ class Tester: ) if "up" in info.options: if info.options["up"]: - testMC( + self.testMC( "Send MC request with up set to true, expect INVALID_OPTION", cdh, rp, @@ -1184,7 +1205,7 @@ class Tester: expectedError=CtapError.ERR.INVALID_OPTION, ) - testGA( + self.testGA( "Send GA request with missing RPID, expect MISSING_PARAMETER", None, cdh, @@ -1192,14 +1213,14 @@ class Tester: expectedError=CtapError.ERR.MISSING_PARAMETER, ) - testGA( + self.testGA( "Send GA request with bad RPID, expect error", {"type": "wrong"}, cdh, allow_list, ) - testGA( + self.testGA( "Send GA request with missing clientDataHash, expect MISSING_PARAMETER", rp["id"], None, @@ -1207,28 +1228,28 @@ class Tester: expectedError=CtapError.ERR.MISSING_PARAMETER, ) - testGA( + self.testGA( "Send GA request with bad clientDataHash, expect error", rp["id"], {"type": "wrong"}, allow_list, ) - testGA( + self.testGA( "Send GA request with bad allow_list, expect error", rp["id"], cdh, {"type": "wrong"}, ) - testGA( + self.testGA( "Send GA request with bad item in allow_list, expect error", rp["id"], cdh, allow_list + ["wrong"], ) - testGA( + self.testGA( "Send GA request with unknown option, expect SUCCESS", rp["id"], cdh, @@ -1239,7 +1260,7 @@ class Tester: if "uv" in info.options: if info.options["uv"]: - res = testGA( + res = self.testGA( "Send GA request with uv set to true, expect SUCCESS", rp["id"], cdh, @@ -1251,7 +1272,7 @@ class Tester: assert res.auth_data.flags & (1 << 2) if "up" in info.options: if info.options["up"]: - res = testGA( + res = self.testGA( "Send GA request with up set to true, expect SUCCESS", rp["id"], cdh, @@ -1262,7 +1283,7 @@ class Tester: with Test("Check that UP flag is set in response"): assert res.auth_data.flags & 1 - testGA( + self.testGA( "Send GA request with bogus type item in allow_list, expect SUCCESS", rp["id"], cdh, @@ -1270,38 +1291,38 @@ class Tester: expectedError=CtapError.ERR.SUCCESS, ) - testGA( + self.testGA( "Send GA request with item missing type field in allow_list, expect error", rp["id"], cdh, allow_list + [{"id": b"1234"}], ) - testGA( + self.testGA( "Send GA request with item containing bad type field in allow_list, expect error", rp["id"], cdh, allow_list + [{"type": b"public-key", "id": b"1234"}], ) - testGA( + self.testGA( "Send GA request with item containing bad id in allow_list, expect error", rp["id"], cdh, allow_list + [{"type": b"public-key", "id": 42}], ) - testGA( + self.testGA( "Send GA request with item missing id in allow_list, expect error", rp["id"], cdh, allow_list + [{"type": b"public-key"}], ) - testReset() + self.testReset() def testRk(pin_code=None): - testGA( + self.testGA( "Send GA request with reset auth, expect NO_CREDENTIALS", rp["id"], cdh, @@ -1316,7 +1337,7 @@ class Tester: pin_token = self.client.pin_protocol.get_pin_token(pin_code) pin_auth = hmac_sha256(pin_token, cdh)[:16] - testMC( + self.testMC( "Send MC request with rk option set to true, expect SUCCESS", cdh, rp, @@ -1331,7 +1352,7 @@ class Tester: options["uv"] = False for i, x in enumerate([user1, user2, user3]): - testMC( + self.testMC( "Send MC request with rk option set to true, expect SUCCESS %d/3" % (i + 1), cdh, @@ -1342,7 +1363,7 @@ class Tester: expectedError=CtapError.ERR.SUCCESS, ) - auth1 = testGA( + auth1 = self.testGA( "Send GA request with no allow_list, expect SUCCESS", rp2["id"], cdh, @@ -1383,7 +1404,7 @@ class Tester: testRk("1234567890") # PinProtocolV1 - res = testCP( + res = self.testCP( "Test getKeyAgreement, expect SUCCESS", pin_protocol, PinProtocolV1.CMD.GET_KEY_AGREEMENT, @@ -1405,7 +1426,7 @@ class Tester: pin_token = self.client.pin_protocol.get_pin_token(pin2) pin_auth = hmac_sha256(pin_token, cdh)[:16] - res_mc = testMC( + res_mc = self.testMC( "Send MC request with new pin auth", cdh, rp, @@ -1418,7 +1439,7 @@ class Tester: with Test("Check UV flag is set"): assert res_mc.auth_data.flags & (1 << 2) - res_ga = testGA( + res_ga = self.testGA( "Send GA request with no allow_list, expect SUCCESS", rp["id"], cdh, @@ -1435,12 +1456,12 @@ class Tester: with Test("Check UV flag is set"): assert res_ga.auth_data.flags & (1 << 2) - testReset() + self.testReset() with Test("Setting pin code, expect SUCCESS"): self.client.pin_protocol.set_pin(pin1) - testReset() + self.testReset() # print("Setting pin code <4 bytes, expect POLICY_VIOLATION ") # try: @@ -1486,7 +1507,7 @@ class Tester: except CtapError as e: print(e) - res_mc = testMC( + res_mc = self.testMC( "Send MC request with no pin_auth, expect PIN_REQUIRED", cdh, rp, @@ -1495,14 +1516,14 @@ class Tester: expectedError=CtapError.ERR.PIN_REQUIRED, ) - res_mc = testGA( + res_mc = self.testGA( "Send GA request with no pin_auth, expect PIN_REQUIRED", rp["id"], cdh, expectedError=CtapError.ERR.PIN_REQUIRED, ) - res = testCP( + res = self.testCP( "Test getRetries, expect SUCCESS", pin_protocol, PinProtocolV1.CMD.GET_RETRIES, @@ -1520,7 +1541,7 @@ class Tester: pin_wrong = "".join(pin_wrong) for i in range(1, 3): - testPP( + self.testPP( "Get pin_token with wrong pin code, expect PIN_INVALID (%d/2)" % i, pin_wrong, expectedError=CtapError.ERR.PIN_INVALID, @@ -1531,7 +1552,7 @@ class Tester: print("Pass") for i in range(1, 3): - testPP( + self.testPP( "Get pin_token with wrong pin code, expect PIN_AUTH_BLOCKED %d/2" % i, pin_wrong, expectedError=CtapError.ERR.PIN_AUTH_BLOCKED, @@ -1543,7 +1564,7 @@ class Tester: pin_token = self.client.pin_protocol.get_pin_token(pin1) pin_auth = hmac_sha256(pin_token, cdh)[:16] - res_mc = testMC( + res_mc = self.testMC( "Send MC request with correct pin_auth", cdh, rp, @@ -1563,7 +1584,7 @@ class Tester: err = CtapError.ERR.PIN_AUTH_BLOCKED elif i >= 9: err = CtapError.ERR.PIN_BLOCKED - testPP( + self.testPP( "Lock out authentictor and check correct error codes %d/9" % i, pin_wrong, expectedError=err, @@ -1580,7 +1601,7 @@ class Tester: if err == CtapError.ERR.PIN_AUTH_BLOCKED: reboot() - res_mc = testMC( + res_mc = self.testMC( "Send MC request with correct pin_auth, expect PIN_BLOCKED", cdh, rp, @@ -1592,13 +1613,13 @@ class Tester: reboot() - testPP( + self.testPP( "Get pin_token with correct pin code, expect PIN_BLOCKED", pin1, expectedError=CtapError.ERR.PIN_BLOCKED, ) - testReset() + self.testReset() print("Done") @@ -1856,6 +1877,9 @@ if __name__ == "__main__": t.test_fido2() t.test_fido2_other() + if "fido2-ext" in sys.argv: + t.test_extensions() + if "rk" in sys.argv: t.test_rk() From 00d86379e5108b79edae2fbc5174e5621ce31bf1 Mon Sep 17 00:00:00 2001 From: Conor Patrick Date: Wed, 20 Mar 2019 16:21:21 -0400 Subject: [PATCH 03/20] parse full hmac-secret --- fido2/ctap.h | 39 ++++++++++++----- fido2/ctap_parse.c | 101 +++++++++++++++++++++++++++++++++++++++------ fido2/ctap_parse.h | 2 +- 3 files changed, 117 insertions(+), 25 deletions(-) diff --git a/fido2/ctap.h b/fido2/ctap.h index 797df60..c67929c 100644 --- a/fido2/ctap.h +++ b/fido2/ctap.h @@ -54,6 +54,10 @@ #define CP_getKeyAgreement 0x07 #define CP_getRetries 0x08 +#define EXT_HMAC_SECRET_COSE_KEY 0x01 +#define EXT_HMAC_SECRET_SALT_ENC 0x02 +#define EXT_HMAC_SECRET_SALT_AUTH 0x03 + #define RESP_versions 0x1 #define RESP_extensions 0x2 #define RESP_aaguid 0x3 @@ -183,7 +187,26 @@ struct rpId typedef struct { - uint8_t hmac_secret; + struct{ + uint8_t x[32]; + uint8_t y[32]; + } pubkey; + + int kty; + int crv; +} COSE_key; + +typedef struct +{ + uint8_t saltEnc[64]; + uint8_t saltAuth[32]; + COSE_key keyAgreement; +} CTAP_hmac_secret; + +typedef struct +{ + uint8_t hmac_secret_present; + CTAP_hmac_secret hmac_secret; } CTAP_extensions; typedef struct @@ -236,22 +259,16 @@ typedef struct CTAP_credentialDescriptor creds[ALLOW_LIST_MAX_SIZE]; uint8_t allowListPresent; + + CTAP_extensions extensions; + } CTAP_getAssertion; typedef struct { int pinProtocol; int subCommand; - struct - { - struct{ - uint8_t x[32]; - uint8_t y[32]; - } pubkey; - - int kty; - int crv; - } keyAgreement; + COSE_key keyAgreement; uint8_t keyAgreementPresent; uint8_t pinAuth[16]; uint8_t pinAuthPresent; diff --git a/fido2/ctap_parse.c b/fido2/ctap_parse.c index 43287e1..74845a2 100644 --- a/fido2/ctap_parse.c +++ b/fido2/ctap_parse.c @@ -556,6 +556,74 @@ uint8_t parse_options(CborValue * val, uint8_t * rk, uint8_t * uv, uint8_t * up) return 0; } +uint8_t ctap_parse_hmac_secret(CTAP_hmac_secret * hs, CborValue * val) +{ + size_t map_length; + size_t salt_len; + uint8_t parsed_count = 0; + int key; + int ret; + unsigned int i; + CborValue map; + + if (cbor_value_get_type(val) != CborMapType) + { + printf2(TAG_ERR,"error, wrong type\n"); + return CTAP2_ERR_INVALID_CBOR_TYPE; + } + + ret = cbor_value_enter_container(val,&map); + check_ret(ret); + + ret = cbor_value_get_map_length(val, &map_length); + check_ret(ret); + + for (i = 0; i < map_length; i++) + { + if (cbor_value_get_type(&map) != CborIntegerType) + { + printf2(TAG_ERR,"Error, expecting CborIntegerTypefor hmac-secret map key, got %s\n", cbor_value_get_type_string(&map)); + return CTAP2_ERR_INVALID_CBOR_TYPE; + } + ret = cbor_value_get_int(&map, &key); + check_ret(ret); + + ret = cbor_value_advance(&map); + check_ret(ret); + + switch(key) + { + case EXT_HMAC_SECRET_COSE_KEY: + ret = parse_cose_key(&map, &hs->keyAgreement); + check_retr(ret); + parsed_count++; + break; + case EXT_HMAC_SECRET_SALT_ENC: + salt_len = 64; + ret = cbor_value_copy_byte_string(&map, hs->saltEnc, &salt_len, NULL); + check_ret(ret); + parsed_count++; + break; + case EXT_HMAC_SECRET_SALT_AUTH: + salt_len = 32; + ret = cbor_value_copy_byte_string(&map, hs->saltAuth, &salt_len, NULL); + check_ret(ret); + parsed_count++; + break; + } + + if (parsed_count != 3) + { + return CTAP2_ERR_MISSING_PARAMETER; + } + + ret = cbor_value_advance(&map); + check_ret(ret); + } + return 0; +} + + uint8_t ctap_parse_extensions(CTAP_extensions * ext, CborValue * val) { CborValue map; @@ -600,14 +668,21 @@ uint8_t ctap_parse_extensions(CTAP_extensions * ext, CborValue * val) ret = cbor_value_advance(&map); check_ret(ret); - if (cbor_value_get_type(&map) == CborBooleanType) + + if (strncmp(key, "hmac-secret",11) == 0) { - if (strncmp(key, "hmac-secret",11) == 0) + if (cbor_value_get_type(&map) == CborBooleanType) { ret = cbor_value_get_boolean(&map, &b); check_ret(ret); - ext->hmac_secret = b; - printf1(TAG_CTAP, "set hmac-secret to %d\r\n", b); + ext->hmac_secret_present = b; + printf1(TAG_CTAP, "set hmac_secret_present to %d\r\n", b); + } + else if (cbor_value_get_type(&map) == CborMapType) + { + ext->hmac_secret_present = 1; + ret = ctap_parse_hmac_secret(&ext->hmac_secret, &map); + check_retr(ret); } } @@ -1003,15 +1078,15 @@ uint8_t ctap_parse_get_assertion(CTAP_getAssertion * GA, uint8_t * request, int return 0; } -uint8_t parse_cose_key(CborValue * it, uint8_t * x, uint8_t * y, int * kty, int * crv) +uint8_t parse_cose_key(CborValue * it, COSE_key * cose) { CborValue map; size_t map_length; int ret,key; unsigned int i; int xkey = 0,ykey = 0; - *kty = 0; - *crv = 0; + cose->kty = 0; + cose->crv = 0; CborType type = cbor_value_get_type(it); @@ -1049,7 +1124,7 @@ uint8_t parse_cose_key(CborValue * it, uint8_t * x, uint8_t * y, int * kty, int printf1(TAG_PARSE,"COSE_KEY_LABEL_KTY\n"); if (cbor_value_get_type(&map) == CborIntegerType) { - ret = cbor_value_get_int_checked(&map, kty); + ret = cbor_value_get_int_checked(&map, &cose->kty); check_ret(ret); } else @@ -1064,7 +1139,7 @@ uint8_t parse_cose_key(CborValue * it, uint8_t * x, uint8_t * y, int * kty, int printf1(TAG_PARSE,"COSE_KEY_LABEL_CRV\n"); if (cbor_value_get_type(&map) == CborIntegerType) { - ret = cbor_value_get_int_checked(&map, crv); + ret = cbor_value_get_int_checked(&map, &cose->crv); check_ret(ret); } else @@ -1074,14 +1149,14 @@ uint8_t parse_cose_key(CborValue * it, uint8_t * x, uint8_t * y, int * kty, int break; case COSE_KEY_LABEL_X: printf1(TAG_PARSE,"COSE_KEY_LABEL_X\n"); - ret = parse_fixed_byte_string(&map, x, 32); + ret = parse_fixed_byte_string(&map, cose->pubkey.x, 32); check_retr(ret); xkey = 1; break; case COSE_KEY_LABEL_Y: printf1(TAG_PARSE,"COSE_KEY_LABEL_Y\n"); - ret = parse_fixed_byte_string(&map, y, 32); + ret = parse_fixed_byte_string(&map, cose->pubkey.y, 32); check_retr(ret); ykey = 1; @@ -1093,7 +1168,7 @@ uint8_t parse_cose_key(CborValue * it, uint8_t * x, uint8_t * y, int * kty, int ret = cbor_value_advance(&map); check_ret(ret); } - if (xkey == 0 || ykey == 0 || *kty == 0 || *crv == 0) + if (xkey == 0 || ykey == 0 || cose->kty == 0 || cose->crv == 0) { return CTAP2_ERR_MISSING_PARAMETER; } @@ -1173,7 +1248,7 @@ uint8_t ctap_parse_client_pin(CTAP_clientPin * CP, uint8_t * request, int length break; case CP_keyAgreement: printf1(TAG_CP,"CP_keyAgreement\n"); - ret = parse_cose_key(&map, CP->keyAgreement.pubkey.x, CP->keyAgreement.pubkey.y, &CP->keyAgreement.kty, &CP->keyAgreement.crv); + ret = parse_cose_key(&map, &CP->keyAgreement); check_retr(ret); CP->keyAgreementPresent = 1; break; diff --git a/fido2/ctap_parse.h b/fido2/ctap_parse.h index 3a177af..38aa398 100644 --- a/fido2/ctap_parse.h +++ b/fido2/ctap_parse.h @@ -30,7 +30,7 @@ uint8_t parse_rp(struct rpId * rp, CborValue * val); uint8_t parse_options(CborValue * val, uint8_t * rk, uint8_t * uv, uint8_t * up); uint8_t parse_allow_list(CTAP_getAssertion * GA, CborValue * it); -uint8_t parse_cose_key(CborValue * it, uint8_t * x, uint8_t * y, int * kty, int * crv); +uint8_t parse_cose_key(CborValue * it, COSE_key * cose); uint8_t ctap_parse_make_credential(CTAP_makeCredential * MC, CborEncoder * encoder, uint8_t * request, int length); From ce3ad0e56f890bdd70324bb681f3b0cada8a72e1 Mon Sep 17 00:00:00 2001 From: Conor Patrick Date: Wed, 20 Mar 2019 16:51:58 -0400 Subject: [PATCH 04/20] bugfix --- fido2/ctap.h | 4 ++++ fido2/ctap_parse.c | 36 +++++++++++++++++++++++++----------- 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/fido2/ctap.h b/fido2/ctap.h index c67929c..06dd0fd 100644 --- a/fido2/ctap.h +++ b/fido2/ctap.h @@ -58,6 +58,9 @@ #define EXT_HMAC_SECRET_SALT_ENC 0x02 #define EXT_HMAC_SECRET_SALT_AUTH 0x03 +#define EXT_HMAC_SECRET_REQUESTED 0x01 +#define EXT_HMAC_SECRET_PARSED 0x02 + #define RESP_versions 0x1 #define RESP_extensions 0x2 #define RESP_aaguid 0x3 @@ -198,6 +201,7 @@ typedef struct typedef struct { + uint8_t salt_len; uint8_t saltEnc[64]; uint8_t saltAuth[32]; COSE_key keyAgreement; diff --git a/fido2/ctap_parse.c b/fido2/ctap_parse.c index 74845a2..0d4ece8 100644 --- a/fido2/ctap_parse.c +++ b/fido2/ctap_parse.c @@ -556,7 +556,7 @@ uint8_t parse_options(CborValue * val, uint8_t * rk, uint8_t * uv, uint8_t * up) return 0; } -uint8_t ctap_parse_hmac_secret(CTAP_hmac_secret * hs, CborValue * val) +uint8_t ctap_parse_hmac_secret(CborValue * val, CTAP_hmac_secret * hs) { size_t map_length; size_t salt_len; @@ -602,6 +602,11 @@ uint8_t ctap_parse_hmac_secret(CTAP_hmac_secret * hs, CborValue * val) salt_len = 64; ret = cbor_value_copy_byte_string(&map, hs->saltEnc, &salt_len, NULL); check_ret(ret); + if (salt_len != 32 && salt_len != 64) + { + return CTAP1_ERR_INVALID_LENGTH; + } + hs->salt_len = salt_len; parsed_count++; break; case EXT_HMAC_SECRET_SALT_AUTH: @@ -612,19 +617,21 @@ uint8_t ctap_parse_hmac_secret(CTAP_hmac_secret * hs, CborValue * val) break; } - if (parsed_count != 3) - { - return CTAP2_ERR_MISSING_PARAMETER; - } - ret = cbor_value_advance(&map); check_ret(ret); } + + if (parsed_count != 3) + { + printf2(TAG_ERR, "ctap_parse_hmac_secret missing parameter. Got %d.\r\n", parsed_count); + return CTAP2_ERR_MISSING_PARAMETER; + } + return 0; } -uint8_t ctap_parse_extensions(CTAP_extensions * ext, CborValue * val) +uint8_t ctap_parse_extensions(CborValue * val, CTAP_extensions * ext) { CborValue map; size_t sz, map_length; @@ -675,14 +682,19 @@ uint8_t ctap_parse_extensions(CTAP_extensions * ext, CborValue * val) { ret = cbor_value_get_boolean(&map, &b); check_ret(ret); - ext->hmac_secret_present = b; + if (b) ext->hmac_secret_present = EXT_HMAC_SECRET_REQUESTED; printf1(TAG_CTAP, "set hmac_secret_present to %d\r\n", b); } else if (cbor_value_get_type(&map) == CborMapType) { - ext->hmac_secret_present = 1; - ret = ctap_parse_hmac_secret(&ext->hmac_secret, &map); + ret = ctap_parse_hmac_secret(&map, &ext->hmac_secret); check_retr(ret); + ext->hmac_secret_present = EXT_HMAC_SECRET_PARSED; + printf1(TAG_CTAP, "parsed hmac_secret request\r\n"); + } + else + { + printf1(TAG_RED, "warning: hmac_secret request ignored for being wrong type\r\n"); } } @@ -801,7 +813,7 @@ uint8_t ctap_parse_make_credential(CTAP_makeCredential * MC, CborEncoder * encod { return CTAP2_ERR_INVALID_CBOR_TYPE; } - ret = ctap_parse_extensions(&MC->extensions, &map); + ret = ctap_parse_extensions(&map, &MC->extensions); check_retr(ret); break; @@ -1024,6 +1036,8 @@ uint8_t ctap_parse_get_assertion(CTAP_getAssertion * GA, uint8_t * request, int break; case GA_extensions: printf1(TAG_GA,"GA_extensions\n"); + ret = ctap_parse_extensions(&map, &GA->extensions); + check_retr(ret); break; case GA_options: From 850381a6333d117b01956c698a2d4d708f8b1805 Mon Sep 17 00:00:00 2001 From: Conor Patrick Date: Wed, 20 Mar 2019 16:52:10 -0400 Subject: [PATCH 05/20] test parsing --- tools/ctap_test.py | 36 ++++++++++++++++++++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/tools/ctap_test.py b/tools/ctap_test.py index bd7b915..3854993 100755 --- a/tools/ctap_test.py +++ b/tools/ctap_test.py @@ -25,6 +25,9 @@ from fido2.ctap2 import ES256, PinProtocolV1 from fido2.utils import Timeout, sha256, hmac_sha256 from fido2.attestation import Attestation +from cryptography.hazmat.backends import default_backend +from cryptography.hazmat.primitives.ciphers import Cipher, algorithms, modes + from solo.fido2 import force_udp_backend from solo.client import SoloClient @@ -770,18 +773,47 @@ class Tester: key_params = [{"type": "public-key", "alg": ES256.ALGORITHM}] cdh = b"123456789abcdef0123456789abcdef0" + salt1 = b"\x5a" * 32 + salt2 = b"\x96" * 32 + with Test("Get info has hmac-secret"): info = self.ctap.get_info() assert "hmac-secret" in info.extensions - self.testMC( + reg = self.testMC( "Send MC with hmac-secret ext set to true, expect SUCCESS", cdh, rp, user, key_params, expectedError=CtapError.ERR.SUCCESS, - other={"extensions": {"hmac-secret": True}}, + other={"extensions": {"hmac-secret": True}, "options": {"rk": True}}, + ) + + with Test("Get shared secret"): + key_agreement, shared_secret = ( + self.client.pin_protocol._init_shared_secret() + ) + cipher = Cipher( + algorithms.AES(shared_secret), + modes.CBC(b"\x00" * 16), + default_backend(), + ) + + enc = cipher.encryptor() + salt_enc = enc.update(salt1) + enc.finalize() + salt_auth = hmac_sha256(shared_secret, salt_enc)[:16] + + auth = self.testGA( + "Send GA request with 1 salt hmac-secret, expect success", + rp["id"], + cdh, + other={ + "extensions": { + "hmac-secret": {1: key_agreement, 2: salt_enc, 3: salt_auth} + } + }, + expectedError=CtapError.ERR.SUCCESS, ) def test_fido2_other(self,): From e8d5bc5829f8a82ce6b68f8857b301ab22e6b00a Mon Sep 17 00:00:00 2001 From: Conor Patrick Date: Wed, 20 Mar 2019 17:43:50 -0400 Subject: [PATCH 06/20] refactor ctap_make_auth_data arguments --- fido2/ctap.c | 51 +++++++++++++++++++++------------------------- fido2/ctap.h | 13 ++++++++---- fido2/ctap_parse.c | 32 ++++++++++++++--------------- 3 files changed, 48 insertions(+), 48 deletions(-) diff --git a/fido2/ctap.c b/fido2/ctap.c index ea14e62..bc42b28 100644 --- a/fido2/ctap.c +++ b/fido2/ctap.c @@ -325,7 +325,7 @@ static int is_matching_rk(CTAP_residentKey * rk, CTAP_residentKey * rk2) } -static int ctap_make_auth_data(struct rpId * rp, CborEncoder * map, uint8_t * auth_data_buf, unsigned int len, CTAP_userEntity * user, uint8_t credtype, int32_t algtype, int32_t * sz, int store) +static int ctap_make_auth_data(struct rpId * rp, CborEncoder * map, uint8_t * auth_data_buf, uint32_t * len, CTAP_credInfo * credInfo) { CborEncoder cose_key; int auth_data_sz, ret; @@ -335,7 +335,7 @@ static int ctap_make_auth_data(struct rpId * rp, CborEncoder * map, uint8_t * au uint8_t * cose_key_buf = auth_data_buf + sizeof(CTAP_authData); - if((sizeof(CTAP_authDataHeader)) > len) + if((sizeof(CTAP_authDataHeader)) > *len) { printf1(TAG_ERR,"assertion fail, auth_data_buf must be at least %d bytes\n", sizeof(CTAP_authData) - sizeof(CTAP_attestHeader)); exit(1); @@ -373,12 +373,12 @@ static int ctap_make_auth_data(struct rpId * rp, CborEncoder * map, uint8_t * au - if (credtype != 0) + if (credInfo != NULL) { // add attestedCredentialData authData->head.flags |= (1 << 6);//include attestation data - cbor_encoder_init(&cose_key, cose_key_buf, len - sizeof(CTAP_authData), 0); + cbor_encoder_init(&cose_key, cose_key_buf, *len - sizeof(CTAP_authData), 0); memmove(authData->attest.aaguid, CTAP_AAGUID, 16); authData->attest.credLenL = sizeof(CredentialId) & 0x00FF; @@ -396,10 +396,10 @@ static int ctap_make_auth_data(struct rpId * rp, CborEncoder * map, uint8_t * au make_auth_tag(authData->head.rpIdHash, authData->attest.id.nonce, count, authData->attest.id.tag); // resident key - if (store) + if (credInfo->rk) { memmove(&rk.id, &authData->attest.id, sizeof(CredentialId)); - memmove(&rk.user, user, sizeof(CTAP_userEntity)); + memmove(&rk.user, &credInfo->user, sizeof(CTAP_userEntity)); unsigned int index = STATE.rk_stored; unsigned int i; @@ -428,7 +428,7 @@ done_rk: //crypto_aes256_encrypt((uint8_t*)&authData->attest.credential.user, CREDENTIAL_ENC_SIZE); printf1(TAG_GREEN, "MADE credId: "); dump_hex1(TAG_GREEN, (uint8_t*) &authData->attest.id, sizeof(CredentialId)); - ctap_generate_cose_key(&cose_key, (uint8_t*)&authData->attest.id, sizeof(CredentialId), credtype, algtype); + ctap_generate_cose_key(&cose_key, (uint8_t*)&authData->attest.id, sizeof(CredentialId), credInfo->publicKeyCredentialType, credInfo->COSEAlgorithmIdentifier); auth_data_sz = sizeof(CTAP_authData) + cbor_encoder_get_buffer_size(&cose_key, cose_key_buf); @@ -445,7 +445,7 @@ done_rk: check_ret(ret); } - if (sz) *sz = auth_data_sz; + *len = auth_data_sz; return 0; } @@ -637,10 +637,10 @@ uint8_t ctap_make_credential(CborEncoder * encoder, uint8_t * request, int lengt CborEncoder map; ret = cbor_encoder_create_map(encoder, &map, 3); check_ret(ret); - int32_t auth_data_sz; + uint32_t auth_data_sz = sizeof(auth_data_buf); - ret = ctap_make_auth_data(&MC.rp, &map, auth_data_buf, sizeof(auth_data_buf), - &MC.user, MC.publicKeyCredentialType, MC.COSEAlgorithmIdentifier, &auth_data_sz, MC.rk); + ret = ctap_make_auth_data(&MC.rp, &map, auth_data_buf, &auth_data_sz, + &MC.credInfo); check_retr(ret); @@ -1043,27 +1043,22 @@ uint8_t ctap_get_assertion(CborEncoder * encoder, uint8_t * request, int length) else #endif { - ret = ctap_make_auth_data(&GA.rp, &map, auth_data_buf, sizeof(auth_data_buf), NULL, 0,0,NULL, 0); + uint32_t len = sizeof(auth_data_buf); + ret = ctap_make_auth_data(&GA.rp, &map, auth_data_buf, &len, NULL); check_retr(ret); } - /*for (int j = 0; j < GA.credLen; j++)*/ - /*{*/ - /*printf1(TAG_GA,"CRED ID (# %d): ", GA.creds[j].credential.enc.count);*/ - /*dump_hex1(TAG_GA, (uint8_t*)&GA.creds[j].credential, sizeof(struct Credential));*/ - /*if (ctap_authenticate_credential(&GA.rp, &GA.creds[j])) // warning encryption will break this*/ - /*{*/ - /*printf1(TAG_GA," Authenticated.\n");*/ - /*}*/ - /*else*/ - /*{*/ - /*printf1(TAG_GA," NOT authentic.\n");*/ - /*}*/ - /*}*/ + if (GA.extensions.hmac_secret_present == EXT_HMAC_SECRET_PARSED) + { + printf1(TAG_GA, "hmac-secret is present\r\n"); + // map_size += 1; + // + // ret = cbor_encode_int(&map, RESP_numberOfCredentials); + // check_ret(ret); + // ret = cbor_encode_int(&map, validCredCount); + // check_ret(ret); + } - // Decrypt here - - // if (validCredCount > 0) { save_credential_list((CTAP_authDataHeader*)auth_data_buf, GA.clientDataHash, GA.creds, validCredCount-1); // skip last one diff --git a/fido2/ctap.h b/fido2/ctap.h index 06dd0fd..d781ce6 100644 --- a/fido2/ctap.h +++ b/fido2/ctap.h @@ -213,20 +213,25 @@ typedef struct CTAP_hmac_secret hmac_secret; } CTAP_extensions; +typedef struct +{ + CTAP_userEntity user; + uint8_t publicKeyCredentialType; + int32_t COSEAlgorithmIdentifier; + uint8_t rk; +} CTAP_credInfo; + typedef struct { uint32_t paramsParsed; uint8_t clientDataHash[CLIENT_DATA_HASH_SIZE]; struct rpId rp; - CTAP_userEntity user; - uint8_t publicKeyCredentialType; - int32_t COSEAlgorithmIdentifier; + CTAP_credInfo credInfo; CborValue excludeList; size_t excludeListSize; - uint8_t rk; uint8_t uv; uint8_t up; diff --git a/fido2/ctap_parse.c b/fido2/ctap_parse.c index 0d4ece8..d0963ea 100644 --- a/fido2/ctap_parse.c +++ b/fido2/ctap_parse.c @@ -128,14 +128,14 @@ uint8_t parse_user(CTAP_makeCredential * MC, CborValue * val) } sz = USER_ID_MAX_SIZE; - ret = cbor_value_copy_byte_string(&map, MC->user.id, &sz, NULL); + ret = cbor_value_copy_byte_string(&map, MC->credInfo.user.id, &sz, NULL); if (ret == CborErrorOutOfMemory) { printf2(TAG_ERR,"Error, USER_ID is too large\n"); return CTAP2_ERR_LIMIT_EXCEEDED; } - MC->user.id_size = sz; - printf1(TAG_GREEN,"parsed id_size: %d\r\n", MC->user.id_size); + MC->credInfo.user.id_size = sz; + printf1(TAG_GREEN,"parsed id_size: %d\r\n", MC->credInfo.user.id_size); check_ret(ret); } else if (strcmp((const char *)key, "name") == 0) @@ -146,12 +146,12 @@ uint8_t parse_user(CTAP_makeCredential * MC, CborValue * val) return CTAP2_ERR_INVALID_CBOR_TYPE; } sz = USER_NAME_LIMIT; - ret = cbor_value_copy_text_string(&map, (char *)MC->user.name, &sz, NULL); + ret = cbor_value_copy_text_string(&map, (char *)MC->credInfo.user.name, &sz, NULL); if (ret != CborErrorOutOfMemory) { // Just truncate the name it's okay check_ret(ret); } - MC->user.name[USER_NAME_LIMIT - 1] = 0; + MC->credInfo.user.name[USER_NAME_LIMIT - 1] = 0; } else if (strcmp((const char *)key, "displayName") == 0) { @@ -161,12 +161,12 @@ uint8_t parse_user(CTAP_makeCredential * MC, CborValue * val) return CTAP2_ERR_INVALID_CBOR_TYPE; } sz = DISPLAY_NAME_LIMIT; - ret = cbor_value_copy_text_string(&map, (char *)MC->user.displayName, &sz, NULL); + ret = cbor_value_copy_text_string(&map, (char *)MC->credInfo.user.displayName, &sz, NULL); if (ret != CborErrorOutOfMemory) { // Just truncate the name it's okay check_ret(ret); } - MC->user.displayName[DISPLAY_NAME_LIMIT - 1] = 0; + MC->credInfo.user.displayName[DISPLAY_NAME_LIMIT - 1] = 0; } else if (strcmp((const char *)key, "icon") == 0) { @@ -176,12 +176,12 @@ uint8_t parse_user(CTAP_makeCredential * MC, CborValue * val) return CTAP2_ERR_INVALID_CBOR_TYPE; } sz = ICON_LIMIT; - ret = cbor_value_copy_text_string(&map, (char *)MC->user.icon, &sz, NULL); + ret = cbor_value_copy_text_string(&map, (char *)MC->credInfo.user.icon, &sz, NULL); if (ret != CborErrorOutOfMemory) { // Just truncate the name it's okay check_ret(ret); } - MC->user.icon[ICON_LIMIT - 1] = 0; + MC->credInfo.user.icon[ICON_LIMIT - 1] = 0; } else @@ -305,8 +305,8 @@ uint8_t parse_pub_key_cred_params(CTAP_makeCredential * MC, CborValue * val) { if (pub_key_cred_param_supported(cred_type, alg_type) == CREDENTIAL_IS_SUPPORTED) { - MC->publicKeyCredentialType = cred_type; - MC->COSEAlgorithmIdentifier = alg_type; + MC->credInfo.publicKeyCredentialType = cred_type; + MC->credInfo.COSEAlgorithmIdentifier = alg_type; MC->paramsParsed |= PARAM_pubKeyCredParams; return 0; } @@ -779,8 +779,8 @@ uint8_t ctap_parse_make_credential(CTAP_makeCredential * MC, CborEncoder * encod ret = parse_user(MC, &map); - printf1(TAG_MC," ID: "); dump_hex1(TAG_MC, MC->user.id, MC->user.id_size); - printf1(TAG_MC," name: %s\n", MC->user.name); + printf1(TAG_MC," ID: "); dump_hex1(TAG_MC, MC->credInfo.user.id, MC->credInfo.user.id_size); + printf1(TAG_MC," name: %s\n", MC->credInfo.user.name); break; case MC_pubKeyCredParams: @@ -788,8 +788,8 @@ uint8_t ctap_parse_make_credential(CTAP_makeCredential * MC, CborEncoder * encod ret = parse_pub_key_cred_params(MC, &map); - printf1(TAG_MC," cred_type: 0x%02x\n", MC->publicKeyCredentialType); - printf1(TAG_MC," alg_type: %d\n", MC->COSEAlgorithmIdentifier); + printf1(TAG_MC," cred_type: 0x%02x\n", MC->credInfo.publicKeyCredentialType); + printf1(TAG_MC," alg_type: %d\n", MC->credInfo.COSEAlgorithmIdentifier); break; case MC_excludeList: @@ -819,7 +819,7 @@ uint8_t ctap_parse_make_credential(CTAP_makeCredential * MC, CborEncoder * encod case MC_options: printf1(TAG_MC,"CTAP_options\n"); - ret = parse_options(&map, &MC->rk, &MC->uv, &MC->up); + ret = parse_options(&map, &MC->credInfo.rk, &MC->uv, &MC->up); check_retr(ret); break; case MC_pinAuth: From bb9b2ea9d4e6b2b6c3f3c8c1f323d17b4b6f028e Mon Sep 17 00:00:00 2001 From: Conor Patrick Date: Wed, 20 Mar 2019 18:10:52 -0400 Subject: [PATCH 07/20] validate saltAuth --- fido2/ctap.c | 36 +++++++++++++++++++++++++++++++++--- fido2/ctap.h | 2 +- fido2/ctap_parse.c | 2 +- 3 files changed, 35 insertions(+), 5 deletions(-) diff --git a/fido2/ctap.c b/fido2/ctap.c index bc42b28..d99f8bc 100644 --- a/fido2/ctap.c +++ b/fido2/ctap.c @@ -325,7 +325,7 @@ static int is_matching_rk(CTAP_residentKey * rk, CTAP_residentKey * rk2) } -static int ctap_make_auth_data(struct rpId * rp, CborEncoder * map, uint8_t * auth_data_buf, uint32_t * len, CTAP_credInfo * credInfo) +static int ctap_make_auth_data(struct rpId * rp, CborEncoder * map, uint8_t * auth_data_buf, uint32_t * len, CTAP_credInfo * credInfo, CTAP_extensions * ext) { CborEncoder cose_key; int auth_data_sz, ret; @@ -438,6 +438,36 @@ done_rk: auth_data_sz = sizeof(CTAP_authDataHeader); } + if (ext != NULL) + { + if (ext->hmac_secret_present == EXT_HMAC_SECRET_PARSED) + { + printf1(TAG_CTAP, "Processing hmac-secret..\r\n"); + uint8_t shared_secret[32]; + uint8_t hmac[32]; + crypto_ecc256_shared_secret((uint8_t*) &ext->hmac_secret.keyAgreement.pubkey, + KEY_AGREEMENT_PRIV, + shared_secret); + crypto_sha256_init(); + crypto_sha256_update(shared_secret, 32); + crypto_sha256_final(shared_secret); + + crypto_sha256_hmac_init(shared_secret, 32, hmac); + crypto_sha256_update(ext->hmac_secret.saltEnc, ext->hmac_secret.saltLen); + crypto_sha256_hmac_final(shared_secret, 32, hmac); + + if (memcmp(ext->hmac_secret.saltAuth, hmac, 16) == 0) + { + printf1(TAG_CTAP, "saltAuth is valid\r\n"); + } + else + { + printf1(TAG_CTAP, "saltAuth is invalid\r\n"); + return CTAP1_ERR_OTHER; + } + } + } + { ret = cbor_encode_int(map,RESP_authData); check_ret(ret); @@ -640,7 +670,7 @@ uint8_t ctap_make_credential(CborEncoder * encoder, uint8_t * request, int lengt uint32_t auth_data_sz = sizeof(auth_data_buf); ret = ctap_make_auth_data(&MC.rp, &map, auth_data_buf, &auth_data_sz, - &MC.credInfo); + &MC.credInfo,NULL); check_retr(ret); @@ -1044,7 +1074,7 @@ uint8_t ctap_get_assertion(CborEncoder * encoder, uint8_t * request, int length) #endif { uint32_t len = sizeof(auth_data_buf); - ret = ctap_make_auth_data(&GA.rp, &map, auth_data_buf, &len, NULL); + ret = ctap_make_auth_data(&GA.rp, &map, auth_data_buf, &len, NULL, &GA.extensions); check_retr(ret); } diff --git a/fido2/ctap.h b/fido2/ctap.h index d781ce6..ce46f5a 100644 --- a/fido2/ctap.h +++ b/fido2/ctap.h @@ -201,7 +201,7 @@ typedef struct typedef struct { - uint8_t salt_len; + uint8_t saltLen; uint8_t saltEnc[64]; uint8_t saltAuth[32]; COSE_key keyAgreement; diff --git a/fido2/ctap_parse.c b/fido2/ctap_parse.c index d0963ea..6cac59c 100644 --- a/fido2/ctap_parse.c +++ b/fido2/ctap_parse.c @@ -606,7 +606,7 @@ uint8_t ctap_parse_hmac_secret(CborValue * val, CTAP_hmac_secret * hs) { return CTAP1_ERR_INVALID_LENGTH; } - hs->salt_len = salt_len; + hs->saltLen = salt_len; parsed_count++; break; case EXT_HMAC_SECRET_SALT_AUTH: From 074225d87a6a822c7358a528a7c309c6aa13d75c Mon Sep 17 00:00:00 2001 From: Conor Patrick Date: Wed, 20 Mar 2019 20:03:12 -0400 Subject: [PATCH 08/20] hmac-secret fully functional --- fido2/ctap.c | 124 +++++++++++++++++++++++++++++++++++++-------------- fido2/ctap.h | 13 +++--- 2 files changed, 98 insertions(+), 39 deletions(-) diff --git a/fido2/ctap.c b/fido2/ctap.c index d99f8bc..785e828 100644 --- a/fido2/ctap.c +++ b/fido2/ctap.c @@ -327,13 +327,18 @@ static int is_matching_rk(CTAP_residentKey * rk, CTAP_residentKey * rk2) static int ctap_make_auth_data(struct rpId * rp, CborEncoder * map, uint8_t * auth_data_buf, uint32_t * len, CTAP_credInfo * credInfo, CTAP_extensions * ext) { - CborEncoder cose_key; - int auth_data_sz, ret; + CborEncoder cose_key, extensions; + + unsigned int auth_data_sz = sizeof(CTAP_authDataHeader); + unsigned int ext_encoder_buf_size; + + int ret; uint32_t count; CTAP_residentKey rk, rk2; CTAP_authData * authData = (CTAP_authData *)auth_data_buf; uint8_t * cose_key_buf = auth_data_buf + sizeof(CTAP_authData); + uint8_t * ext_encoder_buf = NULL; if((sizeof(CTAP_authDataHeader)) > *len) { @@ -433,18 +438,22 @@ done_rk: auth_data_sz = sizeof(CTAP_authData) + cbor_encoder_get_buffer_size(&cose_key, cose_key_buf); } - else - { - auth_data_sz = sizeof(CTAP_authDataHeader); - } + + ext_encoder_buf_size = *len - auth_data_sz; + ext_encoder_buf = auth_data_buf + auth_data_sz; + if (ext != NULL) { if (ext->hmac_secret_present == EXT_HMAC_SECRET_PARSED) { + authData->head.flags |= (1 << 7); + printf1(TAG_CTAP, "Processing hmac-secret..\r\n"); + uint8_t output[64]; uint8_t shared_secret[32]; uint8_t hmac[32]; + uint8_t credRandom[32]; crypto_ecc256_shared_secret((uint8_t*) &ext->hmac_secret.keyAgreement.pubkey, KEY_AGREEMENT_PRIV, shared_secret); @@ -465,6 +474,58 @@ done_rk: printf1(TAG_CTAP, "saltAuth is invalid\r\n"); return CTAP1_ERR_OTHER; } + + // Generate credRandom + crypto_sha256_hmac_init(CRYPTO_TRANSPORT_KEY, 0, credRandom); + crypto_sha256_update((uint8_t*)&ext->hmac_secret.credential->id, sizeof(CredentialId)); + crypto_sha256_hmac_final(CRYPTO_TRANSPORT_KEY, 0, credRandom); + + // Decrypt saltEnc + crypto_aes256_init(shared_secret, NULL); + crypto_aes256_decrypt(ext->hmac_secret.saltEnc, ext->hmac_secret.saltLen); + + // Generate outputs + crypto_sha256_hmac_init(credRandom, 32, output); + crypto_sha256_update(ext->hmac_secret.saltEnc, 32); + crypto_sha256_hmac_final(credRandom, 32, output); + + if (ext->hmac_secret.saltLen == 64) + { + crypto_sha256_hmac_init(credRandom, 32, output + 32); + crypto_sha256_update(ext->hmac_secret.saltEnc + 32, 32); + crypto_sha256_hmac_final(credRandom, 32, output + 32); + } + + // Encrypt for final output + crypto_aes256_init(shared_secret, NULL); + crypto_aes256_encrypt(output, ext->hmac_secret.saltLen); + + // output + cbor_encoder_init(&extensions, ext_encoder_buf, ext_encoder_buf_size, 0); + printf1(TAG_GREEN, "have %d bytes for Extenstions encoder\r\n",ext_encoder_buf_size); + CborEncoder ext_map; + CborEncoder hmac_secret_map; + ret = cbor_encoder_create_map(&extensions, &ext_map, 1); + check_ret(ret); + { + ret = cbor_encode_int(&ext_map,GA_extensions); + check_ret(ret); + CborEncoder hmac_secret_map; + ret = cbor_encoder_create_map(&ext_map, &hmac_secret_map, 1); + check_ret(ret); + { + ret = cbor_encode_text_stringz(&hmac_secret_map, "hmac-secret"); + check_ret(ret); + + ret = cbor_encode_byte_string(&hmac_secret_map, output, ext->hmac_secret.saltLen); + check_ret(ret); + } + ret = cbor_encoder_close_container(&ext_map, &hmac_secret_map); + check_ret(ret); + } + ret = cbor_encoder_close_container(&extensions, &ext_map); + check_ret(ret); + auth_data_sz += cbor_encoder_get_buffer_size(&extensions, ext_encoder_buf); } } @@ -1012,7 +1073,7 @@ uint8_t ctap_get_next_assertion(CborEncoder * encoder) uint8_t ctap_get_assertion(CborEncoder * encoder, uint8_t * request, int length) { CTAP_getAssertion GA; - uint8_t auth_data_buf[sizeof(CTAP_authDataHeader)]; + uint8_t auth_data_buf[sizeof(CTAP_authDataHeader) + 128]; int ret = ctap_parse_get_assertion(&GA,request,length); if (ret != 0) @@ -1058,37 +1119,14 @@ uint8_t ctap_get_assertion(CborEncoder * encoder, uint8_t * request, int length) map_size += 1; } - ret = cbor_encoder_create_map(encoder, &map, map_size); - check_ret(ret); - -#ifdef ENABLE_U2F_EXTENSIONS - if ( is_extension_request((uint8_t*)&GA.creds[validCredCount - 1].credential.id, sizeof(CredentialId)) ) - { - ret = cbor_encode_int(&map,RESP_authData); - check_ret(ret); - memset(auth_data_buf,0,sizeof(auth_data_buf)); - ret = cbor_encode_byte_string(&map, auth_data_buf, sizeof(auth_data_buf)); - check_ret(ret); - } - else -#endif - { - uint32_t len = sizeof(auth_data_buf); - ret = ctap_make_auth_data(&GA.rp, &map, auth_data_buf, &len, NULL, &GA.extensions); - check_retr(ret); - } - if (GA.extensions.hmac_secret_present == EXT_HMAC_SECRET_PARSED) { printf1(TAG_GA, "hmac-secret is present\r\n"); - // map_size += 1; - // - // ret = cbor_encode_int(&map, RESP_numberOfCredentials); - // check_ret(ret); - // ret = cbor_encode_int(&map, validCredCount); - // check_ret(ret); } + ret = cbor_encoder_create_map(encoder, &map, map_size); + check_ret(ret); + if (validCredCount > 0) { save_credential_list((CTAP_authDataHeader*)auth_data_buf, GA.clientDataHash, GA.creds, validCredCount-1); // skip last one @@ -1123,6 +1161,26 @@ uint8_t ctap_get_assertion(CborEncoder * encoder, uint8_t * request, int length) CTAP_credentialDescriptor * cred = &GA.creds[validCredCount - 1]; + GA.extensions.hmac_secret.credential = &cred->credential; + +#ifdef ENABLE_U2F_EXTENSIONS + if ( is_extension_request((uint8_t*)&GA.creds[validCredCount - 1].credential.id, sizeof(CredentialId)) ) + { + ret = cbor_encode_int(&map,RESP_authData); + check_ret(ret); + memset(auth_data_buf,0,sizeof(auth_data_buf)); + ret = cbor_encode_byte_string(&map, auth_data_buf, sizeof(auth_data_buf)); + check_ret(ret); + } + else +#endif + { + uint32_t len = sizeof(auth_data_buf); + ret = ctap_make_auth_data(&GA.rp, &map, auth_data_buf, &len, NULL, &GA.extensions); + check_retr(ret); + } + + ret = ctap_end_get_assertion(&map, cred, auth_data_buf, GA.clientDataHash, add_user_info); check_retr(ret); diff --git a/fido2/ctap.h b/fido2/ctap.h index ce46f5a..53a5d80 100644 --- a/fido2/ctap.h +++ b/fido2/ctap.h @@ -149,9 +149,13 @@ struct Credential { CredentialId id; CTAP_userEntity user; }; - typedef struct Credential CTAP_residentKey; +typedef struct +{ + uint8_t type; + struct Credential credential; +} CTAP_credentialDescriptor; typedef struct { @@ -205,6 +209,7 @@ typedef struct uint8_t saltEnc[64]; uint8_t saltAuth[32]; COSE_key keyAgreement; + struct Credential * credential; } CTAP_hmac_secret; typedef struct @@ -242,11 +247,7 @@ typedef struct } CTAP_makeCredential; -typedef struct -{ - uint8_t type; - struct Credential credential; -} CTAP_credentialDescriptor; + typedef struct { From e22e636475515139f0cb910a0301900aa307e452 Mon Sep 17 00:00:00 2001 From: Conor Patrick Date: Wed, 20 Mar 2019 20:03:25 -0400 Subject: [PATCH 09/20] hmac-secret tested --- tools/ctap_test.py | 77 +++++++++++++++++++++++++++++++++------------- 1 file changed, 56 insertions(+), 21 deletions(-) diff --git a/tools/ctap_test.py b/tools/ctap_test.py index 3854993..133dfdf 100755 --- a/tools/ctap_test.py +++ b/tools/ctap_test.py @@ -47,6 +47,17 @@ def VerifyAttestation(attest, data): verifier().verify(attest.att_statement, attest.auth_data, data.hash) +def shannon_entropy(data): + sum = 0.0 + total = len(data) + for x in range(0, 256): + freq = data.count(x) + p = freq / total + if p > 0: + sum -= p * math.log2(p) + return sum + + class Packet(object): def __init__(self, data): l = len(data) @@ -776,6 +787,8 @@ class Tester: salt1 = b"\x5a" * 32 salt2 = b"\x96" * 32 + self.testReset() + with Test("Get info has hmac-secret"): info = self.ctap.get_info() assert "hmac-secret" in info.extensions @@ -800,21 +813,49 @@ class Tester: default_backend(), ) - enc = cipher.encryptor() - salt_enc = enc.update(salt1) + enc.finalize() - salt_auth = hmac_sha256(shared_secret, salt_enc)[:16] + for salt_list in ((salt1,), (salt1, salt2)): + enc = cipher.encryptor() + salt_enc = b"" + for salt in salt_list: + salt_enc += enc.update(salt) + salt_enc += enc.finalize() - auth = self.testGA( - "Send GA request with 1 salt hmac-secret, expect success", - rp["id"], - cdh, - other={ - "extensions": { - "hmac-secret": {1: key_agreement, 2: salt_enc, 3: salt_auth} - } - }, - expectedError=CtapError.ERR.SUCCESS, - ) + salt_auth = hmac_sha256(shared_secret, salt_enc)[:16] + + auth = self.testGA( + "Send GA request with %d salts hmac-secret, expect success" + % len(salt_list), + rp["id"], + cdh, + other={ + "extensions": { + "hmac-secret": {1: key_agreement, 2: salt_enc, 3: salt_auth} + } + }, + expectedError=CtapError.ERR.SUCCESS, + ) + + with Test( + "Check that hmac-secret is in auth_data extensions and has %d bytes" + % (len(salt_list) * 32) + ): + ext = auth.auth_data.extensions + assert ext + assert "hmac-secret" in ext[4] + assert type(ext[4]["hmac-secret"]) == type(b"") + assert len(ext[4]["hmac-secret"]) == len(salt_list) * 32 + + with Test("Check that shannon_entropy of hmac-secret is good"): + ext = auth.auth_data.extensions + dec = cipher.decryptor() + key = dec.update(ext[4]["hmac-secret"]) + dec.finalize() + + if len(salt_list) == 1: + assert shannon_entropy(ext[4]["hmac-secret"]) > 4.6 + assert shannon_entropy(key) > 4.6 + if len(salt_list) == 2: + assert shannon_entropy(ext[4]["hmac-secret"]) > 5.6 + assert shannon_entropy(key) > 5.6 def test_fido2_other(self,): @@ -1743,15 +1784,9 @@ class Tester: entropy = b"" while len(entropy) < total: entropy += sc.get_rng() - total = len(entropy) with Test("Test entropy is close to perfect"): - sum = 0.0 - for x in range(0, 256): - freq = entropy.count(x) - p = freq / total - sum -= p * math.log2(p) - assert sum > 7.98 + assert shannon_entropy(entropy) > 7.98 print("Entropy is %.5f bits per byte." % sum) with Test("Test Solo version command"): From b62e9906c70910a9a9125bc9e1b28931be3e51c4 Mon Sep 17 00:00:00 2001 From: Conor Patrick Date: Wed, 20 Mar 2019 20:13:16 -0400 Subject: [PATCH 10/20] make new function --- fido2/ctap.c | 184 ++++++++++++++++++++++++++++----------------------- 1 file changed, 102 insertions(+), 82 deletions(-) diff --git a/fido2/ctap.c b/fido2/ctap.c index 785e828..7174635 100644 --- a/fido2/ctap.c +++ b/fido2/ctap.c @@ -324,10 +324,103 @@ static int is_matching_rk(CTAP_residentKey * rk, CTAP_residentKey * rk2) (rk->user.id_size == rk2->user.id_size); } +static int ctap_make_extensions(CTAP_extensions * ext, uint8_t * ext_encoder_buf, unsigned int * ext_encoder_buf_size) +{ + CborEncoder extensions; + int ret; + uint8_t output[64]; + uint8_t shared_secret[32]; + uint8_t hmac[32]; + uint8_t credRandom[32]; + + if (ext->hmac_secret_present == EXT_HMAC_SECRET_PARSED) + { + printf1(TAG_CTAP, "Processing hmac-secret..\r\n"); + + crypto_ecc256_shared_secret((uint8_t*) &ext->hmac_secret.keyAgreement.pubkey, + KEY_AGREEMENT_PRIV, + shared_secret); + crypto_sha256_init(); + crypto_sha256_update(shared_secret, 32); + crypto_sha256_final(shared_secret); + + crypto_sha256_hmac_init(shared_secret, 32, hmac); + crypto_sha256_update(ext->hmac_secret.saltEnc, ext->hmac_secret.saltLen); + crypto_sha256_hmac_final(shared_secret, 32, hmac); + + if (memcmp(ext->hmac_secret.saltAuth, hmac, 16) == 0) + { + printf1(TAG_CTAP, "saltAuth is valid\r\n"); + } + else + { + printf1(TAG_CTAP, "saltAuth is invalid\r\n"); + return CTAP1_ERR_OTHER; + } + + // Generate credRandom + crypto_sha256_hmac_init(CRYPTO_TRANSPORT_KEY, 0, credRandom); + crypto_sha256_update((uint8_t*)&ext->hmac_secret.credential->id, sizeof(CredentialId)); + crypto_sha256_hmac_final(CRYPTO_TRANSPORT_KEY, 0, credRandom); + + // Decrypt saltEnc + crypto_aes256_init(shared_secret, NULL); + crypto_aes256_decrypt(ext->hmac_secret.saltEnc, ext->hmac_secret.saltLen); + + // Generate outputs + crypto_sha256_hmac_init(credRandom, 32, output); + crypto_sha256_update(ext->hmac_secret.saltEnc, 32); + crypto_sha256_hmac_final(credRandom, 32, output); + + if (ext->hmac_secret.saltLen == 64) + { + crypto_sha256_hmac_init(credRandom, 32, output + 32); + crypto_sha256_update(ext->hmac_secret.saltEnc + 32, 32); + crypto_sha256_hmac_final(credRandom, 32, output + 32); + } + + // Encrypt for final output + crypto_aes256_init(shared_secret, NULL); + crypto_aes256_encrypt(output, ext->hmac_secret.saltLen); + + // output + cbor_encoder_init(&extensions, ext_encoder_buf, *ext_encoder_buf_size, 0); + printf1(TAG_GREEN, "have %d bytes for Extenstions encoder\r\n",*ext_encoder_buf_size); + CborEncoder ext_map; + CborEncoder hmac_secret_map; + ret = cbor_encoder_create_map(&extensions, &ext_map, 1); + check_ret(ret); + { + ret = cbor_encode_int(&ext_map,GA_extensions); + check_ret(ret); + CborEncoder hmac_secret_map; + ret = cbor_encoder_create_map(&ext_map, &hmac_secret_map, 1); + check_ret(ret); + { + ret = cbor_encode_text_stringz(&hmac_secret_map, "hmac-secret"); + check_ret(ret); + + ret = cbor_encode_byte_string(&hmac_secret_map, output, ext->hmac_secret.saltLen); + check_ret(ret); + } + ret = cbor_encoder_close_container(&ext_map, &hmac_secret_map); + check_ret(ret); + } + ret = cbor_encoder_close_container(&extensions, &ext_map); + check_ret(ret); + *ext_encoder_buf_size = cbor_encoder_get_buffer_size(&extensions, ext_encoder_buf); + } + else + { + *ext_encoder_buf_size = 0; + } + return 0; +} + static int ctap_make_auth_data(struct rpId * rp, CborEncoder * map, uint8_t * auth_data_buf, uint32_t * len, CTAP_credInfo * credInfo, CTAP_extensions * ext) { - CborEncoder cose_key, extensions; + CborEncoder cose_key; unsigned int auth_data_sz = sizeof(CTAP_authDataHeader); unsigned int ext_encoder_buf_size; @@ -439,94 +532,21 @@ done_rk: } - ext_encoder_buf_size = *len - auth_data_sz; - ext_encoder_buf = auth_data_buf + auth_data_sz; if (ext != NULL) { - if (ext->hmac_secret_present == EXT_HMAC_SECRET_PARSED) + ext_encoder_buf_size = *len - auth_data_sz; + ext_encoder_buf = auth_data_buf + auth_data_sz; + + ret = ctap_make_extensions(ext, ext_encoder_buf, &ext_encoder_buf_size); + check_retr(ret); + if (ext_encoder_buf_size) { authData->head.flags |= (1 << 7); - - printf1(TAG_CTAP, "Processing hmac-secret..\r\n"); - uint8_t output[64]; - uint8_t shared_secret[32]; - uint8_t hmac[32]; - uint8_t credRandom[32]; - crypto_ecc256_shared_secret((uint8_t*) &ext->hmac_secret.keyAgreement.pubkey, - KEY_AGREEMENT_PRIV, - shared_secret); - crypto_sha256_init(); - crypto_sha256_update(shared_secret, 32); - crypto_sha256_final(shared_secret); - - crypto_sha256_hmac_init(shared_secret, 32, hmac); - crypto_sha256_update(ext->hmac_secret.saltEnc, ext->hmac_secret.saltLen); - crypto_sha256_hmac_final(shared_secret, 32, hmac); - - if (memcmp(ext->hmac_secret.saltAuth, hmac, 16) == 0) - { - printf1(TAG_CTAP, "saltAuth is valid\r\n"); - } - else - { - printf1(TAG_CTAP, "saltAuth is invalid\r\n"); - return CTAP1_ERR_OTHER; - } - - // Generate credRandom - crypto_sha256_hmac_init(CRYPTO_TRANSPORT_KEY, 0, credRandom); - crypto_sha256_update((uint8_t*)&ext->hmac_secret.credential->id, sizeof(CredentialId)); - crypto_sha256_hmac_final(CRYPTO_TRANSPORT_KEY, 0, credRandom); - - // Decrypt saltEnc - crypto_aes256_init(shared_secret, NULL); - crypto_aes256_decrypt(ext->hmac_secret.saltEnc, ext->hmac_secret.saltLen); - - // Generate outputs - crypto_sha256_hmac_init(credRandom, 32, output); - crypto_sha256_update(ext->hmac_secret.saltEnc, 32); - crypto_sha256_hmac_final(credRandom, 32, output); - - if (ext->hmac_secret.saltLen == 64) - { - crypto_sha256_hmac_init(credRandom, 32, output + 32); - crypto_sha256_update(ext->hmac_secret.saltEnc + 32, 32); - crypto_sha256_hmac_final(credRandom, 32, output + 32); - } - - // Encrypt for final output - crypto_aes256_init(shared_secret, NULL); - crypto_aes256_encrypt(output, ext->hmac_secret.saltLen); - - // output - cbor_encoder_init(&extensions, ext_encoder_buf, ext_encoder_buf_size, 0); - printf1(TAG_GREEN, "have %d bytes for Extenstions encoder\r\n",ext_encoder_buf_size); - CborEncoder ext_map; - CborEncoder hmac_secret_map; - ret = cbor_encoder_create_map(&extensions, &ext_map, 1); - check_ret(ret); - { - ret = cbor_encode_int(&ext_map,GA_extensions); - check_ret(ret); - CborEncoder hmac_secret_map; - ret = cbor_encoder_create_map(&ext_map, &hmac_secret_map, 1); - check_ret(ret); - { - ret = cbor_encode_text_stringz(&hmac_secret_map, "hmac-secret"); - check_ret(ret); - - ret = cbor_encode_byte_string(&hmac_secret_map, output, ext->hmac_secret.saltLen); - check_ret(ret); - } - ret = cbor_encoder_close_container(&ext_map, &hmac_secret_map); - check_ret(ret); - } - ret = cbor_encoder_close_container(&extensions, &ext_map); - check_ret(ret); - auth_data_sz += cbor_encoder_get_buffer_size(&extensions, ext_encoder_buf); + auth_data_sz += ext_encoder_buf_size; } + } { From 2d233f164e64899a2eb8b36897832668072165ca Mon Sep 17 00:00:00 2001 From: Conor Patrick Date: Wed, 20 Mar 2019 21:03:03 -0400 Subject: [PATCH 11/20] small bug fixes --- fido2/ctap.c | 23 +++++++---------------- tools/ctap_test.py | 3 ++- 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/fido2/ctap.c b/fido2/ctap.c index 7174635..9ef99ad 100644 --- a/fido2/ctap.c +++ b/fido2/ctap.c @@ -33,7 +33,6 @@ static int8_t PIN_BOOT_ATTEMPTS_LEFT = PIN_BOOT_ATTEMPTS; AuthenticatorState STATE; - static void ctap_reset_key_agreement(); static struct { @@ -470,7 +469,6 @@ static int ctap_make_auth_data(struct rpId * rp, CborEncoder * map, uint8_t * au authData->head.flags |= (ctap_is_pin_set() << 2); - if (credInfo != NULL) { // add attestedCredentialData @@ -521,9 +519,6 @@ static int ctap_make_auth_data(struct rpId * rp, CborEncoder * map, uint8_t * au } done_rk: - // DELETE - //crypto_aes256_init(CRYPTO_TRANSPORT_KEY, NULL); - //crypto_aes256_encrypt((uint8_t*)&authData->attest.credential.user, CREDENTIAL_ENC_SIZE); printf1(TAG_GREEN, "MADE credId: "); dump_hex1(TAG_GREEN, (uint8_t*) &authData->attest.id, sizeof(CredentialId)); ctap_generate_cose_key(&cose_key, (uint8_t*)&authData->attest.id, sizeof(CredentialId), credInfo->publicKeyCredentialType, credInfo->COSEAlgorithmIdentifier); @@ -532,8 +527,6 @@ done_rk: } - - if (ext != NULL) { ext_encoder_buf_size = *len - auth_data_sz; @@ -976,6 +969,7 @@ static void save_credential_list(CTAP_authDataHeader * head, uint8_t * clientDat memmove(getAssertionState.clientDataHash, clientDataHash, CLIENT_DATA_HASH_SIZE); memmove(&getAssertionState.authData, head, sizeof(CTAP_authDataHeader)); memmove(getAssertionState.creds, creds, sizeof(CTAP_credentialDescriptor) * (count)); + } getAssertionState.count = count; printf1(TAG_GA,"saved %d credentials\n",count); @@ -1040,7 +1034,6 @@ uint8_t ctap_get_next_assertion(CborEncoder * encoder) CborEncoder map; CTAP_authDataHeader authData; memmove(&authData, &getAssertionState.authData, sizeof(CTAP_authDataHeader)); - // CTAP_authDataHeader * authData = &getAssertionState.authData; CTAP_credentialDescriptor * cred = pop_credential(); @@ -1063,6 +1056,7 @@ uint8_t ctap_get_next_assertion(CborEncoder * encoder) ret = cbor_encoder_create_map(encoder, &map, 3); } + check_ret(ret); printf1(TAG_RED, "RPID hash: "); dump_hex1(TAG_RED, authData.rpIdHash, 32); @@ -1073,6 +1067,7 @@ uint8_t ctap_get_next_assertion(CborEncoder * encoder) check_ret(ret); } + // if only one account for this RP, null out the user details if (!getAssertionState.user_verified) { @@ -1147,11 +1142,7 @@ uint8_t ctap_get_assertion(CborEncoder * encoder, uint8_t * request, int length) ret = cbor_encoder_create_map(encoder, &map, map_size); check_ret(ret); - if (validCredCount > 0) - { - save_credential_list((CTAP_authDataHeader*)auth_data_buf, GA.clientDataHash, GA.creds, validCredCount-1); // skip last one - } - else + if (validCredCount == 0) { printf2(TAG_ERR,"Error, no authentic credential\n"); return CTAP2_ERR_NO_CREDENTIALS; @@ -1188,8 +1179,8 @@ uint8_t ctap_get_assertion(CborEncoder * encoder, uint8_t * request, int length) { ret = cbor_encode_int(&map,RESP_authData); check_ret(ret); - memset(auth_data_buf,0,sizeof(auth_data_buf)); - ret = cbor_encode_byte_string(&map, auth_data_buf, sizeof(auth_data_buf)); + memset(auth_data_buf,0,sizeof(CTAP_authDataHeader)); + ret = cbor_encode_byte_string(&map, auth_data_buf, sizeof(CTAP_authDataHeader)); check_ret(ret); } else @@ -1200,6 +1191,7 @@ uint8_t ctap_get_assertion(CborEncoder * encoder, uint8_t * request, int length) check_retr(ret); } + save_credential_list((CTAP_authDataHeader*)auth_data_buf, GA.clientDataHash, GA.creds, validCredCount-1); // skip last one ret = ctap_end_get_assertion(&map, cred, auth_data_buf, GA.clientDataHash, add_user_info); check_retr(ret); @@ -1522,7 +1514,6 @@ uint8_t ctap_request(uint8_t * pkt_raw, int length, CTAP_RESPONSE * resp) pkt_raw++; length--; - uint8_t * buf = resp->data; cbor_encoder_init(&encoder, buf, resp->data_size, 0); diff --git a/tools/ctap_test.py b/tools/ctap_test.py index 133dfdf..126e51a 100755 --- a/tools/ctap_test.py +++ b/tools/ctap_test.py @@ -1786,7 +1786,8 @@ class Tester: entropy += sc.get_rng() with Test("Test entropy is close to perfect"): - assert shannon_entropy(entropy) > 7.98 + sum = shannon_entropy(entropy) + assert sum > 7.98 print("Entropy is %.5f bits per byte." % sum) with Test("Test Solo version command"): From dbe5283e1f0eb1fef42f18ba8f1b926616b78260 Mon Sep 17 00:00:00 2001 From: Conor Patrick Date: Wed, 20 Mar 2019 21:06:18 -0400 Subject: [PATCH 12/20] test solo commands on fido2 layer --- tools/ctap_test.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tools/ctap_test.py b/tools/ctap_test.py index 126e51a..7b42197 100755 --- a/tools/ctap_test.py +++ b/tools/ctap_test.py @@ -1799,6 +1799,11 @@ class Tester: except ApduError: pass + sc.exchange = sc.exchange_fido2 + with Test("Test Solo version and random commands with fido2 layer"): + assert len(sc.solo_version()) == 3 + sc.get_rng() + def test_bootloader(self,): sc = SoloClient() sc.find_device(self.dev) From 142d4002e5b1d47611679c26e9e3274dba20989b Mon Sep 17 00:00:00 2001 From: Conor Patrick Date: Wed, 20 Mar 2019 23:14:17 -0400 Subject: [PATCH 13/20] remove warning, reduce memory --- fido2/ctap.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fido2/ctap.c b/fido2/ctap.c index 9ef99ad..7b506ed 100644 --- a/fido2/ctap.c +++ b/fido2/ctap.c @@ -386,7 +386,6 @@ static int ctap_make_extensions(CTAP_extensions * ext, uint8_t * ext_encoder_buf cbor_encoder_init(&extensions, ext_encoder_buf, *ext_encoder_buf_size, 0); printf1(TAG_GREEN, "have %d bytes for Extenstions encoder\r\n",*ext_encoder_buf_size); CborEncoder ext_map; - CborEncoder hmac_secret_map; ret = cbor_encoder_create_map(&extensions, &ext_map, 1); check_ret(ret); { @@ -1088,7 +1087,7 @@ uint8_t ctap_get_next_assertion(CborEncoder * encoder) uint8_t ctap_get_assertion(CborEncoder * encoder, uint8_t * request, int length) { CTAP_getAssertion GA; - uint8_t auth_data_buf[sizeof(CTAP_authDataHeader) + 128]; + uint8_t auth_data_buf[sizeof(CTAP_authDataHeader) + 100]; int ret = ctap_parse_get_assertion(&GA,request,length); if (ret != 0) From 946e932b1e6feff3414367b93d82608aa6c0b074 Mon Sep 17 00:00:00 2001 From: Conor Patrick Date: Wed, 20 Mar 2019 23:28:45 -0400 Subject: [PATCH 14/20] refactor to use less ram --- fido2/ctap.c | 56 ++++++++++++++------------ targets/stm32l432/build/application.mk | 2 +- 2 files changed, 31 insertions(+), 27 deletions(-) diff --git a/fido2/ctap.c b/fido2/ctap.c index 7b506ed..024d883 100644 --- a/fido2/ctap.c +++ b/fido2/ctap.c @@ -416,20 +416,16 @@ static int ctap_make_extensions(CTAP_extensions * ext, uint8_t * ext_encoder_buf } -static int ctap_make_auth_data(struct rpId * rp, CborEncoder * map, uint8_t * auth_data_buf, uint32_t * len, CTAP_credInfo * credInfo, CTAP_extensions * ext) +static int ctap_make_auth_data(struct rpId * rp, CborEncoder * map, uint8_t * auth_data_buf, uint32_t * len, CTAP_credInfo * credInfo) { CborEncoder cose_key; unsigned int auth_data_sz = sizeof(CTAP_authDataHeader); - unsigned int ext_encoder_buf_size; - - int ret; uint32_t count; CTAP_residentKey rk, rk2; CTAP_authData * authData = (CTAP_authData *)auth_data_buf; uint8_t * cose_key_buf = auth_data_buf + sizeof(CTAP_authData); - uint8_t * ext_encoder_buf = NULL; if((sizeof(CTAP_authDataHeader)) > *len) { @@ -526,27 +522,9 @@ done_rk: } - if (ext != NULL) - { - ext_encoder_buf_size = *len - auth_data_sz; - ext_encoder_buf = auth_data_buf + auth_data_sz; - ret = ctap_make_extensions(ext, ext_encoder_buf, &ext_encoder_buf_size); - check_retr(ret); - if (ext_encoder_buf_size) - { - authData->head.flags |= (1 << 7); - auth_data_sz += ext_encoder_buf_size; - } - } - { - ret = cbor_encode_int(map,RESP_authData); - check_ret(ret); - ret = cbor_encode_byte_string(map, auth_data_buf, auth_data_sz); - check_ret(ret); - } *len = auth_data_sz; return 0; @@ -743,10 +721,16 @@ uint8_t ctap_make_credential(CborEncoder * encoder, uint8_t * request, int lengt uint32_t auth_data_sz = sizeof(auth_data_buf); ret = ctap_make_auth_data(&MC.rp, &map, auth_data_buf, &auth_data_sz, - &MC.credInfo,NULL); - + &MC.credInfo); check_retr(ret); + { + ret = cbor_encode_int(&map,RESP_authData); + check_ret(ret); + ret = cbor_encode_byte_string(&map, auth_data_buf, auth_data_sz); + check_ret(ret); + } + crypto_ecc256_load_attestation_key(); int sigder_sz = ctap_calculate_signature(auth_data_buf, auth_data_sz, MC.clientDataHash, auth_data_buf, sigbuf, sigder); printf1(TAG_MC,"der sig [%d]: ", sigder_sz); dump_hex1(TAG_MC, sigder, sigder_sz); @@ -1186,8 +1170,28 @@ uint8_t ctap_get_assertion(CborEncoder * encoder, uint8_t * request, int length) #endif { uint32_t len = sizeof(auth_data_buf); - ret = ctap_make_auth_data(&GA.rp, &map, auth_data_buf, &len, NULL, &GA.extensions); + ret = ctap_make_auth_data(&GA.rp, &map, auth_data_buf, &len, NULL); check_retr(ret); + + { + unsigned int ext_encoder_buf_size = sizeof(auth_data_buf) - len; + uint8_t * ext_encoder_buf = auth_data_buf + len; + + ret = ctap_make_extensions(&GA.extensions, ext_encoder_buf, &ext_encoder_buf_size); + check_retr(ret); + if (ext_encoder_buf_size) + { + ((CTAP_authData *)auth_data_buf)->head.flags |= (1 << 7); + len += ext_encoder_buf_size; + } + } + + { + ret = cbor_encode_int(&map,RESP_authData); + check_ret(ret); + ret = cbor_encode_byte_string(&map, auth_data_buf, len); + check_ret(ret); + } } save_credential_list((CTAP_authDataHeader*)auth_data_buf, GA.clientDataHash, GA.creds, validCredCount-1); // skip last one diff --git a/targets/stm32l432/build/application.mk b/targets/stm32l432/build/application.mk index 1dfa8b8..c64e9cf 100644 --- a/targets/stm32l432/build/application.mk +++ b/targets/stm32l432/build/application.mk @@ -46,7 +46,7 @@ DEFINES = -DDEBUG_LEVEL=$(DEBUG) -D$(CHIP) -DAES256=1 -DUSE_FULL_LL_DRIVER -DAP CFLAGS=$(INC) -c $(DEFINES) -Wall -Wextra -Wno-unused-parameter -Wno-missing-field-initializers -fdata-sections -ffunction-sections \ -fomit-frame-pointer $(HW) -g $(VERSION_FLAGS) -LDFLAGS_LIB=$(HW) $(SEARCH) -specs=nano.specs -specs=nosys.specs -Wl,--gc-sections -u _printf_float -lnosys +LDFLAGS_LIB=$(HW) $(SEARCH) -specs=nano.specs -specs=nosys.specs -Wl,--gc-sections -lnosys LDFLAGS=$(HW) $(LDFLAGS_LIB) -T$(LDSCRIPT) -Wl,-Map=$(TARGET).map,--cref -Wl,-Bstatic -ltinycbor ECC_CFLAGS = $(CFLAGS) -DuECC_PLATFORM=5 -DuECC_OPTIMIZATION_LEVEL=4 -DuECC_SQUARE_FUNC=1 -DuECC_SUPPORT_COMPRESSED_POINT=0 From 3a48756f961c6e008f6946019912b6596ad22697 Mon Sep 17 00:00:00 2001 From: Conor Patrick Date: Wed, 20 Mar 2019 23:40:58 -0400 Subject: [PATCH 15/20] remove extra layer of map --- fido2/ctap.c | 15 ++++----------- tools/ctap_test.py | 14 +++++++------- 2 files changed, 11 insertions(+), 18 deletions(-) diff --git a/fido2/ctap.c b/fido2/ctap.c index 024d883..af39f67 100644 --- a/fido2/ctap.c +++ b/fido2/ctap.c @@ -383,16 +383,11 @@ static int ctap_make_extensions(CTAP_extensions * ext, uint8_t * ext_encoder_buf crypto_aes256_encrypt(output, ext->hmac_secret.saltLen); // output - cbor_encoder_init(&extensions, ext_encoder_buf, *ext_encoder_buf_size, 0); printf1(TAG_GREEN, "have %d bytes for Extenstions encoder\r\n",*ext_encoder_buf_size); - CborEncoder ext_map; - ret = cbor_encoder_create_map(&extensions, &ext_map, 1); - check_ret(ret); + cbor_encoder_init(&extensions, ext_encoder_buf, *ext_encoder_buf_size, 0); { - ret = cbor_encode_int(&ext_map,GA_extensions); - check_ret(ret); CborEncoder hmac_secret_map; - ret = cbor_encoder_create_map(&ext_map, &hmac_secret_map, 1); + ret = cbor_encoder_create_map(&extensions, &hmac_secret_map, 1); check_ret(ret); { ret = cbor_encode_text_stringz(&hmac_secret_map, "hmac-secret"); @@ -401,11 +396,9 @@ static int ctap_make_extensions(CTAP_extensions * ext, uint8_t * ext_encoder_buf ret = cbor_encode_byte_string(&hmac_secret_map, output, ext->hmac_secret.saltLen); check_ret(ret); } - ret = cbor_encoder_close_container(&ext_map, &hmac_secret_map); + ret = cbor_encoder_close_container(&extensions, &hmac_secret_map); check_ret(ret); } - ret = cbor_encoder_close_container(&extensions, &ext_map); - check_ret(ret); *ext_encoder_buf_size = cbor_encoder_get_buffer_size(&extensions, ext_encoder_buf); } else @@ -1071,7 +1064,7 @@ uint8_t ctap_get_next_assertion(CborEncoder * encoder) uint8_t ctap_get_assertion(CborEncoder * encoder, uint8_t * request, int length) { CTAP_getAssertion GA; - uint8_t auth_data_buf[sizeof(CTAP_authDataHeader) + 100]; + uint8_t auth_data_buf[sizeof(CTAP_authDataHeader) + 80]; int ret = ctap_parse_get_assertion(&GA,request,length); if (ret != 0) diff --git a/tools/ctap_test.py b/tools/ctap_test.py index 7b42197..a102c84 100755 --- a/tools/ctap_test.py +++ b/tools/ctap_test.py @@ -787,7 +787,7 @@ class Tester: salt1 = b"\x5a" * 32 salt2 = b"\x96" * 32 - self.testReset() + # self.testReset() with Test("Get info has hmac-secret"): info = self.ctap.get_info() @@ -841,20 +841,20 @@ class Tester: ): ext = auth.auth_data.extensions assert ext - assert "hmac-secret" in ext[4] - assert type(ext[4]["hmac-secret"]) == type(b"") - assert len(ext[4]["hmac-secret"]) == len(salt_list) * 32 + assert "hmac-secret" in ext + assert type(ext["hmac-secret"]) == type(b"") + assert len(ext["hmac-secret"]) == len(salt_list) * 32 with Test("Check that shannon_entropy of hmac-secret is good"): ext = auth.auth_data.extensions dec = cipher.decryptor() - key = dec.update(ext[4]["hmac-secret"]) + dec.finalize() + key = dec.update(ext["hmac-secret"]) + dec.finalize() if len(salt_list) == 1: - assert shannon_entropy(ext[4]["hmac-secret"]) > 4.6 + assert shannon_entropy(ext["hmac-secret"]) > 4.6 assert shannon_entropy(key) > 4.6 if len(salt_list) == 2: - assert shannon_entropy(ext[4]["hmac-secret"]) > 5.6 + assert shannon_entropy(ext["hmac-secret"]) > 5.6 assert shannon_entropy(key) > 5.6 def test_fido2_other(self,): From 02e83073e049fdb3dd4ff749afbf24b061428643 Mon Sep 17 00:00:00 2001 From: Conor Patrick Date: Wed, 20 Mar 2019 23:58:42 -0400 Subject: [PATCH 16/20] add hmac-secret to reg response --- fido2/ctap.c | 34 +++++++++++++++++++++++++++++++++- tools/ctap_test.py | 5 +++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/fido2/ctap.c b/fido2/ctap.c index af39f67..fb47271 100644 --- a/fido2/ctap.c +++ b/fido2/ctap.c @@ -401,6 +401,25 @@ static int ctap_make_extensions(CTAP_extensions * ext, uint8_t * ext_encoder_buf } *ext_encoder_buf_size = cbor_encoder_get_buffer_size(&extensions, ext_encoder_buf); } + else if (ext->hmac_secret_present == EXT_HMAC_SECRET_REQUESTED) + { + cbor_encoder_init(&extensions, ext_encoder_buf, *ext_encoder_buf_size, 0); + { + CborEncoder hmac_secret_map; + ret = cbor_encoder_create_map(&extensions, &hmac_secret_map, 1); + check_ret(ret); + { + ret = cbor_encode_text_stringz(&hmac_secret_map, "hmac-secret"); + check_ret(ret); + + ret = cbor_encode_boolean(&hmac_secret_map, 1); + check_ret(ret); + } + ret = cbor_encoder_close_container(&extensions, &hmac_secret_map); + check_ret(ret); + } + *ext_encoder_buf_size = cbor_encoder_get_buffer_size(&extensions, ext_encoder_buf); + } else { *ext_encoder_buf_size = 0; @@ -646,7 +665,7 @@ uint8_t ctap_make_credential(CborEncoder * encoder, uint8_t * request, int lengt CTAP_makeCredential MC; int ret; unsigned int i; - uint8_t auth_data_buf[300]; + uint8_t auth_data_buf[310]; CTAP_credentialDescriptor * excl_cred = (CTAP_credentialDescriptor *) auth_data_buf; uint8_t * sigbuf = auth_data_buf + 32; uint8_t * sigder = auth_data_buf + 32 + 64; @@ -717,6 +736,19 @@ uint8_t ctap_make_credential(CborEncoder * encoder, uint8_t * request, int lengt &MC.credInfo); check_retr(ret); + { + unsigned int ext_encoder_buf_size = sizeof(auth_data_buf) - auth_data_sz; + uint8_t * ext_encoder_buf = auth_data_buf + auth_data_sz; + + ret = ctap_make_extensions(&MC.extensions, ext_encoder_buf, &ext_encoder_buf_size); + check_retr(ret); + if (ext_encoder_buf_size) + { + ((CTAP_authData *)auth_data_buf)->head.flags |= (1 << 7); + auth_data_sz += ext_encoder_buf_size; + } + } + { ret = cbor_encode_int(&map,RESP_authData); check_ret(ret); diff --git a/tools/ctap_test.py b/tools/ctap_test.py index a102c84..4ded73e 100755 --- a/tools/ctap_test.py +++ b/tools/ctap_test.py @@ -803,6 +803,11 @@ class Tester: other={"extensions": {"hmac-secret": True}, "options": {"rk": True}}, ) + with Test("Check 'hmac-secret' is set to true in auth_data extensions"): + assert reg.auth_data.extensions + assert "hmac-secret" in reg.auth_data.extensions + assert reg.auth_data.extensions["hmac-secret"] == True + with Test("Get shared secret"): key_agreement, shared_secret = ( self.client.pin_protocol._init_shared_secret() From d68011ef0412093bd8e47520ca3b8a308c2a26c9 Mon Sep 17 00:00:00 2001 From: Conor Patrick Date: Thu, 21 Mar 2019 00:01:37 -0400 Subject: [PATCH 17/20] remove warnings --- fido2/ctap_parse.c | 6 +++--- targets/stm32l432/src/redirect.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/fido2/ctap_parse.c b/fido2/ctap_parse.c index 6cac59c..fbe3fc0 100644 --- a/fido2/ctap_parse.c +++ b/fido2/ctap_parse.c @@ -635,9 +635,9 @@ uint8_t ctap_parse_extensions(CborValue * val, CTAP_extensions * ext) { CborValue map; size_t sz, map_length; - uint8_t key[16]; - uint8_t ret; - int i; + char key[16]; + int ret; + unsigned int i; bool b; if (cbor_value_get_type(val) != CborMapType) diff --git a/targets/stm32l432/src/redirect.c b/targets/stm32l432/src/redirect.c index da8b19d..3eaf046 100644 --- a/targets/stm32l432/src/redirect.c +++ b/targets/stm32l432/src/redirect.c @@ -39,7 +39,7 @@ int _write (int fd, const void *buf, unsigned long int len) // logbuflen += len; // Send out USB serial - CDC_Transmit_FS(buf, len); + CDC_Transmit_FS(data, len); // if (res == USBD_OK) // logbuflen = 0; #endif From a1a75e4ab57aae6bb2e6291c8a7429eaeb9dce1f Mon Sep 17 00:00:00 2001 From: Conor Patrick Date: Thu, 21 Mar 2019 12:47:15 -0400 Subject: [PATCH 18/20] check errors --- fido2/ctap.c | 2 +- fido2/ctap_parse.c | 4 +-- tools/ctap_test.py | 82 +++++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 81 insertions(+), 7 deletions(-) diff --git a/fido2/ctap.c b/fido2/ctap.c index fb47271..bebc9ab 100644 --- a/fido2/ctap.c +++ b/fido2/ctap.c @@ -354,7 +354,7 @@ static int ctap_make_extensions(CTAP_extensions * ext, uint8_t * ext_encoder_buf else { printf1(TAG_CTAP, "saltAuth is invalid\r\n"); - return CTAP1_ERR_OTHER; + return CTAP2_ERR_EXTENSION_FIRST; } // Generate credRandom diff --git a/fido2/ctap_parse.c b/fido2/ctap_parse.c index fbe3fc0..64cd2f6 100644 --- a/fido2/ctap_parse.c +++ b/fido2/ctap_parse.c @@ -601,11 +601,11 @@ uint8_t ctap_parse_hmac_secret(CborValue * val, CTAP_hmac_secret * hs) case EXT_HMAC_SECRET_SALT_ENC: salt_len = 64; ret = cbor_value_copy_byte_string(&map, hs->saltEnc, &salt_len, NULL); - check_ret(ret); - if (salt_len != 32 && salt_len != 64) + if ((salt_len != 32 && salt_len != 64) || ret == CborErrorOutOfMemory) { return CTAP1_ERR_INVALID_LENGTH; } + check_ret(ret); hs->saltLen = salt_len; parsed_count++; break; diff --git a/tools/ctap_test.py b/tools/ctap_test.py index 4ded73e..984879a 100755 --- a/tools/ctap_test.py +++ b/tools/ctap_test.py @@ -786,6 +786,7 @@ class Tester: salt1 = b"\x5a" * 32 salt2 = b"\x96" * 32 + salt3 = b"\x03" * 32 # self.testReset() @@ -808,6 +809,16 @@ class Tester: assert "hmac-secret" in reg.auth_data.extensions assert reg.auth_data.extensions["hmac-secret"] == True + reg = self.testMC( + "Send MC with fake extension set to true, expect SUCCESS", + cdh, + rp, + user, + key_params, + expectedError=CtapError.ERR.SUCCESS, + other={"extensions": {"tetris": True}}, + ) + with Test("Get shared secret"): key_agreement, shared_secret = ( self.client.pin_protocol._init_shared_secret() @@ -818,14 +829,18 @@ class Tester: default_backend(), ) - for salt_list in ((salt1,), (salt1, salt2)): + def get_salt_params(salts): enc = cipher.encryptor() salt_enc = b"" - for salt in salt_list: + for salt in salts: salt_enc += enc.update(salt) salt_enc += enc.finalize() salt_auth = hmac_sha256(shared_secret, salt_enc)[:16] + return salt_enc, salt_auth + + for salt_list in ((salt1,), (salt1, salt2)): + salt_enc, salt_auth = get_salt_params(salt_list) auth = self.testGA( "Send GA request with %d salts hmac-secret, expect success" @@ -855,12 +870,71 @@ class Tester: dec = cipher.decryptor() key = dec.update(ext["hmac-secret"]) + dec.finalize() + print(shannon_entropy(ext["hmac-secret"])) if len(salt_list) == 1: assert shannon_entropy(ext["hmac-secret"]) > 4.6 assert shannon_entropy(key) > 4.6 if len(salt_list) == 2: - assert shannon_entropy(ext["hmac-secret"]) > 5.6 - assert shannon_entropy(key) > 5.6 + assert shannon_entropy(ext["hmac-secret"]) > 5.4 + assert shannon_entropy(key) > 5.4 + + salt_enc, salt_auth = get_salt_params((salt3,)) + + auth = self.testGA( + "Send GA request with hmac-secret missing keyAgreement, expect error", + rp["id"], + cdh, + other={"extensions": {"hmac-secret": {2: salt_enc, 3: salt_auth}}}, + ) + auth = self.testGA( + "Send GA request with hmac-secret missing saltAuth, expect MISSING_PARAMETER", + rp["id"], + cdh, + other={"extensions": {"hmac-secret": {1: key_agreement, 2: salt_enc}}}, + expectedError=CtapError.ERR.MISSING_PARAMETER, + ) + auth = self.testGA( + "Send GA request with hmac-secret missing saltEnc, expect MISSING_PARAMETER", + rp["id"], + cdh, + other={"extensions": {"hmac-secret": {1: key_agreement, 3: salt_auth}}}, + expectedError=CtapError.ERR.MISSING_PARAMETER, + ) + + bad_auth = list(salt_auth[:]) + bad_auth[len(bad_auth) // 2] = bad_auth[len(bad_auth) // 2] ^ 1 + bad_auth = bytes(bad_auth) + + auth = self.testGA( + "Send GA request with hmac-secret containing bad saltAuth, expect EXTENSION_FIRST", + rp["id"], + cdh, + other={ + "extensions": { + "hmac-secret": {1: key_agreement, 2: salt_enc, 3: bad_auth} + } + }, + expectedError=CtapError.ERR.EXTENSION_FIRST, + ) + + salt4 = b"\x5a" * 16 + salt5 = b"\x96" * 64 + for salt_list in ((salt4,), (salt4, salt5)): + salt_enc, salt_auth = get_salt_params(salt_list) + + salt_auth = hmac_sha256(shared_secret, salt_enc)[:16] + auth = self.testGA( + "Send GA request with incorrect salt length %d, expect INVALID_LENGTH" + % len(salt_enc), + rp["id"], + cdh, + other={ + "extensions": { + "hmac-secret": {1: key_agreement, 2: salt_enc, 3: salt_auth} + } + }, + expectedError=CtapError.ERR.INVALID_LENGTH, + ) def test_fido2_other(self,): From a2a774125fac9f7499c8e4e4c5e305cb3cd15515 Mon Sep 17 00:00:00 2001 From: Nicolas Stalder Date: Fri, 22 Mar 2019 21:40:55 +0100 Subject: [PATCH 19/20] Fix usage and display fido2-ext in it --- tools/ctap_test.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tools/ctap_test.py b/tools/ctap_test.py index 984879a..8697de9 100755 --- a/tools/ctap_test.py +++ b/tools/ctap_test.py @@ -2004,8 +2004,19 @@ def test_find_brute_force(): if __name__ == "__main__": + tests = ( + "solo", + "u2f", + "fido2", + "fido2-ext", + "rk", + "hid", + "ping", + "bootloader", + ) + if len(sys.argv) < 2: - print("Usage: %s [sim] <[u2f]|[fido2]|[rk]|[hid]|[ping]>") + print(f"Usage: {sys.argv[0]} [sim] <{'|'.join(sorted(tests))}>") sys.exit(0) t = Tester() From 9d3144e9b1fdd9a1e51f9b0562adf61f6e4da6e6 Mon Sep 17 00:00:00 2001 From: Nicolas Stalder Date: Fri, 22 Mar 2019 21:56:18 +0100 Subject: [PATCH 20/20] oops. black --- tools/ctap_test.py | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/tools/ctap_test.py b/tools/ctap_test.py index 8697de9..f96c0f9 100755 --- a/tools/ctap_test.py +++ b/tools/ctap_test.py @@ -2004,16 +2004,7 @@ def test_find_brute_force(): if __name__ == "__main__": - tests = ( - "solo", - "u2f", - "fido2", - "fido2-ext", - "rk", - "hid", - "ping", - "bootloader", - ) + tests = ("solo", "u2f", "fido2", "fido2-ext", "rk", "hid", "ping", "bootloader") if len(sys.argv) < 2: print(f"Usage: {sys.argv[0]} [sim] <{'|'.join(sorted(tests))}>")