From: trapexit Date: Sun, 9 Aug 2020 06:14:51 +0000 (+0000) Subject: Add http_port sslflags=CONDITIONAL_AUTH (#510) X-Git-Tag: 4.15-20210522-snapshot~73 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=983fab6e9ea971228eeb6e09ab55bedf113d1d7b;p=thirdparty%2Fsquid.git Add http_port sslflags=CONDITIONAL_AUTH (#510) Enabling this flag removes SSL_VERIFY_FAIL_IF_NO_PEER_CERT from the SSL_CTX_set_verify callback. Meaning a client certificate verify occurs iff provided. --- diff --git a/src/cf.data.pre b/src/cf.data.pre index e43e875a4a..b62eb6a48a 100644 --- a/src/cf.data.pre +++ b/src/cf.data.pre @@ -2399,6 +2399,11 @@ DOC_START Don't request client certificates immediately, but wait until acl processing requires a certificate (not yet implemented). + CONDITIONAL_AUTH + Request a client certificate during the TLS + handshake, but ignore certificate absence in + the TLS client Hello. If the client does + supply a certificate, it is validated. NO_SESSION_REUSE Don't allow for session reuse. Each connection will result in a new SSL session. diff --git a/src/security/PeerOptions.cc b/src/security/PeerOptions.cc index 7c399a4e4a..b8da60d95d 100644 --- a/src/security/PeerOptions.cc +++ b/src/security/PeerOptions.cc @@ -19,6 +19,8 @@ #include "ssl/support.h" #endif +#include + Security::PeerOptions Security::ProxyOutgoingConfig; Security::PeerOptions::PeerOptions() @@ -539,7 +541,7 @@ Security::PeerOptions::parseOptions() /** * Parses the TLS flags squid.conf parameter */ -long +Security::ParsedPortFlags Security::PeerOptions::parseFlags() { if (sslFlags.isEmpty()) @@ -547,11 +549,12 @@ Security::PeerOptions::parseFlags() static struct { SBuf label; - long mask; + ParsedPortFlags mask; } flagTokens[] = { { SBuf("NO_DEFAULT_CA"), SSL_FLAG_NO_DEFAULT_CA }, { SBuf("DELAYED_AUTH"), SSL_FLAG_DELAYED_AUTH }, { SBuf("DONT_VERIFY_PEER"), SSL_FLAG_DONT_VERIFY_PEER }, + { SBuf("CONDITIONAL_AUTH"), SSL_FLAG_CONDITIONAL_AUTH }, { SBuf("DONT_VERIFY_DOMAIN"), SSL_FLAG_DONT_VERIFY_DOMAIN }, { SBuf("NO_SESSION_REUSE"), SSL_FLAG_NO_SESSION_REUSE }, #if X509_V_FLAG_CRL_CHECK @@ -564,10 +567,11 @@ Security::PeerOptions::parseFlags() ::Parser::Tokenizer tok(sslFlags); static const CharacterSet delims("Flag-delimiter", ":,"); - long fl = 0; + ParsedPortFlags fl = 0; do { - long found = 0; + ParsedPortFlags found = 0; for (size_t i = 0; flagTokens[i].mask; ++i) { + // XXX: skips FOO in FOOBAR, missing merged flags and trailing typos if (tok.skip(flagTokens[i].label)) { found = flagTokens[i].mask; break; @@ -584,6 +588,18 @@ Security::PeerOptions::parseFlags() fl |= found; } while (tok.skipOne(delims)); + const auto mutuallyExclusive = + SSL_FLAG_DONT_VERIFY_PEER| + SSL_FLAG_DELAYED_AUTH| + SSL_FLAG_CONDITIONAL_AUTH; + typedef std::bitset ParsedPortFlagBits; + if (ParsedPortFlagBits(fl & mutuallyExclusive).count() > 1) { + if (fl & SSL_FLAG_CONDITIONAL_AUTH) + throw TextException("CONDITIONAL_AUTH is not compatible with NO_DEFAULT_CA and DELAYED_AUTH flags", Here()); + debugs(83, DBG_PARSE_NOTE(DBG_IMPORTANT), "WARNING: Mixtures of incompatible TLS flags" << + " are deprecated and will become a fatal configuration error"); + } + return fl; } diff --git a/src/security/PeerOptions.h b/src/security/PeerOptions.h index 67463ccccd..5bfd742b81 100644 --- a/src/security/PeerOptions.h +++ b/src/security/PeerOptions.h @@ -11,6 +11,7 @@ #include "base/YesNoNone.h" #include "ConfigParser.h" +#include "security/forward.h" #include "security/KeyData.h" class Packable; @@ -69,7 +70,7 @@ public: virtual void dumpCfg(Packable *, const char *pfx) const; private: - long parseFlags(); + ParsedPortFlags parseFlags(); void loadCrlFile(); void loadKeysFile(); @@ -97,7 +98,7 @@ private: bool optsReparse = true; public: - long parsedFlags = 0; ///< parsed value of sslFlags + ParsedPortFlags parsedFlags = 0; ///< parsed value of sslFlags std::list certs; ///< details from the cert= and file= config parameters std::list caFiles; ///< paths of files containing trusted Certificate Authority diff --git a/src/security/ServerOptions.cc b/src/security/ServerOptions.cc index 117bc0b8d8..182745638d 100644 --- a/src/security/ServerOptions.cc +++ b/src/security/ServerOptions.cc @@ -430,20 +430,13 @@ Security::ServerOptions::updateContextClientCa(Security::ContextPointer &ctx) return; } - if (parsedFlags & SSL_FLAG_DELAYED_AUTH) { - debugs(83, 9, "Not requesting client certificates until acl processing requires one"); - SSL_CTX_set_verify(ctx.get(), SSL_VERIFY_NONE, nullptr); - } else { - debugs(83, 9, "Requiring client certificates."); - Ssl::SetupVerifyCallback(ctx); - } + Ssl::ConfigurePeerVerification(ctx, parsedFlags); updateContextCrl(ctx); updateContextTrust(ctx); } else { - debugs(83, 9, "Not requiring any client certificates"); - SSL_CTX_set_verify(ctx.get(), SSL_VERIFY_NONE, NULL); + Ssl::DisablePeerVerification(ctx); } #endif } diff --git a/src/security/forward.h b/src/security/forward.h index 6219efc123..177328031d 100644 --- a/src/security/forward.h +++ b/src/security/forward.h @@ -49,6 +49,7 @@ #define SSL_FLAG_NO_SESSION_REUSE (1<<4) #define SSL_FLAG_VERIFY_CRL (1<<5) #define SSL_FLAG_VERIFY_CRL_ALL (1<<6) +#define SSL_FLAG_CONDITIONAL_AUTH (1<<7) /// Network/connection security abstraction layer namespace Security @@ -136,6 +137,11 @@ typedef std::shared_ptr ParsedOptions; class ParsedOptions {}; // we never parse/use TLS options in this case #endif +/// bitmask representing configured http(s)_port `sslflags` +/// as well tls_outgoing_options `flags`, cache_peer `sslflags`, and +/// icap_service `tls-flags` +typedef long ParsedPortFlags; + class PeerConnector; class PeerOptions; diff --git a/src/ssl/support.cc b/src/ssl/support.cc index 2b5bec1621..b290a4f76d 100644 --- a/src/ssl/support.cc +++ b/src/ssl/support.cc @@ -390,9 +390,37 @@ ssl_verify_cb(int ok, X509_STORE_CTX * ctx) } void -Ssl::SetupVerifyCallback(Security::ContextPointer &ctx) +Ssl::ConfigurePeerVerification(Security::ContextPointer &ctx, const Security::ParsedPortFlags flags) { - SSL_CTX_set_verify(ctx.get(), SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT, ssl_verify_cb); + int mode; + + // assume each flag is exclusive; flags creator must check this assumption + if (flags & SSL_FLAG_DONT_VERIFY_PEER) { + debugs(83, DBG_IMPORTANT, "SECURITY WARNING: Peer certificates are not verified for validity!"); + debugs(83, DBG_IMPORTANT, "UPGRADE NOTICE: The DONT_VERIFY_PEER flag is deprecated. Remove the clientca= option to disable client certificates."); + mode = SSL_VERIFY_NONE; + } + else if (flags & SSL_FLAG_DELAYED_AUTH) { + debugs(83, DBG_PARSE_NOTE(3), "not requesting client certificates until ACL processing requires one"); + mode = SSL_VERIFY_NONE; + } + else if (flags & SSL_FLAG_CONDITIONAL_AUTH) { + debugs(83, DBG_PARSE_NOTE(3), "will request the client certificate but ignore its absense"); + mode = SSL_VERIFY_PEER; + } + else { + debugs(83, DBG_PARSE_NOTE(3), "Requiring client certificates."); + mode = SSL_VERIFY_PEER|SSL_VERIFY_FAIL_IF_NO_PEER_CERT; + } + + SSL_CTX_set_verify(ctx.get(), mode, (mode != SSL_VERIFY_NONE) ? ssl_verify_cb : nullptr); +} + +void +Ssl::DisablePeerVerification(Security::ContextPointer &ctx) +{ + debugs(83, DBG_PARSE_NOTE(3), "Not requiring any client certificates"); + SSL_CTX_set_verify(ctx.get(),SSL_VERIFY_NONE,nullptr); } // "dup" function for SSL_get_ex_new_index("cert_err_check") @@ -528,7 +556,7 @@ Ssl::InitServerContext(Security::ContextPointer &ctx, AnyP::PortCfg &port) } bool -Ssl::InitClientContext(Security::ContextPointer &ctx, Security::PeerOptions &peer, long fl) +Ssl::InitClientContext(Security::ContextPointer &ctx, Security::PeerOptions &peer, Security::ParsedPortFlags fl) { if (!ctx) return false; @@ -579,13 +607,7 @@ Ssl::InitClientContext(Security::ContextPointer &ctx, Security::PeerOptions &pee MaybeSetupRsaCallback(ctx); - if (fl & SSL_FLAG_DONT_VERIFY_PEER) { - debugs(83, 2, "SECURITY WARNING: Peer certificates are not verified for validity!"); - SSL_CTX_set_verify(ctx.get(), SSL_VERIFY_NONE, NULL); - } else { - debugs(83, 9, "Setting certificate verification callback."); - Ssl::SetupVerifyCallback(ctx); - } + Ssl::ConfigurePeerVerification(ctx, fl); return true; } diff --git a/src/ssl/support.h b/src/ssl/support.h index 28c6e5b3ae..f70bcf2b67 100644 --- a/src/ssl/support.h +++ b/src/ssl/support.h @@ -83,10 +83,11 @@ typedef RefCount CertValidationResponsePointer; bool InitServerContext(Security::ContextPointer &, AnyP::PortCfg &); /// initialize a TLS client context with OpenSSL specific settings -bool InitClientContext(Security::ContextPointer &, Security::PeerOptions &, long flags); +bool InitClientContext(Security::ContextPointer &, Security::PeerOptions &, Security::ParsedPortFlags); /// set the certificate verify callback for a context -void SetupVerifyCallback(Security::ContextPointer &); +void ConfigurePeerVerification(Security::ContextPointer &, const Security::ParsedPortFlags); +void DisablePeerVerification(Security::ContextPointer &); /// if required, setup callback for generating ephemeral RSA keys void MaybeSetupRsaCallback(Security::ContextPointer &); diff --git a/src/tests/stub_libsslsquid.cc b/src/tests/stub_libsslsquid.cc index 4643032cbb..27662d2c39 100644 --- a/src/tests/stub_libsslsquid.cc +++ b/src/tests/stub_libsslsquid.cc @@ -54,8 +54,9 @@ namespace Ssl { int AskPasswordCb(char *, int, int, void *) STUB_RETVAL(0) bool InitServerContext(Security::ContextPointer &, AnyP::PortCfg &) STUB_RETVAL(false) -bool InitClientContext(Security::ContextPointer &, Security::PeerOptions &, const char *) STUB_RETVAL(false) -void SetupVerifyCallback(Security::ContextPointer &) STUB +bool InitClientContext(Security::ContextPointer &, Security::PeerOptions &, Security::ParsedPortFlags) STUB_RETVAL(false) +void ConfigurePeerVerification(Security::ContextPointer &, const Security::ParsedPortFlags) STUB +void DisablePeerVerification(Security::ContextPointer &) STUB void MaybeSetupRsaCallback(Security::ContextPointer &) STUB } // namespace Ssl const char *sslGetUserEmail(SSL *ssl) STUB_RETVAL(NULL)