From b75daf87e424f57501c09a57fe592969e396a1aa Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Fri, 29 Sep 2023 12:50:20 -0700 Subject: [PATCH 1/3] Create new parse_cbor to encapsulate cbor2 usage --- webauthn/helpers/__init__.py | 2 ++ webauthn/helpers/exceptions.py | 4 ++++ webauthn/helpers/parse_cbor.py | 22 ++++++++++++++++++++++ 3 files changed, 28 insertions(+) create mode 100644 webauthn/helpers/parse_cbor.py diff --git a/webauthn/helpers/__init__.py b/webauthn/helpers/__init__.py index ff3fdd9..5a2ded0 100644 --- a/webauthn/helpers/__init__.py +++ b/webauthn/helpers/__init__.py @@ -12,6 +12,7 @@ from .parse_authentication_credential_json import parse_authentication_credential_json from .parse_authenticator_data import parse_authenticator_data from .parse_backup_flags import parse_backup_flags +from .parse_cbor import parse_cbor from .parse_client_data_json import parse_client_data_json from .parse_registration_credential_json import parse_registration_credential_json from .validate_certificate_chain import validate_certificate_chain @@ -33,6 +34,7 @@ "parse_authenticator_data", "parse_authentication_credential_json", "parse_backup_flags", + "parse_cbor", "parse_client_data_json", "parse_registration_credential_json", "validate_certificate_chain", diff --git a/webauthn/helpers/exceptions.py b/webauthn/helpers/exceptions.py index b26fc9c..1f58ded 100644 --- a/webauthn/helpers/exceptions.py +++ b/webauthn/helpers/exceptions.py @@ -52,3 +52,7 @@ class InvalidCertificateChain(Exception): class InvalidBackupFlags(Exception): pass + + +class InvalidCBORData(Exception): + pass diff --git a/webauthn/helpers/parse_cbor.py b/webauthn/helpers/parse_cbor.py new file mode 100644 index 0000000..a98ae49 --- /dev/null +++ b/webauthn/helpers/parse_cbor.py @@ -0,0 +1,22 @@ +from typing import Any + +import cbor2 + +from .exceptions import InvalidCBORData + + +def parse_cbor(data: bytes) -> Any: + """ + Attempt to decode CBOR-encoded data. + + Raises: + `helpers.exceptions.InvalidCBORData` if data cannot be decoded + """ + try: + to_return = cbor2.loads(data) + except Exception as exc: + raise InvalidCBORData( + "Could not decode CBOR data" + ) from exc + + return to_return From c531ab0fe8532b697d40e17d69a524dae8fe58f1 Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Fri, 29 Sep 2023 12:57:03 -0700 Subject: [PATCH 2/3] Use parse_cbor over cbor2.loads --- webauthn/helpers/parse_attestation_object.py | 5 ++--- webauthn/helpers/parse_authenticator_data.py | 5 +++-- webauthn/registration/formats/android_key.py | 4 ++-- webauthn/registration/formats/android_safetynet.py | 4 ++-- webauthn/registration/formats/apple.py | 3 ++- webauthn/registration/formats/packed.py | 4 ++-- webauthn/registration/formats/tpm.py | 3 ++- 7 files changed, 15 insertions(+), 13 deletions(-) diff --git a/webauthn/helpers/parse_attestation_object.py b/webauthn/helpers/parse_attestation_object.py index 042899a..42ce801 100644 --- a/webauthn/helpers/parse_attestation_object.py +++ b/webauthn/helpers/parse_attestation_object.py @@ -1,8 +1,7 @@ -import cbor2 - from .parse_attestation_statement import parse_attestation_statement from .parse_authenticator_data import parse_authenticator_data from .structs import AttestationObject +from .parse_cbor import parse_cbor def parse_attestation_object(val: bytes) -> AttestationObject: @@ -10,7 +9,7 @@ def parse_attestation_object(val: bytes) -> AttestationObject: Decode and peel apart the CBOR-encoded blob `response.attestationObject` into structured data. """ - attestation_dict = cbor2.loads(val) + attestation_dict = parse_cbor(val) decoded_attestation_object = AttestationObject( fmt=attestation_dict["fmt"], diff --git a/webauthn/helpers/parse_authenticator_data.py b/webauthn/helpers/parse_authenticator_data.py index 5792def..7d58de1 100644 --- a/webauthn/helpers/parse_authenticator_data.py +++ b/webauthn/helpers/parse_authenticator_data.py @@ -2,6 +2,7 @@ from .exceptions import InvalidAuthenticatorDataStructure from .structs import AttestedCredentialData, AuthenticatorData, AuthenticatorDataFlags +from .parse_cbor import parse_cbor def parse_authenticator_data(val: bytes) -> AuthenticatorData: @@ -75,7 +76,7 @@ def parse_authenticator_data(val: bytes) -> AuthenticatorData: val = bytes(_val) # Load the next CBOR-encoded value - credential_public_key = cbor2.loads(val[pointer:]) + credential_public_key = parse_cbor(val[pointer:]) credential_public_key_bytes = cbor2.dumps(credential_public_key) pointer += len(credential_public_key_bytes) @@ -87,7 +88,7 @@ def parse_authenticator_data(val: bytes) -> AuthenticatorData: authenticator_data.attested_credential_data = attested_cred_data if flags.ed is True: - extension_object = cbor2.loads(val[pointer:]) + extension_object = parse_cbor(val[pointer:]) extension_bytes = cbor2.dumps(extension_object) pointer += len(extension_bytes) authenticator_data.extensions = extension_bytes diff --git a/webauthn/registration/formats/android_key.py b/webauthn/registration/formats/android_key.py index 10906e9..9c441d1 100644 --- a/webauthn/registration/formats/android_key.py +++ b/webauthn/registration/formats/android_key.py @@ -1,7 +1,6 @@ import hashlib from typing import List -import cbor2 from cryptography import x509 from cryptography.exceptions import InvalidSignature from cryptography.hazmat.backends import default_backend @@ -16,6 +15,7 @@ from webauthn.helpers import ( decode_credential_public_key, decoded_public_key_to_cryptography, + parse_cbor, validate_certificate_chain, verify_signature, ) @@ -79,7 +79,7 @@ def verify_android_key( raise InvalidRegistrationResponse(f"{err} (Android Key)") # Extract attStmt bytes from attestation_object - attestation_dict = cbor2.loads(attestation_object) + attestation_dict = parse_cbor(attestation_object) authenticator_data_bytes = attestation_dict["authData"] # Generate a hash of client_data_json diff --git a/webauthn/registration/formats/android_safetynet.py b/webauthn/registration/formats/android_safetynet.py index 5acf807..8da9f47 100644 --- a/webauthn/registration/formats/android_safetynet.py +++ b/webauthn/registration/formats/android_safetynet.py @@ -2,7 +2,6 @@ import hashlib from typing import List -import cbor2 from cryptography import x509 from cryptography.exceptions import InvalidSignature from cryptography.hazmat.backends import default_backend @@ -11,6 +10,7 @@ from webauthn.helpers.cose import COSEAlgorithmIdentifier from webauthn.helpers import ( base64url_to_bytes, + parse_cbor, validate_certificate_chain, verify_safetynet_timestamp, verify_signature, @@ -101,7 +101,7 @@ def verify_android_safetynet( # clientDataHash. # Extract attStmt bytes from attestation_object - attestation_dict = cbor2.loads(attestation_object) + attestation_dict = parse_cbor(attestation_object) authenticator_data_bytes = attestation_dict["authData"] # Generate a hash of client_data_json diff --git a/webauthn/registration/formats/apple.py b/webauthn/registration/formats/apple.py index b1daa1a..63cf664 100644 --- a/webauthn/registration/formats/apple.py +++ b/webauthn/registration/formats/apple.py @@ -15,6 +15,7 @@ from webauthn.helpers import ( decode_credential_public_key, decoded_public_key_to_cryptography, + parse_cbor, validate_certificate_chain, ) from webauthn.helpers.exceptions import ( @@ -55,7 +56,7 @@ def verify_apple( raise InvalidRegistrationResponse(f"{err} (Apple)") # Concatenate authenticatorData and clientDataHash to form nonceToHash. - attestation_dict = cbor2.loads(attestation_object) + attestation_dict = parse_cbor(attestation_object) authenticator_data_bytes = attestation_dict["authData"] client_data_hash = hashlib.sha256() diff --git a/webauthn/registration/formats/packed.py b/webauthn/registration/formats/packed.py index 39119ca..4c9849b 100644 --- a/webauthn/registration/formats/packed.py +++ b/webauthn/registration/formats/packed.py @@ -1,7 +1,6 @@ import hashlib from typing import List -import cbor2 from cryptography import x509 from cryptography.exceptions import InvalidSignature from cryptography.hazmat.backends import default_backend @@ -9,6 +8,7 @@ from webauthn.helpers import ( decode_credential_public_key, decoded_public_key_to_cryptography, + parse_cbor, validate_certificate_chain, verify_signature, ) @@ -42,7 +42,7 @@ def verify_packed( ) # Extract attStmt bytes from attestation_object - attestation_dict = cbor2.loads(attestation_object) + attestation_dict = parse_cbor(attestation_object) authenticator_data_bytes = attestation_dict["authData"] # Generate a hash of client_data_json diff --git a/webauthn/registration/formats/tpm.py b/webauthn/registration/formats/tpm.py index a1d775b..84a3d59 100644 --- a/webauthn/registration/formats/tpm.py +++ b/webauthn/registration/formats/tpm.py @@ -18,6 +18,7 @@ from webauthn.helpers import ( decode_credential_public_key, hash_by_alg, + parse_cbor, validate_certificate_chain, verify_signature, ) @@ -153,7 +154,7 @@ def verify_tpm( ) # Concatenate authenticatorData and clientDataHash to form attToBeSigned. - attestation_dict = cbor2.loads(attestation_object) + attestation_dict = parse_cbor(attestation_object) authenticator_data_bytes: bytes = attestation_dict["authData"] client_data_hash = hash_by_alg(client_data_json) att_to_be_signed = b"".join( From 6f28bcfac1972fdfce1334a129b5db0515eadbdc Mon Sep 17 00:00:00 2001 From: Matthew Miller Date: Fri, 29 Sep 2023 12:57:14 -0700 Subject: [PATCH 3/3] Add new unit test --- tests/test_verify_registration_response.py | 31 +++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/tests/test_verify_registration_response.py b/tests/test_verify_registration_response.py index e8949cf..c693c90 100644 --- a/tests/test_verify_registration_response.py +++ b/tests/test_verify_registration_response.py @@ -4,7 +4,7 @@ import cbor2 from pydantic import ValidationError from webauthn.helpers import base64url_to_bytes, bytes_to_base64url, parse_registration_credential_json -from webauthn.helpers.exceptions import InvalidRegistrationResponse +from webauthn.helpers.exceptions import InvalidRegistrationResponse, InvalidCBORData from webauthn.helpers.known_root_certs import globalsign_r2 from webauthn.helpers.structs import ( AttestationFormat, @@ -251,3 +251,32 @@ def test_supports_dict_credential(self) -> None: ) assert verification.fmt == AttestationFormat.NONE + + def test_raises_useful_error_on_bad_attestation_object(self) -> None: + credential = { + "id": "9y1xA8Tmg1FEmT-c7_fvWZ_uoTuoih3OvR45_oAK-cwHWhAbXrl2q62iLVTjiyEZ7O7n-CROOY494k7Q3xrs_w", + "rawId": "9y1xA8Tmg1FEmT-c7_fvWZ_uoTuoih3OvR45_oAK-cwHWhAbXrl2q62iLVTjiyEZ7O7n-CROOY494k7Q3xrs_w", + "response": { + "attestationObject": "", + "clientDataJSON": "eyJ0eXBlIjoid2ViYXV0aG4uY3JlYXRlIiwiY2hhbGxlbmdlIjoiVHdON240V1R5R0tMYzRaWS1xR3NGcUtuSE00bmdscXN5VjBJQ0psTjJUTzlYaVJ5RnRya2FEd1V2c3FsLWdrTEpYUDZmbkYxTWxyWjUzTW00UjdDdnciLCJvcmlnaW4iOiJodHRwOi8vbG9jYWxob3N0OjUwMDAiLCJjcm9zc09yaWdpbiI6ZmFsc2V9" + }, + "type": "public-key", + "clientExtensionResults": {}, + "transports": [ + "cable" + ] + } + + challenge = base64url_to_bytes( + "TwN7n4WTyGKLc4ZY-qGsFqKnHM4nglqsyV0ICJlN2TO9XiRyFtrkaDwUvsql-gkLJXP6fnF1MlrZ53Mm4R7Cvw" + ) + rp_id = "localhost" + expected_origin = "http://localhost:5000" + + with self.assertRaises(InvalidCBORData): + verify_registration_response( + credential=credential, + expected_challenge=challenge, + expected_origin=expected_origin, + expected_rp_id=rp_id, + )