quarkus icon indicating copy to clipboard operation
quarkus copied to clipboard

Quarkus fails to start with bcrypt password mapper

Open wowselim opened this issue 1 year ago • 15 comments

Describe the bug

When using elytron-security-jdbc and configuring the bcrypt password mapper, the quarkus application fails to start with the following error.

Expected behavior

Application starts successfully.

Actual behavior

We get the following stack trace:

java.lang.RuntimeException: Failed to start quarkus
        at io.quarkus.runner.ApplicationImpl.doStart(Unknown Source)
        at io.quarkus.runtime.Application.start(Application.java:101)
        at io.quarkus.runtime.ApplicationLifecycleManager.run(ApplicationLifecycleManager.java:111)
        at io.quarkus.runtime.Quarkus.run(Quarkus.java:71)
        at io.quarkus.runtime.Quarkus.run(Quarkus.java:44)
        at io.quarkus.runtime.Quarkus.run(Quarkus.java:124)
        at io.quarkus.runner.GeneratedMain.main(Unknown Source)
        at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
        at java.base/java.lang.reflect.Method.invoke(Method.java:580)
        at io.quarkus.runner.bootstrap.StartupActionImpl$1.run(StartupActionImpl.java:113)
        at java.base/java.lang.Thread.run(Thread.java:1583)
Caused by: java.lang.IllegalArgumentException: COM00001: Parameter 'saltColumn' must not be less than 1
        at org.wildfly.common.Assert.checkMinimumParameter(Assert.java:294)
        at org.wildfly.security.auth.realm.jdbc.mapper.PasswordKeyMapper.<init>(PasswordKeyMapper.java:75)
        at org.wildfly.security.auth.realm.jdbc.mapper.PasswordKeyMapper$Builder.build(PasswordKeyMapper.java:417)
        at io.quarkus.elytron.security.jdbc.BcryptPasswordKeyMapperConfig.toPasswordKeyMapper(BcryptPasswordKeyMapperConfig.java:63)
        at io.quarkus.elytron.security.jdbc.JdbcRecorder.registerPrincipalQuery(JdbcRecorder.java:61)
        at io.quarkus.elytron.security.jdbc.JdbcRecorder.createRealm(JdbcRecorder.java:39)
        at io.quarkus.deployment.steps.ElytronSecurityJdbcProcessor$configureJdbcRealmAuthConfig173765586.deploy_0(Unknown Source)
        at io.quarkus.deployment.steps.ElytronSecurityJdbcProcessor$configureJdbcRealmAuthConfig173765586.deploy(Unknown Source)
        ... 11 more

How to Reproduce?

Add these to your application.properties and run the application:

quarkus.security.jdbc.enabled=true
quarkus.security.jdbc.principal-query.sql=SELECT password, role FROM account WHERE email=?
quarkus.security.jdbc.principal-query.bcrypt-password-mapper.enabled=true
quarkus.security.jdbc.principal-query.bcrypt-password-mapper.password-index=1

Output of uname -a or ver

No response

Output of java -version

No response

Quarkus version or git rev

3.7.1

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

Here's a project that simply adds the elytron jdbc dependency and uses the aforementioned config

jdbc-auth-reproducer.zip

wowselim avatar Feb 07 '24 06:02 wowselim

I can see the config properties that might fix it here:

https://quarkus.io/guides/security-jdbc

But there's virtually no documentation on what value these properties should have.

wowselim avatar Feb 07 '24 06:02 wowselim

Also note that I used io.quarkus.elytron.security.common.BcryptUtil to generate passwords and I'd like to store them in one column together with the salt. I don't know how that would play into the config. Any guidance would be appreciated!

wowselim avatar Feb 07 '24 06:02 wowselim

cc @sberyozkin

geoand avatar Feb 07 '24 06:02 geoand

@wowselim The documentation for the Bcrypt related properties says that the indexes of the columns use 1 based numbering, but you only have specified the password index, but no salt and iteration count indexes, so this is a configuration issue. I haven't used this extension but I believe they should point to the table columns containing the bcrypt salt, iteration count etc.

Also note that I used io.quarkus.elytron.security.common.BcryptUtil to generate passwords and I'd like to store them in one column together with the salt.

I believe you should use quarkus.security.jdbc.principal-query.bcrypt-password-mapper.password-index, quarkus.security.jdbc.principal-query.bcrypt-password-mapper.salt-index and quarkus.security.jdbc.principal-query.bcrypt-password-mapper.iteration-count-index to point to the table columns

sberyozkin avatar Feb 07 '24 10:02 sberyozkin

@wowselim The documentation for the Bcrypt related properties says that the indexes of the columns use 1 based numbering, but you only have specified the password index, but no salt and iteration count indexes, so this is a configuration issue. I haven't used this extension but I believe they should point to the table columns containing the bcrypt salt, iteration count etc.

Also note that I used io.quarkus.elytron.security.common.BcryptUtil to generate passwords and I'd like to store them in one column together with the salt.

I believe you should use quarkus.security.jdbc.principal-query.bcrypt-password-mapper.password-index, quarkus.security.jdbc.principal-query.bcrypt-password-mapper.salt-index and quarkus.security.jdbc.principal-query.bcrypt-password-mapper.iteration-count-index to point to the table columns

@sberyozkin what can I do if I want to use bcrypt like it's described here: https://quarkus.io/guides/security-jpa#password-storage-and-hashing

With the default option, passwords are stored and hashed with bcrypt under the Modular Crypt Format (MCF). While using MCF, the hashing algorithm, iteration count, and salt are stored as a part of the hashed value. As such, we do not need dedicated columns to keep them.

Setting all of those other configs to 1 does not work with passwords that were generated using BcryptUtil.

I think having them as separate columns is pretty outdated and most libraries (vert.x, spring boot) do it in this format (as does quarkus as seen above).

wowselim avatar Feb 07 '24 10:02 wowselim

@wowselim

https://quarkus.io/guides/security-jpa#password-storage-and-hashing is a different extension, quarkus-security-jpa.

Setting all of those other configs to 1 does not work with passwords that were generated using BcryptUtil.

This issue is about Quarkus failing to start using quarkus-security-jdbc, and I believe it is about configuring it correctly to point to the already populated table.

I think we can close this issue since your requirement is possibly about using quarkus-security-jpa. My proposal is to close this issue and then you start a Discussion how to use Bcrypt with the security-jpa, do you agree ?

sberyozkin avatar Feb 07 '24 10:02 sberyozkin

@sberyozkin actually I would like to use quarkus-security-jdbc specifically because I use jooq for the db layer and I don’t want a dependency on hibernate. I just wanted to highlight the inconsistency regarding jpa and jdbc when it comes to auth. The jpa approach seems more modern in this aspect.

I think it would be more appropriate to create a ticket to improve the jdbc docs to show how to handle bcrypt since it’s not clear when the docs only use clear text.

wowselim avatar Feb 07 '24 13:02 wowselim

My proposal is to close this issue and then you start a Discussion how to use Bcrypt with the security-jpa, do you agree ?

i think jpa is much better documented (see the link i posted before). The jdbc docs leave out some important details since they only focus on clear text auth.

wowselim avatar Feb 07 '24 13:02 wowselim

@wowselim Sure, security-jdbc was the very first identity provider extension, while indeed we focused on security-jpa and recommend it due its ease of use.

I suppose we can resolve this issue by updating the JDBC docs, like I said, I haven't worked with this extension, but what I understand is that you can use BCryptUtil method to create a hash with a known salt and count, and then, in the tutorial, you can update that Create Table instruction to include the hash, salt and count and refer to their column indexes in the configuration. I agree it is a fairly low level approach, but it is done by design in security-jdbc.

Would you like to test it and it works I can update the docs ?

sberyozkin avatar Feb 07 '24 13:02 sberyozkin

@sberyozkin Sure, I would like to improve the docs and provide a bcrypt example. I've found this on the web:

https://issues.redhat.com/browse/ELY-1497

So it seems like support for this format should already be in wildfly jdbc security realm (org.wildfly.security.auth.realm.jdbc.JdbcSecurityRealm). I would like to do some more research and maybe even add support for this in quarkus.

wowselim avatar Feb 07 '24 17:02 wowselim

Hey, I've asked around on the elytron zulip server and it looks like there's no interest in this feature or a contribution. This issue can be closed since it's essentially a duplicate of #5667.

wowselim avatar Feb 15 '24 09:02 wowselim

@wowselim Hi, can you please try what is suggested in https://github.com/quarkusio/quarkus/issues/38634#issuecomment-1932095959 here ?

sberyozkin avatar Feb 16 '24 11:02 sberyozkin

@wowselim Hi, can you please try what is suggested in #38634 (comment) here ?

@sberyozkin it will not work because BCryptUtil uses the modular crypt format which is not supported by the jdbc security extension. See the javadoc for the hashing function:

https://github.com/quarkusio/quarkus/blob/main/extensions/elytron-security-common/runtime/src/main/java/io/quarkus/elytron/security/common/BcryptUtil.java#L64

wowselim avatar Feb 16 '24 11:02 wowselim

@wowselim If you'd like you can consider opening a PR where BcryptUtil class will have another method for producing a format understood by the JDBC extension ?

sberyozkin avatar Feb 16 '24 22:02 sberyozkin

@wowselim If you'd like you can consider opening a PR where BcryptUtil class will have another method for producing a format understood by the JDBC extension ?

@sberyozkin I don't like the idea of having the three (password, salt, iteration count) split to be honest. It's quite unconventional and not used anywhere else as far as I know. I wrote some custom identity providers to achieve what I wanted. For anyone else interested here's a write up: https://blog.selim.co/2024/02/17/quarkus-form-authentication-using-jdbc.html

wowselim avatar Feb 17 '24 16:02 wowselim

Sounds good @wowselim, thanks, do you have an account on X ? If yes, please consider promoting your post there as well. OK, let me close this issue since you have agreed to it

sberyozkin avatar Feb 20 '24 11:02 sberyozkin

@sberyozkin I shared my post here: https://twitter.com/libslim/status/1758896395379982393

wowselim avatar Feb 20 '24 11:02 wowselim