XWIKI-22257: Hash the superadmin password

[michitux]

Jira URL

https://jira.xwiki.org/browse/XWIKI-22257

Changes

Description

• Support bcrypt-encrypted password hashes for superadmin.
• Document bcrypt support in the configuration file.

Clarifications

• I've chosen bcrypt as it is an acceptable choice for password hashing and supported by htpasswd, offering an easy way to create the password hash outside of XWiki. More secure options unfortunately aren't supported by htpasswd.
• Having a superadmin password of exactly 60 characters starting with $2 seems unlikely enough that I don't think there is any issue of treating such a value as a bcrypt hash and not a plain text password.

Screenshots & Video

No UI changes.

Executed Tests

Built xwiki-platform-oldcore with quality profile.

Expected merging strategy

• Prefers squash: Yes
• Backport on branches:
  • New feature, but we could consider backporting it as it seems safe to me and a nice security improvement.

[tmortagne]

I feel it would be more future-proof (and also give more choices now) to introduce the concept of password prefix (like we do in the password property, but maybe with a slightly safer syntax, like {bcrypt}:$2y$08$2Mel30blRQ7E.XievLW00.AltivcBuU1HEl2mPG2qRGrd7FmWIwB6).

[michitux]

I feel it would be more future-proof (and also give more choices now) to introduce the concept of password prefix (like we do in the password property, but maybe with a slightly safer syntax, like {bcrypt}:$2y$08$2Mel30blRQ7E.XievLW00.AltivcBuU1HEl2mPG2qRGrd7FmWIwB6).

The prefix $2y$ is already the prefix for bcrypt, and its version (2 being bcrypt and the y the version). For argon2id (the currently recommended password hash by the OWASP cheat sheet), the prefix would be $argon2id$. While this is not a standard, it is widely used, see

• https://stackoverflow.com/questions/73594387/what-is-the-standard-for-password-hash-string-encoding
• https://passlib.readthedocs.io/en/stable/modular_crypt_format.html
• https://github.com/P-H-C/phc-string-format/blob/master/phc-sf-spec.md

So in other words, I don't think there is any need to add an extra prefix as long as we use the "standard" password encoding format.

[tmortagne]

Yes, it's just that using a prefix with a value supported by MessageDigest#getInstance() would allow supporting a bunch of algorithms right away quite effortlessly.

Note that it's in no way a blocker comment, I'm just wondering if it would not make things easier in the long run.

[michitux]

Yes, it's just that using a prefix with a value supported by MessageDigest#getInstance() would allow supporting a bunch of algorithms right away quite effortlessly.

At least the algorithms that are required by Java 17 - plain SHA-1 and SHA-256 - aren't suitable for secure password hashing. And I couldn't find anything that this class would support state-of-the-art password hashing methods.

I found that DelegatingPasswordEncoder in spring-security actually uses the same kind of pattern so it might indeed make sense to support that as I couldn't find any implementation of this modular crypt format for Java. I'm wondering if we couldn't just use spring-security-crypto to gain support for a lot of different (also state-of-the-art) password hashes. It doesn't seem to have any dependencies and from what I've seen in the code that I looked at, it doesn't seem to rely on the spring framework in any way. The documentation of the module seems to confirm this:

The code is distributed as part of the core module but has no dependencies on any other Spring Security (or Spring) code.

[tmortagne]

At least the algorithms that are required by Java 17 - plain SHA-1 and SHA-256 - aren't suitable for secure password hashing. And I couldn't find anything that this class would support state-of-the-art password hashing methods.

Yes, but Bouncy Castle (which we already embed) adds a lot of others MessageDigest implementations.

Edit: but BCrypt does not seem to be in that list, not sure why since Bouncy Castle support it.