You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by GitBox <gi...@apache.org> on 2018/07/13 03:45:14 UTC
[GitHub] sohami closed pull request #1366: [DRILL-6581] C++ Client SSL
Implementation Fixes/Improvements
sohami closed pull request #1366: [DRILL-6581] C++ Client SSL Implementation Fixes/Improvements
URL: https://github.com/apache/drill/pull/1366
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/contrib/native/client/example/querySubmitter.cpp b/contrib/native/client/example/querySubmitter.cpp
index 519dd93d5eb..a84d1dbe4d7 100644
--- a/contrib/native/client/example/querySubmitter.cpp
+++ b/contrib/native/client/example/querySubmitter.cpp
@@ -54,7 +54,8 @@ struct Option{
{"certFilePath", "Path to SSL certificate file", false},
{"disableHostnameVerification", "disable host name verification", false},
{"disableCertVerification", "disable certificate verification", false},
- {"useSystemTrustStore", "[Windows only]. Use the system truststore.", false }
+ {"useSystemTrustStore", "[Windows only]. Use the system truststore.", false },
+ {"CustomSSLCtxOptions", "The custom SSL CTX Options", false}
};
@@ -315,6 +316,7 @@ int main(int argc, char* argv[]) {
std::string disableHostnameVerification=qsOptionValues["disableHostnameVerification"];
std::string disableCertVerification=qsOptionValues["disableCertVerification"];
std::string useSystemTrustStore = qsOptionValues["useSystemTrustStore"];
+ std::string customSSLOptions = qsOptionValues["CustomSSLCtxOptions"];
Drill::QueryType type;
@@ -416,6 +418,9 @@ int main(int argc, char* argv[]) {
if (useSystemTrustStore.length() > 0){
props.setProperty(USERPROP_USESYSTEMTRUSTSTORE, useSystemTrustStore);
}
+ if (customSSLOptions.length() > 0){
+ props.setProperty(USERPROP_CUSTOM_SSLCTXOPTIONS, customSSLOptions);
+ }
}
if(client.connect(connectStr.c_str(), &props)!=Drill::CONN_SUCCESS){
diff --git a/contrib/native/client/src/clientlib/channel.cpp b/contrib/native/client/src/clientlib/channel.cpp
index 535fad77ce7..bdc19f7ad33 100644
--- a/contrib/native/client/src/clientlib/channel.cpp
+++ b/contrib/native/client/src/clientlib/channel.cpp
@@ -210,7 +210,19 @@ ChannelContext* ChannelFactory::getChannelContext(channelType_t t, DrillUserProp
verifyMode = boost::asio::ssl::context::verify_none;
}
- pChannelContext = new SSLChannelContext(props, tlsVersion, verifyMode);
+ long customSSLCtxOptions = 0;
+ std::string sslOptions;
+ props->getProp(USERPROP_CUSTOM_SSLCTXOPTIONS, sslOptions);
+ if (!sslOptions.empty()){
+ try{
+ customSSLCtxOptions = boost::lexical_cast<long>(sslOptions);
+ }
+ catch (...){
+ DRILL_LOG(LOG_ERROR) << "Unable to parse custom SSL CTX options." << std::endl;
+ }
+ }
+
+ pChannelContext = new SSLChannelContext(props, tlsVersion, verifyMode, customSSLCtxOptions);
}
break;
#endif
@@ -352,30 +364,32 @@ 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();
((SSLChannelContext_t *) m_pContext)->getSslContext().set_verify_callback(
- boost::asio::ssl::rfc2818_verification(hostPortStr.c_str()));
+ DrillSSLHostnameVerifier(this));
}
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 73273aa7b34..fec4659ccb5 100644
--- a/contrib/native/client/src/clientlib/channel.hpp
+++ b/contrib/native/client/src/clientlib/channel.hpp
@@ -21,6 +21,12 @@
#include "drill/common.hpp"
#include "drill/drillClient.hpp"
#include "streamSocket.hpp"
+#include "errmsgs.hpp"
+
+#if defined(IS_SSL_ENABLED)
+#include <openssl/ssl.h>
+#include <openssl/err.h>
+#endif
namespace Drill {
@@ -34,7 +40,7 @@ class UserProperties;
//parse the connection string and set up the host and port to connect to
connectionStatus_t getDrillbitEndpoint();
-
+
const std::string& getProtocol() const {return m_protocol;}
const std::string& getHost() const {return m_host;}
const std::string& getPort() const {return m_port;}
@@ -83,21 +89,41 @@ class UserProperties;
SSLChannelContext(DrillUserProperties *props,
boost::asio::ssl::context::method tlsVersion,
- boost::asio::ssl::verify_mode verifyMode) :
- ChannelContext(props),
- m_SSLContext(tlsVersion) {
+ boost::asio::ssl::verify_mode verifyMode,
+ const long customSSLCtxOptions = 0) :
+ 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
+ | customSSLCtxOptions
);
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;
@@ -147,9 +173,20 @@ class UserProperties;
ConnectionEndpoint* getEndpoint(){return m_pEndpoint;}
+ ChannelContext_t* getChannelContext(){ return m_pContext; }
+
protected:
connectionStatus_t handleError(connectionStatus_t status, std::string msg);
+ /// @brief Handle protocol handshake exceptions.
+ ///
+ /// @param in_err The error.
+ ///
+ /// @return the connectionStatus.
+ virtual connectionStatus_t HandleProtocolHandshakeException(const boost::system::system_error& in_err){
+ return handleError(CONN_HANDSHAKE_FAILED, in_err.what());
+ }
+
boost::asio::io_service& m_ioService;
boost::asio::io_service m_ioServiceFallback; // used if m_ioService is not provided
AsioStreamSocket* m_pSocket;
@@ -170,7 +207,7 @@ class UserProperties;
try{
m_pSocket->protocolHandshake(useSystemConfig);
} catch (boost::system::system_error e) {
- status = handleError(CONN_HANDSHAKE_FAILED, e.what());
+ status = HandleProtocolHandshakeException(e);
}
return status;
}
@@ -199,6 +236,33 @@ class UserProperties;
:Channel(ioService, host, port){
}
connectionStatus_t init();
+ protected:
+#if defined(IS_SSL_ENABLED)
+ /// @brief Handle protocol handshake exceptions for SSL specific failures.
+ ///
+ /// @param in_err The error.
+ ///
+ /// @return the connectionStatus.
+ connectionStatus_t HandleProtocolHandshakeException(const boost::system::system_error& in_err) {
+ const boost::system::error_code& errcode = in_err.code();
+ if (!(((SSLChannelContext_t *)m_pContext)->GetCertificateHostnameVerificationStatus())){
+ return handleError(
+ CONN_HANDSHAKE_FAILED,
+ getMessage(ERR_CONN_SSL_CN, in_err.what()));
+ }
+ else if (boost::asio::error::get_ssl_category() == errcode.category() &&
+ SSL_R_CERTIFICATE_VERIFY_FAILED == ERR_GET_REASON(errcode.value())){
+ return handleError(
+ CONN_HANDSHAKE_FAILED,
+ getMessage(ERR_CONN_SSL_CERTVERIFY, in_err.what()));
+ }
+ else{
+ return handleError(
+ CONN_HANDSHAKE_FAILED,
+ getMessage(ERR_CONN_SSL_GENERAL, in_err.what()));
+ }
+ }
+#endif
};
class ChannelFactory{
@@ -215,6 +279,52 @@ class UserProperties;
static ChannelContext_t* getChannelContext(channelType_t t, DrillUserProperties* props);
};
+ /// @brief Hostname verification callback wrapper.
+ class DrillSSLHostnameVerifier{
+ public:
+ /// @brief The constructor.
+ ///
+ /// @param in_channel The Channel.
+ DrillSSLHostnameVerifier(Channel* in_channel) : m_channel(in_channel){
+ 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;
+
+ // Gets the channel context.
+ SSLChannelContext_t* context = (SSLChannelContext_t*)(m_channel->getChannelContext());
+
+ // Retrieve the host before we perform Host name verification.
+ // This is because host with ZK mode is selected after the connect() function is called.
+ boost::asio::ssl::rfc2818_verification verifier(m_channel->getEndpoint()->getHost().c_str());
+
+ // Perform verification.
+ bool verified = verifier(in_preverified, in_ctx);
+
+ DRILL_LOG(LOG_DEBUG)
+ << "DrillSSLHostnameVerifier::operator(): Verification Result: "
+ << verified
+ << std::endl;
+
+ // Sets the result back to the context.
+ context->SetCertHostnameVerificationStatus(verified);
+ return verified;
+ }
+
+ private:
+
+ // The SSL channel.
+ Channel* m_channel;
+ };
} // namespace Drill
diff --git a/contrib/native/client/src/clientlib/errmsgs.cpp b/contrib/native/client/src/clientlib/errmsgs.cpp
index c1ac80d0649..82f24fd202e 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. [Details: %s]" },
+ {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 fac646b815f..7bcb80579d8 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/clientlib/userProperties.cpp b/contrib/native/client/src/clientlib/userProperties.cpp
index f1aa82fa3ca..0ad8af1dd73 100644
--- a/contrib/native/client/src/clientlib/userProperties.cpp
+++ b/contrib/native/client/src/clientlib/userProperties.cpp
@@ -35,6 +35,7 @@ const std::map<std::string, uint32_t> DrillUserProperties::USER_PROPERTIES=boos
( USERPROP_DISABLE_HOSTVERIFICATION, USERPROP_FLAGS_BOOLEAN|USERPROP_FLAGS_SSLPROP)
( USERPROP_DISABLE_CERTVERIFICATION, USERPROP_FLAGS_BOOLEAN|USERPROP_FLAGS_SSLPROP)
( USERPROP_USESYSTEMTRUSTSTORE, USERPROP_FLAGS_BOOLEAN|USERPROP_FLAGS_SSLPROP)
+ ( USERPROP_CUSTOM_SSLCTXOPTIONS, USERPROP_FLAGS_STRING|USERPROP_FLAGS_SSLPROP)
( USERPROP_SASL_ENCRYPT, USERPROP_FLAGS_STRING)
;
diff --git a/contrib/native/client/src/include/drill/common.hpp b/contrib/native/client/src/include/drill/common.hpp
index 18cfc69fff5..b5bb522bee0 100644
--- a/contrib/native/client/src/include/drill/common.hpp
+++ b/contrib/native/client/src/include/drill/common.hpp
@@ -173,7 +173,8 @@ typedef enum{
#define USERPROP_PASSWORD "password"
#define USERPROP_SCHEMA "schema"
#define USERPROP_USESSL "enableTLS"
-#define USERPROP_TLSPROTOCOL "TLSProtocol" //TLS version
+#define USERPROP_TLSPROTOCOL "TLSProtocol" //TLS version. The exact TLS version.
+#define USERPROP_CUSTOM_SSLCTXOPTIONS "CustomSSLCtxOptions" // The custom SSL CTX options.
#define USERPROP_CERTFILEPATH "certFilePath" // pem file path and name
// TODO: support truststore protected by password.
// #define USERPROP_CERTPASSWORD "certPassword" // Password for certificate file.
diff --git a/contrib/native/client/src/include/drill/userProperties.hpp b/contrib/native/client/src/include/drill/userProperties.hpp
index f5d6783ea33..fb6c764e723 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.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services