From 9d9907ef5e919744b93c5478b591cc46930b619d Mon Sep 17 00:00:00 2001 From: Alan Jowett Date: Tue, 14 Apr 2020 10:10:19 -0600 Subject: [PATCH] Attempt to download updated JWKs if the client presents an unknown key. Limit the download of keys to every 900s. Resolves: #284 Signed-off-by: Alan Jowett --- src/auth/openidconnect.c | 55 ++++++++++++++++++++++++++++++--- tests/generate_oidc_test_data.c | 10 ++++-- tests/test-oidc | 20 ++++++++++++ 3 files changed, 78 insertions(+), 7 deletions(-) diff --git a/src/auth/openidconnect.c b/src/auth/openidconnect.c index d9ea61fb..ba474272 100644 --- a/src/auth/openidconnect.c +++ b/src/auth/openidconnect.c @@ -35,9 +35,14 @@ #include #include +#define MINIMUM_KEY_REFRESH_INTERVAL (900) + typedef struct oidc_vctx_st { json_t *config; json_t *jwks; + void * pool; + int minimum_jwk_refresh_time; + time_t last_jwks_load_time; } oidc_vctx_st; typedef struct oidc_ctx_st { @@ -46,7 +51,7 @@ typedef struct oidc_ctx_st { int token_verified; } oidc_ctx_st; -static bool oidc_fetch_oidc_keys(void * pool, oidc_vctx_st * vctx); +static bool oidc_fetch_oidc_keys(oidc_vctx_st * vctx); static bool oidc_verify_token(oidc_vctx_st * vctx, const char *token, size_t token_length, char user_name[MAX_USERNAME_SIZE]); @@ -64,6 +69,7 @@ static void oidc_vhost_init(void **vctx, void *pool, void *additional) } vc->config = NULL; vc->jwks = NULL; + vc->pool = pool; if (config == NULL) { syslog(LOG_ERR, "ocserv-oidc: no configuration passed!\n"); @@ -94,7 +100,13 @@ static void oidc_vhost_init(void **vctx, void *pool, void *additional) exit(1); } - if (!oidc_fetch_oidc_keys(pool, vc)) { + if (json_object_get(vc->config, "minimum_jwk_refresh_time")) { + vc->minimum_jwk_refresh_time = json_integer_value(json_object_get(vc->config, "minimum_jwk_refresh_time")); + } else { + vc->minimum_jwk_refresh_time = MINIMUM_KEY_REFRESH_INTERVAL; + } + + if (!oidc_fetch_oidc_keys(vc)) { syslog(LOG_ERR, "ocserv-oidc: failed to load jwks\n"); exit(1); } @@ -303,13 +315,17 @@ static json_t *oidc_fetch_json_from_uri(void * pool, const char *uri) } // Download and parse the JWT keys for this virtual server context -static bool oidc_fetch_oidc_keys(void * pool, oidc_vctx_st * vctx) +static bool oidc_fetch_oidc_keys(oidc_vctx_st * vctx) { bool result = false; json_t *jwks = NULL; json_t *openid_configuration_url = json_object_get(vctx->config, "openid_configuration_url"); + json_t *array; + size_t index; + json_t *value; + if (!openid_configuration_url) { syslog(LOG_AUTH, "ocserv-oidc: openid_configuration_url missing from config\n"); @@ -317,7 +333,7 @@ static bool oidc_fetch_oidc_keys(void * pool, oidc_vctx_st * vctx) } json_t *oidc_config = - oidc_fetch_json_from_uri(pool, + oidc_fetch_json_from_uri(vctx->pool, json_string_value (openid_configuration_url)); @@ -334,7 +350,7 @@ static bool oidc_fetch_oidc_keys(void * pool, oidc_vctx_st * vctx) goto cleanup; } - jwks = oidc_fetch_json_from_uri(pool, json_string_value(jwks_uri)); + jwks = oidc_fetch_json_from_uri(vctx->pool, json_string_value(jwks_uri)); if (!jwks) { syslog(LOG_AUTH, "ocserv-oidc: failed to fetch keys from jwks_uri %s\n", @@ -342,10 +358,27 @@ static bool oidc_fetch_oidc_keys(void * pool, oidc_vctx_st * vctx) goto cleanup; } + array = json_object_get(jwks, "keys"); + if (array == NULL) { + syslog(LOG_AUTH, "ocserv-oidc: JWK keys malformed\n"); + goto cleanup; + } + + // Log the keys obtained + json_array_foreach(array, index, value) { + json_t *key_kid = json_object_get(value, "kid"); + syslog(LOG_INFO, + "ocserv-oidc: fetched new JWK %s\n", + json_string_value(key_kid) + ); + } + if (vctx->jwks) { json_decref(vctx->jwks); } + vctx->last_jwks_load_time = time(0); + vctx->jwks = jwks; jwks = NULL; result = true; @@ -537,8 +570,20 @@ static bool oidc_verify_singature(oidc_vctx_st * vctx, cjose_jws_t * jws) } if (jwk == NULL) { + time_t now; syslog(LOG_AUTH, "ocserv-oidc: JWK with kid=%s not found\n", json_string_value(token_kid)); + + syslog(LOG_AUTH, "ocserv-oidc: attempting to download new JWKs"); + now = time(0); + if ((now - vctx->last_jwks_load_time) > vctx->minimum_jwk_refresh_time) { + oidc_fetch_oidc_keys(vctx); + } + else { + syslog(LOG_AUTH, "ocserv-oidc: skipping JWK refresh"); + } + + // Fail the request and let the client try again. goto cleanup; } diff --git a/tests/generate_oidc_test_data.c b/tests/generate_oidc_test_data.c index 6637a000..13d28b26 100644 --- a/tests/generate_oidc_test_data.c +++ b/tests/generate_oidc_test_data.c @@ -65,6 +65,10 @@ json_t *create_oidc_config(const char *openid_configuration_url, goto cleanup; } + if (json_object_set_new(config, "minimum_jwk_refresh_time", json_integer(0))) { + goto cleanup; + } + required_claims = NULL; result = true; @@ -257,19 +261,21 @@ int main(int argc, char **argv) const char audience[] = "SomeAudience"; const char issuer[] = "SomeIssuer"; const char user_name_claim[] = "preferred_user_name"; - const char kid[] = "My Fake Key"; + char kid[64]; const char user_name[] = "SomeUser"; const char typ[] = "JWT"; const char alg[] = "ES256"; time_t now = time(NULL); + snprintf(kid, sizeof(kid), "key_%ld", now); + if (!getcwd(working_directory, sizeof(working_directory))) { return 1; } strncat(working_directory, "/data", sizeof(working_directory)-1); working_directory[sizeof(working_directory)-1] = 0; - cjose_jwk_t *key = create_key("My Fake Key"); + cjose_jwk_t *key = create_key(kid); generate_config_files(working_directory, key, audience, issuer, user_name_claim); diff --git a/tests/test-oidc b/tests/test-oidc index 2a810630..3daa404b 100755 --- a/tests/test-oidc +++ b/tests/test-oidc @@ -46,6 +46,26 @@ for token in data/fail_*; do fi done +sleep 5s + +# Generate new OIDC keys +# First client should fail, triggering reload of keys +`dirname $0`/gen_oidc_test_data +for token in data/success_*; do + http_result=$(LD_PRELOAD=libsocket_wrapper.so curl --insecure https://$ADDRESS:$PORT --request POST --data config-auth.xml --header "Authorization:Bearer=`cat $token`" --output /dev/null --write-out "%{http_code}") + if [ "$http_result" != "401" ]; then + fail $PID "Token incorrectly accepted returned $http_result" + fi +done + +# Second client should succeed with new keys +for token in data/success_*; do + http_result=$(LD_PRELOAD=libsocket_wrapper.so curl --insecure https://$ADDRESS:$PORT --request POST --data config-auth.xml --header "Authorization:Bearer=`cat $token`" --output /dev/null --write-out "%{http_code}") + if [ "$http_result" != "200" ]; then + fail $PID "Token incorrectly rejected returned $http_result" + fi +done + if ! test -f ${PIDFILE};then fail $PID "Could not find pid file ${PIDFILE}" fi