Biometric Criteria Extension #510

gmandyam · 2017-07-26T02:47:49Z

Biometric Selection Criteria extension

equalsJeffH

this looks nominally ok but IMO requires some terminological polishing.

equalsJeffH · 2017-11-20T17:53:47Z

index.bs

+that could be leveraged when creating the credential. 
+
+: Extension identifier
+:: `biometricBound`


the extension title is "Authenticator Biometric Criteria", yet the extension identifier is "biometricBound" which is inconsistent. FAR and FRR are "biometric matching criteria", yes? Perhaps the latter should be "biometricMatchingCriteria" ? And perhaps the section title ought to be "Biometric Authenticator Matching Criteria" ?

equalsJeffH · 2017-11-20T17:53:53Z

index.bs

+:: Biometric performance bounds:
+
+    <pre class="idl">
+    dictionary biometricCriteria{


perhaps biometricCriteria should be authenticatorBiometricMatchingCriteria ?

equalsJeffH · 2017-11-20T17:55:03Z

index.bs

+
+: Client extension processing
+:: This extension can only be used during {{CredentialsContainer/create()}}. If the client supports the Authenticator Biometric Criteria
+    Extension and biometric authenticators are available, it MUST use the first available biometric authenticator whose FAR and FRR match the bounds as provided.


we perhaps ought to define "biometric authenticators" up in terminology section, and then use [=biometric authenticators=] here.

equalsJeffH

thanks @gmandyam. Other than needing to (i think) define the input in terms of CBOR/CDDL and some further wordsmithing, this seems overall ok. HTH.

equalsJeffH · 2017-12-06T21:33:58Z

index.bs

+        float FAR;
+        float FRR;
+        };
+    </pre>


IIUC, we ought to define the input in terms of CDDL, rather than WebIDL, yes?
see also issue #679 "add CDDL to every extension" and issue #626 "Extensions need to define how their parameters convert to/from CBOR"

equalsJeffH · 2017-12-06T21:38:47Z

index.bs

@@ -3093,6 +3096,43 @@ This [=registration extension=] and [=authentication extension=] enables use of
                    01           -- Subitem 3: CBOR short for Matcher Protection Type Software
    </pre>

+## Biometric Authenticator Matching Criteria Extension (biometricBound) ## {#sctn-authenticator-biometric-criteria-extension}


I would title it:
"Biometric Authenticator Performance Bounds Extension"

equalsJeffH · 2017-12-06T21:43:33Z

index.bs

+## Biometric Authenticator Matching Criteria Extension (biometricBound) ## {#sctn-authenticator-biometric-criteria-extension}
+
+This [=registration extension=] allows a [=[RP]=] to specify the desired performance of a biometric authenticator
+that could be leveraged when creating the credential. 


suggest:
This [=registration extension=] allows [=[RPS]=] to specify the desired performance bounds for selecting [=biometric authenticators=] as candidates to be employed in a [=registration=] ceremony.

equalsJeffH · 2017-12-06T21:45:22Z

index.bs

+    The FAR is the maximum false rejection rate for a biometric authenticator allowed by the [=[RP]=].
+
+: Client extension processing
+:: This extension can only be used during {{CredentialsContainer/create()}}. If the client supports the Authenticator Biometric Criteria


s/Authenticator Biometric Criteria Extension/this extension/

equalsJeffH · 2017-12-06T21:47:17Z

index.bs

+:: Biometric performance bounds:
+
+    <pre class="idl">
+    dictionary authenticatorBiometricMatchingCriteria{


suggest:
s/ authenticatorBiometricMatchingCriteria/ authenticatorBiometricPerfBounds/

gmandyam · 2017-12-11T17:38:33Z

@equalsJeffH

Thanks for review. Tried to address your comments in latest PR.

Regarding CDDL, this extension is intended to terminate in the client and therefore has no required authenticator input. My interpretation of https://w3c.github.io/webauthn/#sctn-client-extension-processing is that CBOR definition is only required when a "client extension input can be used to determine the CBOR authenticator extension input."

emlun · 2017-12-12T15:42:46Z

index.bs

+
+: Client extension processing
+:: This extension can only be used during {{CredentialsContainer/create()}}. If the client supports the this
+    extension and at least one [=biometric authenticator=] is available, it MUST use the first available biometric authenticator whose FAR and FRR match the bounds as provided.


First off: s/the this/this/

Second:

If the client supports the this extension and at least one biometric authenticator is available, it MUST use the first available biometric authenticator whose FAR and FRR match the bounds as provided.

This formulation would likely be confusing with the new hot-plugging approach for create(). We no longer assume that there will be a single instant when you can list all "available" authenticators. Instead they will appear at some point (possibly immediately, e.g., platform authenticators) during the timeout, and the client can't know if the user will plug in an eligible biometric authenticator after the client has found a non-biometric authenticator. I therefore suggest reformulating this to something more like:

If the client supports this extension, it MUST NOT use a biometric authenticator whose FAR or FRR does not match the bounds as provided.

gmandyam · 2017-12-12T23:13:39Z

@emlun

Thanks. Changes made; please review.

emlun

Thanks!

equalsJeffH

just a few consistency items, otherwise LGTM

equalsJeffH · 2017-12-16T00:02:52Z

index.bs

@@ -3861,6 +3864,45 @@ This [=registration extension=] and [=authentication extension=] enables use of
                    01           -- Subitem 3: CBOR short for Matcher Protection Type Software
    </pre>

+
+## Biometric Authenticator Performance Bounds Extension (biometricBound) ## {#sctn-authenticator-biometric-criteria-extension}


nit: s/sctn-authenticator-biometric-criteria-extension/sctn-authenticator-biometric-perf-bounds/

suggest:
s/biometricBound/biometricPerfBounds/

equalsJeffH · 2017-12-29T14:28:44Z

index.bs

+as candidates to be employed in a [=registration=] ceremony. 
+
+: Extension identifier
+:: `biometricMatchingCriteria`


suggest:
s/biometricMatchingCriteria/biometricPerfBounds/

equalsJeffH · 2017-12-29T14:31:23Z

wrt #510 (comment) -- what do others think?

emlun · 2018-01-03T17:29:23Z

Re #510 (comment) : Seems reasonable to me. The appid and authnSel extensions work in the same way.

akshayku · 2018-01-17T17:21:19Z

index.bs

+
+: Client extension processing
+:: This extension can only be used during {{CredentialsContainer/create()}}. If the client supports this
+    extension, it MUST NOT use a biometric authenticator whose FAR or FRR does not match the bounds as provided.


How does client know these values? Are you expecting client to contact some authority who verifies it? FIDO metadata service?

@akshayku

Yes, that is certainly one potential approach. Expectation is that authenticator would've gone through FIDO Biometric Cert, had its performance verified, and agreed to publish its performance data on the FIDO Metadata Service (MDS).

This whole thing seems really brittle. It's more likely to provide a false sense of security than to actually be accurate.

so the above text perhaps ought to additionally indicate that various platform-specific means (eg some possibly based on the MDS) are anticipated for the client platform to use in order to have knowledge of these values.

selfissued

Providing no data of this is kind is better than providing data that provides a false sense of security. Unless there are existing related deployments to cite that successfully use similar functionality, I would consider this too speculative to include in the specification.

Please consider creating a separate specification for this proposed extension. This could still be registered in the IANA WebAuthn Extensions registry, and therefore be available to implementers.

selfissued · 2018-01-22T18:39:44Z

index.bs

+
+: Client extension processing
+:: This extension can only be used during {{CredentialsContainer/create()}}. If the client supports this
+    extension, it MUST NOT use a biometric authenticator whose FAR or FRR does not match the bounds as provided.


This whole thing seems really brittle. It's more likely to provide a false sense of security than to actually be accurate.

gmandyam · 2018-01-22T20:33:25Z

@selfissued

I am sorry; I don't understand your exact concern.

Please consider creating a separate specification for this proposed extension.

Why is https://fidoalliance.org/specs/fido-uaf-v1.2-rd-20171128/fido-metadata-statement-v1.2-rd-20171128.html#dictionary-biometricaccuracydescriptor-members insufficient for this purpose? Do you believe FRR and FAR are not sufficiently defined in this specification, and therefore a new specification is needed?

This whole thing seems really brittle. It's more likely to provide a false sense of security than to actually be accurate.

I don't understand this concern as written. Perhaps you can explain further. Do you believe that FIDO MDS information on biometric accuracy will be inaccurate and/or unverified, for instance?

equalsJeffH

ok, thx, LGTM.

equalsJeffH · 2018-01-24T16:53:35Z

index.bs

+
+: Client extension processing
+:: This extension can only be used during {{CredentialsContainer/create()}}. If the client supports this
+    extension, it MUST NOT use a biometric authenticator whose FAR or FRR does not match the bounds as provided.


so the above text perhaps ought to additionally indicate that various platform-specific means (eg some possibly based on the MDS) are anticipated for the client platform to use in order to have knowledge of these values.

equalsJeffH · 2018-01-24T16:55:58Z

@selfissued

Please consider creating a separate specification for this proposed extension

Hi Mike -- it seems to me that you are proposing a process here that the WG has not itself decided upon and is up to the WG to impose rather than individual members.

emlun · 2018-02-05T16:27:22Z

@selfissued

Providing no data of this is kind is better than providing data that provides a false sense of security.

How would you feel about this if the specified requirement in the extension, instead of

If the client supports this extension, it MUST NOT use a biometric authenticator whose FAR or FRR does not match the bounds as provided.

was something more like this?

If the client supports this extension, it MUST NOT use a biometric authenticator whose FAR or FRR is not certified by an authoritative source to match the bounds as provided.

equalsJeffH · 2018-02-05T17:53:14Z

building on @emlun's fine suggestion, we perhaps ought to add a pointer to where one might obtain such metadata (as we have done elsewhere in the spec):

If the client supports this extension, it MUST NOT use a biometric authenticator whose FAR or FRR is not certified by an authoritative source to match the bounds as provided. For example, the FIDO Metadata Service [FIDOMetadataService] provides one way to access such information.

I note in FIDO Metadata Service section 3.3 "BiometricAccuracyDescriptor dictionary" that there is provision for conveying FAR, FRR, ERR, FAAR and other applicable metadata values.

emlun · 2018-02-05T21:34:15Z

By the way I'd also like to note that in the end it falls on the RP to verify any claimed performance certification by inspecting the attestation statement, and not blindly trust the client to do this verification. It is thus not critical to the integrity of the protocol that the client can derive these properties perfectly or even reliably, but it is of course nice if the extension can enable failing early.

nadalin · 2018-02-06T18:17:03Z

@equalsJeffH @emlun So the extentions are vendor spepcifc that is they can do what they want, so including specifics may not be the correct way to go as that may dictate a specific implementaion

equalsJeffH · 2018-02-06T18:29:54Z

I think what @emlun is teasing-out is a possible impl-cons Note or something similar, that's all.

selfissued

After the merge conflicts are resolved, I'm now OK with this being merged.

equalsJeffH · 2018-02-08T20:38:14Z

After the merge conflicts are resolved, I'm now OK with this being merged.

nadalin · 2018-02-08T23:27:28Z

@gmandyam can you fix the branch conflicts and merge

gmandyam · 2018-02-09T00:08:08Z

@nadalin

Yes - I am working on it. I am getting a Travis build failure currently and am trying to debug.

gmandyam changed the title ~~Update index.bs~~ Biometric Criteria Extension Jul 26, 2017

nadalin added this to the CR milestone Jul 26, 2017

equalsJeffH added subtype:extensions type:technical labels Jul 26, 2017

nadalin requested a review from selfissued July 26, 2017 17:31

nadalin assigned selfissued and gmandyam Jul 26, 2017

equalsJeffH suggested changes Nov 20, 2017

View reviewed changes

equalsJeffH suggested changes Dec 6, 2017

View reviewed changes

emlun requested changes Dec 12, 2017

View reviewed changes

Update index.bs

7b84760

emlun approved these changes Dec 13, 2017

View reviewed changes

akshayku mentioned this pull request Dec 21, 2017

Authnr selection aaguidlist #479

Closed

equalsJeffH suggested changes Dec 29, 2017

View reviewed changes

akshayku reviewed Jan 17, 2018

View reviewed changes

Update index.bs

cbdd72e

selfissued requested changes Jan 22, 2018

View reviewed changes

equalsJeffH reviewed Jan 24, 2018

View reviewed changes

Update index.bs

0edd80c

nadalin added the subtype:impl-cons label Feb 6, 2018

selfissued approved these changes Feb 6, 2018

View reviewed changes

Merge branch 'master' into master

e71d187

equalsJeffH approved these changes Feb 8, 2018

View reviewed changes

gmandyam added 2 commits February 8, 2018 16:16

Update index.bs

12363a8

Update index.bs

971b383

gmandyam merged commit d563d83 into w3c:master Feb 9, 2018

WebAuthnBot pushed a commit that referenced this pull request Feb 9, 2018

Built by Travis-CI: d563d83 Merge pull request #510 from gmandyam/master

22b8841

Biometric Criteria Extension #510

Biometric Criteria Extension #510

Conversation

gmandyam commented Jul 26, 2017 • edited by pr-preview bot Loading

equalsJeffH left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

equalsJeffH left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gmandyam commented Dec 11, 2017

Choose a reason for hiding this comment

gmandyam commented Dec 12, 2017

emlun left a comment

Choose a reason for hiding this comment

equalsJeffH left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

equalsJeffH commented Dec 29, 2017

emlun commented Jan 3, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

selfissued left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gmandyam commented Jan 22, 2018

equalsJeffH left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

equalsJeffH commented Jan 24, 2018

emlun commented Feb 5, 2018

equalsJeffH commented Feb 5, 2018

emlun commented Feb 5, 2018

nadalin commented Feb 6, 2018

equalsJeffH commented Feb 6, 2018

selfissued left a comment

Choose a reason for hiding this comment

equalsJeffH commented Feb 8, 2018

nadalin commented Feb 8, 2018

gmandyam commented Feb 9, 2018

gmandyam commented Jul 26, 2017 •

edited by pr-preview bot

Loading