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

Question on attestation.. #30

Open
sf-troot opened this issue May 20, 2020 · 4 comments
Open

Question on attestation.. #30

sf-troot opened this issue May 20, 2020 · 4 comments

Comments

@sf-troot
Copy link

The method VerifyAttestation takes two certs as input but I don't see where 'attestedCert' is used, should it be here?:

ints.AddCert(attestationCert)

or here?:
return attestationCert.Verify(x509.VerifyOptions{

@jtakkala
Copy link
Contributor

Good catch, looks like attestedCert should be on line 85.

@jtakkala
Copy link
Contributor

jtakkala commented May 20, 2020

This mistake basically means that VerifyAttestation() would never return an error provided a valid attestation cert is passed in, even if the attested cert is invalid. Would be nice to add tests to any fix to prevent a regression here in the future.

@sf-troot
Copy link
Author

Another question:

attestationCert.IsCA = true

In this line the attestationCert is marked as CA and added as an Intermediate, would this stop the 'chain' validation since the CARoot is also added as a root? I am thinking for a proper chain validation this line should not be here? or am I miss understanding something?

@jtakkala
Copy link
Contributor

In this line the attestationCert is marked as CA and added as an Intermediate, would this stop the 'chain' validation since the CARoot is also added as a root? I am thinking for a proper chain validation this line should not be here? or am I miss understanding something?

No, the reason it’s explicitly set here is because some (all?) Yubikey’s don’t set the isCA attribute on the attestation cert, so chain verification would fail. I believe all intermediate CA certs need the isCA attribute set to true.

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

No branches or pull requests

2 participants