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 5ac59a82944c9c207d277723564b09132d81f116..2bc0a1e597daa45f035a14eb38d3738711afa148 100644 (file)
--- a/CONTRIBUTORS
+++ b/CONTRIBUTORS
@@ -55,7 +55,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 5d061019454bcdb4a559d5b39f6998262de12943..ac2586fd924ea55681e8e4339591859e2580b358 100644 (file)
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1033,6 +1033,7 @@ tests_testRock_SOURCES = \
        HttpHeaderTools.h \
        HttpReply.cc \
        tests/stub_HttpRequest.cc \
+       tests/stub_Instance.cc \
        LogTags.cc \
        MasterXaction.cc \
        MasterXaction.h \
@@ -1203,6 +1204,7 @@ tests_testUfs_SOURCES = \
        HttpHeaderTools.h \
        HttpReply.cc \
        tests/stub_HttpRequest.cc \
+       tests/stub_Instance.cc \
        LogTags.cc \
        MasterXaction.cc \
        MasterXaction.h \
@@ -1375,6 +1377,7 @@ tests_testStore_SOURCES = \
        HttpHeaderTools.h \
        tests/stub_HttpReply.cc \
        tests/stub_HttpRequest.cc \
+       tests/stub_Instance.cc \
        MasterXaction.cc \
        MasterXaction.h \
        MemBuf.cc \
@@ -1538,6 +1541,7 @@ tests_testDiskIO_SOURCES = \
        HttpHeaderTools.h \
        HttpReply.cc \
        tests/stub_HttpRequest.cc \
+       tests/stub_Instance.cc \
        LogTags.cc \
        MasterXaction.cc \
        MasterXaction.h \
@@ -1801,6 +1805,7 @@ tests_testHttpRange_SOURCES = \
        HttpReply.cc \
        HttpRequest.cc \
        tests/stub_HttpUpgradeProtocolAccess.cc \
+       tests/stub_Instance.cc \
        IoStats.h \
        tests/stub_IpcIoFile.cc \
        LogTags.cc \
@@ -2061,6 +2066,7 @@ tests_testHttpReply_SOURCES = \
        tests/testHttpReply.cc \
        HttpReply.h \
        tests/stub_HttpRequest.cc \
+       tests/stub_Instance.cc \
        MasterXaction.cc \
        MasterXaction.h \
        MemBuf.cc \
@@ -2188,6 +2194,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 \
@@ -2485,6 +2492,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 2446d1bb7a6454a20fd6c2fc07f76bd17d245b08..ddf2e5881bc25ded0955ec3cda2049a4a1d51af6 100644 (file)
--- a/src/ipc/Port.cc
+++ b/src/ipc/Port.cc
@@ -13,8 +13,10 @@
 #include "comm/Connection.h"
 #include "comm/Read.h"
 #include "CommCalls.h"
+#include "Instance.h"
 #include "ipc/Port.h"
 #include "sbuf/Stream.h"
+#include "sbuf/StringConvert.h"
 #include "tools.h"
 #include "util.h"
 
@@ -52,9 +54,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.c_str());
-    addr.append(processLabel);
+    String addr;
+    addr.append(SBufToString(Instance::NamePrefix(channelPathPfx, processLabel)));
     addr.append('-');
     addr.append(xitoa(id));
     addr.append(".ipc");
@@ -66,9 +67,7 @@ Ipc::Port::CoordinatorAddr()
 {
     static String coordinatorAddr;
     if (!coordinatorAddr.size()) {
-        coordinatorAddr= channelPathPfx;
-        coordinatorAddr.append(service_name.c_str());
-        coordinatorAddr.append(coordinatorAddrLabel);
+        coordinatorAddr.append(SBufToString(Instance::NamePrefix(channelPathPfx, coordinatorAddrLabel)));
         coordinatorAddr.append(".ipc");
     }
     return coordinatorAddr;

diff --git a/src/ipc/mem/Segment.cc b/src/ipc/mem/Segment.cc
index 67ba4431f153214fd1b17e0c7f82242a22ec9711..40ea28e38818db3e12d0378eb2c531f5a4811092 100644 (file)
--- a/src/ipc/mem/Segment.cc
+++ b/src/ipc/mem/Segment.cc
@@ -13,8 +13,9 @@
 #include "compat/shm.h"
 #include "debug/Stream.h"
 #include "fatal.h"
+#include "Instance.h"
 #include "ipc/mem/Segment.h"
-#include "sbuf/SBuf.h"
+#include "sbuf/StringConvert.h"
 #include "SquidConfig.h"
 #include "tools.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.c_str());
+        name.append(SBufToString(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"))
+