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

Spec should not mandate behavior of server #88

Closed
leshi opened this issue May 10, 2016 · 11 comments
Closed

Spec should not mandate behavior of server #88

leshi opened this issue May 10, 2016 · 11 comments

Comments

@leshi
Copy link
Contributor

leshi commented May 10, 2016

This specification should minimally specify (if at all) behavior for servers. This spec is about client behavior and statements like "servers MUST support all types of attestation" are not appropriate.

@apowers313
Copy link
Contributor

For relevant points, see:

  • Section 4.3.1: Compliant servers MUST support all attestation models. Authenticators can choose what attestation model to implement.
  • Section 4.3.2.1.2: The signature is computed over the rawData field. The following algorithms must be implemented by servers:
  • Section 6.5: Servers supporting UVI extensions MUST support a length of up to 32 bytes for the UVI value.
  • Section 4.3.3: Upon receiving an attestation statement, the WebAuthn Relying Party shall:

Like attestation statements and signature formats, this sort of information is useful to those that are trying to use the APIs. Suggesting broad adoption of some set of crypto / attestation formats is important to make sure implementations are broadly interoperable. Also, Section 4.3.3 is generally important to make sure that a server is doing its appropriate security diligence.

@jovasco
Copy link
Contributor

jovasco commented May 13, 2016

+1 to Adam.

Whether or not this is the correct document is another matter but if it is not in this document, it should remain in the FIDO 2.0 specifications.

@equalsJeffH
Copy link
Contributor

+1 to Adam. This spec is more than just an API, it also defines a (cryptographic) protocol between a "challenger/verifier" and a "signer"..

https://docs.google.com/presentation/d/1om__oSew4n48MK_Qcc8deq6hCZ6720-Zvv1PdK0CrjA

..which happen to be a server and a client, respectively. As is commonly done in protocol specs, this spec (at least) needs to provide "implementation considerations" describing the ramifications of various implementation choices on the part of both servers and clients.

@equalsJeffH equalsJeffH modified the milestones: WD-04, WD-03 Nov 9, 2016
vijaybh added a commit that referenced this issue Jan 10, 2017
Refactor the attestation section to clean up exposition. Separated out
signature verification (per format) from trust chaining (done at higher
layer).

Created a separate section for specifying key RP operations. Fixes #88.

RP registration section defines binding of credentials to user accounts.
Fixes #13.

RP registration section also defines options in case of registering the
same credential to different users. Fixes #12.

Cleans up and completes defining the process for verifying assertions,
which had already been largely done by @rlin1. Fixes #102.

Completes drawing the distinction between assertion and attestation
certificates. Fixes #118.

Replace "client platform" with "client" in signature format section to
avoid confusion. Fixes #209.
@equalsJeffH
Copy link
Contributor

fwiw, this issue was closed by eliminating the language objected to by @leshi but without addressing concerns expressed by @apowers313, nor without expressing a plan to address the need for denoting server implementation considerations somewhere.

@apowers313
Copy link
Contributor

The latest spec does seem to have language about what's expected algorithmically, such as verifying an assertion and verifying attestation. (Although not validating an attestation statment). This seems to be comparable to what was noted as Section 4.3.3 above.

The former language of Section 4.3.1 and Section 4.3.2.1.2 specified what crypto and attestation formats a server would need to support. This becomes a interoperability issue. For example, if authenticators implement either ECDSA or RSA and servers support either ECDSA or RSA there is some set of servers and authenticators that simply will not work together because they don't support the same cipher suites.

It would be nice if W3C made some recommendations to get ahead of interoperability issues.

@equalsJeffH
Copy link
Contributor

reopening so we don't loose track of this

@equalsJeffH equalsJeffH reopened this Feb 14, 2017
@vijaybh vijaybh modified the milestones: WD-04, WD-05 Feb 16, 2017
@jyasskin
Copy link
Member

This spec already describes two conformance classes: the UA/Client and the Authenticator. Adding a third, the Relying Party, seems reasonable to me, and I don't think it's terrible to keep it in this document. UA implementers will just be able to ignore that section.

It is, of course, important to specify the UA without assuming the Relying Party behaves as specified and vice versa, but I don't see violations of that in the current spec.

I would suggest that the Relying Party spec specify the whole sequence of operations around the call to makeCredential()/ScopedCredential.create() or getAssertion()/navigator.credentials.get({scoped}) instead of just the code after those functions return. One problem with only specifying the suffix is that it omits the requirement that challenge be a nonce.

@equalsJeffH equalsJeffH modified the milestones: CR, WD-05 Mar 23, 2017
@selfissued
Copy link
Contributor

I agree that we need to continue specifying any security-critical server behaviors. We can decide to move them into their section if people feel strongly about this but I would object to removing them entirely.

@equalsJeffH
Copy link
Contributor

+1 to @jyasskin

in particular:

One problem with only specifying the suffix is that it omits the requirement that challenge be a nonce.

Agreed. and another one is that the challenge be generated on the RP server-side.

@AngeloKai
Copy link
Contributor

The spec already has an extensive server behavior section. The section is considered descriptive suggestion to server. According the comments above, it seems the consensus is the issue should be closed. Should we close the issue? @equalsJeffH @jyasskin

@AngeloKai
Copy link
Contributor

Removing the type:technical label. This doesn't impact the API interface or behavior.

@nadalin nadalin modified the milestones: PR, CR Sep 11, 2017
jyasskin added a commit to jyasskin/webauthn that referenced this issue Oct 3, 2017
jyasskin added a commit to jyasskin/webauthn that referenced this issue Oct 4, 2017
jyasskin added a commit to jyasskin/webauthn that referenced this issue Oct 6, 2017
jyasskin added a commit to jyasskin/webauthn that referenced this issue Oct 10, 2017
equalsJeffH pushed a commit that referenced this issue Oct 12, 2017
* Add a Relying Party conformance class.

Fixes #88.

* Link "Relying Party".
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

9 participants