[DRILL-6581] C++ Client SSL Implementation Fixes/Improvements

5 files changed, 137 insertions, 27 deletions

diff --git a/contrib/native/client/src/clientlib/channel.cpp b/contrib/native/client/src/clientlib/channel.cpp
index 535fad77c..fc978168c 100644
--- a/contrib/native/client/src/clientlib/channel.cpp
+++ b/contrib/native/client/src/clientlib/channel.cpp
@@ -352,30 +352,37 @@ connectionStatus_t SSLStreamChannel::init(){
     connectionStatus_t ret=CONN_SUCCESS;
 
     const DrillUserProperties* props = m_pContext->getUserProperties();
-	std::string useSystemTrustStore;
-	props->getProp(USERPROP_USESYSTEMTRUSTSTORE, useSystemTrustStore);
-	if (useSystemTrustStore != "true"){
-		std::string certFile;
-		props->getProp(USERPROP_CERTFILEPATH, certFile);
-		try{
-			((SSLChannelContext_t*)m_pContext)->getSslContext().load_verify_file(certFile);
-		}
-		catch (boost::system::system_error e){
-			DRILL_LOG(LOG_ERROR) << "Channel initialization failure. Certificate file  "
-				<< certFile
-				<< " could not be loaded."
-				<< std::endl;
-			handleError(CONN_SSLERROR, getMessage(ERR_CONN_SSLCERTFAIL, certFile.c_str(), e.what()));
-			ret = CONN_FAILURE;
-		}
-	}
+    std::string useSystemTrustStore;
+    props->getProp(USERPROP_USESYSTEMTRUSTSTORE, useSystemTrustStore);
+    if (useSystemTrustStore != "true"){
+        std::string certFile;
+        props->getProp(USERPROP_CERTFILEPATH, certFile);
+        try{
+            ((SSLChannelContext_t*)m_pContext)->getSslContext().load_verify_file(certFile);
+        }
+        catch (boost::system::system_error e){
+            DRILL_LOG(LOG_ERROR) << "Channel initialization failure. Certificate file  "
+                << certFile
+                << " could not be loaded."
+                << std::endl;
+            handleError(CONN_SSLERROR, getMessage(ERR_CONN_SSLCERTFAIL, certFile.c_str(), e.what()));
+            ret = CONN_FAILURE;
+            // Stop here to propagate the load/verify certificate error.
+            return ret;
+        }
+    }
 
+    ((SSLChannelContext_t *)m_pContext)->SetCertHostnameVerificationStatus(true);
     std::string disableHostVerification;
     props->getProp(USERPROP_DISABLE_HOSTVERIFICATION, disableHostVerification);
     if (disableHostVerification != "true") {
-        std::string hostPortStr = m_pEndpoint->getHost() + ":" + m_pEndpoint->getPort();
+        // Populate endpoint information before we retrieve host name.
+        m_pEndpoint->parseConnectString();
+        std::string hostStr  = m_pEndpoint->getHost();
         ((SSLChannelContext_t *) m_pContext)->getSslContext().set_verify_callback(
-                boost::asio::ssl::rfc2818_verification(hostPortStr.c_str()));
+                DrillSSLHostnameVerifier(
+                    ((SSLChannelContext_t *)m_pContext), 
+                    boost::asio::ssl::rfc2818_verification(hostStr.c_str())));
     }
 
     m_pSocket=new SslSocket(m_ioService, ((SSLChannelContext_t*)m_pContext)->getSslContext() );
diff --git a/contrib/native/client/src/clientlib/channel.hpp b/contrib/native/client/src/clientlib/channel.hpp
index 73273aa7b..e73911886 100644
--- a/contrib/native/client/src/clientlib/channel.hpp
+++ b/contrib/native/client/src/clientlib/channel.hpp
@@ -21,6 +21,13 @@
 #include "drill/common.hpp"
 #include "drill/drillClient.hpp"
 #include "streamSocket.hpp"
+#include "errmsgs.hpp"
+
+namespace
+{
+// The error message to indicate certificate verification failure.
+#define DRILL_BOOST_SSL_CERT_VERIFY_FAILED  "handshake: certificate verify failed\0"
+}
 
 namespace Drill {
 
@@ -34,14 +41,13 @@ class UserProperties;
 
             //parse the connection string and set up the host and port to connect to
             connectionStatus_t getDrillbitEndpoint();
-
+            void parseConnectString();
             const std::string& getProtocol() const {return m_protocol;}
             const std::string& getHost() const {return m_host;}
             const std::string& getPort() const {return m_port;}
             DrillClientError* getError(){ return m_pError;};
 
         private:
-            void parseConnectString();
             bool isDirectConnection();
             bool isZookeeperConnection();
             connectionStatus_t getDrillbitEndpointFromZk();
@@ -84,20 +90,38 @@ class UserProperties;
         SSLChannelContext(DrillUserProperties *props,
                           boost::asio::ssl::context::method tlsVersion,
                           boost::asio::ssl::verify_mode verifyMode) :
-                ChannelContext(props),
-                m_SSLContext(tlsVersion) {
+                    ChannelContext(props),
+                    m_SSLContext(tlsVersion),
+                    m_certHostnameVerificationStatus(true) 
+            {
                 m_SSLContext.set_default_verify_paths();
                 m_SSLContext.set_options(
                         boost::asio::ssl::context::default_workarounds
                         | boost::asio::ssl::context::no_sslv2
+                        | boost::asio::ssl::context::no_sslv3
                         | boost::asio::ssl::context::single_dh_use
                         );
                 m_SSLContext.set_verify_mode(verifyMode);
             };
+
             ~SSLChannelContext(){};
             boost::asio::ssl::context& getSslContext(){ return m_SSLContext;}
+
+            /// @brief Check the certificate host name verification status.
+            /// 
+            /// @return FALSE if the verification has failed, TRUE otherwise.
+            const bool GetCertificateHostnameVerificationStatus() const { return m_certHostnameVerificationStatus; }
+
+            /// @brief Set the certificate host name verification status.
+            ///
+            /// @param in_result                The host name verification status.
+            void SetCertHostnameVerificationStatus(bool in_result) { m_certHostnameVerificationStatus = in_result; }
+
         private:
             boost::asio::ssl::context m_SSLContext;
+
+            // The flag to indicate the host name verification result.
+            bool m_certHostnameVerificationStatus;
     };
 
     typedef ChannelContext ChannelContext_t; 
@@ -150,6 +174,15 @@ class UserProperties;
         protected:
             connectionStatus_t handleError(connectionStatus_t status, std::string msg);
 
+            /// @brief Handle protocol handshake exceptions.
+            /// 
+            /// @param in_errmsg                The error message.
+            /// 
+            /// @return the connectionStatus.
+            virtual connectionStatus_t HandleProtocolHandshakeException(const char* in_errmsg){
+                return handleError(CONN_HANDSHAKE_FAILED, in_errmsg);
+            }
+
             boost::asio::io_service& m_ioService;
             boost::asio::io_service m_ioServiceFallback; // used if m_ioService is not provided
             AsioStreamSocket* m_pSocket;
@@ -170,7 +203,7 @@ class UserProperties;
                 try{
                     m_pSocket->protocolHandshake(useSystemConfig);
                 } catch (boost::system::system_error e) {
-                    status = handleError(CONN_HANDSHAKE_FAILED, e.what());
+                    status = HandleProtocolHandshakeException(e.what());
                 }
                 return status;
             }
@@ -199,6 +232,29 @@ class UserProperties;
                 :Channel(ioService, host, port){
             }
             connectionStatus_t init();
+        protected:
+            /// @brief Handle protocol handshake exceptions for SSL specific failures.
+            /// 
+            /// @param in_errmsg                The error message.
+            /// 
+            /// @return the connectionStatus.
+            connectionStatus_t HandleProtocolHandshakeException(const char* errmsg) {
+                if (!(((SSLChannelContext_t *)m_pContext)->GetCertificateHostnameVerificationStatus())){
+                    return handleError(
+                        CONN_HANDSHAKE_FAILED,
+                        getMessage(ERR_CONN_SSL_CN));
+                }
+                else if (0 == strcmp(errmsg, DRILL_BOOST_SSL_CERT_VERIFY_FAILED)){
+                    return handleError(
+                        CONN_HANDSHAKE_FAILED,
+                        getMessage(ERR_CONN_SSL_CERTVERIFY, errmsg));
+                }
+                else{
+                    return handleError(
+                        CONN_HANDSHAKE_FAILED,
+                        getMessage(ERR_CONN_SSL_GENERAL, errmsg));
+                }
+            }
     };
 
     class ChannelFactory{
@@ -215,6 +271,48 @@ class UserProperties;
             static ChannelContext_t* getChannelContext(channelType_t t, DrillUserProperties* props);
     };
 
+    /// @brief Hostname verification callback wrapper.
+    class DrillSSLHostnameVerifier{
+        public:
+            /// @brief The constructor.
+            /// 
+            /// @param in_pctx                  The SSL Channel Context.
+            /// @param in_verifier              The wrapped verifier.
+            DrillSSLHostnameVerifier(SSLChannelContext_t* in_pctx, boost::asio::ssl::rfc2818_verification in_verifier) : 
+                m_verifier(in_verifier),
+                m_pctx(in_pctx){
+                DRILL_LOG(LOG_INFO)
+                    << "DrillSSLHostnameVerifier::DrillSSLHostnameVerifier: +++++ Enter +++++" 
+                    << std::endl;
+            }
+
+            /// @brief Perform certificate verification.
+            /// 
+            /// @param in_preverified           Pre-verified indicator.
+            /// @param in_ctx                   Verify context.
+            bool operator()(
+                bool in_preverified,
+                boost::asio::ssl::verify_context& in_ctx){
+                DRILL_LOG(LOG_INFO) << "DrillSSLHostnameVerifier::operator(): +++++ Enter +++++" << std::endl;
+
+                bool verified = m_verifier(in_preverified, in_ctx);
+
+                DRILL_LOG(LOG_DEBUG) 
+                    << "DrillSSLHostnameVerifier::operator(): Verification Result: " 
+                    << verified 
+                    << std::endl;
+
+                m_pctx->SetCertHostnameVerificationStatus(verified);
+                return verified;
+            }
+
+        private:
+            // The inner verifier.
+            boost::asio::ssl::rfc2818_verification m_verifier;
+
+            // The SSL channel context.
+            SSLChannelContext_t* m_pctx;
+    };
 
 } // namespace Drill
 
diff --git a/contrib/native/client/src/clientlib/errmsgs.cpp b/contrib/native/client/src/clientlib/errmsgs.cpp
index c1ac80d06..37f0ac1b9 100644
--- a/contrib/native/client/src/clientlib/errmsgs.cpp
+++ b/contrib/native/client/src/clientlib/errmsgs.cpp
@@ -57,6 +57,9 @@ static Drill::ErrorMessages errorMessages[]={
     {ERR_CONN_NOSERVERENC, ERR_CATEGORY_CONN, 0, "Client needs encryption but encryption is disabled on the server."
         " Please check connection parameters or contact administrator. [Warn: This"
         " could be due to a bad configuration or a security attack is in progress.]"},
+    {ERR_CONN_SSL_GENERAL, ERR_CATEGORY_CONN, 0, "Encountered an exception during SSL handshake. [Details: %s]"},
+    {ERR_CONN_SSL_CN, ERR_CATEGORY_CONN, 0, "SSL certificate host name verification failure." },
+    {ERR_CONN_SSL_CERTVERIFY, ERR_CATEGORY_CONN, 0, "SSL certificate verification failed. [Details: %s]"},
     {ERR_QRY_OUTOFMEM, ERR_CATEGORY_QRY, 0, "Out of memory."},
     {ERR_QRY_COMMERR, ERR_CATEGORY_QRY, 0, "Communication error. %s"},
     {ERR_QRY_INVREADLEN, ERR_CATEGORY_QRY, 0, "Internal Error: Received a message with an invalid read length."},
diff --git a/contrib/native/client/src/clientlib/errmsgs.hpp b/contrib/native/client/src/clientlib/errmsgs.hpp
index fac646b81..7bcb80579 100644
--- a/contrib/native/client/src/clientlib/errmsgs.hpp
+++ b/contrib/native/client/src/clientlib/errmsgs.hpp
@@ -55,7 +55,10 @@ namespace Drill{
 #define ERR_CONN_NOSOCKET       DRILL_ERR_START+23
 #define ERR_CONN_NOSERVERAUTH   DRILL_ERR_START+24
 #define ERR_CONN_NOSERVERENC    DRILL_ERR_START+25
-#define ERR_CONN_MAX            DRILL_ERR_START+25
+#define ERR_CONN_SSL_GENERAL    DRILL_ERR_START+26
+#define ERR_CONN_SSL_CN         DRILL_ERR_START+27
+#define ERR_CONN_SSL_CERTVERIFY DRILL_ERR_START+28
+#define ERR_CONN_MAX            DRILL_ERR_START+28
 
 #define ERR_QRY_OUTOFMEM    ERR_CONN_MAX+1
 #define ERR_QRY_COMMERR     ERR_CONN_MAX+2
diff --git a/contrib/native/client/src/include/drill/userProperties.hpp b/contrib/native/client/src/include/drill/userProperties.hpp
index f5d6783ea..fb6c764e7 100644
--- a/contrib/native/client/src/include/drill/userProperties.hpp
+++ b/contrib/native/client/src/include/drill/userProperties.hpp
@@ -29,8 +29,7 @@ class DECLSPEC_DRILL_CLIENT DrillUserProperties{
 
         DrillUserProperties(){};
         
-        /// @brief Update the property value associate with the property key if the value is 
-        /// empty.
+        /// @brief Sets the default property value.
         /// 
         /// @param in_propName              The property name.
         /// @param in_propValue             The property value.