search

ClickHouse / clickhouse-java

Java client and JDBC driver for ClickHouse
https://clickhouse.com
Apache License 2.0
1.4k stars 512 forks source link

Client SSL certificate usage in strict verification mode prevents from password-based SSL authentication #1454

Open gzhivilo opened 9 months ago

gzhivilo commented 9 months ago

Describe the bug

When SSL verification mode is set to "strict" at both client and server sides and client's certifivate is provided, jdbc connection fails for users authenticated by password. Some investigation follows below.

Steps to reproduce

  1. Set verificationMode to strict for Clickhouse server
  2. Place client certificate, key and CA chain files on your client PC
  3. Try to connect via JDBC using password

Expected behaviour

Connection establishes successfully.

Code example

Properties props = new Properties();
props.setProperty("user","test_user");
props.setProperty("password","******");
props.setProperty("ssl","true");
props.setProperty("sslmode","strict");
props.setProperty("sslrootcert",".\\certs\\ca_certs.pem");
props.setProperty("sslcert",".\\certs\\client_cert.pem");
props.setProperty("sslkey",".\\certs\\client_key.key");
props.setProperty("","");
Connection conn = DriverManager.getConnection(DB_URL,props);

Error log

system.session_log excerpt (reduced for brevity)

type LoginFailure . . . event_date 2023-09-21 . . . user test_user auth_type NO_PASSWORD . . . client_port 53968 interface HTTP client_hostname
client_name
client_revision 0 client_version_major 0 client_version_minor 0 client_version_patch 0 failure_reason test_user: Authentication failed: password is incorrect, or there is no user with such name.

JDBC log is as follows

Exception caught: Code: 516. DB::Exception: test_user: Authentication failed: password is incorrect, or there is no user with such name. (AUTHENTICATION_FAILED) (version 23.1.4.1), server ClickHouseNode [uri=https://*host*:8443/default, options={sslmode=strict,sslkey=*,sslcert=.\certs\client_cert.pem,sslrootcert=.\certs\ca_certs.pem}]@-990356814

Java stack trace doesn't show up

Configuration

Environment

ClickHouse server

Some investigation

Setting up strict verification mode at server side causes a client to provide its certificate in all cases including password-based authentication. Client does it with sslcert property. It provides the password as well in password property but it seems to be discarded by the driver. The problem presumably arises from com.clickhouse.client.http.ClickHouseHttpConnection class, createDefaultHeaders method line 222:

if (config.isSsl() && !ClickHouseChecker.isNullOrEmpty(config.getSslCert())) {
    map.put("x-clickhouse-ssl-certificate-auth", "on");
} else if (!ClickHouseChecker.isNullOrEmpty(credentials.getPassword())) {
    map.put("x-clickhouse-key", credentials.getPassword());
}

Providing client's certificate makes config.getSslCert() nonempty and causes if-branch to work thus discarding the password ("x-clickhouse-key" header).

Possible solution

So the solution at first glance could be just to swap clauses in if-else statement above. It turns into

if (!ClickHouseChecker.isNullOrEmpty(credentials.getPassword())) {
    map.put("x-clickhouse-key", credentials.getPassword());
} else if (config.isSsl() && !ClickHouseChecker.isNullOrEmpty(config.getSslCert())) {
    map.put("x-clickhouse-ssl-certificate-auth", "on");
}

A sideback effect is that users with empty password will always be authenticated using SSL certificate if it is provided. But it seems to be a security enforcement, rather than a problem.

All stated above is just a proposal, guru's comments needed.

zhicwu commented 9 months ago

Thank you for reporting this issue and providing these helpful insights. Skipping password authentication in the presence of a client certificate is a risky assumption that should be avoided.

gzhivilo commented 9 months ago

Hi @zhicwu , I've made the mentioned above changes in the code and performed finctional testing for both original and fixed versions. Chlickhouse server was upgraded to version 23.3.13.1. It appeared that original version of JDBC driver failed to establish connection not only for 'strict' verification mode on the server. Modes 'relaxed' and 'none' were also affected. You can find functional testing results attached. Fixed version worked fine in all the cases. BTW, I wasn't able to reproduce issue #1305 by any means, JDBC driver worked fine! JDBCTestingReport.txt

kingdeepak28 commented 9 months ago

Hi team , I am also facing the same issue . kindly share the solution in terms of code change and let us know when we will get fixed version .

Thanks for your support .

m00dawg commented 4 months ago

Confirmed having a similar issue. The errors seem benign in that CH falls back to password auth for identification but still uses TLS for encryption. The issue I'm running into is these errors are inflating the error log and causing potentially legitimate errors to get lost. Lowering the priority of these from "error" to "warning" might be a sufficient solve here perhaps?

jeeva-travtech commented 4 months ago

This is an issue which is occurring while using DBeaver as well, since it uses Clickhouse JDBC, with the SSL certificates , even with valid username and password I keep getting 'Authentication Failed'

gzhivilo commented 4 months ago

@mzitnik Hi Mark, coud you please include the fix in some upcoming release? Swapping two lines of code is so little for a separate branch. For now, forcing STRICT mode on the server by security reasons (banking, government, etc.) prevents LDAP-authenticated users from connecting to Clickhouse.