This is an automated email from the ASF dual-hosted git repository.
eze pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new 6e955a8 cache_range_requests plugin: detect and handle TSCacheUrlSet
failures which poison the cache (#6464)
6e955a8 is described below
commit 6e955a8a8a28a142c72a7c65576c0d19b8db2570
Author: Brian Olsen <brian_ols...@comcast.com>
AuthorDate: Tue Mar 3 11:18:25 2020 -0700
cache_range_requests plugin: detect and handle TSCacheUrlSet failures which
poison the cache (#6464)
---
.../plugins/cache_range_requests.en.rst | 31 +++++-
.../cache_range_requests/cache_range_requests.cc | 20 +++-
...st.py => cache_range_requests_cachekey.test.py} | 113 ++++++++++++++++-----
.../cache_range_requests_ims.test.py | 2 +-
4 files changed, 128 insertions(+), 38 deletions(-)
diff --git a/doc/admin-guide/plugins/cache_range_requests.en.rst
b/doc/admin-guide/plugins/cache_range_requests.en.rst
index 5d33f7c..6d11d21 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 13381d2..41a7702 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<char *>("ps-cachekey"), no_argument, nullptr, 'p'},
{const_cast<char *>("consider-ims"), no_argument, nullptr, 'c'},
+ {const_cast<char *>("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_ims.test.py
b/tests/gold_tests/pluginTest/cache_range_requests/cache_range_requests_cachekey.test.py
similarity index 53%
copy from
tests/gold_tests/pluginTest/cache_range_requests/cache_range_requests_ims.test.py
copy to
tests/gold_tests/pluginTest/cache_range_requests/cache_range_requests_cachekey.test.py
index f249700..2622694 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_cachekey.test.py
@@ -20,7 +20,7 @@ import os
import time
Test.Summary = '''
-cache_range_requests X-CRR-IMS plugin test
+cache_range_requests with cachekey
'''
## Test description:
@@ -30,16 +30,17 @@ cache_range_requests X-CRR-IMS plugin test
Test.SkipUnless(
Condition.PluginExists('cache_range_requests.so'),
+ Condition.PluginExists('cachekey.so'),
Condition.PluginExists('xdebug.so'),
)
Test.ContinueOnFail = False
-Test.testName = "cache_range_requests"
+Test.testName = "cache_range_requests_cachekey"
-# Define and configure ATS
+# 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")
+server = Test.MakeOriginServer("server", lookup_key="{%uuid}")
# default root
req_chk = {"headers":
@@ -64,11 +65,12 @@ 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" +
- "Range: bytes=0-\r\n" +
+ "uuid: full\r\n" +
"\r\n",
"timestamp": "1469733493.993",
"body": ""
@@ -77,11 +79,9 @@ req_full = {"headers":
res_full = {"headers":
"HTTP/1.1 206 Partial Content\r\n" +
"Accept-Ranges: bytes\r\n" +
- "Cache-Control: max-age=500\r\n" +
- "Content-Range: bytes 0-{0}/{0}\r\n".format(bodylen) +
+ 'Etag: "foo"\r\n' +
+ "Cache-Control: public, max-age=500\r\n" +
"Connection: close\r\n" +
- 'Etag: "772102f4-56f4bc1e6d417"\r\n' +
- "Last-Modified: Sat, 23 Jun 2018 09:27:29 GMT\r\n" +
"\r\n",
"timestamp": "1469733493.993",
"body": body
@@ -89,10 +89,70 @@ res_full = {"headers":
server.addResponse("sessionlog.json", req_full, res_full)
-# cache range requests plugin remap
+# 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=cache_range_requests.so @pparam=--consider-ims',
+ ' @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
@@ -108,34 +168,31 @@ ts.Disk.records_config.update({
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 whole asset into cache
-tr = Test.AddTestRun("0- range cache load")
+# 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 -r 0-'
+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
-
-# set up the IMS date field (go in the future) RFC 2616
-futuretime = time.time() + 100 # seconds
-futurestr = time.strftime("%a, %d %b %Y %H:%M:%S GMT", time.gmtime(futuretime))
-
-# test inner range
-# 1 Test - Fetch range into cache
-tr = Test.AddTestRun("0- cache hit check")
+# 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-'.format(futurestr)
+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: hit",
"expected cache hit")
+ps.Streams.stdout.Content = Testers.ContainsExpression("X-Cache: miss",
"expected cache miss for load")
tr.StillRunningAfter = ts
-# 2 Test - Ensure X-CRR-IMS header results in hit-stale
-tr = Test.AddTestRun("0- range X-CRR-IMS check")
+# 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.example.com/path -r 0- -H
"X-CRR-IMS: {}"'.format(futurestr)
+ps.Command = curl_and_args + ' http://www.fail.com/path -r 0- -H "uuid:
range_fail"'
ps.ReturnCode = 0
-ps.Streams.stdout.Content = Testers.ContainsExpression("X-Cache: hit-stale",
"expected cache hit-stale")
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 f249700..86a7ce9 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 @@ Test.SkipUnless(
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")
