diff --git a/doc/admin-guide/plugins/cache_range_requests.en.rst b/doc/admin-guide/plugins/cache_range_requests.en.rst index 5d33f7c0ac7..6d11d21121a 100644 --- a/doc/admin-guide/plugins/cache_range_requests.en.rst +++ b/doc/admin-guide/plugins/cache_range_requests.en.rst @@ -137,6 +137,29 @@ In order for this to properly work in a CDN each cache in the chain *SHOULD* also contain a remap rule with the :program:`cache_range_requests` plugin with this option set. +Don't modify the Cache Key +-------------------------- + +.. option:: --no-modify-cachekey +.. option:: -n + +With each transaction TSCacheUrlSet may only be called once. When +using the `cache_range_requests` plugin in conjunction with the +`cachekey` plugin the option `--include-headers=Range` should be +added as a `cachekey` parameter with this option. Configuring this +incorrectly *WILL* result in cache poisoning. + +.. code:: + + map http://ats/ http://parent/ \ + @plugin=cachekey.so @pparam=--include-headers=Range \ + @plugin=cache_range_requests.so @pparam=--no-modify-cachekey + +*Without this `cache_range_requests` plugin option* + +*IF* the TSCacheUrlSet call in cache_range_requests fails, an error is +generated in the logs and the cache_range_requests plugin will disable +transaction caching in order to avoid cache poisoning. Configuration examples ====================== @@ -146,23 +169,23 @@ Global plugin .. code:: - cache_range_requests.so --ps-cachekey --consider-ims + cache_range_requests.so --ps-cachekey --consider-ims --no-modify-cachekey or .. code:: - cache_range_requests.so -p -c + cache_range_requests.so -p -c -n Remap plugin ------------ .. code:: - map http://ats http://parent @plugin=cache_range_requests.so @pparam=--ps-cachekey @pparam=--consider-ims + map http://ats http://parent @plugin=cache_range_requests.so @pparam=--ps-cachekey @pparam=--consider-ims @pparam=--no-modify-cachekey or .. code:: - map http://ats http://parent @plugin=cache_range_requests.so @pparam=-p @pparam=-c + map http://ats http://parent @plugin=cache_range_requests.so @pparam=-p @pparam=-c @pparam=-n diff --git a/plugins/cache_range_requests/cache_range_requests.cc b/plugins/cache_range_requests/cache_range_requests.cc index 13381d248e9..41a770221ba 100644 --- a/plugins/cache_range_requests/cache_range_requests.cc +++ b/plugins/cache_range_requests/cache_range_requests.cc @@ -49,6 +49,7 @@ typedef enum parent_select_mode { struct pluginconfig { parent_select_mode_t ps_mode{PS_DEFAULT}; bool consider_ims_header{false}; + bool modify_cache_key{true}; }; struct txndata { @@ -96,6 +97,7 @@ create_pluginconfig(int argc, char *const argv[]) static const struct option longopts[] = { {const_cast("ps-cachekey"), no_argument, nullptr, 'p'}, {const_cast("consider-ims"), no_argument, nullptr, 'c'}, + {const_cast("no-modify-cachekey"), no_argument, nullptr, 'n'}, {nullptr, 0, nullptr, 0}, }; @@ -118,6 +120,10 @@ create_pluginconfig(int argc, char *const argv[]) DEBUG_LOG("Plugin considers the '%.*s' header", (int)X_IMS_HEADER.size(), X_IMS_HEADER.data()); pc->consider_ims_header = true; } break; + case 'n': { + DEBUG_LOG("Plugin doesn't modify cache key"); + pc->modify_cache_key = false; + } break; default: { } break; } @@ -203,12 +209,16 @@ range_header_check(TSHttpTxn txnp, pluginconfig *const pc) TSfree(req_url); } - // set the cache key. - if (TS_SUCCESS != TSCacheUrlSet(txnp, cache_key_url, cache_key_url_length)) { - DEBUG_LOG("failed to change the cache url to %s.", cache_key_url); - } - if (nullptr != pc) { + // set the cache key if configured to. + if (pc->modify_cache_key && TS_SUCCESS != TSCacheUrlSet(txnp, cache_key_url, cache_key_url_length)) { + ERROR_LOG("failed to change the cache url to %s.", cache_key_url); + ERROR_LOG("Disabling cache for this transaction to avoid cache poisoning."); + TSHttpTxnServerRespNoStoreSet(txnp, 1); + TSHttpTxnRespCacheableSet(txnp, 0); + TSHttpTxnReqCacheableSet(txnp, 0); + } + // Optionally set the parent_selection_url to the cache_key url or path if (PS_DEFAULT != pc->ps_mode) { TSMLoc ps_loc = nullptr; diff --git a/tests/gold_tests/pluginTest/cache_range_requests/cache_range_requests_cachekey.test.py b/tests/gold_tests/pluginTest/cache_range_requests/cache_range_requests_cachekey.test.py new file mode 100644 index 00000000000..2622694a04e --- /dev/null +++ b/tests/gold_tests/pluginTest/cache_range_requests/cache_range_requests_cachekey.test.py @@ -0,0 +1,198 @@ +''' +''' +# 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 time + +Test.Summary = ''' +cache_range_requests with cachekey +''' + +## Test description: +# Preload the cache with the entire asset to be range requested. +# Reload remap rule with cache_range_requests plugin +# Request content through the cache_range_requests plugin + +Test.SkipUnless( + Condition.PluginExists('cache_range_requests.so'), + Condition.PluginExists('cachekey.so'), + Condition.PluginExists('xdebug.so'), +) +Test.ContinueOnFail = False +Test.testName = "cache_range_requests_cachekey" + +# Define and configure ATS, enable traffic_ctl config reload +ts = Test.MakeATSProcess("ts", command="traffic_server") + +# Define and configure origin server +server = Test.MakeOriginServer("server", lookup_key="{%uuid}") + +# default root +req_chk = {"headers": + "GET / HTTP/1.1\r\n" + + "Host: www.example.com\r\n" + + "uuid: none\r\n" + + "\r\n", + "timestamp": "1469733493.993", + "body": "" +} + +res_chk = {"headers": + "HTTP/1.1 200 OK\r\n" + + "Connection: close\r\n" + + "\r\n", + "timestamp": "1469733493.993", + "body": "" +} + +server.addResponse("sessionlog.json", req_chk, res_chk) + +body = "lets go surfin now" +bodylen = len(body) + +# this request should work +req_full = {"headers": + "GET /path HTTP/1.1\r\n" + + "Host: www.example.com\r\n" + + "Accept: */*\r\n" + + "uuid: full\r\n" + + "\r\n", + "timestamp": "1469733493.993", + "body": "" +} + +res_full = {"headers": + "HTTP/1.1 206 Partial Content\r\n" + + "Accept-Ranges: bytes\r\n" + + 'Etag: "foo"\r\n' + + "Cache-Control: public, max-age=500\r\n" + + "Connection: close\r\n" + + "\r\n", + "timestamp": "1469733493.993", + "body": body +} + +server.addResponse("sessionlog.json", req_full, res_full) + +# this request should work +req_good = {"headers": + "GET /path HTTP/1.1\r\n" + + "Host: www.example.com\r\n" + + "Accept: */*\r\n" + + "Range: bytes=0-\r\n" + + "uuid: range_full\r\n" + + "\r\n", + "timestamp": "1469733493.993", + "body": "" +} + +res_good = {"headers": + "HTTP/1.1 206 Partial Content\r\n" + + "Accept-Ranges: bytes\r\n" + + 'Etag: "foo"\r\n' + + "Cache-Control: public, max-age=500\r\n" + + "Content-Range: bytes 0-{0}/{0}\r\n".format(bodylen) + + "Connection: close\r\n" + + "\r\n", + "timestamp": "1469733493.993", + "body": body +} + +server.addResponse("sessionlog.json", req_good, res_good) + +# this request should fail with a cache_range_requests asset +req_fail = {"headers": + "GET /path HTTP/1.1\r\n" + + "Host: www.fail.com\r\n" + + "Accept: */*\r\n" + + "Range: bytes=0-\r\n" + + "uuid: range_fail\r\n" + + "\r\n", + "timestamp": "1469733493.993", + "body": "" +} + +res_fail = {"headers": + "HTTP/1.1 206 Partial Content\r\n" + + "Accept-Ranges: bytes\r\n" + + 'Etag: "foo"\r\n' + + "Cache-Control: public, max-age=500\r\n" + + "Content-Range: bytes 0-{0}/{0}\r\n".format(bodylen) + + "Connection: close\r\n" + + "\r\n", + "timestamp": "1469733493.993", + "body": body +} + +server.addResponse("sessionlog.json", req_fail, res_fail) + +# cache range requests plugin remap, working config +ts.Disk.remap_config.AddLine( + 'map http://www.example.com http://127.0.0.1:{}'.format(server.Variables.Port) + + ' @plugin=cachekey.so @pparam=--include-headers=Range' + + ' @plugin=cache_range_requests.so @pparam=--no-modify-cachekey', +) + +# improperly configured cache_range_requests with cachekey +ts.Disk.remap_config.AddLine( + 'map http://www.fail.com http://127.0.0.1:{}'.format(server.Variables.Port) + + ' @plugin=cachekey.so @pparam=--static-prefix=foo' + ' @plugin=cache_range_requests.so', +) + +# cache debug +ts.Disk.plugin_config.AddLine('xdebug.so') + +# minimal configuration +ts.Disk.records_config.update({ + 'proxy.config.diags.debug.enabled': 1, + 'proxy.config.diags.debug.tags': 'cache_range_requests', + 'proxy.config.http.cache.http': 1, + 'proxy.config.http.wait_for_cache': 1, +}) + +curl_and_args = 'curl -s -D /dev/stdout -o /dev/stderr -x localhost:{} -H "x-debug: x-cache"'.format(ts.Variables.port) + +# 0 Test - Fetch full asset into cache (ensure cold) +tr = Test.AddTestRun("full asset fetch") +ps = tr.Processes.Default +ps.StartBefore(server, ready=When.PortOpen(server.Variables.Port)) +ps.StartBefore(Test.Processes.ts, ready=When.PortOpen(ts.Variables.port)) +ps.Command = curl_and_args + ' http://www.example.com/path -H "uuid: full"' +ps.ReturnCode = 0 +ps.Streams.stdout.Content = Testers.ContainsExpression("X-Cache: miss", "expected cache miss for load") +tr.StillRunningAfter = ts + +# 1 Test - Fetch whole asset into cache via range request (ensure cold) +tr = Test.AddTestRun("0- asset fetch") +ps = tr.Processes.Default +ps.Command = curl_and_args + ' http://www.example.com/path -r 0- -H "uuid: range_full"' +ps.ReturnCode = 0 +ps.Streams.stdout.Content = Testers.ContainsExpression("X-Cache: miss", "expected cache miss for load") +tr.StillRunningAfter = ts + +# 2 Test - Ensure assert happens instead of possible cache poisoning. +tr = Test.AddTestRun("Attempt poisoning") +ps = tr.Processes.Default +ps.Command = curl_and_args + ' http://www.fail.com/path -r 0- -H "uuid: range_fail"' +ps.ReturnCode = 0 +tr.StillRunningAfter = ts + +ts.Disk.diags_log.Content = Testers.ContainsExpression("ERROR", "error condition hit") +ts.Disk.diags_log.Content = Testers.ContainsExpression("failed to change the cache url", "ensure failure for misconfiguration") +ts.Disk.diags_log.Content = Testers.ContainsExpression("Disabling cache for this transaction to avoid cache poisoning", "ensure transaction caching disabled") diff --git a/tests/gold_tests/pluginTest/cache_range_requests/cache_range_requests_ims.test.py b/tests/gold_tests/pluginTest/cache_range_requests/cache_range_requests_ims.test.py index f249700ff39..86a7ce9be29 100644 --- a/tests/gold_tests/pluginTest/cache_range_requests/cache_range_requests_ims.test.py +++ b/tests/gold_tests/pluginTest/cache_range_requests/cache_range_requests_ims.test.py @@ -33,7 +33,7 @@ Condition.PluginExists('xdebug.so'), ) Test.ContinueOnFail = False -Test.testName = "cache_range_requests" +Test.testName = "cache_range_requests_ims" # Define and configure ATS ts = Test.MakeATSProcess("ts", command="traffic_server")