From b64f29d3744c37b1506e7c6a46ab3dba171853e3 Mon Sep 17 00:00:00 2001 From: clearswift Date: Tue, 7 Mar 2017 17:03:27 +0000 Subject: [PATCH 1/3] FTP functionality --- ci/tsqa/tests/test_forward_proxy.py | 123 ++++++++++++++++++++ doc/admin-guide/files/records.config.en.rst | 12 ++ mgmt/RecordsConfig.cc | 7 ++ proxy/http/HttpConfig.cc | 5 + proxy/http/HttpConfig.h | 5 +- proxy/http/HttpTransact.cc | 6 +- 6 files changed, 155 insertions(+), 3 deletions(-) create mode 100644 ci/tsqa/tests/test_forward_proxy.py diff --git a/ci/tsqa/tests/test_forward_proxy.py b/ci/tsqa/tests/test_forward_proxy.py new file mode 100644 index 00000000000..dc82d0d3b16 --- /dev/null +++ b/ci/tsqa/tests/test_forward_proxy.py @@ -0,0 +1,123 @@ +''' +Test Forward Proxy +''' +# 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. +import os +import logging +import socket +import select +import threading +import helpers + +log = logging.getLogger(__name__) + + +def thread_dummy_ftp(sock): + ''' + Thread to create a dummy ftp over http response using a socket + ''' + sock.listen(0) + num_requests = 0 + # poll + while True: + select.select([sock], [], []) + try: + connection, addr = sock.accept() + connection.send(( + 'HTTP/1.1 200 OK\r\n' + 'Content-Length: {body_len}\r\n' + 'Content-Type: text/html; charset=UTF-8\r\n' + 'Connection: close\r\n\r\n{body}'.format(body_len=len(str(num_requests)), body=num_requests) + )) + connection.close() + num_requests += 1 + except Exception as e: + print 'connection died!', e + pass + +class FTPHelper(helpers.EnvironmentCase): + @staticmethod + def setUpEnv(cls, env): + ''' + This function is responsible for setting up the environment for this fixture + This includes everything pre-daemon start + ''' + cls.sock_map = {} + + def _add_sock(name): + sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + sock.bind(('127.0.0.1', 0)) + cls.sock_map[name] = sock.getsockname()[1] + return sock + # create a socket where we just bind + _add_sock('bound') + + # create a socket where we bind + listen + sock = _add_sock('listen') + sock.listen(1) + + + sock = _add_sock('dummy_ftp') + t = threading.Thread(target=thread_dummy_ftp, args=(sock,)) + t.daemon = True + t.start() + # forward proxy + cls.configs['records.config']['CONFIG']['proxy.config.url_remap.remap_required'] = 0 + # Optional strictly forward proxy setting + cls.configs['records.config']['CONFIG']['proxy.config.reverse_proxy.enabled'] = 0 + cls.configs['records.config']['CONFIG']['proxy.config.http.response_server_enabled'] = 2 + # only add server headers when there weren't any + cls.configs['records.config']['CONFIG']['proxy.config.http.response_server_enabled'] = 2 + + # enable re-connects, timeout of 1s, max retires of 3 + cls.configs['records.config']['CONFIG']['proxy.config.http.connect_attempts_timeout'] = 1 + cls.configs['records.config']['CONFIG']['proxy.config.http.connect_attempts_max_retries'] = 3 + + @staticmethod + def useDummyFtpSocket(self): + '''Verify that we connect via ftp that there is a connection failure''' + proxy_port=self.configs['records.config']['CONFIG']['proxy.config.http.server_ports'] + url = 'GET ftp://127.0.0.1:{0}/dummy_ftp/s HTTP/1.1\r\nHost: 127.0.0.1:{0}\r\n\r\n'.format(self.sock_map['dummy_ftp']) + ftpSocket = socket.socket() + ftpSocket.connect(('127.0.0.1',int(proxy_port))) + ftpSocket.send(url.encode()) + data = ftpSocket.recv(1024) + gen = data.decode('utf-8') + status=format(gen.split('\r\n')[0].split(' ')[1]) + ftpSocket.close() + return status + +class TestOriginServerFtpEnabled(helpers.EnvironmentCase): + @classmethod + def setUpEnv(cls, env): + FTPHelper.setUpEnv(cls,env) + cls.configs['records.config']['CONFIG']['proxy.config.ftp_enabled'] = 1 + + def test_dummy_ftp(self): + status=FTPHelper.useDummyFtpSocket(self) + self.assertEqual(int(status), 200) + +class TestOriginServerFtpDisabled(helpers.EnvironmentCase): + @classmethod + def setUpEnv(cls, env): + FTPHelper.setUpEnv(cls,env) + cls.configs['records.config']['CONFIG']['proxy.config.ftp_enabled'] = 0 + + def test_dummy_ftp(self): + status=FTPHelper.useDummyFtpSocket(self) + self.assertEqual(int(status), 400) + diff --git a/doc/admin-guide/files/records.config.en.rst b/doc/admin-guide/files/records.config.en.rst index 73416f1f7a1..505e6ba0177 100644 --- a/doc/admin-guide/files/records.config.en.rst +++ b/doc/admin-guide/files/records.config.en.rst @@ -3703,3 +3703,15 @@ Sockets .. _Traffic Shaping: https://cwiki.apache.org/confluence/display/TS/Traffic+Shaping + +FTP +======= + +.. ts:cv:: CONFIG proxy.config.ftp_enabled INT 0 + + default: ``0`` meaning ``off`` all Platforms + + This directive enables ftp over http, preventing the 'protocol not supported' error. + This enables an origin server to be reached if it handles ftp over http + and also enables plugins to be written to support this protocol. + diff --git a/mgmt/RecordsConfig.cc b/mgmt/RecordsConfig.cc index 1a911b1709f..b021582b7a8 100644 --- a/mgmt/RecordsConfig.cc +++ b/mgmt/RecordsConfig.cc @@ -1483,6 +1483,13 @@ static const RecordElement RecordsConfig[] = //########### {RECT_CONFIG, "proxy.config.cache.http.compatibility.4-2-0-fixup", RECD_INT, "1", RECU_DYNAMIC, RR_NULL, RECC_NULL, nullptr, RECA_NULL}, + //########### + //# + //# Enable FTP protocol + //# + //########### + {RECT_CONFIG, "proxy.config.ftp_enabled", RECD_INT, "0", RECU_DYNAMIC, RR_NULL, RECC_INT, "[0-1]", RECA_NULL}, + }; // clang-format on diff --git a/proxy/http/HttpConfig.cc b/proxy/http/HttpConfig.cc index a452d199e63..c77caffc4e9 100644 --- a/proxy/http/HttpConfig.cc +++ b/proxy/http/HttpConfig.cc @@ -1129,6 +1129,8 @@ HttpConfig::startup() // Local Manager HttpEstablishStaticConfigLongLong(c.synthetic_port, "proxy.config.admin.synthetic_port"); + // FTP protocol enabled + HttpEstablishStaticConfigByte(c.forward_proxy_ftp_enabled, "proxy.config.ftp_enabled"); // Cluster time delta gets it own callback since it needs // to use ink_atomic_swap c.cluster_time_delta = 0; @@ -1413,6 +1415,9 @@ HttpConfig::reconfigure() params->oride.client_cert_filename = ats_strdup(m_master.oride.client_cert_filename); params->oride.client_cert_filepath = ats_strdup(m_master.oride.client_cert_filepath); + // + params->forward_proxy_ftp_enabled = INT_TO_BOOL(m_master.forward_proxy_ftp_enabled); + // Local Manager params->synthetic_port = m_master.synthetic_port; diff --git a/proxy/http/HttpConfig.h b/proxy/http/HttpConfig.h index e3c6c52673c..a848e5fc73a 100644 --- a/proxy/http/HttpConfig.h +++ b/proxy/http/HttpConfig.h @@ -824,6 +824,8 @@ struct HttpConfigParams : public ConfigInfo { MgmtInt body_factory_response_max_size; + MgmtByte forward_proxy_ftp_enabled; + private: ///////////////////////////////////// // operator = and copy constructor // @@ -926,7 +928,8 @@ inline HttpConfigParams::HttpConfigParams() parser_allow_non_http(1), keepalive_internal_vc(0), server_session_sharing_pool(TS_SERVER_SESSION_SHARING_POOL_THREAD), - body_factory_response_max_size(8192) + body_factory_response_max_size(8192), + forward_proxy_ftp_enabled(0) { } diff --git a/proxy/http/HttpTransact.cc b/proxy/http/HttpTransact.cc index cd9d5dec7c5..1817c8f83b9 100644 --- a/proxy/http/HttpTransact.cc +++ b/proxy/http/HttpTransact.cc @@ -5333,7 +5333,8 @@ HttpTransact::check_request_validity(State *s, HTTPHdr *incoming_hdr) s->hdr_info.request_content_length = 0; } - if (!((scheme == URL_WKSIDX_HTTP) && (method == HTTP_WKSIDX_GET))) { + if (!(((scheme == URL_WKSIDX_HTTP) || (((scheme == URL_WKSIDX_FTP) && s->http_config_param->forward_proxy_ftp_enabled))) && + (method == HTTP_WKSIDX_GET))) { if (scheme != URL_WKSIDX_HTTP && scheme != URL_WKSIDX_HTTPS && method != HTTP_WKSIDX_CONNECT && !((scheme == URL_WKSIDX_WS || scheme == URL_WKSIDX_WSS) && s->is_websocket)) { if (scheme < 0) { @@ -5402,7 +5403,8 @@ HttpTransact::check_request_validity(State *s, HTTPHdr *incoming_hdr) HttpTransact::ResponseError_t HttpTransact::check_response_validity(State *s, HTTPHdr *incoming_hdr) { - ink_assert(s->next_hop_scheme == URL_WKSIDX_HTTP || s->next_hop_scheme == URL_WKSIDX_HTTPS); + ink_assert(s->next_hop_scheme == URL_WKSIDX_HTTP || s->next_hop_scheme == URL_WKSIDX_HTTPS || + s->next_hop_scheme == URL_WKSIDX_FTP); if (incoming_hdr == nullptr) { return NON_EXISTANT_RESPONSE_HEADER; From 5317a5a5536a24d14da65d31a605e597e61a74b4 Mon Sep 17 00:00:00 2001 From: clearswift Date: Mon, 10 Apr 2017 14:20:19 +0100 Subject: [PATCH 2/3] Added tests for FTP functionality using the AU test system. --- tests/gold_tests/ftp/ftp_disabled.test.py | 17 +++++++++++++ tests/gold_tests/ftp/ftp_enabled.test.py | 24 +++++++++++++++++++ tests/gold_tests/ftp/gold/ftp_disabled.gold | 13 ++++++++++ tests/gold_tests/ftp/gold/ftp_enabled.gold | 13 ++++++++++ .../sessionvalidation/sessionvalidation.py | 6 ----- 5 files changed, 67 insertions(+), 6 deletions(-) create mode 100644 tests/gold_tests/ftp/ftp_disabled.test.py create mode 100644 tests/gold_tests/ftp/ftp_enabled.test.py create mode 100644 tests/gold_tests/ftp/gold/ftp_disabled.gold create mode 100644 tests/gold_tests/ftp/gold/ftp_enabled.gold diff --git a/tests/gold_tests/ftp/ftp_disabled.test.py b/tests/gold_tests/ftp/ftp_disabled.test.py new file mode 100644 index 00000000000..84fc50b1d0c --- /dev/null +++ b/tests/gold_tests/ftp/ftp_disabled.test.py @@ -0,0 +1,17 @@ + +Test.Summary = ''' +Test that FTP traffic is rejected by Trafficserver when not enabled. +''' + +ats = Test.MakeATSProcess("ts") +ats.Disk.records_config.update({ + 'proxy.config.ftp_enabled': 0, + 'proxy.config.url_remap.remap_required': 0 + }) + +test = Test.AddTestRun("FTP rejected by Trafficserver") +test.Processes.Default.Command = 'curl --proxy 127.0.0.1:{0} "ftp://127.0.0.1" --verbose'.format(ats.Variables.port) +test.Processes.Default.ReturnCode=0 +test.Processes.Default.Streams.stderr="gold/ftp_disabled.gold" +test.Processes.Default.StartBefore(ats, ready = When.PortOpen(ats.Variables.port)) + diff --git a/tests/gold_tests/ftp/ftp_enabled.test.py b/tests/gold_tests/ftp/ftp_enabled.test.py new file mode 100644 index 00000000000..c5889de8f46 --- /dev/null +++ b/tests/gold_tests/ftp/ftp_enabled.test.py @@ -0,0 +1,24 @@ + +Test.Summary = ''' +Test that FTP traffic is accepted by Trafficserver when enabled. +''' + +ats = Test.MakeATSProcess("ts") +ats.Disk.records_config.update({ + 'proxy.config.ftp_enabled': 1, + 'proxy.config.url_remap.remap_required': 0 + }) +server = Test.MakeOriginServer("server") + +request_header = {"headers": "GET ftp://127.0.0.1:{0}/ HTTP/1.1\r\nHost: 127.0.0.1:{0}\r\n\r\n".format(server.Variables.Port), "timestamp": "1469733493.993", "body": ""} +response_header = {"headers": "HTTP/1.1 200 OK\r\nConnection: close\r\n\r\n", "timestamp": "1469733493.993", "body": ""} + +server.addResponse("session.json", request_header, response_header) + +test = Test.AddTestRun("FTP accepted by Trafficserver") +test.Processes.Default.Command = 'curl --proxy 127.0.0.1:{0} "ftp://127.0.0.1:{1}" --verbose'.format(ats.Variables.port, server.Variables.Port) +test.Processes.Default.ReturnCode=0 +test.Processes.Default.Streams.stderr="gold/ftp_enabled.gold" +test.Processes.Default.StartBefore(ats, ready = When.PortOpen(ats.Variables.port)) +test.Processes.Default.StartBefore(server, ready = When.PortOpen(server.Variables.Port)) + diff --git a/tests/gold_tests/ftp/gold/ftp_disabled.gold b/tests/gold_tests/ftp/gold/ftp_disabled.gold new file mode 100644 index 00000000000..9be7d2ce9a8 --- /dev/null +++ b/tests/gold_tests/ftp/gold/ftp_disabled.gold @@ -0,0 +1,13 @@ +`` +> GET ftp://127.0.0.1``/ HTTP/1.1 +> Host: 127.0.0.1:`` +> User-Agent: curl/`` +> Accept: */* +`` +< HTTP/1.1 400 Unsupported URL Scheme +< Date: `` +< Proxy-Connection: keep-alive +< Server: ATS/`` +< +`` + diff --git a/tests/gold_tests/ftp/gold/ftp_enabled.gold b/tests/gold_tests/ftp/gold/ftp_enabled.gold new file mode 100644 index 00000000000..5e1fe9c9a5a --- /dev/null +++ b/tests/gold_tests/ftp/gold/ftp_enabled.gold @@ -0,0 +1,13 @@ +`` +> GET ftp://127.0.0.1``/ HTTP/1.1 +> Host: 127.0.0.1:`` +> User-Agent: curl/`` +> Accept: */* +`` +< HTTP/1.1 200 OK +< Date: `` +< Proxy-Connection: keep-alive +< Server: ATS/`` +< +`` + diff --git a/tests/tools/sessionvalidation/sessionvalidation.py b/tests/tools/sessionvalidation/sessionvalidation.py index c33b7c702dd..576edf917c5 100644 --- a/tests/tools/sessionvalidation/sessionvalidation.py +++ b/tests/tools/sessionvalidation/sessionvalidation.py @@ -196,12 +196,6 @@ def validateSingleTransaction(txn): _verbose_print("transaction request Host header doesn't have specified host") retval = False - # reject if the host is localhost (since ATS seems to ignore remap rules for localhost requests) - if "127.0.0.1" in txn_req.getHeaders() or "localhost" in txn_req.getHeaders(): - _verbose_print("transaction request Host is localhost, we must reject because ATS ignores remap rules for localhost requests") - retval = False - - # now validate response if not txn_resp: _verbose_print("no transaction response") From 45c0c4aeaf1d64643888fd905a55c01840ca75ff Mon Sep 17 00:00:00 2001 From: clearswift Date: Mon, 10 Apr 2017 14:45:35 +0100 Subject: [PATCH 3/3] Moved variable to correct location in class. --- proxy/http/HttpConfig.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/proxy/http/HttpConfig.h b/proxy/http/HttpConfig.h index a848e5fc73a..81b0ebfac24 100644 --- a/proxy/http/HttpConfig.h +++ b/proxy/http/HttpConfig.h @@ -818,14 +818,14 @@ struct HttpConfigParams : public ConfigInfo { MgmtByte server_session_sharing_pool; + MgmtByte forward_proxy_ftp_enabled; + // All the overridable configurations goes into this class member, but they // are not copied over until needed ("lazy"). OverridableHttpConfigParams oride; MgmtInt body_factory_response_max_size; - MgmtByte forward_proxy_ftp_enabled; - private: ///////////////////////////////////// // operator = and copy constructor // @@ -928,8 +928,8 @@ inline HttpConfigParams::HttpConfigParams() parser_allow_non_http(1), keepalive_internal_vc(0), server_session_sharing_pool(TS_SERVER_SESSION_SHARING_POOL_THREAD), - body_factory_response_max_size(8192), - forward_proxy_ftp_enabled(0) + forward_proxy_ftp_enabled(0), + body_factory_response_max_size(8192) { }