drill-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From par...@apache.org
Subject [2/2] drill git commit: DRILL-5582: C++ Client: [Threat Modeling] Drillbit may be spoofed by an attacker and this may lead to data being written to the attacker's target instead of Drillbit
Date Fri, 20 Oct 2017 00:19:36 GMT
DRILL-5582: C++ Client: [Threat Modeling] Drillbit may be spoofed by an attacker and this may
lead to data being written to the attacker's target instead of Drillbit

This closes #997


Project: http://git-wip-us.apache.org/repos/asf/drill/repo
Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/f1d1945b
Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/f1d1945b
Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/f1d1945b

Branch: refs/heads/master
Commit: f1d1945b3772bb782039fd6811e34a7de66441c8
Parents: 40d0991
Author: karthik <kmanivannan@maprtech.com>
Authored: Tue Oct 17 16:18:45 2017 -0700
Committer: Parth Chandra <parthc@apache.org>
Committed: Thu Oct 19 17:13:05 2017 -0700

----------------------------------------------------------------------
 .../client/src/clientlib/drillClientImpl.cpp    | 22 ++++++++++++++++++--
 .../client/src/clientlib/drillClientImpl.hpp    |  3 +++
 contrib/native/client/src/clientlib/errmsgs.cpp |  6 ++++++
 contrib/native/client/src/clientlib/errmsgs.hpp |  4 +++-
 .../src/clientlib/saslAuthenticatorImpl.cpp     |  2 ++
 5 files changed, 34 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/drill/blob/f1d1945b/contrib/native/client/src/clientlib/drillClientImpl.cpp
----------------------------------------------------------------------
diff --git a/contrib/native/client/src/clientlib/drillClientImpl.cpp b/contrib/native/client/src/clientlib/drillClientImpl.cpp
index f0bb636..9fdd725 100644
--- a/contrib/native/client/src/clientlib/drillClientImpl.cpp
+++ b/contrib/native/client/src/clientlib/drillClientImpl.cpp
@@ -518,6 +518,21 @@ bool DrillClientImpl::clientNeedsEncryption(const DrillUserProperties*
userPrope
     return needsEncryption;
 }
 
+/*
+ * Checks if the client has explicitly expressed interest in authenticated connections only.
+ * If the USERPROP_PASSWORD or USERPROP_AUTH_MECHANISM connection string properties are
+ * non-empty, then it is implied that the client wants authentication.
+ */
+bool DrillClientImpl::clientNeedsAuthentication(const DrillUserProperties* userProperties)
{
+    bool needsAuthentication = false;
+    if(!userProperties) { return false; }
+    std::string passwd = "";
+    userProperties->getProp(USERPROP_PASSWORD, passwd);
+    std::string authMech = "";
+    userProperties->getProp(USERPROP_AUTH_MECHANISM, authMech);
+    return !passwd.empty() || !authMech.empty();
+}
+
 connectionStatus_t DrillClientImpl::validateHandshake(DrillUserProperties* properties){
 
     DRILL_MT_LOG(DRILL_LOG(LOG_TRACE) << "validateHandShake\n";)
@@ -595,6 +610,10 @@ connectionStatus_t DrillClientImpl::validateHandshake(DrillUserProperties*
prope
 
     switch(this->m_handshakeStatus) {
         case exec::user::SUCCESS:
+            // Check if client needs auth/encryption and server is not requiring it
+            if(clientNeedsAuthentication(properties) || clientNeedsEncryption(properties))
{
+                return handleConnError(CONN_AUTH_FAILED, getMessage(ERR_CONN_NOSERVERAUTH));
+            }
             // reset io_service after handshake is validated before running queries
             m_io_service.reset();
             return CONN_SUCCESS;
@@ -630,8 +649,7 @@ connectionStatus_t DrillClientImpl::handleAuthentication(const DrillUserProperti
 
     // Check if client needs encryption and server is configured for encryption or not before
starting handshake
     if(clientNeedsEncryption(userProperties) && !m_encryptionCtxt.isEncryptionReqd())
{
-        return handleConnError(CONN_AUTH_FAILED, "Client needs encryption but on server side
encryption is disabled."
-                                                 " Please check connection parameters or
contact administrator?");
+        return handleConnError(CONN_AUTH_FAILED, getMessage(ERR_CONN_NOSERVERENC));
     }
 
     try {

http://git-wip-us.apache.org/repos/asf/drill/blob/f1d1945b/contrib/native/client/src/clientlib/drillClientImpl.hpp
----------------------------------------------------------------------
diff --git a/contrib/native/client/src/clientlib/drillClientImpl.hpp b/contrib/native/client/src/clientlib/drillClientImpl.hpp
index dc4a67e..2e7623e 100644
--- a/contrib/native/client/src/clientlib/drillClientImpl.hpp
+++ b/contrib/native/client/src/clientlib/drillClientImpl.hpp
@@ -474,6 +474,9 @@ class DrillClientImpl : public DrillClientImplBase{
 
         void freeMetadata(meta::DrillMetadata* metadata);
 
+        static bool clientNeedsAuthentication(const DrillUserProperties* userProperties);
+
+
     private:
         friend class meta::DrillMetadata;
         friend class DrillClientQueryHandle;

http://git-wip-us.apache.org/repos/asf/drill/blob/f1d1945b/contrib/native/client/src/clientlib/errmsgs.cpp
----------------------------------------------------------------------
diff --git a/contrib/native/client/src/clientlib/errmsgs.cpp b/contrib/native/client/src/clientlib/errmsgs.cpp
index d2d8c18..a5d4a44 100644
--- a/contrib/native/client/src/clientlib/errmsgs.cpp
+++ b/contrib/native/client/src/clientlib/errmsgs.cpp
@@ -52,6 +52,12 @@ static Drill::ErrorMessages errorMessages[]={
     {ERR_CONN_NOCONNSTR, ERR_CATEGORY_CONN, 0, "Cannot connect if either host name or port
number are empty."},
     {ERR_CONN_SSLCERTFAIL, ERR_CATEGORY_CONN, 0, "SSL certificate file %s could not be loaded
(exception message: %s)."},
     {ERR_CONN_NOSOCKET, ERR_CATEGORY_CONN, 0, "Failed to open socket connection."},
+    {ERR_CONN_NOSERVERAUTH, ERR_CATEGORY_CONN, 0, "Client needs a secure connection but server
does not"
+        " support any security mechanisms. Please contact an administrator. [Warn: This"
+        " could be due to a bad configuration or a security attack is in progress.]"},
+    {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_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."},

http://git-wip-us.apache.org/repos/asf/drill/blob/f1d1945b/contrib/native/client/src/clientlib/errmsgs.hpp
----------------------------------------------------------------------
diff --git a/contrib/native/client/src/clientlib/errmsgs.hpp b/contrib/native/client/src/clientlib/errmsgs.hpp
index 246e4bb..c4a0080 100644
--- a/contrib/native/client/src/clientlib/errmsgs.hpp
+++ b/contrib/native/client/src/clientlib/errmsgs.hpp
@@ -54,7 +54,9 @@ namespace Drill{
 #define ERR_CONN_NOCONNSTR      DRILL_ERR_START+21
 #define ERR_CONN_SSLCERTFAIL    DRILL_ERR_START+22
 #define ERR_CONN_NOSOCKET       DRILL_ERR_START+23
-#define ERR_CONN_MAX            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_QRY_OUTOFMEM    ERR_CONN_MAX+1
 #define ERR_QRY_COMMERR     ERR_CONN_MAX+2

http://git-wip-us.apache.org/repos/asf/drill/blob/f1d1945b/contrib/native/client/src/clientlib/saslAuthenticatorImpl.cpp
----------------------------------------------------------------------
diff --git a/contrib/native/client/src/clientlib/saslAuthenticatorImpl.cpp b/contrib/native/client/src/clientlib/saslAuthenticatorImpl.cpp
index 78f99b6..9057a37 100644
--- a/contrib/native/client/src/clientlib/saslAuthenticatorImpl.cpp
+++ b/contrib/native/client/src/clientlib/saslAuthenticatorImpl.cpp
@@ -145,6 +145,8 @@ int SaslAuthenticatorImpl::init(const std::vector<std::string>&
mechanisms, exec
             authMechanismToUse = value;
         }
     }
+    // clientNeedsAuthentication() cannot be false if the code above picks an authMechanism
+    assert (authMechanismToUse.empty() || DrillClientImpl::clientNeedsAuthentication(m_pUserProperties));
     if (authMechanismToUse.empty()) return SASL_NOMECH;
 
     // check if requested mechanism is supported by server


Mime
View raw message