Re: [PATCH] squid 4.4: latest patches (01-03)

[Michael Tremer <michael.tremer@ipfire.org>]

[-- Attachment #1: Type: text/plain, Size: 14571 bytes --]

Hey,

I think it is finally time (and there is a gap in the schedule) for merge squid 4.4.

Could you rebase your branch on next and send a patchset or maybe single patch if it is only that so that I can merge this? Right now I am not sure what patches I have to take to replay this on top of next.

Best,
-Michael

> On 8 Dec 2018, at 17:10, Matthias Fischer <matthias.fischer(a)ipfire.org> wrote:
> 
> For details see:
> http://www.squid-cache.org/Versions/v4/changesets/
> 
> Best,
> Matthias
> 
> Signed-off-by: Matthias Fischer <matthias.fischer(a)ipfire.org>
> ---
> lfs/squid                                     |   3 +
> ...b_exchange_with_a_TLS_cache_peer_307.patch |  91 ++++++++++++
> ...all_format_formally_to_make_dist_325.patch |  22 +++
> .../03_The_handshake_logformat_code_331.patch | 132 ++++++++++++++++++
> 4 files changed, 248 insertions(+)
> create mode 100644 src/patches/squid/01_Fix_netdb_exchange_with_a_TLS_cache_peer_307.patch
> create mode 100644 src/patches/squid/02_Maintenance_add_xz_tarball_format_formally_to_make_dist_325.patch
> create mode 100644 src/patches/squid/03_The_handshake_logformat_code_331.patch
> 
> diff --git a/lfs/squid b/lfs/squid
> index e5e799111..aaa2d0b96 100644
> --- a/lfs/squid
> +++ b/lfs/squid
> @@ -72,6 +72,9 @@ $(subst %,%_MD5,$(objects)) :
> $(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects))
> 	@$(PREBUILD)
> 	@rm -rf $(DIR_APP) && cd $(DIR_SRC) && tar xaf $(DIR_DL)/$(DL_FILE)
> +	cd $(DIR_APP) && patch -Np1 -i $(DIR_SRC)/src/patches/squid/01_Fix_netdb_exchange_with_a_TLS_cache_peer_307.patch
> +	cd $(DIR_APP) && patch -Np1 -i $(DIR_SRC)/src/patches/squid/02_Maintenance_add_xz_tarball_format_formally_to_make_dist_325.patch
> +	cd $(DIR_APP) && patch -Np1 -i $(DIR_SRC)/src/patches/squid/03_The_handshake_logformat_code_331.patch
> 	cd $(DIR_APP) && patch -Np0 -i $(DIR_SRC)/src/patches/squid/squid-4.4-fix-max-file-descriptors.patch
> 
> 	cd $(DIR_APP) && autoreconf -vfi
> diff --git a/src/patches/squid/01_Fix_netdb_exchange_with_a_TLS_cache_peer_307.patch b/src/patches/squid/01_Fix_netdb_exchange_with_a_TLS_cache_peer_307.patch
> new file mode 100644
> index 000000000..09f8961dc
> --- /dev/null
> +++ b/src/patches/squid/01_Fix_netdb_exchange_with_a_TLS_cache_peer_307.patch
> @@ -0,0 +1,91 @@
> +commit bc54d7a6f7ec510a25966f2f800d3ea874657546
> +Author: chi-mf <43963496+chi-mf(a)users.noreply.github.com>
> +Date:   2018-10-30 04:48:40 +0000
> +
> +    Fix netdb exchange with a TLS cache_peer (#307)
> +    
> +    Squid uses http-scheme URLs when sending netdb exchange (and possibly
> +    other) requests to a cache_peer. If a DIRECT path is selected for that
> +    cache_peer URL, then Squid sends a clear text HTTP request to that
> +    cache_peer. If that cache_peer expects a TLS connection, it will reject
> +    that request (with, e.g., error:transaction-end-before-headers),
> +    resulting in an HTTP 503 or 504 netdb fetch error.
> +    
> +    Workaround this by adding an internalRemoteUri() parameter to indicate
> +    whether https or http URL scheme should be used. Netdb fetches from
> +    CachePeer::secure peers now get an https scheme and, hence, a TLS
> +    connection.
> +
> +diff --git a/src/icmp/net_db.cc b/src/icmp/net_db.cc
> +index 0f488de..526093f 100644
> +--- a/src/icmp/net_db.cc
> ++++ b/src/icmp/net_db.cc
> +@@ -1282,7 +1282,7 @@ netdbExchangeStart(void *data)
> + #if USE_ICMP
> +     CachePeer *p = (CachePeer *)data;
> +     static const SBuf netDB("netdb");
> +-    char *uri = internalRemoteUri(p->host, p->http_port, "/squid-internal-dynamic/", netDB);
> ++    char *uri = internalRemoteUri(p->secure.encryptTransport, p->host, p->http_port, "/squid-internal-dynamic/", netDB);
> +     debugs(38, 3, "Requesting '" << uri << "'");
> +     const MasterXaction::Pointer mx = new MasterXaction(XactionInitiator::initIcmp);
> +     HttpRequest *req = HttpRequest::FromUrl(uri, mx);
> +diff --git a/src/internal.cc b/src/internal.cc
> +index 6ebc7a6..ff7b4d6 100644
> +--- a/src/internal.cc
> ++++ b/src/internal.cc
> +@@ -82,7 +82,7 @@ internalStaticCheck(const SBuf &urlPath)
> +  * makes internal url with a given host and port (remote internal url)
> +  */
> + char *
> +-internalRemoteUri(const char *host, unsigned short port, const char *dir, const SBuf &name)
> ++internalRemoteUri(bool encrypt, const char *host, unsigned short port, const char *dir, const SBuf &name)
> + {
> +     static char lc_host[SQUIDHOSTNAMELEN];
> +     assert(host && !name.isEmpty());
> +@@ -115,7 +115,7 @@ internalRemoteUri(const char *host, unsigned short port, const char *dir, const
> +     static MemBuf mb;
> + 
> +     mb.reset();
> +-    mb.appendf("http://" SQUIDSBUFPH, SQUIDSBUFPRINT(tmp.authority()));
> ++    mb.appendf("%s://" SQUIDSBUFPH, encrypt ? "https" : "http", SQUIDSBUFPRINT(tmp.authority()));
> + 
> +     if (dir)
> +         mb.append(dir, strlen(dir));
> +@@ -132,7 +132,10 @@ internalRemoteUri(const char *host, unsigned short port, const char *dir, const
> + char *
> + internalLocalUri(const char *dir, const SBuf &name)
> + {
> +-    return internalRemoteUri(getMyHostname(),
> ++    // XXX: getMy*() may return https_port info, but we force http URIs
> ++    // because we have not checked whether the callers can handle https.
> ++    const bool secure = false;
> ++    return internalRemoteUri(secure, getMyHostname(),
> +                              getMyPort(), dir, name);
> + }
> + 
> +diff --git a/src/internal.h b/src/internal.h
> +index c91f9ac..13a43a6 100644
> +--- a/src/internal.h
> ++++ b/src/internal.h
> +@@ -24,7 +24,7 @@ void internalStart(const Comm::ConnectionPointer &clientConn, HttpRequest *, Sto
> + bool internalCheck(const SBuf &urlPath);
> + bool internalStaticCheck(const SBuf &urlPath);
> + char *internalLocalUri(const char *dir, const SBuf &name);
> +-char *internalRemoteUri(const char *, unsigned short, const char *, const SBuf &);
> ++char *internalRemoteUri(bool, const char *, unsigned short, const char *, const SBuf &);
> + const char *internalHostname(void);
> + int internalHostnameIs(const char *);
> + 
> +diff --git a/src/peer_digest.cc b/src/peer_digest.cc
> +index 36a8705..f515aaa 100644
> +--- a/src/peer_digest.cc
> ++++ b/src/peer_digest.cc
> +@@ -323,7 +323,7 @@ peerDigestRequest(PeerDigest * pd)
> +     if (p->digest_url)
> +         url = xstrdup(p->digest_url);
> +     else
> +-        url = xstrdup(internalRemoteUri(p->host, p->http_port, "/squid-internal-periodic/", SBuf(StoreDigestFileName)));
> ++        url = xstrdup(internalRemoteUri(p->secure.encryptTransport, p->host, p->http_port, "/squid-internal-periodic/", SBuf(StoreDigestFileName)));
> +     debugs(72, 2, url);
> + 
> +     const MasterXaction::Pointer mx = new MasterXaction(XactionInitiator::initCacheDigest);
> diff --git a/src/patches/squid/02_Maintenance_add_xz_tarball_format_formally_to_make_dist_325.patch b/src/patches/squid/02_Maintenance_add_xz_tarball_format_formally_to_make_dist_325.patch
> new file mode 100644
> index 000000000..58ceaa034
> --- /dev/null
> +++ b/src/patches/squid/02_Maintenance_add_xz_tarball_format_formally_to_make_dist_325.patch
> @@ -0,0 +1,22 @@
> +commit 3c23ae8c7431344f8fc50bb5ee8f4b56d08c10a4
> +Author: Amos Jeffries <yadij(a)users.noreply.github.com>
> +Date:   2018-11-11 04:29:58 +0000
> +
> +    Maintenance: add .xz tarball format formally to make dist (#325)
> +    
> +    Automake can now handle generating this format itself and the
> +    experiments of providing it for downstream have gone well.
> +
> +diff --git a/configure.ac b/configure.ac
> +index 3f8af6d..f668567 100644
> +--- a/configure.ac
> ++++ b/configure.ac
> +@@ -10,7 +10,7 @@ AC_PREREQ(2.61)
> + AC_CONFIG_HEADERS([include/autoconf.h])
> + AC_CONFIG_AUX_DIR(cfgaux)
> + AC_CONFIG_SRCDIR([src/main.cc])
> +-AM_INIT_AUTOMAKE([tar-ustar nostdinc subdir-objects])
> ++AM_INIT_AUTOMAKE([tar-ustar nostdinc subdir-objects dist-xz])
> + AC_REVISION($Revision$)dnl
> + AC_PREFIX_DEFAULT(/usr/local/squid)
> + AM_MAINTAINER_MODE
> diff --git a/src/patches/squid/03_The_handshake_logformat_code_331.patch b/src/patches/squid/03_The_handshake_logformat_code_331.patch
> new file mode 100644
> index 000000000..2ce8bdc4a
> --- /dev/null
> +++ b/src/patches/squid/03_The_handshake_logformat_code_331.patch
> @@ -0,0 +1,132 @@
> +commit 0022167d80725513d95b38aaebc90086fc0b6938 (tag: refs/tags/M-staged-PR331, refs/remotes/origin/v4)
> +Author: Christos Tsantilas <christos(a)chtsanti.net>
> +Date:   2018-11-14 15:17:06 +0000
> +
> +    The %>handshake logformat code (#331)
> +    
> +    Logging client "handshake" bytes is useful in at least two contexts:
> +    
> +    * Runtime traffic bypass and bumping/splicing decisions. Identifying
> +      popular clients like Skype for Business (that uses a TLS handshake but
> +      then may not speak TLS) is critical for handling their traffic
> +      correctly. Squid does not have enough ACLs to interrogate most TLS
> +      handshake aspects. Adding more ACLs may still be a good idea, but
> +      initial sketches for SfB handshakes showed rather complex
> +      ACLs/configurations, _and_ no reasonable ACLs would be able to handle
> +      non-TLS handshakes. An external ACL receiving the handshake is in a
> +      much better position to analyze/fingerprint it according to custom
> +      admin needs.
> +    
> +    * A logged handshake can be used to analyze new/unusual traffic or even
> +      trigger security-related alarms.
> +    
> +    The current support is limited to cases where Squid was saving handshake
> +    for other reasons. With enough demand, this initial support can be
> +    extended to all protocols and port configurations.
> +    
> +    This is a Measurement Factory project.
> +
> +diff --git a/src/cf.data.pre b/src/cf.data.pre
> +index fa8af56..a8ca587 100644
> +--- a/src/cf.data.pre
> ++++ b/src/cf.data.pre
> +@@ -4394,6 +4394,37 @@ DOC_START
> + 		<qos	Server connection TOS/DSCP value set by Squid
> + 		<nfmark Server connection netfilter mark set by Squid
> + 
> ++		>handshake Raw client handshake
> ++			Initial client bytes received by Squid on a newly
> ++			accepted TCP connection or inside a just established
> ++			CONNECT tunnel. Squid stops accumulating handshake
> ++			bytes as soon as the handshake parser succeeds or
> ++			fails (determining whether the client is using the
> ++			expected protocol).
> ++
> ++			For HTTP clients, the handshake is the request line.
> ++			For TLS clients, the handshake consists of all TLS
> ++			records up to and including the TLS record that
> ++			contains the last byte of the first ClientHello
> ++			message. For clients using an unsupported protocol,
> ++			this field contains the bytes received by Squid at the
> ++			time of the handshake parsing failure.
> ++
> ++			See the on_unsupported_protocol directive for more
> ++			information on Squid handshake traffic expectations.
> ++
> ++			Current support is limited to these contexts:
> ++			- http_port connections, but only when the
> ++			  on_unsupported_protocol directive is in use.
> ++			- https_port connections (and CONNECT tunnels) that
> ++			  are subject to the ssl_bump peek or stare action.
> ++
> ++			To protect binary handshake data, this field is always
> ++			base64-encoded (RFC 4648 Section 4). If logformat
> ++			field encoding is configured, that encoding is applied
> ++			on top of base64. Otherwise, the computed base64 value
> ++			is recorded as is.
> ++
> + 	Time related format codes:
> + 
> + 		ts	Seconds since epoch
> +diff --git a/src/format/ByteCode.h b/src/format/ByteCode.h
> +index ad230bb..a6f8fd9 100644
> +--- a/src/format/ByteCode.h
> ++++ b/src/format/ByteCode.h
> +@@ -46,6 +46,8 @@ typedef enum {
> +     LFT_CLIENT_LOCAL_TOS,
> +     LFT_CLIENT_LOCAL_NFMARK,
> + 
> ++    LFT_CLIENT_HANDSHAKE,
> ++
> +     /* client connection local squid.conf details */
> +     LFT_LOCAL_LISTENING_IP,
> +     LFT_LOCAL_LISTENING_PORT,
> +diff --git a/src/format/Format.cc b/src/format/Format.cc
> +index c1e19b4..8fd6720 100644
> +--- a/src/format/Format.cc
> ++++ b/src/format/Format.cc
> +@@ -8,6 +8,7 @@
> + 
> + #include "squid.h"
> + #include "AccessLogEntry.h"
> ++#include "base64.h"
> + #include "client_side.h"
> + #include "comm/Connection.h"
> + #include "err_detail_type.h"
> +@@ -547,6 +548,24 @@ Format::Format::assemble(MemBuf &mb, const AccessLogEntry::Pointer &al, int logS
> +             }
> +             break;
> + 
> ++        case LFT_CLIENT_HANDSHAKE:
> ++            if (al->request && al->request->clientConnectionManager.valid()) {
> ++                const auto &handshake = al->request->clientConnectionManager->preservedClientData;
> ++                if (const auto rawLength = handshake.length()) {
> ++                    // add 1 byte to optimize the c_str() conversion below
> ++                    char *buf = sb.rawAppendStart(base64_encode_len(rawLength) + 1);
> ++
> ++                    struct base64_encode_ctx ctx;
> ++                    base64_encode_init(&ctx);
> ++                    auto encLength = base64_encode_update(&ctx, buf, rawLength, reinterpret_cast<const uint8_t*>(handshake.rawContent()));
> ++                    encLength += base64_encode_final(&ctx, buf + encLength);
> ++
> ++                    sb.rawAppendFinish(buf, encLength);
> ++                    out = sb.c_str();
> ++                }
> ++            }
> ++            break;
> ++
> +         case LFT_TIME_SECONDS_SINCE_EPOCH:
> +             // some platforms store time in 32-bit, some 64-bit...
> +             outoff = static_cast<int64_t>(current_time.tv_sec);
> +diff --git a/src/format/Token.cc b/src/format/Token.cc
> +index 186ade5..06c60cf 100644
> +--- a/src/format/Token.cc
> ++++ b/src/format/Token.cc
> +@@ -141,6 +141,7 @@ static TokenTableEntry TokenTableMisc[] = {
> +     TokenTableEntry("<qos", LFT_SERVER_LOCAL_TOS),
> +     TokenTableEntry(">nfmark", LFT_CLIENT_LOCAL_NFMARK),
> +     TokenTableEntry("<nfmark", LFT_SERVER_LOCAL_NFMARK),
> ++    TokenTableEntry(">handshake", LFT_CLIENT_HANDSHAKE),
> +     TokenTableEntry("err_code", LFT_SQUID_ERROR ),
> +     TokenTableEntry("err_detail", LFT_SQUID_ERROR_DETAIL ),
> +     TokenTableEntry("note", LFT_NOTE ),
> -- 
> 2.18.0
>