-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
fix: Allow ExtKeyUsageAny for Webauthn attestation certificates #51201
Conversation
allowed = true // OK, but keep checking | ||
} else { | ||
log.DebugContext(context.Background(), |
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.
Added some logging to help future debugging. The first certificate in the Hello chain fails validation, which is good to know (the second passes so thankfully it works).
For the curious, the error is "x509: unhandled critical extension".
Thanks for the quick review, Noah. Friendly ping @EdwardDowling. |
Thanks everyone for the quick reviews. After mailing this I double-checked other Certificate.Verify calls in the codebase (looks OK, Device Trust has a similar check by Noah). It also occurred to me to check certificate expiration for certs such as the Microsoft TPM one, as that is yet another way this may fail to no fault of the customer. The NotAfter for this particular cert is "Dec 10 21:39:28 2039 GMT", so it looks like we are good in that regard. I'll consider a bit more whether to tweak the "now" date for the check. |
@codingllama See the table below for backport results.
|
) * Allow ExtKeyUsageAny for Webauthn attestation certificates (#51201) * Use logrus instead of slog
) * Allow ExtKeyUsageAny for Webauthn attestation certificates (#51201) * Use logrus instead of slog
) * Allow ExtKeyUsageAny for Webauthn attestation certificates (#51201) * Use logrus instead of slog
Windows Hello attestation checks fail because its attestation certificates set ExtKeyUsage 2.23.133.8.3 (and also 1.3.6.1.4.1.311.21.36, which doesn't seem to be thoroughly documented).
This PR fixes that by allowing any ExtKeyUsage to be part of attestation certificates. That is partly because there are no consts for the OIDs above, but also because it doesn't seem like a reason to fail attestation.
Changelog: Fixed WebAuthn attestation for Windows Hello