HPCC-31141 Validate global sort connect protocol

Add a const signature to the global sort connect protocol, and validate it in the header on connect. This is to exclude 'rogue' attempts to connect to the sort socket, possibly from k8s health type checks. Also remove pointless byte ordering on serialize/deserialize in this context, where both sides are implicitly on same OS/byte-order/version. Signed-off-by: Jake Smith <[email protected]>
hpcc-systems · Jan 31, 2024 · 9fa887f · 9fa887f
1 parent 6cea89d
commit 9fa887f
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 8 deletions.
diff --git a/thorlcr/msort/tsortl.cpp b/thorlcr/msort/tsortl.cpp
@@ -60,8 +60,10 @@ class CREcheck {
 
 #define RECHECK(b) CREcheck checkRE(b)
 
+static constexpr unsigned __int64 transferStreamSignature = 0x0ea614193fb99496; // unique/random pattern to validate against
 struct TransferStreamHeader
 {
+    unsigned __int64 hdrSig = transferStreamSignature; // used to validate hdr is of expected protocol on connect
     rowcount_t numrecs;
     rowcount_t pos;
     unsigned id;
@@ -72,7 +74,6 @@ struct TransferStreamHeader
         crc = getCrc();
     }
     TransferStreamHeader() {}
-    void winrev() { _WINREV(pos); _WINREV(numrecs); _WINREV(id); _WINREV(crc); }
     unsigned getCrc() const
     {
         unsigned retCrc = crc32((const char *)&numrecs, sizeof(numrecs), 0);
@@ -288,7 +289,6 @@ IRowStream *ConnectMergeRead(unsigned id, IThorRowInterfaces *rowif,SocketEndpoi
     nodeaddr.getEndpointHostText(s);
     LOG(MCthorDetailedDebugInfo, thorJob, "ConnectMergeRead(%d,%s,%x,%" RCPF "d,%" RCPF "u)",id,s.str(),(unsigned)(memsize_t)socket.get(),startrec,numrecs);
 #endif
-    hdr.winrev();
     socket->write(&hdr,sizeof(hdr));
     return new CSocketRowStream(id,rowif->queryRowAllocator(),rowif->queryRowDeserializer(),socket);
 }
@@ -314,12 +314,21 @@ ISocketRowWriter *ConnectMergeWrite(IThorRowInterfaces *rowif,ISocket *socket,si
         remaining -= read;
         dst += read;
     }
-    hdr.winrev();
+    if (hdr.hdrSig != transferStreamSignature)
+    {
+        char name[100];
+        int port = socket->peer_name(name,sizeof(name));
+        StringBuffer hdrRaw;
+        hexdump2string((byte const *)&hdr, sizeof(hdr), hdrRaw);
+        throw makeStringExceptionV(TE_SortConnectProtocolErr, "SORT connection protocol mismatch from: %s:%u, raw hdr = %s", name, port, hdrRaw.str());
+    }
     if (hdr.getCrc() != hdr.crc)
     {
         char name[100];
         int port = socket->peer_name(name,sizeof(name));
-        throw makeStringExceptionV(TE_InvalidSortConnect, "Invalid SORT connection from: %s:%u", name, port);
+        StringBuffer hdrRaw;
+        hexdump2string((byte const *)&hdr, sizeof(hdr), hdrRaw);
+        throw makeStringExceptionV(TE_SortConnectCrcErr, "SORT connection failed crc check from: %s:%u, raw hdr = %s", name, port, hdrRaw.str());
     }
     startrec = hdr.pos;
     numrecs = hdr.numrecs;

diff --git a/thorlcr/msort/tsorts1.cpp b/thorlcr/msort/tsorts1.cpp
@@ -403,8 +403,13 @@ protected: friend class CSortMerge;
                 catch (IException *e) // only retry if serialization check failed, indicating possible foreign client connect
                 {
                     PrintExceptionLog(e, "WARNING: Exception(ConnectMergeWrite)");
-                    if (TE_InvalidSortConnect != e->errorCode() || (--numretries==0))
-                        throw;
+
+                    if (TE_SortConnectProtocolErr != e->errorCode())
+                    {
+                        if (TE_SortConnectCrcErr != e->errorCode() || (--numretries==0))
+                            throw;
+                    }
+
                     e->Release();
                     continue;
                 }

diff --git a/thorlcr/shared/thexception.hpp b/thorlcr/shared/thexception.hpp
@@ -158,8 +158,9 @@
 #define TE_RemoteReadFailure                    THOR_ERROR_START + 137
 #define TE_UnsupportedSortOrder                 THOR_ERROR_START + 138
 #define TE_CostExceeded                         THOR_ERROR_START + 139
-#define TE_InvalidSortConnect                   THOR_ERROR_START + 140
-#define TE_Final                                THOR_ERROR_START + 141       // keep this last
+#define TE_SortConnectCrcErr                    THOR_ERROR_START + 140
+#define TE_SortConnectProtocolErr               THOR_ERROR_START + 141
+#define TE_Final                                THOR_ERROR_START + 142       // keep this last
 #define ISTHOREXCEPTION(n) (n > THOR_ERROR_START && n < TE_Final)
 
 #endif