From 4f6990ec5d29c7d0aa25ee75304ca400c11e2c6e Mon Sep 17 00:00:00 2001 From: Christos Tsantilas Date: Mon, 12 Jun 2017 18:40:16 +0300 Subject: [PATCH] ssl::server_name options to control matching logic. Many popular servers use certificates with several "alternative subject names" (SubjectAltName). Many of those names are wildcards. For example, a www.youtube.com certificate currently includes *.google.com and 50+ other subject names, most of which are wildcards. Often, admins want server_name to match any of the subject names. This is useful to match any server belonging to a large conglomerate of companies, all including some *.example.com name in their certificates. The existing server_name functionality addresses this use case well. The new ACL options address several other important use cases: --consensus identifies transactions with a particular server when server's subject name is also present in certificates used by many other servers (e.g., matching transactions with a particular Google server but not with all Youtube servers). --client-requested allows both (a) SNI-based matching even after Squid obtains the server certificate and (b) pinpointing a particular server in a group of different servers all using the same wildcard certificate (e.g., matching appengine.example.com but not www.example.com when the certificate for has *.example.com subject). --server-provided allows matching only after Squid obtains the server certificate and matches any of the conglomerate parts. Also this patch fixes squid to log client SNI when client-first bumping mode is used too. This is a Measurement Factory project. --- src/acl/ServerName.cc | 69 +++++++++++++++++++++++++++++++++++-------- src/acl/ServerName.h | 6 ++++ src/cf.data.pre | 50 ++++++++++++++++++++++++------- src/client_side.cc | 7 ++--- src/client_side.h | 4 +++ src/format/Format.cc | 8 +++-- src/ssl/ServerBump.h | 1 - 7 files changed, 114 insertions(+), 31 deletions(-) diff --git a/src/acl/ServerName.cc b/src/acl/ServerName.cc index 15d02e9c36..fc020fa4ef 100644 --- a/src/acl/ServerName.cc +++ b/src/acl/ServerName.cc @@ -92,21 +92,30 @@ ACLServerNameStrategy::match (ACLData * &data, ACLFilledChecklist *ch assert(checklist != NULL && checklist->request != NULL); const char *serverName = nullptr; - SBuf serverNameKeeper; // because c_str() is not constant + SBuf clientSniKeeper; // because c_str() is not constant if (ConnStateData *conn = checklist->conn()) { - - if (conn->serverBump()) { - if (X509 *peer_cert = conn->serverBump()->serverCert.get()) - return Ssl::matchX509CommonNames(peer_cert, (void *)data, check_cert_domain); - } - - if (conn->sslCommonName().isEmpty()) { + const char *clientRequestedServerName = nullptr; + clientSniKeeper = conn->tlsClientSni(); + if (clientSniKeeper.isEmpty()) { const char *host = checklist->request->url.host(); if (host && *host) // paranoid first condition: host() is never nil - serverName = host; - } else { - serverNameKeeper = conn->sslCommonName(); - serverName = serverNameKeeper.c_str(); + clientRequestedServerName = host; + } else + clientRequestedServerName = clientSniKeeper.c_str(); + + if (useConsensus) { + X509 *peer_cert = conn->serverBump() ? conn->serverBump()->serverCert.get() : nullptr; + // use the client requested name if it matches the server + // certificate or if the certificate is not available + if (!peer_cert || Ssl::checkX509ServerValidity(peer_cert, clientRequestedServerName)) + serverName = clientRequestedServerName; + } else if (useClientRequested) + serverName = clientRequestedServerName; + else { // either no options or useServerProvided + if (X509 *peer_cert = (conn->serverBump() ? conn->serverBump()->serverCert.get() : nullptr)) + return Ssl::matchX509CommonNames(peer_cert, (void *)data, check_cert_domain); + if (!useServerProvided) + serverName = clientRequestedServerName; } } @@ -116,3 +125,39 @@ ACLServerNameStrategy::match (ACLData * &data, ACLFilledChecklist *ch return data->match(serverName); } +const Acl::Options & +ACLServerNameStrategy::options() +{ + static const Acl::BooleanOption ClientRequested; + static const Acl::BooleanOption ServerProvided; + static const Acl::BooleanOption Consensus; + static const Acl::Options MyOptions = { + {"--client-requested", &ClientRequested}, + {"--server-provided", &ServerProvided}, + {"--consensus", &Consensus} + }; + + ClientRequested.linkWith(&useClientRequested); + ServerProvided.linkWith(&useServerProvided); + Consensus.linkWith(&useConsensus); + return MyOptions; +} + +bool +ACLServerNameStrategy::valid() const +{ + int optionCount = 0; + + if (useClientRequested) + optionCount++; + if (useServerProvided) + optionCount++; + if (useConsensus) + optionCount++; + + if (optionCount > 1) { + debugs(28, DBG_CRITICAL, "ERROR: Multiple options given for the server_name ACL"); + return false; + } + return true; +} diff --git a/src/acl/ServerName.h b/src/acl/ServerName.h index 32af2dbd8a..fe4b9c8d0d 100644 --- a/src/acl/ServerName.h +++ b/src/acl/ServerName.h @@ -28,7 +28,13 @@ public: /* ACLStrategy API */ virtual int match (ACLData * &, ACLFilledChecklist *); virtual bool requiresRequest() const {return true;} + virtual const Acl::Options &options(); + virtual bool valid() const; +private: + Acl::BooleanOptionValue useClientRequested; ///< Ignore server-supplied names + Acl::BooleanOptionValue useServerProvided; ///< Ignore client-supplied names + Acl::BooleanOptionValue useConsensus; ///< Ignore mismatching names }; #endif /* SQUID_ACLSERVERNAME_H */ diff --git a/src/cf.data.pre b/src/cf.data.pre index 40b5fe75ea..8a05095f8d 100644 --- a/src/cf.data.pre +++ b/src/cf.data.pre @@ -1368,17 +1368,47 @@ IF USE_OPENSSL # SslBump2: After getting SSL Client Hello info. # SslBump3: After getting SSL Server Hello info. - acl aclname ssl::server_name .foo.com ... + acl aclname ssl::server_name [option] .foo.com ... # matches server name obtained from various sources [fast] # - # The server name is obtained during Ssl-Bump steps from such sources - # as CONNECT request URI, client SNI, and SSL server certificate CN. - # During each Ssl-Bump step, Squid may improve its understanding of a - # "true server name". Unlike dstdomain, this ACL does not perform - # DNS lookups. - # The "none" name can be used to match transactions where Squid + # The ACL computes server name(s) using such information sources as + # CONNECT request URI, TLS client SNI, and TLS server certificate + # subject (CN and SubjectAltName). The computed server name(s) usually + # change with each SslBump step, as more info becomes available: + # * SNI is used as the server name instead of the request URI, + # * subject name(s) from the server certificate (CN and + # SubjectAltName) are used as the server names instead of SNI. + # + # When the ACL computes multiple server names, matching any single + # computed name is sufficient for the ACL to match. + # + # The "none" name can be used to match transactions where the ACL # could not compute the server name using any information source - # already available at the ACL evaluation time. + # that was both available and allowed to be used by the ACL options at + # the ACL evaluation time. + # + # Unlike dstdomain, this ACL does not perform DNS lookups. + # + # An ACL option below may be used to restrict what information + # sources are used to extract the server names from: + # + # --client-requested + # The server name is SNI regardless of what the server says. + # --server-provided + # The server name(s) are the certificate subject name(s), regardless + # of what the client has requested. If the server certificate is + # unavailable, then the name is "none". + # --consensus + # The server name is either SNI (if SNI matches at least one of the + # certificate subject names) or "none" (otherwise). When the server + # certificate is unavailable, the consensus server name is SNI. + # + # Combining multiple options in one ACL is a fatal configuration + # error. + # + # For all options: If no SNI is available, then the CONNECT request + # target (a.k.a. URI) is used instead of SNI (for an intercepted + # connection, this target is the destination IP address). acl aclname ssl::server_name_regex [-i] \.foo\.com ... # regex matches server name obtained from various sources [fast] @@ -4451,9 +4481,7 @@ DOC_START In all other cases, a single dash ("-") is logged. - ssl::>sni SSL client SNI sent to Squid. Available only - after the peek, stare, or splice SSL bumping - actions. + ssl::>sni SSL client SNI sent to Squid. ssl::>cert_subject The Subject field of the received client diff --git a/src/client_side.cc b/src/client_side.cc index caceb6f99c..c89a9257b6 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -3158,8 +3158,7 @@ ConnStateData::parseTlsHandshake() clientConnection->tlsNegotiations()->retrieveParsedInfo(details); if (details && !details->serverName.isEmpty()) { resetSslCommonName(details->serverName.c_str()); - if (sslServerBump) - sslServerBump->clientSni = details->serverName; + tlsClientSni_ = details->serverName; } // We should disable read/write handlers @@ -3392,8 +3391,8 @@ ConnStateData::fakeAConnectRequest(const char *reason, const SBuf &payload) const unsigned short connectPort = clientConnection->local.port(); #if USE_OPENSSL - if (serverBump() && !serverBump()->clientSni.isEmpty()) - connectHost.assign(serverBump()->clientSni); + if (!tlsClientSni_.isEmpty()) + connectHost.assign(tlsClientSni_); else #endif { diff --git a/src/client_side.h b/src/client_side.h index d6486dfba7..fbb33225d6 100644 --- a/src/client_side.h +++ b/src/client_side.h @@ -244,6 +244,7 @@ public: } const SBuf &sslCommonName() const {return sslCommonName_;} void resetSslCommonName(const char *name) {sslCommonName_ = name;} + const SBuf &tlsClientSni() const { return tlsClientSni_; } /// Fill the certAdaptParams with the required data for certificate adaptation /// and create the key for storing/retrieve the certificate to/from the cache void buildSslCertGenerationParams(Ssl::CertificateProperties &certProperties); @@ -373,6 +374,9 @@ private: /// The SSL server host name appears in CONNECT request or the server ip address for the intercepted requests String sslConnectHostOrIp; ///< The SSL server host name as passed in the CONNECT request SBuf sslCommonName_; ///< CN name for SSL certificate generation + + /// TLS client delivered SNI value. Empty string if none has been received. + SBuf tlsClientSni_; String sslBumpCertKey; ///< Key to use to store/retrieve generated certificate /// HTTPS server cert. fetching state for bump-ssl-server-first diff --git a/src/format/Format.cc b/src/format/Format.cc index 42ad1a0c77..152a2a85e7 100644 --- a/src/format/Format.cc +++ b/src/format/Format.cc @@ -1236,9 +1236,11 @@ Format::Format::assemble(MemBuf &mb, const AccessLogEntry::Pointer &al, int logS case LFT_SSL_CLIENT_SNI: if (al->request && al->request->clientConnectionManager.valid()) { - if (Ssl::ServerBump * srvBump = al->request->clientConnectionManager->serverBump()) { - if (!srvBump->clientSni.isEmpty()) - out = srvBump->clientSni.c_str(); + if (const ConnStateData *conn = al->request->clientConnectionManager.get()) { + if (!conn->tlsClientSni().isEmpty()) { + sb = conn->tlsClientSni(); + out = sb.c_str(); + } } } break; diff --git a/src/ssl/ServerBump.h b/src/ssl/ServerBump.h index 3efda035cf..2bb866f302 100644 --- a/src/ssl/ServerBump.h +++ b/src/ssl/ServerBump.h @@ -47,7 +47,6 @@ public: Ssl::BumpMode step3; ///< The SSL bump mode at step3 } act; ///< bumping actions at various bumping steps Ssl::BumpStep step; ///< The SSL bumping step - SBuf clientSni; ///< the SSL client SNI name private: Security::SessionPointer serverSession; ///< The TLS session object on server side. -- 2.47.2