-
Notifications
You must be signed in to change notification settings - Fork 61
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
Checking AAGUID when attestation type is "direct" #239
Comments
Hi @chrismccaw 👋 First off two small things:
Assuming you're sure you need attestation of a device: the PR shows a hint of what direction I believe this should go in. Per § 6.4. Attestation
Whatever you application as an RP should accept can be dependant on so many different things that is impossible to encapsulate in a library. For example, to pass FIDO Conformance Tool tests we need to raise if the metadata has the authenticator is known to be compromised: public_key_credential.verify(expected_challenge)
reports = public_key_credential.response.metadata_entry.status_reports
raise("bad authenticator status") if reports.any? do |report|
["USER_VERIFICATION_BYPASS", "ATTESTATION_KEY_COMPROMISE", "USER_KEY_REMOTE_COMPROMISE",
"USER_KEY_PHYSICAL_COMPROMISE", "REVOKED"].include?(report.status)
end But your use case might be different, e.g. you could want to check with the MDS if the authenticator claiming to be capable of user verification is actually so before allowing it to be used as multi-factor login. Use cases I'd imagine are banks, employers, governments, etc. Interesting to see Chrome now returns a "none" attestation format, that used to be a "NotAllowedError" DOMException. That means you can now check on your server whether the "none" attestation format is acceptable for your use cases via |
@bdewater This is great information. I appreciate you putting it together including the references. The PR you have created looks like the right direction to go in for Full attestation and would be amazing to get the MDS in place Does adding the "direct' attestation perform any sort of extra checking currently? Is there any point using it right now with this library? |
Currently from the top of my head only Safetynet does checks to verify the attestation certificates chain up correctly to a trusted root. The others do not so in theory anyone could have generated the attestation, so no security benefits at the moment. |
Chiming in just to share this link which is relevant to this discussion in some way: https://www.w3.org/TR/webauthn-2/#sctn-attestation-limitations. |
I think it is best to not use The best short term solution is something like this: https://www.w3.org/TR/webauthn-2/#sctn-attestation-limitations. |
@chrismccaw #208 is ready for review, I would appreciate any feedback you might have :) |
Awesome. Will take a look |
@bdewater I agree we wouldn't be able to capture every use case out there. I wonder, however, if we might be able to capture some very general policies regarding "Attestation". I mean, back to the original question from @chrismccaw in this issue, shouldn't the user be able to configure in some way, e.g. that "None" is not an acceptable under policy (Step 16 in https://www.w3.org/TR/webauthn-2/#sctn-registering-a-new-credential), and get the library to fail verification? |
Hi, Discussion here has been helpful and interesting. However, closing for now, given I don't see any obvious actionable item based on the issue title and description. If either of you think otherwise, let me know. We can re-open or create a new more concrete issue. Thank you both! |
I am using Chrome and the
attestation
type is "direct". When I register an additional popup appears asking to "Allow" or "Block" my authenticator details. Even though I "block" the request the#verify
is still successful. Should the#verify
check the presence of theAAGUID
for fido whendirect
attestation is set?The text was updated successfully, but these errors were encountered: