Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

176-better-cbor-parsing-error-handling #179

Merged
merged 3 commits into from
Sep 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 30 additions & 1 deletion tests/test_verify_registration_response.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
)
2 changes: 2 additions & 0 deletions webauthn/helpers/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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",
Expand Down
4 changes: 4 additions & 0 deletions webauthn/helpers/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,7 @@ class InvalidCertificateChain(Exception):

class InvalidBackupFlags(Exception):
pass


class InvalidCBORData(Exception):
pass
5 changes: 2 additions & 3 deletions webauthn/helpers/parse_attestation_object.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
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:
"""
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"],
Expand Down
5 changes: 3 additions & 2 deletions webauthn/helpers/parse_authenticator_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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)

Expand All @@ -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
Expand Down
22 changes: 22 additions & 0 deletions webauthn/helpers/parse_cbor.py
Original file line number Diff line number Diff line change
@@ -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
4 changes: 2 additions & 2 deletions webauthn/registration/formats/android_key.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -16,6 +15,7 @@
from webauthn.helpers import (
decode_credential_public_key,
decoded_public_key_to_cryptography,
parse_cbor,
validate_certificate_chain,
verify_signature,
)
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions webauthn/registration/formats/android_safetynet.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion webauthn/registration/formats/apple.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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()
Expand Down
4 changes: 2 additions & 2 deletions webauthn/registration/formats/packed.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import hashlib
from typing import List

import cbor2
from cryptography import x509
from cryptography.exceptions import InvalidSignature
from cryptography.hazmat.backends import default_backend

from webauthn.helpers import (
decode_credential_public_key,
decoded_public_key_to_cryptography,
parse_cbor,
validate_certificate_chain,
verify_signature,
)
Expand Down Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion webauthn/registration/formats/tpm.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from webauthn.helpers import (
decode_credential_public_key,
hash_by_alg,
parse_cbor,
validate_certificate_chain,
verify_signature,
)
Expand Down Expand Up @@ -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(
Expand Down