> On Nov 17, 2014, at 1:12 PM, sudhe...@apache.org wrote:
>
> Repository: trafficserver
> Updated Branches:
> refs/heads/master 08f19da9b -> 34ca6f2dc
>
>
> [TS-3153]: Ability to disable or modify npn advertisement based on SNI
>
>
> Project: http://git-wip-us.apache.org/repos/asf/trafficserver/repo
> Commit: http://git-wip-us.apache.org/repos/asf/trafficserver/commit/24262d8f
> Tree: http://git-wip-us.apache.org/repos/asf/trafficserver/tree/24262d8f
> Diff: http://git-wip-us.apache.org/repos/asf/trafficserver/diff/24262d8f
>
> Branch: refs/heads/master
> Commit: 24262d8f6a14b6bb7bf7288f6309a68f6dc8589b
> Parents: 08f19da
> Author: Sudheer Vinukonda <sudhe...@yahoo-inc.com>
> Authored: Mon Nov 17 21:10:59 2014 +0000
> Committer: Sudheer Vinukonda <sudhe...@yahoo-inc.com>
> Committed: Mon Nov 17 21:10:59 2014 +0000
>
> ----------------------------------------------------------------------
> configure.ac | 1 +
> iocore/net/P_SSLNetVConnection.h | 6 +
> iocore/net/SSLNetVConnection.cc | 82 +++++++-
> iocore/net/SSLUtils.cc | 5 +
> plugins/experimental/Makefile.am | 1 +
> plugins/experimental/sni_proto_nego/Makefile.am | 21 ++
> .../sni_proto_nego/sni_proto_nego.cc | 194 +++++++++++++++++++
> proxy/InkAPI.cc | 10 +
> proxy/api/ts/ts.h | 1 +
> 9 files changed, 320 insertions(+), 1 deletion(-)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/24262d8f/configure.ac
> ----------------------------------------------------------------------
> diff --git a/configure.ac b/configure.ac
> index 3e4465b..91e9874 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1945,6 +1945,7 @@ AS_IF([test "x$enable_experimental_plugins" = xyes], [
> plugins/experimental/regex_revalidate/Makefile
> plugins/experimental/remap_stats/Makefile
> plugins/experimental/s3_auth/Makefile
> + plugins/experimental/sni_proto_nego/Makefile
> plugins/experimental/sslheaders/Makefile
> plugins/experimental/ssl_cert_loader/Makefile
> plugins/experimental/stale_while_revalidate/Makefile
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/24262d8f/iocore/net/P_SSLNetVConnection.h
> ----------------------------------------------------------------------
> diff --git a/iocore/net/P_SSLNetVConnection.h
> b/iocore/net/P_SSLNetVConnection.h
> index c481c8b..1dc7071 100644
> --- a/iocore/net/P_SSLNetVConnection.h
> +++ b/iocore/net/P_SSLNetVConnection.h
> @@ -122,6 +122,9 @@ public:
> static int advertise_next_protocol(SSL * ssl, const unsigned char ** out,
> unsigned * outlen, void *);
> static int select_next_protocol(SSL * ssl, const unsigned char ** out,
> unsigned char * outlen, const unsigned char * in, unsigned inlen, void *);
>
> + bool modify_npn_advertisement(const unsigned char ** list, unsigned cnt);
> + bool setAdvertiseProtocols(const unsigned char ** list, unsigned cnt);
> +
> Continuation * endpoint() const {
> return npnEndpoint;
> }
> @@ -198,6 +201,9 @@ private:
>
> const SSLNextProtocolSet * npnSet;
> Continuation * npnEndpoint;
> + unsigned char * npnAdvertised;
> + size_t npnszAdvertised;
> + int npnAdvertisedBufIndex;
> };
>
> typedef int (SSLNetVConnection::*SSLNetVConnHandler) (int, void *);
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/24262d8f/iocore/net/SSLNetVConnection.cc
> ----------------------------------------------------------------------
> diff --git a/iocore/net/SSLNetVConnection.cc b/iocore/net/SSLNetVConnection.cc
> index 4a9ec29..60fcbf9 100644
> --- a/iocore/net/SSLNetVConnection.cc
> +++ b/iocore/net/SSLNetVConnection.cc
> @@ -27,6 +27,8 @@
> #include "P_SSLUtils.h"
> #include "InkAPIInternal.h" // Added to include the ssl_hook definitions
>
> +extern unsigned char * append_protocol(const char * proto, unsigned char *
> buf);
This is non-static by accident. Please make a static helper API in
SSLNextProtocolSet to construct the advertisement. Maybe something like this:
size_t SSLNextProtocolSet::create_advertisement(const char **, unsigned,
unsigned char * buf, size_t bufsz)
> +
> // Defined in SSLInternal.c, should probably make a separate include
> // file for this at some point
> void SSL_set_rbio(SSLNetVConnection *sslvc, BIO *rbio);
> @@ -776,7 +778,10 @@ SSLNetVConnection::SSLNetVConnection():
> sslPreAcceptHookState(SSL_HOOKS_INIT),
> sslSNIHookState(SNI_HOOKS_INIT),
> npnSet(NULL),
> - npnEndpoint(NULL)
> + npnEndpoint(NULL),
> + npnAdvertised(NULL),
> + npnszAdvertised(0),
> + npnAdvertisedBufIndex(-1)
You don't need to store npnAdvertisedBufIndex, since the size tells you which
iobuf allocator to use.
> {
> }
>
> @@ -815,6 +820,9 @@ SSLNetVConnection::free(EThread * t) {
> hookOpRequested = TS_SSL_HOOK_OP_DEFAULT;
> npnSet = NULL;
> npnEndpoint= NULL;
> + npnAdvertised = NULL;
> + npnszAdvertised = 0;
> + npnAdvertisedBufIndex = -1;
I think you need to release npnAdvertised here.
>
> if (from_accept_thread) {
> sslNetVCAllocator.free(this);
> @@ -1160,6 +1168,14 @@ SSLNetVConnection::advertise_next_protocol(SSL *ssl,
> const unsigned char **out,
>
> ink_release_assert(netvc != NULL);
>
> + // check if there's a SNI based customized advertisement
> + if (netvc->npnAdvertised && netvc->npnszAdvertised) {
> + *out = netvc->npnAdvertised;
> + *outlen = netvc->npnszAdvertised;
> + return SSL_TLSEXT_ERR_OK;
> + }
I believe that Alan's suggestion was to construct a new SSLNextProtocolSet and
attach it to SSLNetVConnection::npnSet. You could use the lower pointer bits to
know whether you have a shared or unique set.
You also need to make some changes to
SSLNetVConnection::select_next_protocol(). We don't distinguish between NPN and
ALPN.
> + // use default endPoint advertisement
> if (netvc->npnSet && netvc->npnSet->advertiseProtocols(out, outlen)) {
> // Successful return tells OpenSSL to advertise.
> return SSL_TLSEXT_ERR_OK;
> @@ -1168,6 +1184,70 @@ SSLNetVConnection::advertise_next_protocol(SSL *ssl,
> const unsigned char **out,
> return SSL_TLSEXT_ERR_NOACK;
> }
>
> +bool
> +SSLNetVConnection::modify_npn_advertisement(const unsigned char ** list,
> unsigned cnt)
I don't really see why you need this. Just do it inline in the caller.
> +{
> + unsigned char* advertised = npnAdvertised;
> +
> + for (unsigned int i=0; i<cnt; i++) {
> + const char* proto = (const char*) list[i];
> + Debug("ssl", "advertising protocol %s", proto);
> + advertised = append_protocol(proto, advertised);
> + }
> +
> + return true;
> +}
> +
> +bool
> +SSLNetVConnection::setAdvertiseProtocols(const unsigned char ** list,
> unsigned cnt)
This should be spelled setAdvertisedProtocols
> +{
> + size_t total_len = 0;
> +
> + if (cnt == 0) {
list == NULL or cnt == 0 should just fail. The npn set already is the default.
I can see that this would let you return to the default after you have set the
advertisement once, but I don't think that would ever get used.
> + // set default list based on server_ports config
> + if (npnAdvertised) {
> + ink_assert (npnAdvertisedBufIndex >= 0);
> + ioBufAllocator[npnAdvertisedBufIndex].free_void(npnAdvertised);
> + npnAdvertised = NULL;
> + npnszAdvertised = 0;
> + npnAdvertisedBufIndex = -1;
> + }
> + return true;
> + }
> +
> + // validate the modified npn list
> + for (unsigned int i=0; i<cnt; i++) {
Style -- there should be single space around binary operators. i = 0; i < cnt
> + const char* proto = (const char*) list[i];
> + size_t len = strlen(proto);
> +
> + // Both ALPN and NPN only allow 255 bytes of protocol name.
> + if (len > 255) {
> + return false;
> + }
There's no way for this to happen since you require the protocols to already be
in the set.
> +
> + if (!npnSet->findEndpoint((const unsigned char *)proto, len)) {
> + return false;
> + }
> + total_len += (len + 1);
> + }
> +
> + if (npnAdvertised) {
> + ink_assert (npnAdvertisedBufIndex >= 0);
> + ioBufAllocator[npnAdvertisedBufIndex].free_void(npnAdvertised);
> + }
> +
> + npnszAdvertised = total_len;
> + npnAdvertisedBufIndex = buffer_size_to_index(npnszAdvertised);
Should this be iobuffer_size_to_index()?
> + npnAdvertised = (unsigned char
> *)ioBufAllocator[npnAdvertisedBufIndex].alloc_void();
> + if (npnAdvertised == NULL) {
> + npnszAdvertised = 0;
> + npnAdvertisedBufIndex = -1;
> + return false;
> + }
> +
> + return modify_npn_advertisement(list, cnt);
> +}
> +
> // ALPN TLS extension callback. Given the client's set of offered
> // protocols, we have to select a protocol to use for this session.
> int
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/24262d8f/iocore/net/SSLUtils.cc
> ----------------------------------------------------------------------
> diff --git a/iocore/net/SSLUtils.cc b/iocore/net/SSLUtils.cc
> index 537cc9d..a6ef4b7 100644
> --- a/iocore/net/SSLUtils.cc
> +++ b/iocore/net/SSLUtils.cc
> @@ -305,6 +305,11 @@ ssl_servername_callback(SSL * ssl, int * ad, void *
> /*arg*/)
> goto done;
> }
>
> + // set the default
> +#if TS_USE_TLS_NPN
> + SSL_CTX_set_next_protos_advertised_cb(ctx,
> SSLNetVConnection::advertise_next_protocol, NULL);
> +#endif /* TS_USE_TLS_NPN */
What is this change for?
> +
> // Call the plugin SNI code
> reenabled = netvc->callHooks(TS_SSL_SNI_HOOK);
> // If it did not re-enable, return the code to
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/24262d8f/plugins/experimental/Makefile.am
> ----------------------------------------------------------------------
> diff --git a/plugins/experimental/Makefile.am
> b/plugins/experimental/Makefile.am
> index 51b06f0..091557d 100644
> --- a/plugins/experimental/Makefile.am
> +++ b/plugins/experimental/Makefile.am
> @@ -33,6 +33,7 @@ SUBDIRS = \
> regex_revalidate \
> remap_stats \
> s3_auth \
> + sni_proto_nego \
> ssl_cert_loader \
> sslheaders \
> stale_while_revalidate \
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/24262d8f/plugins/experimental/sni_proto_nego/Makefile.am
> ----------------------------------------------------------------------
> diff --git a/plugins/experimental/sni_proto_nego/Makefile.am
> b/plugins/experimental/sni_proto_nego/Makefile.am
> new file mode 100644
> index 0000000..958634c
> --- /dev/null
> +++ b/plugins/experimental/sni_proto_nego/Makefile.am
> @@ -0,0 +1,21 @@
> +# 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.
> +
> +include $(top_srcdir)/build/plugins.mk
> +
> +pkglib_LTLIBRARIES = sni_proto_nego.la
> +sni_proto_nego_la_SOURCES = sni_proto_nego.cc
> +sni_proto_nego_la_LDFLAGS = $(TS_PLUGIN_LDFLAGS)
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/24262d8f/plugins/experimental/sni_proto_nego/sni_proto_nego.cc
> ----------------------------------------------------------------------
> diff --git a/plugins/experimental/sni_proto_nego/sni_proto_nego.cc
> b/plugins/experimental/sni_proto_nego/sni_proto_nego.cc
> new file mode 100644
> index 0000000..cd1f4db
> --- /dev/null
> +++ b/plugins/experimental/sni_proto_nego/sni_proto_nego.cc
> @@ -0,0 +1,194 @@
> +#include <stdio.h>
> +#include <ts/ts.h>
> +#include <ts/apidefs.h>
> +#include <openssl/ssl.h>
> +#include <string>
> +#include <map>
> +#include <string.h>
> +
> +using namespace std;
> +
> +const char* PLUGIN_NAME = "sni_proto_nego";
> +const int MAX_BUFFER_SIZE = 1024;
> +const int MAX_FILE_PATH_SIZE = 1024;
> +const unsigned int MAX_PROTO_LIST_LEN = 100;
> +const unsigned int MAX_PROTO_NAME_LEN = 255;
> +
> +typedef struct {
> + bool enableNpn;
> + unsigned int npn_proto_list_count;
> + unsigned char npn_proto_list [MAX_PROTO_LIST_LEN] [MAX_PROTO_NAME_LEN];
> +} SNIProtoConfig;
> +
> +typedef map<string, SNIProtoConfig> stringMap;
> +static stringMap _sniProtoMap;
> +
> +static
> +bool read_config(char* config_file) {
> + char file_path[MAX_FILE_PATH_SIZE];
> + TSFile file;
> + if (config_file == NULL) {
> + TSError("invalid config file");
> + return false;
> + }
> + TSDebug(PLUGIN_NAME, "trying to open config file in this path: %s",
> file_path);
> + file = TSfopen(config_file, "r");
> + if (file == NULL) {
> + snprintf(file_path, sizeof(file_path), "%s/%s", TSInstallDirGet(),
> config_file);
Config files are always relative to TSConfigDirGet()
> + file = TSfopen(file_path, "r");
> + if (file == NULL) {
> + TSError("Failed to open config file %s", config_file);
> + return false;
> + }
> + }
> + char buffer[MAX_BUFFER_SIZE];
> + memset(buffer, 0, sizeof(buffer));
> + while (TSfgets(file, buffer, sizeof(buffer) - 1) != NULL) {
> + char *eol = 0;
> + // make sure line was not bigger than buffer
> + if ((eol = strchr(buffer, '\n')) == NULL && (eol = strstr(buffer,
> "\r\n")) == NULL) {
> + TSError("sni_proto_nego line too long, did not get a good line in cfg,
> skipping, line: %s", buffer);
> + memset(buffer, 0, sizeof(buffer));
> + continue;
> + }
> + // make sure line has something useful on it
> + if (eol - buffer < 2 || buffer[0] == '#') {
> + memset(buffer, 0, sizeof(buffer));
> + continue;
> + }
> + char* cfg = strtok(buffer, "\n\r\n");
> +
> + if (cfg != NULL) {
> + TSDebug(PLUGIN_NAME, "setting SniProto based on string: %s", cfg);
> +
> + char* domain = strtok(buffer, " ");
> + SNIProtoConfig sniProtoConfig = {1, 1};
> +
> + if (domain) {
> + if ((*domain == '*') && (domain+1) && (*(domain+1)=='.')) {
> + domain += 2;
> + if (domain == NULL) {
> + continue;
> + }
> + }
> + char* sni_proto_config = strtok (NULL, " ");
> + if (sni_proto_config) {
> + sniProtoConfig.enableNpn = atoi(sni_proto_config);
> + TSDebug(PLUGIN_NAME, "npn_proto_config %d",
> sniProtoConfig.enableNpn);
> + sni_proto_config = strtok (NULL, " ");
> + // now get the npn proto advertisment list
> + sni_proto_config = strtok (NULL, " ");
> + sniProtoConfig.npn_proto_list_count = 0;
> + while (sni_proto_config != NULL) {
> + char* proto = strtok(NULL, "|");
> + if ((proto == NULL) ||
> + (sniProtoConfig.npn_proto_list_count >=
> MAX_PROTO_LIST_LEN) ||
> + (strlen(proto) >= MAX_PROTO_NAME_LEN)) {
> + break;
> + }
> +
> _TSstrlcpy((char*)sniProtoConfig.npn_proto_list[sniProtoConfig.npn_proto_list_count++],
> proto, (strlen(proto) + 1));
> + }
> + }
> + _sniProtoMap.insert(make_pair(domain, sniProtoConfig));
> + }
> +
> + memset(buffer, 0, sizeof(buffer));
> + }
> + }
> +
> + TSfclose(file);
> +
> + TSDebug(PLUGIN_NAME, "Done parsing config");
> +
> + return true;
> +}
> +
> +
> +static void
> +init_sni_callback(void *sslNetVC)
> +{
> + TSVConn ssl_vc = reinterpret_cast<TSVConn>(sslNetVC);
> + TSSslConnection sslobj = TSVConnSSLConnectionGet(ssl_vc);
> + SSL *ssl = reinterpret_cast<SSL *>(sslobj);
> + const char *serverName = SSL_get_servername(ssl,
> TLSEXT_NAMETYPE_host_name);
> + SSL_CTX * ctx = SSL_get_SSL_CTX(ssl);
> +
> + if (serverName == NULL) {
> + TSDebug(PLUGIN_NAME, "invalid ssl netVC %p, servername %s for ssl obj
> %p", sslNetVC, serverName, ssl);
> + return;
> + }
> +
> + TSDebug(PLUGIN_NAME, "ssl netVC %p, servername %s for ssl obj %p",
> sslNetVC, serverName, ssl);
> +
> + stringMap::iterator it;
> + it=_sniProtoMap.find(serverName);
> +
> + // check for wild-card domains
> + if(it==_sniProtoMap.end()) {
> + char* domain = strstr((char*)serverName, ".");
> + if (domain && (domain+1)) {
> + it=_sniProtoMap.find(domain+1);
> + }
> + }
> +
> + if (it!=_sniProtoMap.end()) {
> + SNIProtoConfig sniProtoConfig = it->second;
> + if (!sniProtoConfig.enableNpn) {
> + TSDebug(PLUGIN_NAME, "disabling NPN for serverName %s", serverName);
> + SSL_CTX_set_next_protos_advertised_cb(ctx, NULL, NULL);
> + } else {
> + TSDebug(PLUGIN_NAME, "setting NPN advertised list for %s", serverName);
> + TSSslAdvertiseProtocolSet(ssl_vc, (const unsigned char
> **)sniProtoConfig.npn_proto_list, sniProtoConfig.npn_proto_list_count);
> + }
> + } else {
> + TSDebug(PLUGIN_NAME, "setting NPN advertised list for %s", serverName);
> + TSSslAdvertiseProtocolSet(ssl_vc, NULL, 0);
> + }
> +}
> +
> +int
> +SSLSniInitCallbackHandler(TSCont cont, TSEvent id, void* sslNetVC) {
> + (void) cont;
> + TSDebug(PLUGIN_NAME, "SSLSniInitCallbackHandler with id %d", id);
> + switch (id) {
> + case TS_SSL_SNI_HOOK:
> + {
> + init_sni_callback(sslNetVC);
> + }
> + break;
> +
> + default:
> + TSDebug(PLUGIN_NAME, "Unexpected event %d", id);
> + break;
> + }
> +
> + return TS_EVENT_NONE;
> +}
> +
> +void
> +TSPluginInit(int argc, const char *argv[])
> +{
> + (void) argc;
> + TSPluginRegistrationInfo info;
> +
> + info.plugin_name = (char *)("sni_proto_nego");
> + info.vendor_name = (char *)("ats");
> +
> + if (TSPluginRegister(TS_SDK_VERSION_3_0, &info) != TS_SUCCESS) {
> + TSError("Plugin registration failed.");
> + }
> +
> + char* config_file = (char*)"conf/sni_proto_nego/sni_proto_nego.config";
Is this some Yahoo thing?
> +
> + if (argc >= 2) {
> + config_file = (char*)argv[1];
> + }
> +
> + if (!read_config(config_file)) {
> + TSDebug(PLUGIN_NAME, "nothing to do..");
> + return;
> + }
> +
> + TSCont cont = TSContCreate(SSLSniInitCallbackHandler, NULL);
> + TSHttpHookAdd(TS_SSL_SNI_HOOK, cont);
> +}
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/24262d8f/proxy/InkAPI.cc
> ----------------------------------------------------------------------
> diff --git a/proxy/InkAPI.cc b/proxy/InkAPI.cc
> index 62f0870..d61e997 100644
> --- a/proxy/InkAPI.cc
> +++ b/proxy/InkAPI.cc
> @@ -8757,6 +8757,16 @@ tsapi int TSVConnIsSsl(TSVConn sslp)
> return ssl_vc != NULL;
> }
>
> +tsapi TSReturnCode
> +TSSslAdvertiseProtocolSet(TSVConn sslp, const unsigned char ** list,
> unsigned int count)
list should be "const char **", there's no unsigned there.
> +{
> + NetVConnection *vc = reinterpret_cast<NetVConnection*>(sslp);
> + SSLNetVConnection *ssl_vc = dynamic_cast<SSLNetVConnection*>(vc);
> + sdk_assert(sdk_sanity_check_null_ptr((void*)ssl_vc) == TS_SUCCESS);
Don't crash if it's not a SSL VC, just fail.
> + ssl_vc->setAdvertiseProtocols(list, count);
Propagate the return code.
> + return TS_SUCCESS;
> +}
> +
> void
> TSVConnReenable(TSVConn vconn)
> {
>
> http://git-wip-us.apache.org/repos/asf/trafficserver/blob/24262d8f/proxy/api/ts/ts.h
> ----------------------------------------------------------------------
> diff --git a/proxy/api/ts/ts.h b/proxy/api/ts/ts.h
> index b5b0abe..8950b5c 100644
> --- a/proxy/api/ts/ts.h
> +++ b/proxy/api/ts/ts.h
> @@ -1238,6 +1238,7 @@ extern "C"
> tsapi TSSslContext TSSslContextFindByAddr(struct sockaddr const*);
> // Returns 1 if the sslp argument refers to a SSL connection
> tsapi int TSVConnIsSsl(TSVConn sslp);
> + tsapi TSReturnCode TSSslAdvertiseProtocolSet(TSVConn sslp, const unsigned
> char ** list, unsigned int count);
>
> /*
> --------------------------------------------------------------------------
> HTTP transactions */
>
