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

cbor2 exceptions are not caught #176

Closed
imran-iq opened this issue Sep 15, 2023 · 3 comments · Fixed by #179
Closed

cbor2 exceptions are not caught #176

imran-iq opened this issue Sep 15, 2023 · 3 comments · Fixed by #179

Comments

@imran-iq
Copy link

If an invalid payload is sent to a server that uses verify_registration_response

Example:

index 7dab1df..b711a31 100644
--- tests/test_verify_registration_response.py
+++ tests/test_verify_registration_response.py
@@ -21,7 +21,7 @@ class TestVerifyRegistrationResponse(TestCase):
             "id": "9y1xA8Tmg1FEmT-c7_fvWZ_uoTuoih3OvR45_oAK-cwHWhAbXrl2q62iLVTjiyEZ7O7n-CROOY494k7Q3xrs_w",
             "rawId": "9y1xA8Tmg1FEmT-c7_fvWZ_uoTuoih3OvR45_oAK-cwHWhAbXrl2q62iLVTjiyEZ7O7n-CROOY494k7Q3xrs_w",
             "response": {
-                "attestationObject": "o2NmbXRkbm9uZWdhdHRTdG10oGhhdXRoRGF0YVjESZYN5YgOjGh0NBcPZHZgW4_krrmihjLHmVzzuoMdl2NFAAAAFwAAAAAAAAAAAAAAAAAAAAAAQPctcQPE5oNRRJk_nO_371mf7qE7qIodzr0eOf6ACvnMB1oQG165dqutoi1U44shGezu5_gkTjmOPeJO0N8a7P-lAQIDJiABIVggSFbUJF-42Ug3pdM8rDRFu_N5oiVEysPDB6n66r_7dZAiWCDUVnB39FlGypL-qAoIO9xWHtJygo2jfDmHl-_eKFRLDA",
+                "attestationObject": "",
                 "clientDataJSON": "eyJ0eXBlIjoid2ViYXV0aG4uY3JlYXRlIiwiY2hhbGxlbmdlIjoiVHdON240V1R5R0tMYzRaWS1xR3NGcUtuSE00bmdscXN5VjBJQ0psTjJUTzlYaVJ5RnRya2FEd1V2c3FsLWdrTEpYUDZmbkYxTWxyWjUzTW00UjdDdnciLCJvcmlnaW4iOiJodHRwOi8vbG9jYWxob3N0OjUwMDAiLCJjcm9zc09yaWdpbiI6ZmFsc2V9"
             },
             "type": "public-key",

Error when running test:

======================================================================
ERROR: test_verifies_none_attestation_response (tests.test_verify_registration_response.TestVerifyRegistrationResponse.test_verifies_none_attestation_response)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/iiqbal/code/upstream/py_webauthn/tests/test_verify_registration_response.py", line 42, in test_verifies_none_attestation_response
    verification = verify_registration_response(
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/iiqbal/code/upstream/py_webauthn/webauthn/registration/verify_registration_response.py", line 141, in verify_registration_response
    attestation_object = parse_attestation_object(response.attestation_object)
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/iiqbal/code/upstream/py_webauthn/webauthn/helpers/parse_attestation_object.py", line 13, in parse_attestation_object
    attestation_dict = cbor2.loads(val)
                       ^^^^^^^^^^^^^^^^
_cbor2.CBORDecodeEOF: premature end of stream (expected to read 1 bytes, got 0 instead)

----------------------------------------------------------------------

Thoughts on adding a cbor parsing exception similar to the client data json exception?

@imran-iq
Copy link
Author

It's also worth noting that cbo2 is throwing the wrong type of exception at the moment too: agronholm/cbor2#174

@MasterKale
Copy link
Collaborator

I've created #176 to offer up a py_webauthn-specific error code that can be detected when cbor2 fails to decode data. The original error is included for full transparency of what went wrong, but the main exception will be one out of webauthn.helpers.exceptions.

I acknowledge I can do more to smooth out CBOR-related issues. I think I'll try and think through that when I attempt to abstract Pydantic-specific errors as part of addressing #173.

@MasterKale
Copy link
Collaborator

Improved error reporting from CBOR-related decoding errors is now available in webauthn==1.11.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants