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

"JSON serialization" in makeCredential probably needs to be defined more clearly #274

Closed
bzbarsky opened this issue Nov 4, 2016 · 5 comments
Milestone

Comments

@bzbarsky
Copy link

bzbarsky commented Nov 4, 2016

https://w3c.github.io/webauthn/#dom-webauthentication-makecredential step 8 says:

compute the clientDataJSON and clientDataHash

where clientDataJSON is defined as:

This is the UTF-8 encoded JSON serialization [RFC7159] of a ClientData dictionary.

and clientDataHash is defined as:

This is the hash (computed using hashAlg) of clientDataJSON.

Unfortunately, RFC 7159 doesn't define a unique serialization format. For example, the following are all valid JSON serializations of the same data:

{ "foo": 5 }
{"foo":5}
{       "foo"     :      5       }

This is normally not a problem, because when parsed with a JSON parser they will all produce the same data structure. But here we're hashing the serialization here, and the expectation is presumably that the hash is stable for a given ClientData. That means the JSON serialization needs to be specified somewhat more strictly than just "any valid JSON serialization of this data"...

@bzbarsky
Copy link
Author

bzbarsky commented Nov 4, 2016

This applies to https://w3c.github.io/webauthn/#getAssertion step 5 as well.

@equalsJeffH
Copy link
Contributor

@bzbarsky : might you be able to point to extant specs that address this particular issue and that we can use as example(s) to work from? I've searched whatwg.org and w3.org and do not immediately discern applicable results.

@bzbarsky
Copy link
Author

bzbarsky commented Nov 7, 2016

Webcrypto addresses this by explicitly taking an IDL dictionary, converting it to an ES object using https://heycam.github.io/webidl/#dictionary-to-es, then invoking the ES spec's JSON.stringify on it. Search for "JSON.stringify" in that spec.

However note that this is one place where the way the TR version does it is wrong and the way the editor's version does it is better; if you're going to copy verbiage here please copy it from their editor's draft.

@equalsJeffH equalsJeffH added this to the WD-04 milestone Nov 7, 2016
@vijaybh
Copy link
Contributor

vijaybh commented Nov 8, 2016

the expectation is presumably that the hash is stable for a given ClientData

This is not true. There is no such expectation.

Both makeCredential and getAssertion return the actual serialized string clientDataJSON as an ArrayBuffer along with their respective signatures. This should be enough for the RP to check the signature and to verify the contents of the clientData by parsing the stringified JSON. This was done specifically to avoid canonicalization issues like this one.

@bzbarsky
Copy link
Author

bzbarsky commented Nov 8, 2016

There is no such expectation.

Including by RPs? How are you going to enforce that? I expect RPs to end up with just such expectations, forcing implementations to de-facto converge on identical serializations....

This seems like a poster child for Postel's law, in fact: strictness in what you produce (the serialization) should lead to better interop here than allowing different serializations and hoping no one depends on the serialization details.

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