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

Add Rate Limiting and Trust Path to authnr selection criteria #484

Closed
wants to merge 8 commits into from
Closed

Add Rate Limiting and Trust Path to authnr selection criteria #484

wants to merge 8 commits into from

Conversation

gmandyam
Copy link

@gmandyam gmandyam commented May 31, 2017

@equalsJeffH equalsJeffH changed the title Add Rate Limiting to auth selection criteria Add Rate Limiting to authnr selection criteria Jun 1, 2017
@@ -1516,6 +1527,8 @@ When this operation is invoked, the authenticator must perform the following pro
so, return an error code equivalent to "{{NotAllowedError}}" and terminate the operation.
- If the |requireResidentKey| flag is set to |true| and the authenticator cannot store a [=Client-side-resident Credential
Private Key=], return an error code equivalent to "{{ConstraintError}}" and terminate the operation.
- If the |requireRateLimiting| flag is set to |true| and the authenticator cannot support [=Rate Limiting=], return an error code
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see anything in the authenticator operations that allows the authenticator to reject requests that come in too quickly. That needs to exist before this flag makes sense.

And then I wonder why rate limiting should be an optional part of the authenticator at all? Shouldn't we just require that authenticators limit brute force attacks?

Copy link
Contributor

@equalsJeffH equalsJeffH Jun 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And then I wonder why rate limiting should be an optional part of the authenticator at all? Shouldn't we just require that authenticators limit brute force attacks?

Agreed. The recently-announced FIDO Authenticator certification program has a requirement for this -- see row 3.9 in section 2.3 in fido-authenticator-security-requirements

The previously-mentioned UAF authenticator commands "security guidelines" was an apparent forebear to the fido-authenticator-security-requirements

I'm hoping that we can in the W3C context point off to FIDO and/or other orgs as appropriate with regards to such authenticator details.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So will webauthn normatively require FIDO certification for authenticators?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not think so. For example, anyone may implement FIDO UAF (& U2F) v1 protocols and authenticators (certified or not). An RP may accept registrations from uncertified authnrs at their discretion. For many (most?) authnrs, RPs are able to ascertain via the metadata service whether an authnr is certified or not. And if there is no authnr metadata, then that in itself is a piece of information the RP can (and should, if it is a high-value site) factor into its risk decision on whether to honor the authnr and accept the registration, or not.

In UAF we did not designate "rate limiting" as an identified authenticator characteristic (in the FIDO Registry spec). Rather, we state in UAF authenticator commands "security guidelines", in the "Matcher" section, that:

Authenticators must implement anti-hammering protections for their matchers.

..which is enforced as a part of authenticator Level 1 (and above) security certification. (Level 1 is slated to become the default entry-level authnr certification, encompassing the present "functional certification". See here for FIDO certification program info)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@equalsJeffH

I am sorry - I don't understand this response. One one hand, you have stated the following: "Given that rate-limiting of user verification attempts is already a requirement for FIDO-certified authenticators, I wonder whether this is needed.". Yet here you seem to be stating that the webauthn specification should not normatively require authenticators to be FIDO certified.

What would be your preferred approach (in terms of specification text) to ensuring that rate limiting be required across all webauthn authenticators?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gmandyam I do not think the webauthn specification is the proper place for stating detailed authenticator operational characteristics and requirements. One obvious option is to point to appropriate FIDO Alliance documents and programs, such as authenticator certification.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@equalsJeffH

Please feel free to create a PR suggesting as much and I will take a look. Note that https://w3c.github.io/webauthn/#authenticator-ops already puts forward "detailed authenticator operational characteristics and requirements", but I assume a normative reference to FIDO Security requirements can be included in that section.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@equalsJeffH

Another option is to add the rate limiting requirement in the definition of User Verification in Section 3. The current definition already discusses security requirements, so adding a statement on brute force attack prevention does not seem excessive. If you are OK with such an approach, I can close this PR and create a new one with the modified definition.

Copy link
Contributor

@equalsJeffH equalsJeffH left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that rate-limiting of user verification attempts is already a requirement for FIDO-certified authenticators, I wonder whether this is needed. details below.

@@ -306,6 +306,10 @@ The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD", "S
[=credential key pair=] is known as the <dfn>credential private key</dfn>. Note that in the case of [=self
attestation=], the [=credential key pair=] is also used as the [=attestation key pair=], see [=self attestation=]
for details.

: <dfn>Rate Limiting</dfn>
:: Rate limiting is the process by which an authenticator limits the number of unsuccessful authentication attemps within a set
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

by "authentication attemps" do you mean "user verification" (user verif) attempts?

My "fido-ready" S5 (which speaks OSTP, the precursor to UAF) limits the number of unsuccessful user verif (fingerprint modality) attempts. I do not think "user verification limiting" is represented in the protocol (it is not in UAF AFAICT). I do not know whether our mobile app is able to tune the number of "user verification" attempts before it tells me to take a 30sec break...

Copy link
Contributor

@equalsJeffH equalsJeffH Jun 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, UAF authenticator commands "security guidelines" says in the "Matcher" section:

Authenticators must implement anti-hammering protections for their matchers.

which it seems my S5 is doing.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Authentication attempts" are a term-of-art. See https://pages.nist.gov/800-63-3/sp800-63b.html#throttle.

@@ -1516,6 +1527,8 @@ When this operation is invoked, the authenticator must perform the following pro
so, return an error code equivalent to "{{NotAllowedError}}" and terminate the operation.
- If the |requireResidentKey| flag is set to |true| and the authenticator cannot store a [=Client-side-resident Credential
Private Key=], return an error code equivalent to "{{ConstraintError}}" and terminate the operation.
- If the |requireRateLimiting| flag is set to |true| and the authenticator cannot support [=Rate Limiting=], return an error code
Copy link
Contributor

@equalsJeffH equalsJeffH Jun 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And then I wonder why rate limiting should be an optional part of the authenticator at all? Shouldn't we just require that authenticators limit brute force attacks?

Agreed. The recently-announced FIDO Authenticator certification program has a requirement for this -- see row 3.9 in section 2.3 in fido-authenticator-security-requirements

The previously-mentioned UAF authenticator commands "security guidelines" was an apparent forebear to the fido-authenticator-security-requirements

I'm hoping that we can in the W3C context point off to FIDO and/or other orgs as appropriate with regards to such authenticator details.

@gmandyam gmandyam changed the title Add Rate Limiting to authnr selection criteria Add Rate Limiting and Trust Path to authnr selection criteria Jun 14, 2017
@gmandyam
Copy link
Author

Added trust path to PR as per #461

@selfissued
Copy link
Contributor

This PR is doing two unrelated things. I think we got agreement on the call today that we don't need the first (rate limiting) in the WebAuthn spec. Like we decided about Rolf's earlier PR that proposed multiple selection criteria, I think this one should be split into two separate PRs. Could you do that and close this one Giri? Or if we have already decided that we don't need rate limiting, you could just open one new one and close this one. Thanks.

@gmandyam
Copy link
Author

@selfissued

As per discussion on today's (June 14, 2017) conf call, this PR will likely be closed. @equalsJeffH will be proposing acceptable text to encompass rate limiting requirements for authenticators, and the group decided that RP selection of trust path was not necessary or desirable. Thanks.

@nadalin
Copy link
Contributor

nadalin commented Jun 22, 2017

Please close this by 6/23

@gmandyam
Copy link
Author

Closing and replacing with #499

@gmandyam gmandyam closed this Jun 23, 2017
@gmandyam gmandyam mentioned this pull request Mar 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants