Various minor technical fixes #147
Conversation
| extensions. The first bytes before the extensions have a fixed layout as follows: | ||
| The field `rawData` for this type is a byte array of 17 bytes + length of AAGUID + length of public key + length of Credential | ||
| ID + length of clientDataHash + potentially more extensions. The first bytes before the extensions have a fixed layout as | ||
| follows: |
There was a problem hiding this comment.
I think I (finally) figured out how we keep getting various different values for the fixed length of the rawData before the public key itself -- it is because some of us (i.e. myself) take the 17 bytes (or the old 45 bytes) to mean the length of rawData before the first variable length field, rather than the sum of all of the fixed-length rawData fields, which it seems is (now more clearly) stated above.
Thus I think the above paragraph is needlessly misleading and we should update it. Here is a proposal..
The packed attestation
rawDatafield is a variable-length byte array, consisting of various fixed-length and variable-length sub-fields, arranged as given in the below table:
There was a problem hiding this comment.
Overall, other than my prior comment/suggestion, this PR looks good to me -- thanks for reviewing and implementing those various outstanding issues.
There was a problem hiding this comment.
Thanks, that is an excellent suggestion and it makes the text a lot less clunky. Will update the PR shortly.
As per feedback from @equalsJeffH - thanks!
|
This all looks OK to me, too. I'm really glad you caught the last Key Handle / Credential ID changes - I must have looked at that text a dozen times and never recognized it was incongruous. Thank you. |
|
Thanks! Merging, now that this has two reviews. |
All of these are small and contained fixes, and should be non-controversial. The most impactful one is moving secure contexts from SHOULD to MUST. Please review and provide feedback. Thanks!