You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by GitBox <gi...@apache.org> on 2019/01/14 23:49:56 UTC
[trafficserver] Diff for: [GitHub] SolidWallOfCode merged pull request
#4782: TLS Bridge: Cleanup, fix TSError for more consistency.
diff --git a/doc/developer-guide/plugins/example-plugins/tls_bridge.en.rst b/doc/developer-guide/plugins/example-plugins/tls_bridge.en.rst
index 57a4661d7c..29821e65d5 100644
--- a/doc/developer-guide/plugins/example-plugins/tls_bridge.en.rst
+++ b/doc/developer-guide/plugins/example-plugins/tls_bridge.en.rst
@@ -66,7 +66,9 @@ Configuration
=============
|Name| requires at least two instances of |TS| (Ingress and Peer). The client connects to the
-ingress |TS|, and the peer |TS| connects to the service.
+ingress |TS|, and the peer |TS| connects to the service. The Peer could in theory be configured to
+connect on to a further |TS| instance, acting as the ingress to that peer, but that doesn't seem a
+useful case.
#. Disable caching on |TS| in ``records.config``::
@@ -87,12 +89,12 @@ ingress |TS|, and the peer |TS| connects to the service.
The Ingress |TS| also needs ``proxy.config.http.server_ports`` configured to have proxy ports
to which the Client can connect.
-#. Remap on the ingress is not required, however, |TS| requires remap in order to accept the
- request. This can be done by disabling the remap requirement::
+#. By default |TS| requires remap in order to allow to outbound request to the peer. To disable this
+ requirement and allow all connections, use the setting ::
CONFIG proxy.config.url_remap.remap_required INT 0
- In this case |TS| will act as an open proxy which is unlikely to be a good idea, therefore if
+ In this case |TS| will act as an open proxy which is unlikely to be a good idea. Therefore if
this approach is used |TS| will need to run in a restricted environment or use access control
(via ``ip_allow.config`` or ``iptables``).
@@ -104,22 +106,23 @@ ingress |TS|, and the peer |TS| connects to the service.
Remapping will be disabled for the user agent connection and so it will not need a rule.
#. If remap is required on the peer to enable the outbound connection from the peer to the service
- (it is not explicitly disabled) the destination port must be
+ (e.g. required remapping is not explicitly disabled) the destination port must be
explicitly stated [#]_. E.g. ::
map https://service:4443 https://service:4443
Note this remap rule cannot alter the actual HTTP transactions between the client and service
- because those happen inside what is effectively a tunnel between the client and service, supported
- by the two |TS| instances. This rule just allows the ``CONNECT`` sent from the ingress to cause a
- tunnel connection from the peer to the service.
+ because those happen inside what is effectively a tunnel between the client and service,
+ supported by the two |TS| instances. This rule serves to allows the ``CONNECT`` sent from the
+ ingress to cause a tunnel connection from the peer to the service.
#. Configure the Ingress |TS| to verify the Peer server certificate::
CONFIG proxy.config.ssl.client.verify.server.policy STRING ENFORCED
-#. Configure Certificate Authority used by the Ingress |TS| to verify the Peer server certificate. If this
- is a directory all of the certificates in the directory are treated as Certificate Authorites. ::
+#. Configure Certificate Authority used by the Ingress |TS| to verify the Peer server certificate.
+ If this is a directory, all of the certificates in the directory are treated as Certificate
+ Authorities. ::
CONFIG proxy.config.ssl.client.CA.cert.filename STRING </path/to/CA_certificate_file_name>
@@ -150,9 +153,9 @@ ingress |TS|, and the peer |TS| connects to the service.
tls_bridge.so .*[.]service[.]com peer.ats:4443 .*[.]altsvc.ats altpeer.ats:4443
- Mappings can also be specified in an external file. For instance, if the file named "bridge.config" in the default |TS|
- configuration directory contained mappings, the ``plugin.config`` configuration line could look
- like ::
+ Mappings can also be specified in an external file. For instance, if there was file named
+ "bridge.config" in the default |TS| configuration directory which contained mappings, the
+ ``plugin.config`` configuration line could look like ::
tls_bridge.so .*[.]service[.]com peer.ats:4443 --file bridge.config
@@ -162,7 +165,7 @@ ingress |TS|, and the peer |TS| connects to the service.
These are not identical - direct mappings and file mappings are processed in order. This means in
the first example, the direct mapping is checked before any mappping in "bridge.config", and in
- the latter example the mappings in "bridge.config" are checked before the direct mapping. There
+ the latter example the mappings in "bridge.config" are checked before the direct mappings. There
can be multiple "--file" arguments, which are processed in the order they appear in
"plugin.config". The file name can be absolute, or relative. If the file name is relative, it is
relative to the |TS| configuration directory. Therefore, in these examples, "bridge.config" must
diff --git a/plugins/experimental/tls_bridge/tls_bridge.cc b/plugins/experimental/tls_bridge/tls_bridge.cc
index e8b78b6ef9..d295d08d34 100644
--- a/plugins/experimental/tls_bridge/tls_bridge.cc
+++ b/plugins/experimental/tls_bridge/tls_bridge.cc
@@ -1,19 +1,16 @@
/*
- 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
+ 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.
+ 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 "ts/ts.h"
@@ -24,19 +21,21 @@
#include "tscore/ts_file.h"
#include "regex.h"
-#define PLUGIN_NAME "TLS Bridge"
-#define PLUGIN_TAG "tls_bridge"
-
using ts::TextView;
namespace
{
+constexpr char const PLUGIN_NAME[] = "TLS Bridge";
+constexpr char const PLUGIN_TAG[] = "tls_bridge";
+
// Base format string for making the internal CONNECT.
char const CONNECT_FORMAT[] = "CONNECT https:%.*s HTTP/1.1\r\n\r\n";
+// TextView of the 'CONNECT' method string.
const TextView METHOD_CONNECT{TS_HTTP_METHOD_CONNECT, TS_HTTP_LEN_CONNECT};
constexpr TextView CONFIG_FILE_ARG{"--file"};
const std::string TS_CONFIG_DIR{TSConfigDirGet()};
+
}; // namespace
/* ------------------------------------------------------------------------------------ */
@@ -70,6 +69,8 @@ class BridgeConfig
using self_type = BridgeConfig;
/// Construct an item.
+ /// @internal Pass in the compiled regex because no instance of this is created if
+ /// the regex doesn't compile successfully.
Item(std::string_view pattern, Regex &&r, std::string_view service) : _pattern(pattern), _r(std::move(r)), _service(service) {}
std::string _pattern; ///< Original configuration regular expression.
@@ -114,13 +115,12 @@ BridgeConfig::load_pair(std::string_view rxp, std::string_view service, ts::file
if (r.compile(pattern.c_str(), Regex::ANCHORED)) {
_items.emplace_back(rxp, std::move(r), service);
} else {
- char buff[std::numeric_limits<int>::digits10 + 2];
- char const *place = "";
+ char buff[std::numeric_limits<int>::digits10 + 2] = "";
if (ln) {
snprintf(buff, sizeof(buff), " on line %d", ln);
- place = buff;
}
- TSError("%s: Failed to compile regular expression '%.*s' in %s%s", PLUGIN_TAG, int(rxp.size()), rxp.data(), src.c_str(), place);
+ TSError("[%s] Failed to compile regular expression '%.*s' in %s%s", PLUGIN_NAME, int(rxp.size()), rxp.data(), src.c_str(),
+ buff);
}
}
@@ -132,7 +132,7 @@ BridgeConfig::load_config(int argc, const char *argv[])
for (int i = 0; i < argc; i += 2) {
if (argv[i] == CONFIG_FILE_ARG) {
if (i + 1 >= argc) {
- TSError("%s: Invalid '%.*s' argument - no file name found.", PLUGIN_TAG, int(CONFIG_FILE_ARG.size()),
+ TSError("[%s] Invalid '%.*s' argument - no file name found.", PLUGIN_NAME, int(CONFIG_FILE_ARG.size()),
CONFIG_FILE_ARG.data());
} else {
ts::file::path fp(argv[i + 1]);
@@ -143,7 +143,7 @@ BridgeConfig::load_config(int argc, const char *argv[])
// bulk load the file.
std::string content{ts::file::load(fp, ec)};
if (ec) {
- TSError("%s: Invalid '%.*s' argument - unable to read file '%s' : %s.", PLUGIN_TAG, int(CONFIG_FILE_ARG.size()),
+ TSError("[%s] Invalid '%.*s' argument - unable to read file '%s' : %s.", PLUGIN_NAME, int(CONFIG_FILE_ARG.size()),
CONFIG_FILE_ARG.data(), fp.c_str(), ec.message().c_str());
} else {
@@ -162,7 +162,7 @@ BridgeConfig::load_config(int argc, const char *argv[])
service.ltrim_if(&isspace); // dump extra separating space.
// Only need to check service, as if the line isn't empty rxp will also be non-empty.
if (service.empty()) {
- TSError("%s: Invalid line %d in '%s' - no destination service found.", PLUGIN_TAG, line_no, fp.c_str());
+ TSError("[%s] Invalid line %d in '%s' - no destination service found.", PLUGIN_NAME, line_no, fp.c_str());
} else {
this->load_pair(rxp, service, fp, line_no);
}
@@ -170,11 +170,11 @@ BridgeConfig::load_config(int argc, const char *argv[])
}
}
} else if (argv[i][0] == '-') {
- TSError("%s: Unrecognized option '%s'", PLUGIN_TAG, argv[i]);
+ TSError("[%s] Unrecognized option '%s'", PLUGIN_NAME, argv[i]);
i -= 1; // Don't skip next arg.
} else {
if (i + 1 >= argc) {
- TSError("%s: Regular expression '%s' without destination service", PLUGIN_TAG, argv[i]);
+ TSError("[%s] Regular expression '%s' without destination service", PLUGIN_NAME, argv[i]);
} else {
this->load_pair(argv[i], argv[i + 1], plugin_config_fp);
}
@@ -301,7 +301,7 @@ void
Bridge::net_accept(TSVConn vc)
{
char buff[1024];
- int64_t n = snprintf(buff, sizeof(buff), CONNECT_FORMAT, static_cast<int>(_peer.size()), _peer.data());
+ int64_t n = snprintf(buff, sizeof(buff), CONNECT_FORMAT, int(_peer.size()), _peer.data());
TSDebug(PLUGIN_TAG, "Received UA VConn, connecting to peer %.*s", int(_peer.size()), _peer.data());
// UA side intercepted.
@@ -685,12 +685,12 @@ TSPluginInit(int argc, char const *argv[])
TSPluginRegistrationInfo info{PLUGIN_NAME, "Oath:", "solidwallofcode@oath.com"};
if (TSPluginRegister(&info) != TS_SUCCESS) {
- TSError(PLUGIN_NAME ": plugin registration failed.");
+ TSError("[%s] plugin registration failed.", PLUGIN_NAME);
}
Config.load_config(argc - 1, argv + 1);
if (Config.count() <= 0) {
- TSError("%s: No destinations defined, plugin disabled", PLUGIN_TAG);
+ TSError("[%s] No destinations defined, plugin disabled", PLUGIN_NAME);
}
TSCont contp = TSContCreate(CB_Read_Request_Hdr, TSMutexCreate());
With regards,
Apache Git Services