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

Expose AAGUID and use it for packed basic attestation #127

Merged
merged 3 commits into from
Mar 12, 2019

Conversation

bdewater
Copy link
Collaborator

I noticed we missed a step from the spec for verifying packed attestation:

If attestnCert contains an extension with OID 1.3.6.1.4.1.45724.1.1.4 (id-fido-gen-ce-aaguid) verify that the value of this extension matches the aaguid in authenticatorData.

I needed to change the tests from #81 to have an attestation certificate with the AAGUID extension and real authenticator_data.

We'll also need the AAGUID for lookup in the Metadata Service (see #66).

@@ -7,6 +7,8 @@ module AttestationStatement
class Base
class NotSupportedError < Error; end

AAGUID_EXTENSION_OID = "1.3.6.1.4.1.45724.1.1.4"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Putting this here since TPM attestation (which I plan to work on in the next few weeks) needs this too.

Copy link
Contributor

@grzuy grzuy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good finding.
Thank you!

end
end

context "when packed attestation (basic attestation)" do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for moving this test case here, makes sense!

lib/webauthn/authenticator_attestation_response.rb Outdated Show resolved Hide resolved
lib/webauthn/authenticator_data.rb Outdated Show resolved Hide resolved
lib/webauthn/attestation_statement/packed.rb Outdated Show resolved Hide resolved
@bdewater
Copy link
Collaborator Author

Addressed your comments, please have another look @grzuy

@grzuy
Copy link
Contributor

grzuy commented Mar 12, 2019

Great, thank you!

@grzuy grzuy merged commit 7613599 into cedarcode:master Mar 12, 2019
@bdewater bdewater deleted the aaguid branch March 12, 2019 20:32
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 this pull request may close these issues.

None yet

2 participants