-
Notifications
You must be signed in to change notification settings - Fork 53
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
feat: verify TPM attestation statement #162
Conversation
end | ||
|
||
def valid_name | ||
[t_public.name_alg].pack("n") + digest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This approach is what I understood should be done by reading W3C recommendation.
However, conformance tool fails with this approach. See discussion in fido-alliance/conformance-test-tools-resources#396.
The following PR implements the other approach which apparently is inconsistent with the W3C recommendation, but passes the conformance tool test: #164.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merged #164 into this branch. Going with the conformance tool approach for now.
aik_certificate.subject.eql?(CERTIFICATE_EMPTY_NAME) && | ||
certificate_in_use?(aik_certificate) && | ||
aik_certificate.extensions.find { |ext| ext.oid == 'basicConstraints' }&.value == "CA:FALSE" && | ||
aik_certificate.extensions.find { |ext| ext.oid == "extendedKeyUsage" }&.value == OID_TCG_KP_AIK_CERTIFICATE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am thinking it might be worth creating something like an AttestationCertificate
object or something like that, to ease all the logic checking for certificate properties, here in TPM and in other formats also.
Closes #83