You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by GitBox <gi...@apache.org> on 2021/03/13 02:20:32 UTC
[GitHub] [trafficserver] ywkaras opened a new pull request #7598: Add client_allow_list.so experimental plugin.
ywkaras opened a new pull request #7598:
URL: https://github.com/apache/trafficserver/pull/7598
The plugin will fail TLS handshakes if the none of names of the certificate sent by the client appears
is a configured list of allowed certificates. The list of allowed certificate names can be varied
based on the SNI server name the client sends.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on a change in pull request #7598: Add client_allow_list.so experimental plugin.
Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #7598:
URL: https://github.com/apache/trafficserver/pull/7598#discussion_r594783809
##########
File path: plugins/experimental/client_allow_list/main.cc
##########
@@ -0,0 +1,180 @@
+/** @file
+
+ SSL client certificate verification plugin, main source file.
+
+ Checks for specificate names in the client provided certificate and
+ fails the handshake if none of the good names are present
+
+ @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.
+ */
+
+#include <openssl/ssl.h>
+#include <openssl/x509.h>
+#include <openssl/x509v3.h>
+#include <openssl/asn1.h>
+
+#include "client_allow_list.h"
+
+using namespace client_allow_list_plugin;
+
+#if TS_VERSION_MAJOR < 9
+#define TSVConnSslConnectionGet TSVConnSSLConnectionGet
+#endif
+
+namespace
+{
+inline const char *
+ASN1_get_data(ASN1_STRING *cn)
+{
+#if OPENSSL_VERSION_NUMBER < 0x10100000L
Review comment:
In configure.ac?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on a change in pull request #7598: Add client_allow_list.so experimental plugin.
Posted by GitBox <gi...@apache.org>.
maskit commented on a change in pull request #7598:
URL: https://github.com/apache/trafficserver/pull/7598#discussion_r593620934
##########
File path: plugins/experimental/client_allow_list/client_allow_list.h
##########
@@ -0,0 +1,301 @@
+/** @file
+
+ SSL client certificate verification plugin, header file.
+
+ Checks for specificate names in the client provided certificate and
+ fails the handshake if none of the good names are present
+
+ @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.
+ */
+
+#pragma once
+
+#include <vector>
+#include <string>
+#include <unordered_map>
+#include <utility>
+#include <algorithm>
+
+#include <pcre.h>
+
+#if defined(CLIENT_ALLOW_LIST_UNIT_TEST)
+
+struct ClientAllowListUTException {
+};
+
+#else
+
+#include <ts/ts.h>
+
+#endif
+
+#include <string_view>
+
+#define PN "client_allow_list"
+
+namespace client_allow_list_plugin
+{
+struct cname_matcher {
+ std::string _CName;
+ pcre *_compiled_RE = nullptr;
+
+ cname_matcher() = default;
+
+ // No copying.
+ //
+ cname_matcher(cname_matcher const &) = delete;
+ cname_matcher &operator=(cname_matcher const &) = delete;
+
+ // Must be moveable to be able to make a vector of these.
+ //
+ cname_matcher(cname_matcher &&that)
+ {
+ _CName = std::move(that._CName);
+ _compiled_RE = that._compiled_RE;
+ that._compiled_RE = nullptr;
+ }
+ cname_matcher &
+ operator=(cname_matcher &&that)
+ {
+ this->~cname_matcher();
+ ::new (this) cname_matcher(std::move(that));
+ return *this;
+ }
+
+ ~cname_matcher()
+ {
+ if (_compiled_RE != nullptr) {
+ pcre_free(_compiled_RE);
+ }
+ }
+};
+
+// Matchers for cert subject/associated names.
+//
+extern std::vector<cname_matcher> matcher;
+
+// Indexes into matcher vector of matchers to use if there is no list of matchers to use specific to the
+// SNI server name.
+//
+extern std::vector<unsigned> other_matcher_idxs;
+
+// Indexes into matcher vector of matchers to use if there is no list of matchers to use specific to the
+// SNI server name.
+//
+extern std::vector<unsigned> none_matcher_idxs;
+
+// Lookup table from C strings to vectors of unsigned. Comparison of string keys is case-insensitive.
+//
+class MapCStrToUVec
+{
+public:
+ MapCStrToUVec() {}
+
+ // Adds a new entry. Saves a copy of the key string, returns a pointer to the new, empty vector.
+ // Returns nullptr if there is already an entry for key.
+ //
+ std::vector<unsigned> *add(std::string_view key);
+
+ // Returns nullptr if no entry with given key.
+ //
+ std::vector<unsigned> const *find(char const *key);
+
+ std::size_t
+ size() const
+ {
+ return _map.size();
+ }
+
+ ~MapCStrToUVec();
+
+ // No copying.
+ //
+ MapCStrToUVec(MapCStrToUVec const &) = delete;
+ MapCStrToUVec &operator=(MapCStrToUVec const &) = delete;
+
+private:
+ struct _Hash {
+ std::size_t operator()(char const *key) const;
+ };
+
+ struct _Eq {
+ bool operator()(char const *lhs, char const *rhs) const;
+ };
+
+ std::unordered_map<char const *, std::vector<unsigned>, _Hash, _Eq> _map;
+};
+
+class HashFnv1a32
Review comment:
If there's some reasons to not use `ATSHash32FNV1a` we might want to improve it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on a change in pull request #7598: Add client_allow_list.so experimental plugin.
Posted by GitBox <gi...@apache.org>.
maskit commented on a change in pull request #7598:
URL: https://github.com/apache/trafficserver/pull/7598#discussion_r594772689
##########
File path: plugins/experimental/client_allow_list/main.cc
##########
@@ -0,0 +1,180 @@
+/** @file
+
+ SSL client certificate verification plugin, main source file.
+
+ Checks for specificate names in the client provided certificate and
+ fails the handshake if none of the good names are present
+
+ @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.
+ */
+
+#include <openssl/ssl.h>
+#include <openssl/x509.h>
+#include <openssl/x509v3.h>
+#include <openssl/asn1.h>
+
+#include "client_allow_list.h"
+
+using namespace client_allow_list_plugin;
+
+#if TS_VERSION_MAJOR < 9
+#define TSVConnSslConnectionGet TSVConnSSLConnectionGet
+#endif
+
+namespace
+{
+inline const char *
+ASN1_get_data(ASN1_STRING *cn)
+{
+#if OPENSSL_VERSION_NUMBER < 0x10100000L
Review comment:
Checking availability of the function.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras removed a comment on pull request #7598: Add client_allow_list.so experimental plugin.
Posted by GitBox <gi...@apache.org>.
ywkaras removed a comment on pull request #7598:
URL: https://github.com/apache/trafficserver/pull/7598#issuecomment-799852321
[approve CI FreeBSD]
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on a change in pull request #7598: Add client_allow_list.so experimental plugin.
Posted by GitBox <gi...@apache.org>.
maskit commented on a change in pull request #7598:
URL: https://github.com/apache/trafficserver/pull/7598#discussion_r593570744
##########
File path: plugins/experimental/client_allow_list/main.cc
##########
@@ -0,0 +1,180 @@
+/** @file
+
+ SSL client certificate verification plugin, main source file.
+
+ Checks for specificate names in the client provided certificate and
+ fails the handshake if none of the good names are present
+
+ @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.
+ */
+
+#include <openssl/ssl.h>
+#include <openssl/x509.h>
+#include <openssl/x509v3.h>
+#include <openssl/asn1.h>
+
+#include "client_allow_list.h"
+
+using namespace client_allow_list_plugin;
+
+#if TS_VERSION_MAJOR < 9
+#define TSVConnSslConnectionGet TSVConnSSLConnectionGet
+#endif
+
+namespace
+{
+inline const char *
+ASN1_get_data(ASN1_STRING *cn)
+{
+#if OPENSSL_VERSION_NUMBER < 0x10100000L
Review comment:
I don't recommend to use `OPENSSL_VERSION_NUMBER` for new code because it's going to be deprecated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras closed pull request #7598: Add client_allow_list.so experimental plugin.
Posted by GitBox <gi...@apache.org>.
ywkaras closed pull request #7598:
URL: https://github.com/apache/trafficserver/pull/7598
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bryancall commented on pull request #7598: Add client_allow_list.so experimental plugin.
Posted by GitBox <gi...@apache.org>.
bryancall commented on pull request #7598:
URL: https://github.com/apache/trafficserver/pull/7598#issuecomment-802309132
@ywkaras How about switching to std::hash and use string_view?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on a change in pull request #7598: Add client_allow_list.so experimental plugin.
Posted by GitBox <gi...@apache.org>.
maskit commented on a change in pull request #7598:
URL: https://github.com/apache/trafficserver/pull/7598#discussion_r593569158
##########
File path: plugins/experimental/client_allow_list/main.cc
##########
@@ -0,0 +1,180 @@
+/** @file
+
+ SSL client certificate verification plugin, main source file.
+
+ Checks for specificate names in the client provided certificate and
+ fails the handshake if none of the good names are present
+
+ @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.
+ */
+
+#include <openssl/ssl.h>
+#include <openssl/x509.h>
+#include <openssl/x509v3.h>
+#include <openssl/asn1.h>
+
+#include "client_allow_list.h"
+
+using namespace client_allow_list_plugin;
+
+#if TS_VERSION_MAJOR < 9
+#define TSVConnSslConnectionGet TSVConnSSLConnectionGet
+#endif
+
+namespace
+{
+inline const char *
+ASN1_get_data(ASN1_STRING *cn)
+{
+#if OPENSSL_VERSION_NUMBER < 0x10100000L
+ return reinterpret_cast<const char *>(ASN1_STRING_data(cn));
+#else
+ return reinterpret_cast<const char *>(ASN1_STRING_get0_data(cn));
+#endif
+}
+
+bool
+check_names(std::vector<unsigned> const &matcher_idxs, X509 *cert)
+{
+ bool retval = false;
+
+ // Check the common name
+ X509_NAME *subject = X509_get_subject_name(cert);
+ if (subject) {
+ int pos = -1;
+ for (; !retval;) {
+ pos = X509_NAME_get_index_by_NID(subject, NID_commonName, pos);
+ if (pos == -1) {
+ break;
+ }
+
+ X509_NAME_ENTRY *e = X509_NAME_get_entry(subject, pos);
+ ASN1_STRING *cn = X509_NAME_ENTRY_get_data(e);
+ std::string_view subj_name{ASN1_get_data(cn), static_cast<std::size_t>(ASN1_STRING_length(cn))};
+ TSDebug(PN, "checking cert name %.*s", static_cast<int>(subj_name.size()), subj_name.data());
+ retval = check_name(matcher_idxs, subj_name);
+ }
+ }
+ if (!retval) {
+ // Check the subjectAltNanes (if present)
+ GENERAL_NAMES *names = (GENERAL_NAMES *)X509_get_ext_d2i(cert, NID_subject_alt_name, nullptr, nullptr);
+ if (names) {
+ unsigned count = sk_GENERAL_NAME_num(names);
+ for (unsigned i = 0; i < count && !retval; ++i) {
+ GENERAL_NAME *name = sk_GENERAL_NAME_value(names, i);
+ if (name->type == GEN_DNS) {
+ std::string_view dns{ASN1_get_data(name->d.dNSName), static_cast<std::size_t>(ASN1_STRING_length(name->d.dNSName))};
+ retval = check_name(matcher_idxs, dns);
+ }
+ }
+ GENERAL_NAMES_free(names);
+ }
+ }
+ return retval;
+}
+
+int
+CB_client_verify(TSCont cont, TSEvent event, void *edata)
+{
+ TSVConn ssl_vc = reinterpret_cast<TSVConn>(edata);
+
+ std::vector<unsigned> const *matcher_idxs = &none_matcher_idxs;
+
+ // See if we should use a different list of matchers based on Server Name Indication.
+ {
+ TSSslConnection ssl_conn = TSVConnSslConnectionGet(ssl_vc);
+
+ if (!ssl_conn) {
+ TSDebug(PN, "Could not get SSL object for SSL TSVConn %p", ssl_vc);
+ } else {
+ auto ssl_obj = reinterpret_cast<SSL *>(ssl_conn);
+ int sname_type = SSL_get_servername_type(ssl_obj);
+ if (sname_type < 0) {
+ TSDebug(PN, "No SNI servername for SSL TSVConn %p", ssl_vc);
+ } else {
+ // In versions of OpenSSL older than 1.1.1e, this call will return incorrect results in rare cases.
+ // But this call is used in core TS, and the failure cases have not caused problems in production.
+ //
+ char const *sname = SSL_get_servername(ssl_obj, sname_type);
+ if (!sname) {
+ TSDebug(PN, "SSL_get_servername() call failed for SSL TSVConn %p", ssl_vc);
+ } else {
+ auto p = sname_to_matcher_idxs.find(sname);
+ if (p) {
+ TSDebug(PN, "Using specific list of allowed client cert subject/associate names for SNI server name %s", sname);
+ matcher_idxs = p;
+ } else {
+ TSDebug(PN, "No specific list of allowed client cert subject/associate named for SNI server name %s", sname);
+ matcher_idxs = &other_matcher_idxs;
+ }
+ }
+ }
+ }
+ }
+
+ // Is this a good name or not?
+ TSEvent reenable_event = TS_EVENT_CONTINUE;
+ X509_STORE_CTX *ctx = reinterpret_cast<X509_STORE_CTX *>(TSVConnSslVerifyCTXGet(ssl_vc));
+ if (ctx) {
+ STACK_OF(X509) *chain = X509_STORE_CTX_get1_chain(ctx);
+ // X509 *cert = X509_STORE_CTX_get_current_cert(ctx);
+ bool retval = false;
+ for (int i = 0; i < sk_X509_num(chain) && !retval; i++) {
Review comment:
```
experimental/client_allow_list/main.cc:138:23: warning: comparison of integer expressions of different signedness: ‘intand ‘size_t’ {aka ‘long unsigned int’} [-Wsign-compare]
138 | for (int i = 0; i < sk_X509_num(chain) && !retval; i++) {
| ~~^~~~~~~~~~~~~~~~~~~~
```
There's very similar code in `tests/tools/plugins/ssl_client_verify_test.cc`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] alficles commented on a change in pull request #7598: Add client_allow_list.so experimental plugin.
Posted by GitBox <gi...@apache.org>.
alficles commented on a change in pull request #7598:
URL: https://github.com/apache/trafficserver/pull/7598#discussion_r593726203
##########
File path: plugins/experimental/client_allow_list/util.cc
##########
@@ -0,0 +1,349 @@
+/** @file
+
+ SSL client certificate verification plugin, utility source file.
+
+ Checks for specificate names in the client provided certificate and
+ fails the handshake if none of the good names are present
+
+ @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.
+ */
+
+#include <cctype>
+#include <cstddef>
+#include <cstdio>
+#include <cstring>
+
+#include <yaml-cpp/yaml.h>
+
+#include "client_allow_list.h"
+
+#if defined(CLIENT_ALLOW_LIST_UNIT_TEST)
+
+#include <cstdarg>
+
+extern void ut_printf(char const *fmt, std::va_list args);
+
+namespace
+{
+char const *
+TSConfigDirGet()
+{
+ return ".";
+}
+
+void
+TSError(char const *fmt, ...)
+{
+ std::va_list args;
+
+ va_start(args, fmt);
+ ut_printf(fmt, args);
+ va_end(args);
+}
+
+[[noreturn]] void
+TSEmergency(char const *fmt, ...)
+{
+ std::va_list args;
+
+ va_start(args, fmt);
+ ut_printf(fmt, args);
+ va_end(args);
+
+ throw ClientAllowListUTException();
+}
+
+} // end anonymous namespace
+
+#undef TSAssert
+#define TSAssert(EXPR) ((EXPR) ? static_cast<void>(0) : TSEmergency("Assert Failed line=%d", __LINE__))
+
+#endif // defined(CLIENT_ALLOW_LIST_UNIT_TEST)
+
+namespace
+{
+[[noreturn]] void
+bad_node(std::string const &filespec, YAML::Node const &node)
+{
+ TSEmergency(PN ": config error: file=%s line=%d column=%d", filespec.c_str(), node.Mark().line + 1, node.Mark().column + 1);
+
+ throw 0; // Never executed, but avoids compiler warning that this function returns.
+}
+
+} // end anonymous namespace
+
+namespace client_allow_list_plugin
+{
+std::vector<cname_matcher> matcher;
+
+std::vector<unsigned> other_matcher_idxs;
+
+std::vector<unsigned> none_matcher_idxs;
+
+MapCStrToUVec::~MapCStrToUVec()
+{
+ char const *key;
+ auto it = _map.begin();
+ while (it != _map.end()) {
+ key = it->first;
+ _map.erase(it);
+ delete[] key;
+ it = _map.begin();
+ }
+}
+
+MapCStrToUVec sname_to_matcher_idxs;
+
+bool
+check_name(std::vector<unsigned> const &matcher_idxs, std::string_view name)
+{
+ bool name_matched{false};
+ int strvec[30];
+ for (unsigned idx : matcher_idxs) {
+ auto &a_matcher = matcher[idx];
+ if (a_matcher._compiled_RE == nullptr) {
+ if (std::string_view(a_matcher._CName) == name) {
+ name_matched = true;
+ break;
+ }
+ } else {
+ int exec_res{pcre_exec(a_matcher._compiled_RE, nullptr, name.data(), name.size(), 0, 0, strvec, 30)};
+ if (1 == exec_res) {
+ name_matched = true;
+ break;
+
+ } else if (exec_res != PCRE_ERROR_NOMATCH) {
+ TSError(PN ": bad result from pcre_exec=%d for name=%.*s, matcher named %s, idx=%u", exec_res,
+ static_cast<int>(name.size()), name.data(), a_matcher._CName.c_str(), idx);
+ }
+ }
+ }
+ return name_matched;
+}
+
+void
+Init::_yaml_process(char const *config_filespec)
+{
+ std::string filespec;
+ if (config_filespec[0] != '/') {
+ char const *config_dir = TSConfigDirGet();
+
+ if (config_dir) {
+ filespec = std::string(config_dir) + '/';
+ }
+ }
+
+ filespec += config_filespec;
+
+ try {
+ YAML::Node config = YAML::LoadFile(filespec);
+
+ bool none_seen{false}, other_seen{false};
+
+ for (YAML::const_iterator it = config.begin(); it != config.end(); ++it) {
+ std::string key = it->first.as<std::string>();
+
+ std::vector<std::vector<unsigned> *> list_of_matcher_idx_vectors;
+ std::string_view sname;
+ if (key.size() == 0) {
+ TSError(PN ": empty server name list");
+ bad_node(filespec, it->first);
+ }
+ if (std::string::npos != key.find_first_of(std::string_view(" \t\n"))) {
+ TSError(PN ": blank space not allowed in server name list");
+ bad_node(filespec, it->first);
+ }
+ std::size_t pos{0}, end_pos;
+ while (pos < key.size()) {
+ end_pos = key.find_first_of(std::string_view("|,"), pos);
+ if (std::string::npos == end_pos) {
+ end_pos = key.size();
+ } else if (end_pos == pos) {
+ TSError(PN ": empty server name in server name list");
+ bad_node(filespec, it->first);
+ }
+ sname = key.substr(pos, end_pos - pos);
+ pos = end_pos + 1;
+ if ("<none>" == sname) {
+ if (none_seen) {
+ TSError(PN ": <none> used more than once");
+ bad_node(filespec, it->first);
+ }
+ none_seen = true;
+ list_of_matcher_idx_vectors.push_back(&none_matcher_idxs);
+
+ } else if ("<other>" == sname) {
+ if (other_seen) {
+ TSError(PN ": <other> used more than once");
+ bad_node(filespec, it->first);
+ }
+ other_seen = true;
+ list_of_matcher_idx_vectors.push_back(&other_matcher_idxs);
+
+ } else {
+ auto vp = sname_to_matcher_idxs.add(sname);
+ if (!vp) {
+ TSError(PN ": cert names for SNI server name \"%.*s\" previously specified", static_cast<int>(sname.size()),
+ sname.data());
+ bad_node(filespec, it->first);
+ }
+ list_of_matcher_idx_vectors.push_back(vp);
+ }
+ }
+ TSAssert(list_of_matcher_idx_vectors.size() > 0);
+
+ _Populator pop(this);
+ if (it->second.IsSequence()) {
+ for (YAML::const_iterator it2 = it->second.begin(); it2 != it->second.end(); ++it2) {
+ if (!pop.add_cert_name(it2->as<std::string>().c_str())) {
+ bad_node(filespec, *it2);
+ }
+ }
+
+ } else {
+ // Value is scalar, single cert name.
+ if (!pop.add_cert_name(it->second.as<std::string>().c_str())) {
+ bad_node(filespec, it->second);
+ }
+ }
+ *list_of_matcher_idx_vectors[0] = std::move(pop.matcher_idxs);
+
+ // If there is more than one SNI server name, make a copy of the vector of matcher indexes for each
+ // additional one.
+ //
+ for (std::size_t i = 1; i < list_of_matcher_idx_vectors.size(); ++i) {
+ *list_of_matcher_idx_vectors[i] = *list_of_matcher_idx_vectors[0];
+ }
+ }
+ if (sname_to_matcher_idxs.size() == 0) {
+ TSEmergency(PN ": YAML config file %s is empty", filespec.c_str());
+ }
+ } catch (const YAML::Exception &e) {
+ TSEmergency(PN ": YAML::Exception \"%s\" when parsing YAML config file %s", e.what(), filespec.c_str());
+ }
+}
+
+void
+Init::operator()(int n_args, char const *const *arg)
+{
+ if (n_args < 2) {
+ TSEmergency(PN ": must provide at least one plugin parameter");
+ }
+ if (2 == n_args) {
+ std::string_view first_arg(arg[1]);
+ if ((first_arg.size() > 5) && (first_arg.substr(first_arg.size() - 5) == ".yaml")) {
+ // Single parameter that is a yaml conifg file.
+ //
+ _yaml_process(arg[1]);
+ return;
+ }
+ }
+ // Otherwise the arguments are a list of name patterns that all client certs must match.
+ //
+ _process_name_args(n_args - 1, arg + 1);
+}
+
+void
+Init::_process_name_args(int n_names, char const *const *name)
+{
+ _Populator populator(this);
+
+ for (int i = 0; i < n_names; i++) {
+ if (!populator.add_cert_name(name[i])) {
+ TSEmergency(PN ": fatal error");
+ }
+ }
+ other_matcher_idxs = std::move(populator.matcher_idxs);
+ none_matcher_idxs = other_matcher_idxs;
+}
+
+bool
+Init::_Populator::add_cert_name(std::string_view name)
+{
+ if (name.size() == 0) {
+ // Empty name, match nothing.
+ return true;
+ }
+ if (std::count(std::begin(name), std::end(name), '*') > 1) {
+ TSError(PN ": bad certificate name pattern %.*s", static_cast<int>(name.size()), name.data());
+ return false;
+ }
+ unsigned name_idx;
+ auto it = _init->_name_to_idx_map.find(name);
+ if (it != _init->_name_to_idx_map.end()) {
+ name_idx = it->second;
+ if (_idx_present_flag[name_idx]) {
+ TSError(PN ": duplicate name pattern %.*s", static_cast<int>(name.size()), name.data());
+ return false;
+ }
+ } else {
+ // New name pattern, so new matcher.
+
+ name_idx = matcher.size();
+ matcher.emplace_back();
+ matcher[name_idx]._CName = std::string(name.data(), name.size());
+
+ if (name.find("*") != std::string::npos) {
+ // Name pattern has wildcard, so translate it to a PCRE and compile it.
+
+ std::size_t si = 0, di = 1;
+ _init->_insert_tmp[0] = '^';
+
+ while (si < name.size()) {
+ if (_init->_insert_tmp.size() < (di + 8)) {
+ _init->_insert_tmp.resize(di + 128);
+ }
+ if ('.' == name[si]) {
+ std::memcpy(_init->_insert_tmp.data() + di, "\\.", 3);
+ di += 2;
+ } else if ('*' == name[si]) {
+ std::memcpy(_init->_insert_tmp.data() + di, ".{0,}", 5);
Review comment:
This will match multiple domains, which is undoubtedly intended. For example, `*.example.com` should match `www.site.example.com` with no surprise. But someone who writes `www.example.*` might well be surprised to see it match `www.example.evil.com`. Is there a use case for a wildcard anywhere but the very front of a DNS name? Or is this to allow matches for things like "Bill *" to allow matches for all persons named Bill?
##########
File path: plugins/experimental/client_allow_list/util.cc
##########
@@ -0,0 +1,349 @@
+/** @file
+
+ SSL client certificate verification plugin, utility source file.
+
+ Checks for specificate names in the client provided certificate and
+ fails the handshake if none of the good names are present
+
+ @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.
+ */
+
+#include <cctype>
+#include <cstddef>
+#include <cstdio>
+#include <cstring>
+
+#include <yaml-cpp/yaml.h>
+
+#include "client_allow_list.h"
+
+#if defined(CLIENT_ALLOW_LIST_UNIT_TEST)
+
+#include <cstdarg>
+
+extern void ut_printf(char const *fmt, std::va_list args);
+
+namespace
+{
+char const *
+TSConfigDirGet()
+{
+ return ".";
+}
+
+void
+TSError(char const *fmt, ...)
+{
+ std::va_list args;
+
+ va_start(args, fmt);
+ ut_printf(fmt, args);
+ va_end(args);
+}
+
+[[noreturn]] void
+TSEmergency(char const *fmt, ...)
+{
+ std::va_list args;
+
+ va_start(args, fmt);
+ ut_printf(fmt, args);
+ va_end(args);
+
+ throw ClientAllowListUTException();
+}
+
+} // end anonymous namespace
+
+#undef TSAssert
+#define TSAssert(EXPR) ((EXPR) ? static_cast<void>(0) : TSEmergency("Assert Failed line=%d", __LINE__))
+
+#endif // defined(CLIENT_ALLOW_LIST_UNIT_TEST)
+
+namespace
+{
+[[noreturn]] void
+bad_node(std::string const &filespec, YAML::Node const &node)
+{
+ TSEmergency(PN ": config error: file=%s line=%d column=%d", filespec.c_str(), node.Mark().line + 1, node.Mark().column + 1);
+
+ throw 0; // Never executed, but avoids compiler warning that this function returns.
+}
+
+} // end anonymous namespace
+
+namespace client_allow_list_plugin
+{
+std::vector<cname_matcher> matcher;
+
+std::vector<unsigned> other_matcher_idxs;
+
+std::vector<unsigned> none_matcher_idxs;
+
+MapCStrToUVec::~MapCStrToUVec()
+{
+ char const *key;
+ auto it = _map.begin();
+ while (it != _map.end()) {
+ key = it->first;
+ _map.erase(it);
+ delete[] key;
+ it = _map.begin();
+ }
+}
+
+MapCStrToUVec sname_to_matcher_idxs;
+
+bool
+check_name(std::vector<unsigned> const &matcher_idxs, std::string_view name)
+{
+ bool name_matched{false};
+ int strvec[30];
+ for (unsigned idx : matcher_idxs) {
+ auto &a_matcher = matcher[idx];
+ if (a_matcher._compiled_RE == nullptr) {
+ if (std::string_view(a_matcher._CName) == name) {
+ name_matched = true;
+ break;
+ }
+ } else {
+ int exec_res{pcre_exec(a_matcher._compiled_RE, nullptr, name.data(), name.size(), 0, 0, strvec, 30)};
+ if (1 == exec_res) {
+ name_matched = true;
+ break;
+
+ } else if (exec_res != PCRE_ERROR_NOMATCH) {
+ TSError(PN ": bad result from pcre_exec=%d for name=%.*s, matcher named %s, idx=%u", exec_res,
+ static_cast<int>(name.size()), name.data(), a_matcher._CName.c_str(), idx);
+ }
+ }
+ }
+ return name_matched;
+}
+
+void
+Init::_yaml_process(char const *config_filespec)
+{
+ std::string filespec;
+ if (config_filespec[0] != '/') {
+ char const *config_dir = TSConfigDirGet();
+
+ if (config_dir) {
+ filespec = std::string(config_dir) + '/';
+ }
+ }
+
+ filespec += config_filespec;
+
+ try {
+ YAML::Node config = YAML::LoadFile(filespec);
+
+ bool none_seen{false}, other_seen{false};
+
+ for (YAML::const_iterator it = config.begin(); it != config.end(); ++it) {
+ std::string key = it->first.as<std::string>();
+
+ std::vector<std::vector<unsigned> *> list_of_matcher_idx_vectors;
+ std::string_view sname;
+ if (key.size() == 0) {
+ TSError(PN ": empty server name list");
+ bad_node(filespec, it->first);
+ }
+ if (std::string::npos != key.find_first_of(std::string_view(" \t\n"))) {
+ TSError(PN ": blank space not allowed in server name list");
+ bad_node(filespec, it->first);
+ }
+ std::size_t pos{0}, end_pos;
+ while (pos < key.size()) {
+ end_pos = key.find_first_of(std::string_view("|,"), pos);
+ if (std::string::npos == end_pos) {
+ end_pos = key.size();
+ } else if (end_pos == pos) {
+ TSError(PN ": empty server name in server name list");
+ bad_node(filespec, it->first);
+ }
+ sname = key.substr(pos, end_pos - pos);
+ pos = end_pos + 1;
+ if ("<none>" == sname) {
+ if (none_seen) {
+ TSError(PN ": <none> used more than once");
+ bad_node(filespec, it->first);
+ }
+ none_seen = true;
+ list_of_matcher_idx_vectors.push_back(&none_matcher_idxs);
+
+ } else if ("<other>" == sname) {
+ if (other_seen) {
+ TSError(PN ": <other> used more than once");
+ bad_node(filespec, it->first);
+ }
+ other_seen = true;
+ list_of_matcher_idx_vectors.push_back(&other_matcher_idxs);
+
+ } else {
+ auto vp = sname_to_matcher_idxs.add(sname);
+ if (!vp) {
+ TSError(PN ": cert names for SNI server name \"%.*s\" previously specified", static_cast<int>(sname.size()),
+ sname.data());
+ bad_node(filespec, it->first);
+ }
+ list_of_matcher_idx_vectors.push_back(vp);
+ }
+ }
+ TSAssert(list_of_matcher_idx_vectors.size() > 0);
+
+ _Populator pop(this);
+ if (it->second.IsSequence()) {
+ for (YAML::const_iterator it2 = it->second.begin(); it2 != it->second.end(); ++it2) {
+ if (!pop.add_cert_name(it2->as<std::string>().c_str())) {
+ bad_node(filespec, *it2);
+ }
+ }
+
+ } else {
+ // Value is scalar, single cert name.
+ if (!pop.add_cert_name(it->second.as<std::string>().c_str())) {
+ bad_node(filespec, it->second);
+ }
+ }
+ *list_of_matcher_idx_vectors[0] = std::move(pop.matcher_idxs);
+
+ // If there is more than one SNI server name, make a copy of the vector of matcher indexes for each
+ // additional one.
+ //
+ for (std::size_t i = 1; i < list_of_matcher_idx_vectors.size(); ++i) {
+ *list_of_matcher_idx_vectors[i] = *list_of_matcher_idx_vectors[0];
+ }
+ }
+ if (sname_to_matcher_idxs.size() == 0) {
+ TSEmergency(PN ": YAML config file %s is empty", filespec.c_str());
+ }
+ } catch (const YAML::Exception &e) {
+ TSEmergency(PN ": YAML::Exception \"%s\" when parsing YAML config file %s", e.what(), filespec.c_str());
+ }
+}
+
+void
+Init::operator()(int n_args, char const *const *arg)
+{
+ if (n_args < 2) {
+ TSEmergency(PN ": must provide at least one plugin parameter");
+ }
+ if (2 == n_args) {
+ std::string_view first_arg(arg[1]);
+ if ((first_arg.size() > 5) && (first_arg.substr(first_arg.size() - 5) == ".yaml")) {
+ // Single parameter that is a yaml conifg file.
+ //
+ _yaml_process(arg[1]);
+ return;
+ }
+ }
+ // Otherwise the arguments are a list of name patterns that all client certs must match.
+ //
+ _process_name_args(n_args - 1, arg + 1);
+}
+
+void
+Init::_process_name_args(int n_names, char const *const *name)
+{
+ _Populator populator(this);
+
+ for (int i = 0; i < n_names; i++) {
+ if (!populator.add_cert_name(name[i])) {
+ TSEmergency(PN ": fatal error");
+ }
+ }
+ other_matcher_idxs = std::move(populator.matcher_idxs);
+ none_matcher_idxs = other_matcher_idxs;
+}
+
+bool
+Init::_Populator::add_cert_name(std::string_view name)
+{
+ if (name.size() == 0) {
+ // Empty name, match nothing.
+ return true;
+ }
+ if (std::count(std::begin(name), std::end(name), '*') > 1) {
+ TSError(PN ": bad certificate name pattern %.*s", static_cast<int>(name.size()), name.data());
+ return false;
+ }
+ unsigned name_idx;
+ auto it = _init->_name_to_idx_map.find(name);
+ if (it != _init->_name_to_idx_map.end()) {
+ name_idx = it->second;
+ if (_idx_present_flag[name_idx]) {
+ TSError(PN ": duplicate name pattern %.*s", static_cast<int>(name.size()), name.data());
+ return false;
+ }
+ } else {
+ // New name pattern, so new matcher.
+
+ name_idx = matcher.size();
+ matcher.emplace_back();
+ matcher[name_idx]._CName = std::string(name.data(), name.size());
+
+ if (name.find("*") != std::string::npos) {
+ // Name pattern has wildcard, so translate it to a PCRE and compile it.
+
+ std::size_t si = 0, di = 1;
+ _init->_insert_tmp[0] = '^';
+
+ while (si < name.size()) {
+ if (_init->_insert_tmp.size() < (di + 8)) {
+ _init->_insert_tmp.resize(di + 128);
+ }
+ if ('.' == name[si]) {
+ std::memcpy(_init->_insert_tmp.data() + di, "\\.", 3);
Review comment:
This unnecessarily copies in the null terminator after this string. Because the realloc above always reserves at least 8 bytes, this won't break anything as it is, but it's somewhat confusing, especially given the escape character. The copy just a few lines down does not do this.
##########
File path: plugins/experimental/client_allow_list/util.cc
##########
@@ -0,0 +1,349 @@
+/** @file
+
+ SSL client certificate verification plugin, utility source file.
+
+ Checks for specificate names in the client provided certificate and
+ fails the handshake if none of the good names are present
+
+ @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.
+ */
+
+#include <cctype>
+#include <cstddef>
+#include <cstdio>
+#include <cstring>
+
+#include <yaml-cpp/yaml.h>
+
+#include "client_allow_list.h"
+
+#if defined(CLIENT_ALLOW_LIST_UNIT_TEST)
+
+#include <cstdarg>
+
+extern void ut_printf(char const *fmt, std::va_list args);
+
+namespace
+{
+char const *
+TSConfigDirGet()
+{
+ return ".";
+}
+
+void
+TSError(char const *fmt, ...)
+{
+ std::va_list args;
+
+ va_start(args, fmt);
+ ut_printf(fmt, args);
+ va_end(args);
+}
+
+[[noreturn]] void
+TSEmergency(char const *fmt, ...)
+{
+ std::va_list args;
+
+ va_start(args, fmt);
+ ut_printf(fmt, args);
+ va_end(args);
+
+ throw ClientAllowListUTException();
+}
+
+} // end anonymous namespace
+
+#undef TSAssert
+#define TSAssert(EXPR) ((EXPR) ? static_cast<void>(0) : TSEmergency("Assert Failed line=%d", __LINE__))
+
+#endif // defined(CLIENT_ALLOW_LIST_UNIT_TEST)
+
+namespace
+{
+[[noreturn]] void
+bad_node(std::string const &filespec, YAML::Node const &node)
+{
+ TSEmergency(PN ": config error: file=%s line=%d column=%d", filespec.c_str(), node.Mark().line + 1, node.Mark().column + 1);
+
+ throw 0; // Never executed, but avoids compiler warning that this function returns.
+}
+
+} // end anonymous namespace
+
+namespace client_allow_list_plugin
+{
+std::vector<cname_matcher> matcher;
+
+std::vector<unsigned> other_matcher_idxs;
+
+std::vector<unsigned> none_matcher_idxs;
+
+MapCStrToUVec::~MapCStrToUVec()
+{
+ char const *key;
+ auto it = _map.begin();
+ while (it != _map.end()) {
+ key = it->first;
+ _map.erase(it);
+ delete[] key;
+ it = _map.begin();
+ }
+}
+
+MapCStrToUVec sname_to_matcher_idxs;
+
+bool
+check_name(std::vector<unsigned> const &matcher_idxs, std::string_view name)
+{
+ bool name_matched{false};
+ int strvec[30];
+ for (unsigned idx : matcher_idxs) {
+ auto &a_matcher = matcher[idx];
+ if (a_matcher._compiled_RE == nullptr) {
+ if (std::string_view(a_matcher._CName) == name) {
+ name_matched = true;
+ break;
+ }
+ } else {
+ int exec_res{pcre_exec(a_matcher._compiled_RE, nullptr, name.data(), name.size(), 0, 0, strvec, 30)};
+ if (1 == exec_res) {
+ name_matched = true;
+ break;
+
+ } else if (exec_res != PCRE_ERROR_NOMATCH) {
+ TSError(PN ": bad result from pcre_exec=%d for name=%.*s, matcher named %s, idx=%u", exec_res,
+ static_cast<int>(name.size()), name.data(), a_matcher._CName.c_str(), idx);
+ }
+ }
+ }
+ return name_matched;
+}
+
+void
+Init::_yaml_process(char const *config_filespec)
+{
+ std::string filespec;
+ if (config_filespec[0] != '/') {
+ char const *config_dir = TSConfigDirGet();
+
+ if (config_dir) {
+ filespec = std::string(config_dir) + '/';
+ }
+ }
+
+ filespec += config_filespec;
+
+ try {
+ YAML::Node config = YAML::LoadFile(filespec);
+
+ bool none_seen{false}, other_seen{false};
+
+ for (YAML::const_iterator it = config.begin(); it != config.end(); ++it) {
+ std::string key = it->first.as<std::string>();
+
+ std::vector<std::vector<unsigned> *> list_of_matcher_idx_vectors;
+ std::string_view sname;
+ if (key.size() == 0) {
+ TSError(PN ": empty server name list");
+ bad_node(filespec, it->first);
+ }
+ if (std::string::npos != key.find_first_of(std::string_view(" \t\n"))) {
+ TSError(PN ": blank space not allowed in server name list");
+ bad_node(filespec, it->first);
+ }
+ std::size_t pos{0}, end_pos;
+ while (pos < key.size()) {
+ end_pos = key.find_first_of(std::string_view("|,"), pos);
+ if (std::string::npos == end_pos) {
+ end_pos = key.size();
+ } else if (end_pos == pos) {
+ TSError(PN ": empty server name in server name list");
+ bad_node(filespec, it->first);
+ }
+ sname = key.substr(pos, end_pos - pos);
+ pos = end_pos + 1;
+ if ("<none>" == sname) {
+ if (none_seen) {
+ TSError(PN ": <none> used more than once");
+ bad_node(filespec, it->first);
+ }
+ none_seen = true;
+ list_of_matcher_idx_vectors.push_back(&none_matcher_idxs);
+
+ } else if ("<other>" == sname) {
+ if (other_seen) {
+ TSError(PN ": <other> used more than once");
+ bad_node(filespec, it->first);
+ }
+ other_seen = true;
+ list_of_matcher_idx_vectors.push_back(&other_matcher_idxs);
+
+ } else {
+ auto vp = sname_to_matcher_idxs.add(sname);
+ if (!vp) {
+ TSError(PN ": cert names for SNI server name \"%.*s\" previously specified", static_cast<int>(sname.size()),
+ sname.data());
+ bad_node(filespec, it->first);
+ }
+ list_of_matcher_idx_vectors.push_back(vp);
+ }
+ }
+ TSAssert(list_of_matcher_idx_vectors.size() > 0);
+
+ _Populator pop(this);
+ if (it->second.IsSequence()) {
+ for (YAML::const_iterator it2 = it->second.begin(); it2 != it->second.end(); ++it2) {
+ if (!pop.add_cert_name(it2->as<std::string>().c_str())) {
+ bad_node(filespec, *it2);
+ }
+ }
+
+ } else {
+ // Value is scalar, single cert name.
+ if (!pop.add_cert_name(it->second.as<std::string>().c_str())) {
+ bad_node(filespec, it->second);
+ }
+ }
+ *list_of_matcher_idx_vectors[0] = std::move(pop.matcher_idxs);
+
+ // If there is more than one SNI server name, make a copy of the vector of matcher indexes for each
+ // additional one.
+ //
+ for (std::size_t i = 1; i < list_of_matcher_idx_vectors.size(); ++i) {
+ *list_of_matcher_idx_vectors[i] = *list_of_matcher_idx_vectors[0];
+ }
+ }
+ if (sname_to_matcher_idxs.size() == 0) {
+ TSEmergency(PN ": YAML config file %s is empty", filespec.c_str());
+ }
+ } catch (const YAML::Exception &e) {
+ TSEmergency(PN ": YAML::Exception \"%s\" when parsing YAML config file %s", e.what(), filespec.c_str());
+ }
+}
+
+void
+Init::operator()(int n_args, char const *const *arg)
+{
+ if (n_args < 2) {
+ TSEmergency(PN ": must provide at least one plugin parameter");
+ }
+ if (2 == n_args) {
+ std::string_view first_arg(arg[1]);
+ if ((first_arg.size() > 5) && (first_arg.substr(first_arg.size() - 5) == ".yaml")) {
+ // Single parameter that is a yaml conifg file.
+ //
+ _yaml_process(arg[1]);
+ return;
+ }
+ }
+ // Otherwise the arguments are a list of name patterns that all client certs must match.
+ //
+ _process_name_args(n_args - 1, arg + 1);
+}
+
+void
+Init::_process_name_args(int n_names, char const *const *name)
+{
+ _Populator populator(this);
+
+ for (int i = 0; i < n_names; i++) {
+ if (!populator.add_cert_name(name[i])) {
+ TSEmergency(PN ": fatal error");
+ }
+ }
+ other_matcher_idxs = std::move(populator.matcher_idxs);
+ none_matcher_idxs = other_matcher_idxs;
+}
+
+bool
+Init::_Populator::add_cert_name(std::string_view name)
+{
+ if (name.size() == 0) {
+ // Empty name, match nothing.
+ return true;
+ }
+ if (std::count(std::begin(name), std::end(name), '*') > 1) {
+ TSError(PN ": bad certificate name pattern %.*s", static_cast<int>(name.size()), name.data());
+ return false;
+ }
+ unsigned name_idx;
+ auto it = _init->_name_to_idx_map.find(name);
+ if (it != _init->_name_to_idx_map.end()) {
+ name_idx = it->second;
+ if (_idx_present_flag[name_idx]) {
+ TSError(PN ": duplicate name pattern %.*s", static_cast<int>(name.size()), name.data());
+ return false;
+ }
+ } else {
+ // New name pattern, so new matcher.
+
+ name_idx = matcher.size();
+ matcher.emplace_back();
+ matcher[name_idx]._CName = std::string(name.data(), name.size());
+
+ if (name.find("*") != std::string::npos) {
+ // Name pattern has wildcard, so translate it to a PCRE and compile it.
+
+ std::size_t si = 0, di = 1;
+ _init->_insert_tmp[0] = '^';
+
+ while (si < name.size()) {
+ if (_init->_insert_tmp.size() < (di + 8)) {
+ _init->_insert_tmp.resize(di + 128);
+ }
+ if ('.' == name[si]) {
+ std::memcpy(_init->_insert_tmp.data() + di, "\\.", 3);
+ di += 2;
+ } else if ('*' == name[si]) {
+ std::memcpy(_init->_insert_tmp.data() + di, ".{0,}", 5);
+ di += 5;
+ } else {
+ _init->_insert_tmp[di++] = name[si];
+ }
+ ++si;
+ }
+ _init->_insert_tmp[di++] = '$';
+ _init->_insert_tmp[di] = '\0';
Review comment:
This routine escapes only dots. If I understand the plugin correctly, though, we're validating CNs as well, which don't have to be DNS names. That means the CNs we're looking for might contain regex magic within them.
Also, it's worth noting that if an attacker can control the contents of these entries (you can imagine a self-service system where users can set the allowed CNs for certs connecting specifically with their domain, when ATS is hosting domains for multiple users), there's a fairly straightforward ReDoS attack. That might be fine under most threat models and working around it might be hard, so documenting the situation might be easier than fixing it if arbitrary regular expressions are allowed here. If the plan isn't to allow arbitrary regexen, this concern disappears when the input is sanitized.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on a change in pull request #7598: Add client_allow_list.so experimental plugin.
Posted by GitBox <gi...@apache.org>.
maskit commented on a change in pull request #7598:
URL: https://github.com/apache/trafficserver/pull/7598#discussion_r595621692
##########
File path: plugins/experimental/client_allow_list/unit_tests/test.cc
##########
@@ -0,0 +1,360 @@
+/** @file
+
+ SSL client certificate verification plugin, unit test for utility
+ source file.
+
+ @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.
+ */
+
+#include <cstdlib>
+#include <cstring>
+#include <cstdarg>
+#include <cstdio>
+#include <string_view>
+
+#include <client_allow_list.h>
+
+using client_allow_list_plugin::matcher;
+using client_allow_list_plugin::other_matcher_idxs;
+using client_allow_list_plugin::none_matcher_idxs;
+using client_allow_list_plugin::sname_to_matcher_idxs;
+using client_allow_list_plugin::Init;
+using client_allow_list_plugin::check_name;
+
+#define REQUIRE(EXPR) (require((EXPR), #EXPR, __LINE__))
+
+#undef DIR
+#define DIR "experimental/client_allow_list/unit_tests/"
+
+namespace
+{
+void
+require(bool expr, char const *expr_str, int line_num)
+{
+ if (!expr) {
+ std::printf("FAILURE: %s line=%d\n", expr_str, line_num);
+ std::exit(1);
+ }
+}
+
+bool
+matcher_idxs_same(std::vector<unsigned> const &idxs1, std::vector<unsigned> const &idxs2)
+{
+ if (idxs1.size() != idxs2.size()) {
+ return false;
+ }
+ for (std::size_t i = 0; i < idxs1.size(); ++i) {
+ if (idxs1[i] != idxs2[i]) {
+ return false;
+ }
+ }
+ return true;
+}
+
+char ut_printf_output[2 * 1024];
+std::size_t ut_printf_output_size;
+
+#define UT_CHECK_OUTPUT(S) ut_check_output((S), __LINE__)
+
+void
+ut_check_output(const char *expected, int line)
+{
+ if (std::string_view(ut_printf_output, ut_printf_output_size) != expected) {
+ std::printf("FAILURE: unexpected output: code line=%d\n", line);
+ std::printf("EXPECTED: %s\n", expected);
+ std::printf("ACTUAL: %.*s\n", static_cast<int>(ut_printf_output_size), ut_printf_output);
+ std::exit(1);
+ }
+}
+
+template <typename T>
+void
+reset(T &v)
+{
+ v.~T();
+ ::new (&v) T();
+}
+
+// Reset globals between tests, so that multiple configurations can be tested in a single UT executable.
+//
+void
+reset_globals()
+{
+ reset(sname_to_matcher_idxs);
+ reset(none_matcher_idxs);
+ reset(other_matcher_idxs);
+ reset(matcher);
+
+ ut_printf_output_size = 0;
+}
+
+// Returns true if error handled correctly.
+//
+bool
+bad_config(int n_arg, char const *const *arg)
+{
+ bool caught{false};
+
+ try {
+ Init()(n_arg, arg);
+
+ } catch (ClientAllowListUTException &) {
+ caught = true;
+ }
+ REQUIRE(ut_printf_output_size < sizeof(ut_printf_output));
+
+ return caught;
+}
+
+} // end anonymous namespace
+
+void
+ut_printf(char const *fmt, std::va_list args)
+{
+ int n = std::vsnprintf(ut_printf_output + ut_printf_output_size, sizeof(ut_printf_output) - ut_printf_output_size, fmt, args);
+ REQUIRE(n > 0);
+ ut_printf_output_size += n;
+ REQUIRE(ut_printf_output_size < sizeof(ut_printf_output));
+ ut_printf_output[ut_printf_output_size++] = '\n';
+}
+
+int
+main()
+{
+ // Each sub-block is a test of a different configuation.
+
+ // Tests of good configurations.
+
+ {
+ char const *args[] = {"dummy plugin name", "*.bbb"};
+ Init()(2, args);
+
+ REQUIRE(matcher.size() == 1);
+
+ REQUIRE(sname_to_matcher_idxs.size() == 0);
+
+ REQUIRE(other_matcher_idxs.size() == 1);
+ REQUIRE(matcher_idxs_same(other_matcher_idxs, none_matcher_idxs));
+ REQUIRE(check_name(other_matcher_idxs, "aaa.bbb"));
+ }
+ REQUIRE(!ut_printf_output_size);
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", "aaa", "*.bbb", "ccc.*", "ddd.*.eee"};
+ Init()(5, args);
+
+ REQUIRE(matcher.size() == 4);
+
+ REQUIRE(sname_to_matcher_idxs.size() == 0);
+
+ REQUIRE(other_matcher_idxs.size() == 4);
+ REQUIRE(matcher_idxs_same(other_matcher_idxs, none_matcher_idxs));
+ REQUIRE(check_name(other_matcher_idxs, "aaa"));
+ REQUIRE(check_name(other_matcher_idxs, "ddd.xxx.eee"));
+ REQUIRE(check_name(other_matcher_idxs, "aaa.bbb"));
+ }
+ REQUIRE(!ut_printf_output_size);
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "good1.yaml"};
+ Init()(2, args);
+
+ REQUIRE(matcher.size() == 7);
+
+ REQUIRE(sname_to_matcher_idxs.size() == 6);
+
+ {
+ REQUIRE(none_matcher_idxs.size() == 4);
+ auto m2 = sname_to_matcher_idxs.find("yahoo.com");
+ REQUIRE(m2 != nullptr);
+ REQUIRE(matcher_idxs_same(*m2, none_matcher_idxs));
+ REQUIRE(check_name(none_matcher_idxs, "aaa"));
+ REQUIRE(!check_name(none_matcher_idxs, "aa"));
+ REQUIRE(!check_name(none_matcher_idxs, "aab"));
+ REQUIRE(check_name(none_matcher_idxs, "ddd.xxx.eee"));
+ REQUIRE(!check_name(none_matcher_idxs, "ddd.xxx.efe"));
+ REQUIRE(check_name(none_matcher_idxs, "aaa.bbb"));
+ }
+ {
+ auto m = sname_to_matcher_idxs.find("xxx");
+ REQUIRE(m != nullptr);
+ REQUIRE(m->size() == 0);
+ REQUIRE(!check_name(*m, "anything.com"));
+ }
+ {
+ REQUIRE(other_matcher_idxs.size() == 2);
+ auto m2 = sname_to_matcher_idxs.find("huffpost.com");
+ REQUIRE(m2 != nullptr);
+ REQUIRE(matcher_idxs_same(*m2, other_matcher_idxs));
+ auto m3 = sname_to_matcher_idxs.find("aOl.CoM");
+ REQUIRE(m3 != nullptr);
+ REQUIRE(matcher_idxs_same(*m3, other_matcher_idxs));
+ REQUIRE(check_name(other_matcher_idxs, "fff"));
+ REQUIRE(check_name(other_matcher_idxs, "ff.bbb"));
+ }
+ {
+ auto m = sname_to_matcher_idxs.find("uuu");
+ REQUIRE(m != nullptr);
+ REQUIRE(m->size() == 1);
+ REQUIRE(check_name(*m, "vvv"));
+ }
+ {
+ auto m = sname_to_matcher_idxs.find("yyy");
+ REQUIRE(m != nullptr);
+ REQUIRE(m->size() == 1);
+ REQUIRE(check_name(*m, "anything.com"));
+ }
+ {
+ auto m = sname_to_matcher_idxs.find("not_there");
+ REQUIRE(nullptr == m);
+ }
+ }
+ REQUIRE(!ut_printf_output_size);
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "good2.yaml"};
+ Init()(2, args);
+
+ REQUIRE(matcher.size() == 1);
+
+ REQUIRE(sname_to_matcher_idxs.size() == 1);
+
+ REQUIRE(none_matcher_idxs.size() == 0);
+ REQUIRE(other_matcher_idxs.size() == 0);
+
+ {
+ auto m = sname_to_matcher_idxs.find("uuu");
+ REQUIRE(m != nullptr);
+ REQUIRE(m->size() == 1);
+ REQUIRE(check_name(*m, "vvv"));
+ }
+ }
+ REQUIRE(!ut_printf_output_size);
+
+ // Tests of bad configurations.
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name"};
+ REQUIRE(bad_config(1, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: must provide at least one plugin parameter\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", "aaa*bbb*ccc"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: bad certificate name pattern aaa*bbb*ccc\n"
+ "client_allow_list: fatal error\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "not-there.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: YAML::Exception \"bad file\" when parsing YAML config file ./" DIR "not-there.yaml\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad1.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: YAML config file ./" DIR "bad1.yaml is empty\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad2.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: YAML::Exception \"invalid node; this may result from using a map"
+ " iterator as a sequence iterator, or vice-versa\" when parsing YAML config file ./" DIR "bad2.yaml\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad3.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: empty server name list\n"
+ "client_allow_list: config error: file=./" DIR "bad3.yaml line=2 column=1\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad4.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: blank space not allowed in server name list\n"
+ "client_allow_list: config error: file=./" DIR "bad4.yaml line=2 column=1\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad5.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: <none> used more than once\n"
+ "client_allow_list: config error: file=./" DIR "bad5.yaml line=4 column=1\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad6.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: <other> used more than once\n"
+ "client_allow_list: config error: file=./" DIR "bad6.yaml line=4 column=1\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad7.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: cert names for SNI server name \"twice\" previously specified\n"
+ "client_allow_list: config error: file=./" DIR "bad7.yaml line=4 column=1\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad8.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: YAML::Exception \"yaml-cpp: error at line 4, column 3: end of map"
+ " not found\" when parsing YAML config file ./" DIR "bad8.yaml\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad9.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: YAML::Exception \"yaml-cpp: error at line 3, column 3: bad conversion\""
+ " when parsing YAML config file ./" DIR "bad9.yaml\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad10.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: YAML::Exception \"yaml-cpp: error at line 4, column 5: the referenced "
+ "anchor is not defined\" when parsing YAML config file ./" DIR "bad10.yaml\n");
+
+ return 0;
+}
+
+#include <../src/tscore/HashFNV.cc>
Review comment:
Do we need this? Seems like ssl_session_reuse uses HashFNV without including the cc files. And there are some plugins that include files under include/tscore.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on a change in pull request #7598: Add client_allow_list.so experimental plugin.
Posted by GitBox <gi...@apache.org>.
maskit commented on a change in pull request #7598:
URL: https://github.com/apache/trafficserver/pull/7598#discussion_r594784077
##########
File path: plugins/experimental/client_allow_list/main.cc
##########
@@ -0,0 +1,180 @@
+/** @file
+
+ SSL client certificate verification plugin, main source file.
+
+ Checks for specificate names in the client provided certificate and
+ fails the handshake if none of the good names are present
+
+ @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.
+ */
+
+#include <openssl/ssl.h>
+#include <openssl/x509.h>
+#include <openssl/x509v3.h>
+#include <openssl/asn1.h>
+
+#include "client_allow_list.h"
+
+using namespace client_allow_list_plugin;
+
+#if TS_VERSION_MAJOR < 9
+#define TSVConnSslConnectionGet TSVConnSSLConnectionGet
+#endif
+
+namespace
+{
+inline const char *
+ASN1_get_data(ASN1_STRING *cn)
+{
+#if OPENSSL_VERSION_NUMBER < 0x10100000L
Review comment:
Yes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on a change in pull request #7598: Add client_allow_list.so experimental plugin.
Posted by GitBox <gi...@apache.org>.
maskit commented on a change in pull request #7598:
URL: https://github.com/apache/trafficserver/pull/7598#discussion_r595635326
##########
File path: plugins/experimental/client_allow_list/unit_tests/test.cc
##########
@@ -0,0 +1,360 @@
+/** @file
+
+ SSL client certificate verification plugin, unit test for utility
+ source file.
+
+ @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.
+ */
+
+#include <cstdlib>
+#include <cstring>
+#include <cstdarg>
+#include <cstdio>
+#include <string_view>
+
+#include <client_allow_list.h>
+
+using client_allow_list_plugin::matcher;
+using client_allow_list_plugin::other_matcher_idxs;
+using client_allow_list_plugin::none_matcher_idxs;
+using client_allow_list_plugin::sname_to_matcher_idxs;
+using client_allow_list_plugin::Init;
+using client_allow_list_plugin::check_name;
+
+#define REQUIRE(EXPR) (require((EXPR), #EXPR, __LINE__))
+
+#undef DIR
+#define DIR "experimental/client_allow_list/unit_tests/"
+
+namespace
+{
+void
+require(bool expr, char const *expr_str, int line_num)
+{
+ if (!expr) {
+ std::printf("FAILURE: %s line=%d\n", expr_str, line_num);
+ std::exit(1);
+ }
+}
+
+bool
+matcher_idxs_same(std::vector<unsigned> const &idxs1, std::vector<unsigned> const &idxs2)
+{
+ if (idxs1.size() != idxs2.size()) {
+ return false;
+ }
+ for (std::size_t i = 0; i < idxs1.size(); ++i) {
+ if (idxs1[i] != idxs2[i]) {
+ return false;
+ }
+ }
+ return true;
+}
+
+char ut_printf_output[2 * 1024];
+std::size_t ut_printf_output_size;
+
+#define UT_CHECK_OUTPUT(S) ut_check_output((S), __LINE__)
+
+void
+ut_check_output(const char *expected, int line)
+{
+ if (std::string_view(ut_printf_output, ut_printf_output_size) != expected) {
+ std::printf("FAILURE: unexpected output: code line=%d\n", line);
+ std::printf("EXPECTED: %s\n", expected);
+ std::printf("ACTUAL: %.*s\n", static_cast<int>(ut_printf_output_size), ut_printf_output);
+ std::exit(1);
+ }
+}
+
+template <typename T>
+void
+reset(T &v)
+{
+ v.~T();
+ ::new (&v) T();
+}
+
+// Reset globals between tests, so that multiple configurations can be tested in a single UT executable.
+//
+void
+reset_globals()
+{
+ reset(sname_to_matcher_idxs);
+ reset(none_matcher_idxs);
+ reset(other_matcher_idxs);
+ reset(matcher);
+
+ ut_printf_output_size = 0;
+}
+
+// Returns true if error handled correctly.
+//
+bool
+bad_config(int n_arg, char const *const *arg)
+{
+ bool caught{false};
+
+ try {
+ Init()(n_arg, arg);
+
+ } catch (ClientAllowListUTException &) {
+ caught = true;
+ }
+ REQUIRE(ut_printf_output_size < sizeof(ut_printf_output));
+
+ return caught;
+}
+
+} // end anonymous namespace
+
+void
+ut_printf(char const *fmt, std::va_list args)
+{
+ int n = std::vsnprintf(ut_printf_output + ut_printf_output_size, sizeof(ut_printf_output) - ut_printf_output_size, fmt, args);
+ REQUIRE(n > 0);
+ ut_printf_output_size += n;
+ REQUIRE(ut_printf_output_size < sizeof(ut_printf_output));
+ ut_printf_output[ut_printf_output_size++] = '\n';
+}
+
+int
+main()
+{
+ // Each sub-block is a test of a different configuation.
+
+ // Tests of good configurations.
+
+ {
+ char const *args[] = {"dummy plugin name", "*.bbb"};
+ Init()(2, args);
+
+ REQUIRE(matcher.size() == 1);
+
+ REQUIRE(sname_to_matcher_idxs.size() == 0);
+
+ REQUIRE(other_matcher_idxs.size() == 1);
+ REQUIRE(matcher_idxs_same(other_matcher_idxs, none_matcher_idxs));
+ REQUIRE(check_name(other_matcher_idxs, "aaa.bbb"));
+ }
+ REQUIRE(!ut_printf_output_size);
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", "aaa", "*.bbb", "ccc.*", "ddd.*.eee"};
+ Init()(5, args);
+
+ REQUIRE(matcher.size() == 4);
+
+ REQUIRE(sname_to_matcher_idxs.size() == 0);
+
+ REQUIRE(other_matcher_idxs.size() == 4);
+ REQUIRE(matcher_idxs_same(other_matcher_idxs, none_matcher_idxs));
+ REQUIRE(check_name(other_matcher_idxs, "aaa"));
+ REQUIRE(check_name(other_matcher_idxs, "ddd.xxx.eee"));
+ REQUIRE(check_name(other_matcher_idxs, "aaa.bbb"));
+ }
+ REQUIRE(!ut_printf_output_size);
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "good1.yaml"};
+ Init()(2, args);
+
+ REQUIRE(matcher.size() == 7);
+
+ REQUIRE(sname_to_matcher_idxs.size() == 6);
+
+ {
+ REQUIRE(none_matcher_idxs.size() == 4);
+ auto m2 = sname_to_matcher_idxs.find("yahoo.com");
+ REQUIRE(m2 != nullptr);
+ REQUIRE(matcher_idxs_same(*m2, none_matcher_idxs));
+ REQUIRE(check_name(none_matcher_idxs, "aaa"));
+ REQUIRE(!check_name(none_matcher_idxs, "aa"));
+ REQUIRE(!check_name(none_matcher_idxs, "aab"));
+ REQUIRE(check_name(none_matcher_idxs, "ddd.xxx.eee"));
+ REQUIRE(!check_name(none_matcher_idxs, "ddd.xxx.efe"));
+ REQUIRE(check_name(none_matcher_idxs, "aaa.bbb"));
+ }
+ {
+ auto m = sname_to_matcher_idxs.find("xxx");
+ REQUIRE(m != nullptr);
+ REQUIRE(m->size() == 0);
+ REQUIRE(!check_name(*m, "anything.com"));
+ }
+ {
+ REQUIRE(other_matcher_idxs.size() == 2);
+ auto m2 = sname_to_matcher_idxs.find("huffpost.com");
+ REQUIRE(m2 != nullptr);
+ REQUIRE(matcher_idxs_same(*m2, other_matcher_idxs));
+ auto m3 = sname_to_matcher_idxs.find("aOl.CoM");
+ REQUIRE(m3 != nullptr);
+ REQUIRE(matcher_idxs_same(*m3, other_matcher_idxs));
+ REQUIRE(check_name(other_matcher_idxs, "fff"));
+ REQUIRE(check_name(other_matcher_idxs, "ff.bbb"));
+ }
+ {
+ auto m = sname_to_matcher_idxs.find("uuu");
+ REQUIRE(m != nullptr);
+ REQUIRE(m->size() == 1);
+ REQUIRE(check_name(*m, "vvv"));
+ }
+ {
+ auto m = sname_to_matcher_idxs.find("yyy");
+ REQUIRE(m != nullptr);
+ REQUIRE(m->size() == 1);
+ REQUIRE(check_name(*m, "anything.com"));
+ }
+ {
+ auto m = sname_to_matcher_idxs.find("not_there");
+ REQUIRE(nullptr == m);
+ }
+ }
+ REQUIRE(!ut_printf_output_size);
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "good2.yaml"};
+ Init()(2, args);
+
+ REQUIRE(matcher.size() == 1);
+
+ REQUIRE(sname_to_matcher_idxs.size() == 1);
+
+ REQUIRE(none_matcher_idxs.size() == 0);
+ REQUIRE(other_matcher_idxs.size() == 0);
+
+ {
+ auto m = sname_to_matcher_idxs.find("uuu");
+ REQUIRE(m != nullptr);
+ REQUIRE(m->size() == 1);
+ REQUIRE(check_name(*m, "vvv"));
+ }
+ }
+ REQUIRE(!ut_printf_output_size);
+
+ // Tests of bad configurations.
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name"};
+ REQUIRE(bad_config(1, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: must provide at least one plugin parameter\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", "aaa*bbb*ccc"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: bad certificate name pattern aaa*bbb*ccc\n"
+ "client_allow_list: fatal error\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "not-there.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: YAML::Exception \"bad file\" when parsing YAML config file ./" DIR "not-there.yaml\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad1.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: YAML config file ./" DIR "bad1.yaml is empty\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad2.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: YAML::Exception \"invalid node; this may result from using a map"
+ " iterator as a sequence iterator, or vice-versa\" when parsing YAML config file ./" DIR "bad2.yaml\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad3.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: empty server name list\n"
+ "client_allow_list: config error: file=./" DIR "bad3.yaml line=2 column=1\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad4.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: blank space not allowed in server name list\n"
+ "client_allow_list: config error: file=./" DIR "bad4.yaml line=2 column=1\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad5.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: <none> used more than once\n"
+ "client_allow_list: config error: file=./" DIR "bad5.yaml line=4 column=1\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad6.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: <other> used more than once\n"
+ "client_allow_list: config error: file=./" DIR "bad6.yaml line=4 column=1\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad7.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: cert names for SNI server name \"twice\" previously specified\n"
+ "client_allow_list: config error: file=./" DIR "bad7.yaml line=4 column=1\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad8.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: YAML::Exception \"yaml-cpp: error at line 4, column 3: end of map"
+ " not found\" when parsing YAML config file ./" DIR "bad8.yaml\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad9.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: YAML::Exception \"yaml-cpp: error at line 3, column 3: bad conversion\""
+ " when parsing YAML config file ./" DIR "bad9.yaml\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad10.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: YAML::Exception \"yaml-cpp: error at line 4, column 5: the referenced "
+ "anchor is not defined\" when parsing YAML config file ./" DIR "bad10.yaml\n");
+
+ return 0;
+}
+
+#include <../src/tscore/HashFNV.cc>
Review comment:
Tests for access_control and sslheaders link libtscore. I'm not sure if it's better but looks more normal.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] mlibbey commented on pull request #7598: Add client_allow_list.so experimental plugin.
Posted by GitBox <gi...@apache.org>.
mlibbey commented on pull request #7598:
URL: https://github.com/apache/trafficserver/pull/7598#issuecomment-799018654
FWIW, our org uses a field inside the Subject DN eg, when connecting to the ATS with SNI cdn.example.com, only accept mTLS requests with Subject DN field containing the UID=ourorg.group.123, and with specific Issuer DN values. The security peeps also suggest/mandate/whatever that the cert should chain to specific roots. Even if this functionality doesn't make it into this version, might be nice to think through the config language to be able to add it later.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on a change in pull request #7598: Add client_allow_list.so experimental plugin.
Posted by GitBox <gi...@apache.org>.
maskit commented on a change in pull request #7598:
URL: https://github.com/apache/trafficserver/pull/7598#discussion_r594794706
##########
File path: plugins/experimental/client_allow_list/main.cc
##########
@@ -0,0 +1,180 @@
+/** @file
+
+ SSL client certificate verification plugin, main source file.
+
+ Checks for specificate names in the client provided certificate and
+ fails the handshake if none of the good names are present
+
+ @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.
+ */
+
+#include <openssl/ssl.h>
+#include <openssl/x509.h>
+#include <openssl/x509v3.h>
+#include <openssl/asn1.h>
+
+#include "client_allow_list.h"
+
+using namespace client_allow_list_plugin;
+
+#if TS_VERSION_MAJOR < 9
+#define TSVConnSslConnectionGet TSVConnSSLConnectionGet
+#endif
+
+namespace
+{
+inline const char *
+ASN1_get_data(ASN1_STRING *cn)
+{
+#if OPENSSL_VERSION_NUMBER < 0x10100000L
Review comment:
That's up to you. This one is not a blocker.
You could do it with a magic as well. This spell is already in our configure.ac.
```
AC_CHECK_FUNC([ASN1_STRING_get0_data], [],
[AC_DEFINE([ASN1_STRING_get0_data], [ASN1_STRING_data], [Added in OpenSSL 1.1])])
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on a change in pull request #7598: Add client_allow_list.so experimental plugin.
Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #7598:
URL: https://github.com/apache/trafficserver/pull/7598#discussion_r595631135
##########
File path: plugins/experimental/client_allow_list/unit_tests/test.cc
##########
@@ -0,0 +1,360 @@
+/** @file
+
+ SSL client certificate verification plugin, unit test for utility
+ source file.
+
+ @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.
+ */
+
+#include <cstdlib>
+#include <cstring>
+#include <cstdarg>
+#include <cstdio>
+#include <string_view>
+
+#include <client_allow_list.h>
+
+using client_allow_list_plugin::matcher;
+using client_allow_list_plugin::other_matcher_idxs;
+using client_allow_list_plugin::none_matcher_idxs;
+using client_allow_list_plugin::sname_to_matcher_idxs;
+using client_allow_list_plugin::Init;
+using client_allow_list_plugin::check_name;
+
+#define REQUIRE(EXPR) (require((EXPR), #EXPR, __LINE__))
+
+#undef DIR
+#define DIR "experimental/client_allow_list/unit_tests/"
+
+namespace
+{
+void
+require(bool expr, char const *expr_str, int line_num)
+{
+ if (!expr) {
+ std::printf("FAILURE: %s line=%d\n", expr_str, line_num);
+ std::exit(1);
+ }
+}
+
+bool
+matcher_idxs_same(std::vector<unsigned> const &idxs1, std::vector<unsigned> const &idxs2)
+{
+ if (idxs1.size() != idxs2.size()) {
+ return false;
+ }
+ for (std::size_t i = 0; i < idxs1.size(); ++i) {
+ if (idxs1[i] != idxs2[i]) {
+ return false;
+ }
+ }
+ return true;
+}
+
+char ut_printf_output[2 * 1024];
+std::size_t ut_printf_output_size;
+
+#define UT_CHECK_OUTPUT(S) ut_check_output((S), __LINE__)
+
+void
+ut_check_output(const char *expected, int line)
+{
+ if (std::string_view(ut_printf_output, ut_printf_output_size) != expected) {
+ std::printf("FAILURE: unexpected output: code line=%d\n", line);
+ std::printf("EXPECTED: %s\n", expected);
+ std::printf("ACTUAL: %.*s\n", static_cast<int>(ut_printf_output_size), ut_printf_output);
+ std::exit(1);
+ }
+}
+
+template <typename T>
+void
+reset(T &v)
+{
+ v.~T();
+ ::new (&v) T();
+}
+
+// Reset globals between tests, so that multiple configurations can be tested in a single UT executable.
+//
+void
+reset_globals()
+{
+ reset(sname_to_matcher_idxs);
+ reset(none_matcher_idxs);
+ reset(other_matcher_idxs);
+ reset(matcher);
+
+ ut_printf_output_size = 0;
+}
+
+// Returns true if error handled correctly.
+//
+bool
+bad_config(int n_arg, char const *const *arg)
+{
+ bool caught{false};
+
+ try {
+ Init()(n_arg, arg);
+
+ } catch (ClientAllowListUTException &) {
+ caught = true;
+ }
+ REQUIRE(ut_printf_output_size < sizeof(ut_printf_output));
+
+ return caught;
+}
+
+} // end anonymous namespace
+
+void
+ut_printf(char const *fmt, std::va_list args)
+{
+ int n = std::vsnprintf(ut_printf_output + ut_printf_output_size, sizeof(ut_printf_output) - ut_printf_output_size, fmt, args);
+ REQUIRE(n > 0);
+ ut_printf_output_size += n;
+ REQUIRE(ut_printf_output_size < sizeof(ut_printf_output));
+ ut_printf_output[ut_printf_output_size++] = '\n';
+}
+
+int
+main()
+{
+ // Each sub-block is a test of a different configuation.
+
+ // Tests of good configurations.
+
+ {
+ char const *args[] = {"dummy plugin name", "*.bbb"};
+ Init()(2, args);
+
+ REQUIRE(matcher.size() == 1);
+
+ REQUIRE(sname_to_matcher_idxs.size() == 0);
+
+ REQUIRE(other_matcher_idxs.size() == 1);
+ REQUIRE(matcher_idxs_same(other_matcher_idxs, none_matcher_idxs));
+ REQUIRE(check_name(other_matcher_idxs, "aaa.bbb"));
+ }
+ REQUIRE(!ut_printf_output_size);
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", "aaa", "*.bbb", "ccc.*", "ddd.*.eee"};
+ Init()(5, args);
+
+ REQUIRE(matcher.size() == 4);
+
+ REQUIRE(sname_to_matcher_idxs.size() == 0);
+
+ REQUIRE(other_matcher_idxs.size() == 4);
+ REQUIRE(matcher_idxs_same(other_matcher_idxs, none_matcher_idxs));
+ REQUIRE(check_name(other_matcher_idxs, "aaa"));
+ REQUIRE(check_name(other_matcher_idxs, "ddd.xxx.eee"));
+ REQUIRE(check_name(other_matcher_idxs, "aaa.bbb"));
+ }
+ REQUIRE(!ut_printf_output_size);
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "good1.yaml"};
+ Init()(2, args);
+
+ REQUIRE(matcher.size() == 7);
+
+ REQUIRE(sname_to_matcher_idxs.size() == 6);
+
+ {
+ REQUIRE(none_matcher_idxs.size() == 4);
+ auto m2 = sname_to_matcher_idxs.find("yahoo.com");
+ REQUIRE(m2 != nullptr);
+ REQUIRE(matcher_idxs_same(*m2, none_matcher_idxs));
+ REQUIRE(check_name(none_matcher_idxs, "aaa"));
+ REQUIRE(!check_name(none_matcher_idxs, "aa"));
+ REQUIRE(!check_name(none_matcher_idxs, "aab"));
+ REQUIRE(check_name(none_matcher_idxs, "ddd.xxx.eee"));
+ REQUIRE(!check_name(none_matcher_idxs, "ddd.xxx.efe"));
+ REQUIRE(check_name(none_matcher_idxs, "aaa.bbb"));
+ }
+ {
+ auto m = sname_to_matcher_idxs.find("xxx");
+ REQUIRE(m != nullptr);
+ REQUIRE(m->size() == 0);
+ REQUIRE(!check_name(*m, "anything.com"));
+ }
+ {
+ REQUIRE(other_matcher_idxs.size() == 2);
+ auto m2 = sname_to_matcher_idxs.find("huffpost.com");
+ REQUIRE(m2 != nullptr);
+ REQUIRE(matcher_idxs_same(*m2, other_matcher_idxs));
+ auto m3 = sname_to_matcher_idxs.find("aOl.CoM");
+ REQUIRE(m3 != nullptr);
+ REQUIRE(matcher_idxs_same(*m3, other_matcher_idxs));
+ REQUIRE(check_name(other_matcher_idxs, "fff"));
+ REQUIRE(check_name(other_matcher_idxs, "ff.bbb"));
+ }
+ {
+ auto m = sname_to_matcher_idxs.find("uuu");
+ REQUIRE(m != nullptr);
+ REQUIRE(m->size() == 1);
+ REQUIRE(check_name(*m, "vvv"));
+ }
+ {
+ auto m = sname_to_matcher_idxs.find("yyy");
+ REQUIRE(m != nullptr);
+ REQUIRE(m->size() == 1);
+ REQUIRE(check_name(*m, "anything.com"));
+ }
+ {
+ auto m = sname_to_matcher_idxs.find("not_there");
+ REQUIRE(nullptr == m);
+ }
+ }
+ REQUIRE(!ut_printf_output_size);
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "good2.yaml"};
+ Init()(2, args);
+
+ REQUIRE(matcher.size() == 1);
+
+ REQUIRE(sname_to_matcher_idxs.size() == 1);
+
+ REQUIRE(none_matcher_idxs.size() == 0);
+ REQUIRE(other_matcher_idxs.size() == 0);
+
+ {
+ auto m = sname_to_matcher_idxs.find("uuu");
+ REQUIRE(m != nullptr);
+ REQUIRE(m->size() == 1);
+ REQUIRE(check_name(*m, "vvv"));
+ }
+ }
+ REQUIRE(!ut_printf_output_size);
+
+ // Tests of bad configurations.
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name"};
+ REQUIRE(bad_config(1, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: must provide at least one plugin parameter\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", "aaa*bbb*ccc"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: bad certificate name pattern aaa*bbb*ccc\n"
+ "client_allow_list: fatal error\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "not-there.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: YAML::Exception \"bad file\" when parsing YAML config file ./" DIR "not-there.yaml\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad1.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: YAML config file ./" DIR "bad1.yaml is empty\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad2.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: YAML::Exception \"invalid node; this may result from using a map"
+ " iterator as a sequence iterator, or vice-versa\" when parsing YAML config file ./" DIR "bad2.yaml\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad3.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: empty server name list\n"
+ "client_allow_list: config error: file=./" DIR "bad3.yaml line=2 column=1\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad4.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: blank space not allowed in server name list\n"
+ "client_allow_list: config error: file=./" DIR "bad4.yaml line=2 column=1\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad5.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: <none> used more than once\n"
+ "client_allow_list: config error: file=./" DIR "bad5.yaml line=4 column=1\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad6.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: <other> used more than once\n"
+ "client_allow_list: config error: file=./" DIR "bad6.yaml line=4 column=1\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad7.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: cert names for SNI server name \"twice\" previously specified\n"
+ "client_allow_list: config error: file=./" DIR "bad7.yaml line=4 column=1\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad8.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: YAML::Exception \"yaml-cpp: error at line 4, column 3: end of map"
+ " not found\" when parsing YAML config file ./" DIR "bad8.yaml\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad9.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: YAML::Exception \"yaml-cpp: error at line 3, column 3: bad conversion\""
+ " when parsing YAML config file ./" DIR "bad9.yaml\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad10.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: YAML::Exception \"yaml-cpp: error at line 4, column 5: the referenced "
+ "anchor is not defined\" when parsing YAML config file ./" DIR "bad10.yaml\n");
+
+ return 0;
+}
+
+#include <../src/tscore/HashFNV.cc>
Review comment:
These includes are only for unit test / make check. Is there a better way to access this code in the unit test executables.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on a change in pull request #7598: Add client_allow_list.so experimental plugin.
Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #7598:
URL: https://github.com/apache/trafficserver/pull/7598#discussion_r594713463
##########
File path: plugins/experimental/client_allow_list/client_allow_list.h
##########
@@ -0,0 +1,301 @@
+/** @file
+
+ SSL client certificate verification plugin, header file.
+
+ Checks for specificate names in the client provided certificate and
+ fails the handshake if none of the good names are present
+
+ @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.
+ */
+
+#pragma once
+
+#include <vector>
+#include <string>
+#include <unordered_map>
+#include <utility>
+#include <algorithm>
+
+#include <pcre.h>
+
+#if defined(CLIENT_ALLOW_LIST_UNIT_TEST)
+
+struct ClientAllowListUTException {
+};
+
+#else
+
+#include <ts/ts.h>
+
+#endif
+
+#include <string_view>
+
+#define PN "client_allow_list"
+
+namespace client_allow_list_plugin
+{
+struct cname_matcher {
+ std::string _CName;
+ pcre *_compiled_RE = nullptr;
+
+ cname_matcher() = default;
+
+ // No copying.
+ //
+ cname_matcher(cname_matcher const &) = delete;
+ cname_matcher &operator=(cname_matcher const &) = delete;
+
+ // Must be moveable to be able to make a vector of these.
+ //
+ cname_matcher(cname_matcher &&that)
+ {
+ _CName = std::move(that._CName);
+ _compiled_RE = that._compiled_RE;
+ that._compiled_RE = nullptr;
+ }
+ cname_matcher &
+ operator=(cname_matcher &&that)
+ {
+ this->~cname_matcher();
+ ::new (this) cname_matcher(std::move(that));
+ return *this;
+ }
+
+ ~cname_matcher()
+ {
+ if (_compiled_RE != nullptr) {
+ pcre_free(_compiled_RE);
+ }
+ }
+};
+
+// Matchers for cert subject/associated names.
+//
+extern std::vector<cname_matcher> matcher;
+
+// Indexes into matcher vector of matchers to use if there is no list of matchers to use specific to the
+// SNI server name.
+//
+extern std::vector<unsigned> other_matcher_idxs;
+
+// Indexes into matcher vector of matchers to use if there is no list of matchers to use specific to the
+// SNI server name.
+//
+extern std::vector<unsigned> none_matcher_idxs;
+
+// Lookup table from C strings to vectors of unsigned. Comparison of string keys is case-insensitive.
+//
+class MapCStrToUVec
+{
+public:
+ MapCStrToUVec() {}
+
+ // Adds a new entry. Saves a copy of the key string, returns a pointer to the new, empty vector.
+ // Returns nullptr if there is already an entry for key.
+ //
+ std::vector<unsigned> *add(std::string_view key);
+
+ // Returns nullptr if no entry with given key.
+ //
+ std::vector<unsigned> const *find(char const *key);
+
+ std::size_t
+ size() const
+ {
+ return _map.size();
+ }
+
+ ~MapCStrToUVec();
+
+ // No copying.
+ //
+ MapCStrToUVec(MapCStrToUVec const &) = delete;
+ MapCStrToUVec &operator=(MapCStrToUVec const &) = delete;
+
+private:
+ struct _Hash {
+ std::size_t operator()(char const *key) const;
+ };
+
+ struct _Eq {
+ bool operator()(char const *lhs, char const *rhs) const;
+ };
+
+ std::unordered_map<char const *, std::vector<unsigned>, _Hash, _Eq> _map;
+};
+
+class HashFnv1a32
Review comment:
Seems like we should move it to ts/util from tscore it we're going to use it in plugins.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on a change in pull request #7598: Add client_allow_list.so experimental plugin.
Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #7598:
URL: https://github.com/apache/trafficserver/pull/7598#discussion_r595664032
##########
File path: plugins/experimental/client_allow_list/unit_tests/test.cc
##########
@@ -0,0 +1,360 @@
+/** @file
+
+ SSL client certificate verification plugin, unit test for utility
+ source file.
+
+ @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.
+ */
+
+#include <cstdlib>
+#include <cstring>
+#include <cstdarg>
+#include <cstdio>
+#include <string_view>
+
+#include <client_allow_list.h>
+
+using client_allow_list_plugin::matcher;
+using client_allow_list_plugin::other_matcher_idxs;
+using client_allow_list_plugin::none_matcher_idxs;
+using client_allow_list_plugin::sname_to_matcher_idxs;
+using client_allow_list_plugin::Init;
+using client_allow_list_plugin::check_name;
+
+#define REQUIRE(EXPR) (require((EXPR), #EXPR, __LINE__))
+
+#undef DIR
+#define DIR "experimental/client_allow_list/unit_tests/"
+
+namespace
+{
+void
+require(bool expr, char const *expr_str, int line_num)
+{
+ if (!expr) {
+ std::printf("FAILURE: %s line=%d\n", expr_str, line_num);
+ std::exit(1);
+ }
+}
+
+bool
+matcher_idxs_same(std::vector<unsigned> const &idxs1, std::vector<unsigned> const &idxs2)
+{
+ if (idxs1.size() != idxs2.size()) {
+ return false;
+ }
+ for (std::size_t i = 0; i < idxs1.size(); ++i) {
+ if (idxs1[i] != idxs2[i]) {
+ return false;
+ }
+ }
+ return true;
+}
+
+char ut_printf_output[2 * 1024];
+std::size_t ut_printf_output_size;
+
+#define UT_CHECK_OUTPUT(S) ut_check_output((S), __LINE__)
+
+void
+ut_check_output(const char *expected, int line)
+{
+ if (std::string_view(ut_printf_output, ut_printf_output_size) != expected) {
+ std::printf("FAILURE: unexpected output: code line=%d\n", line);
+ std::printf("EXPECTED: %s\n", expected);
+ std::printf("ACTUAL: %.*s\n", static_cast<int>(ut_printf_output_size), ut_printf_output);
+ std::exit(1);
+ }
+}
+
+template <typename T>
+void
+reset(T &v)
+{
+ v.~T();
+ ::new (&v) T();
+}
+
+// Reset globals between tests, so that multiple configurations can be tested in a single UT executable.
+//
+void
+reset_globals()
+{
+ reset(sname_to_matcher_idxs);
+ reset(none_matcher_idxs);
+ reset(other_matcher_idxs);
+ reset(matcher);
+
+ ut_printf_output_size = 0;
+}
+
+// Returns true if error handled correctly.
+//
+bool
+bad_config(int n_arg, char const *const *arg)
+{
+ bool caught{false};
+
+ try {
+ Init()(n_arg, arg);
+
+ } catch (ClientAllowListUTException &) {
+ caught = true;
+ }
+ REQUIRE(ut_printf_output_size < sizeof(ut_printf_output));
+
+ return caught;
+}
+
+} // end anonymous namespace
+
+void
+ut_printf(char const *fmt, std::va_list args)
+{
+ int n = std::vsnprintf(ut_printf_output + ut_printf_output_size, sizeof(ut_printf_output) - ut_printf_output_size, fmt, args);
+ REQUIRE(n > 0);
+ ut_printf_output_size += n;
+ REQUIRE(ut_printf_output_size < sizeof(ut_printf_output));
+ ut_printf_output[ut_printf_output_size++] = '\n';
+}
+
+int
+main()
+{
+ // Each sub-block is a test of a different configuation.
+
+ // Tests of good configurations.
+
+ {
+ char const *args[] = {"dummy plugin name", "*.bbb"};
+ Init()(2, args);
+
+ REQUIRE(matcher.size() == 1);
+
+ REQUIRE(sname_to_matcher_idxs.size() == 0);
+
+ REQUIRE(other_matcher_idxs.size() == 1);
+ REQUIRE(matcher_idxs_same(other_matcher_idxs, none_matcher_idxs));
+ REQUIRE(check_name(other_matcher_idxs, "aaa.bbb"));
+ }
+ REQUIRE(!ut_printf_output_size);
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", "aaa", "*.bbb", "ccc.*", "ddd.*.eee"};
+ Init()(5, args);
+
+ REQUIRE(matcher.size() == 4);
+
+ REQUIRE(sname_to_matcher_idxs.size() == 0);
+
+ REQUIRE(other_matcher_idxs.size() == 4);
+ REQUIRE(matcher_idxs_same(other_matcher_idxs, none_matcher_idxs));
+ REQUIRE(check_name(other_matcher_idxs, "aaa"));
+ REQUIRE(check_name(other_matcher_idxs, "ddd.xxx.eee"));
+ REQUIRE(check_name(other_matcher_idxs, "aaa.bbb"));
+ }
+ REQUIRE(!ut_printf_output_size);
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "good1.yaml"};
+ Init()(2, args);
+
+ REQUIRE(matcher.size() == 7);
+
+ REQUIRE(sname_to_matcher_idxs.size() == 6);
+
+ {
+ REQUIRE(none_matcher_idxs.size() == 4);
+ auto m2 = sname_to_matcher_idxs.find("yahoo.com");
+ REQUIRE(m2 != nullptr);
+ REQUIRE(matcher_idxs_same(*m2, none_matcher_idxs));
+ REQUIRE(check_name(none_matcher_idxs, "aaa"));
+ REQUIRE(!check_name(none_matcher_idxs, "aa"));
+ REQUIRE(!check_name(none_matcher_idxs, "aab"));
+ REQUIRE(check_name(none_matcher_idxs, "ddd.xxx.eee"));
+ REQUIRE(!check_name(none_matcher_idxs, "ddd.xxx.efe"));
+ REQUIRE(check_name(none_matcher_idxs, "aaa.bbb"));
+ }
+ {
+ auto m = sname_to_matcher_idxs.find("xxx");
+ REQUIRE(m != nullptr);
+ REQUIRE(m->size() == 0);
+ REQUIRE(!check_name(*m, "anything.com"));
+ }
+ {
+ REQUIRE(other_matcher_idxs.size() == 2);
+ auto m2 = sname_to_matcher_idxs.find("huffpost.com");
+ REQUIRE(m2 != nullptr);
+ REQUIRE(matcher_idxs_same(*m2, other_matcher_idxs));
+ auto m3 = sname_to_matcher_idxs.find("aOl.CoM");
+ REQUIRE(m3 != nullptr);
+ REQUIRE(matcher_idxs_same(*m3, other_matcher_idxs));
+ REQUIRE(check_name(other_matcher_idxs, "fff"));
+ REQUIRE(check_name(other_matcher_idxs, "ff.bbb"));
+ }
+ {
+ auto m = sname_to_matcher_idxs.find("uuu");
+ REQUIRE(m != nullptr);
+ REQUIRE(m->size() == 1);
+ REQUIRE(check_name(*m, "vvv"));
+ }
+ {
+ auto m = sname_to_matcher_idxs.find("yyy");
+ REQUIRE(m != nullptr);
+ REQUIRE(m->size() == 1);
+ REQUIRE(check_name(*m, "anything.com"));
+ }
+ {
+ auto m = sname_to_matcher_idxs.find("not_there");
+ REQUIRE(nullptr == m);
+ }
+ }
+ REQUIRE(!ut_printf_output_size);
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "good2.yaml"};
+ Init()(2, args);
+
+ REQUIRE(matcher.size() == 1);
+
+ REQUIRE(sname_to_matcher_idxs.size() == 1);
+
+ REQUIRE(none_matcher_idxs.size() == 0);
+ REQUIRE(other_matcher_idxs.size() == 0);
+
+ {
+ auto m = sname_to_matcher_idxs.find("uuu");
+ REQUIRE(m != nullptr);
+ REQUIRE(m->size() == 1);
+ REQUIRE(check_name(*m, "vvv"));
+ }
+ }
+ REQUIRE(!ut_printf_output_size);
+
+ // Tests of bad configurations.
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name"};
+ REQUIRE(bad_config(1, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: must provide at least one plugin parameter\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", "aaa*bbb*ccc"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: bad certificate name pattern aaa*bbb*ccc\n"
+ "client_allow_list: fatal error\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "not-there.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: YAML::Exception \"bad file\" when parsing YAML config file ./" DIR "not-there.yaml\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad1.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: YAML config file ./" DIR "bad1.yaml is empty\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad2.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: YAML::Exception \"invalid node; this may result from using a map"
+ " iterator as a sequence iterator, or vice-versa\" when parsing YAML config file ./" DIR "bad2.yaml\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad3.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: empty server name list\n"
+ "client_allow_list: config error: file=./" DIR "bad3.yaml line=2 column=1\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad4.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: blank space not allowed in server name list\n"
+ "client_allow_list: config error: file=./" DIR "bad4.yaml line=2 column=1\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad5.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: <none> used more than once\n"
+ "client_allow_list: config error: file=./" DIR "bad5.yaml line=4 column=1\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad6.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: <other> used more than once\n"
+ "client_allow_list: config error: file=./" DIR "bad6.yaml line=4 column=1\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad7.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: cert names for SNI server name \"twice\" previously specified\n"
+ "client_allow_list: config error: file=./" DIR "bad7.yaml line=4 column=1\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad8.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: YAML::Exception \"yaml-cpp: error at line 4, column 3: end of map"
+ " not found\" when parsing YAML config file ./" DIR "bad8.yaml\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad9.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: YAML::Exception \"yaml-cpp: error at line 3, column 3: bad conversion\""
+ " when parsing YAML config file ./" DIR "bad9.yaml\n");
+
+ reset_globals();
+ {
+ char const *args[] = {"dummy plugin name", DIR "bad10.yaml"};
+ REQUIRE(bad_config(2, args));
+ }
+ UT_CHECK_OUTPUT("client_allow_list: YAML::Exception \"yaml-cpp: error at line 4, column 5: the referenced "
+ "anchor is not defined\" when parsing YAML config file ./" DIR "bad10.yaml\n");
+
+ return 0;
+}
+
+#include <../src/tscore/HashFNV.cc>
Review comment:
Yes that would work, but it makes the dependencies of the test executable more ambiguous.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on pull request #7598: Add client_allow_list.so experimental plugin.
Posted by GitBox <gi...@apache.org>.
ywkaras commented on pull request #7598:
URL: https://github.com/apache/trafficserver/pull/7598#issuecomment-799852321
[approve CI FreeBSD]
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on pull request #7598: Add client_allow_list.so experimental plugin.
Posted by GitBox <gi...@apache.org>.
ywkaras commented on pull request #7598:
URL: https://github.com/apache/trafficserver/pull/7598#issuecomment-803058430
> @ywkaras is going to change it to use std::hash and string_view
I had forgotten that the reason I didn't use std::string_view as the unordered_map key (with default hash and compare) is that, because the key is the SNI servername, it must be case insensitive ( https://tools.ietf.org/html/rfc3546#page-8 ). I could normalize the servername to lowercase before doing the lookup here: https://github.com/apache/trafficserver/pull/7598/files#diff-a2fcfdba174f68b6444d283948e88441c38cfa09799fc67ae93fadf9e81429e0R203 . But that would require using non-standard variable length stack allocation (alloca() or a variable length array). It seems unlikely an extremely long SNI server name could be seen at this point (allowing for a stack blow DOS attack) but I guess I'd have to double check.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bryancall edited a comment on pull request #7598: Add client_allow_list.so experimental plugin.
Posted by GitBox <gi...@apache.org>.
bryancall edited a comment on pull request #7598:
URL: https://github.com/apache/trafficserver/pull/7598#issuecomment-802309132
@ywkaras is going to change it to use std::hash and string_view
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on a change in pull request #7598: Add client_allow_list.so experimental plugin.
Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #7598:
URL: https://github.com/apache/trafficserver/pull/7598#discussion_r594723047
##########
File path: plugins/experimental/client_allow_list/main.cc
##########
@@ -0,0 +1,180 @@
+/** @file
+
+ SSL client certificate verification plugin, main source file.
+
+ Checks for specificate names in the client provided certificate and
+ fails the handshake if none of the good names are present
+
+ @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.
+ */
+
+#include <openssl/ssl.h>
+#include <openssl/x509.h>
+#include <openssl/x509v3.h>
+#include <openssl/asn1.h>
+
+#include "client_allow_list.h"
+
+using namespace client_allow_list_plugin;
+
+#if TS_VERSION_MAJOR < 9
+#define TSVConnSslConnectionGet TSVConnSSLConnectionGet
+#endif
+
+namespace
+{
+inline const char *
+ASN1_get_data(ASN1_STRING *cn)
+{
+#if OPENSSL_VERSION_NUMBER < 0x10100000L
Review comment:
What is the alternative?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on a change in pull request #7598: Add client_allow_list.so experimental plugin.
Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #7598:
URL: https://github.com/apache/trafficserver/pull/7598#discussion_r594785760
##########
File path: plugins/experimental/client_allow_list/main.cc
##########
@@ -0,0 +1,180 @@
+/** @file
+
+ SSL client certificate verification plugin, main source file.
+
+ Checks for specificate names in the client provided certificate and
+ fails the handshake if none of the good names are present
+
+ @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.
+ */
+
+#include <openssl/ssl.h>
+#include <openssl/x509.h>
+#include <openssl/x509v3.h>
+#include <openssl/asn1.h>
+
+#include "client_allow_list.h"
+
+using namespace client_allow_list_plugin;
+
+#if TS_VERSION_MAJOR < 9
+#define TSVConnSslConnectionGet TSVConnSSLConnectionGet
+#endif
+
+namespace
+{
+inline const char *
+ASN1_get_data(ASN1_STRING *cn)
+{
+#if OPENSSL_VERSION_NUMBER < 0x10100000L
Review comment:
With a herring?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on a change in pull request #7598: Add client_allow_list.so experimental plugin.
Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #7598:
URL: https://github.com/apache/trafficserver/pull/7598#discussion_r594798809
##########
File path: plugins/experimental/client_allow_list/main.cc
##########
@@ -0,0 +1,180 @@
+/** @file
+
+ SSL client certificate verification plugin, main source file.
+
+ Checks for specificate names in the client provided certificate and
+ fails the handshake if none of the good names are present
+
+ @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.
+ */
+
+#include <openssl/ssl.h>
+#include <openssl/x509.h>
+#include <openssl/x509v3.h>
+#include <openssl/asn1.h>
+
+#include "client_allow_list.h"
+
+using namespace client_allow_list_plugin;
+
+#if TS_VERSION_MAJOR < 9
+#define TSVConnSslConnectionGet TSVConnSSLConnectionGet
+#endif
+
+namespace
+{
+inline const char *
+ASN1_get_data(ASN1_STRING *cn)
+{
+#if OPENSSL_VERSION_NUMBER < 0x10100000L
Review comment:
OK thanks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org