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

txAuthSimple does not conform to extension requirements #725

Closed
gmandyam opened this issue Dec 12, 2017 · 9 comments
Closed

txAuthSimple does not conform to extension requirements #725

gmandyam opened this issue Dec 12, 2017 · 9 comments

Comments

@gmandyam
Copy link

As per https://w3c.github.io/webauthn/#sctn-simple-txauth-extension, the authenticator extension input is a CBOR string.

As per https://w3c.github.io/webauthn/#sctn-authenticator-extension-processing, the authenticator extension input is a CBOR map.

Suggested change: proponents of this extension define the authenticator extension input as a CBOR map. I will modify #724 accordingly when this is done.

@Kieun
Copy link
Member

Kieun commented Dec 12, 2017

Authenticator extension input is a CBOR encoded data which is a value for Authenticator extension input argument, form of map. And the key for the map is the extension identifier. So, we don't have to change the description. But, the readers may be confused.
So I suggest followings.

  • change client/authenticator extension input -> client/authenticator extension input value
  • let client extension input -> dictionary (key: identifier, value: client extension input value)
  • let authenticator extension input -> map (key: identifier, value: authenticator extension input value)
  • same for the client/authenticator extension outputs

@emlun
Copy link
Member

emlun commented Dec 12, 2017

I think it's technically correct as currently written, though perhaps a bit dense. What's described as a CBOR map in the second link is not the "authenticator extension input", but "the extensions data part of the authenticator request" which is a map containing the "authenticator extension input"s as values. This distinction could perhaps be made clearer, perhaps by somehow referring to the extensions parameter of authenticatorMakeCredential and/or `authenticatorGetAssertion, but it looks to me like "authenticator extension input" does consistently refer to the individual values of the map and not the whole map.

@gmandyam
Copy link
Author

@Kieun and @emlun

Thanks for the responses. Another option is to make the description of txAuthSimple more like txAuthGeneric (https://w3c.github.io/webauthn/#sctn-generic-txauth-extension), where the client extension input is provided as a CBOR Map, with the corresponding key/value pairs. Note also that client extension input does not have to strictly be in JSON ("The client extension input, which is a value that can be encoded in JSON ..." as per https://w3c.github.io/webauthn/#sctn-extension-request-parameters).

So defining client extension input as txAuthSimpleArg = {prompt: tstr,} would allow for the client extension input to simply be passed as authenticator extension input.

@emlun
Copy link
Member

emlun commented Dec 12, 2017

Another option is to make the description of txAuthSimple more like txAuthGeneric (https://w3c.github.io/webauthn/#sctn-generic-txauth-extension), where the client extension input is provided as a CBOR Map, with the corresponding key/value pairs.

What would be the benefit of this?

Note also that client extension input does not have to strictly be in JSON ("The client extension input, which is a value that can be encoded in JSON ..." as per https://w3c.github.io/webauthn/#sctn-extension-request-parameters).

What do you mean? Doesn't the quoted passage say that the client extension input does have to be a JSON (-encodable) value?

@gmandyam
Copy link
Author

Another option is to make the description of txAuthSimple more like txAuthGeneric (https://w3c.github.io/webauthn/#sctn-generic-txauth-extension), where the client extension input is provided as a CBOR Map, with the corresponding key/value pairs.

What would be the benefit of this?

Avoiding an unnecessary casting between JSON and CBOR (which is not a huge concern), and consistency between txAuthSimple and txAuthGeneric.

What do you mean? Doesn't the quoted passage say that the client extension input does have to be a JSON (-encodable) value?

No it does not. The quoted passage says "can be encoded in JSON", which is not the same as "have to be JSON-encodable".

@nadalin nadalin added this to the CR milestone Dec 13, 2017
@equalsJeffH
Copy link
Contributor

@gmandyam:

this issue is related to issue #626. The specification of #sctn-simple-txauth-extension's input value of "A single JSON string prompt" is likely going to change to be something more explicit e.g.:

txAuthSimpleArg = ( tstr )  

I disagree that we we should make #sctn-simple-txauth-extension's input value be encoded as a CBOR map in the same fashion as #sctn-generic-txauth-extension's args.

@emlun: What do you mean? Doesn't the quoted passage say that the client extension input does have to be a JSON (-encodable) value?

@gmandyam: No it does not. The quoted passage says "can be encoded in JSON", which is not the same as "have to be JSON-encodable".

IMHO the above-cited "can be encoded in JSON" language in section 9.3. Extending request parameters is a bug and ought to be more clearly specified.

@gmandyam
Copy link
Author

@equalsJeffH

Thanks for the guidance. Let us assume that client-extension input must be JSON-encodable. Then shouldn't https://w3c.github.io/webauthn/#sctn-generic-txauth-extension specify its client-exention input as JSON instead of as a CBOR map?

@nadalin
Copy link
Contributor

nadalin commented Jan 4, 2018

@gmandyam @selfissued where do we stand with this ? and 626 ?

@equalsJeffH
Copy link
Contributor

wrt @gmandyam's #725 (comment), see further discussion wrt extension input type in the PR #737.

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