-
Notifications
You must be signed in to change notification settings - Fork 178
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
Add packed attestation optional firmware version attribute #1953
Conversation
… Also, clarify that the AIA extension is optional.
Fixes #1952 |
Co-authored-by: Emil Lundberg <emil@emlun.se>
Here's an example of b64url-encoded packed attestation from a Yubikey. I believe that was the ask? @dwaite
|
Thanks Shane. Based on there, here's a possible example. You said that you wanted all the extensions included. I'm happy to tweak this further if you let me know what needs to be in it.
|
Here is an example with all three extensions, as I understand their specification:
Case in point: while making this example, I too forgot at first to wrap the Here are the extensions formatted similar to the current
The serial number example assumes the answer to my question about serial number format is that serial numbers are positive integers represented in big-endian two's complement notation (but of any length), not opaque byte strings. |
2023-10-04 WG call: We keep serial numbers as opaque byte strings. I've updated the above example accordingly. |
@emlun, would it be possible to get this example without sernum? Since this example will be in the non-enterprise section, I suspect that would be misleading to implementers. We can decide on #1954 if we want a second full example or snippet of a serial number. |
Sure:
Though it occurred to me now that I haven't updated the signature, so the certificate as a whole is not valid (neither of these two, though I haven't checked if the example from @agl has a valid signature). But are we going to include the whole certificate in the example? Currently we only have an example like the second listing in my previous comment. For that example you can just omit the sernum part, the three sections are independent. |
I figure a signature is useful in the Base64-encoded section for someone to look at via tooling, but worth omitting in the textual view. I'll update the PR to show your example now. |
Signature on the example is currently invalid.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
I've updated this with a PEM encoded certificate and with deconstructed versions of the attributes. I request examination of the certificate contents and decomposition to make sure I got them correct/ideal. The code I used to generate these is at https://github.com/dwaite/webauthn-sample-attestation |
Strong expectation to merge this next week. Time for final comments. |
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.
LGTM, sorry for the delay!
Two minor polish comments for optional consideration.
I did a cursory review of the diff text and visually verified the example against existing security keys. I found some trailing whitespace and a comment issue on the example. This should be good to go. |
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.
LGTM
SHA: 354a717 Reason: push, by dwaite Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This describes the FIDO OID and data for the firmware version attribute.
It also changes the AIA attribute above it, which appears to actually have been added to the required attributes list.
Preview | Diff