duo-labs / py_webauthn

Pythonic WebAuthn 🐍
https://duo-labs.github.io/py_webauthn
BSD 3-Clause "New" or "Revised" License
856 stars 171 forks source link

Device trusted_attestation_roots #194

Closed 0xC4DE closed 9 months ago

0xC4DE commented 9 months ago

Hello, I am currently updating an application that utilizes an old version of this library (<1.0). In this version, there was reference to a folder named trusted_attestation_roots in WebAuthnRegistrationResponse. (https://github.com/duo-labs/py_webauthn/blob/2c2c9f7a16cdd96531d3dea8acbb70863bf1f6c3/webauthn/webauthn.py#L255) That was used for the purpose of allowing only specific attestation certificates during registration.

It is unclear to me if there is a similar flow available in the latest version of this library, I do see the flows available in https://github.com/duo-labs/py_webauthn/blob/master/tests/test_verify_registration_response_android_key.py, and other related test files. But I am uncertain if it functions at-all similarly.

Ideally, something in verify_registration_response that can function similarly to the old trusted_attestation_roots would be ideal. But something such as the following:

verify_registration response(
    credential=credential,
    ...,
    attestation_allowed=[AttestationFormat.FIDO_U2F]
)

would also be acceptable, I think.

MasterKale commented 9 months ago

Does the pem_root_certs_bytes_by_fmt argument in verify_registration_response() not fulfill your needs?

https://github.com/duo-labs/py_webauthn/blob/494373e539050b96130c28477289feec65e5e19f/webauthn/registration/verify_registration_response.py#L76-L78

It doesn't do exactly what the old trust_anchor_dir and trusted_attestation_cert_required enabled, but you should be able to achieve something similar by 1) specifying root certs in pem_root_certs_bytes_by_fmt for AttestationFormat.FIDO_U2F, and then 2) rejecting if VerifiedRegistration.fmt is not AttestationFormat.FIDO_U2F.

verification = verify_registration_response(
    credential=credential,
    expected_challenge=challenge,
    expected_origin=expected_origin,
    expected_rp_id=rp_id,
    pem_root_certs_bytes_by_fmt={
        AttestationFormat.FIDO_U2F: [CUSTOM_CERT_PEM.encode("ascii")]
    },
)

if verification.fmt is not AttestationFormat.FIDO_U2F:
    raise Exception("Detected unsupported attestation format")

Something like that 🤔

0xC4DE commented 9 months ago

Hmm. It would be nice if it did handle loading the certs from a filesystem instead of expecting bytes in x509, there is a helper for it though. Pretty sure your solution should work.

Would it be true that pem_root_certs_bytes_by_fmt would only validate certificates that exist under the specific certs provided in CUSTOM_CERT_PEM? I'm unclear since under the hood the pem_root_certs_bytes_by_fmt only extends and maybe allows certs that are signed by FIDO_U2F, but dont exist in the set of custom certs. Instead just providing stronger validation for those that exist under the root FIDO_U2F cert.

As an aside I'm having weird issues with my yubikey, the attestation is coming up as none (at a higher level than this library), even though it should be coming up with something. (I forgot to set attestation=AttestationConveyancePreference.DIRECT, also Yubi uses AttestationFormat.PACKED)

MasterKale commented 9 months ago

Would it be true that pem_root_certs_bytes_by_fmt would only validate certificates that exist under the specific certs provided in CUSTOM_CERT_PEM?

Yes, right now no certificate chain validation occurs because py_webauthn doesn't provide any default certificates. The "extends" you mention isn't augmenting any existing list of certificates with those provided in pem_root_certs_bytes_by_fmt - [].extends([]) is a pythonic way to append values in one list into another list, so it's all one big list.

https://github.com/duo-labs/py_webauthn/blob/eb3b6a3a8466dfe59c9600a1d0214f05f8ed9576/webauthn/registration/verify_registration_response.py#L202-L208

pem_root_certs_bytes is always initially empty, and I'm simply adding into it whatever certs might have been specified in pem_root_certs_bytes_by_fmt for the fmt specified in the attestation statement. I've left it as an exercise for the RP dev as to which certs actually get used for attestation statement certificate chain validation.

MasterKale commented 9 months ago

Yes, right now no certificate chain validation occurs because py_webauthn doesn't provide any default certificates.

I should clarify myself here, a few attestation statement formats include their own known root certificates which means certificate chain validation does occur even if you don't specify any values for pem_root_certs_bytes_by_fmt. This includes "apple", "android-safetynet", and "android-key":

https://github.com/duo-labs/py_webauthn/blob/6c0c5449e4c0c032511dae9f51316c4b6cf85e2e/webauthn/registration/formats/apple.py#L45-L52

https://github.com/duo-labs/py_webauthn/blob/6c0c5449e4c0c032511dae9f51316c4b6cf85e2e/webauthn/registration/formats/android_safetynet.py#L163-L172

https://github.com/duo-labs/py_webauthn/blob/6c0c5449e4c0c032511dae9f51316c4b6cf85e2e/webauthn/registration/formats/android_key.py#L67-L75

For formats like these pem_root_certs_bytes_by_fmt also gives RP's a way to specify updated root certificates if the root certificates included in this package expire. Ideally it'll never need to be used for this but just in case it's possible.

MasterKale commented 9 months ago

I don't think there's anything more to do here so I'm going to close this out. Please feel free to reopen if I've missed an actual issue with the library.