Browse Source

HPCC-22395 Fix Dali WhiteList (make sure using peer IP)

Signed-off-by: Jake Smith <jake.smith@lexisnexisrisk.com>
Jake Smith 6 years ago
parent
commit
fb062c98a6
4 changed files with 30 additions and 8 deletions
  1. 5 0
      dali/base/dacoven.cpp
  2. 9 5
      dali/base/dasess.cpp
  3. 15 3
      system/mp/mpcomm.cpp
  4. 1 0
      system/mp/mpcomm.hpp

+ 5 - 0
dali/base/dacoven.cpp

@@ -321,6 +321,11 @@ public:
         if (comm)
             comm->cancel(srcrank,tag);
     }
+    virtual const SocketEndpoint &queryChannelPeerEndpoint(const SocketEndpoint &sender) override
+    {
+        assertex(comm);
+        return comm->queryChannelPeerEndpoint(sender);
+    }
 };
 
 

+ 9 - 5
dali/base/dasess.cpp

@@ -125,7 +125,7 @@ interface ISessionManagerServer: implements IConnectionMonitor
     virtual void stopSession(SessionId sessid,bool failed) = 0;
     virtual void setClientAuth(IDaliClientAuthConnection *authconn) = 0;
     virtual void setLDAPconnection(IDaliLdapConnection *_ldapconn) = 0;
-    virtual bool authorizeConnection(const INode *client, DaliClientRole role) = 0;
+    virtual bool authorizeConnection(const IpAddress &clientIP, DaliClientRole role) = 0;
     virtual void start() = 0;
     virtual void ready() = 0;
     virtual void stop() = 0;
@@ -554,11 +554,12 @@ public:
                 acceptConnections.wait();
                 acceptConnections.signal();
                 Owned<INode> node(deserializeINode(mb));
+                const SocketEndpoint &peerIP = coven.queryComm().queryChannelPeerEndpoint(mb.getSender());
                 Owned<INode> servernode(deserializeINode(mb));  // hopefully me, but not if forwarded
                 int role=0;
                 if (mb.length()-mb.getPos()>=sizeof(role)) { // a capability block present
                     mb.read(role);
-                    if (!manager.authorizeConnection(node, (DaliClientRole) role))
+                    if (!manager.authorizeConnection(peerIP, (DaliClientRole) role))
                     {
                         MilliSleep(2000); // Delay makes rapid probing of all possible roles slightly more painful.
                         SocketEndpoint sender = mb.getSender();
@@ -568,7 +569,10 @@ public:
                         Owned<IGroup> dummyCoven = createIGroup(1, &na);
                         dummyCoven->serialize(mb);
                         const char *roleName = queryRoleName((DaliClientRole)role);
-                        Owned<IException> e = makeStringExceptionV(-1, "Access denied! [role=%s]", roleName);
+                        StringBuffer ipStr;
+                        peerIP.getIpText(ipStr);
+                        Owned<IException> e = makeStringExceptionV(-1, "Access denied! [client ip=%s, role=%s]", ipStr.str(), roleName);
+                        EXCLOG(e, nullptr);
                         serializeException(e, mb);
                         coven.reply(mb);
                         MilliSleep(100+getRandom()%1000); // Causes client to 'work' for a short time.
@@ -1912,10 +1916,10 @@ public:
     }
 
 
-    bool authorizeConnection(const INode *client, DaliClientRole role)
+    bool authorizeConnection(const IpAddress &clientIP, DaliClientRole role)
     {
         StringBuffer ipStr;
-        client->endpoint().getIpText(ipStr);
+        clientIP.getIpText(ipStr);
         return whiteListHandler.isWhiteListed(ipStr, role);
     }
 

+ 15 - 3
system/mp/mpcomm.cpp

@@ -725,7 +725,7 @@ class CMPChannel: public CInterface
     IArrayOf<ISocket> keptsockets;
     CriticalSection attachsect;
     unsigned __int64 attachaddrval = 0;
-    SocketEndpoint attachep;
+    SocketEndpoint attachep, attachPeerEp;
     atomic_t attachchk;
 
 protected: friend class CMPServer;
@@ -1153,6 +1153,7 @@ public:
                 CriticalBlock block(attachsect);
                 attachaddrval = 0;
                 attachep.set(nullptr);
+                attachPeerEp.set(nullptr);
                 atomic_set(&attachchk, 0);
             }
             if (!keepsocket) {
@@ -1212,6 +1213,8 @@ public:
     {
         return !closed&&(channelsock!=NULL);
     }
+
+    const SocketEndpoint &queryPeerEp() const { return attachPeerEp; }
 };
 
 // Message Handlers (not done as interfaces for speed reasons
@@ -1615,6 +1618,7 @@ void CMPChannel::reset()
     sendwaiting = 0;
     attachaddrval = 0;
     attachep.set(nullptr);
+    attachPeerEp.set(nullptr);
     atomic_set(&attachchk, 0);
     lastxfer = msTick();
 }
@@ -1644,6 +1648,8 @@ bool CMPChannel::attachSocket(ISocket *newsock,const SocketEndpoint &_remoteep,c
         CriticalBlock block(attachsect);
         attachaddrval = addrval;
         attachep = _remoteep;
+        if (newsock)
+            newsock->getPeerEndpoint(attachPeerEp);
         atomic_inc(&attachchk);
     }
 
@@ -2250,7 +2256,7 @@ bool CMPServer::recv(CMessageBuffer &mbuf, const SocketEndpoint *ep, mptag_t tag
         bool notify(CMessageBuffer *msg)
         {
             if ((tag==TAG_ALL)||(tag==msg->getTag())) {
-                SocketEndpoint senderep = msg->getSender();
+                const SocketEndpoint &senderep = msg->getSender();
                 if ((ep==NULL)||ep->equals(senderep)||senderep.isNull()) {
                     if (msg->getReplyTag()==TAG_CANCEL)
                         delete msg;
@@ -2981,6 +2987,13 @@ public:
         parent->removeChannel(channel);
     }
 
+    virtual const SocketEndpoint &queryChannelPeerEndpoint(const SocketEndpoint &sender) override
+    {
+        Owned<CMPChannel> channel = parent->lookup(sender);
+        assertex(channel);
+        return channel->queryPeerEp();
+    }
+
     CCommunicator(CMPServer *_parent,IGroup *_group, bool _outer)
     {
         outer = _outer;
@@ -2992,7 +3005,6 @@ public:
     {
         group->Release();
     }
-
 };
 
 

+ 1 - 0
system/mp/mpcomm.hpp

@@ -57,6 +57,7 @@ interface ICommunicator: extends IInterface
     virtual bool verifyAll(bool duplex=false, unsigned timeout=1000*60*30) = 0;
     virtual void disconnect(INode *node) = 0;
     virtual void barrier() = 0;
+    virtual const SocketEndpoint &queryChannelPeerEndpoint(const SocketEndpoint &sender) = 0;
 };
 
 interface IInterCommunicator: extends IInterface