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

I don't understand how to create a ClientData in makeCredential #273

Closed
bzbarsky opened this issue Nov 4, 2016 · 9 comments
Closed

I don't understand how to create a ClientData in makeCredential #273

bzbarsky opened this issue Nov 4, 2016 · 9 comments
Milestone

Comments

@bzbarsky
Copy link

bzbarsky commented Nov 4, 2016

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

Use attestationChallenge, callerOrigin and rpId, along with the token binding key associated with callerOrigin (if any), to create a ClientData structure representing this request. Choose a hash algorithm for hashAlg and compute the clientDataJSON and clientDataHash.

ClientData is defined to be a dictionary like so:

dictionary ClientData {
  required DOMString           challenge;
  required DOMString           origin;
  required AlgorithmIdentifier hashAlg;
  DOMString                    tokenBinding;
  WebAuthnExtensions           extensions;
};

Alright. So how do I go about creating one?

  1. I guess I should fill in "challenge" from "attestationChallenge"? But "attestationChallenge" is a BufferSource, while "challenge" is a DOMString. How is the DOMString generated from the BufferSource?
  2. I guess I should fill in "origin" from "callerOrigin", but the former is a DOMString while the latter is an origin; in this case a tuple. How is the DOMString generated?
  3. Where in here is rpId used?
  4. This is the first mention of a "token binding key" in this algorithm. What is that, and how does one find out whether there is one associated with callerOrigin?
  5. It's not clear to me what "Choose a hash algorithm for hashAlg" means in practice. hashAlg can be either a string or an ES object. Which one is it expected to be in this case? If it's expected to be an ES object, then we have a problem because then we need to define exactly what "JSON serialization" means for it...
  6. Is "extensions" supposed to be set to anything? At first glance, no, since it's not in the set of data the spec says to use to create the ClientData. But I expect that's a spec bug?
@bzbarsky
Copy link
Author

bzbarsky commented Nov 4, 2016

https://w3c.github.io/webauthn/#getAssertion step 5 has the same issue.

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

jcjones commented Nov 16, 2016

To echo bzbarsky, as AlgorithmIdentifier is typedef'd to (object or DOMString), it's not default serializable.

From the UA's perspective, it appears that I can get away with just declaring all instances of AlgorithmIdentifier in the webidl to be DOMString, as hashAlg is always chosen by the UA.

@annevk
Copy link
Member

annevk commented Feb 15, 2017

(Note that where OP mentions step 8 it is now step 7.)

@jyasskin
Copy link
Member

Looking at https://www.w3.org/TR/WebCryptoAPI/#sha, I think the hash AlgorithmIdentifiers are all just strings or instances of Algorithm itself rather than a subtype, so they have no structure beyond their names. Given that, we could probably make ClientData.hashAlg just a DOMString instead of an AlgorithmIdentifier.

Even if WebAuthn wants to use a more structured hash algorithm in the future, we can probably make hashAlg a union of the right set of Algorithm subtypes, instead of matching WebCrypto's unconstrained object.

@mwatson2 in case I've mis-read WebCrypto.

@bzbarsky
Copy link
Author

You can't do a union of dictionary types, which is why webcrypto has object...

@jyasskin
Copy link
Member

Argh. A specialized dictionary type that combines all the members from the other types we care about then. In any case, DOMString should be enough for the existing hash algorithms.

@vijaybh
Copy link
Contributor

vijaybh commented Feb 16, 2017

I agree with JC's comment above. We could make this a whole lot simpler by just being a bit opinionated and saying hashAlg is a DOMString that's set to a WebCrypto "recognized algorithm name". That should be good enough for now. Even with arbitrary-length hashes, we'll probably see one or two sizes used by everyone for interop purposes.

jyasskin added a commit to jyasskin/webauthn that referenced this issue Feb 16, 2017
I've linked a lot more terms, reordered explanations to be clearer, and
specified some missing behavior.

This fixes w3c#273 and improves w3c#270.
@jyasskin
Copy link
Member

https://w3c.github.io/webauthn/#clientdata-hashalg says to

Use "S256" for SHA-256, "S384" for SHA384, "S512" for SHA512, and "SM3" for SM3

which are not the "recognized algorithm names" for these algorithms. Do you want to remove those aliases or say it's an "alg" value for an algorithm in the JSON Web Signature and Encryption Algorithms Registry ... that can be used without a key?

@vijaybh
Copy link
Contributor

vijaybh commented Feb 16, 2017

Yuck. I knew I was forgetting something. You're right, we had flipped everything to use the JWA names at some point. I don't much care which one we use, since even WebCrypto has an appendix mapping JWK algorithm names to WebCrypto equivalents. @selfissued do you have an opinion here?

@vijaybh vijaybh modified the milestones: WD-04, WD-05 Feb 16, 2017
jyasskin added a commit to jyasskin/webauthn that referenced this issue Feb 22, 2017
I've linked a lot more terms, reordered explanations to be clearer, and
specified some missing behavior.

This fixes w3c#273 and improves w3c#270.
jyasskin added a commit to jyasskin/webauthn that referenced this issue Feb 24, 2017
I've linked a lot more terms, reordered explanations to be clearer, and
specified some missing behavior.

This fixes w3c#273 and improves w3c#270.
@vijaybh vijaybh closed this as completed in 546f82f Mar 1, 2017
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

6 participants