From 90c91c1c86fce51515a988f202fa275599e2001c Mon Sep 17 00:00:00 2001 From: Dylan Souza Date: Fri, 15 Feb 2019 22:45:25 +0000 Subject: [PATCH] Implement aud claim in Uri Signing Plugin The Aud claim is implemented as per the RFC version 16 that can be found here:https://tools.ietf.org/html/draft-ietf-cdni-uri-signing-16 As per the specification, the aud claim can be either a JSON array or a string. The aud claim is stored as raw json in the jwt class in this implementation. It is converted either to an array or a string at validation time. This commit also expands the unit tests quite a bit. Test configs can be provided in the unit_tests directory and parsed in the test framework. JWS validation is also testable now. This commit also fixes two memory leaks 1. Issuers were never being freed on configuration cleanup. 2. Token renewal allocates a tmp json_object without freeing. --- plugins/experimental/uri_signing/config.c | 25 +++ plugins/experimental/uri_signing/config.h | 1 + plugins/experimental/uri_signing/jwt.c | 57 +++++- plugins/experimental/uri_signing/jwt.h | 3 +- plugins/experimental/uri_signing/parse.c | 5 + .../uri_signing/unit_tests/testConfig.config | 102 ++++++++++ .../unit_tests/uri_signing_test.cc | 179 ++++++++++++++++++ 7 files changed, 364 insertions(+), 8 deletions(-) create mode 100644 plugins/experimental/uri_signing/unit_tests/testConfig.config diff --git a/plugins/experimental/uri_signing/config.c b/plugins/experimental/uri_signing/config.c index b52b94470b3..96429148e90 100644 --- a/plugins/experimental/uri_signing/config.c +++ b/plugins/experimental/uri_signing/config.c @@ -45,6 +45,7 @@ struct config { char **issuer_names; struct signer signer; struct auth_directive *auth_directives; + char *id; }; cjose_jwk_t ** @@ -80,6 +81,12 @@ find_key_by_kid(struct config *cfg, const char *issuer, const char *kid) return NULL; } +const char * +config_get_id(struct config *cfg) +{ + return cfg->id; +} + struct config * config_new(size_t n) { @@ -105,6 +112,7 @@ config_new(size_t n) cfg->signer.alg = NULL; cfg->auth_directives = NULL; + cfg->id = NULL; PluginDebug("New config object created at %p", cfg); return cfg; @@ -117,6 +125,7 @@ config_delete(struct config *cfg) return; } hdestroy_r(cfg->issuers); + free(cfg->issuers); for (cjose_jwk_t ***jwkis = cfg->jwkis; *jwkis; ++jwkis) { for (cjose_jwk_t **jwks = *jwkis; *jwks; ++jwks) { @@ -126,6 +135,10 @@ config_delete(struct config *cfg) } free(cfg->jwkis); + if (cfg->id) { + free(cfg->id); + } + for (char **name = cfg->issuer_names; *name; ++name) { free(*name); } @@ -259,6 +272,18 @@ read_config(const char *path) renewal_kid = json_string_value(renewal_kid_json); } + json_t *id_json = json_object_get(jwks, "id"); + const char *id; + if (id_json) { + id = json_string_value(id_json); + if (id) { + cfg->id = malloc(strlen(id) + 1); + strcpy(cfg->id, id); + PluginDebug("Found Id in the config: %s", cfg->id); + } + } + json_decref(id_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 75a82f24d88..a22ec5d273d 100644 --- a/plugins/experimental/uri_signing/config.h +++ b/plugins/experimental/uri_signing/config.h @@ -33,3 +33,4 @@ struct signer *config_signer(struct config *); 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); diff --git a/plugins/experimental/uri_signing/jwt.c b/plugins/experimental/uri_signing/jwt.c index a38565c5385..b6ad5ccf92a 100644 --- a/plugins/experimental/uri_signing/jwt.c +++ b/plugins/experimental/uri_signing/jwt.c @@ -56,7 +56,7 @@ parse_jwt(json_t *raw) jwt->raw = raw; jwt->iss = json_string_value(json_object_get(raw, "iss")); jwt->sub = json_string_value(json_object_get(raw, "sub")); - jwt->aud = json_string_value(json_object_get(raw, "aud")); + jwt->aud = json_object_get(raw, "aud"); jwt->exp = parse_number(json_object_get(raw, "exp")); jwt->nbf = parse_number(json_object_get(raw, "nbf")); jwt->iat = parse_number(json_object_get(raw, "iat")); @@ -77,6 +77,8 @@ jwt_delete(struct jwt *jwt) if (!jwt) { return; } + + json_decref(jwt->aud); json_decref(jwt->raw); free(jwt); } @@ -116,11 +118,6 @@ jwt_validate(struct jwt *jwt) return false; } - if (!unsupported_string_claim(jwt->aud)) { - PluginDebug("Initial JWT Failure: aud unsupported"); - return false; - } - if (now() > jwt->exp) { PluginDebug("Initial JWT Failure: expired token"); return false; @@ -159,6 +156,43 @@ jwt_validate(struct jwt *jwt) return true; } +bool +jwt_check_aud(json_t *aud, const char *id) +{ + if (!aud) { + return true; + } + if (!id) { + return false; + } + /* If aud is a string, do a simple string comparison */ + const char *aud_str = json_string_value(aud); + if (aud_str) { + PluginDebug("Checking aud %s agaisnt token aud string \"%s\"", id, aud_str); + /* Both strings will be null terminated per jansson docs */ + if (strcmp(aud_str, id) == 0) { + return true; + } + return false; + } + PluginDebug("Checking aud %s agaisnt token aud array", id); + /* If aud is an array, check all items */ + if (json_is_array(aud)) { + size_t index; + json_t *aud_item; + json_array_foreach(aud, index, aud_item) + { + aud_str = json_string_value(aud_item); + if (aud_str) { + if (strcmp(aud_str, id) == 0) { + return true; + } + } + } + } + return false; +} + bool jwt_check_uri(const char *cdniuc, const char *uri) { @@ -219,6 +253,14 @@ renew_copy_string(json_t *new_json, const char *name, const char *old) } } +void +renew_copy_raw(json_t *new_json, const char *name, json_t *old_json) +{ + if (old_json) { + json_object_set_new(new_json, name, old_json); + } +} + void renew_copy_real(json_t *new_json, const char *name, double old) { @@ -251,7 +293,7 @@ renew(struct jwt *jwt, const char *iss, cjose_jwk_t *jwk, const char *alg, const json_t *new_json = json_object(); renew_copy_string(new_json, "iss", iss); /* use issuer of new signing key */ renew_copy_string(new_json, "sub", jwt->sub); - renew_copy_string(new_json, "aud", jwt->aud); + renew_copy_raw(new_json, "aud", jwt->aud); renew_copy_real(new_json, "exp", now() + jwt->cdniets); /* expire ets seconds hence */ renew_copy_real(new_json, "nbf", jwt->nbf); renew_copy_real(new_json, "iat", now()); /* issued now */ @@ -261,6 +303,7 @@ renew(struct jwt *jwt, const char *iss, cjose_jwk_t *jwk, const char *alg, const renew_copy_integer(new_json, "cdnistt", jwt->cdnistt); char *pt = json_dumps(new_json, JSON_COMPACT); + json_decref(new_json); cjose_header_t *hdr = cjose_header_new(NULL); if (!hdr) { diff --git a/plugins/experimental/uri_signing/jwt.h b/plugins/experimental/uri_signing/jwt.h index 5e09f028e7c..95efbbc1da0 100644 --- a/plugins/experimental/uri_signing/jwt.h +++ b/plugins/experimental/uri_signing/jwt.h @@ -23,7 +23,7 @@ struct jwt { json_t *raw; const char *iss; const char *sub; - const char *aud; + json_t *aud; double exp; double nbf; double iat; @@ -39,6 +39,7 @@ struct jwt { struct jwt *parse_jwt(json_t *raw); void jwt_delete(struct jwt *jwt); bool jwt_validate(struct jwt *jwt); +bool jwt_check_aud(json_t *aud, const char *id); bool jwt_check_uri(const char *cdniuc, const char *uri); struct _cjose_jwk_int; diff --git a/plugins/experimental/uri_signing/parse.c b/plugins/experimental/uri_signing/parse.c index 099acdb315f..43667659922 100644 --- a/plugins/experimental/uri_signing/parse.c +++ b/plugins/experimental/uri_signing/parse.c @@ -229,6 +229,11 @@ validate_jws(cjose_jws_t *jws, struct config *cfg, const char *uri, size_t uri_c } } + if (!jwt_check_aud(jwt->aud, config_get_id(cfg))) { + PluginDebug("Valid key for %16p that does not match aud.", jws); + goto jwt_fail; + } + if (!jwt_check_uri(jwt->cdniuc, uri)) { PluginDebug("Valid key for %16p that does not match uri.", jws); goto jwt_fail; diff --git a/plugins/experimental/uri_signing/unit_tests/testConfig.config b/plugins/experimental/uri_signing/unit_tests/testConfig.config new file mode 100644 index 00000000000..aeecf363434 --- /dev/null +++ b/plugins/experimental/uri_signing/unit_tests/testConfig.config @@ -0,0 +1,102 @@ +{ + "Master Issuer": { + "renewal_kid": "6", + "id": "tester", + "auth_directives": [ + { + "auth": "allow", + "uri": "regex:invalid" + } + ], + "keys": [ + { + "alg": "HS256", + "k": "nxb7fyO5Z2hGz9E3oKm1357ptvC2su5QwQUb4YaIaIc", + "kid": "0", + "kty": "oct" + }, + { + "alg": "HS256", + "k": "cXKukBqFvQ0n3WAuRnWfExC14dmHdGoJULoZjGu9tJC", + "kid": "1", + "kty": "oct" + }, + { + "alg": "HS256", + "k": "38pJlSXfX87jWL0a03luml9QzUmM4qts1nmfIHA3B7r", + "kid": "2", + "kty": "oct" + }, + { + "alg": "HS256", + "k": "zNQPphknDGvzR5kA7IonXIDWKMyB1b8NpGmmDNlpgtM", + "kid": "3", + "kty": "oct" + }, + { + "alg": "HS256", + "k": "iB2ogCmQRt7r5hW7pgyP5FqiFcCl53MPQvfXv8wrZAn", + "kid": "4", + "kty": "oct" + }, + { + "alg": "HS256", + "k": "GJMCTyZhNoSOZvUOKmmY9MtGSLaONNLHqtKwsC3MWKo", + "kid": "5", + "kty": "oct" + }, + { + "alg": "HS256", + "k": "u2LziZKJFBnOfjUQUmvot7C9t91jj7ocJPIU9aDdbUl", + "kid": "6", + "kty": "oct" + }, + { + "alg": "HS256", + "k": "DRBKrBh87NYkH3UzfW1tWbiXCYXiYGZUE9w1orZngL0", + "kid": "7", + "kty": "oct" + }, + { + "alg": "HS256", + "k": "KNNKFbun8lEs7GbiKlo9mYGNdvpt33tdFzHbNnasDyP", + "kid": "8", + "kty": "oct" + }, + { + "alg": "HS256", + "k": "yb6kOddMUdupPRSkWMUdE6jrWT4MqUnVyTjpeJBYIqp", + "kid": "9", + "kty": "oct" + } + ] + }, + "Second Issuer": { + "keys": [ + { + "alg": "HS256", + "k": "testkey1", + "kid": "one", + "kty": "oct" + }, + { + "alg": "HS256", + "k": "testkey2", + "kid": "two", + "kty": "oct" + }, + { + "alg": "HS256", + "k": "testkey3", + "kid": "three", + "kty": "oct" + }, + { + "alg": "HS256", + "k": "testkey4", + "kid": "four", + "kty": "oct" + } + ] + } +} 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 f39758e097c..fe938160ccf 100644 --- a/plugins/experimental/uri_signing/unit_tests/uri_signing_test.cc +++ b/plugins/experimental/uri_signing/unit_tests/uri_signing_test.cc @@ -30,6 +30,7 @@ extern "C" { #include "../normalize.h" #include "../parse.h" #include "../match.h" +#include "../config.h" } bool @@ -475,4 +476,182 @@ TEST_CASE("5", "[RegexTests]") } SECTION("Alternation") { REQUIRE(match_regex("cat|dog", "dog")); } + fprintf(stderr, "\n"); +} + +TEST_CASE("6", "[AudTests]") +{ + INFO("TEST 6, Test Aud Matching"); + + json_error_t *err = NULL; + SECTION("Standard aud string match") + { + json_t *raw = json_loads("{\"aud\": \"tester\"}", 0, err); + json_t *aud = json_object_get(raw, "aud"); + REQUIRE(jwt_check_aud(aud, "tester")); + json_decref(aud); + json_decref(raw); + } + + SECTION("Standard aud array match") + { + json_t *raw = json_loads("{\"aud\": [ \"foo\", \"bar\", \"tester\"]}", 0, err); + json_t *aud = json_object_get(raw, "aud"); + REQUIRE(jwt_check_aud(aud, "tester")); + json_decref(aud); + json_decref(raw); + } + + SECTION("Standard aud string mismatch") + { + json_t *raw = json_loads("{\"aud\": \"foo\"}", 0, err); + json_t *aud = json_object_get(raw, "aud"); + REQUIRE(!jwt_check_aud(aud, "tester")); + json_decref(aud); + json_decref(raw); + } + + SECTION("Standard aud array mismatch") + { + json_t *raw = json_loads("{\"aud\": [\"foo\", \"bar\", \"foobar\"]}", 0, err); + json_t *aud = json_object_get(raw, "aud"); + REQUIRE(!jwt_check_aud(aud, "tester")); + json_decref(aud); + json_decref(raw); + } + + SECTION("Integer trying to pass as an aud") + { + json_t *raw = json_loads("{\"aud\": 1}", 0, err); + json_t *aud = json_object_get(raw, "aud"); + REQUIRE(!jwt_check_aud(aud, "tester")); + json_decref(aud); + json_decref(raw); + } + + SECTION("Integer mixed into a passing aud array") + { + json_t *raw = json_loads("{\"aud\": [1, \"foo\", \"bar\", \"tester\"]}", 0, err); + json_t *aud = json_object_get(raw, "aud"); + REQUIRE(jwt_check_aud(aud, "tester")); + json_decref(aud); + json_decref(raw); + } + + SECTION("Case sensitive test for single string") + { + json_t *raw = json_loads("{\"aud\": \"TESTer\"}", 0, err); + json_t *aud = json_object_get(raw, "aud"); + REQUIRE(!jwt_check_aud(aud, "tester")); + json_decref(aud); + json_decref(raw); + } + + SECTION("Case sensitive test for array") + { + json_t *raw = json_loads("{\"aud\": [1, \"foo\", \"bar\", \"Tester\"]}", 0, err); + json_t *aud = json_object_get(raw, "aud"); + REQUIRE(!jwt_check_aud(aud, "tester")); + json_decref(aud); + json_decref(raw); + } + + fprintf(stderr, "\n"); +} + +TEST_CASE("7", "[TestsConfig]") +{ + INFO("TEST 7, Config Loading and Config Functions"); + + SECTION("Config Loading ID Field") + { + struct config *cfg = read_config("experimental/uri_signing/unit_tests/testConfig.config"); + REQUIRE(cfg != NULL); + REQUIRE(strcmp(config_get_id(cfg), "tester") == 0); + config_delete(cfg); + } + fprintf(stderr, "\n"); +} + +bool +jws_validation_helper(const char *url, const char *package, struct config *cfg) +{ + size_t url_ct = strlen(url); + size_t strip_ct = 0; + char uri_strip[url_ct + 1]; + memset(uri_strip, 0, sizeof uri_strip); + cjose_jws_t *jws = get_jws_from_uri(url, url_ct, package, uri_strip, url_ct, &strip_ct); + if (!jws) { + return false; + } + struct jwt *jwt = validate_jws(jws, cfg, uri_strip, strip_ct); + if (jwt) { + jwt_delete(jwt); + cjose_jws_release(jws); + return true; + } + cjose_jws_release(jws); + return false; +} + +TEST_CASE("8", "[TestsWithConfig]") +{ + INFO("TEST 8, Tests Involving Validation with Config"); + struct config *cfg = read_config("experimental/uri_signing/unit_tests/testConfig.config"); + + SECTION("Validation of Valid Aud String in JWS") + { + REQUIRE(jws_validation_helper("http://www.foobar.com/" + "URISigningPackage=eyJLZXlJREtleSI6IjUiLCJhbGciOiJIUzI1NiJ9." + "eyJjZG5pZXRzIjozMCwiY2RuaXN0dCI6MSwiaXNzIjoiTWFzdGVyIElzc3VlciIsImF1ZCI6InRlc3RlciIsImNkbml1YyI6" + "InJlZ2V4Omh0dHA6Ly93d3cuZm9vYmFyLmNvbS8qIn0.InBxVm6OOAglNqc-U5wAZaRQVebJ9PK7Y9i7VFHWYHU", + "URISigningPackage", cfg)); + fprintf(stderr, "\n"); + } + + SECTION("Validation of Invalid Aud String in JWS") + { + REQUIRE(!jws_validation_helper("http://www.foobar.com/" + "URISigningPackage=eyJLZXlJREtleSI6IjUiLCJhbGciOiJIUzI1NiJ9." + "eyJjZG5pZXRzIjozMCwiY2RuaXN0dCI6MSwiaXNzIjoiTWFzdGVyIElzc3VlciIsImF1ZCI6ImJhZCIsImNkbml1YyI6InJ" + "lZ2V4Omh0dHA6Ly93d3cuZm9vYmFyLmNvbS8qIn0.aCOo8gOBa5G1RKkkzgWYwc79dPRw_fQUC0k1sWcjkyM", + "URISigningPackage", cfg)); + fprintf(stderr, "\n"); + } + + SECTION("Validation of Valid Aud Array in JWS") + { + REQUIRE(jws_validation_helper( + "http://www.foobar.com/" + "URISigningPackage=eyJLZXlJREtleSI6IjUiLCJhbGciOiJIUzI1NiJ9." + "eyJjZG5pZXRzIjozMCwiY2RuaXN0dCI6MSwiaXNzIjoiTWFzdGVyIElzc3VlciIsImF1ZCI6WyJiYWQiLCJpbnZhbGlkIiwidGVzdGVyIl0sImNkbml1YyI6InJl" + "Z2V4Omh0dHA6Ly93d3cuZm9vYmFyLmNvbS8qIn0.7lyepZMzc_odieKvOTN2U-k1gLwRKS8KJIvDFQXDqGs", + "URISigningPackage", cfg)); + fprintf(stderr, "\n"); + } + + SECTION("Validation of Invalid Aud Array in JWS") + { + REQUIRE(!jws_validation_helper( + "http://www.foobar.com/" + "URISigningPackage=eyJLZXlJREtleSI6IjUiLCJhbGciOiJIUzI1NiJ9." + "eyJjZG5pZXRzIjozMCwiY2RuaXN0dCI6MSwiaXNzIjoiTWFzdGVyIElzc3VlciIsImF1ZCI6WyJiYWQiLCJpbnZhbGlkIiwiZm9vYmFyIl0sImNkbml1YyI6InJl" + "Z2V4Omh0dHA6Ly93d3cuZm9vYmFyLmNvbS8qIn0.CU3WMJAPs0uRC7NKXvatVG9uU9SANdZzqO0GdQUatxk", + "URISigningPackage", cfg)); + fprintf(stderr, "\n"); + } + + SECTION("Validation of Valid Aud Array Mixed types in JWS") + { + REQUIRE(jws_validation_helper( + "http://www.foobar.com/" + "URISigningPackage=eyJLZXlJREtleSI6IjUiLCJhbGciOiJIUzI1NiJ9." + "eyJjZG5pZXRzIjozMCwiY2RuaXN0dCI6MSwiaXNzIjoiTWFzdGVyIElzc3VlciIsImF1ZCI6WyJiYWQiLDEsImZvb2JhciIsInRlc3RlciJdLCJjZG5pdWMiOiJy" + "ZWdleDpodHRwOi8vd3d3LmZvb2Jhci5jb20vKiJ9._vlXsA3r7RPje2ZdMnpaGTwIsdNMjuQWPEHRkGKTVL8", + "URISigningPackage", cfg)); + fprintf(stderr, "\n"); + } + + config_delete(cfg); + fprintf(stderr, "\n"); }