diff --git a/base/poco/NetSSL_OpenSSL/src/SSLManager.cpp b/base/poco/NetSSL_OpenSSL/src/SSLManager.cpp index 94b66cdcf5f1e..80d85855fe3c9 100644 --- a/base/poco/NetSSL_OpenSSL/src/SSLManager.cpp +++ b/base/poco/NetSSL_OpenSSL/src/SSLManager.cpp @@ -340,7 +340,9 @@ void SSLManager::initDefaultContext(bool server) { _ptrDefaultClientContext->enableSessionCache(cacheSessions); } - bool extendedVerification = config.getBool(prefix + CFG_EXTENDED_VERIFICATION, false); + /// Only an outbound connection has a requested host name to check: on an accepted socket this + /// would run against the client's address, which a client certificate does not name. + bool extendedVerification = config.getBool(prefix + CFG_EXTENDED_VERIFICATION, !server); if (server) _ptrDefaultServerContext->enableExtendedCertificateVerification(extendedVerification); else diff --git a/docs/reference/settings/server-settings/_server_settings_outside_source.mdx b/docs/reference/settings/server-settings/_server_settings_outside_source.mdx index 23af0e0fa6040..8a323dfa5e587 100644 --- a/docs/reference/settings/server-settings/_server_settings_outside_source.mdx +++ b/docs/reference/settings/server-settings/_server_settings_outside_source.mdx @@ -1170,7 +1170,7 @@ Keys for server/client settings: | `certificateFile` | Path to the client/server certificate file in PEM format. You can omit it if `privateKeyFile` contains the certificate. | | | `cipherList` | Supported OpenSSL encryptions. | `ALL:!ADH:!LOW:!EXP:!MD5:!3DES:@STRENGTH` | | `disableProtocols` | Protocols that are not allowed to be used. | | -| `extendedVerification` | If enabled, verify that the certificate CN or SAN matches the peer hostname. | `false` | +| `extendedVerification` | If enabled, verify that the certificate CN or SAN matches the peer hostname. | `true` for `openSSL.client`, `false` for `openSSL.server` | | `fips` | Activates OpenSSL FIPS mode. Supported if the library's OpenSSL version supports FIPS. | `false` | | `invalidCertificateHandler` | Class (a subclass of CertificateHandler) for verifying invalid certificates. For example: ` RejectCertificateHandler ` . | `RejectCertificateHandler` | | `loadDefaultCAFile` | Wether built-in CA certificates for OpenSSL will be used. ClickHouse assumes that builtin CA certificates are in the file `/etc/ssl/cert.pem` (resp. the directory `/etc/ssl/certs`) or in file (resp. directory) specified by the environment variable `SSL_CERT_FILE` (resp. `SSL_CERT_DIR`). | `true` | diff --git a/src/Core/ServerSettings.cpp b/src/Core/ServerSettings.cpp index 5d47a1b810c50..4c36ce7ff4ef4 100644 --- a/src/Core/ServerSettings.cpp +++ b/src/Core/ServerSettings.cpp @@ -1915,7 +1915,7 @@ Configured as `named_collections_storage.type` (`` (resp. the directory ``) or in file (resp. directory) specified by the environment variable `` (resp. ``).)", 0, "openSSL.client.loadDefaultCAFile") \ DECLARE(String, openssl_client_chipher_list, "ALL:!ADH:!LOW:!EXP:!MD5:!3DES:@STRENGTH", R"(Supported OpenSSL encryptions.)", 0, "openSSL.client.cipherList") \ DECLARE(Bool, openssl_client_cache_sessions, false, R"(Enables or disables caching sessions. Must be used in combination with ``. Acceptable values: ``, ``.)", 0, "openSSL.client.cacheSessions") \ - DECLARE(Bool, openssl_client_extended_verification, false, R"(If enabled, verify that the certificate CN or SAN matches the peer hostname.)", 0, "openSSL.client.extendedVerification") \ + DECLARE(Bool, openssl_client_extended_verification, true, R"(If enabled, verify that the certificate CN or SAN matches the peer hostname.)", 0, "openSSL.client.extendedVerification") \ DECLARE(Bool, openssl_client_required_tls_v1, false, R"(Require a TLSv1 connection. Acceptable values: ``, ``.)", 0, "openSSL.client.requireTLSv1") \ DECLARE(Bool, openssl_client_required_tls_v1_1, false, R"(Require a TLSv1.1 connection. Acceptable values: ``, ``.)", 0, "openSSL.client.requireTLSv1_1") \ DECLARE(Bool, openssl_client_required_tls_v1_2, false, R"(Require a TLSv1.2 connection. Acceptable values: ``, ``.)", 0, "openSSL.client.requireTLSv1_2") \ diff --git a/tests/integration/test_accept_invalid_certificate/configs/ssl_config_ca_signed.xml b/tests/integration/test_accept_invalid_certificate/configs/ssl_config_ca_signed.xml new file mode 100644 index 0000000000000..b1dccbb15c66e --- /dev/null +++ b/tests/integration/test_accept_invalid_certificate/configs/ssl_config_ca_signed.xml @@ -0,0 +1,15 @@ + + + 9440 + + + + /etc/clickhouse-server/config.d/client-cert.pem + /etc/clickhouse-server/config.d/client-key.pem + /etc/clickhouse-server/config.d/ca-cert.pem + none + + + diff --git a/tests/integration/test_accept_invalid_certificate/test.py b/tests/integration/test_accept_invalid_certificate/test.py index afea8011c4b99..b66f311b92355 100644 --- a/tests/integration/test_accept_invalid_certificate/test.py +++ b/tests/integration/test_accept_invalid_certificate/test.py @@ -9,6 +9,7 @@ SCRIPT_DIR = os.path.dirname(os.path.realpath(__file__)) MAX_RETRY = 5 +CA_CERT = f"{SCRIPT_DIR}/certs/ca-cert.pem" cluster = ClickHouseCluster(__file__) instance = cluster.add_instance( @@ -35,6 +36,18 @@ ) +node2 = cluster.add_instance( + "node2", + main_configs=[ + "configs/ssl_config_ca_signed.xml", + "certs/client-key.pem", + "certs/client-cert.pem", + "certs/ca-cert.pem", + ], + with_zookeeper=False, +) + + @pytest.fixture(scope="module", autouse=True) def started_cluster(): try: @@ -61,6 +74,36 @@ def started_cluster(): """ +# node2 presents a certificate this CA signs, so the chain is valid and only the name can fail. +config_ca_signed = """ + + + {caConfig} + false + + +""" + +config_ca_signed_sni_override = """ + client + + + {caConfig} + false + + +""" + +config_ca_signed_no_extended_verification = """ + + + {caConfig} + false + false + + +""" + def execute_query_native(node, query, config): fd, config_path = tempfile.mkstemp( @@ -132,3 +175,29 @@ def test_strict_connection_reject(): config_connection_accept.format(ip_address=f"{instance.ip_address}"), ) assert "certificate verify failed" in str(err.value) + + +def test_hostname_mismatch_rejected_by_default(): + with pytest.raises(Exception) as err: + execute_query_native(node2, "SELECT 1", config_ca_signed.format(caConfig=CA_CERT)) + assert "Unacceptable certificate" in str(err.value) + + +def test_hostname_match_accepted(): + assert ( + execute_query_native( + node2, "SELECT 1", config_ca_signed_sni_override.format(caConfig=CA_CERT) + ) + == "1\n" + ) + + +def test_extended_verification_disabled(): + assert ( + execute_query_native( + node2, + "SELECT 1", + config_ca_signed_no_extended_verification.format(caConfig=CA_CERT), + ) + == "1\n" + ) diff --git a/tests/integration/test_acme_tls/configs/config.xml b/tests/integration/test_acme_tls/configs/config.xml index 4594c72e885e3..0977ed12d7114 100644 --- a/tests/integration/test_acme_tls/configs/config.xml +++ b/tests/integration/test_acme_tls/configs/config.xml @@ -14,6 +14,9 @@ + + false AcceptCertificateHandler diff --git a/tests/integration/test_acme_tls/configs/config_multi.xml b/tests/integration/test_acme_tls/configs/config_multi.xml index 56c9d415886c4..9cda6eb0ba878 100644 --- a/tests/integration/test_acme_tls/configs/config_multi.xml +++ b/tests/integration/test_acme_tls/configs/config_multi.xml @@ -14,6 +14,9 @@ + + false AcceptCertificateHandler diff --git a/tests/integration/test_bind_host/configs/config.d/ssl_conf.xml b/tests/integration/test_bind_host/configs/config.d/ssl_conf.xml index c11c5736d96f8..e51749f354385 100644 --- a/tests/integration/test_bind_host/configs/config.d/ssl_conf.xml +++ b/tests/integration/test_bind_host/configs/config.d/ssl_conf.xml @@ -7,6 +7,9 @@ /etc/clickhouse-server/config.d/server.key + + false AcceptCertificateHandler diff --git a/tests/integration/test_ssl_cert_authentication/test.py b/tests/integration/test_ssl_cert_authentication/test.py index ca4e6147f928b..118172905b9a9 100644 --- a/tests/integration/test_ssl_cert_authentication/test.py +++ b/tests/integration/test_ssl_cert_authentication/test.py @@ -44,6 +44,7 @@ def started_cluster(): config = """ + {sslHost} strict @@ -62,6 +63,7 @@ def execute_query_native(node, query, user, cert_name, password=None): certificateFile=f"{SCRIPT_DIR}/certs/{cert_name}-cert.pem", privateKeyFile=f"{SCRIPT_DIR}/certs/{cert_name}-key.pem", caConfig=f"{SCRIPT_DIR}/certs/ca-cert.pem", + sslHost=SSL_HOST, ) file = open(config_path, "w")