From: Alex Rousskov Date: Mon, 24 May 2021 20:02:52 +0000 (+0000) Subject: Add tls_key_log to report TLS communication secrets (#759) X-Git-Tag: 4.15-20210527-snapshot~1 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=refs%2Fpull%2F458%2Fhead;p=thirdparty%2Fsquid.git Add tls_key_log to report TLS communication secrets (#759) The new log is meant for triage using traffic inspection tools like Wireshark. Also known as SSLKEYLOGFILE, it is a common feature across TLS-capable applications. By default, the new feature is disabled and has negligible performance impact. We have rejected the idea of adding an access_log %code instead, as detailed in squid-dev thread titled "RFC: tls_key_log: report TLS pre-master secrets, other key material". Such %code can still be added. http://lists.squid-cache.org/pipermail/squid-dev/2020-July/009605.html ---- Generalized StoreClient::fillChecklist() API into Acl::ChecklistFiller Code that establishes TLS connections outsources logging of secrets to Security::KeyLogger. Security::KeyLogger::shouldLog() must consult tls_key_log ACLs before logging the secrets but it does not know much about the code/context that delegated this logging to KeyLogger. That code could fill the checklist in advance, but that is an inferior solution because it * prevents outsourcing a rather mundane responsibility to KeyLogger * creates a potentially stale Checklist, not reflecting TLS progress * wastes resources on filling a checklist that may not be needed While adding ConnStateData::fillChecklist() and looking through ConnStateData code for examples on how to fill a checklist, it became clear that there is a lot of code duplication in this area, including some subtle problems with various checklist.foo assignments possibly missing in some places (it is difficult to know for sure because some checklist methods may extract the same information from _multiple_ sources). For example, * on_unsupported_protocol and ssl_bump ignored acl_uses_indirect_client * ident_lookup_access and delay_pool_access lacked port info and other ConnStateData details beyond src/dst addresses Now, most of that duplicated code is replaced with unified fillChecklist() calls. This unification is a significant improvement that provides additional justification for the fillChecklist() design. Also renamed ACLFilledChecklist::conn() to setConn(). Renaming is not necessary for the compiler but will help detect no longer necessary conn() setting calls when this code is ported to other Squid versions. --- diff --git a/acinclude/lib-checks.m4 b/acinclude/lib-checks.m4 index ae3d24b09e..c75a870412 100644 --- a/acinclude/lib-checks.m4 +++ b/acinclude/lib-checks.m4 @@ -106,12 +106,16 @@ AC_DEFUN([SQUID_CHECK_LIBSSL_API],[ AH_TEMPLATE(HAVE_LIBSSL_SSL_CIPHER_FIND, "Define to 1 if the SSL_CIPHER_find() OpenSSL API function exists") AH_TEMPLATE(HAVE_LIBSSL_SSL_CTX_SET_TMP_RSA_CALLBACK, "Define to 1 if the SSL_CTX_set_tmp_rsa_callback() OpenSSL API function exists") AH_TEMPLATE(HAVE_LIBSSL_SSL_SESSION_GET_ID, "Define to 1 if the SSL_SESSION_get_id() OpenSSL API function exists") + AH_TEMPLATE(HAVE_LIBSSL_SSL_GET_CLIENT_RANDOM, "Define to 1 if the SSL_get_client_random() OpenSSL API function exists") + AH_TEMPLATE(HAVE_LIBSSL_SSL_SESSION_GET_MASTER_KEY, "Define to 1 if the SSL_SESSION_get_master_key() OpenSSL API function exists") SQUID_STATE_SAVE(check_openssl_libssl_api) LIBS="$LIBS $SSLLIB" AC_CHECK_LIB(ssl, OPENSSL_init_ssl, AC_DEFINE(HAVE_LIBSSL_OPENSSL_INIT_SSL, 1)) AC_CHECK_LIB(ssl, SSL_CIPHER_find, AC_DEFINE(HAVE_LIBSSL_SSL_CIPHER_FIND, 1)) AC_CHECK_LIB(ssl, SSL_CTX_set_tmp_rsa_callback, AC_DEFINE(HAVE_LIBSSL_SSL_CTX_SET_TMP_RSA_CALLBACK, 1)) AC_CHECK_LIB(ssl, SSL_SESSION_get_id, AC_DEFINE(HAVE_LIBSSL_SSL_SESSION_GET_ID, 1)) + AC_CHECK_LIB(ssl, SSL_get_client_random, AC_DEFINE(HAVE_LIBSSL_SSL_GET_CLIENT_RANDOM, 1)) + AC_CHECK_LIB(ssl, SSL_SESSION_get_master_key, AC_DEFINE(HAVE_LIBSSL_SSL_SESSION_GET_MASTER_KEY, 1)) SQUID_STATE_ROLLBACK(check_openssl_libssl_api) ]) diff --git a/compat/openssl.h b/compat/openssl.h index 02aee70222..a3b2c1b5de 100644 --- a/compat/openssl.h +++ b/compat/openssl.h @@ -150,6 +150,46 @@ extern "C" { } #endif +#if !HAVE_LIBSSL_SSL_GET_CLIENT_RANDOM + inline size_t + SSL_get_client_random(const SSL *ssl, unsigned char *outStart, size_t outSizeMax) + { + if (!ssl || !ssl->s3) + return 0; + + const auto &source = ssl->s3->client_random; + + const auto sourceSize = sizeof(source); + if (!outSizeMax) + return sourceSize; // special API case for sourceSize discovery + + const auto sourceStart = reinterpret_cast(source); + const auto outSize = std::min(sourceSize, outSizeMax); + assert(outStart); + memmove(outStart, sourceStart, outSize); + return outSize; + } +#endif /* HAVE_LIBSSL_SSL_GET_CLIENT_RANDOM */ + +#if !HAVE_LIBSSL_SSL_SESSION_GET_MASTER_KEY + inline size_t + SSL_SESSION_get_master_key(const SSL_SESSION *session, unsigned char *outStart, size_t outSizeMax) + { + if (!session || !session->master_key || session->master_key_length <= 0) + return 0; + + const auto sourceSize = static_cast(session->master_key_length); + if (!outSizeMax) + return sourceSize; // special API case for sourceSize discovery + + const auto sourceStart = reinterpret_cast(session->master_key); + const auto outSize = std::min(sourceSize, outSizeMax); + assert(outStart); + memmove(outStart, sourceStart, outSize); + return outSize; + } +#endif /* HAVE_LIBSSL_SSL_SESSION_GET_MASTER_KEY */ + #if !HAVE_OPENSSL_TLS_CLIENT_METHOD #define TLS_client_method SSLv23_client_method #endif diff --git a/src/ConfigParser.cc b/src/ConfigParser.cc index 4fd966de08..2e07193579 100644 --- a/src/ConfigParser.cc +++ b/src/ConfigParser.cc @@ -7,6 +7,7 @@ */ #include "squid.h" +#include "acl/Gadgets.h" #include "base/Here.h" #include "cache_cf.h" #include "ConfigParser.h" @@ -578,6 +579,44 @@ ConfigParser::closeDirective() // TODO: cfg_directive = nullptr; // currently in generated code } +SBuf +ConfigParser::token(const char *expectedTokenDescription) +{ + if (const auto extractedToken = NextToken()) { + debugs(3, 5, CurrentLocation() << ' ' << expectedTokenDescription << ": " << extractedToken); + return SBuf(extractedToken); + } + throw TextException(ToSBuf("missing ", expectedTokenDescription), Here()); +} + +bool +ConfigParser::skipOptional(const char *keyword) +{ + assert(keyword); + if (const auto nextToken = PeekAtToken()) { + if (strcmp(nextToken, keyword) == 0) { + (void)NextToken(); + return true; + } + return false; // the next token on the line is not the optional keyword + } + return false; // no more tokens (i.e. we are at the end of the line) +} + +Acl::Tree * +ConfigParser::optionalAclList() +{ + if (!skipOptional("if")) + return nullptr; // OK: the directive has no ACLs + + Acl::Tree *acls = nullptr; + const auto aclCount = aclParseAclList(*this, &acls, cfg_directive); + assert(acls); + if (aclCount <= 0) + throw TextException("missing ACL name(s) after 'if' keyword", Here()); + return acls; +} + bool ConfigParser::CfgFile::startParse(char *path) { diff --git a/src/ConfigParser.h b/src/ConfigParser.h index b53722cf5c..d6a80a4be9 100644 --- a/src/ConfigParser.h +++ b/src/ConfigParser.h @@ -9,6 +9,7 @@ #ifndef SQUID_CONFIGPARSER_H #define SQUID_CONFIGPARSER_H +#include "acl/forward.h" #include "sbuf/forward.h" #include "SquidString.h" @@ -56,11 +57,20 @@ public: /// rejects configuration due to a repeated directive void rejectDuplicateDirective(); + /// extracts and returns a required token + SBuf token(const char *expectedTokenDescription); + /// extracts an optional key=value token or returns false /// rejects configurations with empty keys or empty values /// key and value have lifetime of the current line/directive bool optionalKvPair(char * &key, char * &value); + /// either extracts the given (optional) token or returns false + bool skipOptional(const char *keyword); + + /// parses an [if [!]...] construct + Acl::Tree *optionalAclList(); + static void ParseUShort(unsigned short *var); static void ParseBool(bool *var); static const char *QuoteString(const String &var); diff --git a/src/Debug.h b/src/Debug.h index e8e33e51bc..a6dd7d6dfd 100644 --- a/src/Debug.h +++ b/src/Debug.h @@ -281,5 +281,8 @@ operator <<(std::ostream &os, const AsHex number) template inline AsHex asHex(const Integer n) { return AsHex(n); } +/// Prints the first n data bytes using hex notation. Does nothing if n is 0. +void PrintHex(std::ostream &, const char *data, size_t n); + #endif /* SQUID_DEBUG_H */ diff --git a/src/DelayId.cc b/src/DelayId.cc index e2882fe25f..5cd831fec4 100644 --- a/src/DelayId.cc +++ b/src/DelayId.cc @@ -86,20 +86,18 @@ DelayId::DelayClient(ClientHttpRequest * http, HttpReply *reply) } ACLFilledChecklist ch(DelayPools::delay_data[pool].access, r, NULL); - if (reply) { + clientAclChecklistFill(ch, http); + if (!ch.reply && reply) { ch.reply = reply; HTTPMSGLOCK(reply); } + // overwrite ACLFilledChecklist acl_uses_indirect_client-based decision #if FOLLOW_X_FORWARDED_FOR if (Config.onoff.delay_pool_uses_indirect_client) ch.src_addr = r->indirect_client_addr; else #endif /* FOLLOW_X_FORWARDED_FOR */ ch.src_addr = r->client_addr; - ch.my_addr = r->my_addr; - - if (http->getConn() != NULL) - ch.conn(http->getConn()); if (DelayPools::delay_data[pool].theComposite().getRaw() && ch.fastCheck().allowed()) { diff --git a/src/Makefile.am b/src/Makefile.am index 0acf98a10c..3fd067344e 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1165,6 +1165,7 @@ tests_testRock_SOURCES = \ tests/stub_cache_manager.cc \ cbdata.cc \ tests/stub_client_db.cc \ + tests/stub_client_side.cc \ tests/stub_client_side_request.cc \ tests/stub_debug.cc \ tests/stub_errorpage.cc \ @@ -1332,13 +1333,13 @@ tests_testUfs_SOURCES = \ tests/testUfs.cc \ tests/testUfs.h \ tests/stub_access_log.cc \ - tests/stub_acl.cc \ tests/stub_cache_cf.cc \ cache_cf.h \ tests/stub_cache_manager.cc \ cbdata.cc \ tests/stub_client_db.cc \ client_db.h \ + tests/stub_client_side.cc \ tests/stub_client_side_request.cc \ tests/stub_debug.cc \ tests/stub_errorpage.cc \ @@ -1515,11 +1516,11 @@ tests_testStore_SOURCES = \ Transients.cc \ tests/stub_UdsOp.cc \ tests/stub_access_log.cc \ - tests/stub_acl.cc \ tests/stub_cache_cf.cc \ cache_cf.h \ tests/stub_cache_manager.cc \ cbdata.cc \ + tests/stub_client_side.cc \ tests/stub_client_side_request.cc \ tests/stub_comm.cc \ tests/stub_debug.cc \ @@ -1681,13 +1682,13 @@ tests_testDiskIO_SOURCES = \ Transients.cc \ tests/stub_UdsOp.cc \ tests/stub_access_log.cc \ - tests/stub_acl.cc \ tests/stub_cache_cf.cc \ cache_cf.h \ tests/stub_cache_manager.cc \ cbdata.cc \ tests/stub_client_db.cc \ client_db.h \ + tests/stub_client_side.cc \ tests/stub_client_side_request.cc \ client_side_request.h \ tests/stub_debug.cc \ @@ -2177,6 +2178,7 @@ tests_testHttpReply_SOURCES = \ tests/stub_cache_cf.cc \ cache_cf.h \ tests/stub_cache_manager.cc \ + tests/stub_client_side.cc \ cbdata.cc \ cbdata.h \ tests/stub_comm.cc \ @@ -2830,6 +2832,7 @@ tests_testConfigParser_SOURCES = \ tests/stub_MemBuf.cc \ tests/stub_SBufDetailedStats.cc \ String.cc \ + tests/stub_acl.cc \ tests/stub_cache_cf.cc \ cache_cf.h \ tests/stub_cbdata.cc \ diff --git a/src/SquidConfig.h b/src/SquidConfig.h index e773e8256c..a680d006c5 100644 --- a/src/SquidConfig.h +++ b/src/SquidConfig.h @@ -184,6 +184,7 @@ public: #if ICAP_CLIENT CustomLog *icaplogs; #endif + Security::KeyLog *tlsKeys; ///< one optional tls_key_log int rotateNumber; } Log; char *adminEmail; diff --git a/src/StoreClient.h b/src/StoreClient.h index fb0a72625a..299a5b1473 100644 --- a/src/StoreClient.h +++ b/src/StoreClient.h @@ -9,6 +9,7 @@ #ifndef SQUID_STORECLIENT_H #define SQUID_STORECLIENT_H +#include "acl/ChecklistFiller.h" #include "base/forward.h" #include "dlink.h" #include "StoreIOBuffer.h" @@ -21,7 +22,7 @@ class ACLFilledChecklist; class LogTags; /// a storeGetPublic*() caller -class StoreClient +class StoreClient: public Acl::ChecklistFiller { public: @@ -31,9 +32,6 @@ public: virtual LogTags *loggingTags() const = 0; protected: - /// configure the ACL checklist with the current transaction state - virtual void fillChecklist(ACLFilledChecklist &) const = 0; - /// \returns whether the caller must collapse on the given entry /// Before returning true, updates common collapsing-related stats. /// See also: StoreEntry::hittingRequiresCollapsing(). diff --git a/src/acl/ChecklistFiller.h b/src/acl/ChecklistFiller.h new file mode 100644 index 0000000000..d34826ed3a --- /dev/null +++ b/src/acl/ChecklistFiller.h @@ -0,0 +1,30 @@ +/* + * Copyright (C) 1996-2020 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_ACL_CHECKLIST_FILLER_H +#define SQUID_ACL_CHECKLIST_FILLER_H + +#include "acl/forward.h" + +namespace Acl +{ + +/// an interface for those capable of configuring an ACLFilledChecklist object +class ChecklistFiller +{ +public: + virtual ~ChecklistFiller() = default; + + /// configure the given checklist (to reflect the current transaction state) + virtual void fillChecklist(ACLFilledChecklist &) const = 0; +}; + +} // namespace Acl + +#endif /* SQUID_ACL_CHECKLIST_FILLER_H */ + diff --git a/src/acl/FilledChecklist.cc b/src/acl/FilledChecklist.cc index ed59a5eca8..d2aa2d12dc 100644 --- a/src/acl/FilledChecklist.cc +++ b/src/acl/FilledChecklist.cc @@ -146,12 +146,20 @@ ACLFilledChecklist::conn() const } void -ACLFilledChecklist::conn(ConnStateData *aConn) +ACLFilledChecklist::setConn(ConnStateData *aConn) { - if (conn() == aConn) - return; - assert (conn() == NULL); + if (conn_ == aConn) + return; // no new information + + // no conn_ replacement/removal to reduce inconsistent fill concerns + assert(!conn_); + assert(aConn); + + // To reduce inconsistent fill concerns, we should be the only ones calling + // fillConnectionLevelDetails(). Set conn_ first so that the filling method + // can detect (some) direct calls from others. conn_ = cbdataReference(aConn); + aConn->fillConnectionLevelDetails(*this); } int @@ -252,8 +260,8 @@ void ACLFilledChecklist::setRequest(HttpRequest *httpRequest) src_addr = request->client_addr; my_addr = request->my_addr; - if (request->clientConnectionManager.valid()) - conn(request->clientConnectionManager.get()); + if (const auto cmgr = request->clientConnectionManager.get()) + setConn(cmgr); } } diff --git a/src/acl/FilledChecklist.h b/src/acl/FilledChecklist.h index 1b22851cc0..dd06015fc7 100644 --- a/src/acl/FilledChecklist.h +++ b/src/acl/FilledChecklist.h @@ -51,7 +51,7 @@ public: int fd() const; /// set either conn - void conn(ConnStateData *); + void setConn(ConnStateData *); /// set the client side FD void fd(int aDescriptor); diff --git a/src/acl/Gadgets.cc b/src/acl/Gadgets.cc index c07c8a4f29..adbc2e83d9 100644 --- a/src/acl/Gadgets.cc +++ b/src/acl/Gadgets.cc @@ -186,7 +186,7 @@ aclParseAccessLine(const char *directive, ConfigParser &, acl_access **treep) } // aclParseAclList does not expect or set actions (cf. aclParseAccessLine) -void +size_t aclParseAclList(ConfigParser &, Acl::Tree **treep, const char *label) { // accommodate callers unable to convert their ACL list context to string @@ -200,7 +200,7 @@ aclParseAclList(ConfigParser &, Acl::Tree **treep, const char *label) Acl::AndNode *rule = new Acl::AndNode; rule->context(ctxLine.content(), config_input_line); - rule->lineParse(); + const auto aclCount = rule->lineParse(); MemBuf ctxTree; ctxTree.init(); @@ -215,6 +215,8 @@ aclParseAclList(ConfigParser &, Acl::Tree **treep, const char *label) assert(treep); assert(!*treep); *treep = tree; + + return aclCount; } void diff --git a/src/acl/Gadgets.h b/src/acl/Gadgets.h index 3836524b29..a9bac46fed 100644 --- a/src/acl/Gadgets.h +++ b/src/acl/Gadgets.h @@ -33,16 +33,16 @@ void aclDestroyAclList(ACLList **); void aclParseAccessLine(const char *directive, ConfigParser &parser, Acl::Tree **); /// Parses a single line of a "some context followed by acls" directive (e.g., note n v). /// The label parameter identifies the context (for debugging). -/// \ingroup ACLAPI -void aclParseAclList(ConfigParser &parser, Acl::Tree **, const char *label); +/// \returns the number of parsed ACL names +size_t aclParseAclList(ConfigParser &parser, Acl::Tree **, const char *label); /// Template to convert various context labels to strings. \ingroup ACLAPI template -inline -void aclParseAclList(ConfigParser &parser, Acl::Tree **tree, const Any any) +inline size_t +aclParseAclList(ConfigParser &parser, Acl::Tree **tree, const Any any) { std::ostringstream buf; buf << any; - aclParseAclList(parser, tree, buf.str().c_str()); + return aclParseAclList(parser, tree, buf.str().c_str()); } /// \ingroup ACLAPI diff --git a/src/acl/InnerNode.cc b/src/acl/InnerNode.cc index cda5677398..98cbc4f75e 100644 --- a/src/acl/InnerNode.cc +++ b/src/acl/InnerNode.cc @@ -39,9 +39,8 @@ Acl::InnerNode::add(ACL *node) aclRegister(node); } -// one call parses one "acl name acltype name1 name2 ..." line // kids use this method to handle [multiple] parse() calls correctly -void +size_t Acl::InnerNode::lineParse() { // XXX: not precise, may change when looping or parsing multiple lines @@ -50,6 +49,7 @@ Acl::InnerNode::lineParse() // expect a list of ACL names, each possibly preceded by '!' for negation + size_t count = 0; while (const char *t = ConfigParser::strtokFile()) { const bool negated = (*t == '!'); if (negated) @@ -61,7 +61,7 @@ Acl::InnerNode::lineParse() if (a == NULL) { debugs(28, DBG_CRITICAL, "ACL not found: " << t); self_destruct(); - return; + return count; // not reached } // append(negated ? new NotNode(a) : a); @@ -69,9 +69,11 @@ Acl::InnerNode::lineParse() add(new NotNode(a)); else add(a); + + ++count; } - return; + return count; } SBufList diff --git a/src/acl/InnerNode.h b/src/acl/InnerNode.h index 210f8869bc..4ba5dfbb33 100644 --- a/src/acl/InnerNode.h +++ b/src/acl/InnerNode.h @@ -34,8 +34,9 @@ public: virtual bool empty() const; virtual SBufList dump() const; - /// parses one "acl name type acl1 acl2..." line, appending to nodes - void lineParse(); + /// parses a [ [!]acl1 [!]acl2... ] sequence, appending to nodes + /// \returns the number of parsed ACL names + size_t lineParse(); /// appends the node to the collection and takes control over it void add(ACL *node); diff --git a/src/acl/Makefile.am b/src/acl/Makefile.am index ef48c33147..eb35c048af 100644 --- a/src/acl/Makefile.am +++ b/src/acl/Makefile.am @@ -20,6 +20,7 @@ libapi_la_SOURCES = \ BoolOps.h \ Checklist.cc \ Checklist.h \ + ChecklistFiller.h \ InnerNode.cc \ InnerNode.h \ Options.cc \ diff --git a/src/acl/forward.h b/src/acl/forward.h index f4d7cf2666..6ae09aae30 100644 --- a/src/acl/forward.h +++ b/src/acl/forward.h @@ -23,10 +23,11 @@ namespace Acl { class Address; +class AndNode; class Answer; +class ChecklistFiller; class InnerNode; class NotNode; -class AndNode; class OrNode; class Tree; diff --git a/src/adaptation/icap/Xaction.cc b/src/adaptation/icap/Xaction.cc index 09b2835836..de561d975d 100644 --- a/src/adaptation/icap/Xaction.cc +++ b/src/adaptation/icap/Xaction.cc @@ -67,6 +67,9 @@ public: } private: + /* Acl::ChecklistFiller API */ + virtual void fillChecklist(ACLFilledChecklist &) const; + Adaptation::Icap::ServiceRep::Pointer icapService; }; } // namespace Ssl @@ -727,16 +730,20 @@ Ssl::IcapPeerConnector::initialize(Security::SessionPointer &serverSession) SBuf *host = new SBuf(icapService->cfg().secure.sslDomain); SSL_set_ex_data(serverSession.get(), ssl_ex_index_server, host); setClientSNI(serverSession.get(), host->c_str()); - - ACLFilledChecklist *check = static_cast(SSL_get_ex_data(serverSession.get(), ssl_ex_index_cert_error_check)); - if (check) - check->dst_peer_name = *host; #endif Security::SetSessionResumeData(serverSession, icapService->sslSession); return true; } +void +Ssl::IcapPeerConnector::fillChecklist(ACLFilledChecklist &checklist) const +{ + Security::PeerConnector::fillChecklist(checklist); + if (checklist.dst_peer_name.isEmpty()) + checklist.dst_peer_name = icapService->cfg().secure.sslDomain; +} + void Ssl::IcapPeerConnector::noteNegotiationDone(ErrorState *error) { diff --git a/src/cache_cf.cc b/src/cache_cf.cc index 8413a006ff..ef4dec77f6 100644 --- a/src/cache_cf.cc +++ b/src/cache_cf.cc @@ -1504,6 +1504,8 @@ free_acl(ACL ** ae) void dump_acl_list(StoreEntry * entry, ACLList * head) { + // XXX: Should dump ACL names like "foo !bar" but dumps parsing context like + // "(clientside_tos 0x11 line)". dump_SBufList(entry, head->dump()); } @@ -2022,7 +2024,10 @@ ParseAclWithAction(acl_access **access, const Acl::Answer &action, const char *d Acl::AndNode *rule = new Acl::AndNode; name.Printf("(%s rule)", desc); rule->context(name.c_str(), config_input_line); - acl ? rule->add(acl) : rule->lineParse(); + if (acl) + rule->add(acl); + else + rule->lineParse(); (*access)->add(rule, action); } diff --git a/src/cf.data.depend b/src/cf.data.depend index 19ab312b77..036009bf93 100644 --- a/src/cf.data.depend +++ b/src/cf.data.depend @@ -74,6 +74,7 @@ TokenOrQuotedString refreshpattern removalpolicy securePeerOptions +Security::KeyLog* acl size_t IpAddress_list string diff --git a/src/cf.data.pre b/src/cf.data.pre index 71bef6f107..de9369fdd4 100644 --- a/src/cf.data.pre +++ b/src/cf.data.pre @@ -5459,6 +5459,64 @@ DOC_START To disable, enter "none". DOC_END +NAME: tls_key_log +TYPE: Security::KeyLog* +DEFAULT: none +LOC: Config.Log.tlsKeys +IFDEF: USE_OPENSSL +DOC_START + Configures whether and where Squid records pre-master secret and + related encryption details for TLS connections accepted or established + by Squid. These connections include connections accepted at + https_port, TLS connections opened to origin servers/cache_peers/ICAP + services, and TLS tunnels bumped by Squid using the SslBump feature. + This log (a.k.a. SSLKEYLOGFILE) is meant for triage with traffic + inspection tools like Wireshark. + + tls_key_log [options] [if [!]...] + + WARNING: This log allows anybody to decrypt the corresponding + encrypted TLS connections, both in-flight and postmortem. + + At most one log file is supported at this time. Repeated tls_key_log + directives are treated as fatal configuration errors. By default, no + log is created or updated. + + If the log file does not exist, Squid creates it. Otherwise, Squid + appends an existing log file. + + The directive is consulted whenever a TLS connection is accepted or + established by Squid. TLS connections that fail the handshake may be + logged if Squid got enough information to form a log record. A record + is logged only if all of the configured ACLs match. + + While transport-related ACLs like src and dst should work, Squid may + not have access to higher-level information. For example, when logging + accepted https_port connections, Squid does not yet have access to the + expected HTTPS request. Similarly, an HTTPS response is not available + when logging most TLS connections established by Squid. + + The log record format is meant to be compatible with TLS deciphering + features of Wireshark which relies on fields like CLIENT_RANDOM and + RSA Master-Key. A single log record usually spans multiple lines. + Technical documentation for that format is maintained inside the + Wireshark code (e.g., see tls_keylog_process_lines() comments as of + Wireshark commit e3d44136f0f0026c5e893fa249f458073f3b7328). TLS key + log does not support custom record formats. + + This clause only supports fast acl types. + See http://wiki.squid-cache.org/SquidFaq/SquidAcl for details. + + See access_log's : parameter for a list of supported + logging destinations. + + TLS key log supports all access_log key=value options with the + exception of logformat=name. + + Requires Squid built with OpenSSL support. +DOC_END + + COMMENT_START OPTIONS FOR TROUBLESHOOTING ----------------------------------------------------------------------------- diff --git a/src/client_side.cc b/src/client_side.cc index 25d503101e..586d6ba954 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -108,7 +108,9 @@ #include "proxyp/Parser.h" #include "sbuf/Stream.h" #include "security/Io.h" +#include "security/CommunicationSecrets.h" #include "security/NegotiationHistory.h" +#include "security/KeyLog.h" #include "servers/forward.h" #include "SquidConfig.h" #include "SquidTime.h" @@ -1544,10 +1546,9 @@ bool ConnStateData::serveDelayedError(Http::Stream *context) bool allowDomainMismatch = false; if (Config.ssl_client.cert_error) { - ACLFilledChecklist check(Config.ssl_client.cert_error, request, dash_str); - check.al = http->al; + ACLFilledChecklist check(Config.ssl_client.cert_error, nullptr); check.sslErrors = new Security::CertErrors(Security::CertError(SQUID_X509_V_ERR_DOMAIN_MISMATCH, srvCert)); - check.syncAle(request, http->log_uri); + clientAclChecklistFill(check, http); allowDomainMismatch = check.fastCheck().allowed(); delete check.sslErrors; check.sslErrors = NULL; @@ -1604,21 +1605,15 @@ ConnStateData::tunnelOnError(const HttpRequestMethod &method, const err_type req return false; } - const auto context = pipeline.front(); - const auto http = context ? context->http : nullptr; - const auto request = http ? http->request : nullptr; - - ACLFilledChecklist checklist(Config.accessList.on_unsupported_protocol, request, nullptr); - checklist.al = http ? http->al : nullptr; + ACLFilledChecklist checklist(Config.accessList.on_unsupported_protocol, nullptr); checklist.requestErrorType = requestError; - checklist.src_addr = clientConnection->remote; - checklist.my_addr = clientConnection->local; - checklist.conn(this); - const char *log_uri = http ? http->log_uri : nullptr; - checklist.syncAle(request, log_uri); + fillChecklist(checklist); auto answer = checklist.fastCheck(); if (answer.allowed() && answer.kind == 1) { debugs(33, 3, "Request will be tunneled to server"); + const auto context = pipeline.front(); + const auto http = context ? context->http : nullptr; + const auto request = http ? http->request : nullptr; if (context) context->finished(); // Will remove from pipeline queue Comm::SetSelect(clientConnection->fd, COMM_SELECT_READ, NULL, NULL, 0); @@ -1859,11 +1854,8 @@ ConnStateData::proxyProtocolValidateClient() if (!Config.accessList.proxyProtocol) return proxyProtocolError("PROXY client not permitted by default ACL"); - ACLFilledChecklist ch(Config.accessList.proxyProtocol, NULL, clientConnection->rfc931); - ch.src_addr = clientConnection->remote; - ch.my_addr = clientConnection->local; - ch.conn(this); - + ACLFilledChecklist ch(Config.accessList.proxyProtocol, nullptr); + fillChecklist(ch); if (!ch.fastCheck().allowed()) return proxyProtocolError("PROXY client not permitted by ACLs"); @@ -2281,8 +2273,7 @@ ConnStateData::whenClientIpKnown() #if USE_IDENT if (Ident::TheConfig.identLookup) { ACLFilledChecklist identChecklist(Ident::TheConfig.identLookup, NULL, NULL); - identChecklist.src_addr = clientConnection->remote; - identChecklist.my_addr = clientConnection->local; + fillChecklist(identChecklist); if (identChecklist.fastCheck().allowed()) Ident::Start(clientConnection, clientIdentDone, this); } @@ -2299,13 +2290,9 @@ ConnStateData::whenClientIpKnown() const auto &pools = ClientDelayPools::Instance()->pools; if (pools.size()) { ACLFilledChecklist ch(NULL, NULL, NULL); - + fillChecklist(ch); // TODO: we check early to limit error response bandwidth but we // should recheck when we can honor delay_pool_uses_indirect - // TODO: we should also pass the port details for myportname here. - ch.src_addr = clientConnection->remote; - ch.my_addr = clientConnection->local; - for (unsigned int pool = 0; pool < pools.size(); ++pool) { /* pools require explicit 'allow' to assign a client into them */ @@ -2338,6 +2325,23 @@ ConnStateData::whenClientIpKnown() // kids must extend to actually start doing something (e.g., reading) } +Security::IoResult +ConnStateData::acceptTls() +{ + const auto handshakeResult = Security::Accept(*clientConnection); + +#if USE_OPENSSL + // log ASAP, even if the handshake has not completed (or failed) + const auto fd = clientConnection->fd; + assert(fd >= 0); + keyLogger.checkpoint(*fd_table[fd].ssl, *this); +#else + // TODO: Support fd_table[fd].ssl dereference in other builds. +#endif + + return handshakeResult; +} + /** Handle a new connection on an HTTP socket. */ void httpAccept(const CommAcceptCbParams ¶ms) @@ -2387,7 +2391,7 @@ clientNegotiateSSL(int fd, void *data) { ConnStateData *conn = (ConnStateData *)data; - const auto handshakeResult = Security::Accept(*conn->clientConnection); + const auto handshakeResult = conn->acceptTls(); switch (handshakeResult.category) { case Security::IoResult::ioSuccess: break; @@ -2600,8 +2604,7 @@ ConnStateData::postHttpsAccept() // TODO: Use these request/ALE when waiting for new bumped transactions. ACLFilledChecklist *acl_checklist = new ACLFilledChecklist(Config.accessList.ssl_bump, request, NULL); - acl_checklist->src_addr = clientConnection->remote; - acl_checklist->my_addr = port->s; + fillChecklist(*acl_checklist); // Build a local AccessLogEntry to allow requiresAle() acls work acl_checklist->al = connectAle; acl_checklist->al->cache.start_time = current_time; @@ -2688,9 +2691,8 @@ void ConnStateData::buildSslCertGenerationParams(Ssl::CertificateProperties &cer if (X509 *mimicCert = sslServerBump->serverCert.get()) certProperties.mimicCert.resetAndLock(mimicCert); - ACLFilledChecklist checklist(NULL, sslServerBump->request.getRaw(), - clientConnection != NULL ? clientConnection->rfc931 : dash_str); - checklist.sslErrors = cbdataReference(sslServerBump->sslErrors()); + ACLFilledChecklist checklist(nullptr, sslServerBump->request.getRaw()); + fillChecklist(checklist); for (sslproxy_cert_adapt *ca = Config.ssl_client.cert_adapt; ca != NULL; ca = ca->next) { // If the algorithm already set, then ignore it. @@ -3082,14 +3084,10 @@ ConnStateData::startPeekAndSplice() // Run a accessList check to check if want to splice or continue bumping ACLFilledChecklist *acl_checklist = new ACLFilledChecklist(Config.accessList.ssl_bump, sslServerBump->request.getRaw(), nullptr); - acl_checklist->al = http ? http->al : nullptr; - //acl_checklist->src_addr = params.conn->remote; - //acl_checklist->my_addr = s->s; acl_checklist->banAction(Acl::Answer(ACCESS_ALLOWED, Ssl::bumpNone)); acl_checklist->banAction(Acl::Answer(ACCESS_ALLOWED, Ssl::bumpClientFirst)); acl_checklist->banAction(Acl::Answer(ACCESS_ALLOWED, Ssl::bumpServerFirst)); - const char *log_uri = http ? http->log_uri : nullptr; - acl_checklist->syncAle(sslServerBump->request.getRaw(), log_uri); + fillChecklist(*acl_checklist); acl_checklist->nonBlockingCheck(httpsSslBumpStep2AccessCheckDone, this); return; } @@ -3115,7 +3113,7 @@ ConnStateData::startPeekAndSplice() // expect Security::Accept() to ask us to write (our) TLS server Hello. We // also allow an ioWantRead result in case some fancy TLS extension that // Squid does not yet understand requires reading post-Hello client bytes. - const auto handshakeResult = Security::Accept(*clientConnection); + const auto handshakeResult = acceptTls(); if (!handshakeResult.wantsIo()) return handleSslBumpHandshakeError(handshakeResult); @@ -3592,18 +3590,60 @@ clientAclChecklistCreate(const acl_access * acl, ClientHttpRequest * http) void clientAclChecklistFill(ACLFilledChecklist &checklist, ClientHttpRequest *http) { - checklist.setRequest(http->request); - checklist.al = http->al; - checklist.syncAle(http->request, http->log_uri); - - // TODO: If http->getConn is always http->request->clientConnectionManager, - // then call setIdent() inside checklist.setRequest(). Otherwise, restore - // USE_IDENT lost in commit 94439e4. - ConnStateData * conn = http->getConn(); - const char *ident = (cbdataReferenceValid(conn) && - conn && conn->clientConnection) ? - conn->clientConnection->rfc931 : dash_str; - checklist.setIdent(ident); + assert(http); + + if (!checklist.request && http->request) + checklist.setRequest(http->request); + + if (!checklist.al && http->al) { + checklist.al = http->al; + checklist.syncAle(http->request, http->log_uri); + if (!checklist.reply && http->al->reply) { + checklist.reply = http->al->reply.getRaw(); + HTTPMSGLOCK(checklist.reply); + } + } + + if (const auto conn = http->getConn()) + checklist.setConn(conn); // may already be set +} + +void +ConnStateData::fillChecklist(ACLFilledChecklist &checklist) const +{ + const auto context = pipeline.front(); + if (const auto http = context ? context->http : nullptr) + return clientAclChecklistFill(checklist, http); // calls checklist.setConn() + + // no requests, but we always have connection-level details + // TODO: ACL checks should not require a mutable ConnStateData. Adjust the + // code that accidentally violates that principle to remove this const_cast! + checklist.setConn(const_cast(this)); + + // Set other checklist fields inside our fillConnectionLevelDetails() rather + // than here because clientAclChecklistFill() code path calls that method + // (via ACLFilledChecklist::setConn()) rather than calling us directly. +} + +void +ConnStateData::fillConnectionLevelDetails(ACLFilledChecklist &checklist) const +{ + assert(checklist.conn() == this); + assert(clientConnection); + + if (!checklist.request) { // preserve (better) addresses supplied by setRequest() + checklist.src_addr = clientConnection->remote; + checklist.my_addr = clientConnection->local; // TODO: or port->s? + } + +#if USE_OPENSSL + if (!checklist.sslErrors && sslServerBump) + checklist.sslErrors = cbdataReference(sslServerBump->sslErrors()); +#endif + + if (!checklist.rfc931[0]) // checklist creator may have supplied it already + checklist.setIdent(clientConnection->rfc931); + } bool diff --git a/src/client_side.h b/src/client_side.h index 7bc8f5026e..80da334420 100644 --- a/src/client_side.h +++ b/src/client_side.h @@ -11,7 +11,7 @@ #ifndef SQUID_CLIENTSIDE_H #define SQUID_CLIENTSIDE_H -#include "acl/forward.h" +#include "acl/ChecklistFiller.h" #include "base/RunnersRegistry.h" #include "clientStreamForward.h" #include "comm.h" @@ -27,6 +27,7 @@ #if USE_AUTH #include "auth/UserRequest.h" #endif +#include "security/KeyLogger.h" #if USE_OPENSSL #include "security/forward.h" #include "security/Handshake.h" @@ -75,7 +76,11 @@ class ServerBump; * managing, or for graceful half-close use the stopReceiving() or * stopSending() methods. */ -class ConnStateData : public Server, public HttpControlMsgSink, private IndependentRunner +class ConnStateData: + public Server, + public HttpControlMsgSink, + public Acl::ChecklistFiller, + private IndependentRunner { public: @@ -243,6 +248,10 @@ public: /// The caller assumes responsibility for connection closure detection. void stopPinnedConnectionMonitoring(); + /// Starts or resumes accepting a TLS connection. TODO: Make this helper + /// method protected after converting clientNegotiateSSL() into a method. + Security::IoResult acceptTls(); + /// the second part of old httpsAccept, waiting for future HttpsServer home void postHttpsAccept(); @@ -360,11 +369,24 @@ public: /// emplacement/convenience wrapper for updateError(const Error &) void updateError(const err_type c, const ErrorDetailPointer &d) { updateError(Error(c, d)); } + /* Acl::ChecklistFiller API */ + virtual void fillChecklist(ACLFilledChecklist &) const; + + /// fillChecklist() obligations not fulfilled by the front request + /// TODO: This is a temporary ACLFilledChecklist::setConn() callback to + /// allow filling checklist using our non-public information sources. It + /// should be removed as unnecessary by making ACLs extract the information + /// they need from the ACLFilledChecklist::conn() without filling/copying. + void fillConnectionLevelDetails(ACLFilledChecklist &) const; + // Exposed to be accessible inside the ClientHttpRequest constructor. // TODO: Remove. Make sure there is always a suitable ALE instead. /// a problem that occurred without a request (e.g., while parsing headers) Error bareError; + /// managers logging of the being-accepted TLS connection secrets + Security::KeyLogger keyLogger; + protected: void startDechunkingRequest(); void finishDechunkingRequest(bool withSuccess); diff --git a/src/debug.cc b/src/debug.cc index 59ad1e9505..319f470263 100644 --- a/src/debug.cc +++ b/src/debug.cc @@ -880,14 +880,17 @@ ForceAlert(std::ostream& s) return s; } -/// print data bytes using hex notation void -Raw::printHex(std::ostream &os) const +PrintHex(std::ostream &os, const char *data, const size_t n) { + if (!n) + return; + assert(data); + const auto savedFill = os.fill('0'); const auto savedFlags = os.flags(); // std::ios_base::fmtflags os << std::hex; - std::for_each(data_, data_ + size_, + std::for_each(data, data + n, [&os](const char &c) { os << std::setw(2) << static_cast(c); }); os.flags(savedFlags); os.fill(savedFill); @@ -912,7 +915,7 @@ Raw::print(std::ostream &os) const os << ' '; if (data_) { if (useHex_) - printHex(os); + PrintHex(os, data_, size_); else os.write(data_, size_); } else { diff --git a/src/main.cc b/src/main.cc index 4b3988ec94..e0f9449236 100644 --- a/src/main.cc +++ b/src/main.cc @@ -906,6 +906,7 @@ mainReconfigureStart(void) #if ICAP_CLIENT icapLogClose(); #endif + Security::CloseLogs(); eventAdd("mainReconfigureFinish", &mainReconfigureFinish, NULL, 0, 1, false); @@ -977,6 +978,7 @@ mainReconfigureFinish(void *) Adaptation::Config::Finalize(enableAdaptation); #endif + Security::OpenLogs(); #if ICAP_CLIENT icapLogOpen(); #endif @@ -1049,6 +1051,7 @@ mainRotate(void) storeDirWriteCleanLogs(1); storeLogRotate(); /* store.log */ accessLogRotate(); /* access.log */ + Security::RotateLogs(); #if ICAP_CLIENT icapLogRotate(); /*icap.log*/ #endif @@ -1204,6 +1207,8 @@ mainInitialize(void) accessLogInit(); + Security::OpenLogs(); + #if ICAP_CLIENT icapLogOpen(); #endif diff --git a/src/sbuf/SBuf.h b/src/sbuf/SBuf.h index 74b6eef3a7..be916a8480 100644 --- a/src/sbuf/SBuf.h +++ b/src/sbuf/SBuf.h @@ -145,6 +145,10 @@ public: return *this; } + // XXX: assign(s,n)/append(s,n) calls do not assign or append a c-string as + // documented -- they do not stop at the first NUL character! They assign or + // append the entire raw memory area, including any embedded NUL characters. + /** Import a c-string into a SBuf, copying the data. * * It is the caller's duty to free the imported string, if needed. diff --git a/src/security/CommunicationSecrets.cc b/src/security/CommunicationSecrets.cc new file mode 100644 index 0000000000..1637d6f148 --- /dev/null +++ b/src/security/CommunicationSecrets.cc @@ -0,0 +1,157 @@ +/* + * Copyright (C) 1996-2020 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 "base/CharacterSet.h" +#include "security/Session.h" +#include "security/CommunicationSecrets.h" + +#include + +// TODO: Support SSL_CTX_set_keylog_callback() available since OpenSSL v1.1.1. + +Security::CommunicationSecrets::CommunicationSecrets(const Connection &sconn) +{ +#if USE_OPENSSL + getClientRandom(sconn); + + if (const auto session = SSL_get_session(&sconn)) { + getMasterKey(*session); + getSessionId(*session); + } +#else + // Secret extraction is not supported in builds using other TLS libraries. + // Secret extraction is impractical in builds without TLS libraries. + (void)sconn; +#endif +} + +bool +Security::CommunicationSecrets::gotAll() const +{ + return !id.isEmpty() && !random.isEmpty() && !key.isEmpty(); +} + +bool +Security::CommunicationSecrets::learnNew(const CommunicationSecrets &news) +{ + auto sawChange = false; + + if (id != news.id && !news.id.isEmpty()) { + id = news.id; + sawChange = true; + } + + if (random != news.random && !news.random.isEmpty()) { + random = news.random; + sawChange = true; + } + + if (key != news.key && !news.key.isEmpty()) { + key = news.key; + sawChange = true; + } + + return sawChange; +} + +/// writes the given secret (in hex) or, if there is no secret, a placeholder +static void +PrintSecret(std::ostream &os, const SBuf &secret) +{ + if (!secret.isEmpty()) + PrintHex(os, secret.rawContent(), secret.length()); + else + os << '-'; +} + +void +Security::CommunicationSecrets::record(std::ostream &os) const { + // Print SSLKEYLOGFILE blobs that contain at least one known secret. + // See Wireshark tls_keylog_process_lines() source code for format details. + + // RSA Session-ID:... Master-Key:... + if (id.length() || key.length()) { + os << "RSA"; + PrintSecret(os << " Session-ID:", id); + PrintSecret(os << " Master-Key:", key); + os << "\n"; + } + + // CLIENT_RANDOM ... ... + if (random.length() || key.length()) { + os << "CLIENT_RANDOM "; + PrintSecret(os, random); + os << ' '; + // we may have already printed the key on a separate Master-Key: line above, + // but the CLIENT_RANDOM line format includes the same key info + PrintSecret(os, key); + os << "\n"; + } +} + +#if USE_OPENSSL +/// Clears the given secret if it is likely to contain no secret information. +/// When asked for a secret too early, OpenSSL (successfully!) returns a copy of +/// the secret _storage_ (filled with zeros) rather than an actual secret. +static void +IgnorePlaceholder(SBuf &secret) +{ + static const auto NulChar = CharacterSet("NUL").add('\0'); + if (secret.findFirstNotOf(NulChar) == SBuf::npos) // all zeros + secret.clear(); +} + +void +Security::CommunicationSecrets::getClientRandom(const Connection &sconn) +{ + random.clear(); + const auto expectedLength = SSL_get_client_random(&sconn, nullptr, 0); + if (!expectedLength) + return; + + // no auto due to reinterpret_casting of the result below + char * const space = random.rawAppendStart(expectedLength); + const auto actualLength = SSL_get_client_random(&sconn, + reinterpret_cast(space), expectedLength); + random.rawAppendFinish(space, actualLength); + + IgnorePlaceholder(random); +} + +void +Security::CommunicationSecrets::getSessionId(const Session &session) +{ + id.clear(); + unsigned int idLength = 0; + // no auto due to reinterpret_casting of the result below + const unsigned char * const idStart = SSL_SESSION_get_id(&session, &idLength); + if (idStart && idLength) + id.assign(reinterpret_cast(idStart), idLength); + + IgnorePlaceholder(id); +} + +void +Security::CommunicationSecrets::getMasterKey(const Session &session) +{ + key.clear(); + const auto expectedLength = SSL_SESSION_get_master_key(&session, nullptr, 0); + if (!expectedLength) + return; + + // no auto due to reinterpret_casting of the result below + char * const space = key.rawAppendStart(expectedLength); + const auto actualLength = SSL_SESSION_get_master_key(&session, + reinterpret_cast(space), expectedLength); + key.rawAppendFinish(space, actualLength); + + IgnorePlaceholder(key); +} +#endif /* USE_OPENSSL */ + diff --git a/src/security/CommunicationSecrets.h b/src/security/CommunicationSecrets.h new file mode 100644 index 0000000000..f79e3ff97d --- /dev/null +++ b/src/security/CommunicationSecrets.h @@ -0,0 +1,56 @@ +/* + * Copyright (C) 1996-2020 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_SECURITY_COMMUNICATION_SECRETS_H +#define SQUID_SRC_SECURITY_COMMUNICATION_SECRETS_H + +#include "sbuf/SBuf.h" +#include "security/forward.h" + +#include + +namespace Security { + +/// extracts and formats TLS exchange info for (later) decryption that exchange: +/// early secrets, handshake secrets, (pre)master key, client random, etc. +class CommunicationSecrets +{ +public: + CommunicationSecrets() = default; + explicit CommunicationSecrets(const Connection &sconn); + + /// whether we know all the secrets that could be extracted + bool gotAll() const; + + /// copy all new secrets (i.e. previously unknown or changed) + /// while preserving previously known secrets that have disappeared + /// \returns whether any secrets were copied (i.e. this object has changed) + bool learnNew(const CommunicationSecrets &news); + + /// logs all known secrets using a (multiline) SSLKEYLOGFILE format + void record(std::ostream &) const; + +private: +#if USE_OPENSSL + void getClientRandom(const Connection &sconn); + void getSessionId(const Session &session); + void getMasterKey(const Session &session); +#else + // Secret extraction is not supported in builds using other TLS libraries. + // Secret extraction is impractical in builds without TLS libraries. +#endif + + SBuf id; ///< TLS session ID + SBuf random; ///< CLIENT_RANDOM from the TLS connection + SBuf key; ///< TLS session (pre-)master key +}; + +} // namespace Security + +#endif /* SQUID_SRC_SECURITY_COMMUNICATION_SECRETS_H */ + diff --git a/src/security/KeyLog.cc b/src/security/KeyLog.cc new file mode 100644 index 0000000000..e5027f240f --- /dev/null +++ b/src/security/KeyLog.cc @@ -0,0 +1,120 @@ +/* + * Copyright (C) 1996-2020 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 "acl/FilledChecklist.h" +#include "acl/Gadgets.h" +#include "acl/Tree.h" +#include "base/CharacterSet.h" +#include "base/CodeContext.h" +#include "ConfigOption.h" +#include "HttpReply.h" +#include "log/File.h" +#include "Parsing.h" +#include "sbuf/Stream.h" +#include "security/CommunicationSecrets.h" +#include "security/KeyLog.h" +#include "security/Session.h" +#include "SquidConfig.h" + +Security::KeyLog::KeyLog(ConfigParser &parser) +{ + filename = xstrdup(parser.token("destination").c_str()); + parseOptions(parser, nullptr); + aclList = parser.optionalAclList(); + + // we use a built-in format that does not have/need a dedicated enum value + assert(!type); + assert(!logFormat); + type = Log::Format::CLF_NONE; +} + +void +Security::KeyLog::record(const CommunicationSecrets &secrets) +{ + assert(logfile); + + SBufStream os; + + // report current context to ease small-scale triage of logging problems + os << "# " << logfile->sequence_number; + if (const auto &ctx = CodeContext::Current()) + os << ' ' << *ctx; + os << '\n'; + + secrets.record(os); + const auto buf = os.buf(); + + logfileLineStart(logfile); + logfilePrintf(logfile, SQUIDSBUFPH, SQUIDSBUFPRINT(buf)); + logfileLineEnd(logfile); +} + +void +Security::KeyLog::dump(std::ostream &os) const +{ + os << filename; + dumpOptions(os); + if (aclList) { + // TODO: Use Acl::dump() after fixing the XXX in dump_acl_list(). + for (const auto &acl: aclList->treeDump("if", &Acl::AllowOrDeny)) + os << ' ' << acl; + } +} + +void +Security::OpenLogs() +{ + if (Config.Log.tlsKeys) + Config.Log.tlsKeys->open(); +} + +void +Security::RotateLogs() +{ + if (Config.Log.tlsKeys) + Config.Log.tlsKeys->rotate(); +} + +void +Security::CloseLogs() +{ + if (Config.Log.tlsKeys) + Config.Log.tlsKeys->close(); +} + +// GCC v6 requires "reopening" of the namespace here, instead of the usual +// definitions like Configuration::Component::Parse(): +// error: specialization of Configuration::Component... in different namespace +// TODO: Refactor to use the usual style after we stop GCC v6 support. +namespace Configuration { + +template <> +Security::KeyLog * +Configuration::Component::Parse(ConfigParser &parser) +{ + return new Security::KeyLog(parser); +} + +template <> +void +Configuration::Component::Print(std::ostream &os, Security::KeyLog* const & keyLog) +{ + assert(keyLog); + keyLog->dump(os); +} + +template <> +void +Configuration::Component::Free(Security::KeyLog * const keyLog) +{ + delete keyLog; +} + +} // namespace Configuration + diff --git a/src/security/KeyLog.h b/src/security/KeyLog.h new file mode 100644 index 0000000000..4393e36f18 --- /dev/null +++ b/src/security/KeyLog.h @@ -0,0 +1,38 @@ +/* + * Copyright (C) 1996-2020 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_SECURITY_KEYLOG_H +#define SQUID_SRC_SECURITY_KEYLOG_H + +#include "log/CustomLog.h" +#include "log/forward.h" +#include "sbuf/SBuf.h" +#include "security/forward.h" + +namespace Security { + +/// a single tls_key_log directive configuration and logging handler +class KeyLog: public FormattedLog +{ +public: + explicit KeyLog(ConfigParser&); + + /// whether record() preconditions are currently satisfied + bool canLog() const { return logfile != nullptr; } + + /// writes a single (but multi-line) key log entry + void record(const CommunicationSecrets &); + + /// reproduces explicitly-configured squid.conf settings + void dump(std::ostream &) const; +}; + +} // namespace Security + +#endif /* SQUID_SRC_SECURITY_KEYLOG_H */ + diff --git a/src/security/KeyLogger.cc b/src/security/KeyLogger.cc new file mode 100644 index 0000000000..5eb97582c1 --- /dev/null +++ b/src/security/KeyLogger.cc @@ -0,0 +1,86 @@ +/* + * Copyright (C) 1996-2020 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 "acl/ChecklistFiller.h" +#include "acl/FilledChecklist.h" +#include "MasterXaction.h" +#include "security/CommunicationSecrets.h" +#include "security/KeyLog.h" +#include "security/KeyLogger.h" +#include "security/Session.h" +#include "SquidConfig.h" + +#include + +void +Security::KeyLogger::maybeLog(const Connection &sconn, const Acl::ChecklistFiller &caller) +{ + if (!shouldLog(caller)) { + done_ = true; // do not try again + return; + } + + Security::CommunicationSecrets newSecrets(sconn); + if (!secrets.learnNew(newSecrets)) // no new secrets extracted + return; // will retry extracting secrets during the next checkpoint() + + // SSLKEYLOGFILE consumers probably discard incomplete record lines. To + // avoid providing incomplete/unusable info in _each_ record, we always + // record all the learned secrets, including any previously recorded ones. + Config.Log.tlsKeys->record(secrets); + + // optimization: here, we assume learned secrets do not change + if (secrets.gotAll()) + done_ = true; +} + +bool +Security::KeyLogger::shouldLog(const Acl::ChecklistFiller &caller) const +{ + // First, always check preconditions that may change, becoming unmet/false + + if (!Config.Log.tlsKeys) + return false; // default: admin does not want us to log (implicitly) + + if (!Config.Log.tlsKeys->canLog()) { + debugs(33, 3, "no: problems with the logging module"); + return false; + } + + if (done_) { // paranoid: we should not even be called w/o transaction + debugs(33, 2, "BUG: caller problems or logged earlier"); + return false; + } + + // Second, do the ACL-related checks (that are presumed to be stable) + + // We can keep wanted_ a boolean (instead of a tri-state) member because if + // shouldLog() returns false, there will be no further shouldLog() calls. + if (wanted_) + return true; // was allowed to log earlier + + const auto acls = Config.Log.tlsKeys->aclList; + if (!acls) { + debugs(33, 7, "yes: no ACLs"); + wanted_ = true; + return true; + } + + ACLFilledChecklist checklist; + caller.fillChecklist(checklist); + if (!checklist.fastCheck(acls).allowed()) { + debugs(33, 4, "no: admin does not want us to log (explicitly)"); + return false; + } + + debugs(33, 5, "yes: ACLs matched"); + wanted_ = true; + return true; +} + diff --git a/src/security/KeyLogger.h b/src/security/KeyLogger.h new file mode 100644 index 0000000000..0df795ee85 --- /dev/null +++ b/src/security/KeyLogger.h @@ -0,0 +1,57 @@ +/* + * Copyright (C) 1996-2020 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_SECURITY_KEY_LOGGER_H +#define SQUID_SRC_SECURITY_KEY_LOGGER_H + +#include "acl/forward.h" +#include "security/CommunicationSecrets.h" + +#include + +class MasterXaction; +typedef RefCount MasterXactionPointer; + +namespace Security { + +/// manages collecting and logging secrets of a TLS connection to tls_key_log +class KeyLogger +{ +public: + /// (quickly decides whether logging might be needed and) logs if possible + /// this method is a performance optimization wrapper for slower maybeLog() + void checkpoint(const Connection &, const Acl::ChecklistFiller &); + +private: + /// (slowly checks logging preconditions and) logs if possible + void maybeLog(const Connection &, const Acl::ChecklistFiller &); + + /// (slowly checks) whether logging is possible now + bool shouldLog(const Acl::ChecklistFiller &) const; + + /// connection secrets learned so far + CommunicationSecrets secrets; + + /// whether to prevent further logging attempts + bool done_ = false; + + /// whether we know that the admin wants us to log this connection keys + mutable bool wanted_ = false; +}; + +} // namespace Security + +inline void +Security::KeyLogger::checkpoint(const Connection &sconn, const Acl::ChecklistFiller &caller) +{ + if (!done_) + maybeLog(sconn, caller); +} + +#endif /* SQUID_SRC_SECURITY_KEY_LOGGER_H */ + diff --git a/src/security/Makefile.am b/src/security/Makefile.am index 12b7058468..88cf3fb7b5 100644 --- a/src/security/Makefile.am +++ b/src/security/Makefile.am @@ -16,6 +16,8 @@ libsecurity_la_SOURCES = \ BlindPeerConnector.cc \ BlindPeerConnector.h \ CertError.h \ + CommunicationSecrets.cc \ + CommunicationSecrets.h \ Context.h \ EncryptorAnswer.cc \ EncryptorAnswer.h \ @@ -27,6 +29,10 @@ libsecurity_la_SOURCES = \ Io.h \ KeyData.cc \ KeyData.h \ + KeyLog.cc \ + KeyLog.h \ + KeyLogger.cc \ + KeyLogger.h \ LockingPointer.h \ NegotiationHistory.cc \ NegotiationHistory.h \ diff --git a/src/security/PeerConnector.cc b/src/security/PeerConnector.cc index 9bc307e2cb..6d51b7fec6 100644 --- a/src/security/PeerConnector.cc +++ b/src/security/PeerConnector.cc @@ -86,6 +86,24 @@ Security::PeerConnector::start() mustStop("Security::PeerConnector TLS socket initialize failed"); } +void +Security::PeerConnector::fillChecklist(ACLFilledChecklist &checklist) const +{ + if (!checklist.al) + checklist.al = al; + checklist.syncAle(request.getRaw(), nullptr); + // checklist.fd(fd); XXX: need client FD here + +#if USE_OPENSSL + if (!checklist.serverCert) { + if (const auto session = fd_table[serverConnection()->fd].ssl.get()) + checklist.serverCert.resetWithoutLocking(SSL_get_peer_certificate(session)); + } +#else + // checklist.serverCert is not maintained in other builds +#endif +} + void Security::PeerConnector::commCloseHandler(const CommCloseCbParams ¶ms) { @@ -138,9 +156,7 @@ Security::PeerConnector::initialize(Security::SessionPointer &serverSession) // The list is used in ssl_verify_cb() and is freed in ssl_free(). if (acl_access *acl = ::Config.ssl_client.cert_error) { ACLFilledChecklist *check = new ACLFilledChecklist(acl, request.getRaw(), dash_str); - check->al = al; - check->syncAle(request.getRaw(), nullptr); - // check->fd(fd); XXX: need client FD here + fillChecklist(*check); SSL_set_ex_data(serverSession.get(), ssl_ex_index_cert_error_check, check); } } @@ -189,6 +205,9 @@ Security::PeerConnector::negotiate() #if USE_OPENSSL auto &sconn = *fd_table[fd].ssl; + // log ASAP, even if the handshake has not completed (or failed) + keyLogger.checkpoint(sconn, *this); + // OpenSSL v1 APIs do not allow unthreaded applications like Squid to fetch // missing certificates _during_ OpenSSL certificate validation. Our // handling of X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY (abbreviated @@ -348,9 +367,7 @@ Security::PeerConnector::sslCrtvdCheckForErrors(Ssl::CertValidationResponse cons if (acl_access *acl = ::Config.ssl_client.cert_error) { check = new ACLFilledChecklist(acl, request.getRaw(), dash_str); - check->al = al; - check->syncAle(request.getRaw(), nullptr); - check->serverCert.resetWithoutLocking(SSL_get_peer_certificate(session.get())); + fillChecklist(*check); } Security::CertErrors *errs = nullptr; diff --git a/src/security/PeerConnector.h b/src/security/PeerConnector.h index acf5c4bfe2..819dd1fa9b 100644 --- a/src/security/PeerConnector.h +++ b/src/security/PeerConnector.h @@ -10,12 +10,14 @@ #define SQUID_SRC_SECURITY_PEERCONNECTOR_H #include "acl/Acl.h" +#include "acl/ChecklistFiller.h" #include "base/AsyncCbdataCalls.h" #include "base/AsyncJob.h" #include "CommCalls.h" #include "http/forward.h" #include "security/EncryptorAnswer.h" #include "security/forward.h" +#include "security/KeyLogger.h" #if USE_OPENSSL #include "ssl/support.h" #endif @@ -41,7 +43,7 @@ typedef RefCount IoResultPointer; * Contains common code and interfaces of various specialized PeerConnector's, * including peer certificate validation code. */ -class PeerConnector: virtual public AsyncJob +class PeerConnector: virtual public AsyncJob, public Acl::ChecklistFiller { CBDATA_CLASS(PeerConnector); @@ -74,6 +76,9 @@ protected: virtual void swanSong(); virtual const char *status() const; + /* Acl::ChecklistFiller API */ + virtual void fillChecklist(ACLFilledChecklist &) const; + /// The connection read timeout callback handler. void commTimeoutHandler(const CommTimeoutCbParams &); @@ -188,6 +193,9 @@ private: /// The maximum number of inter-dependent Downloader jobs a worker may initiate static const unsigned int MaxNestedDownloads = 3; + /// managers logging of the being-established TLS connection secrets + Security::KeyLogger keyLogger; + AsyncCall::Pointer closeHandler; ///< we call this when the connection closed time_t negotiationTimeout; ///< the SSL connection timeout to use time_t startTime; ///< when the peer connector negotiation started diff --git a/src/security/Session.h b/src/security/Session.h index a8f7c46da2..4a62fd6e25 100644 --- a/src/security/Session.h +++ b/src/security/Session.h @@ -12,6 +12,7 @@ #include "base/HardFun.h" #include "comm/forward.h" #include "security/LockingPointer.h" +#include "security/forward.h" #include @@ -43,11 +44,19 @@ bool CreateServerSession(const Security::ContextPointer &, const Comm::Connectio #if USE_OPENSSL typedef SSL Connection; +using Session = SSL_SESSION; + typedef std::shared_ptr SessionPointer; typedef std::unique_ptr> SessionStatePointer; #elif USE_GNUTLS +// to be finalized when it is actually needed/used +struct Connection {}; + +// to be finalized when it is actually needed/used +struct Session {}; + typedef std::shared_ptr SessionPointer; // wrapper function to get around gnutls_free being a typedef @@ -57,6 +66,8 @@ typedef std::unique_ptr SessionPointer; typedef std::unique_ptr SessionStatePointer; diff --git a/src/security/forward.h b/src/security/forward.h index 7a5dfcc5de..f7023e4bbe 100644 --- a/src/security/forward.h +++ b/src/security/forward.h @@ -156,7 +156,9 @@ enum Type { // TODO: Either move to Security::Io or remove/restrict the Io namespace. class IoResult; +class CommunicationSecrets; class KeyData; +class KeyLog; #if USE_OPENSSL typedef long ParsedOptions; @@ -188,6 +190,12 @@ class ServerOptions; class ErrorDetail; typedef RefCount ErrorDetailPointer; +std::ostream &operator <<(std::ostream &, const KeyLog &); + +void OpenLogs(); ///< opens logs enabled in the current configuration +void RotateLogs(); ///< rotates logs opened by OpenLogs() +void CloseLogs(); ///< closes logs opened by OpenLogs() + } // namespace Security /// Squid-specific TLS handling errors (a subset of ErrorCode) diff --git a/src/store_client.cc b/src/store_client.cc index 9b76138f32..e6bcbee7b9 100644 --- a/src/store_client.cc +++ b/src/store_client.cc @@ -84,13 +84,6 @@ StoreClient::startCollapsingOn(const StoreEntry &e, const bool doingRevalidation return true; } -void -StoreClient::fillChecklist(ACLFilledChecklist &checklist) const -{ - // TODO: Consider moving all CF-related methods into a new dedicated class. - Must(!"startCollapsingOn() caller must override fillChecklist()"); -} - /* store_client */ bool diff --git a/src/tests/stub_acl.cc b/src/tests/stub_acl.cc index c35b8446aa..12d5bb70a5 100644 --- a/src/tests/stub_acl.cc +++ b/src/tests/stub_acl.cc @@ -9,4 +9,10 @@ /* DEBUG: section 28 Access Control */ #include "squid.h" +#include "acl/Gadgets.h" + +#define STUB_API "acl/" +#include "tests/STUB.h" + +size_t aclParseAclList(ConfigParser &, Acl::Tree **, const char *) STUB_RETVAL(0) diff --git a/src/tests/stub_client_side.cc b/src/tests/stub_client_side.cc index 150cdf9695..a80732edfc 100644 --- a/src/tests/stub_client_side.cc +++ b/src/tests/stub_client_side.cc @@ -41,6 +41,7 @@ void ConnStateData::requestTimeout(const CommTimeoutCbParams &) STUB void ConnStateData::swanSong() STUB void ConnStateData::quitAfterError(HttpRequest *) STUB NotePairs::Pointer ConnStateData::notes() STUB_RETVAL(NotePairs::Pointer()) +void ConnStateData::fillConnectionLevelDetails(ACLFilledChecklist &) const STUB #if USE_OPENSSL void ConnStateData::httpsPeeked(PinnedIdleContext) STUB void ConnStateData::getSslContextStart() STUB @@ -59,4 +60,5 @@ void clientHttpConnectionsClose(void) STUB void httpRequestFree(void *) STUB void clientPackRangeHdr(const HttpReplyPointer &, const HttpHdrRangeSpec *, String, MemBuf *) STUB void clientPackTermBound(String, MemBuf *) STUB +void clientAclChecklistFill(ACLFilledChecklist &, ClientHttpRequest *) STUB diff --git a/src/tests/stub_libsecurity.cc b/src/tests/stub_libsecurity.cc index 8ed26989a9..1ee1889ca5 100644 --- a/src/tests/stub_libsecurity.cc +++ b/src/tests/stub_libsecurity.cc @@ -42,6 +42,9 @@ namespace Security void KeyData::loadFromFiles(const AnyP::PortCfg &, const char *) STUB } +#include "security/KeyLogger.h" +void Security::KeyLogger::maybeLog(const Connection &, const Acl::ChecklistFiller &) STUB + #include "security/ErrorDetail.h" Security::ErrorDetail::ErrorDetail(ErrorCode, const CertPointer &, const CertPointer &, const char *) STUB #if USE_OPENSSL @@ -74,6 +77,7 @@ void PeerConnector::start() STUB bool PeerConnector::doneAll() const STUB_RETVAL(true) void PeerConnector::swanSong() STUB const char *PeerConnector::status() const STUB_RETVAL("") +void PeerConnector::fillChecklist(ACLFilledChecklist &) const STUB void PeerConnector::commCloseHandler(const CommCloseCbParams &) STUB void PeerConnector::commTimeoutHandler(const CommTimeoutCbParams &) STUB bool PeerConnector::initialize(Security::SessionPointer &) STUB_RETVAL(false)