A few improvements in the implementation of SSL certificate authentication.

docs/ja/operations/external-authenticators/ssl-x509.md

@@ -0,0 +1 @@
+../../../en/operations/external-authenticators/ssl-x509.md

docs/zh/operations/external-authenticators/ssl-x509.md

@@ -0,0 +1 @@
+../../../en/operations/external-authenticators/ssl-x509.md

src/Access/Authentication.cpp

@@ -5,7 +5,6 @@
 #include <Access/LDAPClient.h>
 #include <Access/GSSAcceptor.h>
 #include <Common/Exception.h>
-#include <Poco/Net/SecureStreamSocketImpl.h>
 #include <Poco/SHA1Engine.h>
 #include <Common/typeid_cast.h>
 
@@ -15,7 +14,6 @@ namespace DB
 namespace ErrorCodes
 {
     extern const int NOT_IMPLEMENTED;
-    extern const int WRONG_PASSWORD;
 }
 
 namespace
@@ -69,23 +67,6 @@ namespace
     }
 }
 
-std::string getPeerCertificateCommonName(const Poco::Net::SocketImpl * socketImpl)
-{
-    std::string cn;
-
-    if (socketImpl->secure())
-    {
-        // expect socket is an instance of SecureStreamSocket
-        const Poco::Net::SecureStreamSocketImpl * secureSocketImpl = dynamic_cast<const Poco::Net::SecureStreamSocketImpl *>(socketImpl);
-        if (secureSocketImpl && secureSocketImpl->havePeerCertificate())
-        {
-            Poco::Crypto::X509Certificate cert = secureSocketImpl->peerCertificate();
-            cn = cert.commonName();
-        }
-    }
-
-    return cn;
-}
 
 bool Authentication::areCredentialsValid(const Credentials & credentials, const AuthenticationData & auth_data, const ExternalAuthenticators & external_authenticators)
 {
@@ -170,7 +151,7 @@ bool Authentication::areCredentialsValid(const Credentials & credentials, const
         }
     }
 
-    if (const auto * certificate_credentials = typeid_cast<const CertificateCredentials *>(&credentials))
+    if (const auto * ssl_certificate_credentials = typeid_cast<const SSLCertificateCredentials *>(&credentials))
     {
         switch (auth_data.getType())
         {
@@ -185,11 +166,7 @@ bool Authentication::areCredentialsValid(const Credentials & credentials, const
                 throw Authentication::Require<GSSAcceptorContext>(auth_data.getKerberosRealm());
 
             case AuthenticationType::SSL_CERTIFICATE:
-                // N.B. the certificate should only be trusted when 'strict' SSL mode is enabled
-                if (!auth_data.getSSLCertificateCommonNames().contains(certificate_credentials->getX509CommonName()))
-                    throw Exception("X.509 certificate is not on allowed list", ErrorCodes::WRONG_PASSWORD);
-
-                return true;
+                return auth_data.getSSLCertificateCommonNames().contains(ssl_certificate_credentials->getCommonName());
 
             case AuthenticationType::MAX:
                 break;

src/Access/Authentication.h

@@ -2,7 +2,6 @@
 
 #include <Access/Common/AuthenticationData.h>
 #include <Common/Exception.h>
-#include <Poco/Net/SocketImpl.h>
 #include <base/types.h>
 
 
@@ -16,8 +15,6 @@ namespace ErrorCodes
 class Credentials;
 class ExternalAuthenticators;
 
-std::string getPeerCertificateCommonName(const Poco::Net::SocketImpl * socketImpl);
-
 /// TODO: Try to move this checking to Credentials.
 struct Authentication
 {

src/Access/Common/AuthenticationData.cpp

@@ -210,4 +210,12 @@ void AuthenticationData::setPasswordHashBinary(const Digest & hash)
     throw Exception("setPasswordHashBinary(): authentication type " + toString(type) + " not supported", ErrorCodes::NOT_IMPLEMENTED);
 }
 
+
+void AuthenticationData::setSSLCertificateCommonNames(boost::container::flat_set<String> common_names_)
+{
+    if (common_names_.empty())
+        throw Exception("The 'SSL CERTIFICATE' authentication type requires a non-empty list of common names.", ErrorCodes::BAD_ARGUMENTS);
+    ssl_certificate_common_names = std::move(common_names_);
+}
+
 }

src/Access/Common/AuthenticationData.h

@@ -85,7 +85,7 @@ public:
     void setKerberosRealm(const String & realm) { kerberos_realm = realm; }
 
     const boost::container::flat_set<String> & getSSLCertificateCommonNames() const { return ssl_certificate_common_names; }
-    void setSSLCertificateCommonNames(boost::container::flat_set<String> common_names_) { ssl_certificate_common_names = std::move(common_names_); }
+    void setSSLCertificateCommonNames(boost::container::flat_set<String> common_names_);
 
     friend bool operator ==(const AuthenticationData & lhs, const AuthenticationData & rhs);
     friend bool operator !=(const AuthenticationData & lhs, const AuthenticationData & rhs) { return !(lhs == rhs); }

src/Access/Credentials.cpp

@@ -48,18 +48,18 @@ void AlwaysAllowCredentials::setUserName(const String & user_name_)
     user_name = user_name_;
 }
 
-CertificateCredentials::CertificateCredentials(const String & user_name_, const String & x509CommonName_)
+SSLCertificateCredentials::SSLCertificateCredentials(const String & user_name_, const String & common_name_)
     : Credentials(user_name_)
-    , x509CommonName(x509CommonName_)
+    , common_name(common_name_)
 {
     is_ready = true;
 }
 
-const String & CertificateCredentials::getX509CommonName() const
+const String & SSLCertificateCredentials::getCommonName() const
 {
     if (!isReady())
         throwNotReady();
-    return x509CommonName;
+    return common_name;
 }
 
 BasicCredentials::BasicCredentials()

src/Access/Credentials.h

@@ -38,16 +38,15 @@ public:
     void setUserName(const String & user_name_);
 };
 
-class CertificateCredentials
+class SSLCertificateCredentials
     : public Credentials
 {
 public:
-    explicit CertificateCredentials(const String & user_name_, const String & x509CommonName_);
-
-    const String & getX509CommonName() const;
+    explicit SSLCertificateCredentials(const String & user_name_, const String & common_name_);
+    const String & getCommonName() const;
 
 private:
-    String x509CommonName;
+    String common_name;
 };
 
 class BasicCredentials

src/Interpreters/Session.cpp

@@ -1,7 +1,6 @@
 #include <Interpreters/Session.h>
 
 #include <Access/AccessControl.h>
-#include <Access/Authentication.h>
 #include <Access/Credentials.h>
 #include <Access/ContextAccess.h>
 #include <Access/User.h>
@@ -298,30 +297,6 @@ void Session::authenticate(const String & user_name, const String & password, co
     authenticate(BasicCredentials{user_name, password}, address);
 }
 
-void Session::authenticate(const String & user_name, const String & password, const Poco::Net::StreamSocket & socket)
-{
-    switch (getAuthenticationType(user_name))
-    {
-        case AuthenticationType::NO_PASSWORD:
-        case AuthenticationType::PLAINTEXT_PASSWORD:
-        case AuthenticationType::DOUBLE_SHA1_PASSWORD:
-        case AuthenticationType::SHA256_PASSWORD:
-        case AuthenticationType::LDAP:
-            authenticate(BasicCredentials{user_name, password}, socket.peerAddress());
-            break;
-
-        case AuthenticationType::KERBEROS:
-            throw Authentication::Require<BasicCredentials>("ClickHouse Basic Authentication");
-
-        case AuthenticationType::SSL_CERTIFICATE:
-            authenticate(CertificateCredentials{user_name, getPeerCertificateCommonName(socket.impl())}, socket.peerAddress());
-            break;
-
-        case AuthenticationType::MAX:
-            break;
-    }
-}
-
 void Session::authenticate(const Credentials & credentials_, const Poco::Net::SocketAddress & address_)
 {
     if (session_context)

src/Interpreters/Session.h

@@ -4,7 +4,6 @@
 #include <Access/Common/AuthenticationData.h>
 #include <Interpreters/ClientInfo.h>
 #include <Interpreters/Context_fwd.h>
-#include <Poco/Net/StreamSocket.h>
 
 #include <chrono>
 #include <memory>
@@ -50,7 +49,6 @@ public:
     /// Sets the current user, checks the credentials and that the specified address is allowed to connect from.
     /// The function throws an exception if there is no such user or password is wrong.
     void authenticate(const String & user_name, const String & password, const Poco::Net::SocketAddress & address);
-    void authenticate(const String & user_name, const String & password, const Poco::Net::StreamSocket & socket);
     void authenticate(const Credentials & credentials_, const Poco::Net::SocketAddress & address_);
 
     /// Returns a reference to session ClientInfo.

src/Server/HTTP/HTTPServerRequest.cpp

@@ -9,11 +9,15 @@
 #include <Server/HTTP/HTTPServerResponse.h>
 #include <Server/HTTP/ReadHeaders.h>
 
-#include <Poco/Crypto/X509Certificate.h>
 #include <Poco/Net/HTTPHeaderStream.h>
 #include <Poco/Net/HTTPStream.h>
 #include <Poco/Net/NetException.h>
+
+#if USE_SSL
 #include <Poco/Net/SecureStreamSocketImpl.h>
+#include <Poco/Net/SSLException.h>
+#include <Poco/Net/X509Certificate.h>
+#endif
 
 namespace DB
 {
@@ -71,11 +75,31 @@ bool HTTPServerRequest::checkPeerConnected() const
     return true;
 }
 
-Poco::Net::SocketImpl * HTTPServerRequest::getSocket() const
+#if USE_SSL
+bool HTTPServerRequest::havePeerCertificate() const
 {
-    return socket;
+    if (!secure)
+        return false;
+
+    const Poco::Net::SecureStreamSocketImpl * secure_socket = dynamic_cast<const Poco::Net::SecureStreamSocketImpl *>(socket);
+    if (!secure_socket)
+        return false;
+
+    return secure_socket->havePeerCertificate();
 }
 
+Poco::Net::X509Certificate HTTPServerRequest::peerCertificate() const
+{
+    if (secure)
+    {
+        const Poco::Net::SecureStreamSocketImpl * secure_socket = dynamic_cast<const Poco::Net::SecureStreamSocketImpl *>(socket);
+        if (secure_socket)
+            return secure_socket->peerCertificate();
+    }
+    throw Poco::Net::SSLException("No certificate available");
+}
+#endif
+
 void HTTPServerRequest::readRequest(ReadBuffer & in)
 {
     char ch;

src/Server/HTTP/HTTPServerRequest.h

@@ -3,9 +3,12 @@
 #include <Interpreters/Context_fwd.h>
 #include <IO/ReadBuffer.h>
 #include <Server/HTTP/HTTPRequest.h>
+#include <Common/config.h>
 
 #include <Poco/Net/HTTPServerSession.h>
 
+namespace Poco::Net { class X509Certificate; }
+
 namespace DB
 {
 
@@ -38,7 +41,10 @@ public:
     /// Returns the server's address.
     const Poco::Net::SocketAddress & serverAddress() const { return server_address; }
 
-    Poco::Net::SocketImpl * getSocket() const;
+#if USE_SSL
+    bool havePeerCertificate() const;
+    Poco::Net::X509Certificate peerCertificate() const;
+#endif
 
 private:
     /// Limits for basic sanity checks when reading a header

src/Server/HTTPHandler.cpp

@@ -50,6 +50,10 @@
 #include <iomanip>
 #include <sstream>
 
+#if USE_SSL
+#include <Poco/Net/X509Certificate.h>
+#endif
+
 
 namespace DB
 {
@@ -106,6 +110,7 @@ namespace ErrorCodes
     extern const int BAD_REQUEST_PARAMETER;
     extern const int INVALID_SESSION_TIMEOUT;
     extern const int HTTP_LENGTH_REQUIRED;
+    extern const int SUPPORT_IS_DISABLED;
 }
 
 namespace
@@ -322,81 +327,90 @@ bool HTTPHandler::authenticateUser(
     std::string user = request.get("X-ClickHouse-User", "");
     std::string password = request.get("X-ClickHouse-Key", "");
     std::string quota_key = request.get("X-ClickHouse-Quota", "");
-    std::string x509_auth = request.get("X-ClickHouse-X509Authentication", "");
+
+    /// The header 'X-ClickHouse-SSL-Certificate-Auth: on' enables checking the common name
+    /// extracted from the SSL certificate used for this connection instead of checking password.
+    bool has_ssl_certificate_auth = (request.get("X-ClickHouse-X509Authentication", "") == "yes");
+    bool has_auth_headers = !user.empty() || !password.empty() || !quota_key.empty() || has_ssl_certificate_auth;
+
+    /// User name and password can be passed using HTTP Basic auth or query parameters
+    /// (both methods are insecure).
+    bool has_http_credentials = request.hasCredentials();
+    bool has_credentials_in_query_params = params.has("user") || params.has("password") || params.has("quota_key");
 
     std::string spnego_challenge;
     std::string certificate_common_name;
 
-    LOG_DEBUG(log, "X-ClickHouse-X509Authentication=\"{}\"", x509_auth);
-
-    bool has_basic_auth = request.hasCredentials() || params.has("user") || params.has("password") || params.has("quota_key");
-    bool has_header_auth = !user.empty() && !password.empty() && !quota_key.empty();
-    bool has_x509_auth = !user.empty() && (x509_auth == "yes"); // header values are case sensitive
-
-    if (has_x509_auth)
-    {
-        if (has_header_auth || has_basic_auth)
-            throw Exception("Invalid authentication: it is not allowed to use SSL X.509 certificate authentication and other authentication methods simultaneously", ErrorCodes::AUTHENTICATION_FAILED);
-
-        certificate_common_name = getPeerCertificateCommonName(request.getSocket());
-        if (certificate_common_name.empty())
-            throw Exception("Invalid authentication: empty X.509 certificate Common Name", ErrorCodes::AUTHENTICATION_FAILED);
-
-        LOG_DEBUG(log, "certificate_common_name=\"{}\"", certificate_common_name);
-    }
-    else if (has_header_auth)
+    if (has_auth_headers)
     {
         /// It is prohibited to mix different authorization schemes.
-        if (has_basic_auth)
-            throw Exception("Invalid authentication: it is not allowed to use X-ClickHouse HTTP headers and other authentication methods simultaneously", ErrorCodes::AUTHENTICATION_FAILED);
-    }
-    else
-    {
-        /// User name and password can be passed using query parameters
-        /// or using HTTP Basic auth (both methods are insecure).
-        if (request.hasCredentials())
+        if (has_http_credentials)
+            throw Exception("Invalid authentication: it is not allowed to use SSL certificate authentication and Authorization HTTP header simultaneously", ErrorCodes::AUTHENTICATION_FAILED);
+        if (has_credentials_in_query_params)
+            throw Exception("Invalid authentication: it is not allowed to use SSL certificate authentication and authentication via parameters simultaneously simultaneously", ErrorCodes::AUTHENTICATION_FAILED);
+
+        if (has_ssl_certificate_auth)
         {
-            /// It is prohibited to mix different authorization schemes.
-            if (params.has("user") || params.has("password"))
-                throw Exception("Invalid authentication: it is not allowed to use Authorization HTTP header and authentication via parameters simultaneously", ErrorCodes::AUTHENTICATION_FAILED);
+#if USE_SSL
+            if (!password.empty())
+                throw Exception("Invalid authentication: it is not allowed to use SSL certificate authentication and authentication via password simultaneously", ErrorCodes::AUTHENTICATION_FAILED);
 
-            std::string scheme;
-            std::string auth_info;
-            request.getCredentials(scheme, auth_info);
+            if (request.havePeerCertificate())
+                certificate_common_name = request.peerCertificate().commonName();
 
-            if (Poco::icompare(scheme, "Basic") == 0)
-            {
-                HTTPBasicCredentials credentials(auth_info);
-                user = credentials.getUsername();
-                password = credentials.getPassword();
-            }
-            else if (Poco::icompare(scheme, "Negotiate") == 0)
-            {
-                spnego_challenge = auth_info;
+            if (certificate_common_name.empty())
+                throw Exception("Invalid authentication: SSL certificate authentication requires nonempty certificate's Common Name", ErrorCodes::AUTHENTICATION_FAILED);
+#else
+            throw Exception(
+                "SSL certificate authentication disabled because ClickHouse was built without SSL library",
+                ErrorCodes::SUPPORT_IS_DISABLED);
+#endif
+        }
+    }
+    else if (has_http_credentials)
+    {
+        /// It is prohibited to mix different authorization schemes.
+        if (has_credentials_in_query_params)
+            throw Exception("Invalid authentication: it is not allowed to use Authorization HTTP header and authentication via parameters simultaneously", ErrorCodes::AUTHENTICATION_FAILED);
 
-                if (spnego_challenge.empty())
-                    throw Exception("Invalid authentication: SPNEGO challenge is empty", ErrorCodes::AUTHENTICATION_FAILED);
-            }
-            else
-            {
-                throw Exception("Invalid authentication: '" + scheme + "' HTTP Authorization scheme is not supported", ErrorCodes::AUTHENTICATION_FAILED);
-            }
+        std::string scheme;
+        std::string auth_info;
+        request.getCredentials(scheme, auth_info);
+
+        if (Poco::icompare(scheme, "Basic") == 0)
+        {
+            HTTPBasicCredentials credentials(auth_info);
+            user = credentials.getUsername();
+            password = credentials.getPassword();
+        }
+        else if (Poco::icompare(scheme, "Negotiate") == 0)
+        {
+            spnego_challenge = auth_info;
+
+            if (spnego_challenge.empty())
+                throw Exception("Invalid authentication: SPNEGO challenge is empty", ErrorCodes::AUTHENTICATION_FAILED);
         }
         else
         {
-            user = params.get("user", "default");
-            password = params.get("password", "");
+            throw Exception("Invalid authentication: '" + scheme + "' HTTP Authorization scheme is not supported", ErrorCodes::AUTHENTICATION_FAILED);
         }
 
         quota_key = params.get("quota_key", "");
     }
+    else
+    {
+        /// If the user name is not set we assume it's the 'default' user.
+        user = params.get("user", "default");
+        password = params.get("password", "");
+        quota_key = params.get("quota_key", "");
+    }
 
     if (!certificate_common_name.empty())
     {
         if (!request_credentials)
-            request_credentials = std::make_unique<CertificateCredentials>(user, certificate_common_name);
+            request_credentials = std::make_unique<SSLCertificateCredentials>(user, certificate_common_name);
 
-        auto * certificate_credentials = dynamic_cast<CertificateCredentials *>(request_credentials.get());
+        auto * certificate_credentials = dynamic_cast<SSLCertificateCredentials *>(request_credentials.get());
         if (!certificate_credentials)
             throw Exception("Invalid authentication: expected SSL certificate authorization scheme", ErrorCodes::AUTHENTICATION_FAILED);
     }

src/Server/TCPHandler.cpp

@@ -1177,7 +1177,7 @@ void TCPHandler::receiveHello()
         return;
     }
 
-    session->authenticate(user, password, socket());
+    session->authenticate(user, password, socket().peerAddress());
 }