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

#getAssertion alg needs to pass authenticator selection requirements to authenticatorGetAssertion operation #644

Closed
equalsJeffH opened this issue Oct 13, 2017 · 24 comments · Fixed by #672

Comments

@equalsJeffH
Copy link
Contributor

equalsJeffH commented Oct 13, 2017

Ostensibly, RPs want/need to pass authenticatorSelection.requireUserVerification (aka "uv" in CTAP parlance) in the #getAssertion call, per issue #629.

However, authenticatorSelectionCriteria is part of MakePublicKeyCredentialOptions, which is not passed at all to #getAssertion, and thus is not subsequently passed to webauthn's authenticatorGetAssertion operation, which subsequently needs to pass both "uv" and "up" booleans as options to, for example, CTAP's authenticatorGetAssertion command.

See also issue #629

@nadalin
Copy link
Contributor

nadalin commented Oct 16, 2017

@equalsJeffH We are not doing any authenticator selection enhancements in this release, yiu can use the extensions to do what you need for now.

@nadalin nadalin modified the milestones: WD-07, L2-WD-00 Oct 16, 2017
@leshi
Copy link
Contributor

leshi commented Oct 24, 2017

It's not an enhancement. You will need to pass the UV bit in getAssertion or else the authenticator won't know whether or not to enforce UV and ask for a PIN, etc.

@leshi
Copy link
Contributor

leshi commented Oct 24, 2017

It's an oversight left over from when we added UV to create() but forgot to do the same for get()

@emlun
Copy link
Member

emlun commented Oct 25, 2017

Actually it looks intentional to me - §5.4.4 Authenticator Selection Criteria reads:

requireUserVerification
[...] If the parameter is set to true, the authenticator MUST perform user verification when performing the create() operation and future §5.1.4 Use an existing credential to make an assertion operations when it is requested to verify the credential.

Note the and future "make an assertion" operations.

@emlun
Copy link
Member

emlun commented Oct 25, 2017

That said, I support adding UV as a parameter to get() also.

@jcjones
Copy link
Contributor

jcjones commented Oct 25, 2017

I agree with adding this, too.

@akshayku
Copy link
Contributor

@leshi: I was referring to comments in #629 (comment)

@leshi
Copy link
Contributor

leshi commented Oct 25, 2017

Thanks for pointing that out, @emlun. I appreciate you taking the time to find this in the spec! ^_^

Let me rewind the clock a bit, take us all back in time.

The reason we added a UV bit in the signature format way back when was so that a key can be created with UV, but then use by the RP sometimes with and sometimes without user verification. The alternative was to create two keys (one to be used with UV and one to be used without), but we all decided that this was a bad idea.

In any case, as currently defined (see note from @emlun above), it is impossible to sometimes generate assertions with UV and sometimes without using the same key.

Also, as currently defined, we won't be able to support future use cases such as silent authenticators, uv caching, etc.

@akshayku
Copy link
Contributor

akshayku commented Oct 25, 2017

Let's separate silent behavior from "uv" discussion. We don't want that ever to happen. and corresponding option is "up" and not "uv". We won't be able to support any design which cannot achieve this requirement.

"UV" caching is different and I don't think we are ready for it in this version and we should target next version for this.

Regarding "uv", conversation is slightly complex. As I said in #629, one workflow where RP can decide whether they care or not about "uv" or not is when they receive the signature. In another workflow, where RP always want it, it can reject that signature if authenticator is not capable of doing it. Only case where RP does not really need user verification to happen, current API does not support that semantics when a UV capable authenticators comes along. This also is tricky, as RP does not know what kind of authenticator user will plug it in, when RP just asks for assertion without providing any credential ID (without any userID knowledge flows). In that sense, RP is just asking me tell me what you have (what credential, whether UV is set or not etc). RP cannot decide in advance what kind of user it is and what kind of authenticator he/she has and correspondingly does not provide any allowed credentialID list .

Anyway, looks like current design does not support one use case where RP does not want user verification to happen every-time it asks for assertion. For all other cases, signature reflects what you got and RP can act on it. If people really care about that scenario, below proposal IMO achieves all above cases.

enum name of userVerificationRequirement with values of Required, NotRequired and AuthenticatorBehavior, with default value being AuthenticatorBehavior.

  • Required: Authenticator MUST enforce user verification and signature MUST
    have UV bit set.

  • NotRequired: Platform MUST not ask for user verification even if authenticator is capable of it. This is for scenario where touch is enough for RP and it does not want UI pops on authenticator or platform for ClientPin Case. May be in case it wants to use the authenticator as a second factor device.

  • AuthenticatorBehavior: Platform performs the default behavior of authenticator and signature reflects that. It is kind of RP saying, tell me what you have and I will decide what level of authentication you can have or RP does not care about user verification but OK if it receives it as it ignores it. For Authenticators, which don't support user verification, it will not set "UV" bit set. For authenticators that can support user verification, platform will send such a request and signature will reflect "UV" bit.

@emlun
Copy link
Member

emlun commented Oct 26, 2017

I propose renaming the NotRequired enum value to Forbidden. NotRequired is confusing because "not required" is logically equivalent to "allowed, but not required". Forbidden on the other hand unambiguously communicates "not required and not allowed".

@akshayku
Copy link
Contributor

What we are discussing is platform semantics when calling authenticators. Authenticators are still free to send "uv" bit even if platform has not asked for it (for example, in case of face/fingerprint authenticators). Forbidden is a very strong word and gives a sense that authenticators are not allowed to set it. I like NotRequired better.

@emlun
Copy link
Member

emlun commented Oct 26, 2017

Now you're contradicting yourself. Your previous comment reads:

NotRequired: Authenticator MUST not ask for user verification even if authenticator is capable of it. [...]

Assuming that you mean "Authenticator MUST NOT ask...", I interpret this to mean that the authenticator is indeed not allowed to set the UV bit.

@akshayku
Copy link
Contributor

Ah. I realized the typo in the initial proposal. Apologies.

I meant to say "Platform MUST not ask for user verification" instead of "Authenticator MUST not ask for". I edited the comment.

For example, some vendor builds a finger print authenticator whose semantics are that it always do user verification before releasing any signature. It may be their differentiation from other authenticators and it is their business decision. Same thing with face authenticators or some other kind. Whatever, it's their business decision.

What we are saying is, RP may say that it does not need it ( may be UV/Client PIN semantics brings more user friction for majority of its customers) and platform must not ask for it. It does not mean that RP is going to reject it when authenticators give UV bit anyway because its their only way to release the signature. It is an additional security that RP does not need but can ignore it if that comes and does not harm them.

If authenticator supports both type of signatures, (with UV/ClientPin semantics and with only touch), in that case, Platform will not ask for UV/ClientPin semantics and authenticator is free to give with signature without UV bit.

@akshayku
Copy link
Contributor

akshayku commented Nov 1, 2017

Just to be clear here, this proposal is not enabling silent authenticators.

From our view, enabling silent authenticators is very very dangerous from the RP's perspective and from general user's perspective and we cannot support that. That behavior potentially can be viewed as super cookie with no way to removing it even if user turn on all the knobs.

@nadalin nadalin modified the milestones: L2-WD-00, WD-07 Nov 1, 2017
@equalsJeffH
Copy link
Contributor Author

equalsJeffH commented Nov 2, 2017

Rather than NotRequired in #644 (comment), how about Optional ?

@equalsJeffH
Copy link
Contributor Author

wrt "silent authenticators", perhaps discuss over in issue #199 ?

@akshayku
Copy link
Contributor

akshayku commented Nov 3, 2017

Optional is also OK with me if you think it better explain the requirement that platform will not send "uv" or try to do ClientPin even if authenticator supports those features. Authenticator is still free to choose how it wants to authenticate even if platform has not asked for it (For example BIO authenticators).

Let me know.

Yeah, lets discuss "silent authenticators" in #199.

@emlun
Copy link
Member

emlun commented Nov 3, 2017

Is there a big enough difference between NotRequired and AuthenticatorBehavior to be worth making this an enumeration instead of a Boolean flag?

  • required: true would be equivalent to Required
  • required: false (default) would let the platform/authenticator do whatever it wants, but with the knowledge that UV is not required. It would probably choose to not ask for UV, or perhaps ask for UV but allow skipping it, or in some cases ignore the flag and ask anyway.

@akshayku
Copy link
Contributor

akshayku commented Nov 3, 2017

It is.

In one scenario, RP wants to not pop up Client PIN UI/user verification due to friction and it OK with only user presence. In another scenario, RP accepts both type of signatures and as he does not know what kind of user is present, asks for default behavior and acts accordingly.

@emlun
Copy link
Member

emlun commented Nov 3, 2017

Ah, so you mean that the RP may (1) require UV; (2) want, but not require, UV; (3) prefer skipping UV? Then the enum makes a lot of sense.

I thought you meant that the RP may (1) require UV; (2) prefer skipping UV; (3) not care at all about UV. In that case there's not really that big a difference between (2) and (3).

@akshayku
Copy link
Contributor

akshayku commented Nov 3, 2017

(1: Required): Require UV
(2: Not Required): prefer skipping UV
(3: Authenticator Behaviour): Supports both UV and non UV scenario and care about both type of users/authenticators.

Does it makes sense?

@equalsJeffH
Copy link
Contributor Author

Note that relevant other issues really ought to be addressed at the same time at this one (cf. #524 (comment)):
webauthn issue #645, CTAP issues fido-alliance/fido-2-specs#364, fido-alliance/fido-2-specs#365.

see also:
#524 (comment)
#524 (comment)

https://github.com/w3c/webauthn/issues/629#issuecomment-337067688

@emlun
Copy link
Member

emlun commented Nov 4, 2017

I can take a stab at this.

@equalsJeffH These links seem to be broken...

[...] CTAP issues fido-alliance/fido-2-specs#364, fido-alliance/fido-2-specs#365.

@equalsJeffH
Copy link
Contributor Author

@emlun

These links seem to be broken

you need to be annointed as a part of https://github.com/fido-alliance org

emlun added a commit that referenced this issue Nov 20, 2017
Fix #644: Add UV parameter to getAssertion
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.

6 participants