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

Fix max length requirements for the throttler metadata #10670

Merged
merged 1 commit into from
Aug 24, 2018

Conversation

ChristophWurst
Copy link
Member

If a failed login is logged, we save the username as metadata
in the bruteforce throttler. To prevent database error due to
very long strings, this truncates the username at 64 bytes in
the assumption that no real username is longer than that.

If a failed login is logged, we save the username as metadata
in the bruteforce throttler. To prevent database error due to
very long strings, this truncates the username at 64 bytes in
the assumption that no real username is longer than that.long strings,

Signed-off-by: Christoph Wurst <[email protected]>
@ChristophWurst ChristophWurst added bug 3. to review Waiting for reviews labels Aug 13, 2018
@ChristophWurst ChristophWurst added this to the Nextcloud 14 milestone Aug 13, 2018
@ChristophWurst ChristophWurst self-assigned this Aug 13, 2018
Copy link
Member

@blizzz blizzz left a comment

Choose a reason for hiding this comment

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

strictly speaking, login names (which is what is passed in thereto) can be longer than usernames. For the local backend, it is also possible to login via email, which could be longer than the restriction on the username. And other backends could have other restrictions. In practice, this size should be safe. Alternatively, we can max out the 255 bytes the meta data col can take, just need to consider the json overhead (although this spot wouldn't be the exactly right one). It's likely good enough.

@rullzer
Copy link
Member

rullzer commented Aug 13, 2018

Or we should switch the metadata table to hold more than 255 chars...

@blizzz
Copy link
Member

blizzz commented Aug 13, 2018

Or we should switch the metadata table to hold more than 255 chars...

yes, however that would be mostly to please the script kiddies to send us their short novels.

Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

👍

@MorrisJobke MorrisJobke merged commit 6099786 into master Aug 24, 2018
@MorrisJobke MorrisJobke deleted the fix/login-throttle-username-length branch August 24, 2018 14:25
@rullzer rullzer mentioned this pull request Aug 24, 2018
3 tasks
@MorrisJobke MorrisJobke mentioned this pull request Aug 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants