Skip to content

Commit

Permalink
Remove Ident protocol support (#1827)
Browse files Browse the repository at this point in the history 
Ident protocol (RFC 931 obsoleted by RFC 1413) has been considered
seriously insecure and broken since at least 2009 when SANS issued an
update recommending its removal from all networks. Squid Ident
implementation suffered from assertions (since 2021 commit e227da8) and
memory leaks (since 2015 commit fbbea66). Ident implementation design
increased ACLChecklist::goAsync() design complexity.

An external ACL helper can be written to perform Ident transactions.

Configurations using ident/ident_regex ACLs, %ui logformat codes, %IDENT
external_acl_type format code, or ident_lookup_access/ident_timeout
directives are now rejected, leading to fatal startup failures:

    FATAL: Invalid ACL type 'ident_regex'
    FATAL: Invalid ACL type 'ident'
    ERROR: configuration failure: Unsupported %code: '%ui'
    ERROR: configuration failure: Unsupported %code: '%IDENT'
    squid.conf(6): unrecognized: 'ident_lookup_access'
    squid.conf(7): unrecognized: 'ident_timeout'

To avoid inconveniencing admins that do _not_ use Ident features, access
logs with "common" and "combined" logformats now always receive a dash
in the position of what used to be a %ui record field.

Co-authored-by: Amos Jeffries <squid3@treenet.co.nz>
  • Loading branch information
@eduard-bagdasaryan @yadij @squid-anubis
2 people authored and squid-anubis committed Jun 2, 2024
1 parent 096c1f9 commit e94ff52
Show file tree
Hide file tree
Showing 72 changed files with 101 additions and 843 deletions.
9 changes: 0 additions & 9 deletions configure.ac
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1522,14 +1522,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],
Expand Down Expand Up @@ -2608,7 +2600,6 @@ AC_CONFIG_FILES([
src/http/url_rewriters/fake/Makefile
src/http/url_rewriters/LFS/Makefile
src/icmp/Makefile
src/ident/Makefile
src/ip/Makefile
src/ipc/Makefile
src/log/Makefile
Expand Down
8 changes: 0 additions & 8 deletions doc/Programming-Guide/03_MajorComponents.dox
View file
Original file line number Diff line number Diff line change
Expand Up @@ -289,14 +289,6 @@ TODO: get debugs() documenting as if it was a function.
format.
TODO: get RFCs linked from ietf

\section IdentLookups Ident Lookups
\par
These routines support RFC 931 (http://www.ietf.org/rfc/rfc931.txt)
"Ident" lookups. An ident
server running on a host will report the user name associated
with a connected TCP socket. Some sites use this facility for
access control and logging purposes.

\section MemoryManagement Memory Management
\par
These routines allocate and manage pools of memory for
Expand Down
1 change: 0 additions & 1 deletion doc/debug-sections.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ section 28 Access Control
section 29 Authenticator
section 29 NTLM Authenticator
section 29 Negotiate Authenticator
section 30 Ident (RFC 931)
section 31 Hypertext Caching Protocol
section 32 Asynchronous Disk I/O
section 33 Client Request Pipeline
Expand Down
2 changes: 1 addition & 1 deletion doc/manuals/ar.po
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2167,7 +2167,7 @@ msgstr ""
msgid ""
"User is just a unique key value. The above example uses %LOGIN and the "
"username but any of the B<external_acl_type> format tags can be substituted "
"in its place. B<%EXT_TAG> , B<%LOGIN> , B<%IDENT> , B<%EXT_USER> , B<"
"in its place. B<%EXT_TAG> , B<%LOGIN> , B<%EXT_USER> , B<"
"%SRC> , B<%SRCEUI48> , and B<%SRCEUI64> are all likely candidates for client "
"identification. The Squid wiki has more examples at https://wiki.squid-cache."
"org/ConfigExamples."
Expand Down
2 changes: 1 addition & 1 deletion doc/manuals/cs.po
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2153,7 +2153,7 @@ msgstr ""
msgid ""
"User is just a unique key value. The above example uses %LOGIN and the "
"username but any of the B<external_acl_type> format tags can be substituted "
"in its place. B<%EXT_TAG> , B<%LOGIN> , B<%IDENT> , B<%EXT_USER> , B<"
"in its place. B<%EXT_TAG> , B<%LOGIN> , B<%EXT_USER> , B<"
"%SRC> , B<%SRCEUI48> , and B<%SRCEUI64> are all likely candidates for client "
"identification. The Squid wiki has more examples at https://wiki.squid-cache."
"org/ConfigExamples."
Expand Down
2 changes: 1 addition & 1 deletion doc/manuals/de.po
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2216,7 +2216,7 @@ msgstr ""
msgid ""
"User is just a unique key value. The above example uses %LOGIN and the "
"username but any of the B<external_acl_type> format tags can be substituted "
"in its place. B<%EXT_TAG> , B<%LOGIN> , B<%IDENT> , B<%EXT_USER> , B<"
"in its place. B<%EXT_TAG> , B<%LOGIN> , B<%EXT_USER> , B<"
"%SRC> , B<%SRCEUI48> , and B<%SRCEUI64> are all likely candidates for client "
"identification. The Squid wiki has more examples at https://wiki.squid-cache."
"org/ConfigExamples."
Expand Down
2 changes: 1 addition & 1 deletion doc/manuals/en.po
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2382,7 +2382,7 @@ msgstr ""
msgid ""
"User is just a unique key value. The above example uses %LOGIN and the "
"username but any of the B<external_acl_type> format tags can be substituted "
"in its place. B<%EXT_TAG> , B<%LOGIN> , B<%IDENT> , B<%EXT_USER> , B<"
"in its place. B<%EXT_TAG> , B<%LOGIN> , B<%EXT_USER> , B<"
"%SRC> , B<%SRCEUI48> , and B<%SRCEUI64> are all likely candidates for client "
"identification. The Squid wiki has more examples at https://wiki.squid-cache."
"org/ConfigExamples."
Expand Down
2 changes: 1 addition & 1 deletion doc/manuals/en_AU.po
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2295,7 +2295,7 @@ msgstr ""
msgid ""
"User is just a unique key value. The above example uses %LOGIN and the "
"username but any of the B<external_acl_type> format tags can be substituted "
"in its place. B<%EXT_TAG> , B<%LOGIN> , B<%IDENT> , B<%EXT_USER> , B<"
"in its place. B<%EXT_TAG> , B<%LOGIN> , B<%EXT_USER> , B<"
"%SRC> , B<%SRCEUI48> , and B<%SRCEUI64> are all likely candidates for client "
"identification. The Squid wiki has more examples at https://wiki.squid-cache."
"org/ConfigExamples."
Expand Down
2 changes: 1 addition & 1 deletion doc/manuals/manuals.pot
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1628,7 +1628,7 @@ msgstr ""
msgid ""
"User is just a unique key value. The above example uses %LOGIN and the "
"username but any of the B<external_acl_type> format tags can be substituted "
"in its place. B<%EXT_TAG> , B<%LOGIN> , B<%IDENT> , B<%EXT_USER> , B<%SRC> "
"in its place. B<%EXT_TAG> , B<%LOGIN> , B<%EXT_USER> , B<%SRC> "
", B<%SRCEUI48> , and B<%SRCEUI64> are all likely candidates for client "
"identification. The Squid wiki has more examples at "
"https://wiki.squid-cache.org/ConfigExamples."
Expand Down
2 changes: 1 addition & 1 deletion doc/manuals/ru.po
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2226,7 +2226,7 @@ msgstr ""
msgid ""
"User is just a unique key value. The above example uses %LOGIN and the "
"username but any of the B<external_acl_type> format tags can be substituted "
"in its place. B<%EXT_TAG> , B<%LOGIN> , B<%IDENT> , B<%EXT_USER> , B<"
"in its place. B<%EXT_TAG> , B<%LOGIN> , B<%EXT_USER> , B<"
"%SRC> , B<%SRCEUI48> , and B<%SRCEUI64> are all likely candidates for client "
"identification. The Squid wiki has more examples at https://wiki.squid-cache."
"org/ConfigExamples."
Expand Down
41 changes: 41 additions & 0 deletions doc/release-notes/release-7.sgml.in
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ The Squid-@SQUID_RELEASE@ change history can be <url url="https://github.com/squ
<item>Cache Manager changes
<item>Removed purge tool
<item>Remove deprecated languages
<item>Remove Ident protocol support
</itemize>

<p>Most user-facing changes are reflected in squid.conf (see further below).
Expand Down Expand Up @@ -103,6 +104,24 @@ The Squid-@SQUID_RELEASE@ change history can be <url url="https://github.com/squ
<p>See <url url="https://en.wikipedia.org/wiki/List_of_ISO_639_language_codes"> for
the full ISO-639 list. HTTP uses the 2-letter (set 1) codes.

<sect1>Removed Ident protocol support

<p>Ident protocol (RFC 931 obsoleted by RFC 1413) has been considered
seriously insecure and broken since at least 2009 when SANS issued an update
recommending its removal from all networks. Squid Ident implementation had its
own set of problems (that could not be addressed without significant code
refactoring).

<p>Configurations using ident/ident_regex ACLs, %ui logformat codes, %IDENT
external_acl_type format code, or ident_lookup_access/ident_timeout directives
are now rejected, leading to fatal startup failures.

<p>To avoid inconveniencing admins that do <em>not</em> use Ident features,
access logs with "common" and "combined" logformats now always receive a dash
in the position of what used to be a %ui record field.

<p>If necessary, an external ACL helper can be written to perform Ident transactions
and deliver the user identity to Squid through the **user=** annotation.

<sect>Changes to squid.conf since Squid-@SQUID_RELEASE_OLD@
<p>
Expand Down Expand Up @@ -143,6 +162,8 @@ This section gives an account of those changes in three categories:
necessary.
<p>Changed <em>http_status</em> to detect and handle overlapping
status and status-range values. Merging where necessary.
<p>Removed <em>ident</em> with Ident protocol support.
<p>Removed <em>ident_regex</em> with Ident protocol support.

<tag>buffered_logs</tag>
<p>Honor the <em>off</em> setting in 'udp' access_log module.
Expand All @@ -151,6 +172,17 @@ This section gives an account of those changes in three categories:
<p>Removed the <em>non_peers</em> action. See the Cache Manager
<ref id="mgr" name="section"> for details.

<tag>access_log</tag>
<p>Built-in <em>common</em> and <em>combined</em> logformats now always
receive a dash character ("-") in the position of what used to be a
<em>%ui</em> record field.

<tag>logformat</tag>
<p>Removed <em>%ui</em> format code with Ident protocol support.

<tag>external_acl_type</tag>
<p>Removed <em>%IDENT</em> format code with Ident protocol support.

</descrip>

<sect1>Removed directives<label id="removeddirectives">
Expand All @@ -172,6 +204,12 @@ This section gives an account of those changes in three categories:
<p>The corresponding code has not built for many years, indicating that the
feature is unused.

<tag>ident_lookup_access</tag>
<p>Ident protocol is no longer supported natively.

<tag>ident_timeout</tag>
<p>Ident protocol is no longer supported natively.

</descrip>


Expand Down Expand Up @@ -232,6 +270,9 @@ This section gives an account of those changes in three categories:
<p>The code enabled by this preprocessor macro has not built for many
years, indicating that the feature is unused.

<tag>--disable-ident-lookups</tag>
<p>The option was dropped during Ident protocol support removal.

</descrip>

<sect>Copyright
Expand Down
1 change: 0 additions & 1 deletion squid.dox
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2088,7 +2088,6 @@ PREDEFINED = __cplusplus \
USE_HTCP \
USE_HTTP_VIOLATIONS \
USE_ICMP \
USE_IDENT \
USE_IPV6 \
USE_KQUEUE \
USE_LOADABLE_MODULES \
Expand Down
12 changes: 0 additions & 12 deletions src/AccessLogEntry.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,18 +96,6 @@ AccessLogEntry::syncNotes(HttpRequest *req)
assert(notes == req->notes());
}

const char *
AccessLogEntry::getClientIdent() const
{
if (tcpClient)
return tcpClient->rfc931;

if (cache.rfc931 && *cache.rfc931)
return cache.rfc931;

return nullptr;
}

const char *
AccessLogEntry::getExtUser() const
{
Expand Down
4 changes: 0 additions & 4 deletions src/AccessLogEntry.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,6 @@ 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;

/// Fetch the external ACL provided 'user=' string, or nil if none is available.
const char *getExtUser() const;

Expand Down Expand Up @@ -157,7 +154,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;
Expand Down
8 changes: 0 additions & 8 deletions src/AclRegs.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,6 @@
#endif
#include "base/RegexPattern.h"
#include "ExternalACL.h"
#if USE_IDENT
#include "ident/AclIdent.h"
#endif
#if SQUID_SNMP
#include "snmp_core.h"
#endif
Expand Down Expand Up @@ -273,11 +270,6 @@ 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); });
RegisterMaker("ext_user_regex", [](TypeName name)->Node* { return new ACLExtUser(new ACLRegexData, name); });
Expand Down
2 changes: 1 addition & 1 deletion src/DelayId.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
ch.updateReply(reply);
// overwrite ACLFilledChecklist acl_uses_indirect_client-based decision
Expand Down
10 changes: 5 additions & 5 deletions src/FwdState.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,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);
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;

Expand All @@ -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);
Expand All @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions src/HttpHeaderTools.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.updateAle(al);

// XXX: The two "It was denied" clauses below mishandle cases with no
Expand Down Expand Up @@ -492,7 +492,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.updateAle(al);

for (HeaderWithAclList::const_iterator hwa = headersAdd.begin(); hwa != headersAdd.end(); ++hwa) {
Expand Down
2 changes: 1 addition & 1 deletion src/HttpReply.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,7 @@ HttpReply::calcMaxBodySize(HttpRequest& request) const
if (!Config.ReplyBodySize)
return;

ACLFilledChecklist ch(nullptr, &request, nullptr);
ACLFilledChecklist ch(nullptr, &request);
ch.updateReply(this);
for (AclSizeLimit *l = Config.ReplyBodySize; l; l = l -> next) {
/* if there is no ACL list or if the ACLs listed match use this size value */
Expand Down
4 changes: 2 additions & 2 deletions src/HttpRequest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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();
Expand Down
Loading

0 comments on commit e94ff52

Please sign in to comment.