[
https://issues.apache.org/jira/browse/JAMES-3674?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17447191#comment-17447191
]
Benoit Tellier commented on JAMES-3674:
---------------------------------------
+1 about maintaining retro-compactibility and reducing changes. As an
adminitstrator willing to upgrade my security policy, I should be able to
perform a rolling update.
That being said, while we are at it, changing the DB structure for passwords we
might want to do it right. Hence I do think [~ieugen] got a point here.
As always I understand that [~kotto] might already be operating on top of a
database hashed with a simple SHA-512 hash based on the password. Hence:
- The fact of allowing SHA-512 hashes might be a very strong motivation to
push the contribution forward. We could allow such an algorithm (IMO do not
harm much)
- We could easily reuse this work to plug PBKDF2 as a default (recommended)
choice. - instead of SHA-512 as a default in Karsten proposal.
- We could easily add a "salt collumn". Just like we did add an "algorithm"
collumn. If it benefits [~kotto] we could add a fallback mechanism to the
username if empty.
- We could rely on Password4J for the PBKDF2 digest, but we might use row java
as well, which avoids an additional dependency, on a critical topic
(passwords...), that also might not be widely adopted (orange lights for
Password4J according to me: 150 stars is not that much, 15 dependent projects
without well known ones. https://www.baeldung.com/java-password-hashing
presents how to do PBKDF2 with Java crypto:
{code:java}
SecureRandom random = new SecureRandom();
byte[] salt = new byte[16];
random.nextBytes(salt);
KeySpec spec = new PBEKeySpec(password.toCharArray(), salt, 65536, 128);
SecretKeyFactory factory = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1");
byte[] hash = factory.generateSecret(spec).getEncoded();
{code}
A bit less fluent than the Password4J variation but it clearly includes all of
the components.
Bonus point with PBKDF2: Being able to configure key length and iteration
counts.
> Re username change, my understanding is that James does not support it, i.e.
> the name/email address is the account identifier, and if you want a different
> one you need a new account. Thus this is not really an issue. Correct me if I
> am wrong please.
Yes your understanding is correct. The notion of "account" does not exist and
is tidely coupled with the username. Addressing this would be a (massive)
refactoring.
> However, it will definitely have a performance impact, which is actually a
> feature in this case.
I'm not expecting too much of an impact. We could conduct micro benchmarks to
verify and secure this point.
Anyway I am not opposed to a "simple" SHA-512 alternative.
> Since I am not a core maintainer, I cannot decide if the issue is worth it.
IMO everybody voice should be heard, and authority posture have not their place
in an apache community.
Thus your opinion clearly matters.
> That said, this PR does not prevent adoption of Password4j in the future.
+1 We can start with the adoption of the SHA-512 salted hash algorithm you
propose.
I would encourage storing the hash in the DB as well as we are already
modifying the storage format (with a fallback to the username if no salt is
stored?) The change would be easy.
I then devote myself for the PBKF2 algorithm implementation if needed ;-)
> Support password salting and hash scheme upgrading
> --------------------------------------------------
>
> Key: JAMES-3674
> URL: https://issues.apache.org/jira/browse/JAMES-3674
> Project: James Server
> Issue Type: Improvement
> Components: UsersStore & UsersRepository
> Affects Versions: master
> Reporter: Karsten Otto
> Priority: Major
> Time Spent: 5.5h
> Remaining Estimate: 0h
>
> Currently, James does not use salt during password hashing, so its password
> database is vulnerable to rainbow table cracking if someone ever manages to
> steal it. Furthermore, there is no mechanism to upgrade user passwords to
> stronger/different hashing once they are created (cf. legacy hashing mode).
> This is a problem for any installation that does not employ an external LDAP
> user database.
> A simple solution is to include the user name as salt in the password hash.
> For this purpose, the {{hashingMode}} choices in {{usersrepository.xml}}
> should include an new mode "salted" in addition to "legacy" and "default".
> Additionally, the database should include an explicit column in the user
> table, which specifies the {{hashingMode}} of the stored password, and is
> used during verification. However, when a user changes the password, the
> configured {{algorithm}} and {{hashingMode}} from {{usersrepository.xml}}
> will be used instead. This way, the database gradually upgrades over time to
> the preferred setting.
> T-Shirt size L.
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
