From f9a52507acee0817b9cc91ec194bcea894f77ee0 Mon Sep 17 00:00:00 2001 From: Hans Zandbelt Date: Tue, 31 Oct 2023 21:08:03 +0100 Subject: [PATCH] remove obsolete support for Token Binding RFC 8471 Signed-off-by: Hans Zandbelt --- ChangeLog | 1 + auth_openidc.conf | 19 ----- src/config.c | 51 ------------ src/metadata.c | 22 +----- src/mod_auth_openidc.c | 13 +--- src/mod_auth_openidc.h | 16 +--- src/oauth.c | 8 +- src/parse.c | 52 ------------- src/parse.h | 5 -- src/proto.c | 54 +------------ src/session.c | 23 ------ src/util.c | 171 ----------------------------------------- test/test.c | 6 +- 13 files changed, 9 insertions(+), 432 deletions(-) diff --git a/ChangeLog b/ChangeLog index 7ce559c6..d82a8b49 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,6 +1,7 @@ 10/31/2023 - add capability to seamlessly rollover OIDCCryptoPassphrase using a (temporary) 2nd value that holds the old one - bump to 2.4.15rc1 +- remove obsolete support for Token Binding https://www.rfc-editor.org/rfc/rfc8471.html (id_token, access_token, session cookie) 10/30/2023 - do not apply logout_on_error and authenticate_on_error when a parallel refresh token request is detected diff --git a/auth_openidc.conf b/auth_openidc.conf index 5bd068d3..56753317 100644 --- a/auth_openidc.conf +++ b/auth_openidc.conf @@ -301,15 +301,6 @@ # NB: this can be overridden on a per-OP basis in the .conf file using the key: pkce_method #OIDCPKCEMethod [plain|S256|referred_tb] -# The OpenID Connect Bound Authentication policy used, -# see: http://openid.net/specs/openid-connect-token-bound-authentication-1_0.html -# "disabled": no referred token binding will be requested from the User Agent upon redirection to the OP -# "optional": referred token binding will be requested, the "cnf["tbh"]" claim is optional on return -# "required": referred token binding will be requested, the "cnf["tbh"]" claim must be present when the Client supports Token Binding -# "enforced": referred token binding will be requested, the "cnf["tbh"]" claim must be present and the User Agent must support Token Binding -# When not defined the default is "optional". -#OIDCTokenBindingPolicy [disabled|optional|required|enforced] - # (used only in dynamic client registration) # Define the Client JWKs URL (e.g. https://localhost/protected/?jwks=rsa)") that will be # used during client registration to point to the JWK set with public keys for this client. @@ -496,16 +487,6 @@ # When not defined the default "header" is used. #OIDCOAuthAcceptTokenAs [header|post|query|cookie[:|basic]+ -# The Token Binding policy used for OAuth 2.0 Access Tokens -# see: https://tools.ietf.org/html/draft-ietf-oauth-token-binding -# "disabled": no token binding ID will be verified in the access token, present or not -# "optional": the "cnf["tbh"]" claim is optional in the introspection result or the JWT access token, if it is present it will be verified -# "required": the "cnf["tbh"]" claim must be present when the Client supports Token Binding -# "enforced": the "cnf["tbh"]" claim must be present and the Client must support Token Binding -# When not defined the default is "optional". -#OIDCOAuthAccessTokenBindingPolicy [disabled|optional|required|enforced] - - ######################################################################################## # # Cookie Settings diff --git a/src/config.c b/src/config.c index dbff176e..27f040ab 100644 --- a/src/config.c +++ b/src/config.c @@ -154,10 +154,6 @@ #define OIDC_DEFAULT_UNAUTZ_ACTION OIDC_UNAUTZ_RETURN403 /* defines for how long provider metadata will be cached */ #define OIDC_DEFAULT_PROVIDER_METADATA_REFRESH_INTERVAL 0 -/* defines the default token binding policy for a provider */ -#define OIDC_DEFAULT_PROVIDER_TOKEN_BINDING_POLICY OIDC_TOKEN_BINDING_POLICY_OPTIONAL -/* defines the default token binding policy for OAuth 2.0 access tokens */ -#define OIDC_DEFAULT_OAUTH_ACCESS_TOKEN_BINDING_POLICY OIDC_TOKEN_BINDING_POLICY_OPTIONAL /* define the default HTTP method used to send the authentication request to the provider */ #define OIDC_DEFAULT_AUTH_REQUEST_METHOD OIDC_AUTH_REQUEST_METHOD_GET /* define whether the issuer will be added to the redirect uri by default to mitigate the IDP mixup attack */ @@ -205,7 +201,6 @@ #define OIDCUserInfoEncryptedResponseAlg "OIDCUserInfoEncryptedResponseAlg" #define OIDCUserInfoEncryptedResponseEnc "OIDCUserInfoEncryptedResponseEnc" #define OIDCUserInfoTokenMethod "OIDCUserInfoTokenMethod" -#define OIDCTokenBindingPolicy "OIDCTokenBindingPolicy" #define OIDCSSLValidateServer "OIDCSSLValidateServer" #define OIDCValidateIssuer "OIDCValidateIssuer" #define OIDCClientName "OIDCClientName" @@ -284,7 +279,6 @@ #define OIDCProviderAuthRequestMethod "OIDCProviderAuthRequestMethod" #define OIDCBlackListedClaims "OIDCBlackListedClaims" #define OIDCOAuthServerMetadataURL "OIDCOAuthServerMetadataURL" -#define OIDCOAuthAccessTokenBindingPolicy "OIDCOAuthAccessTokenBindingPolicy" #define OIDCRefreshAccessTokenBeforeExpiry "OIDCRefreshAccessTokenBeforeExpiry" #define OIDCStateInputHeaders "OIDCStateInputHeaders" #define OIDCRedirectURLsAllowed "OIDCRedirectURLsAllowed" @@ -777,8 +771,6 @@ const char* oidc_parse_pkce_type(apr_pool_t *pool, const char *arg, *type = &oidc_pkce_plain; } else if (_oidc_strcmp(arg, OIDC_PKCE_METHOD_S256) == 0) { *type = &oidc_pkce_s256; - } else if (_oidc_strcmp(arg, OIDC_PKCE_METHOD_REFERRED_TB) == 0) { - *type = &oidc_pkce_referred_tb; } return NULL; @@ -1291,20 +1283,6 @@ static const char* oidc_set_filtered_claims(cmd_parms *cmd, void *m, return NULL; } -/* - * set the token binding policy - */ -static const char* oidc_set_token_binding_policy(cmd_parms *cmd, - void *struct_ptr, const char *arg) { - oidc_cfg *cfg = (oidc_cfg*) ap_get_module_config(cmd->server->module_config, - &auth_openidc_module); - int offset = (int) (long) cmd->info; - int *token_binding_policy = (int*) ((char*) cfg + offset); - const char *rv = oidc_parse_token_binding_policy(cmd->pool, arg, - token_binding_policy); - return OIDC_CONFIG_DIR_RV(cmd, rv); -} - /* * set the claim prefix */ @@ -1772,11 +1750,6 @@ static void oidc_merge_provider_config(apr_pool_t *pool, oidc_provider_t *dst, add->request_object != NULL ? add->request_object : base->request_object; - dst->token_binding_policy = - add->token_binding_policy - != OIDC_DEFAULT_PROVIDER_TOKEN_BINDING_POLICY ? - add->token_binding_policy : base->token_binding_policy; - dst->issuer_specific_redirect_uri = add->issuer_specific_redirect_uri != OIDC_DEFAULT_PROVIDER_ISSUER_SPECIFIC_REDIRECT_URI ? @@ -1850,9 +1823,6 @@ void* oidc_create_server_config(apr_pool_t *pool, server_rec *svr) { c->oauth.verify_public_keys = NULL; c->oauth.verify_shared_keys = NULL; - c->oauth.access_token_binding_policy = - OIDC_DEFAULT_OAUTH_ACCESS_TOKEN_BINDING_POLICY; - c->cache = &oidc_cache_shm; c->cache_cfg = NULL; c->cache_encrypt = OIDC_CONFIG_POS_INT_UNSET; @@ -1925,9 +1895,6 @@ void* oidc_create_server_config(apr_pool_t *pool, server_rec *svr) { c->provider_metadata_refresh_interval = OIDC_DEFAULT_PROVIDER_METADATA_REFRESH_INTERVAL; - c->provider.token_binding_policy = - OIDC_DEFAULT_PROVIDER_TOKEN_BINDING_POLICY; - c->info_hook_data = NULL; c->black_listed_claims = NULL; c->white_listed_claims = NULL; @@ -2068,12 +2035,6 @@ void* oidc_merge_server_config(apr_pool_t *pool, void *BASE, void *ADD) { add->oauth.verify_shared_keys : base->oauth.verify_shared_keys; - c->oauth.access_token_binding_policy = - add->oauth.access_token_binding_policy - != OIDC_DEFAULT_OAUTH_ACCESS_TOKEN_BINDING_POLICY ? - add->oauth.access_token_binding_policy : - base->oauth.access_token_binding_policy; - c->http_timeout_long.request_timeout = add->http_timeout_long.request_timeout != OIDC_DEFAULT_HTTP_REQUEST_TIMEOUT_LONG ? add->http_timeout_long.request_timeout : @@ -3397,12 +3358,6 @@ const command_rec oidc_config_cmds[] = { (void *)APR_OFFSETOF(oidc_cfg, provider.userinfo_token_method), RSRC_CONF, "The method that is used to present the access token to the userinfo endpoint; must be one of [authz_header|post_param]"), - AP_INIT_TAKE1(OIDCTokenBindingPolicy, - oidc_set_token_binding_policy, - (void *)APR_OFFSETOF(oidc_cfg, provider.token_binding_policy), - RSRC_CONF, - "The token binding policy used with the provider; must be one of [disabled|optional|required|enforced]"), - AP_INIT_TAKE1(OIDCSSLValidateServer, oidc_set_ssl_validate_slot, (void*)APR_OFFSETOF(oidc_cfg, provider.ssl_validate_server), @@ -3893,12 +3848,6 @@ const command_rec oidc_config_cmds[] = { (void*)APR_OFFSETOF(oidc_cfg, oauth.metadata_url), RSRC_CONF, "Authorization Server metadata URL."), - AP_INIT_TAKE1(OIDCOAuthAccessTokenBindingPolicy, - oidc_set_token_binding_policy, - (void *)APR_OFFSETOF(oidc_cfg, oauth.access_token_binding_policy), - RSRC_CONF, - "The token binding policy used for access tokens; must be one of [disabled|optional|required|enforced]"), - AP_INIT_TAKE12(OIDCRefreshAccessTokenBeforeExpiry, oidc_set_refresh_access_token_before_expiry, (void *)APR_OFFSETOF(oidc_dir_cfg, refresh_access_token_before_expiry), diff --git a/src/metadata.c b/src/metadata.c index 1d8aef9f..05ad75ab 100644 --- a/src/metadata.c +++ b/src/metadata.c @@ -92,7 +92,6 @@ extern module AP_MODULE_DECLARE_DATA auth_openidc_module; #define OIDC_METADATA_FRONTCHANNEL_LOGOUT_URI "frontchannel_logout_uri" #define OIDC_METADATA_BACKCHANNEL_LOGOUT_URI "backchannel_logout_uri" #define OIDC_METADATA_POST_LOGOUT_REDIRECT_URIS "post_logout_redirect_uris" -#define OIDC_METADATA_IDTOKEN_BINDING_CNF "id_token_token_binding_cnf" #define OIDC_METADATA_SSL_VALIDATE_SERVER "ssl_validate_server" #define OIDC_METADATA_VALIDATE_ISSUER "validate_issuer" #define OIDC_METADATA_SCOPE "scope" @@ -115,7 +114,6 @@ extern module AP_MODULE_DECLARE_DATA auth_openidc_module; #define OIDC_METADATA_TOKEN_ENDPOINT_TLS_CLIENT_KEY_PWD "token_endpoint_tls_client_key_pwd" #define OIDC_METADATA_REQUEST_OBJECT "request_object" #define OIDC_METADATA_USERINFO_TOKEN_METHOD "userinfo_token_method" -#define OIDC_METADATA_TOKEN_BINDING_POLICY "token_binding_policy" #define OIDC_METADATA_AUTH_REQUEST_METHOD "auth_request_method" #define OIDC_METADATA_ISSUER_SPECIFIC_REDIRECT_URI "issuer_specific_redirect_uri" @@ -580,11 +578,6 @@ static apr_byte_t oidc_metadata_client_register(request_rec *r, oidc_cfg *cfg, OIDC_REDIRECT_URI_REQUEST_LOGOUT, OIDC_BACKCHANNEL_STYLE_LOGOUT_PARAM_VALUE))); - if (provider->token_binding_policy > OIDC_TOKEN_BINDING_POLICY_DISABLED) { - json_object_set_new(data, OIDC_METADATA_IDTOKEN_BINDING_CNF, - json_string(OIDC_CLAIM_CNF_TBH)); - } - if (cfg->default_slo_url != NULL) { json_object_set_new(data, OIDC_METADATA_POST_LOGOUT_REDIRECT_URIS, json_pack("[s]", @@ -667,8 +660,7 @@ static apr_byte_t oidc_metadata_jwks_retrieve_and_cache(request_rec *r, } // TODO: add issuer? - if (oidc_proto_validate_jwt(r, jwt, NULL, TRUE, FALSE, -1, - OIDC_TOKEN_BINDING_POLICY_DISABLED) == FALSE) + if (oidc_proto_validate_jwt(r, jwt, NULL, TRUE, FALSE, -1) == FALSE) return FALSE; oidc_debug(r, "successfully verified and validated JWKs JWT"); @@ -1417,18 +1409,6 @@ apr_byte_t oidc_metadata_conf_parse(request_rec *r, oidc_cfg *cfg, else provider->userinfo_token_method = OIDC_USER_INFO_TOKEN_METHOD_HEADER; - /* see if we've got a custom token binding policy */ - char *policy = NULL; - oidc_metadata_get_valid_string(r, j_conf, - OIDC_METADATA_TOKEN_BINDING_POLICY, oidc_valid_token_binding_policy, - &policy, - NULL); - if (policy != NULL) - oidc_parse_token_binding_policy(r->pool, policy, - &provider->token_binding_policy); - else - provider->token_binding_policy = cfg->provider.token_binding_policy; - /* see if we've got a custom HTTP method for passing the auth request */ oidc_metadata_get_valid_string(r, j_conf, OIDC_METADATA_AUTH_REQUEST_METHOD, oidc_valid_auth_request_method, &method, diff --git a/src/mod_auth_openidc.c b/src/mod_auth_openidc.c index 27624b81..eda139d3 100644 --- a/src/mod_auth_openidc.c +++ b/src/mod_auth_openidc.c @@ -243,14 +243,6 @@ static char* oidc_get_browser_state_hash(request_rec *r, oidc_cfg *c, /* concat the nonce parameter to the hash input */ apr_sha1_update(&sha1, nonce, _oidc_strlen(nonce)); - /* concat the token binding ID if present */ - value = oidc_util_get_provided_token_binding_id(r); - if (value != NULL) { - oidc_debug(r, - "Provided Token Binding ID environment variable found; adding its value to the state"); - apr_sha1_update(&sha1, value, _oidc_strlen(value)); - } - /* finalize the hash input and calculate the resulting hash output */ unsigned char hash[OIDC_SHA1_LEN]; apr_sha1_final(hash, &sha1); @@ -3431,12 +3423,9 @@ static int oidc_handle_logout_backchannel(request_rec *r, oidc_cfg *cfg) { goto out; } - // oidc_proto_validate_idtoken would try and require a token binding cnf - // if the policy is set to "required", so don't use that here if (oidc_proto_validate_jwt(r, jwt, provider->validate_issuer ? provider->issuer : NULL, FALSE, FALSE, - provider->idtoken_iat_slack, - OIDC_TOKEN_BINDING_POLICY_DISABLED) == FALSE) + provider->idtoken_iat_slack) == FALSE) goto out; /* verify the "aud" and "azp" values */ diff --git a/src/mod_auth_openidc.h b/src/mod_auth_openidc.h index a2135db9..f49081aa 100644 --- a/src/mod_auth_openidc.h +++ b/src/mod_auth_openidc.h @@ -267,11 +267,6 @@ APLOG_USE_MODULE(auth_openidc); /* https://www.ietf.org/id/draft-ietf-oauth-mtls-12 */ #define OIDC_TB_CFG_FINGERPRINT_ENV_VAR "TB_SSL_CLIENT_CERT_FINGERPRINT" -#define OIDC_TOKEN_BINDING_POLICY_DISABLED 0 -#define OIDC_TOKEN_BINDING_POLICY_OPTIONAL 1 -#define OIDC_TOKEN_BINDING_POLICY_REQUIRED 2 -#define OIDC_TOKEN_BINDING_POLICY_ENFORCED 3 - #define OIDC_STATE_INPUT_HEADERS_USER_AGENT 1 #define OIDC_STATE_INPUT_HEADERS_X_FORWARDED_FOR 2 @@ -293,7 +288,6 @@ typedef struct oidc_proto_pkce_t { extern oidc_proto_pkce_t oidc_pkce_plain; extern oidc_proto_pkce_t oidc_pkce_s256; -extern oidc_proto_pkce_t oidc_pkce_referred_tb; typedef struct oidc_jwks_uri_t { char *uri; @@ -350,7 +344,6 @@ typedef struct oidc_provider_t { int userinfo_token_method; char *request_object; int auth_request_method; - int token_binding_policy; int issuer_specific_redirect_uri; } oidc_provider_t ; @@ -389,7 +382,6 @@ typedef struct oidc_oauth_t { apr_hash_t *verify_shared_keys; char *verify_jwks_uri; apr_array_header_t *verify_public_keys; - int access_token_binding_policy; } oidc_oauth_t; typedef struct oidc_outgoing_proxy_t { @@ -637,9 +629,6 @@ apr_byte_t oidc_oauth_get_bearer_token(request_rec *r, const char **access_token #define OIDC_CLAIM_C_HASH "c_hash" #define OIDC_CLAIM_RFP "rfp" #define OIDC_CLAIM_TARGET_LINK_URI "target_link_uri" -#define OIDC_CLAIM_CNF "cnf" -#define OIDC_CLAIM_CNF_TBH "tbh" -#define OIDC_CLAIM_CNF_X5T_S256 "x5t#S256" #define OIDC_CLAIM_SID "sid" #define OIDC_CLAIM_EVENTS "events" @@ -747,7 +736,7 @@ apr_array_header_t *oidc_proto_supported_flows(apr_pool_t *pool); apr_byte_t oidc_proto_flow_is_supported(apr_pool_t *pool, const char *flow); apr_byte_t oidc_proto_validate_authorization_response(request_rec *r, const char *response_type, const char *requested_response_mode, char **code, char **id_token, char **access_token, char **token_type, const char *used_response_mode); apr_byte_t oidc_proto_jwt_verify(request_rec *r, oidc_cfg *cfg, oidc_jwt_t *jwt, const oidc_jwks_uri_t *jwks_uri, int ssl_validate_server, apr_hash_t *symmetric_keys, const char *alg); -apr_byte_t oidc_proto_validate_jwt(request_rec *r, oidc_jwt_t *jwt, const char *iss, apr_byte_t exp_is_mandatory, apr_byte_t iat_is_mandatory, int iat_slack, int token_binding_policy); +apr_byte_t oidc_proto_validate_jwt(request_rec *r, oidc_jwt_t *jwt, const char *iss, apr_byte_t exp_is_mandatory, apr_byte_t iat_is_mandatory, int iat_slack); apr_byte_t oidc_proto_generate_nonce(request_rec *r, char **nonce, int len); apr_byte_t oidc_proto_validate_aud_and_azp(request_rec *r, oidc_cfg *cfg, oidc_provider_t *provider, oidc_jwt_payload_t *id_token_payload); @@ -903,7 +892,6 @@ char *oidc_util_get_chunked_cookie(request_rec *r, const char *cookieName, int c void oidc_util_set_chunked_cookie(request_rec *r, const char *cookieName, const char *cookieValue, apr_time_t expires, int chunkSize, const char *ext); apr_byte_t oidc_util_create_symmetric_key(request_rec *r, const char *client_secret, unsigned int r_key_len, const char *hash_algo, apr_byte_t set_kid, oidc_jwk_t **jwk); apr_hash_t * oidc_util_merge_symmetric_key(apr_pool_t *pool, const apr_array_header_t *keys, oidc_jwk_t *jwk); -const char *oidc_util_get_provided_token_binding_id(const request_rec *r); char *oidc_util_http_query_encoded_url(request_rec *r, const char *url, const apr_table_t *params); char *oidc_util_get_full_path(apr_pool_t *pool, const char *abs_or_rel_filename); apr_byte_t oidc_enabled(request_rec *r); @@ -938,7 +926,6 @@ const char *oidc_util_apr_expr_exec(request_rec *r, const oidc_apr_expr_t *expr, #define OIDC_HTTP_HDR_EXPIRES "Expires" #define OIDC_HTTP_HDR_X_FRAME_OPTIONS "X-Frame-Options" #define OIDC_HTTP_HDR_WWW_AUTHENTICATE "WWW-Authenticate" -#define OIDC_HTTP_HDR_INCLUDE_REFERRED_TOKEN_BINDING_ID "Include-Referred-Token-Binding-ID" #define OIDC_HTTP_HDR_VAL_XML_HTTP_REQUEST "XMLHttpRequest" #define OIDC_HTTP_HDR_VAL_NAVIGATE "navigate" @@ -965,7 +952,6 @@ void oidc_util_hdr_out_location_set(const request_rec *r, const char *value); const char *oidc_util_hdr_out_location_get(const request_rec *r); void oidc_util_hdr_err_out_add(const request_rec *r, const char *name, const char *value); apr_byte_t oidc_util_hdr_in_accept_contains(const request_rec *r, const char *needle); -apr_byte_t oidc_util_json_validate_cnf(request_rec *r, json_t *jwt, int token_binding_policy); apr_byte_t oidc_util_html_send_in_template(request_rec *r, const char *filename, char **static_template_content, const char *arg1, int arg1_esc, const char *arg2, int arg2_esc, int status_code); // oidc_metadata.c diff --git a/src/oauth.c b/src/oauth.c index 7684710d..1fc5b400 100644 --- a/src/oauth.c +++ b/src/oauth.c @@ -458,11 +458,6 @@ static apr_byte_t oidc_oauth_resolve_access_token(request_rec *r, oidc_cfg *c, if (oidc_util_decode_json_and_check_error(r, s_json, &result) == FALSE) return FALSE; - /* check the token binding ID in the introspection result */ - if (oidc_util_json_validate_cnf(r, result, - c->oauth.access_token_binding_policy) == FALSE) - return FALSE; - json_t *active = json_object_get(result, OIDC_PROTO_ACTIVE); apr_time_t cache_until = apr_time_now() + apr_time_from_sec(60); if (active != NULL) { @@ -601,8 +596,7 @@ static apr_byte_t oidc_oauth_validate_jwt_access_token(request_rec *r, * validate the access token JWT by validating the (optional) exp claim * don't enforce anything around iat since it doesn't make much sense for access tokens */ - if (oidc_proto_validate_jwt(r, jwt, NULL, FALSE, FALSE, -1, - c->oauth.access_token_binding_policy) == FALSE) { + if (oidc_proto_validate_jwt(r, jwt, NULL, FALSE, FALSE, -1) == FALSE) { oidc_jwt_destroy(jwt); return FALSE; } diff --git a/src/parse.c b/src/parse.c index ef28e837..398990a3 100644 --- a/src/parse.c +++ b/src/parse.c @@ -443,7 +443,6 @@ const char *oidc_valid_pkce_method(apr_pool_t *pool, const char *arg) { static char *options[] = { OIDC_PKCE_METHOD_PLAIN, OIDC_PKCE_METHOD_S256, - OIDC_PKCE_METHOD_REFERRED_TB, NULL }; return oidc_valid_string_option(pool, arg, options); } @@ -1236,57 +1235,6 @@ const char *oidc_parse_info_hook_data(apr_pool_t *pool, const char *arg, return NULL; } -#define OIDC_TOKEN_BINDING_POLICY_DISABLED_STR "disabled" -#define OIDC_TOKEN_BINDING_POLICY_OPTIONAL_STR "optional" -#define OIDC_TOKEN_BINDING_POLICY_REQUIRED_STR "required" -#define OIDC_TOKEN_BINDING_POLICY_ENFORCED_STR "enforced" - -const char *oidc_token_binding_policy2str(apr_pool_t *pool, int v) { - if (v == OIDC_TOKEN_BINDING_POLICY_DISABLED) - return OIDC_TOKEN_BINDING_POLICY_DISABLED; - if (v == OIDC_TOKEN_BINDING_POLICY_OPTIONAL) - return OIDC_TOKEN_BINDING_POLICY_OPTIONAL_STR; - if (v == OIDC_TOKEN_BINDING_POLICY_REQUIRED) - return OIDC_TOKEN_BINDING_POLICY_REQUIRED_STR; - if (v == OIDC_TOKEN_BINDING_POLICY_ENFORCED) - return OIDC_TOKEN_BINDING_POLICY_ENFORCED_STR; - return NULL; -} - -/* - * check token binding policy string value - */ -const char *oidc_valid_token_binding_policy(apr_pool_t *pool, const char *arg) { - static char *options[] = { - OIDC_TOKEN_BINDING_POLICY_DISABLED_STR, - OIDC_TOKEN_BINDING_POLICY_OPTIONAL_STR, - OIDC_TOKEN_BINDING_POLICY_REQUIRED_STR, - OIDC_TOKEN_BINDING_POLICY_ENFORCED_STR, - NULL }; - return oidc_valid_string_option(pool, arg, options); -} - -/* - * parse token binding policy - */ -const char *oidc_parse_token_binding_policy(apr_pool_t *pool, const char *arg, - int *policy) { - const char *rv = oidc_valid_token_binding_policy(pool, arg); - if (rv != NULL) - return rv; - - if (_oidc_strcmp(arg, OIDC_TOKEN_BINDING_POLICY_DISABLED_STR) == 0) - *policy = OIDC_TOKEN_BINDING_POLICY_DISABLED; - else if (_oidc_strcmp(arg, OIDC_TOKEN_BINDING_POLICY_OPTIONAL_STR) == 0) - *policy = OIDC_TOKEN_BINDING_POLICY_OPTIONAL; - else if (_oidc_strcmp(arg, OIDC_TOKEN_BINDING_POLICY_REQUIRED_STR) == 0) - *policy = OIDC_TOKEN_BINDING_POLICY_REQUIRED; - else if (_oidc_strcmp(arg, OIDC_TOKEN_BINDING_POLICY_ENFORCED_STR) == 0) - *policy = OIDC_TOKEN_BINDING_POLICY_ENFORCED; - - return NULL; -} - #define OIDC_AUTH_REQUEST_METHOD_GET_STR "GET" #define OIDC_AUTH_REQEUST_METHOD_POST_STR "POST" diff --git a/src/parse.h b/src/parse.h index 5c458aaf..2e779c18 100644 --- a/src/parse.h +++ b/src/parse.h @@ -62,7 +62,6 @@ #define OIDC_PKCE_METHOD_PLAIN "plain" #define OIDC_PKCE_METHOD_S256 "S256" -#define OIDC_PKCE_METHOD_REFERRED_TB "referred_tb" #define OIDC_ENDPOINT_AUTH_CLIENT_SECRET_BASIC "client_secret_basic" @@ -91,7 +90,6 @@ const char* oidc_valid_jwks_refresh_interval(apr_pool_t *pool, int v); const char* oidc_valid_idtoken_iat_slack(apr_pool_t *pool, int v); const char* oidc_valid_userinfo_refresh_interval(apr_pool_t *pool, int v); const char* oidc_valid_userinfo_token_method(apr_pool_t *pool, const char *arg); -const char* oidc_valid_token_binding_policy(apr_pool_t *pool, const char *arg); const char* oidc_valid_auth_request_method(apr_pool_t *pool, const char *arg); const char* oidc_valid_max_number_of_state_cookies(apr_pool_t *pool, int v); @@ -144,9 +142,6 @@ const char* oidc_parse_userinfo_token_method(apr_pool_t *pool, const char *arg, int *int_value); const char* oidc_parse_info_hook_data(apr_pool_t *pool, const char *arg, apr_hash_t **hook_data); -const char* oidc_parse_token_binding_policy(apr_pool_t *pool, const char *arg, - int *int_value); -const char* oidc_token_binding_policy2str(apr_pool_t *pool, int v); const char* oidc_parse_auth_request_method(apr_pool_t *pool, const char *arg, int *method); const char* oidc_parse_max_number_of_state_cookies(apr_pool_t *pool, diff --git a/src/proto.c b/src/proto.c index b51348f2..9765346a 100644 --- a/src/proto.c +++ b/src/proto.c @@ -771,12 +771,6 @@ int oidc_proto_authorization_request(request_rec *r, return HTTP_INTERNAL_SERVER_ERROR; } - /* add a referred token binding request for the provider if enabled */ - if ((provider->token_binding_policy > OIDC_TOKEN_BINDING_POLICY_DISABLED) - && (oidc_util_get_provided_token_binding_id(r) != NULL)) - oidc_util_hdr_err_out_add(r, - OIDC_HTTP_HDR_INCLUDE_REFERRED_TOKEN_BINDING_ID, "true"); - /* cleanup */ oidc_proto_state_destroy(proto_state); @@ -877,35 +871,6 @@ static apr_byte_t oidc_proto_pkce_verifier_s256(request_rec *r, return TRUE; } -/* - * PCKE "referred_tb" proto state - */ -static apr_byte_t oidc_proto_pkce_state_referred_tb(request_rec *r, - char **state) { - *state = NULL; - return TRUE; -} - -/* - * PCKE "referred_tb" code_challenge - */ -static apr_byte_t oidc_proto_pkce_challenge_referred_tb(request_rec *r, - const char *state, char **code_challenge) { - // state should be NULL - *code_challenge = OIDC_PKCE_METHOD_REFERRED_TB; - return TRUE; -} - -/* - * PCKE "referred_tb" code_verifier - */ -static apr_byte_t oidc_proto_pkce_verifier_referred_tb(request_rec *r, - const char *state, char **code_verifier) { - const char *tb_id = oidc_util_get_provided_token_binding_id(r); - *code_verifier = tb_id ? apr_pstrdup(r->pool, tb_id) : NULL; - return TRUE; -} - /* * PKCE plain */ @@ -926,15 +891,6 @@ oidc_proto_pkce_t oidc_pkce_s256 = { oidc_proto_pkce_challenge_s256 }; -/* - * PKCE referred_tb - */ -oidc_proto_pkce_t oidc_pkce_referred_tb = { - OIDC_PKCE_METHOD_REFERRED_TB, - oidc_proto_pkce_state_referred_tb, - oidc_proto_pkce_verifier_referred_tb, - oidc_proto_pkce_challenge_referred_tb }; - #define OIDC_PROTO_STATE_ISSUER "i" #define OIDC_PROTO_STATE_ORIGINAL_URL "ou" #define OIDC_PROTO_STATE_ORIGINAL_METHOD "om" @@ -1322,7 +1278,7 @@ static apr_byte_t oidc_proto_validate_exp(request_rec *r, oidc_jwt_t *jwt, */ apr_byte_t oidc_proto_validate_jwt(request_rec *r, oidc_jwt_t *jwt, const char *iss, apr_byte_t exp_is_mandatory, - apr_byte_t iat_is_mandatory, int iat_slack, int token_binding_policy) { + apr_byte_t iat_is_mandatory, int iat_slack) { if (iss != NULL) { @@ -1351,11 +1307,6 @@ apr_byte_t oidc_proto_validate_jwt(request_rec *r, oidc_jwt_t *jwt, if (oidc_proto_validate_iat(r, jwt, iat_is_mandatory, iat_slack) == FALSE) return FALSE; - /* check the token binding ID in the JWT */ - if (oidc_util_json_validate_cnf(r, jwt->payload.value.json, - token_binding_policy) == FALSE) - return FALSE; - return TRUE; } @@ -1381,8 +1332,7 @@ static apr_byte_t oidc_proto_validate_idtoken(request_rec *r, /* validate the ID Token JWT, requiring iss match, and valid exp + iat */ if (oidc_proto_validate_jwt(r, jwt, provider->validate_issuer ? provider->issuer : NULL, TRUE, TRUE, - provider->idtoken_iat_slack, - provider->token_binding_policy) == FALSE) + provider->idtoken_iat_slack) == FALSE) return FALSE; /* check if the required-by-spec "sub" claim is present */ diff --git a/src/session.c b/src/session.c index 0774f28a..78b25a39 100644 --- a/src/session.c +++ b/src/session.c @@ -49,8 +49,6 @@ extern module AP_MODULE_DECLARE_DATA auth_openidc_module; #define OIDC_SESSION_REMOTE_USER_KEY "r" /* the name of the session expiry attribute in the session */ #define OIDC_SESSION_EXPIRY_KEY "e" -/* the name of the provided token binding attribute in the session */ -#define OIDC_SESSION_PROVIDED_TOKEN_BINDING_KEY "ptb" /* the name of the session identifier in the session */ #define OIDC_SESSION_SESSION_ID "i" /* the name of the sub attribute in the session */ @@ -291,19 +289,6 @@ apr_byte_t oidc_session_extract(request_rec *r, oidc_session_t *z) { goto out; } - oidc_session_get(r, z, OIDC_SESSION_PROVIDED_TOKEN_BINDING_KEY, - &ses_p_tb_id); - - if (ses_p_tb_id != NULL) { - env_p_tb_id = oidc_util_get_provided_token_binding_id(r); - if ((env_p_tb_id == NULL) - || (_oidc_strcmp(env_p_tb_id, ses_p_tb_id) != 0)) { - oidc_error(r, - "the Provided Token Binding ID stored in the session doesn't match the one presented by the user agent"); - oidc_session_clear(r, z); - } - } - oidc_session_get(r, z, OIDC_SESSION_REMOTE_USER_KEY, &z->remote_user); oidc_session_get(r, z, OIDC_SESSION_SID_KEY, &z->sid); @@ -353,19 +338,11 @@ apr_byte_t oidc_session_save(request_rec *r, oidc_session_t *z, &auth_openidc_module); apr_byte_t rc = FALSE; - const char *p_tb_id = oidc_util_get_provided_token_binding_id(r); if (z->state != NULL) { oidc_session_set(r, z, OIDC_SESSION_REMOTE_USER_KEY, z->remote_user); json_object_set_new(z->state, OIDC_SESSION_EXPIRY_KEY, json_integer(apr_time_sec(z->expiry))); - - if ((first_time) && (p_tb_id != NULL)) { - oidc_debug(r, - "Provided Token Binding ID environment variable found; adding its value to the session state"); - oidc_session_set(r, z, OIDC_SESSION_PROVIDED_TOKEN_BINDING_KEY, - p_tb_id); - } } if (c->session_type == OIDC_SESSION_TYPE_SERVER_CACHE) diff --git a/src/util.c b/src/util.c index 2266d836..7fe6be28 100644 --- a/src/util.c +++ b/src/util.c @@ -3019,177 +3019,6 @@ const char* oidc_util_hdr_out_location_get(const request_rec *r) { return oidc_util_hdr_out_get(r, OIDC_HTTP_HDR_LOCATION); } -const char* oidc_util_get_provided_token_binding_id(const request_rec *r) { - const char *result = NULL; - if (r->subprocess_env != NULL) - result = apr_table_get(r->subprocess_env, OIDC_TB_CFG_PROVIDED_ENV_VAR); - return result; -} - -const char* oidc_util_get_client_cert_fingerprint(request_rec *r) { - const char *fingerprint = NULL; - - if (r->subprocess_env == NULL) - goto end; - - fingerprint = apr_table_get(r->subprocess_env, - OIDC_TB_CFG_FINGERPRINT_ENV_VAR); - if (fingerprint == NULL) { - oidc_debug(r, "no %s environment variable found", - OIDC_TB_CFG_FINGERPRINT_ENV_VAR); - goto end; - } - -end: - - return fingerprint; -} - -apr_byte_t oidc_util_json_validate_cnf_tbh(request_rec *r, - int token_binding_policy, const char *tbh_str) { - const char *tbp_str = NULL; - char *tbp = NULL; - int tbp_len = -1; - unsigned char *tbp_hash = NULL; - unsigned int tbp_hash_len = -1; - char *tbh = NULL; - int tbh_len = -1; - - tbp_str = oidc_util_get_provided_token_binding_id(r); - if (tbp_str == NULL) { - oidc_debug(r, - "no Provided Token Binding ID environment variable found"); - goto out_err; - } - - tbp_len = oidc_base64url_decode(r->pool, &tbp, tbp_str); - if (tbp_len <= 0) { - oidc_warn(r, - "Provided Token Binding ID environment variable could not be decoded"); - goto out_err; - } - - if (oidc_jose_hash_bytes(r->pool, OIDC_JOSE_ALG_SHA256, - (const unsigned char*) tbp, tbp_len, &tbp_hash, &tbp_hash_len, - NULL) == FALSE) { - oidc_warn(r, - "hashing Provided Token Binding ID environment variable failed"); - goto out_err; - } - - tbh_len = oidc_base64url_decode(r->pool, &tbh, tbh_str); - if (tbh_len <= 0) { - oidc_warn(r, "cnf[\"tbh\"] provided but it could not be decoded"); - goto out_err; - } - - if (tbp_hash_len != tbh_len) { - oidc_warn(r, - "hash length of provided token binding ID environment variable: %d does not match length of cnf[\"tbh\"]: %d", - tbp_hash_len, tbh_len); - goto out_err; - } - - if (memcmp(tbp_hash, tbh, tbh_len) != 0) { - oidc_warn(r, - "hash of provided token binding ID environment variable does not match cnf[\"tbh\"]"); - goto out_err; - } - - oidc_debug(r, - "hash of provided token binding ID environment variable matches cnf[\"tbh\"]"); - - return TRUE; - -out_err: - - if (token_binding_policy == OIDC_TOKEN_BINDING_POLICY_OPTIONAL) - return TRUE; - if (token_binding_policy == OIDC_TOKEN_BINDING_POLICY_ENFORCED) - return FALSE; - - // token_binding_policy == OIDC_TOKEN_BINDING_POLICY_REQURIED - return (tbp_str == NULL); -} - -apr_byte_t oidc_util_json_validate_cnf_x5t_s256(request_rec *r, - int token_binding_policy, const char *x5t_256_str) { - const char *fingerprint = NULL; - - fingerprint = oidc_util_get_client_cert_fingerprint(r); - if (fingerprint == NULL) { - oidc_debug(r, "no certificate (fingerprint) provided"); - goto out_err; - } - - if (_oidc_strcmp(fingerprint, x5t_256_str) != 0) { - oidc_warn(r, - "fingerprint of provided cert (%s) does not match cnf[\"x5t#S256\"] (%s)", - fingerprint, x5t_256_str); - goto out_err; - } - - oidc_debug(r, "fingerprint of provided cert (%s) matches cnf[\"x5t#S256\"]", - fingerprint); - - return TRUE; - - out_err: - - if (token_binding_policy == OIDC_TOKEN_BINDING_POLICY_OPTIONAL) - return TRUE; - if (token_binding_policy == OIDC_TOKEN_BINDING_POLICY_ENFORCED) - return FALSE; - - // token_binding_policy == OIDC_TOKEN_BINDING_POLICY_REQURIED - return (fingerprint == NULL); -} - -/* - * validate the "cnf" claim in a JWT payload - */ -apr_byte_t oidc_util_json_validate_cnf(request_rec *r, json_t *jwt, - int token_binding_policy) { - char *tbh_str = NULL; - - oidc_debug(r, "enter: policy=%s", - oidc_token_binding_policy2str(r->pool, token_binding_policy)); - - if (token_binding_policy == OIDC_TOKEN_BINDING_POLICY_DISABLED) - return TRUE; - - json_t *cnf = json_object_get(jwt, OIDC_CLAIM_CNF); - if (cnf == NULL) { - oidc_debug(r, "no \"%s\" claim found in the token", OIDC_CLAIM_CNF); - goto out_err; - } - - oidc_jose_get_string(r->pool, cnf, OIDC_CLAIM_CNF_TBH, FALSE, &tbh_str, - NULL); - if (tbh_str != NULL) - return oidc_util_json_validate_cnf_tbh(r, token_binding_policy, tbh_str); - - oidc_jose_get_string(r->pool, cnf, OIDC_CLAIM_CNF_X5T_S256, FALSE, &tbh_str, - NULL); - if (tbh_str != NULL) - return oidc_util_json_validate_cnf_x5t_s256(r, token_binding_policy, - tbh_str); - - oidc_debug(r, - " \"%s\" claim found in the token but no \"%s\" or \"%s\" key found inside", - OIDC_CLAIM_CNF, OIDC_CLAIM_CNF_TBH, OIDC_CLAIM_CNF_X5T_S256); - -out_err: - - if (token_binding_policy == OIDC_TOKEN_BINDING_POLICY_OPTIONAL) - return TRUE; - //if (token_binding_policy == OIDC_TOKEN_BINDING_POLICY_ENFORCED) - // return FALSE; - // token_binding_policy == OIDC_TOKEN_BINDING_POLICY_REQUIRED - // TODO: we don't know which token binding the client supports, do we ? - return FALSE; -} - oidc_jwk_t* oidc_util_key_list_first(const apr_array_header_t *key_list, int kty, const char *use) { oidc_jwk_t *rv = NULL; diff --git a/test/test.c b/test/test.c index fc2799ac..4c6584a1 100755 --- a/test/test.c +++ b/test/test.c @@ -1164,7 +1164,6 @@ static char * test_proto_authorization_request(request_rec *r) { provider.response_type = "code"; provider.auth_request_params = "jan=piet&foo=#"; provider.request_object = NULL; - provider.token_binding_policy = OIDC_TOKEN_BINDING_POLICY_OPTIONAL; provider.auth_request_method = OIDC_AUTH_REQUEST_METHOD_GET; const char *redirect_uri = "https://www.example.com/protected/"; @@ -1323,8 +1322,8 @@ static char * test_proto_validate_jwt(request_rec *r) { r->pool, err); TST_ASSERT_ERR("oidc_proto_validate_jwt", - oidc_proto_validate_jwt(r, jwt, s_issuer, TRUE, TRUE, 10, OIDC_TOKEN_BINDING_POLICY_DISABLED), - r->pool, err); + oidc_proto_validate_jwt(r, jwt, s_issuer, TRUE, TRUE, 10), r->pool, + err); oidc_jwk_destroy(jwk); oidc_jwt_destroy(jwt); @@ -1948,7 +1947,6 @@ static request_rec * test_setup(apr_pool_t *pool) { "https://idp.example.com/authorize"; cfg->provider.scope = "openid"; cfg->provider.client_id = "client_id"; - cfg->provider.token_binding_policy = OIDC_TOKEN_BINDING_POLICY_OPTIONAL; cfg->redirect_uri = "https://www.example.com/protected/"; oidc_dir_cfg *d_cfg = oidc_create_dir_config(request->pool, NULL);