From 7882d73721ac1e6baf8ac6c495db84f922f2ee66 Mon Sep 17 00:00:00 2001 From: Hans Zandbelt Date: Fri, 20 Jan 2023 10:27:18 +0100 Subject: [PATCH] fix OIDCOAuthVerifyCertFiles #989; add OIDCProviderVerifyCertFiles #990 - add OIDCProviderVerifyCertFiles option to statically configure ID token validation keys; see #989; thanks @madsfreek - fix bug in OIDCOAuthVerifyCertFiles where cert(s) would be cast to apr_hash_t instead of apr_array_header_t; see #990; thanks @bommo1 - bump to 2.4.12.3rc0 Signed-off-by: Hans Zandbelt --- AUTHORS | 1 + ChangeLog | 5 +++++ auth_openidc.conf | 8 ++++++++ configure.ac | 2 +- src/config.c | 15 ++++++++++++++- src/mod_auth_openidc.c | 2 +- src/mod_auth_openidc.h | 3 ++- src/oauth.c | 10 ++++------ src/proto.c | 2 +- 9 files changed, 37 insertions(+), 11 deletions(-) diff --git a/AUTHORS b/AUTHORS index 60025ba1..a13b0c1c 100644 --- a/AUTHORS +++ b/AUTHORS @@ -87,3 +87,4 @@ reporting bugs, providing fixes, suggesting useful features or other: Nikhil Chaudhari Quentin Gillet Brent van Laere + Mads Freek Petersen diff --git a/ChangeLog b/ChangeLog index 8d81e36a..c866c22a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +01/20/2023 +- add OIDCProviderVerifyCertFiles option to statically configure ID token validation keys; see #989; thanks @madsfreek +- fix bug in OIDCOAuthVerifyCertFiles where cert(s) would be cast to apr_hash_t instead of apr_array_header_t; see #990; thanks @bommo1 +- bump to 2.4.12.3rc0 + 12/28/2022 - update sample/test Dockerfile to Ubuntu Jammy diff --git a/auth_openidc.conf b/auth_openidc.conf index 70521c9d..494ebf91 100644 --- a/auth_openidc.conf +++ b/auth_openidc.conf @@ -65,6 +65,14 @@ # Used when OIDCProviderMetadataURL is not defined or the metadata obtained from that URL does not set it. #OIDCProviderJwksUri +# The fully qualified names of the files that contain the X.509 certificates with the RSA public +# keys that can be used for ID Token verification. +# NB: this is one or more key tuples where a key tuple consists of: +# [#] +# and the key identifier part is required when the ID Token contains a "kid" in its header. +# When not defined, ID Token validation key material has to be obtained through OIDCProviderJwksUri or OIDCProviderMetadataURL +#OIDCProviderVerifyCertFiles ([#])+ + # OpenID Connect Provider Token Endpoint URL (e.g. https://localhost:9031/as/token.oauth2) # Used when OIDCProviderMetadataURL is not defined or the metadata obtained from that URL does not set it. #OIDCProviderTokenEndpoint diff --git a/configure.ac b/configure.ac index e2a80325..d0928560 100644 --- a/configure.ac +++ b/configure.ac @@ -1,4 +1,4 @@ -AC_INIT([mod_auth_openidc],[2.4.12.2],[hans.zandbelt@openidc.com]) +AC_INIT([mod_auth_openidc],[2.4.12.3rc0],[hans.zandbelt@openidc.com]) AC_SUBST(NAMEVER, AC_PACKAGE_TARNAME()-AC_PACKAGE_VERSION()) diff --git a/src/config.c b/src/config.c index 3d17da5a..98c7dcba 100644 --- a/src/config.c +++ b/src/config.c @@ -173,6 +173,7 @@ #define OIDCProviderEndSessionEndpoint "OIDCProviderEndSessionEndpoint" #define OIDCProviderBackChannelLogoutSupported "OIDCProviderBackChannelLogoutSupported" #define OIDCProviderJwksUri "OIDCProviderJwksUri" +#define OIDCProviderVerifyCertFiles "OIDCProviderVerifyCertFiles" #define OIDCResponseType "OIDCResponseType" #define OIDCResponseMode "OIDCResponseMode" #define OIDCPublicKeyFiles "OIDCPublicKeyFiles" @@ -1417,6 +1418,7 @@ void oidc_cfg_provider_init(oidc_provider_t *provider) { provider->check_session_iframe = NULL; provider->end_session_endpoint = NULL; provider->jwks_uri = NULL; + provider->verify_public_keys = NULL; provider->backchannel_logout_supported = OIDC_CONFIG_POS_INT_UNSET; provider->ssl_validate_server = OIDC_DEFAULT_SSL_VALIDATE_SERVER; @@ -1633,6 +1635,10 @@ void* oidc_merge_server_config(apr_pool_t *pool, void *BASE, void *ADD) { c->provider.jwks_uri = add->provider.jwks_uri != NULL ? add->provider.jwks_uri : base->provider.jwks_uri; + c->provider.verify_public_keys = + add->provider.verify_public_keys != NULL ? + add->provider.verify_public_keys : + base->provider.verify_public_keys; c->provider.client_id = add->provider.client_id != NULL ? add->provider.client_id : base->provider.client_id; @@ -2677,7 +2683,9 @@ static apr_status_t oidc_cleanup_child(void *data) { // can do this even though we haven't got a deep copy // since references within the object will be set to NULL - oidc_jwk_list_destroy_hash(sp->process->pool, + oidc_jwk_list_destroy(sp->process->pool, + cfg->provider.verify_public_keys); + oidc_jwk_list_destroy(sp->process->pool, cfg->oauth.verify_public_keys); oidc_jwk_list_destroy_hash(sp->process->pool, cfg->oauth.verify_shared_keys); @@ -3058,6 +3066,11 @@ const command_rec oidc_config_cmds[] = { (void *)APR_OFFSETOF(oidc_cfg, provider.jwks_uri), RSRC_CONF, "Define the OpenID OP JWKS URL (e.g.: https://localhost:9031/pf/JWKS)"), + AP_INIT_ITERATE(OIDCProviderVerifyCertFiles, + oidc_set_public_key_files, + (void*)APR_OFFSETOF(oidc_cfg, provider.verify_public_keys), + RSRC_CONF, + "The fully qualified names of the files that contain the X.509 certificates that contains the RSA public keys that can be used for ID token validation."), AP_INIT_TAKE1(OIDCResponseType, oidc_set_response_type, (void *)APR_OFFSETOF(oidc_cfg, provider.response_type), diff --git a/src/mod_auth_openidc.c b/src/mod_auth_openidc.c index c4a89418..c7f6c0e5 100644 --- a/src/mod_auth_openidc.c +++ b/src/mod_auth_openidc.c @@ -3024,7 +3024,7 @@ static int oidc_handle_logout_backchannel(request_rec *r, oidc_cfg *cfg) { oidc_jwks_uri_t jwks_uri = { provider->jwks_uri, provider->jwks_refresh_interval, provider->ssl_validate_server }; if (oidc_proto_jwt_verify(r, cfg, jwt, &jwks_uri, - oidc_util_merge_symmetric_key(r->pool, NULL, jwk), + oidc_util_merge_symmetric_key(r->pool, provider->verify_public_keys, jwk), provider->id_token_signed_response_alg) == FALSE) { oidc_error(r, "id_token signature could not be validated, aborting"); diff --git a/src/mod_auth_openidc.h b/src/mod_auth_openidc.h index 5112eec1..a67bbfbb 100644 --- a/src/mod_auth_openidc.h +++ b/src/mod_auth_openidc.h @@ -296,6 +296,7 @@ typedef struct oidc_provider_t { char *check_session_iframe; char *end_session_endpoint; char *jwks_uri; + apr_array_header_t *verify_public_keys; char *client_id; char *client_secret; char *token_endpoint_tls_client_key; @@ -364,7 +365,7 @@ typedef struct oidc_oauth_t { oidc_remote_user_claim_t remote_user_claim; apr_hash_t *verify_shared_keys; char *verify_jwks_uri; - apr_hash_t *verify_public_keys; + apr_array_header_t *verify_public_keys; int access_token_binding_policy; } oidc_oauth_t; diff --git a/src/oauth.c b/src/oauth.c index 889c0e5e..bfc245de 100644 --- a/src/oauth.c +++ b/src/oauth.c @@ -612,7 +612,7 @@ static apr_byte_t oidc_oauth_validate_jwt_access_token(request_rec *r, oidc_debug(r, "verify JWT against %d statically configured public keys and %d shared keys, with JWKs URI set to %s", c->oauth.verify_public_keys ? - apr_hash_count(c->oauth.verify_public_keys) : 0, + c->oauth.verify_public_keys->nelts : 0, c->oauth.verify_shared_keys ? apr_hash_count(c->oauth.verify_shared_keys) : 0, c->oauth.verify_jwks_uri); @@ -620,11 +620,9 @@ static apr_byte_t oidc_oauth_validate_jwt_access_token(request_rec *r, // TODO: we're re-using the OIDC provider JWKs refresh interval here... oidc_jwks_uri_t jwks_uri = { c->oauth.verify_jwks_uri, c->provider.jwks_refresh_interval, c->oauth.ssl_validate_server }; - if (oidc_proto_jwt_verify(r, c, jwt, &jwks_uri, - oidc_util_merge_key_sets_hash(r->pool, c->oauth.verify_public_keys, - c->oauth.verify_shared_keys), NULL) == FALSE) { - oidc_error(r, - "JWT access token signature could not be validated, aborting"); + if (oidc_proto_jwt_verify(r, c, jwt, &jwks_uri, oidc_util_merge_key_sets(r->pool, c->oauth.verify_shared_keys, c->oauth.verify_public_keys), NULL) + == FALSE) { + oidc_error(r, "JWT access token signature could not be validated, aborting"); oidc_jwt_destroy(jwt); return FALSE; } diff --git a/src/proto.c b/src/proto.c index 26fcd26b..470ec7de 100644 --- a/src/proto.c +++ b/src/proto.c @@ -1668,7 +1668,7 @@ apr_byte_t oidc_proto_parse_idtoken(request_rec *r, oidc_cfg *cfg, oidc_jwks_uri_t jwks_uri = { provider->jwks_uri, provider->jwks_refresh_interval, provider->ssl_validate_server }; if (oidc_proto_jwt_verify(r, cfg, *jwt, &jwks_uri, - oidc_util_merge_symmetric_key(r->pool, NULL, jwk), + oidc_util_merge_symmetric_key(r->pool, provider->verify_public_keys, jwk), provider->id_token_signed_response_alg) == FALSE) { oidc_error(r,