measurement-factory · eduard-bagdasaryan · Jan 29, 2024 · Feb 1, 2024 · Feb 5, 2024 · Feb 8, 2024
diff --git a/configure.ac b/configure.ac
@@ -1943,14 +1943,6 @@ AC_MSG_NOTICE([Support for X-Forwarded-For enabled: ${enable_follow_x_forwarded_
 SQUID_DEFINE_BOOL(FOLLOW_X_FORWARDED_FOR,$enable_follow_x_forwarded_for,
   [Enable following X-Forwarded-For headers])
 
-AC_ARG_ENABLE(ident-lookups,
-  AS_HELP_STRING([--disable-ident-lookups],
-                 [Remove code that supports performing Ident (RFC 931) lookups.]), [
-  SQUID_YESNO([$enableval],[--enable-ident-lookups])
-])
-AC_MSG_NOTICE([Support for Ident lookups enabled: ${enable_ident_lookups:=yes}])
-SQUID_DEFINE_BOOL(USE_IDENT,$enable_ident_lookups,[Support for Ident (RFC 931) lookups])
-
 dnl Select Default hosts file location
 AC_ARG_ENABLE(default-hostsfile,
   AS_HELP_STRING([--enable-default-hostsfile=path],

diff --git a/src/AccessLogEntry.cc b/src/AccessLogEntry.cc
@@ -96,16 +96,10 @@ AccessLogEntry::syncNotes(HttpRequest *req)
         assert(notes == req->notes());
 }
 
-const char *
+Ident::User
 AccessLogEntry::getClientIdent() const
 {
-    if (tcpClient)
-        return tcpClient->rfc931;
-
-    if (cache.rfc931 && *cache.rfc931)
-        return cache.rfc931;
-
-    return nullptr;
+    return acceptedClientConnection ? acceptedClientConnection->ident : std::nullopt;
 }
 
 const char *

diff --git a/src/AccessLogEntry.h b/src/AccessLogEntry.h
@@ -60,8 +60,8 @@ class AccessLogEntry: public CodeContext
     /// Side effect: Enables reverse DNS lookups of future client addresses.
     const char *getLogClientFqdn(char *buf, size_t bufSize) const;
 
-    /// Fetch the client IDENT string, or nil if none is available.
-    const char *getClientIdent() const;
+    /// \returns rfc931 user identity (including empty strings), if any
+    Ident::User getClientIdent() const;
 
     /// Fetch the external ACL provided 'user=' string, or nil if none is available.
     const char *getExtUser() const;
@@ -79,6 +79,13 @@ class AccessLogEntry: public CodeContext
 
     SBuf url;
 
+    // TODO: Adaptation::Icap::ALE (and other Squid-initiated transactions) do not have
+    // accepted connections. A future Adaptation::Icap::ALE must have these removed.
+    /// initializes client connection with a connection accepted by Squid
+    void initAcceptedConnection(Comm::ConnectionPointer p) { acceptedClientConnection = p; tcpClient = p; }
+    /// \returns a connection accepted by Squid
+    Comm::ConnectionPointer getAcceptedConnection() const { return acceptedClientConnection; }
+
     /// TCP/IP level details about the client connection
     Comm::ConnectionPointer tcpClient;
     // TCP/IP level details about the server or peer connection
@@ -157,7 +164,6 @@ class AccessLogEntry: public CodeContext
         LogTags code;
         struct timeval start_time; ///< The time the master transaction started
         struct timeval trTime; ///< The response time
-        const char *rfc931 = nullptr;
         const char *extuser = nullptr;
 #if USE_OPENSSL
         const char *ssluser = nullptr;
@@ -281,6 +287,10 @@ class AccessLogEntry: public CodeContext
     /// Client URI (or equivalent) for effectiveVirginUrl() when HttpRequest is
     /// missing. This member is ignored unless the request member is nil.
     SBuf virginUrlForMissingRequest_;
+
+    /// The same as tcpClient for transactions accepted by Squid.
+    /// Nil for transactions open by Squid (e.g., ICAP transactions).
+    Comm::ConnectionPointer acceptedClientConnection;
 };
 
 class ACLChecklist;

diff --git a/src/AclRegs.cc b/src/AclRegs.cc
@@ -101,9 +101,7 @@
 #endif
 #include "base/RegexPattern.h"
 #include "ExternalACL.h"
-#if USE_IDENT
 #include "ident/AclIdent.h"
-#endif
 #if SQUID_SNMP
 #include "snmp_core.h"
 #endif
@@ -273,10 +271,8 @@ Acl::Init()
     RegisterMaker("eui64", [](TypeName name)->Node* { return new ACLEui64(name); });
 #endif
 
-#if USE_IDENT
     RegisterMaker("ident", [](TypeName name)->Node* { return new ACLIdent(new ACLUserData, name); });
     RegisterMaker("ident_regex", [](TypeName name)->Node* { return new ACLIdent(new ACLRegexData, name); });
-#endif
 
 #if USE_AUTH
     RegisterMaker("ext_user", [](TypeName name)->Node* { return new ACLExtUser(new ACLUserData, name); });

diff --git a/src/DelayId.cc b/src/DelayId.cc
@@ -86,7 +86,7 @@ DelayId::DelayClient(ClientHttpRequest * http, HttpReply *reply)
             continue;
         }
 
-        ACLFilledChecklist ch(DelayPools::delay_data[pool].access, r, nullptr);
+        ACLFilledChecklist ch(DelayPools::delay_data[pool].access, r);
         clientAclChecklistFill(ch, http);
         if (!ch.reply && reply) {
             ch.reply = reply;

diff --git a/src/FwdState.cc b/src/FwdState.cc
@@ -352,7 +352,7 @@ FwdState::Start(const Comm::ConnectionPointer &clientConn, StoreEntry *entry, Ht
          * Intentionally replace the src_addr automatically selected by the checklist code
          * we do NOT want the indirect client address to be tested here.
          */
-        ACLFilledChecklist ch(Config.accessList.miss, request, nullptr);
+        ACLFilledChecklist ch(Config.accessList.miss, request);
         ch.al = al;
         ch.src_addr = request->client_addr;
         ch.syncAle(request, nullptr);
@@ -1136,7 +1136,7 @@ FwdState::connectStart()
     cs->setHost(request->url.host());
     bool retriable = checkRetriable();
     if (!retriable && Config.accessList.serverPconnForNonretriable) {
-        ACLFilledChecklist ch(Config.accessList.serverPconnForNonretriable, request, nullptr);
+        ACLFilledChecklist ch(Config.accessList.serverPconnForNonretriable, request);
         ch.al = al;
         ch.syncAle(request, nullptr);
         retriable = ch.fastCheck().allowed();
@@ -1504,7 +1504,7 @@ getOutgoingAddress(HttpRequest * request, const Comm::ConnectionPointer &conn)
         return; // anything will do.
     }
 
-    ACLFilledChecklist ch(nullptr, request, nullptr);
+    ACLFilledChecklist ch(nullptr, request);
     ch.dst_peer_name = conn->getPeer() ? conn->getPeer()->name : nullptr;
     ch.dst_addr = conn->remote;
 
@@ -1531,7 +1531,7 @@ GetTosToServer(HttpRequest * request, Comm::Connection &conn)
     if (!Ip::Qos::TheConfig.tosToServer)
         return 0;
 
-    ACLFilledChecklist ch(nullptr, request, nullptr);
+    ACLFilledChecklist ch(nullptr, request);
     ch.dst_peer_name = conn.getPeer() ? conn.getPeer()->name : nullptr;
     ch.dst_addr = conn.remote;
     return aclMapTOS(Ip::Qos::TheConfig.tosToServer, &ch);
@@ -1544,7 +1544,7 @@ GetNfmarkToServer(HttpRequest * request, Comm::Connection &conn)
     if (!Ip::Qos::TheConfig.nfmarkToServer)
         return 0;
 
-    ACLFilledChecklist ch(nullptr, request, nullptr);
+    ACLFilledChecklist ch(nullptr, request);
     ch.dst_peer_name = conn.getPeer() ? conn.getPeer()->name : nullptr;
     ch.dst_addr = conn.remote;
     const auto mc = aclFindNfMarkConfig(Ip::Qos::TheConfig.nfmarkToServer, &ch);

diff --git a/src/HttpHeaderTools.cc b/src/HttpHeaderTools.cc
@@ -297,7 +297,7 @@ httpHdrMangle(HttpHeaderEntry * e, HttpRequest * request, HeaderManglers *hms, c
         return 1;
     }
 
-    ACLFilledChecklist checklist(hm->access_list, request, nullptr);
+    ACLFilledChecklist checklist(hm->access_list, request);
 
     checklist.al = al;
     if (al && al->reply) {
@@ -497,7 +497,7 @@ HeaderManglers::find(const HttpHeaderEntry &e) const
 void
 httpHdrAdd(HttpHeader *heads, HttpRequest *request, const AccessLogEntryPointer &al, HeaderWithAclList &headersAdd)
 {
-    ACLFilledChecklist checklist(nullptr, request, nullptr);
+    ACLFilledChecklist checklist(nullptr, request);
 
     checklist.al = al;
     if (al && al->reply) {

diff --git a/src/HttpReply.cc b/src/HttpReply.cc
@@ -594,7 +594,7 @@ HttpReply::calcMaxBodySize(HttpRequest& request) const
     if (!Config.ReplyBodySize)
         return;
 
-    ACLFilledChecklist ch(nullptr, &request, nullptr);
+    ACLFilledChecklist ch(nullptr, &request);
     // XXX: cont-cast becomes irrelevant when checklist is HttpReply::Pointer
     ch.reply = const_cast<HttpReply *>(this);
     HTTPMSGLOCK(ch.reply);

diff --git a/src/HttpRequest.cc b/src/HttpRequest.cc
@@ -601,7 +601,7 @@ HttpRequest::getRangeOffsetLimit()
 
     rangeOffsetLimit = 0; // default value for rangeOffsetLimit
 
-    ACLFilledChecklist ch(nullptr, this, nullptr);
+    ACLFilledChecklist ch(nullptr, this);
     ch.src_addr = client_addr;
     ch.my_addr =  my_addr;
 
@@ -798,7 +798,7 @@ HttpRequest::manager(const CbcPointer<ConnStateData> &aMgr, const AccessLogEntry
         const bool proxyProtocolPort = port ? port->flags.proxySurrogate : false;
         if (flags.interceptTproxy && !proxyProtocolPort) {
             if (Config.accessList.spoof_client_ip) {
-                ACLFilledChecklist *checklist = new ACLFilledChecklist(Config.accessList.spoof_client_ip, this, clientConnection->rfc931);
+                const auto checklist = new ACLFilledChecklist(Config.accessList.spoof_client_ip, this);
                 checklist->al = al;
                 checklist->syncAle(this, nullptr);
                 flags.spoofClientIp = checklist->fastCheck().allowed();

diff --git a/src/Notes.cc b/src/Notes.cc
@@ -69,7 +69,7 @@ Note::addValue(const char *value, const bool quoted, const char *descr, const Va
 bool
 Note::match(HttpRequest *request, HttpReply *reply, const AccessLogEntry::Pointer &al, SBuf &matched)
 {
-    ACLFilledChecklist ch(nullptr, request, nullptr);
+    ACLFilledChecklist ch(nullptr, request);
     ch.al = al;
     ch.reply = reply;
     ch.syncAle(request, nullptr);

diff --git a/src/acl/FilledChecklist.cc b/src/acl/FilledChecklist.cc
@@ -42,7 +42,6 @@ ACLFilledChecklist::ACLFilledChecklist() :
     my_addr.setEmpty();
     src_addr.setEmpty();
     dst_addr.setEmpty();
-    rfc931[0] = '\0';
 }
 
 ACLFilledChecklist::~ACLFilledChecklist()
@@ -108,13 +107,6 @@ ACLFilledChecklist::verifyAle() const
         showDebugWarning("HttpReply object");
         al->reply = reply;
     }
-
-#if USE_IDENT
-    if (*rfc931 && !al->cache.rfc931) {
-        showDebugWarning("IDENT");
-        al->cache.rfc931 = xstrdup(rfc931);
-    }
-#endif
 }
 
 void
@@ -208,7 +200,7 @@ ACLFilledChecklist::markSourceDomainChecked()
  *    *not* delete the list.  After the callback function returns,
  *    checkCallback() will delete the list (i.e., self).
  */
-ACLFilledChecklist::ACLFilledChecklist(const acl_access *A, HttpRequest *http_request, const char *ident):
+ACLFilledChecklist::ACLFilledChecklist(const acl_access *A, HttpRequest *http_request):
     dst_rdns(nullptr),
     reply(nullptr),
 #if USE_AUTH
@@ -226,11 +218,9 @@ ACLFilledChecklist::ACLFilledChecklist(const acl_access *A, HttpRequest *http_re
     my_addr.setEmpty();
     src_addr.setEmpty();
     dst_addr.setEmpty();
-    rfc931[0] = '\0';
 
     changeAcl(A);
     setRequest(http_request);
-    setIdent(ident);
 }
 
 void ACLFilledChecklist::setRequest(HttpRequest *httpRequest)
@@ -250,16 +240,3 @@ void ACLFilledChecklist::setRequest(HttpRequest *httpRequest)
             setConn(cmgr);
     }
 }
-
-void
-ACLFilledChecklist::setIdent(const char *ident)
-{
-#if USE_IDENT
-    assert(!rfc931[0]);
-    if (ident)
-        xstrncpy(rfc931, ident, USER_IDENT_SZ);
-#else
-    (void)ident;
-#endif
-}
-
diff --git a/src/acl/FilledChecklist.h b/src/acl/FilledChecklist.h
@@ -35,13 +35,11 @@ class ACLFilledChecklist: public ACLChecklist
 
 public:
     ACLFilledChecklist();
-    ACLFilledChecklist(const acl_access *, HttpRequest *, const char *ident = nullptr);
+    ACLFilledChecklist(const acl_access *, HttpRequest *);
     ~ACLFilledChecklist() override;
 
     /// configure client request-related fields for the first time
     void setRequest(HttpRequest *);
-    /// configure rfc931 user identity for the first time
-    void setIdent(const char *userIdentity);
 
 public:
     /// The client connection manager
@@ -69,6 +67,9 @@ class ACLFilledChecklist: public ACLChecklist
     void syncAle(HttpRequest *adaptedRequest, const char *logUri) const override;
     void verifyAle() const override;
 
+    /// \copydoc AccessLogEntry::getClientIdent()
+    Ident::User ident() const { return al ? al->getClientIdent() : std::nullopt; }
+
 public:
     Ip::Address src_addr;
     Ip::Address dst_addr;
@@ -79,7 +80,6 @@ class ACLFilledChecklist: public ACLChecklist
     HttpRequest::Pointer request;
     HttpReply *reply;
 
-    char rfc931[USER_IDENT_SZ];
 #if USE_AUTH
     Auth::UserRequest::Pointer auth_user_request;
 #endif

diff --git a/src/adaptation/AccessCheck.cc b/src/adaptation/AccessCheck.cc
@@ -130,7 +130,7 @@ Adaptation::AccessCheck::checkCandidates()
         if (AccessRule *r = FindRule(topCandidate())) {
             /* BUG 2526: what to do when r->acl is empty?? */
             // XXX: we do not have access to conn->rfc931 here.
-            acl_checklist = new ACLFilledChecklist(r->acl, filter.request, dash_str);
+            acl_checklist = new ACLFilledChecklist(r->acl, filter.request);
             if ((acl_checklist->reply = filter.reply))
                 HTTPMSGLOCK(acl_checklist->reply);
             acl_checklist->al = filter.al;

diff --git a/src/adaptation/icap/History.h b/src/adaptation/icap/History.h
@@ -9,6 +9,7 @@
 #ifndef SQUID_SRC_ADAPTATION_ICAP_HISTORY_H
 #define SQUID_SRC_ADAPTATION_ICAP_HISTORY_H
 
+#include "AccessLogEntry.h"
 #include "base/RefCount.h"
 #include "enums.h"
 #include "LogTags.h"
@@ -36,14 +37,14 @@ class History: public RefCountable
     /// \param[out] total time taken for all ICAP processing
     void processingTime(struct timeval &total) const;
 
-    String rfc931; ///< the username from ident
 #if USE_OPENSSL
     String ssluser; ///< the username from SSL
 #endif
     LogTags logType; ///< the squid request status (TCP_MISS etc)
 
     String log_uri; ///< the request uri
     size_t req_sz; ///< the request size
+    AccessLogEntry::Pointer acceptedClientAle;
 
 private:
     void currentTime(struct timeval &) const; ///< time since current start or zero

diff --git a/src/adaptation/icap/Launcher.cc b/src/adaptation/icap/Launcher.cc
@@ -141,7 +141,7 @@ bool Adaptation::Icap::Launcher::canRepeat(Adaptation::Icap::XactAbortInfo &info
         return true;
 
     ACLFilledChecklist *cl =
-        new ACLFilledChecklist(TheConfig.repeat, info.icapRequest, dash_str);
+        new ACLFilledChecklist(TheConfig.repeat, info.icapRequest);
     cl->reply = info.icapReply;
     HTTPMSGLOCK(cl->reply);
 

diff --git a/src/adaptation/icap/ModXact.cc b/src/adaptation/icap/ModXact.cc
@@ -1341,8 +1341,8 @@ void Adaptation::Icap::ModXact::finalizeLogInfo()
     // XXX: This reply (and other ALE members!) may have been needed earlier.
     al.reply = adapted_reply_;
 
-    if (h->rfc931.size())
-        al.cache.rfc931 = h->rfc931.termedBuf();
+    if (h->acceptedClientAle)
+        al.initAcceptedConnection(h->acceptedClientAle->getAcceptedConnection());
 
 #if USE_OPENSSL
     if (h->ssluser.size())

diff --git a/src/adaptation/icap/icap_log.cc b/src/adaptation/icap/icap_log.cc
@@ -60,7 +60,7 @@ icapLogRotate()
 void icapLogLog(AccessLogEntry::Pointer &al)
 {
     if (IcapLogfileStatus == LOG_ENABLE) {
-        ACLFilledChecklist checklist(nullptr, al->adapted_request, nullptr);
+        ACLFilledChecklist checklist(nullptr, al->adapted_request);
         if (al->reply) {
             checklist.reply = al->reply.getRaw();
             HTTPMSGLOCK(checklist.reply);

diff --git a/src/auth/UserRequest.cc b/src/auth/UserRequest.cc
@@ -465,7 +465,7 @@ static Auth::ConfigVector &
 schemesConfig(HttpRequest *request, HttpReply *rep)
 {
     if (!Auth::TheConfig.schemeLists.empty() && Auth::TheConfig.schemeAccess) {
-        ACLFilledChecklist ch(nullptr, request, nullptr);
+        ACLFilledChecklist ch(nullptr, request);
         ch.reply = rep;
         HTTPMSGLOCK(ch.reply);
         const auto answer = ch.fastCheck(Auth::TheConfig.schemeAccess);

diff --git a/src/cf.data.pre b/src/cf.data.pre
@@ -2093,7 +2093,6 @@ DOC_END
 
 NAME: ident_lookup_access
 TYPE: acl_access
-IFDEF: USE_IDENT
 DEFAULT: none
 DEFAULT_DOC: Unless rules exist in squid.conf, IDENT is not fetched.
 LOC: Ident::TheConfig.identLookup
@@ -7469,7 +7468,6 @@ DOC_END
 
 NAME: ident_timeout
 TYPE: time_t
-IFDEF: USE_IDENT
 LOC: Ident::TheConfig.timeout
 DEFAULT: 10 seconds
 DOC_START