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

Expired tokens should not trigger bruteforce protection #12131

Closed
rullzer opened this issue Oct 29, 2018 · 2 comments · Fixed by #12140
Closed

Expired tokens should not trigger bruteforce protection #12131

rullzer opened this issue Oct 29, 2018 · 2 comments · Fixed by #12140
Assignees
Labels
1. to develop Accepted and waiting to be taken care of enhancement
Milestone

Comments

@rullzer
Copy link
Member

rullzer commented Oct 29, 2018

When a token expires (for example OAuth tokens) and authentication is attempted with it we fail.
This is fine as authentication should fail with expired tokens.

However we should not register this as a bruteforce attempts. As it is actually not that.

CC: @ChristophWurst @Dagefoerde

@rullzer rullzer added enhancement 1. to develop Accepted and waiting to be taken care of labels Oct 29, 2018
@rullzer rullzer self-assigned this Oct 29, 2018
@nextcloud-bot
Copy link
Member

GitMate.io thinks possibly related issues are #11104 (Expire tokens hardening), #11103 ([stable14] Expire tokens hardening), #12130 (Reset bruteforce on token refresh OAuth), #3156 (Clear bruteforce protection from user upon successful login), and #2626 (Add bruteforce protection to two factor challenge endpoint).

@Dagefoerde
Copy link
Member

Yes, those are false positives. Also, as the (attempted, invalid) token is pretty much the same every time this would be a very stupid brute force attempt ;)

rullzer added a commit that referenced this issue Oct 30, 2018
Fixes #12131

If we hit an expired token there is no need to continue checking. Since
we know it is a token.

We also should not register this with the bruteforce throttler as it is
actually a valid token. Just expired. Instead the authentication should
fail. And buisness continues as usual.

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@rullzer rullzer added this to the Nextcloud 15 milestone Oct 30, 2018
rullzer added a commit that referenced this issue Oct 30, 2018
Fixes #12131

If we hit an expired token there is no need to continue checking. Since
we know it is a token.

We also should not register this with the bruteforce throttler as it is
actually a valid token. Just expired. Instead the authentication should
fail. And buisness continues as usual.

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
rullzer added a commit that referenced this issue Oct 30, 2018
Fixes #12131

If we hit an expired token there is no need to continue checking. Since
we know it is a token.

We also should not register this with the bruteforce throttler as it is
actually a valid token. Just expired. Instead the authentication should
fail. And buisness continues as usual.

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Dagefoerde pushed a commit to Dagefoerde/server that referenced this issue Oct 31, 2018
Fixes nextcloud#12131

If we hit an expired token there is no need to continue checking. Since
we know it is a token.

We also should not register this with the bruteforce throttler as it is
actually a valid token. Just expired. Instead the authentication should
fail. And buisness continues as usual.

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Dagefoerde pushed a commit to Dagefoerde/server that referenced this issue Oct 31, 2018
Fixes nextcloud#12131

If we hit an expired token there is no need to continue checking. Since
we know it is a token.

We also should not register this with the bruteforce throttler as it is
actually a valid token. Just expired. Instead the authentication should
fail. And buisness continues as usual.

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
rullzer added a commit that referenced this issue Nov 2, 2018
Fixes #12131

If we hit an expired token there is no need to continue checking. Since
we know it is a token.

We also should not register this with the bruteforce throttler as it is
actually a valid token. Just expired. Instead the authentication should
fail. And buisness continues as usual.

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop Accepted and waiting to be taken care of enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants