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

Does AuthenticationExtensionsClientOutputs key have to be extension identifier or not? #1430

Closed
ynojima opened this issue May 30, 2020 · 7 comments · Fixed by #1439
Closed

Comments

@ynojima
Copy link
Contributor

ynojima commented May 30, 2020

In section 9.3(https://w3c.github.io/webauthn/#sctn-extension-request-parameters), it is stated that

The entry key is the extension identifier

But Credential Protection extension defined in CTAP2.1, whose extension identifier is credProtect, has following definition. Its key doesn't match the extention identifier and moreover, it has two keys.

partial dictionary AuthenticationExtensionsClientInputs {
  USVString credentialProtectionPolicy;
  boolean enforceCredentialProtectionPolicy = false;
};

Does the extension definision have to be updated? or Is the extension key naming rule to be relaxed?

@selfissued
Copy link
Contributor

The entry key in this case is credProtect.

@equalsJeffH
Copy link
Contributor

on 2020-06-03 call: @emlun will investigate

@emlun
Copy link
Member

emlun commented Jun 3, 2020

As far as I can tell @ynojima is right that the CTAP2.1 definition departs from how the WebAuthn extensions framework is intended to work. Fortunately this only affects clients - authenticators that have already shipped with the feature will not have to change if we want to fix it.

The extensions framework in WebAuthn would expect the registration input to look like this:

{
  "publicKey": {
    "challenge":  ...,
    ...,
    "authenticatorSelection": {
      "userVerification": "required"
    },
    "extensions": {
      "credProtect": {
        "credentialProtectionPolicy": "userVerificationRequired",
        "enforceCredentialProtectionPolicy": true
      }
    }
  }
}

However in Chromium 83.0.4103.61 this causes the extension to be ignored. You indeed have to specify the extension like this:

{
  "publicKey": {
    "challenge":  ...,
    ...,
    "authenticatorSelection": {
      "userVerification": "required"
    },
    "extensions": {
      "credentialProtectionPolicy": "userVerificationRequired",
      "enforceCredentialProtectionPolicy": true
    }
  }
}

which causes the extension to be processed and reflected in the authenticator extension output.

On the authenticator side, the extension input is simply an integer "extensions": { "credProtect": 3 } computed by the client, which incidentally does match the WebAuthn input structure.

So in summary: Chromium implements the extension as specified in CTAP2.1, but CTAP2.1 does not follow the extension input structure expected by WebAuthn. CTAP could change the specification to match the WebAuthn structure, in which case only client code needs to change. Alternatively, WebAuthn could relax the soft requirement that extension inputs be grouped under a key named as the extension identifier.

@akshayku
Copy link
Contributor

akshayku commented Jun 4, 2020

I always assumed the second behavior. Another extensions which works this way is hmac-secret with parameter hmacCreateSecret defined directly in extensions map.

@equalsJeffH, Can you check appid ones?

This was referenced Jun 5, 2020
@equalsJeffH
Copy link
Contributor

this is what is in https://w3c.github.io/webauthn/#sctn-appid-extension

partial dictionary AuthenticationExtensionsClientInputs {
  USVString appid;
};

@emlun
Copy link
Member

emlun commented Jun 10, 2020

2020-06-10 WG call: all extensions defined in the WebAuthn spec follow the specified pattern; will add a note mentioning that there exist other extensions that do not.

@emlun
Copy link
Member

emlun commented Jun 11, 2020

On a related note, §9.2 Defining Extensions says:

Any client extension that is processed by the client MUST return a client extension output value so that the WebAuthn Relying Party knows that the extension was honored by the client.

...which is currently not obeyed by the appidExclude extension. We should either relax this restriction or add an extension output to appidExclude. @agl Thoughs?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants