Reduce UDS/segment name clashes across same-service instances (#2023)

Add a PID file name hash to the names of the shared memory segments and
Unix Domain Sockets. Since all instances running on the same host are
supposed to have unique PID files, this addition significantly reduces
the probability of name clashes when running multiple Squid instances
with the same service name (i.e. the same `squid -n` parameter value
that defaults to "squid").

A clash may still happen if two different PID file names have the same
hash or if multiple instances disable PID file management with
`pid_filename none`. Clashes may also happen in environments where Squid
does not even use service name for naming shared memory segments.

Examples of UDS and shared memory segment names (while using default
service name):

    /var/run/squid/squid-SLWQ-kid-1.ipc
    /var/run/squid/squid-SLWQ-coordinator.ipc
    /dev/shm/squid-SLWQ-tls_session_cache.shm
    /dev/shm/squid-SLWQ-transients_map_slices.shm

This change is a reference point for automated CONTRIBUTORS updates.

diff --git a/CONTRIBUTORS b/CONTRIBUTORS
index 16912d69bdb83c60692a17471c61363d60763c09..e95216701e64bb0099068176818d3669e9062091 100644 (file)
--- a/CONTRIBUTORS
+++ b/CONTRIBUTORS
@@ -54,7 +54,7 @@ Thank you!
     Andrew Tridgell
     Andrey <rybakovandrey85@gmail.com>
     Andrey Shorin <tolsty@tushino.com>
-    ankor2023 <138755079+ankor2023@users.noreply.github.com>
+    Ankor <ankor2023@gmail.com>
     Anonymous <bigparrot@pirateperfection.com>
     Anonymous <redskilldough@gmail.com>
     Anonymous Pootle User

diff --git a/src/Instance.cc b/src/Instance.cc
index 1f3b10cc69db9ef258410c0d6551a7d1cc4e7ca8..3460a95021aeef159ff943c88cb6cb07afd9ba38 100644 (file)
--- a/src/Instance.cc
+++ b/src/Instance.cc
@@ -11,6 +11,7 @@
 #include "debug/Messages.h"
 #include "fs_io.h"
 #include "Instance.h"
+#include "md5.h"
 #include "parser/Tokenizer.h"
 #include "sbuf/Stream.h"
 #include "SquidConfig.h"
@@ -222,3 +223,43 @@ Instance::WriteOurPid()
     debugs(50, Important(23), "Created " << TheFile);
 }
 
+/// A hash that is likely to be unique across instances running on the same host
+/// because such concurrent instances should use unique PID filenames.
+/// All instances with disabled PID file maintenance have the same hash value.
+/// \returns a 4-character string suitable for use in file names and similar contexts
+static SBuf
+PidFilenameHash()
+{
+    uint8_t hash[SQUID_MD5_DIGEST_LENGTH];
+
+    SquidMD5_CTX ctx;
+    SquidMD5Init(&ctx);
+    const auto name = PidFilenameCalc();
+    SquidMD5Update(&ctx, name.rawContent(), name.length());
+    SquidMD5Final(hash, &ctx);
+
+    // converts raw hash byte at a given position to a filename-suitable character
+    const auto hashAt = [&hash](const size_t idx) {
+        const auto safeChars = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";
+        return safeChars[hash[idx] % strlen(safeChars)];
+    };
+
+    SBuf buf;
+    buf.appendf("%c%c%c%c", hashAt(0), hashAt(1), hashAt(2), hashAt(3));
+    return buf;
+}
+
+SBuf
+Instance::NamePrefix(const char * const head, const char * const tail)
+{
+    SBuf buf(head);
+    buf.append(service_name);
+    buf.append("-");
+    buf.append(PidFilenameHash());
+    if (tail) {
+        // TODO: Remove leading "-" from callers and explicitly add it here.
+        buf.append(tail);
+    }
+    return buf;
+}
+

diff --git a/src/Instance.h b/src/Instance.h
index b0e8bdff3acd2d59e8c9933d9acff67fe9be12a5..2a83abaaafb9193324ecb788188a1fb9fc1cd609 100644 (file)
--- a/src/Instance.h
+++ b/src/Instance.h
@@ -9,6 +9,8 @@
 #ifndef SQUID_SRC_INSTANCE_H
 #define SQUID_SRC_INSTANCE_H
 
+#include "sbuf/forward.h"
+
 #if HAVE_SYS_TYPES_H
 #include <sys/types.h>
 #endif
@@ -30,6 +32,14 @@ void WriteOurPid();
 /// Throws if PID file maintenance is disabled.
 pid_t Other();
 
+/// A service_name-derived string that is likely to be unique across all Squid
+/// instances concurrently running on the same host (as long as they do not
+/// disable PID file maintenance).
+/// \param head is used at the beginning of the generated name
+/// \param tail is used at the end of the generated name (when not nil)
+/// \returns a head-...tail string suitable for making file and shm segment names
+SBuf NamePrefix(const char *head, const char *tail = nullptr);
+
 } // namespace Instance
 
 #endif /* SQUID_SRC_INSTANCE_H */

diff --git a/src/Makefile.am b/src/Makefile.am
index 1ef2860f5bc71572e1d399d53f22e60f57ff8186..d89d7bff83cf1591051709f41b4bde901e5ec51e 100644 (file)
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1032,6 +1032,7 @@ tests_testRock_SOURCES = \
        HttpHeaderTools.h \
        HttpReply.cc \
        tests/stub_HttpRequest.cc \
+       tests/stub_Instance.cc \
        LogTags.cc \
        MasterXaction.cc \
        MasterXaction.h \
@@ -1202,6 +1203,7 @@ tests_testUfs_SOURCES = \
        HttpHeaderTools.h \
        HttpReply.cc \
        tests/stub_HttpRequest.cc \
+       tests/stub_Instance.cc \
        LogTags.cc \
        MasterXaction.cc \
        MasterXaction.h \
@@ -1374,6 +1376,7 @@ tests_testStore_SOURCES = \
        HttpHeaderTools.h \
        tests/stub_HttpReply.cc \
        tests/stub_HttpRequest.cc \
+       tests/stub_Instance.cc \
        MasterXaction.cc \
        MasterXaction.h \
        MemBuf.cc \
@@ -1537,6 +1540,7 @@ tests_testDiskIO_SOURCES = \
        HttpHeaderTools.h \
        HttpReply.cc \
        tests/stub_HttpRequest.cc \
+       tests/stub_Instance.cc \
        LogTags.cc \
        MasterXaction.cc \
        MasterXaction.h \
@@ -1800,6 +1804,7 @@ tests_testHttpRange_SOURCES = \
        HttpReply.cc \
        HttpRequest.cc \
        tests/stub_HttpUpgradeProtocolAccess.cc \
+       tests/stub_Instance.cc \
        IoStats.h \
        tests/stub_IpcIoFile.cc \
        LogTags.cc \
@@ -2060,6 +2065,7 @@ tests_testHttpReply_SOURCES = \
        tests/testHttpReply.cc \
        HttpReply.h \
        tests/stub_HttpRequest.cc \
+       tests/stub_Instance.cc \
        MasterXaction.cc \
        MasterXaction.h \
        MemBuf.cc \
@@ -2187,6 +2193,7 @@ tests_testHttpRequest_SOURCES = \
        tests/testHttpRequest.cc \
        tests/testHttpRequestMethod.cc \
        tests/stub_HttpUpgradeProtocolAccess.cc \
+       tests/stub_Instance.cc \
        IoStats.h \
        tests/stub_IpcIoFile.cc \
        LogTags.cc \
@@ -2484,6 +2491,7 @@ tests_testCacheManager_SOURCES = \
        HttpReply.cc \
        HttpRequest.cc \
        tests/stub_HttpUpgradeProtocolAccess.cc \
+       tests/stub_Instance.cc \
        IoStats.h \
        tests/stub_IpcIoFile.cc \
        LogTags.cc \

diff --git a/src/ipc/Port.cc b/src/ipc/Port.cc
index 566327f2d4b0a51fadd3d6ff61382ab12f06bdc3..8c7a4cdc83702099cd3c9ff643c77d711f3a32ab 100644 (file)
--- a/src/ipc/Port.cc
+++ b/src/ipc/Port.cc
@@ -13,6 +13,7 @@
 #include "comm/Connection.h"
 #include "comm/Read.h"
 #include "CommCalls.h"
+#include "Instance.h"
 #include "ipc/Port.h"
 #include "sbuf/Stream.h"
 #include "tools.h"
@@ -52,9 +53,8 @@ bool Ipc::Port::doneAll() const
 String Ipc::Port::MakeAddr(const char* processLabel, int id)
 {
     assert(id >= 0);
-    String addr = channelPathPfx;
-    addr.append(service_name);
-    addr.append(processLabel);
+    String addr;
+    addr.append(Instance::NamePrefix(channelPathPfx, processLabel));
     addr.append('-');
     addr.append(xitoa(id));
     addr.append(".ipc");
@@ -66,9 +66,7 @@ Ipc::Port::CoordinatorAddr()
 {
     static String coordinatorAddr;
     if (!coordinatorAddr.size()) {
-        coordinatorAddr= channelPathPfx;
-        coordinatorAddr.append(service_name);
-        coordinatorAddr.append(coordinatorAddrLabel);
+        coordinatorAddr.append(Instance::NamePrefix(channelPathPfx, coordinatorAddrLabel));
         coordinatorAddr.append(".ipc");
     }
     return coordinatorAddr;

diff --git a/src/ipc/mem/Segment.cc b/src/ipc/mem/Segment.cc
index dbd458b57a53405704c4b839441e9fa73393f37a..075572483937de998f20425f65147ade558ee3b5 100644 (file)
--- a/src/ipc/mem/Segment.cc
+++ b/src/ipc/mem/Segment.cc
@@ -13,6 +13,7 @@
 #include "compat/shm.h"
 #include "debug/Stream.h"
 #include "fatal.h"
+#include "Instance.h"
 #include "ipc/mem/Segment.h"
 #include "sbuf/SBuf.h"
 #include "SquidConfig.h"
@@ -277,8 +278,7 @@ Ipc::Mem::Segment::GenerateName(const char *id)
         if (name[name.size()-1] != '/')
             name.append('/');
     } else {
-        name.append('/');
-        name.append(service_name);
+        name.append(Instance::NamePrefix("/"));
         name.append('-');
     }
 

diff --git a/src/tests/Stub.am b/src/tests/Stub.am
index ad03078aba7c7cd2122fab976ff5c5d3d8793eff..2f9d15d279e2b0666cf4b4428947b00ee7d7ebc4 100644 (file)
--- a/src/tests/Stub.am
+++ b/src/tests/Stub.am
@@ -22,6 +22,7 @@ STUB_SOURCE = \
        tests/stub_HttpReply.cc \
        tests/stub_HttpRequest.cc \
        tests/stub_HttpUpgradeProtocolAccess.cc \
+       tests/stub_Instance.cc \
        tests/stub_IpcIoFile.cc \
        tests/stub_MemBuf.cc \
        tests/stub_MemObject.cc \

diff --git a/src/tests/stub_Instance.cc b/src/tests/stub_Instance.cc
new file mode 100644 (file)
index 0000000..72fa443
--- /dev/null
+++ b/src/tests/stub_Instance.cc
@@ -0,0 +1,20 @@
+/*
+ * Copyright (C) 1996-2025 The Squid Software Foundation and contributors
+ *
+ * Squid software is distributed under GPLv2+ license and includes
+ * contributions from numerous individuals and organizations.
+ * Please see the COPYING and CONTRIBUTORS files for details.
+ */
+
+#include "squid.h"
+#include "Instance.h"
+#include "sbuf/SBuf.h"
+
+#define STUB_API "Instance.cc"
+#include "tests/STUB.h"
+
+void Instance::ThrowIfAlreadyRunning() STUB
+void Instance::WriteOurPid() STUB
+pid_t Instance::Other() STUB_RETVAL({})
+SBuf Instance::NamePrefix(const char *, const char *) STUB_RETVAL_NOP(SBuf("squid-0"))
+