From 6a2a3c199bcc7bcfcc5fe6856305e4f0e90436ca Mon Sep 17 00:00:00 2001 From: Dylan Souza Date: Tue, 15 Jan 2019 18:50:55 +0000 Subject: [PATCH] Strip token from upstream if conifigured and dynamically allocate string buffers Adds a configuration option to strip uri signing tokens from both the cache key URL and the upstream URL. Additionally it was pointed out that some statically allocated buffers were too small in some of the string manipulating functions (normalize and strip token). These buffers are now dynamically allocated since the maximum buffer size is known for these. --- plugins/experimental/uri_signing/README.md | 27 +++--- plugins/experimental/uri_signing/common.h | 4 + plugins/experimental/uri_signing/config.c | 16 +++- plugins/experimental/uri_signing/config.h | 1 + plugins/experimental/uri_signing/cookie.c | 1 - plugins/experimental/uri_signing/jwt.c | 21 +++-- plugins/experimental/uri_signing/match.c | 1 - plugins/experimental/uri_signing/parse.c | 1 - .../unit_tests/uri_signing_test.cc | 12 ++- .../experimental/uri_signing/uri_signing.c | 83 ++++++++++++++++--- 10 files changed, 130 insertions(+), 37 deletions(-) diff --git a/plugins/experimental/uri_signing/README.md b/plugins/experimental/uri_signing/README.md index 8180586bf9a..398b9869958 100644 --- a/plugins/experimental/uri_signing/README.md +++ b/plugins/experimental/uri_signing/README.md @@ -1,8 +1,7 @@ URI Signing Plugin ================== -This remap plugin implements the draft URI Signing protocol documented here: -https://tools.ietf.org/html/draft-ietf-cdni-uri-signing-16 . +This remap plugin implements the draft URI Signing protocol documented [here](https://tools.ietf.org/html/draft-ietf-cdni-uri-signing-16): It takes a single argument: the name of a config file that contains key information. @@ -77,16 +76,25 @@ It's worth noting that multiple issuers can provide `auth_directives`. Each issuer will be processed in order and any issuer can provide access to a path. -### Token Stripping +### More Configuration Options -When The boolean strip_token parameter is set to true, the plugin removes the +**Strip Token** +When the strip_token parameter is set to true, the plugin removes the token from both the url that is sent upstream to the origin and the url that -is used as the cache key. It can be set like this: +is used as the cache key. The strip_token parameter defaults to false and should +be set by only one issuer. +**ID** +The id field takes a string indicating the identification of the entity processing the request. +This is used in aud claim checks to ensure that the receiver is the intended audience of a +tokenized request. The id parameter can only be set by one issuer. + +Example: { "Kabletown URI Authority": { "renewal_kid": "Second Key", "strip_token" : true, + "id" : "mycdn", "auth_directives": [ ⋮ ] @@ -95,8 +103,6 @@ is used as the cache key. It can be set like this: ] } -The strip_token parameter defaults to false and should be set by only one issuer. - Usage ----- @@ -107,10 +113,9 @@ will receive a 403 Forbidden response, instead of receiving content. Tokens will be found in either of these places: - A query parameter named `URISigningPackage`. The value must be the JWT. + - A path parameter named `URISigningPackage`. The value must be the JWT. - A cookie named `URISigningPackage`. The value of the cookie must be the JWT. -Path parameters will not be searched for JWTs. - ### Supported Claims The following claims are understood: @@ -118,6 +123,8 @@ The following claims are understood: - `iss`: Must be present. The issuer is used to locate the key for verification. - `sub`: May be present, but is not validated. - `exp`: Expired tokens are not valid. + - `nbf`: Tokens processed before this time are not valid. + - `aud`: Token aud claim strings must match the configured id to be considered valid. - `iat`: May be present, but is not validated. - `cdniv`: Must be missing or 1. - `cdniuc`: Validated last, after key verificationD. **Only `regex` is supported!** @@ -129,8 +136,6 @@ The following claims are understood: These claims are not supported. If they are present, the token will not validate: - - `aud` - - `nbf` - `jti` - `cdnicrit` - `cdniip` diff --git a/plugins/experimental/uri_signing/common.h b/plugins/experimental/uri_signing/common.h index 10505fa8d8a..467d0cee257 100644 --- a/plugins/experimental/uri_signing/common.h +++ b/plugins/experimental/uri_signing/common.h @@ -16,6 +16,8 @@ * limitations under the License. */ +#pragma once + #define PLUGIN_NAME "uri_signing" #ifdef URI_SIGNING_UNIT_TEST @@ -24,6 +26,8 @@ #define PluginDebug(fmt, ...) PrintToStdErr("(%s) %s:%d:%s() " fmt "\n", PLUGIN_NAME, __FILE__, __LINE__, __func__, ##__VA_ARGS__) #define PluginError(fmt, ...) PrintToStdErr("(%s) %s:%d:%s() " fmt "\n", PLUGIN_NAME, __FILE__, __LINE__, __func__, ##__VA_ARGS__) +#define TSmalloc(x) malloc(x) +#define TSfree(p) free(p) void PrintToStdErr(const char *fmt, ...); #else diff --git a/plugins/experimental/uri_signing/config.c b/plugins/experimental/uri_signing/config.c index 96429148e90..cdf2f2db4ce 100644 --- a/plugins/experimental/uri_signing/config.c +++ b/plugins/experimental/uri_signing/config.c @@ -21,8 +21,6 @@ #include "timing.h" #include "jwt.h" -#include - #include #include @@ -46,6 +44,7 @@ struct config { struct signer signer; struct auth_directive *auth_directives; char *id; + bool strip_token; }; cjose_jwk_t ** @@ -87,6 +86,12 @@ config_get_id(struct config *cfg) return cfg->id; } +bool +config_strip_token(struct config *cfg) +{ + return cfg->strip_token; +} + struct config * config_new(size_t n) { @@ -114,6 +119,8 @@ config_new(size_t n) cfg->auth_directives = NULL; cfg->id = NULL; + cfg->strip_token = false; + PluginDebug("New config object created at %p", cfg); return cfg; } @@ -284,6 +291,11 @@ read_config(const char *path) } json_decref(id_json); + json_t *strip_json = json_object_get(jwks, "strip_token"); + if (strip_json) { + cfg->strip_token = json_boolean_value(strip_json); + } + size_t jwks_ct = json_array_size(key_ary); cjose_jwk_t **jwks = (*jwkis++ = malloc((jwks_ct + 1) * sizeof *jwks)); PluginDebug("Created table with size %d", cfg->issuers->size); diff --git a/plugins/experimental/uri_signing/config.h b/plugins/experimental/uri_signing/config.h index a22ec5d273d..87688374c85 100644 --- a/plugins/experimental/uri_signing/config.h +++ b/plugins/experimental/uri_signing/config.h @@ -34,3 +34,4 @@ struct _cjose_jwk_int **find_keys(struct config *cfg, const char *issuer); struct _cjose_jwk_int *find_key_by_kid(struct config *cfg, const char *issuer, const char *kid); bool uri_matches_auth_directive(struct config *cfg, const char *uri, size_t uri_ct); const char *config_get_id(struct config *cfg); +bool config_strip_token(struct config *cfg); diff --git a/plugins/experimental/uri_signing/cookie.c b/plugins/experimental/uri_signing/cookie.c index 45a2e43b961..1e9fc7f9a30 100644 --- a/plugins/experimental/uri_signing/cookie.c +++ b/plugins/experimental/uri_signing/cookie.c @@ -18,7 +18,6 @@ #include "cookie.h" #include "common.h" -#include #include const char * diff --git a/plugins/experimental/uri_signing/jwt.c b/plugins/experimental/uri_signing/jwt.c index 020d0cac0f8..f14ecb6e289 100644 --- a/plugins/experimental/uri_signing/jwt.c +++ b/plugins/experimental/uri_signing/jwt.c @@ -20,7 +20,6 @@ #include "jwt.h" #include "match.h" #include "normalize.h" -#include "ts/ts.h" #include #include #include @@ -206,13 +205,13 @@ jwt_check_uri(const char *cdniuc, const char *uri) int uri_ct = strlen(uri); int buff_ct = uri_ct + 2; int err; - char normal_uri[buff_ct]; - + char *normal_uri = (char *)TSmalloc(buff_ct); memset(normal_uri, 0, buff_ct); + err = normalize_uri(uri, uri_ct, normal_uri, buff_ct); if (err) { - return false; + goto fail_jwt; } const char *kind = cdniuc, *container = cdniuc; @@ -220,27 +219,35 @@ jwt_check_uri(const char *cdniuc, const char *uri) ++container; } if (!*container) { - return false; + goto fail_jwt; } ++container; size_t len = container - kind; + bool status; PluginDebug("Comparing with match kind \"%.*s\" on \"%s\" to normalized URI \"%s\"", (int)len - 1, kind, container, normal_uri); switch (len) { case sizeof CONT_URI_HASH_STR: if (!strncmp(CONT_URI_HASH_STR, kind, len - 1)) { - return match_hash(container, normal_uri); + status = match_hash(container, normal_uri); + TSfree(normal_uri); + return status; } PluginDebug("Expected kind %s, but did not find it in \"%.*s\"", CONT_URI_HASH_STR, (int)len - 1, kind); break; case sizeof CONT_URI_REGEX_STR: if (!strncmp(CONT_URI_REGEX_STR, kind, len - 1)) { - return match_regex(container, normal_uri); + status = match_regex(container, normal_uri); + TSfree(normal_uri); + return status; } PluginDebug("Expected kind %s, but did not find it in \"%.*s\"", CONT_URI_REGEX_STR, (int)len - 1, kind); break; } PluginDebug("Unknown match kind \"%.*s\"", (int)len - 1, kind); + +fail_jwt: + TSfree(normal_uri); return false; } diff --git a/plugins/experimental/uri_signing/match.c b/plugins/experimental/uri_signing/match.c index 40d4d472e93..faea8953dfc 100644 --- a/plugins/experimental/uri_signing/match.c +++ b/plugins/experimental/uri_signing/match.c @@ -18,7 +18,6 @@ #include #include "common.h" -#include "ts/ts.h" #include #include diff --git a/plugins/experimental/uri_signing/parse.c b/plugins/experimental/uri_signing/parse.c index 43667659922..3ca10b21d23 100644 --- a/plugins/experimental/uri_signing/parse.c +++ b/plugins/experimental/uri_signing/parse.c @@ -25,7 +25,6 @@ #include #include #include -#include #include cjose_jws_t * diff --git a/plugins/experimental/uri_signing/unit_tests/uri_signing_test.cc b/plugins/experimental/uri_signing/unit_tests/uri_signing_test.cc index fe938160ccf..522db9999fd 100644 --- a/plugins/experimental/uri_signing/unit_tests/uri_signing_test.cc +++ b/plugins/experimental/uri_signing/unit_tests/uri_signing_test.cc @@ -58,19 +58,22 @@ normalize_uri_helper(const char *uri, const char *expected_normal) size_t uri_ct = strlen(uri); int buff_size = uri_ct + 2; int err; - char uri_normal[buff_size]; + char *uri_normal = (char *)malloc(buff_size); memset(uri_normal, 0, buff_size); err = normalize_uri(uri, uri_ct, uri_normal, buff_size); if (err) { + free(uri_normal); return false; } if (expected_normal && strcmp(expected_normal, uri_normal) == 0) { + free(uri_normal); return true; } + free(uri_normal); return false; } @@ -101,8 +104,10 @@ jws_parsing_helper(const char *uri, const char *paramName, const char *expected_ bool resp; size_t uri_ct = strlen(uri); size_t strip_ct = 0; - char uri_strip[uri_ct + 1]; - memset(uri_strip, 0, sizeof uri_strip); + + char *uri_strip = (char *)malloc(uri_ct + 1); + memset(uri_strip, 0, uri_ct + 1); + cjose_jws_t *jws = get_jws_from_uri(uri, uri_ct, paramName, uri_strip, uri_ct, &strip_ct); if (jws) { resp = true; @@ -114,6 +119,7 @@ jws_parsing_helper(const char *uri, const char *paramName, const char *expected_ resp = false; } cjose_jws_release(jws); + free(uri_strip); return resp; } diff --git a/plugins/experimental/uri_signing/uri_signing.c b/plugins/experimental/uri_signing/uri_signing.c index fadd269e540..f85a87b5ed3 100644 --- a/plugins/experimental/uri_signing/uri_signing.c +++ b/plugins/experimental/uri_signing/uri_signing.c @@ -22,10 +22,10 @@ #include "jwt.h" #include "timing.h" -#include #include #include +#include #include #include @@ -157,6 +157,8 @@ TSRemapDoRemap(void *ih, TSHttpTxn txnp, TSRemapRequestInfo *rri) int cpi = 0; int url_ct = 0; const char *url = NULL; + char *strip_uri = NULL; + TSRemapStatus status = TSREMAP_NO_REMAP; const char *package = "URISigningPackage"; @@ -176,9 +178,12 @@ TSRemapDoRemap(void *ih, TSHttpTxn txnp, TSRemapRequestInfo *rri) checkpoints[cpi++] = mark_timer(&t); } - char strip_uri[2000] = {0}; + int strip_size = url_ct + 1; + strip_uri = (char *)TSmalloc(strip_size); + memset(strip_uri, 0, strip_size); + size_t strip_ct; - cjose_jws_t *jws = get_jws_from_uri(url, url_ct, package, strip_uri, 2000, &strip_ct); + cjose_jws_t *jws = get_jws_from_uri(url, url_ct, package, strip_uri, strip_size, &strip_ct); if (cpi < max_cpi) { checkpoints[cpi++] = mark_timer(&t); @@ -186,6 +191,9 @@ TSRemapDoRemap(void *ih, TSHttpTxn txnp, TSRemapRequestInfo *rri) int checked_cookies = 0; if (!jws) { check_cookies: + /* There is no valid token in the url */ + strncpy(strip_uri, url, url_ct); + strip_ct = url_ct; ++checked_cookies; TSMLoc field; @@ -218,6 +226,55 @@ TSRemapDoRemap(void *ih, TSHttpTxn txnp, TSRemapRequestInfo *rri) checkpoints[cpi++] = mark_timer(&t); } jws = get_jws_from_cookie(&client_cookie, &client_cookie_sz_ct, package); + } else { + /* There has been a JWS found in the url */ + /* Strip the token from the URL for upstream if configured to do so */ + if (config_strip_token((struct config *)ih)) { + if ((int)strip_ct != url_ct) { + int map_url_ct = 0; + char *map_url = NULL; + char *map_strip_uri = NULL; + map_url = TSUrlStringGet(rri->requestBufp, rri->requestUrl, &map_url_ct); + + PluginDebug("Stripping Token from requestUrl: %s", map_url); + + int map_strip_size = map_url_ct + 1; + map_strip_uri = (char *)TSmalloc(map_strip_size); + memset(map_strip_uri, 0, map_strip_size); + size_t map_strip_ct = 0; + + cjose_jws_t *map_jws = get_jws_from_uri(map_url, map_url_ct, package, map_strip_uri, map_strip_size, &map_strip_ct); + cjose_jws_release(map_jws); + + char *strip_uri_start = &map_strip_uri[0]; + char *strip_uri_end = &map_strip_uri[map_strip_ct]; + PluginDebug("Stripping token from upstream url to: %s", strip_uri_start); + + TSParseResult parse_rc = TSUrlParse(rri->requestBufp, rri->requestUrl, (const char **)&strip_uri_start, strip_uri_end); + if (map_url != NULL) { + TSfree(map_url); + } + if (map_strip_uri != NULL) { + TSfree(map_strip_uri); + } + + if (parse_rc != TS_PARSE_DONE) { + PluginDebug("Error in TSUrlParse"); + goto fail; + } + status = TSREMAP_DID_REMAP; + } + } + } + /* Check auth_dir and pass through if configured */ + if (uri_matches_auth_directive((struct config *)ih, url, url_ct)) { + if (url != NULL) { + TSfree((void *)url); + } + if (strip_uri != NULL) { + TSfree(strip_uri); + } + return TSREMAP_NO_REMAP; } if (!jws) { goto fail; @@ -226,8 +283,10 @@ TSRemapDoRemap(void *ih, TSHttpTxn txnp, TSRemapRequestInfo *rri) if (cpi < max_cpi) { checkpoints[cpi++] = mark_timer(&t); } + struct jwt *jwt = validate_jws(jws, (struct config *)ih, strip_uri, strip_ct); cjose_jws_release(jws); + if (cpi < max_cpi) { checkpoints[cpi++] = mark_timer(&t); } @@ -239,6 +298,8 @@ TSRemapDoRemap(void *ih, TSHttpTxn txnp, TSRemapRequestInfo *rri) } } + /* There has been a validated JWT found in either the cookie or url */ + struct signer *signer = config_signer((struct config *)ih); char *cookie = renew(jwt, signer->issuer, signer->jwk, signer->alg, package); jwt_delete(jwt); @@ -260,16 +321,13 @@ TSRemapDoRemap(void *ih, TSHttpTxn txnp, TSRemapRequestInfo *rri) last_mark = checkpoints[i]; } PluginDebug("Spent %" PRId64 " ns uri_signing verification of %.*s.", mark_timer(&t), url_ct, url); + TSfree((void *)url); - return TSREMAP_NO_REMAP; -fail: - if (uri_matches_auth_directive((struct config *)ih, url, url_ct)) { - if (url != NULL) { - TSfree((void *)url); - } - return TSREMAP_NO_REMAP; + if (strip_uri != NULL) { + TSfree(strip_uri); } - + return status; +fail: PluginDebug("Invalid JWT for %.*s", url_ct, url); TSHttpTxnStatusSet(txnp, TS_HTTP_STATUS_FORBIDDEN); PluginDebug("Spent %" PRId64 " ns uri_signing verification of %.*s.", mark_timer(&t), url_ct, url); @@ -277,6 +335,9 @@ TSRemapDoRemap(void *ih, TSHttpTxn txnp, TSRemapRequestInfo *rri) if (url != NULL) { TSfree((void *)url); } + if (strip_uri != NULL) { + TSfree(strip_uri); + } return TSREMAP_DID_REMAP; }