DRILL-5943: Avoid the strong check introduced by DRILL-5582 for PLAIN mechanism

This closes #1028

3 files changed, 28 insertions, 16 deletions

diff --git a/contrib/native/client/src/clientlib/drillClientImpl.cpp b/contrib/native/client/src/clientlib/drillClientImpl.cpp
index 343556185..60ae7a409 100644
--- a/contrib/native/client/src/clientlib/drillClientImpl.cpp
+++ b/contrib/native/client/src/clientlib/drillClientImpl.cpp
@@ -520,17 +520,23 @@ bool DrillClientImpl::clientNeedsEncryption(const DrillUserProperties* userPrope
 
 /*
  * 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.
+ * If the USERPROP_AUTH_MECHANISM connection string properties is non-empty and not equal to PLAIN,
+ * then it is implied that the client wants authentication.
+ *
+ * Explicitly skipping PLAIN to maintain forward compatibility with 1.9 Drillbit and it doesn't matter
+ * if this security check is not there for PLAIN mechanism
  */
 bool DrillClientImpl::clientNeedsAuthentication(const DrillUserProperties* userProperties) {
     bool needsAuthentication = false;
-    if(!userProperties) { return false; }
-    std::string passwd = "";
-    userProperties->getProp(USERPROP_PASSWORD, passwd);
+    if(!userProperties) { return needsAuthentication; }
     std::string authMech = "";
     userProperties->getProp(USERPROP_AUTH_MECHANISM, authMech);
-    return !passwd.empty() || !authMech.empty();
+    boost::algorithm::to_lower(authMech);
+
+    if(!authMech.empty() && SaslAuthenticatorImpl::PLAIN_NAME != authMech) {
+        needsAuthentication = true;
+    }
+    return needsAuthentication;
 }
 
 connectionStatus_t DrillClientImpl::validateHandshake(DrillUserProperties* properties){
diff --git a/contrib/native/client/src/clientlib/saslAuthenticatorImpl.cpp b/contrib/native/client/src/clientlib/saslAuthenticatorImpl.cpp
index c03cb6c0b..1f32b5e77 100644
--- a/contrib/native/client/src/clientlib/saslAuthenticatorImpl.cpp
+++ b/contrib/native/client/src/clientlib/saslAuthenticatorImpl.cpp
@@ -28,16 +28,16 @@
 
 namespace Drill {
 
-static const std::string DEFAULT_SERVICE_NAME = "drill";
+const std::string DEFAULT_SERVICE_NAME = "drill";
+const int PREFERRED_MIN_SSF = 56;
+const std::string SaslAuthenticatorImpl::KERBEROS_SIMPLE_NAME = "kerberos";
+const std::string SaslAuthenticatorImpl::PLAIN_NAME = "plain";
 
-static const std::string KERBEROS_SIMPLE_NAME = "kerberos";
-static const std::string KERBEROS_SASL_NAME = "gssapi";
-static const std::string PLAIN_NAME = "plain";
-static const int PREFERRED_MIN_SSF = 56;
+const std::string KERBEROS_SASL_NAME = "gssapi";
 
 const std::map<std::string, std::string> SaslAuthenticatorImpl::MECHANISM_MAPPING = boost::assign::map_list_of
-    (KERBEROS_SIMPLE_NAME, KERBEROS_SASL_NAME)
-    (PLAIN_NAME, PLAIN_NAME)
+    (SaslAuthenticatorImpl::KERBEROS_SIMPLE_NAME, KERBEROS_SASL_NAME)
+    (SaslAuthenticatorImpl::PLAIN_NAME, SaslAuthenticatorImpl::PLAIN_NAME)
 ;
 
 boost::mutex SaslAuthenticatorImpl::s_mutex;
@@ -138,15 +138,17 @@ int SaslAuthenticatorImpl::init(const std::vector<std::string>& mechanisms, exec
             m_ppwdSecret = (sasl_secret_t *) malloc(sizeof(sasl_secret_t) + length);
             std::memcpy(m_ppwdSecret->data, value.c_str(), length);
             m_ppwdSecret->len = length;
-            authMechanismToUse = PLAIN_NAME;
+            authMechanismToUse = SaslAuthenticatorImpl::PLAIN_NAME;
         } else if (USERPROP_USERNAME == key) {
             m_username = value;
         } else if (USERPROP_AUTH_MECHANISM == key) {
             authMechanismToUse = value;
         }
     }
-    // clientNeedsAuthentication() cannot be false if the code above picks an authMechanism
-    assert (authMechanismToUse.empty() || DrillClientImpl::clientNeedsAuthentication(m_pUserProperties));
+    // clientNeedsAuthentication() cannot be false if the code above picks an authMechanism other than PLAIN
+    assert (authMechanismToUse.empty() || authMechanismToUse == SaslAuthenticatorImpl::PLAIN_NAME ||
+            DrillClientImpl::clientNeedsAuthentication(m_pUserProperties));
+
     m_authMechanismName = authMechanismToUse;
     if (authMechanismToUse.empty()) return SASL_NOMECH;
 
diff --git a/contrib/native/client/src/clientlib/saslAuthenticatorImpl.hpp b/contrib/native/client/src/clientlib/saslAuthenticatorImpl.hpp
index bf61e9dc8..a0ee5a5c9 100644
--- a/contrib/native/client/src/clientlib/saslAuthenticatorImpl.hpp
+++ b/contrib/native/client/src/clientlib/saslAuthenticatorImpl.hpp
@@ -59,6 +59,10 @@ public:
 
     const char *getErrorMessage(int errorCode);
 
+    static const std::string KERBEROS_SIMPLE_NAME;
+
+    static const std::string PLAIN_NAME;
+
 private:
 
     static const std::map<std::string, std::string> MECHANISM_MAPPING;