[GitHub] drill pull request #997: DRILL-5882: C++ Client: [Threat Modeling] Drillbit ...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/drill/pull/997


---

[GitHub] drill pull request #997: DRILL-5882: C++ Client: [Threat Modeling] Drillbit ...

[bitblender]

Github user bitblender commented on a diff in the pull request:

https://github.com/apache/drill/pull/997#discussion_r145568130
  
--- Diff: contrib/native/client/src/clientlib/drillClientImpl.cpp ---
@@ -595,6 +611,12 @@ 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
--- End diff --

Yes. The control flow goes through the AUTH_REQUIRED case when the server 
requires auth.


---

[GitHub] drill pull request #997: DRILL-5882: C++ Client: [Threat Modeling] Drillbit ...

[bitblender]

Github user bitblender commented on a diff in the pull request:

https://github.com/apache/drill/pull/997#discussion_r145567205
  
--- Diff: contrib/native/client/src/clientlib/drillClientImpl.cpp ---
@@ -595,6 +611,12 @@ 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)) {
--- End diff --

- Externalized the messages to errmsgs.cpp 
- Changed the error message to "Client needs a secure connection but server 
does not support... " to account for the case where auth and/or enc is required 
by the client but missing on the server


---

[GitHub] drill pull request #997: DRILL-5882: C++ Client: [Threat Modeling] Drillbit ...

[parthchandra]

Github user parthchandra commented on a diff in the pull request:

https://github.com/apache/drill/pull/997#discussion_r145510774
  
--- Diff: contrib/native/client/src/clientlib/drillClientImpl.cpp ---
@@ -595,6 +611,12 @@ 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)) {
--- End diff --

Also, do you need two different messages : auth required and encr required ?


---