zookeeper-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [zookeeper] hanm commented on a change in pull request #118: ZOOKEEPER-1634: hardening security by teaching server to enforce client authentication.
Date Mon, 17 Jun 2019 00:53:41 GMT
hanm commented on a change in pull request #118: ZOOKEEPER-1634: hardening security by teaching
server to enforce client authentication.
URL: https://github.com/apache/zookeeper/pull/118#discussion_r294109710
 
 

 ##########
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java
 ##########
 @@ -1322,23 +1322,55 @@ public void processPacket(ServerCnxn cnxn, ByteBuffer incomingBuffer)
throws IOE
             }
             return;
         } else if (h.getType() == OpCode.sasl) {
-            Record rsp = processSasl(incomingBuffer,cnxn);
-            ReplyHeader rh = new ReplyHeader(h.getXid(), 0, KeeperException.Code.OK.intValue());
-            cnxn.sendResponse(rh,rsp, "response"); // not sure about 3rd arg..what is it?
-            return;
+            processSasl(incomingBuffer,cnxn, h);
         } else {
+          if (shouldRequireClientSaslAuth() && !hasCnxSASLAuthenticated(cnxn)) {
+            ReplyHeader replyHeader = new ReplyHeader(h.getXid(), 0,
+                Code.SESSIONCLOSEDREQUIRESASLAUTH.intValue());
+            cnxn.sendResponse(replyHeader, null, "response");
+            cnxn.sendCloseSession();
+            cnxn.disableRecv();
+          } else {
             Request si = new Request(cnxn, cnxn.getSessionId(), h.getXid(),
-              h.getType(), incomingBuffer, cnxn.getAuthInfo());
+                h.getType(), incomingBuffer, cnxn.getAuthInfo());
             si.setOwner(ServerCnxn.me);
             // Always treat packet from the client as a possible
             // local request.
             setLocalSessionFlag(si);
             submitRequest(si);
-            return;
+          }
         }
     }
 
-    private Record processSasl(ByteBuffer incomingBuffer, ServerCnxn cnxn) throws IOException
{
+  private boolean shouldAllowSaslFailedClientsConnect() {
+    String allowSaslFailedClients = System.getProperty("zookeeper.allowSaslFailedClients");
+    if (allowSaslFailedClients == null) {
+      return false;
+    } else {
+      return allowSaslFailedClients.equals("true");
+    }
+  }
+
+  private boolean shouldRequireClientSaslAuth() {
+    String sessionRequireClientSASLAuth = System.getProperty("zookeeper.sessionRequireClientSASLAuth");
 
 Review comment:
   The cost of caching properties is, we have to invent new variables to store these properties
and sometimes, we need worry about cache coherence as the properties could be updated at runtime
(mostly for unit test cases where we switch properties settings.). This will increase code
complexity. The cost of accessing a property is negligible, so I prefer using simple code
to access the properties. This is also consistent with existing code base where almost all
properties are accessed without being cached.
   
   I do update the PR to use a simpler method to query the properties though.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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

Mime
View raw message