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.

Sign up for GitHub

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

Jump to bottom

Support more password schemes from passlib #1662

Closed
4 tasks done
Diman0 opened this issue Oct 14, 2020 · 3 comments · Fixed by #1753
Closed
4 tasks done

Support more password schemes from passlib #1662

Diman0 opened this issue Oct 14, 2020 · 3 comments · Fixed by #1753
Labels
type/enhancement Enhances existing functionality

Comments

@Diman0
Copy link
Member

Diman0 commented Oct 14, 2020

Before you open your issue

  • Check if no issue or pull-request for this already exists.
  • Check documentation and FAQ. (Tip, use the search function on the documentation page)
  • You understand Mailu is made by volunteers in their free time — be conscise, civil and accept that delays can occur.
  • The title of the issue should be short and simple. It should contain specific terms related to the actual issue. Be specific while writing the title.

Description

Mailu uses the passlib python library for handling password hashes. The password hashes that can be used must be passed to the constructor of passlib. These are currently the supported hashes.

    scheme_dict = {'PBKDF2': "pbkdf2_sha512",
                   'BLF-CRYPT': "bcrypt",
                   'SHA512-CRYPT': "sha512_crypt",
                   'SHA256-CRYPT': "sha256_crypt",
                   'MD5-CRYPT': "md5_crypt",
                   'CRYPT': "des_crypt"}

In the mailu chat someone asked for support SSHA512 (salted ssh512). It would be nice if we could support more hashing algorithms. This enables users to export/import users from other systems that use these hash algorithms.

What must happen for implementing this enhancement request?

  1. modify requirements.txt to use a newer version of passlib to have support for newer hashes. Version 1.7.1 of passlib is currently used. Version 1.7.3 also has support for salted sha512.
  2. modify models.py with additional hash algorithms. eg.
    scheme_dict = {'PBKDF2': "pbkdf2_sha512",
                   'BLF-CRYPT': "bcrypt",
                   'SHA512-CRYPT': "sha512_crypt",
                   'SHA256-CRYPT': "sha256_crypt",
                   'MD5-CRYPT': "md5_crypt",
                   'CRYPT': "des_crypt",
                   'SSHA256': "ldap_salted_sha256"
                   'SSHA512': "ldap_salted_sha512"  }

The proper solution is probably to make it possible to configure the scheme_dict via the Mailu.env file. This way you can still add support for other hashes via the mailu.env file.
@Diman0 Diman0 added the type/enhancement Enhances existing functionality label Oct 14, 2020
mlow added a commit to mlow/Mailu that referenced this issue Dec 6, 2020
@mlow
Add support for SSHA256, SSHA512 password hashing
3b58c9d 
The SSHA256 and SSHA512 schemes are "pre tagged", i.e. they have
`{SSHA256}` or `{SSHA512}` already prepended out of hashlib, so the
check_password and set_password logic is modified to support this.

passlib was updated to 1.7.4 to add support for `ldap_salted_sha256` and
`ldap_salted_sha512`.

This addition is particuilarly useful for users coming from other mail
systems (for example, iRedMail).

Closes Mailu#1662
@mlow mlow mentioned this issue Dec 6, 2020
Closed
1 task
mlow added a commit to mlow/Mailu that referenced this issue Dec 6, 2020
@mlow
Add support for SSHA256, SSHA512 password hashing
539fffd 
The SSHA256 and SSHA512 schemes are "pre tagged", i.e. they have
`{SSHA256}` or `{SSHA512}` already prepended out of hashlib, so the
check_password and set_password logic is modified to support this.

passlib was updated to 1.7.4 to add support for `ldap_salted_sha256` and
`ldap_salted_sha512`.

This addition is particuilarly useful for users coming from other mail
systems (for example, iRedMail).

Closes Mailu#1662
mlow added a commit to mlow/Mailu that referenced this issue Dec 6, 2020
@mlow
Add support for SSHA256, SSHA512 password hashing
17d4ef3 
The SSHA256 and SSHA512 schemes are "pre tagged", i.e. they have
`{SSHA256}` or `{SSHA512}` already prepended out of hashlib, so the
check_password and set_password logic is modified to support this.

passlib was updated to 1.7.4 to add support for `ldap_salted_sha256` and
`ldap_salted_sha512`.

This addition is particuilarly useful for users coming from other mail
systems (for example, iRedMail).

Closes Mailu#1662
@nextgens
Copy link
Contributor

nextgens commented Jan 16, 2021 •
edited

I've added a comment on #1194 (comment) regarding introducing new DoS vectors.

It may be worth documenting that aspect when new hash schemes are added.

@nextgens nextgens mentioned this issue Jan 18, 2021
Closed
This was referenced Feb 2, 2021
Closed
Merged
@nextgens
Copy link
Contributor

@Diman0 Please review #1753 ; it allows for all hash types supported by passlib to be used

@Diman0
Copy link
Member Author

Diman0 commented Mar 8, 2021

I'm closing this PR. It is not required anymore with the changes from nextgens PR1753..

@Diman0 Diman0 closed this as completed Mar 8, 2021
bors bot added a commit that referenced this issue Mar 8, 2021
@bors @nextgens
Merge #1753
9985f81 
1753: Better password storage r=mergify[bot] a=nextgens

## What type of PR?

Enhancement: optimization of the logic to speedup authentication requests, support the import of most hashes passlib supports.

## What does this PR do?

- it changes the default password cold-storage format to sha256+bcrypt
- it enhances the logic to ensure that no CPU cycles are wasted when valid credentials are found
- it fixes token authentication on /webdav/
- it lowers the number of rounds used for token storage (on the basis that they are high-entropy: not bruteforceable and speed matters)
- it introduces a new setting to set the number of rounds used by the password hashing function (CREDENTIAL_ROUNDS). The setting can be adjusted as required and existing hashes will be migrated to the new cost-factor.
- it updates the version of passlib in use and enables all supported hash types (that will be converted to the current settings on first use)
- it removes the PASSWORD_SCHEME setting

### Related issue(s)
- close #1194
- close #1662
- close #1706

## Prerequistes
Before we can consider review and merge, please make sure the following list is done and checked.
If an entry in not applicable, you can check it or remove it from the list.

- [x] In case of feature or enhancement: documentation updated accordingly
- [x] Unless it's docs or a minor change: add [changelog](https://mailu.io/master/contributors/guide.html#changelog) entry file.


Co-authored-by: Florent Daigniere <nextgens@freenetproject.org>
bors bot added a commit that referenced this issue Mar 8, 2021
@bors @nextgens
Merge #1753
08e0488 
1753: Better password storage r=mergify[bot] a=nextgens

## What type of PR?

Enhancement: optimization of the logic to speedup authentication requests, support the import of most hashes passlib supports.

## What does this PR do?

- it changes the default password cold-storage format to sha256+bcrypt
- it enhances the logic to ensure that no CPU cycles are wasted when valid credentials are found
- it fixes token authentication on /webdav/
- it lowers the number of rounds used for token storage (on the basis that they are high-entropy: not bruteforceable and speed matters)
- it introduces a new setting to set the number of rounds used by the password hashing function (CREDENTIAL_ROUNDS). The setting can be adjusted as required and existing hashes will be migrated to the new cost-factor.
- it updates the version of passlib in use and enables all supported hash types (that will be converted to the current settings on first use)
- it removes the PASSWORD_SCHEME setting

### Related issue(s)
- close #1194
- close #1662
- close #1706

## Prerequistes
Before we can consider review and merge, please make sure the following list is done and checked.
If an entry in not applicable, you can check it or remove it from the list.

- [x] In case of feature or enhancement: documentation updated accordingly
- [x] Unless it's docs or a minor change: add [changelog](https://mailu.io/master/contributors/guide.html#changelog) entry file.


Co-authored-by: Florent Daigniere <nextgens@freenetproject.org>
bors bot added a commit that referenced this issue Mar 9, 2021
@bors @nextgens
Merge #1753
7e2db9c 
1753: Better password storage r=nextgens a=nextgens

## What type of PR?

Enhancement: optimization of the logic to speedup authentication requests, support the import of most hashes passlib supports.

## What does this PR do?

- it changes the default password cold-storage format to sha256+bcrypt
- it enhances the logic to ensure that no CPU cycles are wasted when valid credentials are found
- it fixes token authentication on /webdav/
- it lowers the number of rounds used for token storage (on the basis that they are high-entropy: not bruteforceable and speed matters)
- it introduces a new setting to set the number of rounds used by the password hashing function (CREDENTIAL_ROUNDS). The setting can be adjusted as required and existing hashes will be migrated to the new cost-factor.
- it updates the version of passlib in use and enables all supported hash types (that will be converted to the current settings on first use)
- it removes the PASSWORD_SCHEME setting

### Related issue(s)
- close #1194
- close #1662
- close #1706

## Prerequistes
Before we can consider review and merge, please make sure the following list is done and checked.
If an entry in not applicable, you can check it or remove it from the list.

- [x] In case of feature or enhancement: documentation updated accordingly
- [x] Unless it's docs or a minor change: add [changelog](https://mailu.io/master/contributors/guide.html#changelog) entry file.


Co-authored-by: Florent Daigniere <nextgens@freenetproject.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
type/enhancement Enhances existing functionality
Projects
None yet
Milestone
No milestone
2 participants
@nextgens @Diman0