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

Represent binary data as ArrayBuffers instead of base64-encoded DOMStrings #77

Merged
merged 3 commits into from
May 3, 2016

Conversation

vijaybh
Copy link
Contributor

@vijaybh vijaybh commented Apr 30, 2016

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.

…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.
Remove base64 encoding from the rawData fields and return them directly
as ArrayBuffers.
@jcjones
Copy link
Contributor

jcjones commented May 3, 2016

Your decision to maintain the Base64-encoding in the ClientData structure makes sense to me. The other revisions, including the section move and the type changes, I also endorse.

LGTM. r=me.

@equalsJeffH
Copy link
Contributor

LGTM.

@vijaybh
Copy link
Contributor Author

vijaybh commented May 3, 2016

Merging this in now that we have two reviews.

@vijaybh vijaybh merged commit 634c269 into master May 3, 2016
@vijaybh vijaybh deleted the vijaybh/61-buffers branch May 3, 2016 21:42
@equalsJeffH equalsJeffH restored the vijaybh/61-buffers branch September 12, 2016 23:50
@vijaybh vijaybh deleted the vijaybh/61-buffers branch September 13, 2016 06:06
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

Successfully merging this pull request may close these issues.

None yet

3 participants