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

TAG review feedback: Should we be using array types instead of base64-encoded DOMStrings? #61

Closed
vijaybh opened this issue Mar 30, 2016 · 2 comments
Assignees
Milestone

Comments

@vijaybh
Copy link
Contributor

vijaybh commented Mar 30, 2016

I believe Alex Russell mentioned this in person at the San Francisco F2F, elaborating on comments in w3ctag/design-reviews#97

It may seem more natural to use BufferSource, ArrayBuffer, or ArrayBufferView in various places where we have base64-encoded DOMStrings currently. This may also make it easier for script authors to manipulate these locally if needed (e.g. WebCrypto APIs take array types) and it avoids prescribing the use of base64.

@vijaybh vijaybh self-assigned this Mar 30, 2016
@nadalin nadalin added this to the FPWD milestone Mar 30, 2016
@vijaybh
Copy link
Contributor Author

vijaybh commented Apr 12, 2016

Candidates for conversion to ArrayBuffer:

  • Credential ID (e.g. U2F keyHandle is binary blob)
  • Attestation: signature, rawData, clientData, claimedAAGUID
  • assertionChallenge (for getAssertion)
  • attestationChallenge (for makeCredential)

Not sure about: extensions.

@equalsJeffH
Copy link
Contributor

vijaybh added a commit that referenced this issue Apr 30, 2016
…rings

Fixes #61.

I switched the main API completely from base64-encoded DOMStrings to
Buffersource (for input parameters) and ArrayBuffer (for output
parameters). The actual signatures are still computed over the same data
as before, so signatures computed after this change will be compatible
with those computed before, except for being represented differently.

I moved the ClientData section into the Authenticator model section
since it is not directly used by script authors. This structure still
does base64 encoding of the challenge, for two reasons. First, this
maintains backward compatibility. Second, it is more natural to
represent a binary challenge in JSON as base64 rather than the clunky
array notation.

I would like to advocate for also changing the rawData in the TPM and
packed attestation formats to ArrayBuffers so we can sign directly over
the data without base64 encoding. That would seem to simplify
processing. However this would break compatibility so I would like to
gather opinions from the group before making that change. On the bright
side, I do not know of any implementations producing WebAuthn
attestation statements in these formats yet.
vijaybh added a commit that referenced this issue May 3, 2016
…rings (#77)

* Represent binary data as ArrayBuffers instead of base64-encoded DOMStrings

Fixes #61.

I switched the main API completely from base64-encoded DOMStrings to
Buffersource (for input parameters) and ArrayBuffer (for output
parameters). The actual signatures are still computed over the same data
as before, so signatures computed after this change will be compatible
with those computed before, except for being represented differently.

I moved the ClientData section into the Authenticator model section
since it is not directly used by script authors. This structure still
does base64 encoding of the challenge, for two reasons. First, this
maintains backward compatibility. Second, it is more natural to
represent a binary challenge in JSON as base64 rather than the clunky
array notation.

I would like to advocate for also changing the rawData in the TPM and
packed attestation formats to ArrayBuffers so we can sign directly over
the data without base64 encoding. That would seem to simplify
processing. However this would break compatibility so I would like to
gather opinions from the group before making that change. On the bright
side, I do not know of any implementations producing WebAuthn
attestation statements in these formats yet.

* Remove base64 from packed and TPM attestation formats

Remove base64 encoding from the rawData fields and return them directly
as ArrayBuffers.

* Clarify Android attestation procedure
vijaybh added a commit that referenced this issue Jun 15, 2016
- Resolve inconsistent wording in different parts of the section: fix
places that incorrectly suggested extensions cannot be used with
makeCredential, and resolve conflict between wording that said client
arguments are required vs. not.
- Treat implementations that choose to pass through extensions as a
first-class citizen by requiring that extensions be defined to allow for
this. Fixes #97.
- Fix all the predefined extensions to conform to the updated
guidelines.
- Minor issues: AAGUID should be binary not string (see #61, fixes #51)
and encode true as Boolean in CBOR instead of forcing it to integer
value 1.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants