Add simple autest and subsequent fixes

traeak · jrushford · commit ea3aa04b88bb · 2020-01-10T10:49:03.000-07:00
diff --git a/plugins/experimental/uri_signing/common.h b/plugins/experimental/uri_signing/common.h
@@ -33,7 +33,10 @@ void PrintToStdErr(const char *fmt, ...);
 #else
 
 #include "ts/ts.h"
-#define PluginDebug(...) TSDebug("uri_signing", PLUGIN_NAME " " __VA_ARGS__)
-#define PluginError(...) PluginDebug(__VA_ARGS__), TSError(PLUGIN_NAME " " __VA_ARGS__)
+#define __FILENAME__ (strrchr(__FILE__, '/') ? strrchr(__FILE__, '/') + 1 : __FILE__)
+#define PluginDebug(fmt, ...) TSDebug(PLUGIN_NAME, "[%s:% 4d] %s(): " fmt, __FILENAME__, __LINE__, __func__, ##__VA_ARGS__);
+#define PluginError(fmt, ...)      \
+  PluginDebug(fmt, ##__VA_ARGS__); \
+  TSError("[%s:% 4d] %s(): " fmt, __FILENAME__, __LINE__, __func__, ##__VA_ARGS__);
 
 #endif
diff --git a/plugins/experimental/uri_signing/config.c b/plugins/experimental/uri_signing/config.c
@@ -289,7 +289,6 @@ read_config(const char *path)
         PluginDebug("Found Id in the config: %s", cfg->id);
       }
     }
-    json_decref(id_json);
 
     json_t *strip_json = json_object_get(jwks, "strip_token");
     if (strip_json) {
diff --git a/plugins/experimental/uri_signing/parse.c b/plugins/experimental/uri_signing/parse.c
@@ -31,8 +31,8 @@ cjose_jws_t *
 get_jws_from_uri(const char *uri, size_t uri_ct, const char *paramName, char *strip_uri, size_t buff_ct, size_t *strip_ct)
 {
   /* Reserved characters as defined by the URI Generic Syntax RFC: https://tools.ietf.org/html/rfc3986#section-2.2 */
-  static const char *reserved_string  = ":/?#[]@!$&\'()*+,;=";
-  static const char *sub_delim_string = "!$&\'()*+,;=";
+  static char const *const reserved_string  = ":/?#[]@!$&\'()*+,;=";
+  static char const *const sub_delim_string = "!$&\'()*+,;=";
 
   /* If param name ends in reserved character this will be treated as the termination symbol when parsing for package. Default is
    * '='. */
diff --git a/plugins/experimental/uri_signing/parse.h b/plugins/experimental/uri_signing/parse.h
@@ -21,6 +21,8 @@
 #include <stdlib.h>
 
 struct _cjose_jws_int;
+
+/* For now strip_ct returns size of string *including* the null terminator */
 struct _cjose_jws_int *get_jws_from_uri(const char *uri, size_t uri_ct, const char *paramName, char *strip_uri, size_t buff_ct,
                                         size_t *strip_ct);
 struct _cjose_jws_int *get_jws_from_cookie(const char **cookie, size_t *cookie_ct, const char *paramName);
diff --git a/plugins/experimental/uri_signing/python_signer/uri_signer.py b/plugins/experimental/uri_signing/python_signer/uri_signer.py
@@ -8,7 +8,7 @@
 # "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
+#    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,
@@ -25,49 +25,107 @@
 from jose import jwt
 
 def main():
-    parser = argparse.ArgumentParser()
-    parser.add_argument('-c', '--config',
-                        help="Configuration File",
-                        required=True)
-    parser.add_argument('-u', '--uri',
-                        help="URI to sign",
-                        required=True)
-    args = parser.parse_args()
+  parser = argparse.ArgumentParser()
+  parser.add_argument('-c', '--config',
+            help="Configuration File",
+            required=True)
+  parser.add_argument('-u', '--uri',
+            help="URI to sign",
+            required=True)
 
-    with open(args.config, 'r') as f:
-        config = json.load(f)
+  # helpers
+  parser.add_argument('--key_index', type=int, nargs=1)
+  parser.add_argument('--token_lifetime', type=int, nargs=1)
 
-    keys = config["keys"]
+  # override arguments -- claims
+  parser.add_argument('--aud', nargs=1)
+  parser.add_argument('--cdniets', type=int, nargs=1)
+  parser.add_argument('--cdnistd', type=int, nargs=1)
+  parser.add_argument('--cdnistt', type=int, nargs=1)
+  parser.add_argument('--exp', type=int, nargs=1)
+  parser.add_argument('--iss', nargs=1)
 
-    # Randomly select a key
+  # override arguments -- key
+  parser.add_argument('--alg', nargs=1)
+  parser.add_argument('--k', nargs=1)
+  parser.add_argument('--kid', nargs=1)
+  parser.add_argument('--kty', nargs=1)
+
+  args = parser.parse_args()
+
+  with open(args.config, 'r') as f:
+    config = json.load(f)
+
+  keys = config["keys"]
+
+  # Select a key, either explicitly or randomly
+  key_index = 0
+  if args.key_index:
+    key_index = args.key_index[0]
+    print("args key_index " + str(key_index))
+  else:
     key_index = random.randint(0,len(keys)-1)
-    print("Using Key: " + str(keys[key_index]["kid"]) + " to sign URI.")
-    key = keys[key_index]
+    print("randomizing key index")
+
+  print("Using key_index " + str(key_index))
+
+  print("Using Key: " + str(keys[key_index]["kid"]) + " to sign URI.")
+  key = keys[key_index]
+
+  # Build Out claimset
+  claimset = {}
+  if "iss" in config.keys():
+    claimset["iss"] = config["iss"]
+
+  if "token_lifetime" in config.keys():
+    claimset["exp"] = int(time.time()) + config["token_lifetime"]
+  else:
+    claimset["exp"] = int(time.time()) + 30
+
+  if "aud" in config.keys():
+    claimset["aud"] = config["aud"]
+
+  if "cdnistt" in config.keys():
+    if config["cdnistt"]:
+      claimset["cdnistt"] = 1
+      if "cdniets" in config.keys():
+        claimset["cdniets"] = config["cdniets"]
+      else:
+        claimset["cdniets"] = 30
+
+
+  # process override args - simple
+  if args.iss:
+    claimset["iss"] = args.iss[0]
+  if args.exp:
+    claimset["exp"] = args.exp[0]
+  if args.aud:
+    claimset["aud"] = args.aud[0]
 
-    # Build Out claimset
-    claimset = {}
-    if ("iss" in config.keys()):
-        claimset["iss"] = config["iss"]
+  # process override args - complex
+  if args.cdnistt:
+    claimset["cdnistt"] = args.cdnistt[0]
 
-    if ("token_lifetime" in config.keys()):
-        claimset["exp"] = int(time.time()) + config["token_lifetime"]
-    else:
-        claimset["exp"] = int(time.time()) + 30
+  if "cdnistt" in config.keys():
+    if args.cdniets:
+      claimset["cdniets"] = arg.cdniets[0]
 
-    if("aud" in config.keys()):
-        claimset["aud"] = config["aud"]
+  # specific key overrides
+  if args.alg:
+    key["alg"] = args.alg[0]
+  if args.kid:
+    key["kid"] = args.kid[0]
+  if args.kty:
+    key["kty"] = args.kty[0]
+  if args.k:
+    key["k"] = args.k[0]
 
-    if("cdnistt"  in config.keys()):
-        if config["cdnistt"]:
-            claimset["cdnistt"] = 1
-            if("cdniets" in config.keys()):
-                claimset["cdniets"] = config["cdniets"]
-            else:
-                claimset["cdniets"] = 30
+  print(claimset)
+  print(key)
 
-    Token = jwt.encode(claimset,key,algorithm=key["alg"])
+  Token = jwt.encode(claimset,key,algorithm=key["alg"])
 
-    print("Signed URL: " + args.uri + "?urisigning=" + Token)
+  print("Signed URL: " + args.uri + "?URISigningPackage=" + Token)
 
 if __name__ == "__main__":
-     main()
+   main()
diff --git a/plugins/experimental/uri_signing/uri_signing.c b/plugins/experimental/uri_signing/uri_signing.c
@@ -62,10 +62,23 @@ TSRemapNewInstance(int argc, char *argv[], void **ih, char *errbuf, int errbuf_s
 
   TSDebug(PLUGIN_NAME, "Initializing remap function of %s -> %s with config from %s", argv[0], argv[1], argv[2]);
 
-  const char *install_dir = TSInstallDirGet();
-  size_t config_file_ct   = snprintf(NULL, 0, "%s/%s/%s", install_dir, "etc/trafficserver", argv[2]);
-  char *config_file       = malloc(config_file_ct + 1);
-  (void)snprintf(config_file, config_file_ct + 1, "%s/%s/%s", install_dir, "etc/trafficserver", argv[2]);
+  char const *const fname = argv[2];
+
+  if (0 == strlen(fname)) {
+    snprintf(errbuf, errbuf_size, "[TSRemapNewKeyInstance] - Invalid config file name for %s -> %s", argv[0], argv[1]);
+    return TS_ERROR;
+  }
+
+  char *config_file = NULL;
+  if ('/' == fname[0]) {
+    config_file = strdup(fname);
+  } else {
+    char const *const config_dir = TSConfigDirGet();
+    size_t const config_file_ct  = snprintf(NULL, 0, "%s/%s", config_dir, fname);
+    config_file                  = malloc(config_file_ct + 1);
+    (void)snprintf(config_file, config_file_ct + 1, "%s/%s", config_dir, fname);
+  }
+
   TSDebug(PLUGIN_NAME, "config file name: %s", config_file);
   struct config *cfg = read_config(config_file);
   if (!cfg) {
@@ -159,8 +172,9 @@ TSRemapDoRemap(void *ih, TSHttpTxn txnp, TSRemapRequestInfo *rri)
   const char *url         = NULL;
   char *strip_uri         = NULL;
   TSRemapStatus status    = TSREMAP_NO_REMAP;
+  bool checked_auth       = false;
 
-  const char *package = "URISigningPackage";
+  static char const *const package = "URISigningPackage";
 
   TSMBuffer mbuf;
   TSMLoc ul;
@@ -207,7 +221,11 @@ TSRemapDoRemap(void *ih, TSHttpTxn txnp, TSRemapRequestInfo *rri)
     field = TSMimeHdrFieldFind(buffer, hdr, "Cookie", 6);
     if (field == TS_NULL_MLOC) {
       TSHandleMLocRelease(buffer, TS_NULL_MLOC, hdr);
-      goto fail;
+      if (!checked_auth) {
+        goto check_auth;
+      } else {
+        goto fail;
+      }
     }
 
     const char *client_cookie;
@@ -218,7 +236,11 @@ TSRemapDoRemap(void *ih, TSHttpTxn txnp, TSRemapRequestInfo *rri)
     TSHandleMLocRelease(buffer, TS_NULL_MLOC, hdr);
 
     if (!client_cookie || !client_cookie_ct) {
-      goto fail;
+      if (!checked_auth) {
+        goto check_auth;
+      } else {
+        goto fail;
+      }
     }
     size_t client_cookie_sz_ct = client_cookie_ct;
   check_more_cookies:
@@ -246,11 +268,15 @@ TSRemapDoRemap(void *ih, TSHttpTxn txnp, TSRemapRequestInfo *rri)
         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);
+        char const *strip_uri_start = map_strip_uri;
 
-        TSParseResult parse_rc = TSUrlParse(rri->requestBufp, rri->requestUrl, (const char **)&strip_uri_start, strip_uri_end);
+        /* map_strip_uri is null terminated */
+        size_t const mlen         = strlen(strip_uri_start);
+        char const *strip_uri_end = strip_uri_start + mlen;
+
+        PluginDebug("Stripping token from upstream url to: %.*s", (int)mlen, strip_uri_start);
+
+        TSParseResult parse_rc = TSUrlParse(rri->requestBufp, rri->requestUrl, &strip_uri_start, strip_uri_end);
         if (map_url != NULL) {
           TSfree(map_url);
         }
@@ -266,8 +292,10 @@ TSRemapDoRemap(void *ih, TSHttpTxn txnp, TSRemapRequestInfo *rri)
       }
     }
   }
+check_auth:
   /* Check auth_dir and pass through if configured */
   if (uri_matches_auth_directive((struct config *)ih, url, url_ct)) {
+    PluginDebug("Auth directive matched for %.*s", url_ct, url);
     if (url != NULL) {
       TSfree((void *)url);
     }
@@ -276,6 +304,8 @@ TSRemapDoRemap(void *ih, TSHttpTxn txnp, TSRemapRequestInfo *rri)
     }
     return TSREMAP_NO_REMAP;
   }
+  checked_auth = true;
+
   if (!jws) {
     goto fail;
   }
diff --git a/tests/Pipfile b/tests/Pipfile
@@ -34,6 +34,7 @@ gunicorn = "*"
 httpbin = "*"
 microserver = ">=1.0.4"
 jsonschema = "*"
+python-jose = "*"
 
 [requires]
 python_version = "3"
diff --git a/tests/gold_tests/pluginTest/uri_signing/config.json b/tests/gold_tests/pluginTest/uri_signing/config.json
@@ -0,0 +1,27 @@
+{
+ "issuer": {
+  "id": "issuer",
+  "renewal_kid": "1",
+  "strip_token": true,
+  "auth_directives": [
+   {
+    "auth": "allow",
+    "uri": "regex:.*crossdomain.xml"
+   }
+  ],
+  "keys": [
+   {
+    "alg": "HS256",
+    "k": "SECRET00",
+    "kid": "0",
+    "kty": "oct"
+   },
+   {
+    "alg": "HS256",
+    "k": "SECRET01",
+    "kid": "1",
+    "kty": "oct"
+   }
+  ]
+ }
+}
diff --git a/tests/gold_tests/pluginTest/uri_signing/gold/200.gold b/tests/gold_tests/pluginTest/uri_signing/gold/200.gold
@@ -0,0 +1,13 @@
+``
+> GET `` HTTP/1.1
+> User-Agent: curl/``
+> Host: somehost``
+> Accept: */*
+> Proxy-Connection: ``
+``
+< HTTP/1.1 200 OK
+< Content-Length: ``
+< Date: ``
+< Proxy-Connection: ``
+< Server: ATS/``
+``
diff --git a/tests/gold_tests/pluginTest/uri_signing/gold/403.gold b/tests/gold_tests/pluginTest/uri_signing/gold/403.gold
@@ -0,0 +1,16 @@
+``
+> GET `` HTTP/1.1
+> User-Agent: curl/``
+> Host: somehost``
+> Accept: */*
+> Proxy-Connection: ``
+``
+< HTTP/1.1 403 Forbidden
+< Date: ``
+< Proxy-Connection: ``
+< Server: ATS/``
+< Cache-Control: no-store
+< Content-Type: text/html
+< Content-Language: en
+< Content-Length: ``
+``
diff --git a/tests/gold_tests/pluginTest/uri_signing/run_sign.sh b/tests/gold_tests/pluginTest/uri_signing/run_sign.sh
diff --git a/tests/gold_tests/pluginTest/uri_signing/signer.json b/tests/gold_tests/pluginTest/uri_signing/signer.json
diff --git a/tests/gold_tests/pluginTest/uri_signing/uri_signing.test.py b/tests/gold_tests/pluginTest/uri_signing/uri_signing.test.py

Original file line number	Diff line number	Diff line change
`@@ -289,7 +289,6 @@ read_config(const char *path)`
`289`	`289`	`PluginDebug("Found Id in the config: %s", cfg->id);`
`290`	`290`	`}`
`291`	`291`	`}`
`292`		`- json_decref(id_json);`
`293`	`292`
`294`	`293`	`json_t *strip_json = json_object_get(jwks, "strip_token");`
`295`	`294`	`if (strip_json) {`