This is an automated email from the ASF dual-hosted git repository. zwoop pushed a commit to branch 8.1.x in repository https://gitbox.apache.org/repos/asf/trafficserver.git
commit 3398a1681f5e2eb5f5e8c469736e1697d9f02c2d Author: Brian Olsen <[email protected]> AuthorDate: Fri Feb 28 16:49:06 2020 +0000 cache_range_requests plugin: detect and handle TSCacheUrlSet failures which poison the cache (cherry picked from commit 8021a8e77723784b576592f5d93c703733706078) --- .../plugins/cache_range_requests.en.rst | 191 ++++++++++++++++++++ .../cache_range_requests/cache_range_requests.cc | 105 +++++++---- .../cache_range_requests_cachekey.test.py | 198 +++++++++++++++++++++ 3 files changed, 457 insertions(+), 37 deletions(-) diff --git a/doc/admin-guide/plugins/cache_range_requests.en.rst b/doc/admin-guide/plugins/cache_range_requests.en.rst new file mode 100644 index 0000000..6d11d21 --- /dev/null +++ b/doc/admin-guide/plugins/cache_range_requests.en.rst @@ -0,0 +1,191 @@ +.. 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:: ../../common.defs + +.. _admin-plugins-cache-range-requests: + + +Cache Range Requests Plugin +*************************** + +Description +=========== + +Most origin servers support HTTP/1.1 range requests (rfc 7233). +ATS internally handles range request caching in one of 2 ways: + +* Don't cache range requests. +* Only server range requests from a wholly cached object. + +This plugin allows you to remap individual range requests so that they +are stored as individual objects in the ATS cache when subsequent range +requests are likely to use the same range. This spreads range requests +over multiple stripes thereby reducing I/O wait and system load averages. + +:program:`cache_range_requests` reads the range request header byte range +value and then creates a new ``cache key URL`` using the original request +url with the range value appended to it. The range header is removed +where appropriate from the requests and the origin server response code +is changed from a 206 to a 200 to insure that the object is written to +cache using the new cache key url. The response code sent to the client +will be changed back to a 206 and all requests to the origin server will +contain the range header so that the correct response is received. + +The :program:`cache_range_requests` plugin by itself has no logic to +efficiently manage overlapping ranges. It is best to use this plugin +in conjunction with a smart client that only requests predetermined +non overlapping cache ranges (request blocking) or as a helper for the +:program:`slice` plugin. + +Only requests which contain the ``Range: <units>=`` GET header +will be served by the :program:`cache_range_requests` plugin. + +If/when ATS implements partial object caching this plugin will +become deprecated. + +*NOTE* Given a multi range request the :program:`cache_range_requests` +only processes the first range and ignores the rest. + +How to run the plugin +===================== + +The plugin can run as a global plugin (a single global instance configured +using :file:`plugin.config`) or as per-remap plugin (a separate instance +configured per remap rule in :file:`remap.config`). + +Global instance +--------------- + +.. code:: + + $ cat plugin.config + cache_range_request.so + + +Per-remap instance +------------------ + +.. code:: + + $cat remap.config + map http://www.example.com http://www.origin.com \ + @plugin=cache_range_requests.so + + +If both global and per-remap instance are used the per-remap configuration +would take precedence (per-remap configuration would be applied and the +global configuration ignored). + +Plugin options +============== + + +Parent Selection as Cache Key +----------------------------- + +.. option:: --ps-cachekey +.. option:: -p + +Without this option parent selection is based solely on the hash of a +URL Path a URL is requested from the same upstream parent cache listed +in parent.config + + +With this option parent selection is based on the full ``cache key URL`` +which includes information about the partial content range. In this mode, +all requests (include partial content) will use consistent hashing method +for parent selection. + + +X-CRR-IMS header support +------------------------ + +.. option:: --consider-ims +.. option:: -c + +To support slice plugin self healing an option to force revalidation +after cache lookup complete was added. This option is triggered by a +special header: + +.. code:: + + X-CRR-IMS: Tue, 19 Nov 2019 13:26:45 GMT + +When this header is provided and a `cache hit fresh` is encoutered the +``Date`` header of the object in cache is compared to this header date +value. If the cache date is *less* than this IMS date then the object +is marked as STALE and an appropriate If-Modified-Since or If-Match +request along with this X-CRR-IMS header is passed up to the parent. + +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 +====================== + +Global plugin +------------- + +.. code:: + + cache_range_requests.so --ps-cachekey --consider-ims --no-modify-cachekey + +or + +.. code:: + + 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 @pparam=--no-modify-cachekey + +or + +.. code:: + + map http://ats http://parent @plugin=cache_range_requests.so @pparam=-p @pparam=-c @pparam=-n diff --git a/plugins/experimental/cache_range_requests/cache_range_requests.cc b/plugins/experimental/cache_range_requests/cache_range_requests.cc index b0b61ec..22f4151 100644 --- a/plugins/experimental/cache_range_requests/cache_range_requests.cc +++ b/plugins/experimental/cache_range_requests/cache_range_requests.cc @@ -26,11 +26,13 @@ * requests. */ -#include <cstdio> -#include <cstring> #include "ts/ts.h" #include "ts/remap.h" +#include <cstdio> +#include <cstring> +#include <getopt.h> + #define PLUGIN_NAME "cache_range_requests" #define DEBUG_LOG(fmt, ...) TSDebug(PLUGIN_NAME, "[%s:%d] %s(): " fmt, __FILE__, __LINE__, __func__, ##__VA_ARGS__) #define ERROR_LOG(fmt, ...) TSError("[%s:%d] %s(): " fmt, __FILE__, __LINE__, __func__, ##__VA_ARGS__) @@ -41,7 +43,8 @@ typedef enum parent_select_mode { } parent_select_mode_t; struct pluginconfig { - parent_select_mode_t ps_mode; + parent_select_mode_t ps_mode{PS_DEFAULT}; + bool modify_cache_key{true}; }; struct txndata { @@ -56,7 +59,7 @@ static void handle_server_read_response(TSHttpTxn, struct txndata *); static int remove_header(TSMBuffer, TSMLoc, const char *, int); static bool set_header(TSMBuffer, TSMLoc, const char *, int, const char *, int); static int transaction_handler(TSCont, TSEvent, void *); -static struct pluginconfig *create_pluginconfig(int argc, const char *argv[]); +static struct pluginconfig *create_pluginconfig(int argc, char *const argv[]); static void delete_pluginconfig(struct pluginconfig *); // pluginconfig struct (global plugin only) @@ -68,26 +71,50 @@ static struct pluginconfig *gPluginConfig = nullptr; * Walk plugin argument list and updates config */ static struct pluginconfig * -create_pluginconfig(int argc, const char *argv[]) +create_pluginconfig(int argc, char *const argv[]) { - struct pluginconfig *pc = nullptr; - - pc = (struct pluginconfig *)TSmalloc(sizeof(struct pluginconfig)); + struct pluginconfig *pc = new pluginconfig; if (nullptr == pc) { ERROR_LOG("Can't allocate pluginconfig"); return nullptr; } - // Plugin uses default ATS selection (hash of URL path) - pc->ps_mode = PS_DEFAULT; + static const struct option longopts[] = { + {const_cast<char *>("ps-cachekey"), no_argument, nullptr, 'p'}, + {const_cast<char *>("no-modify-cachekey"), no_argument, nullptr, 'n'}, + {nullptr, 0, nullptr, 0}, + }; - // Walk through param list. - for (int c = 0; c < argc; c++) { - if (strcmp("ps_mode:cache_key_url", argv[c]) == 0) { - pc->ps_mode = PS_CACHEKEY_URL; + // getopt assumes args start at '1' + ++argc; + --argv; + + for (;;) { + int const opt = getopt_long(argc, argv, "", longopts, nullptr); + if (-1 == opt) { break; } + + switch (opt) { + case 'p': { + DEBUG_LOG("Plugin modifies parent selection key"); + pc->ps_mode = PS_CACHEKEY_URL; + } break; + case 'n': { + DEBUG_LOG("Plugin doesn't modify cache key"); + pc->modify_cache_key = false; + } break; + default: { + DEBUG_LOG("Unknown option: '%c'", opt); + } break; + } + } + + // Backwards compatibility + if (optind < argc && 0 == strcmp("ps_mode:cache_key_url", argv[optind])) { + DEBUG_LOG("Plugin modifies parent selection key (deprecated)"); + pc->ps_mode = PS_CACHEKEY_URL; } return pc; @@ -101,7 +128,7 @@ delete_pluginconfig(struct pluginconfig *pc) { if (nullptr != pc) { DEBUG_LOG("Delete struct pluginconfig"); - TSfree(pc); + delete pc; pc = nullptr; } } @@ -165,23 +192,29 @@ range_header_check(TSHttpTxn txnp, struct pluginconfig *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 (nullptr != pc && PS_DEFAULT != pc->ps_mode) { - TSMLoc ps_loc = nullptr; - - if (PS_CACHEKEY_URL == pc->ps_mode) { - const char *start = cache_key_url; - const char *end = cache_key_url + cache_key_url_length; - if (TS_SUCCESS == TSUrlCreate(hdr_bufp, &ps_loc) && - TS_PARSE_DONE == TSUrlParse(hdr_bufp, ps_loc, &start, end) && // This should always succeed. - TS_SUCCESS == TSHttpTxnParentSelectionUrlSet(txnp, hdr_bufp, ps_loc)) { - DEBUG_LOG("Set Parent Selection URL to cache_key_url: %s", cache_key_url); - TSHandleMLocRelease(hdr_bufp, TS_NULL_MLOC, ps_loc); + // Optionally set the parent_selection_url to the cache_key url or path + if (PS_DEFAULT != pc->ps_mode) { + TSMLoc ps_loc = nullptr; + + if (PS_CACHEKEY_URL == pc->ps_mode) { + const char *start = cache_key_url; + const char *end = cache_key_url + cache_key_url_length; + if (TS_SUCCESS == TSUrlCreate(hdr_bufp, &ps_loc) && + TS_PARSE_DONE == TSUrlParse(hdr_bufp, ps_loc, &start, end) && // This should always succeed. + TS_SUCCESS == TSHttpTxnParentSelectionUrlSet(txnp, hdr_bufp, ps_loc)) { + DEBUG_LOG("Set Parent Selection URL to cache_key_url: %s", cache_key_url); + TSHandleMLocRelease(hdr_bufp, TS_NULL_MLOC, ps_loc); + } } } } @@ -411,11 +444,10 @@ TSRemapNewInstance(int argc, char *argv[], void **ih, char * /*errbuf */, int /* } // Skip over the Remap params - const char **plugin_argv = const_cast<const char **>(argv + 2); - argc -= 2; + char *const *plugin_argv = const_cast<char *const *>(argv); // Parse the argument list. - *ih = (struct pluginconfig *)create_pluginconfig(argc, plugin_argv); + *ih = (struct pluginconfig *)create_pluginconfig(argc - 2, plugin_argv + 2); if (*ih == nullptr) { ERROR_LOG("Can't create pluginconfig"); @@ -472,9 +504,8 @@ TSPluginInit(int argc, const char *argv[]) if (nullptr == gPluginConfig) { if (argc > 1) { // Skip ahead of first param (name of traffic server plugin shared object) - const char **plugin_argv = const_cast<const char **>(argv + 1); - argc -= 1; - gPluginConfig = create_pluginconfig(argc, plugin_argv); + char *const *plugin_argv = const_cast<char *const *>(argv); + gPluginConfig = create_pluginconfig(argc - 1, plugin_argv + 1); } } 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 0000000..2622694 --- /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")
