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 192dc83  Strip token from upstream if conifigured and dynamically 
allocate string buffers
192dc83 is described below

commit 192dc8300209ed17b0ff1c96aafba0f4096b27b2
Author: Dylan Souza <[email protected]>
AuthorDate: Tue Jan 15 18:50:55 2019 +0000

    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 -
 .../uri_signing/unit_tests/uri_signing_test.cc     | 12 +++-
 plugins/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 8180586..398b986 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 10505fa..467d0ce 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 9642914..cdf2f2d 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 <ts/ts.h>
-
 #include <cjose/cjose.h>
 #include <jansson.h>
 
@@ -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 a22ec5d..8768837 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 45a2e43..1e9fc7f 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 <ts/ts.h>
 #include <string.h>
 
 const char *
diff --git a/plugins/experimental/uri_signing/jwt.c 
b/plugins/experimental/uri_signing/jwt.c
index 020d0ca..f14ecb6 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 <jansson.h>
 #include <cjose/cjose.h>
 #include <math.h>
@@ -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 40d4d47..faea895 100644
--- a/plugins/experimental/uri_signing/match.c
+++ b/plugins/experimental/uri_signing/match.c
@@ -18,7 +18,6 @@
 
 #include <regex.h>
 #include "common.h"
-#include "ts/ts.h"
 #include <stdbool.h>
 #include <string.h>
 
diff --git a/plugins/experimental/uri_signing/parse.c 
b/plugins/experimental/uri_signing/parse.c
index 4366765..3ca10b2 100644
--- a/plugins/experimental/uri_signing/parse.c
+++ b/plugins/experimental/uri_signing/parse.c
@@ -25,7 +25,6 @@
 #include <cjose/cjose.h>
 #include <jansson.h>
 #include <string.h>
-#include <ts/ts.h>
 #include <inttypes.h>
 
 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 fe93816..522db99 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 fadd269..f85a87b 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 <ts/ts.h>
 #include <ts/remap.h>
 
 #include <stdio.h>
+#include <stdlib.h>
 #include <string.h>
 #include <inttypes.h>
 
@@ -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 @@ fail:
   if (url != NULL) {
     TSfree((void *)url);
   }
+  if (strip_uri != NULL) {
+    TSfree(strip_uri);
+  }
 
   return TSREMAP_DID_REMAP;
 }

Reply via email to