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

explain challenge's security importance and use in both registration and authentication operations #404

Closed
equalsJeffH opened this issue Apr 14, 2017 · 16 comments
Assignees
Labels
security-tracker Group bringing to attention of security, or tracked by the security Group but not needing response. subtype:impl-cons type:editorial

Comments

@equalsJeffH
Copy link
Contributor

need to expand the explanation of "challenge" in the spec -- challenge is used in both regstn and authn protocol runs and has security implications. It MUST NOT be created on the client-side -- rather, it MUST be randomly created on the server-side with very low probability of collisions, and verified upon receipt back from the client and end of protocol run in order to provide protocol-run round-trip integrity. this needs to be explained in the spec and the code in the examples altered to reflect this -- especially since developers will simply cut'n'paste our examples! :-P

@equalsJeffH equalsJeffH added subtype:impl-cons security-tracker Group bringing to attention of security, or tracked by the security Group but not needing response. type:editorial type:technical labels Apr 14, 2017
@equalsJeffH equalsJeffH added this to the CR milestone Apr 14, 2017
@jyasskin
Copy link
Member

#88 (comment) proposes a way to write these MUSTs, which constrain the behavior of the server rather than the browser or authenticator.

@equalsJeffH
Copy link
Contributor Author

wrt the examples (aka "sample scenarios"):

var challenge = new TextEncoder().encode("climb a mountain");

perhaps ought to be:

// server generates 32 byte random challenge value, memorizes it for validation of the upcoming 
// response, base64-encodes it, and sends it to client as part of the web page running in the user 
// agent, who then turns it back into a byte array thusly:
var challenge = window.atob("PGifxAoBwCkWkm4b1CiIl5otCphiIh6MijdjbWFjomA=");

this is applicable to all the examples in this section.

@jyasskin
Copy link
Member

atob() returns a DOMString rather than an ArrayBuffer.

Uint8Array.from(window.atob("PGifxAoBwCkWkm4b1CiIl5otCphiIh6MijdjbWFjomA="), c=>c.charCodeAt(0))

would work.

@equalsJeffH
Copy link
Contributor Author

cool, thx @jyasskin -- hm, I am gonna have to dig around and figger out the details on how this "mapFn" gizmothingee works: c=>c.charCodeAt(0)

@mikewest
Copy link
Member

Did y'all consider requiring the value to be a base64-encoded DOMString, and doing the decoding/bufferizing as part of the makeCredential and getAssertion algorithms? base64 seems like a sufficient hurdle that folks wouldn't generate it themselves client-side, and they wouldn't have to come back to the spec to look up how to convert a string into an ArrayBuffer. I know I would have had to look that up...

@equalsJeffH
Copy link
Contributor Author

@mikewest:

y'all consider requiring the value to be a base64-encoded DOMString, and doing the decoding/bufferizing as part of the makeCredential and getAssertion algorithms?

no, we did not think of that -- yes, I'm thinking your suggestion is the way to go -- thanks!

@AngeloKai
Copy link
Contributor

@equalsJeffH @mikewest Are you proposing we change the challenge to be a DOMString? @vijaybh @leshi I remembered the reason why challenge is base64url encoding is partially because of CTAP, correct?

@mikewest
Copy link
Member

@equalsJeffH @mikewest Are you proposing we change the challenge to be a DOMString?

I'm suggesting that, yes. Coupled with changes to the algorithms which would throw a TypeError if the string isn't a valid base64url-encoded string, that seems sufficiently unlikely to be a) generated client-side, or b) confusing. :)

@vijaybh @leshi I remembered the reason why challenge is base64url encoding is partially because of CTAP, correct?

For clarity, the challenge property is currently a BufferSource. We base64url-encode it as part of the algorithms above when creating the CollectedClientData object. I'd suggest we simply drop that, and expect the developer to provide a pre-encoded string.

@vijaybh
Copy link
Contributor

vijaybh commented Apr 19, 2017

The challenge was originally a base64 encoded DOMString. We changed it to BufferSource in response to TAG feedback. See #61.

@mikewest
Copy link
Member

@slightlyoff: How do you feel about the BufferSource recommendation, given the code snippets discussed in #404 (comment)?

@equalsJeffH
Copy link
Contributor Author

The challenge was originally a base64 encoded DOMString. We changed it to BufferSource in response to TAG feedback. See #61.

thanks @vijaybh -- this seems to be a lesson that we now have long enough history that we ought to search prior issues/PRs rather than rely on memory (as I mistakenly did)...

@AngeloKai
Copy link
Contributor

Assigning this to @equalsJeffH because he proposed the issue.

@equalsJeffH
Copy link
Contributor Author

I'm fine with working on this issue. we still would like some input from @slightlyoff it seems.

@jcjones
Copy link
Contributor

jcjones commented Nov 9, 2017

Re: #404 (comment) and #404 (comment), let's not hold their hands about how to get the octets from their back-end to WebAuthn. I suggest this compromise:

// server generates 32 byte random challenge value, memorizes it for validation of the upcoming 
// response, and sends it to client as part of the web page running in the user agent, who then
// turns it back into a byte array thusly:
var challenge = new Uint8Array([21,31,105 /* 29 more random bytes generated by the server */]);

And in the Security Considerations section, add:

== Challenges == 
As a cryptographic protocol, Web Authentication is dependent upon randomized challenges
to avoid replay attacks. Therefore, the [=challenge=] fields MUST be randomly generated
by the [=Relying Party=] in an environment they trust, and the client's responses' challenge
fields MUST match what was generated. This should be done in a fashion that does not rely
upon a client's behavior; e.g.: the Relying Party should store the challenge temporarily
until the operation is complete. Tolerating a mismatch will compromise the security
of the protocol.

@equalsJeffH
Copy link
Contributor Author

LGTM - thanks!!

@kpaulh
Copy link
Contributor

kpaulh commented Nov 9, 2017

"and the client's responses' challenge fields" - bit of a mouthful. "and the challenge in the client's response MUST..." ?

jcjones added a commit to jcjones/webauthn that referenced this issue Nov 9, 2017
jcjones added a commit to jcjones/webauthn that referenced this issue Nov 9, 2017
jcjones added a commit that referenced this issue Nov 13, 2017
Fix #404 - Add a Security Consideration for Cryptographic Challenges
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
security-tracker Group bringing to attention of security, or tracked by the security Group but not needing response. subtype:impl-cons type:editorial
Projects
None yet
Development

No branches or pull requests

7 participants