w3c / webauthn

Web Authentication: An API for accessing Public Key Credentials
https://w3c.github.io/webauthn/
Other
1.16k stars 166 forks source link

credentialList needs to be non-empty in order to call authenticatorGetAssertion #481

Closed rlin1 closed 7 years ago

rlin1 commented 7 years ago

The authenticatorGetAssertion (section 5.2.2) is defined such that the authenticator handles empty "list of credentials acceptable to the RP" itself (i.e. building list of available credentials on the authenticator and asking user for disambiguation).

Our "Use existing credential get" algorithm will never invoke authenticatorGetAssertion without knowing the (non-empty) credentialList in advance. At this time we allow the platform to "execute a platform-specific procedure to determine which, if any, credentials in publicKeyOptions.allowList are present on this authenticator". But this doesn't necessarily work with roaming authenticators.

As a consequence, our current approach doesn't allow roaming authenticators (i.e. in which the platform doesn't know which credentials are related to the authenticator) without expecting it to be used as a second-factor (i.e. in a situation in which the RP server could already provide the credentialList).

We could modify step 13.3 in section 4.1.4 to say: if (there was a platform-specific procedure to determine which credentials are present on the authenticator) { if (credentialList is empty) then continue; } else { If C.transports is not empty, the client SHOULD select one transport from transports. Then, using transport, invoke the authenticatorGetAssertion operation on authenticator, with rpId, clientDataHash, empty credentialList, and authenticatorExtensions as parameters }

This would invoke authenticatorGetAssertion and would allow the authenticator to select the appropriate credential.

rlin1 commented 7 years ago

Other opinions?

rlin1 commented 7 years ago

Is this a duplicate of #387?

equalsJeffH commented 7 years ago

not a duplicate of issue #387 tho closely related i think. I will try to address this in PR #427, if I understand this issue correctly.

A consideration regarding roaming authnrs is that if a roaming authnr is also multi-factor (nee first-factor), then the credential private key is stored in the authnr's internal storage -- i think this is the rationale behind your 3d parag where you say:

our current approach doesn't allow roaming authenticators (i.e. in which the platform doesn't know which credentials are related to the authenticator)

..yes?

where you say:

without expecting it to be used as a second-factor (i.e. in a situation in which the RP server could already provide the credentialList).

Should "without" actually be "unless" ?

We could modify step 13.3 in section 4.1.4 to say:

It seems you are referring to the https://www.w3.org/TR/webauthn/ spec revision? Might be better to refer to the editor's draft. section 4.1.4 is {#getAssertion}. Tho the step you refer to is now step 15.

rlin1 commented 7 years ago

"unless" might be better.

Replace step (yes: now 15)

  1. If |credentialList| [=list/is empty=] then [=continue=]

by:

  1. If executing that platform-specific procedure succeeded and |credentialList| 
        [=list/is empty=] then [=continue=].

  1. If executing that platform-specific procedure failed and |credentialList| 
     [=list/is empty=]:
      1. If <code>|C|.{{transports}}</code> [=list/is not empty=], the client 
          SHOULD select one |transport| from {{transports}}. Then, using 
          |transport|, invoke the [=authenticatorGetAssertion=] operation on 
          |authenticator|, with |rpId|, |clientDataHash|, empty |credentialList|, 
          and |authenticatorExtensions| as parameters.
      1. Otherwise, using local configuration knowledge of the appropriate transport 
          to use with |authenticator|, invoke the [=authenticatorGetAssertion=] operation 
          on |authenticator| with |rpId|, |clientDataHash|, empty |credentialList|, and
          |clientExtensions| as parameters.
rlin1 commented 7 years ago

Yes: this change is for roaming authenticators having their internal key storage and the ability to act as the sole-multi-factor (not only as 2nd factor).

equalsJeffH commented 7 years ago

wrt https://github.com/w3c/webauthn/issues/481#issuecomment-305422881 by @rlin1 above...

hm. I am not sure how "the platform-specific procedure succeeded and |credentialList| [=list/is empty=]" relates to roaming first-factor authnrs? would you please describe the use case in more detail?

also, i am wondering whether in the following step "If executing that platform-specific procedure failed and |credentialList| [=list/is empty=]:" you actually mean to say "if....is not empty". This is because the first substep assumes that |credentialList| is not empty.

Please have a look at the current state of this PR to see if how it is presently written addresses this overall issue. I am thinking it might do so, but am unsure. thx.

rlin1 commented 7 years ago

I didn't invent the "platform-specific" procedure - it was there before. I think such platform specific procedure works fine for bound authenticators. At this time I don't see a functionality in CTAP that would allow any platform implementing such procedure for roaming authenticators.

As a consequence I think the best thing a platform can do in such case is sending an empty credential list to the (roaming) authenticator. According to the web authn spec on authenticatorGetAssertion the authenticator can handle empty credential lists.

equalsJeffH commented 7 years ago

I didn't invent the "platform-specific" procedure - it was there before.

yes, I realize that.

I think such platform specific procedure works fine for bound authenticators. At this time I don't see a functionality in CTAP that would allow any platform implementing such procedure for roaming authenticators.

Ok.

As a consequence I think the best thing a platform can do in such case is sending an empty credential list to the (roaming) authenticator. According to the web authn spec on authenticatorGetAssertion the authenticator can handle empty credential lists.

Ok, then perhaps the current state of PR #427 addresses this.