This is an automated email from the ASF dual-hosted git repository. shinrich pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit cbbdd53cc2836df415bb3970edd7bf17b65e30eb Author: Susan Hinrichs <[email protected]> AuthorDate: Fri Aug 2 22:23:08 2019 +0000 Adjust SNI alpn support --- configs/sni.yaml.default | 4 +- doc/admin-guide/files/sni.yaml.en.rst | 12 ++--- iocore/net/P_SNIActionPerformer.h | 30 +++++------ iocore/net/P_SSLNetVConnection.h | 29 +++++++--- iocore/net/P_SSLNextProtocolAccept.h | 7 ++- iocore/net/P_SSLNextProtocolSet.h | 9 +--- iocore/net/P_SSLSNI.h | 5 -- iocore/net/SNIActionPerformer.cc | 58 -------------------- iocore/net/SSLNetProcessor.cc | 1 - iocore/net/SSLNetVConnection.cc | 43 ++++++++++++--- iocore/net/SSLNextProtocolAccept.cc | 15 ++---- iocore/net/SSLNextProtocolSet.cc | 62 +++------------------- iocore/net/SSLSNIConfig.cc | 19 +------ iocore/net/YamlSNIConfig.cc | 6 ++- iocore/net/YamlSNIConfig.h | 8 ++- proxy/http/HttpProxyServerMain.cc | 33 ++---------- src/traffic_server/InkAPI.cc | 8 ++- src/traffic_server/traffic_server.cc | 2 + tests/gold_tests/h2/h2disable.test.py | 4 +- ...test.py => h2disable_no_accept_threads.test.py} | 7 +-- .../h2/{h2disable.test.py => h2enable.test.py} | 30 +++++------ ....test.py => h2enable_no_accept_threads.test.py} | 31 +++++------ 22 files changed, 154 insertions(+), 269 deletions(-) diff --git a/configs/sni.yaml.default b/configs/sni.yaml.default index 6b9080c..021312e 100644 --- a/configs/sni.yaml.default +++ b/configs/sni.yaml.default @@ -7,7 +7,7 @@ # YAML-based Configuration file # Format : # Actions available: -# disable_h2 - removes H2 from the protocol list advertised by ATS; parameter required = None, parameters = true or false +# http2 - adds or removes HTTP/2 (H2) from the protocol list advertised by ATS; parameter required = None, parameters = on or off # verify_client - sets the verification flag for verifying the client certificate; parameters = one of 'NONE', 'MODERATE' or 'STRICT' # verify_origin_server - sets the verification flag for verifying the server certificate; parameters = one of 'NONE', 'MODERATE' or 'STRICT' # client_cert - sets the client certificate to present to the server specified in dest_host; parameters = certificate file . @@ -19,7 +19,7 @@ # Example: # sni: # - fqdn: one.com -# disable_h2: true +# http2: off # verify_origin_server: STRICT # client_cert: somepem.pem # verify_client: MODERATE diff --git a/doc/admin-guide/files/sni.yaml.en.rst b/doc/admin-guide/files/sni.yaml.en.rst index 98f2dff..8556b7a 100644 --- a/doc/admin-guide/files/sni.yaml.en.rst +++ b/doc/admin-guide/files/sni.yaml.en.rst @@ -97,16 +97,14 @@ client_key The file containing the client private key that corres |TS| tries to use a private key in client_cert. Otherwise, :ts:cv:`proxy.config.ssl.client.private_key.filename` is used. +http2 Indicates whether the H2 protocol should be added to or removed from the + protocol negotiation list. The valid values are :code:`on` or :code:`off`. -disable_h2 :code:`true` or :code:`false`. - - If :code:`false` then HTTP/2 is removed from - the valid next protocol list. It is not an error to set this to :code:`false` - for proxy ports on which HTTP/2 is not enabled. +disable_h2 Deprecated for the more general h2 setting. Setting disable_h2 + to :code:`true` is the same as setting http2 to :code:`on`. tunnel_route Destination as an FQDN and port, separated by a colon ``:``. - This will forward all traffic to the specified destination without first terminating the incoming TLS connection. @@ -175,7 +173,7 @@ Disable HTTP/2 for ``no-http2.example.com``. sni: - fqdn: no-http2.example.com - disable_h2: true + http2: off Require client certificate verification for ``example.com`` and any server name ending with ``.yahoo.com``. Therefore, client request for a server name ending with yahoo.com (e.g., def.yahoo.com, abc.yahoo.com etc.) will cause |TS| require and verify the client certificate. By contrast, |TS| will allow a client certificate to be provided for ``example.com`` and if it is, |TS| will require the certificate to be valid. diff --git a/iocore/net/P_SNIActionPerformer.h b/iocore/net/P_SNIActionPerformer.h index a045410..1de840c 100644 --- a/iocore/net/P_SNIActionPerformer.h +++ b/iocore/net/P_SNIActionPerformer.h @@ -34,9 +34,6 @@ #include <vector> #include "P_SSLNextProtocolAccept.h" #include "tscore/ink_inet.h" -#include <unordered_map> - -extern std::unordered_map<int, SSLNextProtocolSet *> snpsMap; class ActionItem { @@ -45,24 +42,28 @@ public: virtual ~ActionItem(){}; }; -class DisableH2 : public ActionItem +class ControlH2 : public ActionItem { public: - DisableH2() {} - ~DisableH2() override {} + ControlH2(bool turn_on) : enable_h2(turn_on) {} + ~ControlH2() override {} int SNIAction(Continuation *cont) const override { - auto ssl_vc = dynamic_cast<SSLNetVConnection *>(cont); - auto accept_obj = ssl_vc ? ssl_vc->accept_object : nullptr; - if (accept_obj && accept_obj->snpa && ssl_vc) { - if (auto it = snpsMap.find(accept_obj->id); it != snpsMap.end()) { - ssl_vc->registerNextProtocolSet(it->second); + auto ssl_vc = dynamic_cast<SSLNetVConnection *>(cont); + if (ssl_vc) { + if (!enable_h2) { + ssl_vc->disableProtocol(TS_ALPN_PROTOCOL_INDEX_HTTP_2_0); + } else if (enable_h2) { + ssl_vc->enableProtocol(TS_ALPN_PROTOCOL_INDEX_HTTP_2_0); } } return SSL_TLSEXT_ERR_OK; } + +private: + bool enable_h2 = false; }; class TunnelDestination : public ActionItem @@ -178,10 +179,3 @@ public: } } }; - -class SNIActionPerformer -{ -public: - SNIActionPerformer() = default; - static int PerformAction(Continuation *cont, const char *servername); -}; diff --git a/iocore/net/P_SSLNetVConnection.h b/iocore/net/P_SSLNetVConnection.h index 952cccb..ef3b625 100644 --- a/iocore/net/P_SSLNetVConnection.h +++ b/iocore/net/P_SSLNetVConnection.h @@ -42,6 +42,7 @@ #include "P_EventSystem.h" #include "P_UnixNetVConnection.h" #include "P_UnixNet.h" +#include "records/I_RecHttp.h" // These are included here because older OpenSSL libraries don't have them. // Don't copy these defines, or use their values directly, they are merely @@ -141,7 +142,7 @@ public: int sslClientHandShakeEvent(int &err); void net_read_io(NetHandler *nh, EThread *lthread) override; int64_t load_buffer_and_write(int64_t towrite, MIOBufferAccessor &buf, int64_t &total_written, int &needs) override; - void registerNextProtocolSet(SSLNextProtocolSet *); + void registerNextProtocolSet(SSLNextProtocolSet *, const SessionProtocolSet &protos); void do_io_close(int lerrno = -1) override; //////////////////////////////////////////////////////////// @@ -161,6 +162,15 @@ public: return npnEndpoint; } + void disableProtocol(int idx); + void enableProtocol(int idx); + + void + setEnabledProtocols(const SessionProtocolSet &protos) + { + this->protoenabled = protos; + } + bool getSSLClientRenegotiationAbort() const { @@ -451,13 +461,16 @@ private: const SSLNextProtocolSet *npnSet = nullptr; Continuation *npnEndpoint = nullptr; - SessionAccept *sessionAcceptPtr = nullptr; - bool sslTrace = false; - int64_t redoWriteSize = 0; - char *tunnel_host = nullptr; - in_port_t tunnel_port = 0; - bool tunnel_decrypt = false; - X509_STORE_CTX *verify_cert = nullptr; + SessionProtocolSet protoenabled; + // Local copies of the npn strings + unsigned char *npn = nullptr; + size_t npnsz = 0; + + int64_t redoWriteSize = 0; + char *tunnel_host = nullptr; + in_port_t tunnel_port = 0; + bool tunnel_decrypt = false; + X509_STORE_CTX *verify_cert = nullptr; }; typedef int (SSLNetVConnection::*SSLNetVConnHandler)(int, void *); diff --git a/iocore/net/P_SSLNextProtocolAccept.h b/iocore/net/P_SSLNextProtocolAccept.h index e9c2c8b..2c2c5de 100644 --- a/iocore/net/P_SSLNextProtocolAccept.h +++ b/iocore/net/P_SSLNextProtocolAccept.h @@ -29,6 +29,7 @@ #include "P_SSLNetVConnection.h" #include "P_SSLNextProtocolSet.h" #include "I_IOBuffer.h" +#include "records/I_RecHttp.h" class SSLNextProtocolAccept : public SessionAccept { @@ -43,13 +44,10 @@ public: // lifetime is at least as long as that of the acceptor. bool registerEndpoint(const char *protocol, Continuation *handler); - // Unregister the handler. Returns false if this protocol is not registered - // or if it is not registered for the specified handler. - bool unregisterEndpoint(const char *protocol, Continuation *handler); + void enableProtocols(const SessionProtocolSet &protos); SLINK(SSLNextProtocolAccept, link); SSLNextProtocolSet *getProtoSet(); - SSLNextProtocolSet *cloneProtoSet(); // noncopyable SSLNextProtocolAccept(const SSLNextProtocolAccept &) = delete; // disabled @@ -61,6 +59,7 @@ private: MIOBuffer *buffer; // XXX do we really need this? Continuation *endpoint; SSLNextProtocolSet protoset; + SessionProtocolSet protoenabled; bool transparent_passthrough; friend struct SSLNextProtocolTrampoline; diff --git a/iocore/net/P_SSLNextProtocolSet.h b/iocore/net/P_SSLNextProtocolSet.h index 61d1ded..8b05b0a 100644 --- a/iocore/net/P_SSLNextProtocolSet.h +++ b/iocore/net/P_SSLNextProtocolSet.h @@ -25,6 +25,7 @@ #include "tscore/List.h" #include "I_Net.h" +#include "records/I_RecHttp.h" class Continuation; @@ -35,12 +36,9 @@ public: ~SSLNextProtocolSet(); bool registerEndpoint(const char *, Continuation *); - bool unregisterEndpoint(const char *, Continuation *); - bool unregisterEndpoint(const char *proto); - bool advertiseProtocols(const unsigned char **out, unsigned *len) const; - SSLNextProtocolSet *clone() const; Continuation *findEndpoint(const unsigned char *, unsigned) const; + bool create_npn_advertisement(const SessionProtocolSet &enabled, unsigned char **npn, size_t *len) const; struct NextProtocolEndpoint { // NOTE: the protocol and endpoint are NOT copied. The caller is @@ -60,8 +58,5 @@ public: SSLNextProtocolSet &operator=(const SSLNextProtocolSet &) = delete; // disabled private: - mutable unsigned char *npn = nullptr; - mutable size_t npnsz = 0; - NextProtocolEndpoint::list_type endpoints; }; diff --git a/iocore/net/P_SSLSNI.h b/iocore/net/P_SSLSNI.h index 970c53a..4cf71ff 100644 --- a/iocore/net/P_SSLSNI.h +++ b/iocore/net/P_SSLSNI.h @@ -38,8 +38,6 @@ #include <strings.h> #include "YamlSNIConfig.h" -#include <unordered_map> - // Properties for the next hop server struct NextHopProperty { std::string name; // name of the server @@ -99,9 +97,7 @@ public: NextHopProperty prop; }; -// typedef HashMap<cchar *, StringHashFns, actionVector *> SNIMap; typedef std::vector<actionElement> SNIList; -// typedef HashMap<cchar *, StringHashFns, NextHopProperty *> NextHopPropertyTable; typedef std::vector<NextHopItem> NextHopPropertyList; struct SNIConfigParams : public ConfigInfo { @@ -123,7 +119,6 @@ struct SNIConfig { static void reconfigure(); static SNIConfigParams *acquire(); static void release(SNIConfigParams *params); - static void cloneProtoSet(); // clones the protoset of all the netaccept objects created and unregisters h2 typedef ConfigProcessor::scoped_config<SNIConfig, SNIConfigParams> scoped_config; diff --git a/iocore/net/SNIActionPerformer.cc b/iocore/net/SNIActionPerformer.cc deleted file mode 100644 index 842345e..0000000 --- a/iocore/net/SNIActionPerformer.cc +++ /dev/null @@ -1,58 +0,0 @@ -/** @file - - A brief file description - - @section license License - - Licensed to the Apache Software Foundation (ASF) under one - or more contributor license agreements. See the NOTICE file - distributed with this work for additional information - regarding copyright ownership. The ASF licenses this file - to you under the Apache License, Version 2.0 (the - "License"); you may not use this file except in compliance - with the License. You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - */ - -/*************************** -*- Mod: C++ -*- ****************************** - SNIActionPerformer.cc - Created On : 05/02/2017 - - Description: - SNI based Configuration in ATS - ****************************************************************************/ -#include "P_SNIActionPerformer.h" -#include "tscore/ink_memory.h" -#include "P_SSLSNI.h" -#include "P_Net.h" -#include "P_SSLNextProtocolAccept.h" -#include "P_SSLUtils.h" - -extern std::unordered_map<int, SSLNextProtocolSet *> snpsMap; - -int -SNIActionPerformer::PerformAction(Continuation *cont, const char *servername) -{ - SNIConfig::scoped_config params; - auto actionvec = params->get(servername); - if (!actionvec) { - Debug("ssl_sni", "%s not available in the map", servername); - } else { - for (auto it : *actionvec) { - if (it) { - auto ret = it->SNIAction(cont); - if (ret != SSL_TLSEXT_ERR_OK) { - return ret; - } - } - } - } - return SSL_TLSEXT_ERR_OK; -} diff --git a/iocore/net/SSLNetProcessor.cc b/iocore/net/SSLNetProcessor.cc index bec9a3d..fd85508 100644 --- a/iocore/net/SSLNetProcessor.cc +++ b/iocore/net/SSLNetProcessor.cc @@ -35,7 +35,6 @@ SSLNetProcessor ssl_NetProcessor; NetProcessor &sslNetProcessor = ssl_NetProcessor; -SNIActionPerformer sni_action_performer; #if TS_USE_TLS_OCSP struct OCSPContinuation : public Continuation { diff --git a/iocore/net/SSLNetVConnection.cc b/iocore/net/SSLNetVConnection.cc index 0c374e1..2dbe2d9 100644 --- a/iocore/net/SSLNetVConnection.cc +++ b/iocore/net/SSLNetVConnection.cc @@ -889,6 +889,11 @@ SSLNetVConnection::clear() SSL_free(ssl); ssl = nullptr; } + if (npn) { + ats_free(npn); + npn = nullptr; + npnsz = 0; + } sslHandshakeStatus = SSL_HANDSHAKE_ONGOING; sslHandshakeBeginTime = 0; @@ -902,7 +907,6 @@ SSLNetVConnection::clear() npnSet = nullptr; npnEndpoint = nullptr; free_handshake_buffers(); - sslTrace = false; super::clear(); } @@ -1454,9 +1458,31 @@ SSLNetVConnection::sslClientHandShakeEvent(int &err) } void -SSLNetVConnection::registerNextProtocolSet(SSLNextProtocolSet *s) +SSLNetVConnection::disableProtocol(int idx) +{ + this->protoenabled.markOut(idx); + // Update the npn string + if (npnSet) { + npnSet->create_npn_advertisement(protoenabled, &npn, &npnsz); + } +} + +void +SSLNetVConnection::enableProtocol(int idx) { - this->npnSet = s; + this->protoenabled.markIn(idx); + // Update the npn string + if (npnSet) { + npnSet->create_npn_advertisement(protoenabled, &npn, &npnsz); + } +} + +void +SSLNetVConnection::registerNextProtocolSet(SSLNextProtocolSet *s, const SessionProtocolSet &protos) +{ + this->protoenabled = protos; + this->npnSet = s; + npnSet->create_npn_advertisement(protoenabled, &npn, &npnsz); } // NextProtocolNegotiation TLS extension callback. The NPN extension @@ -1468,7 +1494,10 @@ SSLNetVConnection::advertise_next_protocol(SSL *ssl, const unsigned char **out, SSLNetVConnection *netvc = SSLNetVCAccess(ssl); ink_release_assert(netvc != nullptr); - if (netvc->npnSet && netvc->npnSet->advertiseProtocols(out, outlen)) { + + if (netvc->npn && netvc->npnsz) { + *out = netvc->npn; + *outlen = netvc->npnsz; // Successful return tells OpenSSL to advertise. return SSL_TLSEXT_ERR_OK; } @@ -1483,14 +1512,12 @@ SSLNetVConnection::select_next_protocol(SSL *ssl, const unsigned char **out, uns const unsigned char *in ATS_UNUSED, unsigned inlen ATS_UNUSED, void *) { SSLNetVConnection *netvc = SSLNetVCAccess(ssl); - const unsigned char *npn = nullptr; - unsigned npnsz = 0; ink_release_assert(netvc != nullptr); - if (netvc->npnSet && netvc->npnSet->advertiseProtocols(&npn, &npnsz)) { + if (netvc->npn && netvc->npnsz) { // SSL_select_next_proto chooses the first server-offered protocol that appears in the clients protocol set, ie. the // server selects the protocol. This is a n^2 search, so it's preferable to keep the protocol set short. - if (SSL_select_next_proto((unsigned char **)out, outlen, npn, npnsz, in, inlen) == OPENSSL_NPN_NEGOTIATED) { + if (SSL_select_next_proto((unsigned char **)out, outlen, netvc->npn, netvc->npnsz, in, inlen) == OPENSSL_NPN_NEGOTIATED) { Debug("ssl", "selected ALPN protocol %.*s", (int)(*outlen), *out); return SSL_TLSEXT_ERR_OK; } diff --git a/iocore/net/SSLNextProtocolAccept.cc b/iocore/net/SSLNextProtocolAccept.cc index ed3fc5a..7d4b39f 100644 --- a/iocore/net/SSLNextProtocolAccept.cc +++ b/iocore/net/SSLNextProtocolAccept.cc @@ -135,7 +135,8 @@ SSLNextProtocolAccept::mainEvent(int event, void *edata) // force the SSLNetVConnection to complete the SSL handshake. Don't tell // the endpoint that there is an accept to handle until the read completes // and we know which protocol was negotiated. - netvc->registerNextProtocolSet(&this->protoset); + netvc->setEnabledProtocols(this->protoenabled); + netvc->registerNextProtocolSet(&this->protoset, this->protoenabled); netvc->do_io_read(new SSLNextProtocolTrampoline(this, netvc->mutex), 0, this->buffer); return EVENT_CONT; default: @@ -159,10 +160,10 @@ SSLNextProtocolAccept::registerEndpoint(const char *protocol, Continuation *hand return this->protoset.registerEndpoint(protocol, handler); } -bool -SSLNextProtocolAccept::unregisterEndpoint(const char *protocol, Continuation *handler) +void +SSLNextProtocolAccept::enableProtocols(const SessionProtocolSet &protos) { - return this->protoset.unregisterEndpoint(protocol, handler); + this->protoenabled = protos; } SSLNextProtocolAccept::SSLNextProtocolAccept(Continuation *ep, bool transparent_passthrough) @@ -177,12 +178,6 @@ SSLNextProtocolAccept::getProtoSet() return &this->protoset; } -SSLNextProtocolSet * -SSLNextProtocolAccept::cloneProtoSet() -{ - return this->protoset.clone(); -} - SSLNextProtocolAccept::~SSLNextProtocolAccept() { free_MIOBuffer(this->buffer); diff --git a/iocore/net/SSLNextProtocolSet.cc b/iocore/net/SSLNextProtocolSet.cc index 04c6330..ccd9941 100644 --- a/iocore/net/SSLNextProtocolSet.cc +++ b/iocore/net/SSLNextProtocolSet.cc @@ -25,6 +25,7 @@ #include "ts/apidefs.h" #include "tscore/ink_platform.h" #include "P_SSLNextProtocolSet.h" +#include "tscpp/util/TextView.h" // For currently defined protocol strings, see // http://technotes.googlecode.com/git/nextprotoneg.html. The OpenSSL @@ -42,8 +43,8 @@ append_protocol(const char *proto, unsigned char *buf) return buf + sz; } -static bool -create_npn_advertisement(const SSLNextProtocolSet::NextProtocolEndpoint::list_type &endpoints, unsigned char **npn, size_t *len) +bool +SSLNextProtocolSet::create_npn_advertisement(const SessionProtocolSet &enabled, unsigned char **npn, size_t *len) const { const SSLNextProtocolSet::NextProtocolEndpoint *ep; unsigned char *advertised; @@ -65,8 +66,10 @@ create_npn_advertisement(const SSLNextProtocolSet::NextProtocolEndpoint::list_ty } for (ep = endpoints.head; ep != nullptr; ep = endpoints.next(ep)) { - Debug("ssl", "advertising protocol %s, %p", ep->protocol, ep->endpoint); - advertised = append_protocol(ep->protocol, advertised); + if (enabled.contains(globalSessionProtocolNameRegistry.toIndex(ts::TextView{ep->protocol, strlen(ep->protocol)}))) { + Debug("ssl", "advertising protocol %s, %p", ep->protocol, ep->endpoint); + advertised = append_protocol(ep->protocol, advertised); + } } return true; @@ -78,31 +81,6 @@ fail: return false; } -// copies the protocols but not the endpoints - -SSLNextProtocolSet * -SSLNextProtocolSet::clone() const -{ - const SSLNextProtocolSet::NextProtocolEndpoint *ep; - SSLNextProtocolSet *newProtoSet = new SSLNextProtocolSet(); - for (ep = this->endpoints.head; ep != nullptr; ep = this->endpoints.next(ep)) { - newProtoSet->registerEndpoint(ep->protocol, ep->endpoint); - } - return newProtoSet; -} - -bool -SSLNextProtocolSet::advertiseProtocols(const unsigned char **out, unsigned *len) const -{ - if (npn && npnsz) { - *out = npn; - *len = npnsz; - return true; - } - - return false; -} - bool SSLNextProtocolSet::registerEndpoint(const char *proto, Continuation *ep) { @@ -115,36 +93,12 @@ SSLNextProtocolSet::registerEndpoint(const char *proto, Continuation *ep) if (!findEndpoint((const unsigned char *)proto, len)) { this->endpoints.push(new NextProtocolEndpoint(proto, ep)); - - if (npn) { - ats_free(npn); - npn = nullptr; - npnsz = 0; - } - create_npn_advertisement(this->endpoints, &npn, &npnsz); - return true; } return false; } -bool -SSLNextProtocolSet::unregisterEndpoint(const char *proto, Continuation *ep) -{ - for (NextProtocolEndpoint *e = this->endpoints.head; e; e = this->endpoints.next(e)) { - if (strcmp(proto, e->protocol) == 0 && (ep == nullptr || e->endpoint == ep)) { - // Protocol must be registered only once; no need to remove - // any more entries. - this->endpoints.remove(e); - create_npn_advertisement(this->endpoints, &npn, &npnsz); - return true; - } - } - - return false; -} - Continuation * SSLNextProtocolSet::findEndpoint(const unsigned char *proto, unsigned len) const { @@ -161,8 +115,6 @@ SSLNextProtocolSet::SSLNextProtocolSet() {} SSLNextProtocolSet::~SSLNextProtocolSet() { - ats_free(this->npn); - for (NextProtocolEndpoint *ep; (ep = this->endpoints.pop());) { delete ep; } diff --git a/iocore/net/SSLSNIConfig.cc b/iocore/net/SSLSNIConfig.cc index 895f698..32b8826 100644 --- a/iocore/net/SSLSNIConfig.cc +++ b/iocore/net/SSLSNIConfig.cc @@ -40,8 +40,6 @@ #include <pcre.h> static ConfigUpdateHandler<SNIConfig> *sniConfigUpdate; -struct NetAccept; -std::unordered_map<int, SSLNextProtocolSet *> snpsMap; const NextHopProperty * SNIConfigParams::getPropertyConfig(const std::string &servername) const @@ -66,8 +64,8 @@ SNIConfigParams::loadSNIConfig() Debug("ssl", "name: %s", item.fqdn.data()); // set SNI based actions to be called in the ssl_servername_only callback - if (item.disable_h2) { - ai->actions.push_back(std::make_unique<DisableH2>()); + if (item.offer_h2.has_value()) { + ai->actions.push_back(std::make_unique<ControlH2>(item.offer_h2.value())); } if (item.verify_client_level != 255) { ai->actions.push_back(std::make_unique<VerifyClient>(item.verify_client_level)); @@ -163,19 +161,6 @@ SNIConfig::startup() } void -SNIConfig::cloneProtoSet() -{ - SCOPED_MUTEX_LOCK(lock, naVecMutex, this_ethread()); - for (auto na : naVec) { - if (na->snpa) { - auto snps = na->snpa->cloneProtoSet(); - snps->unregisterEndpoint(TS_ALPN_PROTOCOL_HTTP_2_0, nullptr); - snpsMap.emplace(na->id, snps); - } - } -} - -void SNIConfig::reconfigure() { SNIConfigParams *params = new SNIConfigParams; diff --git a/iocore/net/YamlSNIConfig.cc b/iocore/net/YamlSNIConfig.cc index d2f6f9d..3ce5123 100644 --- a/iocore/net/YamlSNIConfig.cc +++ b/iocore/net/YamlSNIConfig.cc @@ -103,6 +103,7 @@ std::set<std::string> valid_sni_config_keys = {TS_fqdn, TS_verify_server_properties, TS_client_cert, TS_client_key, + TS_http2, TS_ip_allow #if TS_USE_HELLO_CB , @@ -129,7 +130,10 @@ template <> struct convert<YamlSNIConfig::Item> { return false; // servername must be present } if (node[TS_disable_h2]) { - item.disable_h2 = node[TS_disable_h2].as<bool>(); + item.offer_h2 = false; + } + if (node[TS_http2]) { + item.offer_h2 = node[TS_http2].as<bool>(); } // enum diff --git a/iocore/net/YamlSNIConfig.h b/iocore/net/YamlSNIConfig.h index 0e6b8fc..d528388 100644 --- a/iocore/net/YamlSNIConfig.h +++ b/iocore/net/YamlSNIConfig.h @@ -23,6 +23,7 @@ #include <vector> #include <string> +#include <optional> #include "tscore/Errata.h" @@ -39,6 +40,7 @@ TSDECL(client_cert); TSDECL(client_key); TSDECL(ip_allow); TSDECL(valid_tls_versions_in); +TSDECL(http2); #undef TSDECL const int start = 0; @@ -50,18 +52,20 @@ struct YamlSNIConfig { forward_route, // decrypt data and then blind tunnel action verify_server_policy, // this applies to server side vc only verify_server_properties, // this applies to server side vc only - client_cert + client_cert, + h2 // this applies to client side only }; enum class Level { NONE = 0, MODERATE, STRICT }; enum class Policy : uint8_t { DISABLED = 0, PERMISSIVE, ENFORCED, UNSET }; enum class Property : uint8_t { NONE = 0, SIGNATURE_MASK = 0x1, NAME_MASK = 0x2, ALL_MASK = 0x3, UNSET }; enum class TLSProtocol : uint8_t { TLSv1 = 0, TLSv1_1, TLSv1_2, TLSv1_3, TLS_MAX = TLSv1_3 }; + enum class Control : uint8_t { NONE = 0, ENABLE, DISABLE }; YamlSNIConfig() {} struct Item { std::string fqdn; - bool disable_h2 = false; + std::optional<bool> offer_h2; // Has no value by default, so do not initialize! uint8_t verify_client_level = 255; std::string tunnel_destination; bool tunnel_decrypt = false; diff --git a/proxy/http/HttpProxyServerMain.cc b/proxy/http/HttpProxyServerMain.cc index 70f17b7..78f05a3 100644 --- a/proxy/http/HttpProxyServerMain.cc +++ b/proxy/http/HttpProxyServerMain.cc @@ -95,20 +95,6 @@ ssl_register_protocol(const char *protocol, Continuation *contp) return true; } -bool -ssl_unregister_protocol(const char *protocol, Continuation *contp) -{ - SCOPED_MUTEX_LOCK(lock, ssl_plugin_mutex, this_ethread()); - - for (SSLNextProtocolAccept *ssl = ssl_plugin_acceptors.head; ssl; ssl = ssl_plugin_acceptors.next(ssl)) { - // Ignore possible failure because we want to try to unregister - // from all SSL ports. - ssl->unregisterEndpoint(protocol, contp); - } - - return true; -} - ///////////////////////////////////////////////////////////////// // // main() @@ -236,21 +222,10 @@ MakeHttpProxyAcceptor(HttpProxyAcceptor &acceptor, HttpProxyPort &port, unsigned // the least important protocol first: // http/1.0, http/1.1, h2 - // HTTP - if (port.m_session_protocol_preference.contains(TS_ALPN_PROTOCOL_INDEX_HTTP_1_0)) { - ssl->registerEndpoint(TS_ALPN_PROTOCOL_HTTP_1_0, http); - } - - if (port.m_session_protocol_preference.contains(TS_ALPN_PROTOCOL_INDEX_HTTP_1_1)) { - ssl->registerEndpoint(TS_ALPN_PROTOCOL_HTTP_1_1, http); - } - - // HTTP2 - if (port.m_session_protocol_preference.contains(TS_ALPN_PROTOCOL_INDEX_HTTP_2_0)) { - Http2SessionAccept *acc = new Http2SessionAccept(accept_opt); - - ssl->registerEndpoint(TS_ALPN_PROTOCOL_HTTP_2_0, acc); - } + ssl->enableProtocols(port.m_session_protocol_preference); + ssl->registerEndpoint(TS_ALPN_PROTOCOL_HTTP_1_0, http); + ssl->registerEndpoint(TS_ALPN_PROTOCOL_HTTP_1_1, http); + ssl->registerEndpoint(TS_ALPN_PROTOCOL_HTTP_2_0, new Http2SessionAccept(accept_opt)); SCOPED_MUTEX_LOCK(lock, ssl_plugin_mutex, this_ethread()); ssl_plugin_acceptors.push(ssl); diff --git a/src/traffic_server/InkAPI.cc b/src/traffic_server/InkAPI.cc index edb4c31..268820e 100644 --- a/src/traffic_server/InkAPI.cc +++ b/src/traffic_server/InkAPI.cc @@ -7160,7 +7160,7 @@ TSNetAccept(TSCont contp, int port, int domain, int accept_threads) /* From proxy/http/HttpProxyServerMain.c: */ extern bool ssl_register_protocol(const char *, Continuation *); -extern bool ssl_unregister_protocol(const char *, Continuation *); +// extern bool ssl_unregister_protocol(const char *, Continuation *); TSReturnCode TSNetAcceptNamedProtocol(TSCont contp, const char *protocol) @@ -7170,7 +7170,7 @@ TSNetAcceptNamedProtocol(TSCont contp, const char *protocol) sdk_assert(sdk_sanity_check_continuation(contp) == TS_SUCCESS); if (!ssl_register_protocol(protocol, (INKContInternal *)contp)) { - ssl_unregister_protocol(protocol, (INKContInternal *)contp); + // ssl_unregister_protocol(protocol, (INKContInternal *)contp); return TS_ERROR; } @@ -9415,6 +9415,7 @@ TSSslTicketKeyUpdate(char *ticketData, int ticketDataLen) SSLTicketKeyConfig::reconfigure_data(ticketData, ticketDataLen); } +#ifdef OLD void TSRegisterProtocolSet(TSVConn sslp, TSNextProtocolSet ps) { @@ -9435,6 +9436,7 @@ TSUnregisterProtocol(TSNextProtocolSet protoset, const char *protocol) } return nullptr; } +#endif TSAcceptor TSAcceptorGet(TSVConn sslp) @@ -9467,6 +9469,7 @@ TSAcceptorCount() return naVec.size(); } +#ifdef OLD // clones the protoset associated with netAccept TSNextProtocolSet TSGetcloneProtoSet(TSAcceptor tna) @@ -9475,6 +9478,7 @@ TSGetcloneProtoSet(TSAcceptor tna) // clone protoset return (na && na->snpa) ? reinterpret_cast<TSNextProtocolSet>(na->snpa->cloneProtoSet()) : nullptr; } +#endif tsapi int TSVConnIsSsl(TSVConn sslp) diff --git a/src/traffic_server/traffic_server.cc b/src/traffic_server/traffic_server.cc index 6d50bbb..61fe2cc 100644 --- a/src/traffic_server/traffic_server.cc +++ b/src/traffic_server/traffic_server.cc @@ -1958,7 +1958,9 @@ main(int /* argc ATS_UNUSED */, const char **argv) start_HttpProxyServer(); // PORTS_READY_HOOK called from in here } } +#ifdef OLD SNIConfig::cloneProtoSet(); +#endif // Plugins can register their own configuration names so now after they've done that // check for unexpected names. This is very late because remap plugins must be allowed to // fire up as well. diff --git a/tests/gold_tests/h2/h2disable.test.py b/tests/gold_tests/h2/h2disable.test.py index b784665..945eedf 100644 --- a/tests/gold_tests/h2/h2disable.test.py +++ b/tests/gold_tests/h2/h2disable.test.py @@ -59,9 +59,9 @@ ts.Disk.records_config.update({ ts.Disk.sni_yaml.AddLines([ 'sni:', '- fqdn: bar.com', - ' disable_h2: true', + ' http2: off', '- fqdn: bob.*.com', - ' disable_h2: true', + ' http2: off', ]) tr = Test.AddTestRun("Negotiate-h2") diff --git a/tests/gold_tests/h2/h2disable.test.py b/tests/gold_tests/h2/h2disable_no_accept_threads.test.py similarity index 97% copy from tests/gold_tests/h2/h2disable.test.py copy to tests/gold_tests/h2/h2disable_no_accept_threads.test.py index b784665..bbf5dd0 100644 --- a/tests/gold_tests/h2/h2disable.test.py +++ b/tests/gold_tests/h2/h2disable_no_accept_threads.test.py @@ -53,15 +53,16 @@ ts.Disk.records_config.update({ 'proxy.config.ssl.server.cert.path': '{0}'.format(ts.Variables.SSLDir), 'proxy.config.ssl.server.private_key.path': '{0}'.format(ts.Variables.SSLDir), 'proxy.config.ssl.server.cipher_suite': 'ECDHE-RSA-AES128-GCM-SHA256:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES128-SHA256:ECDHE-RSA-AES256-SHA384:AES128-GCM-SHA256:AES256-GCM-SHA384:ECDHE-RSA-RC4-SHA:ECDHE-RSA-AES128-SHA:ECDHE-RSA-AES256-SHA:RC4-SHA:RC4-MD5:AES128-SHA:AES256-SHA:DES-CBC3-SHA!SRP:!DSS:!PSK:!aNULL:!eNULL:!SSLv2', - 'proxy.config.url_remap.pristine_host_hdr': 1 + 'proxy.config.url_remap.pristine_host_hdr': 1, + 'proxy.config.accept_threads': 0 }) ts.Disk.sni_yaml.AddLines([ 'sni:', '- fqdn: bar.com', - ' disable_h2: true', + ' http2: off', '- fqdn: bob.*.com', - ' disable_h2: true', + ' http2: off', ]) tr = Test.AddTestRun("Negotiate-h2") diff --git a/tests/gold_tests/h2/h2disable.test.py b/tests/gold_tests/h2/h2enable.test.py similarity index 80% copy from tests/gold_tests/h2/h2disable.test.py copy to tests/gold_tests/h2/h2enable.test.py index b784665..7ea7dc5 100644 --- a/tests/gold_tests/h2/h2disable.test.py +++ b/tests/gold_tests/h2/h2enable.test.py @@ -18,7 +18,7 @@ import os Test.Summary = ''' -Test disabling H2 on a per domain basis +Test enabling H2 on a per domain basis ''' # need Curl @@ -27,7 +27,7 @@ Test.SkipUnless( ) # Define default ATS -ts = Test.MakeATSProcess("ts", select_ports=True, enable_tls=True) +ts = Test.MakeATSProcess("ts", select_ports=False, enable_tls=True) server = Test.MakeOriginServer("server") request_header = {"headers": "GET / HTTP/1.1\r\n\r\n", "timestamp": "1469733493.993", "body": ""} @@ -45,53 +45,53 @@ ts.Disk.ssl_multicert_config.AddLine( 'dest_ip=* ssl_cert_name=server.pem ssl_key_name=server.key' ) -# Case 1, global config policy=permissive properties=signature -# override for foo.com policy=enforced properties=all +# Set up port 4444 with HTTP1 only, no HTTP/2 ts.Disk.records_config.update({ 'proxy.config.diags.debug.enabled': 0, 'proxy.config.diags.debug.tags': 'http|ssl', 'proxy.config.ssl.server.cert.path': '{0}'.format(ts.Variables.SSLDir), 'proxy.config.ssl.server.private_key.path': '{0}'.format(ts.Variables.SSLDir), 'proxy.config.ssl.server.cipher_suite': 'ECDHE-RSA-AES128-GCM-SHA256:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES128-SHA256:ECDHE-RSA-AES256-SHA384:AES128-GCM-SHA256:AES256-GCM-SHA384:ECDHE-RSA-RC4-SHA:ECDHE-RSA-AES128-SHA:ECDHE-RSA-AES256-SHA:RC4-SHA:RC4-MD5:AES128-SHA:AES256-SHA:DES-CBC3-SHA!SRP:!DSS:!PSK:!aNULL:!eNULL:!SSLv2', - 'proxy.config.url_remap.pristine_host_hdr': 1 + 'proxy.config.url_remap.pristine_host_hdr': 1, + 'proxy.config.http.server_ports': '{0}:ssl:proto=http {1}'.format(ts.Variables.ssl_port, ts.Variables.port) }) ts.Disk.sni_yaml.AddLines([ 'sni:', '- fqdn: bar.com', - ' disable_h2: true', + ' http2: on', '- fqdn: bob.*.com', - ' disable_h2: true', + ' http2: on', ]) -tr = Test.AddTestRun("Negotiate-h2") +tr = Test.AddTestRun("Do-not-Negotiate-h2") tr.Processes.Default.Command = "curl -v -k --resolve 'foo.com:{0}:127.0.0.1' https://foo.com:{0}".format(ts.Variables.ssl_port) tr.ReturnCode = 0 -tr.Processes.Default.StartBefore(server) -tr.Processes.Default.StartBefore(Test.Processes.ts) +tr.Processes.Default.StartBefore(server, ready=When.PortOpen(server.Variables.Port)) +tr.Processes.Default.StartBefore(Test.Processes.ts, ready=When.PortOpen(ts.Variables.ssl_port)) tr.StillRunningAfter = server tr.StillRunningAfter = ts tr.Processes.Default.TimeOut = 5 tr.Processes.Default.Streams.All = Testers.ExcludesExpression("Could Not Connect", "Curl attempt should have succeeded") -tr.Processes.Default.Streams.All += Testers.ContainsExpression("Using HTTP2", "Curl should negotiate HTTP2") +tr.Processes.Default.Streams.All += Testers.ExcludesExpression("Using HTTP2", "Curl should negotiate HTTP2") tr.TimeOut = 5 -tr2 = Test.AddTestRun("Do not negotiate h2") +tr2 = Test.AddTestRun("Do negotiate h2") tr2.Processes.Default.Command = "curl -v -k --resolve 'bar.com:{0}:127.0.0.1' https://bar.com:{0}".format(ts.Variables.ssl_port) tr2.ReturnCode = 0 tr2.StillRunningAfter = server tr2.Processes.Default.TimeOut = 5 tr2.StillRunningAfter = ts tr2.Processes.Default.Streams.All = Testers.ExcludesExpression("Could Not Connect", "Curl attempt should have succeeded") -tr2.Processes.Default.Streams.All += Testers.ExcludesExpression("Using HTTP2", "Curl should not negotiate HTTP2") +tr2.Processes.Default.Streams.All += Testers.ContainsExpression("Using HTTP2", "Curl should not negotiate HTTP2") tr2.TimeOut = 5 -tr2 = Test.AddTestRun("Do not negotiate h2") +tr2 = Test.AddTestRun("Do negotiate h2") tr2.Processes.Default.Command = "curl -v -k --resolve 'bob.foo.com:{0}:127.0.0.1' https://bob.foo.com:{0}".format(ts.Variables.ssl_port) tr2.ReturnCode = 0 tr2.StillRunningAfter = server tr2.Processes.Default.TimeOut = 5 tr2.StillRunningAfter = ts tr2.Processes.Default.Streams.All = Testers.ExcludesExpression("Could Not Connect", "Curl attempt should have succeeded") -tr2.Processes.Default.Streams.All += Testers.ExcludesExpression("Using HTTP2", "Curl should not negotiate HTTP2") +tr2.Processes.Default.Streams.All += Testers.ContainsExpression("Using HTTP2", "Curl should not negotiate HTTP2") tr2.TimeOut = 5 diff --git a/tests/gold_tests/h2/h2disable.test.py b/tests/gold_tests/h2/h2enable_no_accept_threads.test.py similarity index 80% copy from tests/gold_tests/h2/h2disable.test.py copy to tests/gold_tests/h2/h2enable_no_accept_threads.test.py index b784665..0149e54 100644 --- a/tests/gold_tests/h2/h2disable.test.py +++ b/tests/gold_tests/h2/h2enable_no_accept_threads.test.py @@ -18,7 +18,7 @@ import os Test.Summary = ''' -Test disabling H2 on a per domain basis +Test enabling H2 on a per domain basis ''' # need Curl @@ -27,7 +27,7 @@ Test.SkipUnless( ) # Define default ATS -ts = Test.MakeATSProcess("ts", select_ports=True, enable_tls=True) +ts = Test.MakeATSProcess("ts", select_ports=False, enable_tls=True) server = Test.MakeOriginServer("server") request_header = {"headers": "GET / HTTP/1.1\r\n\r\n", "timestamp": "1469733493.993", "body": ""} @@ -45,53 +45,54 @@ ts.Disk.ssl_multicert_config.AddLine( 'dest_ip=* ssl_cert_name=server.pem ssl_key_name=server.key' ) -# Case 1, global config policy=permissive properties=signature -# override for foo.com policy=enforced properties=all +# Set up port 4444 with HTTP1 only, no HTTP/2 ts.Disk.records_config.update({ 'proxy.config.diags.debug.enabled': 0, 'proxy.config.diags.debug.tags': 'http|ssl', 'proxy.config.ssl.server.cert.path': '{0}'.format(ts.Variables.SSLDir), 'proxy.config.ssl.server.private_key.path': '{0}'.format(ts.Variables.SSLDir), 'proxy.config.ssl.server.cipher_suite': 'ECDHE-RSA-AES128-GCM-SHA256:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES128-SHA256:ECDHE-RSA-AES256-SHA384:AES128-GCM-SHA256:AES256-GCM-SHA384:ECDHE-RSA-RC4-SHA:ECDHE-RSA-AES128-SHA:ECDHE-RSA-AES256-SHA:RC4-SHA:RC4-MD5:AES128-SHA:AES256-SHA:DES-CBC3-SHA!SRP:!DSS:!PSK:!aNULL:!eNULL:!SSLv2', - 'proxy.config.url_remap.pristine_host_hdr': 1 + 'proxy.config.url_remap.pristine_host_hdr': 1, + 'proxy.config.http.server_ports': '{0}:ssl:proto=http {1}'.format(ts.Variables.ssl_port, ts.Variables.port), + 'proxy.config.accept_threads': 0 }) ts.Disk.sni_yaml.AddLines([ 'sni:', '- fqdn: bar.com', - ' disable_h2: true', + ' http2: on', '- fqdn: bob.*.com', - ' disable_h2: true', + ' http2: on', ]) -tr = Test.AddTestRun("Negotiate-h2") +tr = Test.AddTestRun("Do-not-Negotiate-h2") tr.Processes.Default.Command = "curl -v -k --resolve 'foo.com:{0}:127.0.0.1' https://foo.com:{0}".format(ts.Variables.ssl_port) tr.ReturnCode = 0 -tr.Processes.Default.StartBefore(server) -tr.Processes.Default.StartBefore(Test.Processes.ts) +tr.Processes.Default.StartBefore(server, ready=When.PortOpen(server.Variables.Port)) +tr.Processes.Default.StartBefore(Test.Processes.ts, ready=When.PortOpen(ts.Variables.ssl_port)) tr.StillRunningAfter = server tr.StillRunningAfter = ts tr.Processes.Default.TimeOut = 5 tr.Processes.Default.Streams.All = Testers.ExcludesExpression("Could Not Connect", "Curl attempt should have succeeded") -tr.Processes.Default.Streams.All += Testers.ContainsExpression("Using HTTP2", "Curl should negotiate HTTP2") +tr.Processes.Default.Streams.All += Testers.ExcludesExpression("Using HTTP2", "Curl should negotiate HTTP2") tr.TimeOut = 5 -tr2 = Test.AddTestRun("Do not negotiate h2") +tr2 = Test.AddTestRun("Do negotiate h2") tr2.Processes.Default.Command = "curl -v -k --resolve 'bar.com:{0}:127.0.0.1' https://bar.com:{0}".format(ts.Variables.ssl_port) tr2.ReturnCode = 0 tr2.StillRunningAfter = server tr2.Processes.Default.TimeOut = 5 tr2.StillRunningAfter = ts tr2.Processes.Default.Streams.All = Testers.ExcludesExpression("Could Not Connect", "Curl attempt should have succeeded") -tr2.Processes.Default.Streams.All += Testers.ExcludesExpression("Using HTTP2", "Curl should not negotiate HTTP2") +tr2.Processes.Default.Streams.All += Testers.ContainsExpression("Using HTTP2", "Curl should not negotiate HTTP2") tr2.TimeOut = 5 -tr2 = Test.AddTestRun("Do not negotiate h2") +tr2 = Test.AddTestRun("Do negotiate h2") tr2.Processes.Default.Command = "curl -v -k --resolve 'bob.foo.com:{0}:127.0.0.1' https://bob.foo.com:{0}".format(ts.Variables.ssl_port) tr2.ReturnCode = 0 tr2.StillRunningAfter = server tr2.Processes.Default.TimeOut = 5 tr2.StillRunningAfter = ts tr2.Processes.Default.Streams.All = Testers.ExcludesExpression("Could Not Connect", "Curl attempt should have succeeded") -tr2.Processes.Default.Streams.All += Testers.ExcludesExpression("Using HTTP2", "Curl should not negotiate HTTP2") +tr2.Processes.Default.Streams.All += Testers.ContainsExpression("Using HTTP2", "Curl should not negotiate HTTP2") tr2.TimeOut = 5
