Backport #48989 to 23.3: Add fallback to password auth after failed SSL auth

src/Server/TCPHandler.cpp

@@ -1247,10 +1247,17 @@ void TCPHandler::receiveHello()
         Poco::Net::SecureStreamSocket secure_socket(socket());
         if (secure_socket.havePeerCertificate())
         {
-            session->authenticate(
-                SSLCertificateCredentials{user, secure_socket.peerCertificate().commonName()},
-                getClientAddress(client_info));
-            return;
+            try
+            {
+                session->authenticate(
+                    SSLCertificateCredentials{user, secure_socket.peerCertificate().commonName()},
+                    getClientAddress(client_info));
+                return;
+            }
+            catch (...)
+            {
+                tryLogCurrentException(log, "SSL authentication failed, falling back to password authentication");
+            }
         }
     }
 #endif

tests/integration/test_ssl_cert_authentication/test.py

@@ -38,37 +38,6 @@ def started_cluster():
         cluster.shutdown()
 
 
-def get_ssl_context(cert_name):
-    context = WrapSSLContextWithSNI(SSL_HOST, ssl.PROTOCOL_TLS_CLIENT)
-    context.load_verify_locations(cafile=f"{SCRIPT_DIR}/certs/ca-cert.pem")
-    if cert_name:
-        context.load_cert_chain(
-            f"{SCRIPT_DIR}/certs/{cert_name}-cert.pem",
-            f"{SCRIPT_DIR}/certs/{cert_name}-key.pem",
-        )
-        context.verify_mode = ssl.CERT_REQUIRED
-    context.check_hostname = True
-    return context
-
-
-def execute_query_https(
-    query, user, enable_ssl_auth=True, cert_name=None, password=None
-):
-    url = (
-        f"https://{instance.ip_address}:{HTTPS_PORT}/?query={urllib.parse.quote(query)}"
-    )
-    request = urllib.request.Request(url)
-    request.add_header("X-ClickHouse-User", user)
-    if enable_ssl_auth:
-        request.add_header("X-ClickHouse-SSL-Certificate-Auth", "on")
-    if password:
-        request.add_header("X-ClickHouse-Key", password)
-    response = urllib.request.urlopen(
-        request, context=get_ssl_context(cert_name)
-    ).read()
-    return response.decode("utf-8")
-
-
 config = """<clickhouse>
     <openSSL>
         <client>
@@ -86,7 +55,7 @@ def execute_query_https(
 </clickhouse>"""
 
 
-def execute_query_native(node, query, user, cert_name):
+def execute_query_native(node, query, user, cert_name, password=None):
     config_path = f"{SCRIPT_DIR}/configs/client.xml"
 
     formatted = config.format(
@@ -108,46 +77,119 @@ def execute_query_native(node, query, user, cert_name):
     )
 
     try:
-        result = client.query(query, user=user)
+        result = client.query(query, user=user, password=password)
         remove(config_path)
         return result
     except:
         remove(config_path)
         raise
 
 
-def test_https():
+def test_native():
     assert (
-        execute_query_https("SELECT currentUser()", user="john", cert_name="client1")
+        execute_query_native(
+            instance, "SELECT currentUser()", user="john", cert_name="client1"
+        )
         == "john\n"
     )
     assert (
-        execute_query_https("SELECT currentUser()", user="lucy", cert_name="client2")
+        execute_query_native(
+            instance, "SELECT currentUser()", user="lucy", cert_name="client2"
+        )
         == "lucy\n"
     )
     assert (
-        execute_query_https("SELECT currentUser()", user="lucy", cert_name="client3")
+        execute_query_native(
+            instance, "SELECT currentUser()", user="lucy", cert_name="client3"
+        )
         == "lucy\n"
     )
 
 
-def test_native():
+def test_native_wrong_cert():
+    # Wrong certificate: different user's certificate
+    with pytest.raises(Exception) as err:
+        execute_query_native(
+            instance, "SELECT currentUser()", user="john", cert_name="client2"
+        )
+    assert "AUTHENTICATION_FAILED" in str(err.value)
+
+    # Wrong certificate: self-signed certificate.
+    # In this case clickhouse-client itself will throw an error
+    with pytest.raises(Exception) as err:
+        execute_query_native(
+            instance, "SELECT currentUser()", user="john", cert_name="wrong"
+        )
+    assert "UNKNOWN_CA" in str(err.value)
+
+
+def test_native_fallback_to_password():
+    # Unrelated certificate, correct password
     assert (
         execute_query_native(
-            instance, "SELECT currentUser()", user="john", cert_name="client1"
+            instance,
+            "SELECT currentUser()",
+            user="jane",
+            cert_name="client2",
+            password="qwe123",
         )
-        == "john\n"
+        == "jane\n"
     )
-    assert (
+
+    # Unrelated certificate, wrong password
+    with pytest.raises(Exception) as err:
         execute_query_native(
-            instance, "SELECT currentUser()", user="lucy", cert_name="client2"
+            instance,
+            "SELECT currentUser()",
+            user="jane",
+            cert_name="client2",
+            password="wrong",
+        )
+    assert "AUTHENTICATION_FAILED" in str(err.value)
+
+
+def get_ssl_context(cert_name):
+    context = WrapSSLContextWithSNI(SSL_HOST, ssl.PROTOCOL_TLS_CLIENT)
+    context.load_verify_locations(cafile=f"{SCRIPT_DIR}/certs/ca-cert.pem")
+    if cert_name:
+        context.load_cert_chain(
+            f"{SCRIPT_DIR}/certs/{cert_name}-cert.pem",
+            f"{SCRIPT_DIR}/certs/{cert_name}-key.pem",
         )
+        context.verify_mode = ssl.CERT_REQUIRED
+    context.check_hostname = True
+    return context
+
+
+def execute_query_https(
+    query, user, enable_ssl_auth=True, cert_name=None, password=None
+):
+    url = (
+        f"https://{instance.ip_address}:{HTTPS_PORT}/?query={urllib.parse.quote(query)}"
+    )
+    request = urllib.request.Request(url)
+    request.add_header("X-ClickHouse-User", user)
+    if enable_ssl_auth:
+        request.add_header("X-ClickHouse-SSL-Certificate-Auth", "on")
+    if password:
+        request.add_header("X-ClickHouse-Key", password)
+    response = urllib.request.urlopen(
+        request, context=get_ssl_context(cert_name)
+    ).read()
+    return response.decode("utf-8")
+
+
+def test_https():
+    assert (
+        execute_query_https("SELECT currentUser()", user="john", cert_name="client1")
+        == "john\n"
+    )
+    assert (
+        execute_query_https("SELECT currentUser()", user="lucy", cert_name="client2")
         == "lucy\n"
     )
     assert (
-        execute_query_native(
-            instance, "SELECT currentUser()", user="lucy", cert_name="client3"
-        )
+        execute_query_https("SELECT currentUser()", user="lucy", cert_name="client3")
         == "lucy\n"
     )
 
@@ -178,23 +220,6 @@ def test_https_wrong_cert():
         )
 
 
-def test_native_wrong_cert():
-    # Wrong certificate: different user's certificate
-    with pytest.raises(Exception) as err:
-        execute_query_native(
-            instance, "SELECT currentUser()", user="john", cert_name="client2"
-        )
-    assert "AUTHENTICATION_FAILED" in str(err.value)
-
-    # Wrong certificate: self-signed certificate.
-    # In this case clickhouse-client itself will throw an error
-    with pytest.raises(Exception) as err:
-        execute_query_native(
-            instance, "SELECT currentUser()", user="john", cert_name="wrong"
-        )
-    assert "UNKNOWN_CA" in str(err.value)
-
-
 def test_https_non_ssl_auth():
     # Users with non-SSL authentication are allowed, in this case we can skip sending a client certificate at all (because "verificationMode" is set to "relaxed").
     # assert execute_query_https("SELECT currentUser()", user="peter", enable_ssl_auth=False) == "peter\n"