diff --git a/ChangeLog b/ChangeLog index 4ccab73e..b2f5c47a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,12 @@ +10/04/2024 +- ensure backwards compatibility with versions <2.4.16.x when a JSON array of string values + is provided in the "aud" claim of the ID token; required by (at least) Oracle IDCS + see #1272 and #1273; thanks @lufik and @tydalforce +- add OIDCIDTokenAudValues configuration primitive that allows for explicit (and exhaustive) + configuration of the list of accepted values in the "aud" claim of the ID token + e.g. as required for passing FAPI 2 conformance testing +- bump to 2.4.16.5rc0 + 09/29/2024 - release 2.4.16.4 diff --git a/auth_openidc.conf b/auth_openidc.conf index 45080b84..be016e7f 100644 --- a/auth_openidc.conf +++ b/auth_openidc.conf @@ -347,6 +347,12 @@ # NB: this can be overridden on a per-OP basis in the .conf file using the key: id_token_encrypted_response_enc #OIDCIDTokenEncryptedResponseEnc [A128CBC-HS256|A256CBC-HS512|A256GCM] +# The accepted value(s) of the "aud" claim in the ID token, restricted to only those values that have been defined here. +# The convienience value "@" can be used to refer to the configured client id (i.e. in case of dynamic client registration). +# When not defined the default is to accept any list of values (or a single string value) that includes value of OIDCClientID. +# NB: this can be overridden on a per-OP basis in the .conf file using the key: id_token_aud_values with the value set to a JSON array of strings. +#OIDCIDTokenAudValues + + # The algorithm that the OP should use to sign the UserInfo response # When not defined the default (by spec) is that the OP does not sign the response. # (ES??? algorithms only supported when using OpenSSL >= 1.0) diff --git a/configure.ac b/configure.ac index 4cc57922..72573ea7 100644 --- a/configure.ac +++ b/configure.ac @@ -1,4 +1,4 @@ -AC_INIT([mod_auth_openidc],[2.4.16.4],[hans.zandbelt@openidc.com]) +AC_INIT([mod_auth_openidc],[2.4.16.5rc0],[hans.zandbelt@openidc.com]) AC_SUBST(NAMEVER, AC_PACKAGE_TARNAME()-AC_PACKAGE_VERSION()) diff --git a/src/cfg/cfg.c b/src/cfg/cfg.c index de30a584..02288b14 100644 --- a/src/cfg/cfg.c +++ b/src/cfg/cfg.c @@ -52,6 +52,13 @@ #include "session.h" #include "util.h" +const char *oidc_cfg_string_list_add(apr_pool_t *pool, apr_array_header_t **list, const char *arg) { + if (*list == NULL) + *list = apr_array_make(pool, 1, sizeof(const char *)); + APR_ARRAY_PUSH(*list, const char *) = arg; + return NULL; +} + #define OIDC_DEFAULT_ACTION_ON_USERINFO_ERROR OIDC_ON_ERROR_502 OIDC_CFG_MEMBER_FUNC_TYPE_GET(action_on_userinfo_error, oidc_on_error_action_t, OIDC_DEFAULT_ACTION_ON_USERINFO_ERROR) diff --git a/src/cfg/cfg.h b/src/cfg/cfg.h index 3ed0eec9..776b3670 100644 --- a/src/cfg/cfg.h +++ b/src/cfg/cfg.h @@ -186,6 +186,7 @@ void *oidc_cfg_server_merge(apr_pool_t *pool, void *BASE, void *ADD); int oidc_cfg_post_config(oidc_cfg_t *cfg, server_rec *s); void oidc_cfg_child_init(apr_pool_t *pool, oidc_cfg_t *cfg, server_rec *s); void oidc_cfg_cleanup_child(oidc_cfg_t *cfg, server_rec *s); +const char *oidc_cfg_string_list_add(apr_pool_t *pool, apr_array_header_t **list, const char *arg); #define OIDC_CFG_MEMBER_FUNC_NAME(member, type, method) oidc_##type##_##member##_##method diff --git a/src/cfg/cmds.c b/src/cfg/cmds.c index e6c32783..ac093a7e 100644 --- a/src/cfg/cmds.c +++ b/src/cfg/cmds.c @@ -465,6 +465,11 @@ const command_rec oidc_cfg_cmds[] = { OIDCIDTokenEncryptedResponseEnc, id_token_encrypted_response_enc, "The algorithm that the OP should use to encrypt to the id_token with the Content Encryption Key (used only in dynamic client registration); must be one of [A128CBC-HS256|A256CBC-HS512|A256GCM]"), + OIDC_CFG_CMD_PROVIDER( + AP_INIT_ITERATE, + OIDCIDTokenAudValues, + id_token_aud_values, + "Accepted \"aud\" claim values in the ID token."), OIDC_CFG_CMD_PROVIDER( AP_INIT_TAKE1, OIDCUserInfoSignedResponseAlg, diff --git a/src/cfg/dir.c b/src/cfg/dir.c index f40e9044..4db9ca0b 100644 --- a/src/cfg/dir.c +++ b/src/cfg/dir.c @@ -244,21 +244,14 @@ const char *oidc_cmd_dir_accept_oauth_token_in_set(cmd_parms *cmd, void *m, cons /* * specify cookies names to pass/strip */ -static const char *oidc_cfg_dir_cookie_names_set(apr_pool_t *pool, apr_array_header_t **cookie_names, const char *arg) { - if (*cookie_names == NULL) - *cookie_names = apr_array_make(pool, 3, sizeof(const char *)); - APR_ARRAY_PUSH(*cookie_names, const char *) = arg; - return NULL; -} - const char *oidc_cmd_dir_strip_cookies_set(cmd_parms *cmd, void *m, const char *arg) { oidc_dir_cfg_t *dir_cfg = (oidc_dir_cfg_t *)m; - return oidc_cfg_dir_cookie_names_set(cmd->pool, &dir_cfg->strip_cookies, arg); + return oidc_cfg_string_list_add(cmd->pool, &dir_cfg->strip_cookies, arg); } const char *oidc_cmd_dir_pass_cookies_set(cmd_parms *cmd, void *m, const char *arg) { oidc_dir_cfg_t *dir_cfg = (oidc_dir_cfg_t *)m; - return oidc_cfg_dir_cookie_names_set(cmd->pool, &dir_cfg->pass_cookies, arg); + return oidc_cfg_string_list_add(cmd->pool, &dir_cfg->pass_cookies, arg); } #define OIDC_CFG_DIR_MEMBER_FUNC_GET(member, type, def_val, unset_val) \ diff --git a/src/cfg/provider.c b/src/cfg/provider.c index 9f445c16..7802c12f 100644 --- a/src/cfg/provider.c +++ b/src/cfg/provider.c @@ -89,6 +89,7 @@ struct oidc_provider_t { char *id_token_signed_response_alg; char *id_token_encrypted_response_alg; char *id_token_encrypted_response_enc; + apr_array_header_t *id_token_aud_values; char *userinfo_signed_response_alg; char *userinfo_encrypted_response_alg; char *userinfo_encrypted_response_enc; @@ -240,6 +241,28 @@ OIDC_PROVIDER_TYPE_MEMBER_FUNCS_PASSPHRASE(token_endpoint_tls_client_key_pwd) OIDC_PROVIDER_MEMBER_FUNCS_KEYS(verify_public_keys) OIDC_PROVIDER_MEMBER_FUNCS_KEYS(client_keys) +/* + * string list + */ + +#define OIDC_PROVIDER_MEMBER_FUNCS_STR_LIST(member) \ + \ + const char *oidc_cfg_provider_##member##_set_str_list(apr_pool_t *pool, oidc_provider_t *provider, \ + apr_array_header_t *arg) { \ + provider->member = arg; \ + return NULL; \ + } \ + \ + const char *oidc_cfg_provider_##member##_set(apr_pool_t *pool, oidc_provider_t *provider, const char *arg) { \ + return oidc_cfg_string_list_add(pool, &provider->member, arg); \ + } \ + OIDC_PROVIDER_MEMBER_FUNCS_TYPE_DEF(member, const apr_array_header_t *, NULL) + +/* + * id token aud values + */ +OIDC_PROVIDER_MEMBER_FUNCS_STR_LIST(id_token_aud_values) + /* * PKCE */ @@ -701,6 +724,8 @@ static void oidc_cfg_provider_init(oidc_provider_t *provider) { provider->request_object = NULL; provider->response_require_iss = OIDC_CONFIG_POS_INT_UNSET; + + provider->id_token_aud_values = NULL; } void oidc_cfg_provider_merge(apr_pool_t *pool, oidc_provider_t *dst, const oidc_provider_t *base, @@ -812,6 +837,9 @@ void oidc_cfg_provider_merge(apr_pool_t *pool, oidc_provider_t *dst, const oidc_ dst->response_require_iss = add->response_require_iss != OIDC_CONFIG_POS_INT_UNSET ? add->response_require_iss : base->response_require_iss; + + dst->id_token_aud_values = + add->id_token_aud_values != NULL ? add->id_token_aud_values : base->id_token_aud_values; } oidc_provider_t *oidc_cfg_provider_create(apr_pool_t *pool) { diff --git a/src/cfg/provider.h b/src/cfg/provider.h index fe5653ba..79db348f 100644 --- a/src/cfg/provider.h +++ b/src/cfg/provider.h @@ -122,6 +122,7 @@ typedef struct oidc_jwks_uri_t { #define OIDCIDTokenSignedResponseAlg "OIDCIDTokenSignedResponseAlg" #define OIDCIDTokenEncryptedResponseAlg "OIDCIDTokenEncryptedResponseAlg" #define OIDCIDTokenEncryptedResponseEnc "OIDCIDTokenEncryptedResponseEnc" +#define OIDCIDTokenAudValues "OIDCIDTokenAudValues" #define OIDCUserInfoSignedResponseAlg "OIDCUserInfoSignedResponseAlg" #define OIDCUserInfoEncryptedResponseAlg "OIDCUserInfoEncryptedResponseAlg" #define OIDCUserInfoEncryptedResponseEnc "OIDCUserInfoEncryptedResponseEnc" @@ -178,6 +179,11 @@ typedef struct oidc_jwks_uri_t { OIDC_CFG_PROVIDER_MEMBER_FUNCS_TYPE_DECL(member, type) \ void OIDC_CFG_MEMBER_FUNC_NAME(member, cfg_provider, int_set)(oidc_provider_t * provider, type arg); +#define OIDC_CFG_PROVIDER_MEMBER_FUNCS_STR_LIST_DECL(member) \ + OIDC_CFG_PROVIDER_MEMBER_FUNCS_TYPE_DECL(member, const apr_array_header_t *) \ + const char *OIDC_CFG_MEMBER_FUNC_NAME(member, cfg_provider, set_str_list)(apr_pool_t *, oidc_provider_t *, \ + apr_array_header_t *); + OIDC_CFG_PROVIDER_MEMBER_FUNCS_STR_DECL(metadata_url) OIDC_CFG_PROVIDER_MEMBER_FUNCS_STR_DECL(issuer) OIDC_CFG_PROVIDER_MEMBER_FUNCS_STR_DECL(authorization_endpoint_url); @@ -212,6 +218,9 @@ OIDC_CFG_PROVIDER_MEMBER_FUNCS_STR_DECL(userinfo_encrypted_response_alg) OIDC_CFG_PROVIDER_MEMBER_FUNCS_STR_DECL(userinfo_encrypted_response_enc) OIDC_CFG_PROVIDER_MEMBER_FUNCS_STR_DECL(request_object) +// string list +OIDC_CFG_PROVIDER_MEMBER_FUNCS_STR_LIST_DECL(id_token_aud_values) + // keys OIDC_CFG_PROVIDER_MEMBER_FUNCS_KEYS_DECL(verify_public_keys) OIDC_CFG_PROVIDER_MEMBER_FUNCS_KEYS_DECL(client_keys) diff --git a/src/metadata.c b/src/metadata.c index 9a686221..709fe689 100644 --- a/src/metadata.c +++ b/src/metadata.c @@ -85,6 +85,7 @@ #define OIDC_METADATA_ID_TOKEN_SIGNED_RESPONSE_ALG "id_token_signed_response_alg" #define OIDC_METADATA_ID_TOKEN_ENCRYPTED_RESPONSE_ALG "id_token_encrypted_response_alg" #define OIDC_METADATA_ID_TOKEN_ENCRYPTED_RESPONSE_ENC "id_token_encrypted_response_enc" +#define OIDC_METADATA_ID_TOKEN_AUD_VALUES "id_token_aud_values" #define OIDC_METADATA_USERINFO_SIGNED_RESPONSE_ALG "userinfo_signed_response_alg" #define OIDC_METADATA_USERINFO_ENCRYPTED_RESPONSE_ALG "userinfo_encrypted_response_alg" #define OIDC_METADATA_USERINFO_ENCRYPTED_RESPONSE_ENC "userinfo_encrypted_response_enc" @@ -1228,7 +1229,7 @@ apr_byte_t oidc_metadata_conf_parse(request_rec *r, oidc_cfg_t *cfg, json_t *j_c const char *rv = NULL; char *value = NULL; int ivalue = OIDC_CONFIG_POS_INT_UNSET; - apr_array_header_t *keys = NULL; + apr_array_header_t *keys = NULL, *auds = NULL; oidc_util_json_object_get_string(r->pool, j_conf, OIDC_METADATA_CLIENT_JWKS_URI, &value, oidc_cfg_provider_client_jwks_uri_get(oidc_cfg_provider_get(cfg))); @@ -1263,6 +1264,14 @@ apr_byte_t oidc_metadata_conf_parse(request_rec *r, oidc_cfg_t *cfg, json_t *j_c oidc_cfg_provider_id_token_encrypted_response_enc_get(oidc_cfg_provider_get(cfg))); OIDC_METADATA_PROVIDER_SET(id_token_encrypted_response_enc, value, rv) + oidc_util_json_object_get_string_array(r->pool, j_conf, OIDC_METADATA_ID_TOKEN_AUD_VALUES, &auds, + oidc_cfg_provider_id_token_aud_values_get(oidc_cfg_provider_get(cfg))); + if (auds != NULL) { + rv = oidc_cfg_provider_id_token_aud_values_set_str_list(r->pool, provider, auds); + if (rv != NULL) + oidc_error(r, "oidc_cfg_provider_aud_values_set: %s", rv); + } + /* get the (optional) signing & encryption settings for the userinfo response */ oidc_util_json_object_get_string( r->pool, j_conf, OIDC_METADATA_USERINFO_SIGNED_RESPONSE_ALG, &value, diff --git a/src/proto/id_token.c b/src/proto/id_token.c index dc00159b..ad4a3dce 100644 --- a/src/proto/id_token.c +++ b/src/proto/id_token.c @@ -96,6 +96,8 @@ apr_byte_t oidc_proto_idtoken_validate_nonce(request_rec *r, oidc_cfg_t *cfg, oi return TRUE; } +#define OIDC_PROTO_IDTOKEN_AUD_CLIENT_ID_SPECIAL_VALUE "@" + /* * validate the "aud" and "azp" claims in the id_token payload */ @@ -103,6 +105,10 @@ apr_byte_t oidc_proto_idtoken_validate_aud_and_azp(request_rec *r, oidc_cfg_t *c oidc_jwt_payload_t *id_token_payload) { char *azp = NULL; + const char *s_aud = NULL; + const apr_array_header_t *arr = NULL; + int i = 0; + oidc_jose_get_string(r->pool, id_token_payload->value.json, OIDC_CLAIM_AZP, FALSE, &azp, NULL); /* @@ -122,45 +128,92 @@ apr_byte_t oidc_proto_idtoken_validate_aud_and_azp(request_rec *r, oidc_cfg_t *c json_t *aud = json_object_get(id_token_payload->value.json, OIDC_CLAIM_AUD); if (aud != NULL) { + arr = oidc_cfg_provider_id_token_aud_values_get(provider); + /* check if it is a single-value */ if (json_is_string(aud)) { - /* a single-valued audience must be equal to our client_id */ - if (_oidc_strcmp(json_string_value(aud), oidc_cfg_provider_client_id_get(provider)) != 0) { - oidc_error(r, - "the configured client_id (%s) did not match the \"%s\" claim value (%s) in " - "the id_token", - oidc_cfg_provider_client_id_get(provider), OIDC_CLAIM_AUD, - json_string_value(aud)); - return FALSE; + if (arr == NULL) { + + /* a single-valued audience must be equal to our client_id */ + if (_oidc_strcmp(json_string_value(aud), oidc_cfg_provider_client_id_get(provider)) != + 0) { + oidc_error(r, + "the configured client_id (%s) did not match the \"%s\" claim value " + "(%s) in " + "the id_token", + oidc_cfg_provider_client_id_get(provider), OIDC_CLAIM_AUD, + json_string_value(aud)); + return FALSE; + } + + } else { + + for (i = 0; i < arr->nelts; i++) { + s_aud = APR_ARRAY_IDX(arr, i, const char *); + if (_oidc_strcmp(s_aud, OIDC_PROTO_IDTOKEN_AUD_CLIENT_ID_SPECIAL_VALUE) == 0) + s_aud = oidc_cfg_provider_client_id_get(provider); + if (_oidc_strcmp(json_string_value(aud), s_aud) == 0) + break; + } + + if (i == arr->nelts) { + oidc_error( + r, "none of our configured audience values could be found in \"%s\" claim", + OIDC_CLAIM_AUD); + return FALSE; + } } /* check if this is a multi-valued audience */ } else if (json_is_array(aud)) { - if ((json_array_size(aud) > 1) && (azp == NULL)) { - oidc_warn(r, - "the \"%s\" claim value in the id_token is an array with more than 1 " - "element, but \"%s\" claim is not present (a SHOULD in the spec...)", - OIDC_CLAIM_AUD, OIDC_CLAIM_AZP); - } - - if (oidc_util_json_array_has_value(r, aud, oidc_cfg_provider_client_id_get(provider)) == - FALSE) { - oidc_error(r, - "our configured client_id (%s) could not be found in the array of values " - "for \"%s\" claim", - oidc_cfg_provider_client_id_get(provider), OIDC_CLAIM_AUD); - return FALSE; - } - - if (json_array_size(aud) > 1) { - oidc_error( - r, - "our configured client_id (%s) was found in the array of values " - "for \"%s\" claim, but there are other unknown/untrusted values included as well", - oidc_cfg_provider_client_id_get(provider), OIDC_CLAIM_AUD); - return FALSE; + if (arr == NULL) { + + if ((json_array_size(aud) > 1) && (azp == NULL)) { + oidc_warn(r, + "the \"%s\" claim value in the id_token is an array with more than 1 " + "element, but \"%s\" claim is not present (a SHOULD in the spec...)", + OIDC_CLAIM_AUD, OIDC_CLAIM_AZP); + } + + if (oidc_util_json_array_has_value(r, aud, oidc_cfg_provider_client_id_get(provider)) == + FALSE) { + oidc_error( + r, + "our configured client_id (%s) could not be found in the array of values " + "for \"%s\" claim", + oidc_cfg_provider_client_id_get(provider), OIDC_CLAIM_AUD); + return FALSE; + } + + } else { + + /* handle explicit and exhaustive configuration of acceptable audience values */ + + for (i = 0; i < arr->nelts; i++) { + s_aud = APR_ARRAY_IDX(arr, i, const char *); + if (_oidc_strcmp(s_aud, OIDC_PROTO_IDTOKEN_AUD_CLIENT_ID_SPECIAL_VALUE) == 0) + s_aud = oidc_cfg_provider_client_id_get(provider); + if (oidc_util_json_array_has_value(r, aud, s_aud) == FALSE) { + oidc_error(r, + "our configured audience value (%s) could not be found in " + "the array of values " + "for \"%s\" claim", + APR_ARRAY_IDX(arr, i, const char *), OIDC_CLAIM_AUD); + return FALSE; + } + } + + if (json_array_size(aud) > arr->nelts) { + oidc_error( + r, + "our configured audience values are all present in the array of values " + "for \"%s\" claim, but there are other unknown/untrusted values included " + "as well", + OIDC_CLAIM_AUD); + return FALSE; + } } } else { diff --git a/src/util.c b/src/util.c index 04ced975..1375821e 100644 --- a/src/util.c +++ b/src/util.c @@ -1703,6 +1703,27 @@ apr_byte_t oidc_util_json_object_get_string(apr_pool_t *pool, json_t *json, cons return TRUE; } +/* + * get (optional) string array from a JSON object + */ +apr_byte_t oidc_util_json_object_get_string_array(apr_pool_t *pool, json_t *json, const char *name, + apr_array_header_t **value, const apr_array_header_t *default_value) { + json_t *v = NULL, *arr = NULL; + size_t i = 0; + *value = (default_value != NULL) ? apr_array_copy(pool, default_value) : NULL; + if (json != NULL) { + arr = json_object_get(json, name); + if ((arr != NULL) && (json_is_array(arr))) { + *value = apr_array_make(pool, json_array_size(arr), sizeof(const char *)); + for (i = 0; i < json_array_size(arr); i++) { + v = json_array_get(arr, i); + APR_ARRAY_PUSH(*value, const char *) = apr_pstrdup(pool, json_string_value(v)); + } + } + } + return TRUE; +} + /* * get (optional) int from a JSON object */ diff --git a/src/util.h b/src/util.h index 5d45c45f..69500ffb 100644 --- a/src/util.h +++ b/src/util.h @@ -92,6 +92,8 @@ apr_byte_t oidc_util_spaced_string_equals(apr_pool_t *pool, const char *a, const apr_byte_t oidc_util_spaced_string_contains(apr_pool_t *pool, const char *str, const char *match); apr_byte_t oidc_util_json_object_get_string(apr_pool_t *pool, json_t *json, const char *name, char **value, const char *default_value); +apr_byte_t oidc_util_json_object_get_string_array(apr_pool_t *pool, json_t *json, const char *name, + apr_array_header_t **value, const apr_array_header_t *default_value); apr_byte_t oidc_util_json_object_get_int(const json_t *json, const char *name, int *value, const int default_value); apr_byte_t oidc_util_json_object_get_bool(const json_t *json, const char *name, int *value, const int default_value); char *oidc_util_html_escape(apr_pool_t *pool, const char *input);