From 4cc72bcd9739312905b224cde4bdaaa804c8cc10 Mon Sep 17 00:00:00 2001 From: Conor Patrick Date: Wed, 10 Apr 2019 12:11:31 -0400 Subject: [PATCH 01/12] rearrange cbor encoding order in make_credential and get_info --- fido2/ctap.c | 115 +++++++++++++++++++++++++++------------------------ 1 file changed, 61 insertions(+), 54 deletions(-) diff --git a/fido2/ctap.c b/fido2/ctap.c index a93fb66..cc9a0ff 100644 --- a/fido2/ctap.c +++ b/fido2/ctap.c @@ -117,28 +117,6 @@ uint8_t ctap_get_info(CborEncoder * encoder) check_ret(ret); } - ret = cbor_encode_uint(&map, RESP_maxMsgSize); - check_ret(ret); - { - ret = cbor_encode_int(&map, CTAP_MAX_MESSAGE_SIZE); - check_ret(ret); - } - - ret = cbor_encode_uint(&map, RESP_pinProtocols); - check_ret(ret); - { - ret = cbor_encoder_create_array(&map, &pins, 1); - check_ret(ret); - { - ret = cbor_encode_int(&pins, 1); - check_ret(ret); - } - ret = cbor_encoder_close_container(&map, &pins); - check_ret(ret); - } - - - ret = cbor_encode_uint(&map, RESP_options); check_ret(ret); { @@ -188,6 +166,30 @@ uint8_t ctap_get_info(CborEncoder * encoder) check_ret(ret); } + ret = cbor_encode_uint(&map, RESP_maxMsgSize); + check_ret(ret); + { + ret = cbor_encode_int(&map, CTAP_MAX_MESSAGE_SIZE); + check_ret(ret); + } + + ret = cbor_encode_uint(&map, RESP_pinProtocols); + check_ret(ret); + { + ret = cbor_encoder_create_array(&map, &pins, 1); + check_ret(ret); + { + ret = cbor_encode_int(&pins, 1); + check_ret(ret); + } + ret = cbor_encoder_close_container(&map, &pins); + check_ret(ret); + } + + + + + } ret = cbor_encoder_close_container(encoder, &map); @@ -730,6 +732,14 @@ 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); + + { + ret = cbor_encode_int(&map,RESP_fmt); + check_ret(ret); + ret = cbor_encode_text_stringz(&map, "packed"); + check_ret(ret); + } + uint32_t auth_data_sz = sizeof(auth_data_buf); ret = ctap_make_auth_data(&MC.rp, &map, auth_data_buf, &auth_data_sz, @@ -763,13 +773,6 @@ uint8_t ctap_make_credential(CborEncoder * encoder, uint8_t * request, int lengt ret = ctap_add_attest_statement(&map, sigder, sigder_sz); check_retr(ret); - { - ret = cbor_encode_int(&map,RESP_fmt); - check_ret(ret); - ret = cbor_encode_text_stringz(&map, "packed"); - check_ret(ret); - } - ret = cbor_encoder_close_container(encoder, &map); check_ret(ret); return CTAP1_ERR_SUCCESS; @@ -797,13 +800,6 @@ static uint8_t ctap_add_credential_descriptor(CborEncoder * map, CTAP_credential ret = cbor_encoder_create_map(map, &desc, 2); check_ret(ret); - { - ret = cbor_encode_text_string(&desc, "type", 4); - check_ret(ret); - - ret = cbor_encode_text_string(&desc, "public-key", 10); - check_ret(ret); - } { ret = cbor_encode_text_string(&desc, "id", 2); check_ret(ret); @@ -812,6 +808,15 @@ static uint8_t ctap_add_credential_descriptor(CborEncoder * map, CTAP_credential check_ret(ret); } + { + ret = cbor_encode_text_string(&desc, "type", 4); + check_ret(ret); + + ret = cbor_encode_text_string(&desc, "public-key", 10); + check_ret(ret); + } + + ret = cbor_encoder_close_container(map, &desc); check_ret(ret); @@ -843,6 +848,13 @@ uint8_t ctap_add_user_entity(CborEncoder * map, CTAP_userEntity * user) if (dispname) { + + ret = cbor_encode_text_string(&entity, "icon", 4); + check_ret(ret); + + ret = cbor_encode_text_stringz(&entity, (const char *)user->icon); + check_ret(ret); + ret = cbor_encode_text_string(&entity, "name", 4); check_ret(ret); @@ -855,13 +867,6 @@ uint8_t ctap_add_user_entity(CborEncoder * map, CTAP_userEntity * user) ret = cbor_encode_text_stringz(&entity, (const char *)user->displayName); check_ret(ret); - ret = cbor_encode_text_string(&entity, "icon", 4); - check_ret(ret); - - ret = cbor_encode_text_stringz(&entity, (const char *)user->icon); - check_ret(ret); - - } ret = cbor_encoder_close_container(map, &entity); @@ -1007,11 +1012,11 @@ uint8_t ctap_end_get_assertion(CborEncoder * map, CTAP_credentialDescriptor * cr if (add_user) { printf1(TAG_GREEN, "adding user details to output\r\n"); - ret = ctap_add_user_entity(map, &cred->credential.user); + ret = ctap_add_user_entity(map, &cred->credential.user); // 4 check_retr(ret); } - ret = ctap_add_credential_descriptor(map, cred); + ret = ctap_add_credential_descriptor(map, cred); // 1 check_retr(ret); crypto_ecc256_load_key((uint8_t*)&cred->credential.id, sizeof(CredentialId), NULL, 0); @@ -1028,7 +1033,7 @@ uint8_t ctap_end_get_assertion(CborEncoder * map, CTAP_credentialDescriptor * cr } { - ret = cbor_encode_int(map, RESP_signature); + ret = cbor_encode_int(map, RESP_signature); // 3 check_ret(ret); ret = cbor_encode_byte_string(map, sigder, sigder_sz); check_ret(ret); @@ -1166,13 +1171,7 @@ uint8_t ctap_get_assertion(CborEncoder * encoder, uint8_t * request, int length) printf1(TAG_GA,"CRED ID (# %d)\n", GA.creds[j].credential.id.count); } - if (validCredCount > 1) - { - ret = cbor_encode_int(&map, RESP_numberOfCredentials); - check_ret(ret); - ret = cbor_encode_int(&map, validCredCount); - check_ret(ret); - } + CTAP_credentialDescriptor * cred = &GA.creds[validCredCount - 1]; @@ -1196,7 +1195,7 @@ uint8_t ctap_get_assertion(CborEncoder * encoder, uint8_t * request, int length) ((CTAP_authData *)auth_data_buf)->head.flags &= ~(1 << 2); ((CTAP_authData *)auth_data_buf)->head.flags |= (getAssertionState.user_verified << 2); - + { unsigned int ext_encoder_buf_size = sizeof(auth_data_buf) - len; uint8_t * ext_encoder_buf = auth_data_buf + len; @@ -1223,6 +1222,14 @@ uint8_t ctap_get_assertion(CborEncoder * encoder, uint8_t * request, int length) ret = ctap_end_get_assertion(&map, cred, auth_data_buf, GA.clientDataHash, add_user_info); check_retr(ret); + if (validCredCount > 1) + { + ret = cbor_encode_int(&map, RESP_numberOfCredentials); + check_ret(ret); + ret = cbor_encode_int(&map, validCredCount); + check_ret(ret); + } + ret = cbor_encoder_close_container(encoder, &map); check_ret(ret); From 86393c46b468829b87fc342edf92c2305bd2715d Mon Sep 17 00:00:00 2001 From: Conor Patrick Date: Wed, 10 Apr 2019 12:11:43 -0400 Subject: [PATCH 02/12] Test it is correct --- tools/testing/tests/fido2.py | 63 ++++++++++++++++++++++++++++-------- 1 file changed, 50 insertions(+), 13 deletions(-) diff --git a/tools/testing/tests/fido2.py b/tools/testing/tests/fido2.py index 2ab09e6..4c23e1e 100644 --- a/tools/testing/tests/fido2.py +++ b/tools/testing/tests/fido2.py @@ -2,8 +2,9 @@ from __future__ import print_function, absolute_import, unicode_literals import time from random import randint import array +import struct - +from fido2 import cbor from fido2.ctap import CtapError from fido2.ctap2 import ES256, PinProtocolV1 @@ -229,6 +230,15 @@ class FIDO2Tests(Tester): def test_get_info(self,): with Test("Get info"): info = self.ctap.get_info() + print(bytes(info)) + print(cbor.loads(bytes(info))) + + with Test("Check dictionary keys are sorted from lowest to highest"): + last = 0 + for x in cbor.loads(bytes(info))[0]: + print("%d < %d" % (last, x)) + assert last < x + last = x with Test("Check FIDO2 string is in VERSIONS field"): assert "FIDO_2_0" in info.versions @@ -274,6 +284,26 @@ class FIDO2Tests(Tester): key_params, expectedError=CtapError.ERR.SUCCESS, ) + + with Test("Check response dictionary keys are sorted from lowest to highest"): + last = 0 + for x in cbor.loads(bytes(prev_reg))[0]: + assert last < x + last = x + + with Test("Check COSE KEY dictionary keys are sorted from lowest to highest"): + last = 0 + data = prev_reg.auth_data.credential_data + c_len = struct.unpack(">H", data[16:18])[0] + cred_id = data[18 : 18 + c_len] + pub_key, _ = cbor.loads(data[18 + c_len :]) + for x in pub_key: + if x > 0: + assert last < x + else: + assert last > x + last = x + allow_list = [ { "id": prev_reg.auth_data.credential_data.credential_id, @@ -616,6 +646,13 @@ class FIDO2Tests(Tester): expectedError=CtapError.ERR.SUCCESS, ) + with Test("Check response dictionary keys are sorted from lowest to highest"): + last = 0 + print(cbor.loads(bytes(prev_auth))[0]) + for x in cbor.loads(bytes(prev_auth))[0]: + assert last < x + last = x + with Test("Test auth_data is 37 bytes"): assert len(prev_auth.auth_data) == 37 @@ -1085,15 +1122,15 @@ class FIDO2Tests(Tester): self.test_get_info() self.test_get_assertion() - - self.test_make_credential() - - self.test_rk(None) - - self.test_client_pin() - - self.testReset() - - self.test_extensions() - - print("Done") + # + # self.test_make_credential() + # + # self.test_rk(None) + # + # self.test_client_pin() + # + # self.testReset() + # + # self.test_extensions() + # + # print("Done") From 5f49f4680e8390af3561437db9a9a65b32ab8ecf Mon Sep 17 00:00:00 2001 From: Conor Patrick Date: Wed, 10 Apr 2019 12:22:35 -0400 Subject: [PATCH 03/12] re-order items in get_assertion response --- fido2/ctap.c | 61 ++++++++++++++++++++++++++-------------------------- 1 file changed, 30 insertions(+), 31 deletions(-) diff --git a/fido2/ctap.c b/fido2/ctap.c index cc9a0ff..acea4de 100644 --- a/fido2/ctap.c +++ b/fido2/ctap.c @@ -1002,23 +1002,23 @@ static CTAP_credentialDescriptor * pop_credential() } // adds 2 to map, or 3 if add_user is true -uint8_t ctap_end_get_assertion(CborEncoder * map, CTAP_credentialDescriptor * cred, uint8_t * auth_data_buf, uint8_t * clientDataHash, int add_user) +uint8_t ctap_end_get_assertion(CborEncoder * map, CTAP_credentialDescriptor * cred, uint8_t * auth_data_buf, unsigned int auth_data_buf_sz, uint8_t * clientDataHash, int add_user) { int ret; uint8_t sigbuf[64]; uint8_t sigder[72]; int sigder_sz; - if (add_user) - { - printf1(TAG_GREEN, "adding user details to output\r\n"); - ret = ctap_add_user_entity(map, &cred->credential.user); // 4 - check_retr(ret); - } - ret = ctap_add_credential_descriptor(map, cred); // 1 check_retr(ret); + { + ret = cbor_encode_int(map,RESP_authData); // 2 + check_ret(ret); + ret = cbor_encode_byte_string(map, auth_data_buf, auth_data_buf_sz); + check_ret(ret); + } + crypto_ecc256_load_key((uint8_t*)&cred->credential.id, sizeof(CredentialId), NULL, 0); #ifdef ENABLE_U2F_EXTENSIONS @@ -1038,6 +1038,15 @@ uint8_t ctap_end_get_assertion(CborEncoder * map, CTAP_credentialDescriptor * cr ret = cbor_encode_byte_string(map, sigder, sigder_sz); check_ret(ret); } + + if (add_user) + { + printf1(TAG_GREEN, "adding user details to output\r\n"); + ret = ctap_add_user_entity(map, &cred->credential.user); // 4 + check_retr(ret); + } + + return 0; } @@ -1073,12 +1082,7 @@ uint8_t ctap_get_next_assertion(CborEncoder * encoder) check_ret(ret); printf1(TAG_RED, "RPID hash: "); dump_hex1(TAG_RED, authData.rpIdHash, 32); - { - ret = cbor_encode_int(&map,RESP_authData); - check_ret(ret); - ret = cbor_encode_byte_string(&map, (uint8_t *)&authData, sizeof(CTAP_authDataHeader)); - check_ret(ret); - } + // if only one account for this RP, null out the user details @@ -1089,7 +1093,7 @@ uint8_t ctap_get_next_assertion(CborEncoder * encoder) } - ret = ctap_end_get_assertion(&map, cred, (uint8_t *)&authData, getAssertionState.clientDataHash, add_user_info); + ret = ctap_end_get_assertion(&map, cred, (uint8_t *)&authData, sizeof(CTAP_authDataHeader), getAssertionState.clientDataHash, add_user_info); // 4, 1, 3 check_retr(ret); ret = cbor_encoder_close_container(encoder, &map); @@ -1177,54 +1181,49 @@ uint8_t ctap_get_assertion(CborEncoder * encoder, uint8_t * request, int length) GA.extensions.hmac_secret.credential = &cred->credential; + uint32_t auth_data_buf_sz = sizeof(auth_data_buf); + #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); + ret = cbor_encode_int(&map,RESP_authData); // 2 check_ret(ret); memset(auth_data_buf,0,sizeof(CTAP_authDataHeader)); - ret = cbor_encode_byte_string(&map, auth_data_buf, sizeof(CTAP_authDataHeader)); - check_ret(ret); + auth_data_buf_sz = sizeof(CTAP_authDataHeader); } else #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, &auth_data_buf_sz, NULL); check_retr(ret); ((CTAP_authData *)auth_data_buf)->head.flags &= ~(1 << 2); ((CTAP_authData *)auth_data_buf)->head.flags |= (getAssertionState.user_verified << 2); { - unsigned int ext_encoder_buf_size = sizeof(auth_data_buf) - len; - uint8_t * ext_encoder_buf = auth_data_buf + len; + unsigned int ext_encoder_buf_size = sizeof(auth_data_buf) - auth_data_buf_sz; + uint8_t * ext_encoder_buf = auth_data_buf + auth_data_buf_sz; 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; + auth_data_buf_sz += 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 - ret = ctap_end_get_assertion(&map, cred, auth_data_buf, GA.clientDataHash, add_user_info); + ret = ctap_end_get_assertion(&map, cred, auth_data_buf, auth_data_buf_sz, GA.clientDataHash, add_user_info); // 1,2,3,4 check_retr(ret); if (validCredCount > 1) { - ret = cbor_encode_int(&map, RESP_numberOfCredentials); + ret = cbor_encode_int(&map, RESP_numberOfCredentials); // 5 check_ret(ret); ret = cbor_encode_int(&map, validCredCount); check_ret(ret); From 5fc8d214fd59a819f31eb45fb1ab4556f9274c5c Mon Sep 17 00:00:00 2001 From: Conor Patrick Date: Wed, 10 Apr 2019 12:47:23 -0400 Subject: [PATCH 04/12] remove add_user param --- fido2/ctap.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/fido2/ctap.c b/fido2/ctap.c index acea4de..ccc2a4f 100644 --- a/fido2/ctap.c +++ b/fido2/ctap.c @@ -1002,7 +1002,7 @@ static CTAP_credentialDescriptor * pop_credential() } // adds 2 to map, or 3 if add_user is true -uint8_t ctap_end_get_assertion(CborEncoder * map, CTAP_credentialDescriptor * cred, uint8_t * auth_data_buf, unsigned int auth_data_buf_sz, uint8_t * clientDataHash, int add_user) +uint8_t ctap_end_get_assertion(CborEncoder * map, CTAP_credentialDescriptor * cred, uint8_t * auth_data_buf, unsigned int auth_data_buf_sz, uint8_t * clientDataHash) { int ret; uint8_t sigbuf[64]; @@ -1039,7 +1039,7 @@ uint8_t ctap_end_get_assertion(CborEncoder * map, CTAP_credentialDescriptor * cr check_ret(ret); } - if (add_user) + if (cred->credential.user.id_size) { printf1(TAG_GREEN, "adding user details to output\r\n"); ret = ctap_add_user_entity(map, &cred->credential.user); // 4 @@ -1065,9 +1065,8 @@ uint8_t ctap_get_next_assertion(CborEncoder * encoder) } auth_data_update_count(&authData); - int add_user_info = cred->credential.user.id_size; - if (add_user_info) + if (cred->credential.user.id_size) { printf1(TAG_GREEN, "adding user info to assertion response\r\n"); ret = cbor_encoder_create_map(encoder, &map, 4); @@ -1078,13 +1077,9 @@ 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); - - - // if only one account for this RP, null out the user details if (!getAssertionState.user_verified) { @@ -1092,8 +1087,7 @@ uint8_t ctap_get_next_assertion(CborEncoder * encoder) memset(cred->credential.user.name, 0, USER_NAME_LIMIT); } - - ret = ctap_end_get_assertion(&map, cred, (uint8_t *)&authData, sizeof(CTAP_authDataHeader), getAssertionState.clientDataHash, add_user_info); // 4, 1, 3 + ret = ctap_end_get_assertion(&map, cred, (uint8_t *)&authData, sizeof(CTAP_authDataHeader), getAssertionState.clientDataHash); check_retr(ret); ret = cbor_encoder_close_container(encoder, &map); @@ -1136,13 +1130,12 @@ uint8_t ctap_get_assertion(CborEncoder * encoder, uint8_t * request, int length) printf1(TAG_GA, "ALLOW_LIST has %d creds\n", GA.credLen); int validCredCount = ctap_filter_invalid_credentials(&GA); - int add_user_info = GA.creds[validCredCount - 1].credential.user.id_size; if (validCredCount > 1) { map_size += 1; } - if (add_user_info) + if (GA.creds[validCredCount - 1].credential.user.id_size) { map_size += 1; } @@ -1218,7 +1211,7 @@ uint8_t ctap_get_assertion(CborEncoder * encoder, uint8_t * request, int length) 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, auth_data_buf_sz, GA.clientDataHash, add_user_info); // 1,2,3,4 + ret = ctap_end_get_assertion(&map, cred, auth_data_buf, auth_data_buf_sz, GA.clientDataHash); // 1,2,3,4 check_retr(ret); if (validCredCount > 1) From ffa9ad4923fe73f30c670455d9c6348efcbac54a Mon Sep 17 00:00:00 2001 From: Conor Patrick Date: Wed, 10 Apr 2019 12:47:39 -0400 Subject: [PATCH 05/12] refactor cbor sorting test --- tools/testing/tests/fido2.py | 48 ++++++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 19 deletions(-) diff --git a/tools/testing/tests/fido2.py b/tools/testing/tests/fido2.py index 4c23e1e..3a3bbb4 100644 --- a/tools/testing/tests/fido2.py +++ b/tools/testing/tests/fido2.py @@ -3,6 +3,7 @@ import time from random import randint import array import struct +from functools import cmp_to_key from fido2 import cbor from fido2.ctap import CtapError @@ -34,6 +35,30 @@ def VerifyAttestation(attest, data): verifier().verify(attest.att_statement, attest.auth_data, data.hash) +def TestCborKeysSorted(cbor_bytes): + def sort_cbor_keys(x1, x2): + if isinstance(x1, int) and isinstance(x2, int): + if x1 >= 0 and x2 >= 0: + return x1 > x2 + elif x1 < 0 and x2 >= 0: + return x1 > x2 + elif x1 >= 0 and x2 < 0: + return x1 < x2 + else: + return x1 < x2 + + last = 0 + l = [x for x in cbor.loads(cbor_bytes)[0]] + l_sorted = l[:] + l_sorted = sorted(l_sorted, key=cmp_to_key(sort_cbor_keys)) + print("sorted", l_sorted) + print("real", l) + for i in range(0, len(l)): + if l[i] != l_sorted[i]: + raise ValueError("Cbor list item %d: %d is out of order" % (i, l[i])) + return l + + class FIDO2Tests(Tester): def __init__(self, tester=None): super().__init__(tester) @@ -234,11 +259,7 @@ class FIDO2Tests(Tester): print(cbor.loads(bytes(info))) with Test("Check dictionary keys are sorted from lowest to highest"): - last = 0 - for x in cbor.loads(bytes(info))[0]: - print("%d < %d" % (last, x)) - assert last < x - last = x + TestCborKeysSorted(bytes(info)) with Test("Check FIDO2 string is in VERSIONS field"): assert "FIDO_2_0" in info.versions @@ -287,22 +308,13 @@ class FIDO2Tests(Tester): with Test("Check response dictionary keys are sorted from lowest to highest"): last = 0 - for x in cbor.loads(bytes(prev_reg))[0]: - assert last < x - last = x with Test("Check COSE KEY dictionary keys are sorted from lowest to highest"): last = 0 data = prev_reg.auth_data.credential_data c_len = struct.unpack(">H", data[16:18])[0] cred_id = data[18 : 18 + c_len] - pub_key, _ = cbor.loads(data[18 + c_len :]) - for x in pub_key: - if x > 0: - assert last < x - else: - assert last > x - last = x + TestCborKeysSorted(data[18 + c_len :]) allow_list = [ { @@ -649,9 +661,7 @@ class FIDO2Tests(Tester): with Test("Check response dictionary keys are sorted from lowest to highest"): last = 0 print(cbor.loads(bytes(prev_auth))[0]) - for x in cbor.loads(bytes(prev_auth))[0]: - assert last < x - last = x + TestCborKeysSorted(bytes(prev_auth)) with Test("Test auth_data is 37 bytes"): assert len(prev_auth.auth_data) == 37 @@ -1123,7 +1133,7 @@ class FIDO2Tests(Tester): self.test_get_assertion() # - # self.test_make_credential() + self.test_make_credential() # # self.test_rk(None) # From f8635f1682f549455d714fdfe939d3b0272139e5 Mon Sep 17 00:00:00 2001 From: Conor Patrick Date: Wed, 10 Apr 2019 13:12:33 -0400 Subject: [PATCH 06/12] refactor tests --- tools/testing/tests/fido2.py | 59 +++++++++++++++++++++++++++--------- 1 file changed, 45 insertions(+), 14 deletions(-) diff --git a/tools/testing/tests/fido2.py b/tools/testing/tests/fido2.py index 3a3bbb4..3c6e80f 100644 --- a/tools/testing/tests/fido2.py +++ b/tools/testing/tests/fido2.py @@ -36,26 +36,49 @@ def VerifyAttestation(attest, data): def TestCborKeysSorted(cbor_bytes): + # Cbor canonical ordering of keys. + # https://fidoalliance.org/specs/fido-v2.0-ps-20190130/fido-client-to-authenticator-protocol-v2.0-ps-20190130.html#ctap2-canonical-cbor-encoding-form def sort_cbor_keys(x1, x2): + # >0 x1 goes second, <0 x1 goes first if isinstance(x1, int) and isinstance(x2, int): if x1 >= 0 and x2 >= 0: - return x1 > x2 + return x1 - x2 elif x1 < 0 and x2 >= 0: - return x1 > x2 + return 1 elif x1 >= 0 and x2 < 0: - return x1 < x2 + return -1 else: - return x1 < x2 + return -x1 + x2 - last = 0 - l = [x for x in cbor.loads(cbor_bytes)[0]] + if isinstance(x1, (str, bytes)) and isinstance(x2, int): + return 1 + elif isinstance(x1, int) and isinstance(x2, (str, bytes)): + return -1 + + if len(x1) == len(x2): + for i in range(0, len(x1)): + v1 = x1[i] if isinstance(x1[i], int) else ord(x1[i]) + v2 = x2[i] if isinstance(x2[i], int) else ord(x2[i]) + if v1 != v2: + return v1 - v2 + return 0 + + return len(x1) - len(x2) + + cbor_map = cbor_bytes + if isinstance(cbor_map, bytes): + cbor_map = cbor.loads(cbor_bytes)[0] + + l = [x for x in cbor_map] l_sorted = l[:] l_sorted = sorted(l_sorted, key=cmp_to_key(sort_cbor_keys)) + print("sorted", l_sorted) print("real", l) + for i in range(0, len(l)): if l[i] != l_sorted[i]: - raise ValueError("Cbor list item %d: %d is out of order" % (i, l[i])) + raise ValueError(f"Cbor list item {i}: {l[i]} is out of order") return l @@ -258,9 +281,12 @@ class FIDO2Tests(Tester): print(bytes(info)) print(cbor.loads(bytes(info))) - with Test("Check dictionary keys are sorted from lowest to highest"): + with Test("Check map keys are sorted correctly"): TestCborKeysSorted(bytes(info)) + with Test("Check options keys are sorted correctly"): + TestCborKeysSorted(cbor.loads(bytes(info))[0][4]) + with Test("Check FIDO2 string is in VERSIONS field"): assert "FIDO_2_0" in info.versions @@ -306,10 +332,14 @@ class FIDO2Tests(Tester): expectedError=CtapError.ERR.SUCCESS, ) - with Test("Check response dictionary keys are sorted from lowest to highest"): - last = 0 + with Test("Check response map keys are sorted correctly"): + TestCborKeysSorted(bytes(prev_reg)) - with Test("Check COSE KEY dictionary keys are sorted from lowest to highest"): + with Test("Check attestation statement map keys are sorted correctly"): + att_statement = cbor.loads(bytes(prev_reg))[0][3] + TestCborKeysSorted(att_statement) + + with Test("Check COSE KEY dictionary keys are sorted correctly"): last = 0 data = prev_reg.auth_data.credential_data c_len = struct.unpack(">H", data[16:18])[0] @@ -658,11 +688,12 @@ class FIDO2Tests(Tester): expectedError=CtapError.ERR.SUCCESS, ) - with Test("Check response dictionary keys are sorted from lowest to highest"): - last = 0 - print(cbor.loads(bytes(prev_auth))[0]) + with Test("Check response map keys are sorted correctly"): TestCborKeysSorted(bytes(prev_auth)) + with Test("Check credential map keys are sorted correctly"): + TestCborKeysSorted(cbor.loads(bytes(prev_auth))[0][1]) + with Test("Test auth_data is 37 bytes"): assert len(prev_auth.auth_data) == 37 From 7068be9cd5c3007d7c74791ed3e70864dd5af283 Mon Sep 17 00:00:00 2001 From: Conor Patrick Date: Wed, 10 Apr 2019 13:13:38 -0400 Subject: [PATCH 07/12] reorder options --- fido2/ctap.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/fido2/ctap.c b/fido2/ctap.c index ccc2a4f..c7f2d55 100644 --- a/fido2/ctap.c +++ b/fido2/ctap.c @@ -123,13 +123,6 @@ uint8_t ctap_get_info(CborEncoder * encoder) ret = cbor_encoder_create_map(&map, &options,4); check_ret(ret); { - ret = cbor_encode_text_string(&options, "plat", 4); - check_ret(ret); - { - ret = cbor_encode_boolean(&options, 0); // Not attached to platform - check_ret(ret); - } - ret = cbor_encode_text_string(&options, "rk", 2); check_ret(ret); { @@ -153,6 +146,15 @@ uint8_t ctap_get_info(CborEncoder * encoder) // ret = cbor_encode_boolean(&options, 0); // check_ret(ret); // } + + ret = cbor_encode_text_string(&options, "plat", 4); + check_ret(ret); + { + ret = cbor_encode_boolean(&options, 0); // Not attached to platform + check_ret(ret); + } + + ret = cbor_encode_text_string(&options, "clientPin", 9); check_ret(ret); { From c71bbd86892e7cb9870116d68fc37741ad4a4efd Mon Sep 17 00:00:00 2001 From: Conor Patrick Date: Wed, 10 Apr 2019 13:42:38 -0400 Subject: [PATCH 08/12] re-enable all tests --- tools/testing/tests/fido2.py | 67 +++++++++++++++--------------------- 1 file changed, 27 insertions(+), 40 deletions(-) diff --git a/tools/testing/tests/fido2.py b/tools/testing/tests/fido2.py index 3c6e80f..9c16eb8 100644 --- a/tools/testing/tests/fido2.py +++ b/tools/testing/tests/fido2.py @@ -73,15 +73,28 @@ def TestCborKeysSorted(cbor_bytes): l_sorted = l[:] l_sorted = sorted(l_sorted, key=cmp_to_key(sort_cbor_keys)) - print("sorted", l_sorted) - print("real", l) - for i in range(0, len(l)): if l[i] != l_sorted[i]: + print("sorted", l_sorted) + print("real", l) raise ValueError(f"Cbor list item {i}: {l[i]} is out of order") return l +# hot patch cbor map parsing to test the order of keys in map +_load_map_old = cbor.load_map + + +def _load_map_new(ai, data): + values, data = _load_map_old(ai, data) + TestCborKeysSorted(values) + return values, data + + +cbor.load_map = _load_map_new +cbor._DESERIALIZERS[5] = _load_map_new + + class FIDO2Tests(Tester): def __init__(self, tester=None): super().__init__(tester) @@ -281,12 +294,6 @@ class FIDO2Tests(Tester): print(bytes(info)) print(cbor.loads(bytes(info))) - with Test("Check map keys are sorted correctly"): - TestCborKeysSorted(bytes(info)) - - with Test("Check options keys are sorted correctly"): - TestCborKeysSorted(cbor.loads(bytes(info))[0][4]) - with Test("Check FIDO2 string is in VERSIONS field"): assert "FIDO_2_0" in info.versions @@ -332,20 +339,6 @@ class FIDO2Tests(Tester): expectedError=CtapError.ERR.SUCCESS, ) - with Test("Check response map keys are sorted correctly"): - TestCborKeysSorted(bytes(prev_reg)) - - with Test("Check attestation statement map keys are sorted correctly"): - att_statement = cbor.loads(bytes(prev_reg))[0][3] - TestCborKeysSorted(att_statement) - - with Test("Check COSE KEY dictionary keys are sorted correctly"): - last = 0 - data = prev_reg.auth_data.credential_data - c_len = struct.unpack(">H", data[16:18])[0] - cred_id = data[18 : 18 + c_len] - TestCborKeysSorted(data[18 + c_len :]) - allow_list = [ { "id": prev_reg.auth_data.credential_data.credential_id, @@ -688,12 +681,6 @@ class FIDO2Tests(Tester): expectedError=CtapError.ERR.SUCCESS, ) - with Test("Check response map keys are sorted correctly"): - TestCborKeysSorted(bytes(prev_auth)) - - with Test("Check credential map keys are sorted correctly"): - TestCborKeysSorted(cbor.loads(bytes(prev_auth))[0][1]) - with Test("Test auth_data is 37 bytes"): assert len(prev_auth.auth_data) == 37 @@ -1163,15 +1150,15 @@ class FIDO2Tests(Tester): self.test_get_info() self.test_get_assertion() - # + self.test_make_credential() - # - # self.test_rk(None) - # - # self.test_client_pin() - # - # self.testReset() - # - # self.test_extensions() - # - # print("Done") + + self.test_rk(None) + + self.test_client_pin() + + self.testReset() + + self.test_extensions() + + print("Done") From 46dd4fe818675e0f48d884f42eb230c99fb7129b Mon Sep 17 00:00:00 2001 From: Conor Patrick Date: Wed, 10 Apr 2019 13:56:23 -0400 Subject: [PATCH 09/12] unused import --- tools/testing/tests/fido2.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tools/testing/tests/fido2.py b/tools/testing/tests/fido2.py index 9c16eb8..7327d12 100644 --- a/tools/testing/tests/fido2.py +++ b/tools/testing/tests/fido2.py @@ -2,7 +2,6 @@ from __future__ import print_function, absolute_import, unicode_literals import time from random import randint import array -import struct from functools import cmp_to_key from fido2 import cbor From 7a49169492f37aa8e5b5e5255902f7e8ab9046be Mon Sep 17 00:00:00 2001 From: Conor Patrick Date: Thu, 11 Apr 2019 00:04:33 -0400 Subject: [PATCH 10/12] consider major type and refactor --- tools/testing/tests/fido2.py | 59 ++++++++++++++++++------------------ 1 file changed, 29 insertions(+), 30 deletions(-) diff --git a/tools/testing/tests/fido2.py b/tools/testing/tests/fido2.py index 7327d12..8b388f5 100644 --- a/tools/testing/tests/fido2.py +++ b/tools/testing/tests/fido2.py @@ -34,49 +34,48 @@ def VerifyAttestation(attest, data): verifier().verify(attest.att_statement, attest.auth_data, data.hash) +def cbor_key_to_representative(key): + if isinstance(key, int): + if key >= 0: + return (0, key) + return (1, -key) + elif isinstance(key, bytes): + return (2, key) + elif isinstance(key, str): + return (3, key) + else: + raise ValueError(key) + + +def cmp_cbor_keys(a, b): + a = cbor_key_to_representative(a) + b = cbor_key_to_representative(b) + if a[0] != b[0]: + return a[0] - b[0] + return not ((a[1] > b[1]) - (a[1] < b[1])) + + def TestCborKeysSorted(cbor_bytes): # Cbor canonical ordering of keys. # https://fidoalliance.org/specs/fido-v2.0-ps-20190130/fido-client-to-authenticator-protocol-v2.0-ps-20190130.html#ctap2-canonical-cbor-encoding-form - def sort_cbor_keys(x1, x2): - # >0 x1 goes second, <0 x1 goes first - if isinstance(x1, int) and isinstance(x2, int): - if x1 >= 0 and x2 >= 0: - return x1 - x2 - elif x1 < 0 and x2 >= 0: - return 1 - elif x1 >= 0 and x2 < 0: - return -1 - else: - return -x1 + x2 - - if isinstance(x1, (str, bytes)) and isinstance(x2, int): - return 1 - elif isinstance(x1, int) and isinstance(x2, (str, bytes)): - return -1 - - if len(x1) == len(x2): - for i in range(0, len(x1)): - v1 = x1[i] if isinstance(x1[i], int) else ord(x1[i]) - v2 = x2[i] if isinstance(x2[i], int) else ord(x2[i]) - if v1 != v2: - return v1 - v2 - return 0 - - return len(x1) - len(x2) cbor_map = cbor_bytes if isinstance(cbor_map, bytes): cbor_map = cbor.loads(cbor_bytes)[0] l = [x for x in cbor_map] - l_sorted = l[:] - l_sorted = sorted(l_sorted, key=cmp_to_key(sort_cbor_keys)) + l_sorted = sorted(l[:], key=cmp_to_key(cmp_cbor_keys)) + + for i in range(len(l)): + + if not isinstance(l[i], (str, int)): + raise ValueError(f"Cbor map key {l[i]} must be int or str for CTAP2") - for i in range(0, len(l)): if l[i] != l_sorted[i]: print("sorted", l_sorted) print("real", l) - raise ValueError(f"Cbor list item {i}: {l[i]} is out of order") + raise ValueError(f"Cbor map item {i}: {l[i]} is out of order") + return l From ca80329b4c0741946de14509710c6d2a586fe0ec Mon Sep 17 00:00:00 2001 From: Conor Patrick Date: Thu, 11 Apr 2019 13:22:55 -0400 Subject: [PATCH 11/12] Compare string length and sort from start of string --- tools/testing/tests/fido2.py | 55 +++++++++++++++++++++++++++++++----- 1 file changed, 48 insertions(+), 7 deletions(-) diff --git a/tools/testing/tests/fido2.py b/tools/testing/tests/fido2.py index 8b388f5..d3f0865 100644 --- a/tools/testing/tests/fido2.py +++ b/tools/testing/tests/fido2.py @@ -47,25 +47,46 @@ def cbor_key_to_representative(key): raise ValueError(key) +def cbor_str_cmp(a, b): + if isinstance(a, str) or isinstance(b, str): + a = a.encode("utf8") + b = b.encode("utf8") + + if len(a) == len(b): + for x, y in zip(a, b): + if x != y: + return x - y + return 0 + else: + return len(a) - len(b) + + def cmp_cbor_keys(a, b): a = cbor_key_to_representative(a) b = cbor_key_to_representative(b) if a[0] != b[0]: return a[0] - b[0] - return not ((a[1] > b[1]) - (a[1] < b[1])) + if a[0] in (2, 3): + return cbor_str_cmp(a[1], b[1]) + else: + return (a[1] > b[1]) - (a[1] < b[1]) -def TestCborKeysSorted(cbor_bytes): +def TestCborKeysSorted(cbor_obj): # Cbor canonical ordering of keys. # https://fidoalliance.org/specs/fido-v2.0-ps-20190130/fido-client-to-authenticator-protocol-v2.0-ps-20190130.html#ctap2-canonical-cbor-encoding-form - cbor_map = cbor_bytes - if isinstance(cbor_map, bytes): - cbor_map = cbor.loads(cbor_bytes)[0] + if isinstance(cbor_obj, bytes): + cbor_obj = cbor.loads(cbor_obj)[0] + + if isinstance(cbor_obj, dict): + l = [x for x in cbor_obj] + else: + l = cbor_obj - l = [x for x in cbor_map] l_sorted = sorted(l[:], key=cmp_to_key(cmp_cbor_keys)) - + print(l) + print(l_sorted) for i in range(len(l)): if not isinstance(l[i], (str, int)): @@ -96,6 +117,26 @@ cbor._DESERIALIZERS[5] = _load_map_new class FIDO2Tests(Tester): def __init__(self, tester=None): super().__init__(tester) + self.self_test() + + def self_test(self,): + cbor_key_list_sorted = [ + 0, + 1, + 1, + 2, + 3, + -1, + -2, + "b", + "c", + "aa", + "aaa", + "aab", + "baa", + "bbb", + ] + TestCborKeysSorted(cbor_key_list_sorted) def run(self,): self.test_fido2() From 78579c27dc4439fc38f5d4d50b993e345b07d836 Mon Sep 17 00:00:00 2001 From: Conor Patrick Date: Thu, 11 Apr 2019 13:42:17 -0400 Subject: [PATCH 12/12] Refactor and self test the CBOR sorting --- tools/testing/tests/fido2.py | 20 +++++++++++++++----- tools/testing/tests/tester.py | 11 +++++++++-- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/tools/testing/tests/fido2.py b/tools/testing/tests/fido2.py index d3f0865..60fdb05 100644 --- a/tools/testing/tests/fido2.py +++ b/tools/testing/tests/fido2.py @@ -85,16 +85,13 @@ def TestCborKeysSorted(cbor_obj): l = cbor_obj l_sorted = sorted(l[:], key=cmp_to_key(cmp_cbor_keys)) - print(l) - print(l_sorted) + for i in range(len(l)): if not isinstance(l[i], (str, int)): raise ValueError(f"Cbor map key {l[i]} must be int or str for CTAP2") if l[i] != l_sorted[i]: - print("sorted", l_sorted) - print("real", l) raise ValueError(f"Cbor map item {i}: {l[i]} is out of order") return l @@ -136,7 +133,20 @@ class FIDO2Tests(Tester): "baa", "bbb", ] - TestCborKeysSorted(cbor_key_list_sorted) + with Test("Self test CBOR sorting"): + TestCborKeysSorted(cbor_key_list_sorted) + + with Test("Self test CBOR sorting integers", catch=ValueError): + TestCborKeysSorted([1, 0]) + + with Test("Self test CBOR sorting major type", catch=ValueError): + TestCborKeysSorted([-1, 0]) + + with Test("Self test CBOR sorting strings", catch=ValueError): + TestCborKeysSorted(["bb", "a"]) + + with Test("Self test CBOR sorting same length strings", catch=ValueError): + TestCborKeysSorted(["ab", "aa"]) def run(self,): self.test_fido2() diff --git a/tools/testing/tests/tester.py b/tools/testing/tests/tester.py index e0ae813..10cd1d3 100644 --- a/tools/testing/tests/tester.py +++ b/tools/testing/tests/tester.py @@ -28,14 +28,21 @@ class Packet(object): class Test: - def __init__(self, msg): + def __init__(self, msg, catch=None): self.msg = msg + self.catch = catch def __enter__(self,): print(self.msg) def __exit__(self, a, b, c): - print("Pass") + if self.catch is None: + print("Pass") + elif isinstance(b, self.catch): + print("Pass") + return b + else: + raise RuntimeError(f"Expected exception {self.catch} did not occur.") class Tester: