diff --git a/CONTRIBUTORS b/CONTRIBUTORS index 39bed1dc008..72f038730a6 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -521,6 +521,7 @@ Thank you! Tomas Hozza tomofumi-yoshida <51390036+tomofumi-yoshida@users.noreply.github.com> Tony Lorimer + Tony Walker trapexit Trever Adams Tsantilas Christos diff --git a/src/acl/ServerName.cc b/src/acl/ServerName.cc index f48629dddca..0659a096603 100644 --- a/src/acl/ServerName.cc +++ b/src/acl/ServerName.cc @@ -11,9 +11,11 @@ #include "squid.h" #include "acl/FilledChecklist.h" #include "acl/ServerName.h" +#include "anyp/Host.h" #include "client_side.h" #include "http/Stream.h" #include "HttpRequest.h" +#include "sbuf/Stream.h" #include "ssl/bio.h" #include "ssl/ServerBump.h" #include "ssl/support.h" @@ -45,31 +47,50 @@ ACLServerNameData::match(const char *host) } -/// A helper function to be used with Ssl::matchX509CommonNames(). -/// \retval 0 when the name (cn or an alternate name) matches acl data -/// \retval 1 when the name does not match -template -int -check_cert_domain( void *check_data, ASN1_STRING *cn_data) +namespace Acl { + +/// GeneralNameMatcher for matching configured ACL parameters +class ServerNameMatcher: public Ssl::GeneralNameMatcher { - char cn[1024]; - ACLData * data = (ACLData *)check_data; - - if (cn_data->length > (int)sizeof(cn) - 1) - return 1; // ignore data that does not fit our buffer - - char *s = reinterpret_cast(cn_data->data); - char *d = cn; - for (int i = 0; i < cn_data->length; ++i, ++d, ++s) { - if (*s == '\0') - return 1; // always a domain mismatch. contains 0x00 - *d = *s; - } - cn[cn_data->length] = '\0'; - debugs(28, 4, "Verifying certificate name/subjectAltName " << cn); - if (data->match(cn)) - return 0; - return 1; +public: + explicit ServerNameMatcher(ServerNameCheck::Parameters &p): parameters(p) {} + +protected: + /* GeneralNameMatcher API */ + bool matchDomainName(const Dns::DomainName &) const override; + bool matchIp(const Ip::Address &) const override; + +private: + // TODO: Make ServerNameCheck::Parameters::match() and this reference constant. + ServerNameCheck::Parameters ¶meters; ///< configured ACL parameters +}; + +} // namespace Acl + +bool +Acl::ServerNameMatcher::matchDomainName(const Dns::DomainName &domain) const +{ + return parameters.match(SBuf(domain).c_str()); // TODO: Upgrade string-matching ACLs to SBuf +} + +bool +Acl::ServerNameMatcher::matchIp(const Ip::Address &ip) const +{ + // We are given an Ip::Address, but our ACL parameters use case-insensitive + // string equality (::matchDomainName) or regex string matches. There are + // many ways to convert an IPv6 address to a string, but only one format can + // correctly match certain configured parameters. Our ssl::server_name docs + // request the following ACL parameter formatting (that this to-string + // conversion code produces): IPv6 addresses use "::" notation (where + // applicable) and are not bracketed. + // + // Similar problems affect dstdomain ACLs. TODO: Instead of relying on users + // reading docs and following their inet_ntop(3) implementation to match + // IPv6 addresses handled by matchDomainName(), enhance matchDomainName() + // code and ACL parameter storage to support Ip::Address objects. + char hostStr[MAX_IPSTRLEN]; + (void)ip.toStr(hostStr, sizeof(hostStr)); // no brackets + return parameters.match(hostStr); } int @@ -79,37 +100,41 @@ Acl::ServerNameCheck::match(ACLChecklist * const ch) assert(checklist != nullptr && checklist->request != nullptr); - const char *serverName = nullptr; - SBuf clientSniKeeper; // because c_str() is not constant + std::optional serverNameFromConn; if (ConnStateData *conn = checklist->conn()) { - 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 - clientRequestedServerName = host; - } else - clientRequestedServerName = clientSniKeeper.c_str(); + std::optional clientRequestedServerName; + const auto &clientSni = conn->tlsClientSni(); + if (clientSni.isEmpty()) { + clientRequestedServerName = checklist->request->url.parsedHost(); + } else { + // RFC 6066: "The hostname is represented as a byte string using + // ASCII encoding"; "Literal IPv4 and IPv6 addresses are not + // permitted". TODO: Store TlsDetails::serverName and similar + // domains using a new domain-only type instead of SBuf. + clientRequestedServerName = AnyP::Host::ParseSimpleDomainName(clientSni); + } 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; + if (!peer_cert || !clientRequestedServerName || + Ssl::HasSubjectName(*peer_cert, *clientRequestedServerName)) + serverNameFromConn = clientRequestedServerName; } else if (useClientRequested) - serverName = clientRequestedServerName; + serverNameFromConn = clientRequestedServerName; else { // either no options or useServerProvided if (X509 *peer_cert = (conn->serverBump() ? conn->serverBump()->serverCert.get() : nullptr)) - return Ssl::matchX509CommonNames(peer_cert, data.get(), check_cert_domain); + return Ssl::HasMatchingSubjectName(*peer_cert, ServerNameMatcher(*data)); if (!useServerProvided) - serverName = clientRequestedServerName; + serverNameFromConn = clientRequestedServerName; } } - if (!serverName) - serverName = "none"; - + std::optional printedServerName; + if (serverNameFromConn) + printedServerName = ToSBuf(*serverNameFromConn); // no brackets + const auto serverName = printedServerName ? printedServerName->c_str() : "none"; return data->match(serverName); } diff --git a/src/anyp/Host.cc b/src/anyp/Host.cc new file mode 100644 index 00000000000..ed1549981c5 --- /dev/null +++ b/src/anyp/Host.cc @@ -0,0 +1,113 @@ +/* + * Copyright (C) 1996-2023 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 "anyp/Host.h" + +#include + +std::optional +AnyP::Host::ParseIp(const Ip::Address &ip) +{ + // any preparsed IP value is acceptable + debugs(23, 7, ip); + return Host(ip); +} + +/// common parts of FromSimpleDomain() and FromWildDomain() +std::optional +AnyP::Host::ParseDomainName(const SBuf &rawName) +{ + if (rawName.isEmpty()) { + debugs(23, 3, "rejecting empty name"); + return std::nullopt; + } + + // Reject bytes incompatible with rfc1035NamePack() and ::matchDomainName() + // implementations (at least). Such bytes can come from percent-encoded HTTP + // URIs or length-based X.509 fields, for example. Higher-level parsers must + // reject or convert domain name encodings like UTF-16, but this low-level + // check works as an additional (albeit unreliable) layer of defense against + // those (unsupported by Squid DNS code) encodings. + if (rawName.find('\0') != SBuf::npos) { + debugs(83, 3, "rejecting ASCII NUL character in " << rawName); + return std::nullopt; + } + + // TODO: Consider rejecting names with isspace(3) bytes. + + debugs(23, 7, rawName); + return Host(rawName); +} + +std::optional +AnyP::Host::ParseSimpleDomainName(const SBuf &rawName) +{ + if (rawName.find('*') != SBuf::npos) { + debugs(23, 3, "rejecting wildcard in " << rawName); + return std::nullopt; + } + return ParseDomainName(rawName); +} + +std::optional +AnyP::Host::ParseWildDomainName(const SBuf &rawName) +{ + const static SBuf wildcardLabel("*."); + if (rawName.startsWith(wildcardLabel)) { + if (rawName.find('*', 2) != SBuf::npos) { + debugs(23, 3, "rejecting excessive wildcards in " << rawName); + return std::nullopt; + } + // else: fall through to final checks + } else { + if (rawName.find('*', 0) != SBuf::npos) { + // this case includes "*" and "example.*" input + debugs(23, 3, "rejecting unsupported wildcard in " << rawName); + return std::nullopt; + } + // else: fall through to final checks + } + return ParseDomainName(rawName); +} + +std::ostream & +AnyP::operator <<(std::ostream &os, const Host &host) +{ + if (const auto ip = host.ip()) { + char buf[MAX_IPSTRLEN]; + (void)ip->toStr(buf, sizeof(buf)); // no brackets + os << buf; + } else { + // If Host object creators start applying Uri::Decode() to reg-names, + // then we must start applying Uri::Encode() here, but only to names + // that require it. See "The reg-name syntax allows percent-encoded + // octets" paragraph in RFC 3986. + const auto domainName = host.domainName(); + Assure(domainName); + os << *domainName; + } + return os; +} + +std::ostream & +AnyP::operator <<(std::ostream &os, const Bracketed &hostWrapper) +{ + bool addBrackets = false; + if (const auto ip = hostWrapper.host.ip()) + addBrackets = ip->isIPv6(); + + if (addBrackets) + os << '['; + os << hostWrapper.host; + if (addBrackets) + os << ']'; + + return os; +} + diff --git a/src/anyp/Host.h b/src/anyp/Host.h new file mode 100644 index 00000000000..b3f5018db7d --- /dev/null +++ b/src/anyp/Host.h @@ -0,0 +1,81 @@ +/* + * Copyright (C) 1996-2023 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. + */ + +#ifndef SQUID_SRC_ANYP_HOST_H +#define SQUID_SRC_ANYP_HOST_H + +#include "dns/forward.h" +#include "ip/Address.h" +#include "sbuf/SBuf.h" + +#include +#include +#include + +namespace AnyP +{ + +/// either a domain name (as defined in DNS RFC 1034) or an IP address +class Host +{ +public: + /// converts an already parsed IP address to a Host object + static std::optional ParseIp(const Ip::Address &); + + /// Parses input as a literal ASCII domain name (A-labels OK; see RFC 5890). + /// Does not allow wildcards; \sa ParseWildDomainName(). + static std::optional ParseSimpleDomainName(const SBuf &); + + /// Same as ParseSimpleDomainName() but allows the first label to be a + /// wildcard (RFC 9525 Section 6.3). + static std::optional ParseWildDomainName(const SBuf &); + + // Accessor methods below are mutually exclusive: Exactly one method is + // guaranteed to return a result other than std::nullopt. + + /// stored IPv or IPv6 address (if any) + /// + /// Ip::Address::isNoAddr() may be true for the returned address. + /// Ip::Address::isAnyAddr() may be true for the returned address. + auto ip() const { return std::get_if(&raw_); } + + /// stored domain name (if any) + auto domainName() const { return std::get_if(&raw_); } + +private: + using Storage = std::variant; + + static std::optional ParseDomainName(const SBuf &); + + // use a Parse*() function to create Host objects + Host(const Storage &raw): raw_(raw) {} + + Storage raw_; ///< the host we are providing access to +}; + +/// helps print Host value in RFC 3986 Section 3.2.2 format, with square +/// brackets around an IPv6 address (if the Host value is an IPv6 address) +class Bracketed +{ +public: + explicit Bracketed(const Host &aHost): host(aHost) {} + const Host &host; +}; + +/// prints Host value _without_ square brackets around an IPv6 address (even +/// when the Host value is an IPv6 address); \sa Bracketed +std::ostream &operator <<(std::ostream &, const Host &); + +/// prints Host value _without_ square brackets around an IPv6 address (even +/// when the Host value is an IPv6 address); \sa Bracketed +std::ostream &operator <<(std::ostream &, const Bracketed &); + +} // namespace Anyp + +#endif /* SQUID_SRC_ANYP_HOST_H */ + diff --git a/src/anyp/Makefile.am b/src/anyp/Makefile.am index 62c720e2d5e..14363e01958 100644 --- a/src/anyp/Makefile.am +++ b/src/anyp/Makefile.am @@ -10,6 +10,8 @@ include $(top_srcdir)/src/Common.am noinst_LTLIBRARIES = libanyp.la libanyp_la_SOURCES = \ + Host.cc \ + Host.h \ PortCfg.cc \ PortCfg.h \ ProtocolType.cc \ diff --git a/src/anyp/Uri.cc b/src/anyp/Uri.cc index 590074491dd..a5ab45cd03f 100644 --- a/src/anyp/Uri.cc +++ b/src/anyp/Uri.cc @@ -9,6 +9,7 @@ /* DEBUG: section 23 URL Parsing */ #include "squid.h" +#include "anyp/Host.h" #include "anyp/Uri.h" #include "base/Raw.h" #include "globals.h" @@ -133,6 +134,7 @@ AnyP::Uri::host(const char *src) touch(); } +// TODO: Replace with ToSBuf(parsedHost()) or similar. SBuf AnyP::Uri::hostOrIp() const { @@ -144,6 +146,25 @@ AnyP::Uri::hostOrIp() const return SBuf(host()); } +std::optional +AnyP::Uri::parsedHost() const +{ + if (hostIsNumeric()) + return Host::ParseIp(hostIP()); + + // XXX: Interpret host subcomponent as reg-name representing a DNS name. It + // may actually be, for example, a URN namespace ID (NID; see RFC 8141), but + // current Squid APIs do not support adequate representation of those cases. + const SBuf regName(host()); + + if (regName.find('%') != SBuf::npos) { + debugs(23, 3, "rejecting percent-encoded reg-name: " << regName); + return std::nullopt; // TODO: Decode() instead + } + + return Host::ParseSimpleDomainName(regName); +} + const SBuf & AnyP::Uri::path() const { diff --git a/src/anyp/Uri.h b/src/anyp/Uri.h index 81090e63cd8..863c9db374b 100644 --- a/src/anyp/Uri.h +++ b/src/anyp/Uri.h @@ -9,6 +9,7 @@ #ifndef SQUID_SRC_ANYP_URI_H #define SQUID_SRC_ANYP_URI_H +#include "anyp/forward.h" #include "anyp/UriScheme.h" #include "ip/Address.h" #include "rfc2181.h" @@ -76,6 +77,10 @@ class Uri int hostIsNumeric(void) const {return hostIsNumeric_;} Ip::Address const & hostIP(void) const {return hostAddr_;} + /// Successfully interpreted non-empty host subcomponent of the authority + /// component (if any). XXX: Remove hostOrIp() and print Host instead. + std::optional parsedHost() const; + /// \returns the host subcomponent of the authority component /// If the host is an IPv6 address, returns that IP address with /// [brackets]. See RFC 3986 Section 3.2.2. diff --git a/src/anyp/forward.h b/src/anyp/forward.h index b7f03e6f075..8c81bd71552 100644 --- a/src/anyp/forward.h +++ b/src/anyp/forward.h @@ -17,6 +17,8 @@ namespace AnyP class PortCfg; typedef RefCount PortCfgPointer; +class Bracketed; +class Host; class Uri; class UriScheme; diff --git a/src/cf.data.pre b/src/cf.data.pre index 1c0d2583738..fbd74460399 100644 --- a/src/cf.data.pre +++ b/src/cf.data.pre @@ -1609,6 +1609,12 @@ IF USE_OPENSSL # # Unlike dstdomain, this ACL does not perform DNS lookups. # + # A server name may be an IP address. For example, subject alternative + # names (a.k.a. SANs) in some real server certificates include IPv4 and + # IPv6 entries. Internally, Squid uses inet_ntop(3) to prep IP names for + # matching. When using IPv6 names, use "::" notation (if applicable). + # Do not use brackets. For example: 1080::8:800:200c:417a. + # # An ACL option below may be used to restrict what information # sources are used to extract the server names from: # @@ -1632,6 +1638,10 @@ IF USE_OPENSSL acl aclname ssl::server_name_regex [-i] \.foo\.com ... # regex matches server name obtained from various sources [fast] + # + # See ssl::server_name for details, including IPv6 address formatting + # caveats. Use case-insensitive matching (i.e. -i option) to reduce + # dependency on how Squid formats or sanitizes server names. acl aclname connections_encrypted # matches transactions with all HTTP messages received over TLS diff --git a/src/client_side.cc b/src/client_side.cc index 2ec12ddca1f..deb64d19ea7 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -59,6 +59,7 @@ #include "squid.h" #include "acl/FilledChecklist.h" +#include "anyp/Host.h" #include "anyp/PortCfg.h" #include "base/AsyncCallbacks.h" #include "base/Subscription.h" @@ -1470,9 +1471,13 @@ bool ConnStateData::serveDelayedError(Http::Stream *context) // when we can extract the intended name from the bumped HTTP request. if (const Security::CertPointer &srvCert = sslServerBump->serverCert) { HttpRequest *request = http->request; - if (!Ssl::checkX509ServerValidity(srvCert.get(), request->url.host())) { + const auto host = request->url.parsedHost(); + if (host && Ssl::HasSubjectName(*srvCert, *host)) { + debugs(33, 5, "certificate matches requested host: " << *host); + return false; + } else { debugs(33, 2, "SQUID_X509_V_ERR_DOMAIN_MISMATCH: Certificate " << - "does not match domainname " << request->url.host()); + "does not match request target " << RawPointer(host)); bool allowDomainMismatch = false; if (Config.ssl_client.cert_error) { diff --git a/src/dns/forward.h b/src/dns/forward.h index 0a0f57f804c..1ebbb8a59c3 100644 --- a/src/dns/forward.h +++ b/src/dns/forward.h @@ -10,6 +10,7 @@ #define SQUID_SRC_DNS_FORWARD_H #include "ip/forward.h" +#include "sbuf/forward.h" class rfc1035_rr; @@ -23,6 +24,22 @@ class LookupDetails; void Init(void); +/// A DNS domain name as described in RFC 1034 and RFC 1035. +/// +/// The object creator is responsible for removing any encodings (e.g., URI +/// percent-encoding) other than ASCII Compatible Encoding (ACE; RFC 5890) prior +/// to creating a DomainName object. Domain names are stored as dot-separated +/// ASCII substrings, with each substring representing a domain name label. +/// DomainName strings are suitable for creating DNS queries and byte-by-byte +/// case-insensitive comparison with configured dstdomain ACL parameters. +/// +/// Even though an empty domain name is valid in DNS, DomainName objects are +/// never empty. +/// +/// The first label of a DomainName object may be a "*" wildcard (RFC 9525 +/// Section 6.3) if and only if the object creator explicitly allows wildcards. +using DomainName = SBuf; + } // namespace Dns // internal DNS client API diff --git a/src/ip/Address.cc b/src/ip/Address.cc index 62a07cfb9c2..37570298e0f 100644 --- a/src/ip/Address.cc +++ b/src/ip/Address.cc @@ -40,6 +40,16 @@ } printf("\n"); assert(b); \ } +std::optional +Ip::Address::Parse(const char * const raw) +{ + Address tmp; + // TODO: Merge with lookupHostIP() after removing DNS lookups from Ip. + if (tmp.lookupHostIP(raw, false)) + return tmp; + return std::nullopt; +} + int Ip::Address::cidr() const { diff --git a/src/ip/Address.h b/src/ip/Address.h index 49021a77d2a..8f49bb29cf8 100644 --- a/src/ip/Address.h +++ b/src/ip/Address.h @@ -31,6 +31,8 @@ #include #endif +#include + namespace Ip { @@ -41,6 +43,14 @@ class Address { public: + /// Creates an IP address object by parsing a given c-string. Accepts all + /// three forms of IPv6 addresses from RFC 4291 section 2.2. Examples of + /// valid input: 0, 1.0, 1.2.3.4, ff01::101, and ::FFFF:129.144.52.38. + /// Fails if input contains characters before or after a valid IP address. + /// For example, fails if given a bracketed IPv6 address (e.g., [::1]). + /// \returns std::nullopt if parsing fails + static std::optional
Parse(const char *); + /** @name Constructors */ /*@{*/ Address() { setEmpty(); } @@ -62,6 +72,11 @@ class Address Address& operator =(struct sockaddr_in6 const &s); bool operator =(const struct hostent &s); bool operator =(const struct addrinfo &s); + + /// Interprets the given c-string as an IP address and, upon success, + /// assigns that address. Does nothing if that interpretation fails. + /// \returns whether the assignment was performed + /// \deprecated Use Parse() instead. bool operator =(const char *s); /*@}*/ @@ -233,8 +248,7 @@ class Address unsigned int toHostStr(char *buf, const unsigned int len) const; /// Empties the address and then slowly imports the IP from a possibly - /// [bracketed] portless host. For the semi-reverse operation, see - /// toHostStr() which does export the port. + /// [bracketed] portless host. For the reverse operation, see toHostStr(). /// \returns whether the conversion was successful bool fromHost(const char *hostWithoutPort); diff --git a/src/security/ErrorDetail.cc b/src/security/ErrorDetail.cc index 66fbb5b4d1e..25c03e93488 100644 --- a/src/security/ErrorDetail.cc +++ b/src/security/ErrorDetail.cc @@ -574,48 +574,53 @@ Security::ErrorDetail::printSubject(std::ostream &os) const } #if USE_OPENSSL -/// a helper class to print CNs extracted using Ssl::matchX509CommonNames() -class CommonNamesPrinter +/// prints X.509 names extracted using Ssl::HasMatchingSubjectName() +class CommonNamesPrinter: public Ssl::GeneralNameMatcher { public: explicit CommonNamesPrinter(std::ostream &os): os_(os) {} - /// Ssl::matchX509CommonNames() visitor that reports the given name (if any) - static int PrintName(void *, ASN1_STRING *); + /// the number of names printed so far + mutable size_t printed = 0; - /// whether any names have been printed so far - bool printed = false; +protected: + /* GeneralNameMatcher API */ + bool matchDomainName(const Dns::DomainName &) const override; + bool matchIp(const Ip::Address &) const override; private: - void printName(const ASN1_STRING *); + std::ostream &itemStream() const; std::ostream &os_; ///< destination for printed names }; -int -CommonNamesPrinter::PrintName(void * const printer, ASN1_STRING * const name) +bool +CommonNamesPrinter::matchDomainName(const Dns::DomainName &domain) const { - assert(printer); - static_cast(printer)->printName(name); - return 1; + // TODO: Convert html_quote() into an std::ostream manipulator accepting (buf, n). + itemStream() << html_quote(SBuf(domain).c_str()); + return false; // keep going } -/// prints an HTML-quoted version of the given common name (as a part of the -/// printed names list) -void -CommonNamesPrinter::printName(const ASN1_STRING * const name) +bool +CommonNamesPrinter::matchIp(const Ip::Address &ip) const { - if (name && name->length) { - if (printed) - os_ << ", "; - - // TODO: Convert html_quote() into an std::ostream manipulator accepting (buf, n). - SBuf buf(reinterpret_cast(name->data), name->length); - os_ << html_quote(buf.c_str()); + char hostStr[MAX_IPSTRLEN]; + (void)ip.toStr(hostStr, sizeof(hostStr)); // no html_quote() is needed; no brackets + itemStream().write(hostStr, strlen(hostStr)); + return false; // keep going +} - printed = true; - } +/// prints a comma in front of each item except for the very first one +/// \returns a stream for printing the name +std::ostream & +CommonNamesPrinter::itemStream() const +{ + if (printed++) + os_ << ", "; + return os_; } + #endif // USE_OPENSSL /// a list of the broken certificates CN and alternate names @@ -625,7 +630,7 @@ Security::ErrorDetail::printCommonName(std::ostream &os) const #if USE_OPENSSL if (broken_cert.get()) { CommonNamesPrinter printer(os); - Ssl::matchX509CommonNames(broken_cert.get(), &printer, printer.PrintName); + (void)Ssl::HasMatchingSubjectName(*broken_cert, printer); if (printer.printed) return; } diff --git a/src/security/cert_generators/file/Makefile.am b/src/security/cert_generators/file/Makefile.am index 070dc95e608..a94b026dbaf 100644 --- a/src/security/cert_generators/file/Makefile.am +++ b/src/security/cert_generators/file/Makefile.am @@ -24,11 +24,13 @@ security_file_certgen_SOURCES = \ security_file_certgen_LDADD = \ $(top_builddir)/src/ssl/libsslutil.la \ + $(top_builddir)/src/ip/libip.la \ $(top_builddir)/src/sbuf/libsbuf.la \ $(top_builddir)/src/debug/libdebug.la \ $(top_builddir)/src/error/liberror.la \ $(top_builddir)/src/comm/libminimal.la \ $(top_builddir)/src/mem/libminimal.la \ + $(top_builddir)/src/anyp/libanyp.la \ $(top_builddir)/src/base/libbase.la \ $(top_builddir)/src/time/libtime.la \ $(SSLLIB) \ diff --git a/src/ssl/gadgets.cc b/src/ssl/gadgets.cc index 1f8ac9d76d3..a86629af4fb 100644 --- a/src/ssl/gadgets.cc +++ b/src/ssl/gadgets.cc @@ -7,8 +7,10 @@ */ #include "squid.h" +#include "anyp/Host.h" #include "base/IoManip.h" #include "error/SysErrorDetail.h" +#include "ip/Address.h" #include "sbuf/Stream.h" #include "security/Io.h" #include "ssl/gadgets.h" @@ -467,6 +469,62 @@ mimicExtensions(Security::CertPointer & cert, Security::CertPointer const &mimic return added; } +SBuf +Ssl::AsnToSBuf(const ASN1_STRING &buffer) +{ + return SBuf(reinterpret_cast(buffer.data), buffer.length); +} + +/// OpenSSL ASN1_STRING_to_UTF8() wrapper +static std::optional +ParseAsUtf8(const ASN1_STRING &asnBuffer) +{ + unsigned char *utfBuffer = nullptr; + const auto conversionResult = ASN1_STRING_to_UTF8(&utfBuffer, &asnBuffer); + if (conversionResult < 0) { + debugs(83, 3, "failed" << Ssl::ReportAndForgetErrors); + return std::nullopt; + } + Assure(utfBuffer); + const auto utfChars = reinterpret_cast(utfBuffer); + const auto utfLength = static_cast(conversionResult); + Ssl::UniqueCString bufferDestroyer(utfChars); + return SBuf(utfChars, utfLength); +} + +std::optional +Ssl::ParseAsSimpleDomainNameOrIp(const SBuf &text) +{ + if (const auto ip = Ip::Address::Parse(SBuf(text).c_str())) + return AnyP::Host::ParseIp(*ip); + return AnyP::Host::ParseSimpleDomainName(text); +} + +std::optional +Ssl::ParseCommonNameAt(X509_NAME &name, const int cnIndex) +{ + const auto cn = X509_NAME_ENTRY_get_data(X509_NAME_get_entry(&name, cnIndex)); + if (!cn) { + debugs(83, 7, "no CN at " << cnIndex); + return std::nullopt; + } + + // CN buffer usually contains an ASCII domain name, but X.509 and TLS allow + // other name encodings (e.g., UTF-16), and some CNs are not domain names + // (e.g., organization name or perhaps even a dotted IP address). We do our + // best to identify IP addresses and treat anything else as a domain name. + // TODO: Do not treat CNs with spaces or without periods as domain names. + + // OpenSSL does not offer ASN1_STRING_to_ASCII(), so we convert to UTF-8 + // that usually "works" for further parsing/validation/comparison purposes + // even though Squid code will treat multi-byte characters as char bytes. + // TODO: Confirm that OpenSSL preserves UTF-8 when we add a "DNS:..." SAN. + + if (const auto utf = ParseAsUtf8(*cn)) + return ParseAsSimpleDomainNameOrIp(*utf); + return std::nullopt; +} + /// Adds a new subjectAltName extension contining Subject CN or returns false /// expects the caller to check for the existing subjectAltName extension static bool @@ -480,16 +538,17 @@ addAltNameWithSubjectCn(Security::CertPointer &cert) if (loc < 0) return false; - ASN1_STRING *cn_data = X509_NAME_ENTRY_get_data(X509_NAME_get_entry(name, loc)); - if (!cn_data) - return false; - - char dnsName[1024]; // DNS names are limited to 256 characters - const int res = snprintf(dnsName, sizeof(dnsName), "DNS:%*s", cn_data->length, cn_data->data); - if (res <= 0 || res >= static_cast(sizeof(dnsName))) + const auto cn = Ssl::ParseCommonNameAt(*name, loc); + if (!cn) return false; - X509_EXTENSION *ext = X509V3_EXT_conf_nid(nullptr, nullptr, NID_subject_alt_name, dnsName); + // We create an "IP:address" or "DNS:name" text that X509V3_EXT_conf_nid() + // then parses and converts to OpenSSL GEN_IPADD or GEN_DNS GENERAL_NAME. + // TODO: Use X509_add1_ext_i2d() to add a GENERAL_NAME extension directly: + // https://github.com/openssl/openssl/issues/11706#issuecomment-633180151 + const auto altNamePrefix = cn->ip() ? "IP:" : "DNS:"; + auto altName = ToSBuf(altNamePrefix, *cn); + const auto ext = X509V3_EXT_conf_nid(nullptr, nullptr, NID_subject_alt_name, altName.c_str()); if (!ext) return false; diff --git a/src/ssl/gadgets.h b/src/ssl/gadgets.h index 72da1781d68..db5e52d6d4c 100644 --- a/src/ssl/gadgets.h +++ b/src/ssl/gadgets.h @@ -11,11 +11,14 @@ #if USE_OPENSSL +#include "anyp/forward.h" #include "base/HardFun.h" #include "compat/openssl.h" +#include "sbuf/forward.h" #include "security/forward.h" #include "ssl/crtd_message.h" +#include #include #if HAVE_OPENSSL_ASN1_H @@ -278,6 +281,16 @@ bool certificateMatchesProperties(X509 *peer_cert, CertificateProperties const & */ const char *CommonHostName(X509 *x509); +/// converts ASN1_STRING to SBuf +SBuf AsnToSBuf(const ASN1_STRING &); + +/// interprets X.509 Subject or Issuer name entry (at the given position) as CN +std::optional ParseCommonNameAt(X509_NAME &, int); + +/// interprets the given buffer as either a textual representation of an IP +/// address (if possible) or a domain name without wildcard support (otherwise) +std::optional ParseAsSimpleDomainNameOrIp(const SBuf &); + /** \ingroup ServerProtocolSSLAPI * Returns Organization from the certificate. diff --git a/src/ssl/support.cc b/src/ssl/support.cc index e55c7860472..7026301b272 100644 --- a/src/ssl/support.cc +++ b/src/ssl/support.cc @@ -16,12 +16,14 @@ #if USE_OPENSSL #include "acl/FilledChecklist.h" +#include "anyp/Host.h" #include "anyp/PortCfg.h" #include "anyp/Uri.h" #include "fatal.h" #include "fd.h" #include "fde.h" #include "globals.h" +#include "ip/Address.h" #include "ipc/MemMap.h" #include "security/CertError.h" #include "security/Certificate.h" @@ -55,6 +57,66 @@ std::vector Ssl::BumpModeStr = { /*,"err"*/ }; +namespace Ssl { + +/// GeneralNameMatcher for matching a single AnyP::Host given at construction time +class OneNameMatcher: public GeneralNameMatcher +{ +public: + explicit OneNameMatcher(const AnyP::Host &needle): needle_(needle) {} + +protected: + /* GeneralNameMatcher API */ + bool matchDomainName(const Dns::DomainName &) const override; + bool matchIp(const Ip::Address &) const override; + + AnyP::Host needle_; ///< a name we are looking for +}; + +} // namespace Ssl + +bool +Ssl::GeneralNameMatcher::match(const GeneralName &name) const +{ + if (const auto domain = name.domainName()) + return matchDomainName(*domain); + if (const auto ip = name.ip()) + return matchIp(*ip); + Assure(!"unreachable code: the above `if` statements must cover all name variants"); + return false; +} + +bool +Ssl::OneNameMatcher::matchDomainName(const Dns::DomainName &rawName) const { + // TODO: Add debugs() stream manipulator to safely (i.e. without breaking + // cache.log message framing) dump raw input that may contain new lines. Use + // here and in similar contexts where we report such raw input. + debugs(83, 5, "needle=" << needle_ << " domain=" << rawName); + if (needle_.ip()) { + // for example, a 127.0.0.1 IP needle does not match DNS:127.0.0.1 SAN + debugs(83, 7, "needle is an IP; mismatch"); + return false; + } + + Assure(needle_.domainName()); + auto domainNeedle = *needle_.domainName(); + + auto name = rawName; + if (name.length() > 0 && name[0] == '*') + name.consume(1); + + return ::matchDomainName(domainNeedle.c_str(), name.c_str(), mdnRejectSubsubDomains) == 0; +} + +bool +Ssl::OneNameMatcher::matchIp(const Ip::Address &ip) const { + debugs(83, 5, "needle=" << needle_ << " ip=" << ip); + if (const auto needleIp = needle_.ip()) + return (*needleIp == ip); + debugs(83, 7, "needle is not an IP; mismatch"); + return false; +} + /** \defgroup ServerProtocolSSLInternal Server-Side SSL Internals \ingroup ServerProtocolSSLAPI @@ -192,68 +254,85 @@ int Ssl::asn1timeToString(ASN1_TIME *tm, char *buf, int len) return write; } -int Ssl::matchX509CommonNames(X509 *peer_cert, void *check_data, int (*check_func)(void *check_data, ASN1_STRING *cn_data)) +static std::optional +ParseSubjectAltName(const GENERAL_NAME &san) { - assert(peer_cert); + switch(san.type) { + case GEN_DNS: { + Assure(san.d.dNSName); + // GEN_DNS is an IA5STRING. IA5STRING is a subset of ASCII that does not + // need to be converted to UTF-8 (or some such) before we parse it. + const auto buffer = Ssl::AsnToSBuf(*san.d.dNSName); + return AnyP::Host::ParseWildDomainName(buffer); + } - X509_NAME *name = X509_get_subject_name(peer_cert); + case GEN_IPADD: { + // san.d.iPAddress is OpenSSL ASN1_OCTET_STRING + Assure(san.d.iPAddress); - for (int i = X509_NAME_get_index_by_NID(name, NID_commonName, -1); i >= 0; i = X509_NAME_get_index_by_NID(name, NID_commonName, i)) { + // RFC 5280 section 4.2.1.6 signals IPv4/IPv6 address family using data length - ASN1_STRING *cn_data = X509_NAME_ENTRY_get_data(X509_NAME_get_entry(name, i)); - - if ( (*check_func)(check_data, cn_data) == 0) - return 1; - } + if (san.d.iPAddress->length == 4) { + struct in_addr addr; + static_assert(sizeof(addr.s_addr) == 4); + memcpy(&addr.s_addr, san.d.iPAddress->data, sizeof(addr.s_addr)); + const Ip::Address ip(addr); + return AnyP::Host::ParseIp(ip); + } - STACK_OF(GENERAL_NAME) * altnames; - altnames = (STACK_OF(GENERAL_NAME)*)X509_get_ext_d2i(peer_cert, NID_subject_alt_name, nullptr, nullptr); + if (san.d.iPAddress->length == 16) { + struct in6_addr addr; + static_assert(sizeof(addr.s6_addr) == 16); + memcpy(&addr.s6_addr, san.d.iPAddress->data, sizeof(addr.s6_addr)); + const Ip::Address ip(addr); + return AnyP::Host::ParseIp(ip); + } - if (altnames) { - int numalts = sk_GENERAL_NAME_num(altnames); - for (int i = 0; i < numalts; ++i) { - const GENERAL_NAME *check = sk_GENERAL_NAME_value(altnames, i); - if (check->type != GEN_DNS) { - continue; - } - ASN1_STRING *cn_data = check->d.dNSName; + debugs(83, 3, "unexpected length of an IP address SAN: " << san.d.iPAddress->length); + return std::nullopt; + } - if ( (*check_func)(check_data, cn_data) == 0) { - sk_GENERAL_NAME_pop_free(altnames, GENERAL_NAME_free); - return 1; - } - } - sk_GENERAL_NAME_pop_free(altnames, GENERAL_NAME_free); + default: + debugs(83, 3, "unsupported SAN kind: " << san.type); + return std::nullopt; } - return 0; } -static int check_domain( void *check_data, ASN1_STRING *cn_data) +bool +Ssl::HasMatchingSubjectName(X509 &cert, const GeneralNameMatcher &matcher) { - char cn[1024]; - const char *server = (const char *)check_data; - - if (cn_data->length == 0) - return 1; // zero length cn, ignore - - if (cn_data->length > (int)sizeof(cn) - 1) - return 1; //if does not fit our buffer just ignore + const auto name = X509_get_subject_name(&cert); + for (int i = X509_NAME_get_index_by_NID(name, NID_commonName, -1); i >= 0; i = X509_NAME_get_index_by_NID(name, NID_commonName, i)) { + debugs(83, 7, "checking CN at " << i); + if (const auto cn = ParseCommonNameAt(*name, i)) { + if (matcher.match(*cn)) + return true; + } + } - char *s = reinterpret_cast(cn_data->data); - char *d = cn; - for (int i = 0; i < cn_data->length; ++i, ++d, ++s) { - if (*s == '\0') - return 1; // always a domain mismatch. contains 0x00 - *d = *s; + const Ssl::GENERAL_NAME_STACK_Pointer sans(static_cast( + X509_get_ext_d2i(&cert, NID_subject_alt_name, nullptr, nullptr))); + if (sans) { + const auto sanCount = sk_GENERAL_NAME_num(sans.get()); + for (int i = 0; i < sanCount; ++i) { + debugs(83, 7, "checking SAN at " << i); + const auto rawSan = sk_GENERAL_NAME_value(sans.get(), i); + Assure(rawSan); + if (const auto san = ParseSubjectAltName(*rawSan)) { + if (matcher.match(*san)) + return true; + } + } } - cn[cn_data->length] = '\0'; - debugs(83, 4, "Verifying server domain " << server << " to certificate name/subjectAltName " << cn); - return matchDomainName(server, (cn[0] == '*' ? cn + 1 : cn), mdnRejectSubsubDomains); + + debugs(83, 7, "no matches"); + return false; } -bool Ssl::checkX509ServerValidity(X509 *cert, const char *server) +bool +Ssl::HasSubjectName(X509 &cert, const AnyP::Host &host) { - return matchX509CommonNames(cert, (void *)server, check_domain); + return HasMatchingSubjectName(cert, OneNameMatcher(host)); } /// adjusts OpenSSL validation results for each verified certificate in ctx @@ -295,8 +374,20 @@ ssl_verify_cb(int ok, X509_STORE_CTX * ctx) // Check for domain mismatch only if the current certificate is the peer certificate. if (!dont_verify_domain && server && peer_cert.get() == X509_STORE_CTX_get_current_cert(ctx)) { - if (!Ssl::checkX509ServerValidity(peer_cert.get(), server->c_str())) { - debugs(83, 2, "SQUID_X509_V_ERR_DOMAIN_MISMATCH: Certificate " << *peer_cert << " does not match domainname " << server); + // XXX: This code does not know where the server name came from. The + // name may be valid but not compatible with requirements assumed or + // enforced by the AnyP::Host::ParseSimpleDomainName() call below. + // TODO: Store AnyP::Host (or equivalent) in ssl_ex_index_server. + if (const auto host = Ssl::ParseAsSimpleDomainNameOrIp(*server)) { + if (Ssl::HasSubjectName(*peer_cert, *host)) { + debugs(83, 5, "certificate subject matches " << *host); + } else { + debugs(83, 2, "SQUID_X509_V_ERR_DOMAIN_MISMATCH: Certificate " << *peer_cert << " does not match domainname " << *host); + ok = 0; + error_no = SQUID_X509_V_ERR_DOMAIN_MISMATCH; + } + } else { + debugs(83, 2, "SQUID_X509_V_ERR_DOMAIN_MISMATCH: Cannot check whether certificate " << *peer_cert << " subject matches malformed domainname " << *server); ok = 0; error_no = SQUID_X509_V_ERR_DOMAIN_MISMATCH; } diff --git a/src/ssl/support.h b/src/ssl/support.h index 53e64c102fd..18d29ab241e 100644 --- a/src/ssl/support.h +++ b/src/ssl/support.h @@ -13,9 +13,13 @@ #if USE_OPENSSL +#include "anyp/forward.h" #include "base/CbDataList.h" +#include "base/TypeTraits.h" #include "comm/forward.h" #include "compat/openssl.h" +#include "dns/forward.h" +#include "ip/Address.h" #include "sbuf/SBuf.h" #include "security/Session.h" #include "ssl/gadgets.h" @@ -31,6 +35,8 @@ #endif #include #include +#include +#include /** \defgroup ServerProtocolSSLAPI Server-Side SSL API @@ -143,6 +149,21 @@ inline const char *bumpMode(int bm) /// certificates indexed by issuer name typedef std::multimap CertsIndexedList; +/// A successfully extracted/parsed certificate "name" field. See RFC 5280 +/// GeneralName and X520CommonName types for examples of information sources. +/// For now, we only support the same two name variants as AnyP::Host: +/// +/// * An IPv4 or an IPv6 address. This info comes (with very little validation) +/// from RFC 5280 "iPAddress" variant of a subjectAltName +/// +/// * A domain name or domain name wildcard (e.g., *.example.com). This info +/// comes (with very little validation) from a source like these two: +/// - RFC 5280 "dNSName" variant of a subjectAltName extension (GeneralName +/// index is 2, underlying value type is IA5String); +/// - RFC 5280 X520CommonName component of a Subject distinguished name field +/// (underlying value type is DirectoryName). +using GeneralName = AnyP::Host; + /** * Load PEM-encoded certificates from the given file. */ @@ -273,25 +294,28 @@ bool configureSSLUsingPkeyAndCertFromMemory(SSL *ssl, const char *data, AnyP::Po */ void useSquidUntrusted(SSL_CTX *sslContext); -/** - \ingroup ServerProtocolSSLAPI - * Iterates over the X509 common and alternate names and to see if matches with given data - * using the check_func. - \param peer_cert The X509 cert to check - \param check_data The data with which the X509 CNs compared - \param check_func The function used to match X509 CNs. The CN data passed as ASN1_STRING data - \return 1 if any of the certificate CN matches, 0 if none matches. - */ -int matchX509CommonNames(X509 *peer_cert, void *check_data, int (*check_func)(void *check_data, ASN1_STRING *cn_data)); +/// an algorithm for checking/testing/comparing X.509 certificate names +class GeneralNameMatcher: public Interface +{ +public: + /// whether the given name satisfies algorithm conditions + bool match(const Ssl::GeneralName &) const; -/** - \ingroup ServerProtocolSSLAPI - * Check if the certificate is valid for a server - \param cert The X509 cert to check. - \param server The server name. - \return true if the certificate is valid for the server or false otherwise. - */ -bool checkX509ServerValidity(X509 *cert, const char *server); +protected: + // The methods below implement public match() API for each of the + // GeneralName variants. For each public match() method call, exactly one of + // these methods is called. + + virtual bool matchDomainName(const Dns::DomainName &) const = 0; + virtual bool matchIp(const Ip::Address &) const = 0; +}; + +/// Determines whether at least one common or alternate subject names matches. +/// The first match (if any) terminates the search. +bool HasMatchingSubjectName(X509 &, const GeneralNameMatcher &); + +/// whether at least one common or alternate subject name matches the given one +bool HasSubjectName(X509 &, const AnyP::Host &); /** \ingroup ServerProtocolSSLAPI diff --git a/src/tests/stub_libip.cc b/src/tests/stub_libip.cc index b46ba075fc2..109dce317b7 100644 --- a/src/tests/stub_libip.cc +++ b/src/tests/stub_libip.cc @@ -13,6 +13,7 @@ #include "tests/STUB.h" #include "ip/Address.h" +std::optional Ip::Address::Parse(const char *) STUB_RETVAL(std::nullopt) Ip::Address::Address(const struct in_addr &) STUB Ip::Address::Address(const struct sockaddr_in &) STUB Ip::Address::Address(const struct in6_addr &) STUB diff --git a/src/tests/stub_libsslsquid.cc b/src/tests/stub_libsslsquid.cc index 6253bbced52..a3cd170e6b2 100644 --- a/src/tests/stub_libsslsquid.cc +++ b/src/tests/stub_libsslsquid.cc @@ -69,8 +69,8 @@ bool generateUntrustedCert(Security::CertPointer &, Security::PrivateKeyPointer Security::ContextPointer GenerateSslContext(CertificateProperties const &, Security::ServerOptions &, bool) STUB_RETVAL(Security::ContextPointer()) bool verifySslCertificate(const Security::ContextPointer &, CertificateProperties const &) STUB_RETVAL(false) Security::ContextPointer GenerateSslContextUsingPkeyAndCertFromMemory(const char *, Security::ServerOptions &, bool) STUB_RETVAL(Security::ContextPointer()) -int matchX509CommonNames(X509 *, void *, int (*)(void *, ASN1_STRING *)) STUB_RETVAL(0) -bool checkX509ServerValidity(X509 *, const char *) STUB_RETVAL(false) +bool HasMatchingSubjectName(X509 &, const GeneralNameMatcher &) STUB_RETVAL(false) +bool HasSubjectName(X509 &, const AnyP::Host &) STUB_RETVAL(false) int asn1timeToString(ASN1_TIME *, char *, int) STUB_RETVAL(0) void setClientSNI(SSL *, const char *) STUB SBuf GetX509PEM(X509 *) STUB_RETVAL(SBuf())