From 6ae48480b5a0646cb10f3e8aa63f3841fc2e94ae Mon Sep 17 00:00:00 2001 From: Hans Zandbelt Date: Thu, 29 Aug 2024 18:36:06 +0200 Subject: [PATCH] fix setting OIDCPKCEMethod none; closes #1256; thanks @eoliphan Signed-off-by: Hans Zandbelt --- ChangeLog | 3 +++ src/cfg/provider.c | 2 +- src/handle/request.c | 2 +- src/metadata.c | 4 +--- src/proto/pkce.c | 5 +++++ src/proto/proto.h | 1 + src/proto/request.c | 2 +- src/proto/response.c | 2 +- 8 files changed, 14 insertions(+), 7 deletions(-) diff --git a/ChangeLog b/ChangeLog index e0fe56de..f2887a55 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,6 @@ +08/29/2024 +- fix setting OIDCPKCEMethod none; closes #1256; thanks @eoliphan + 08/28/2024 - re-introduce OIDCSessionMaxDuration 0; see #1252 - bump to 2.4.16.2dev diff --git a/src/cfg/provider.c b/src/cfg/provider.c index 9937a863..9f445c16 100644 --- a/src/cfg/provider.c +++ b/src/cfg/provider.c @@ -254,7 +254,7 @@ const char *oidc_cfg_provider_pkce_set(apr_pool_t *pool, oidc_provider_t *provid provider->pkce = &oidc_pkce_s256; return NULL; } else if (_oidc_strcmp(arg, OIDC_PKCE_METHOD_NONE) == 0) { - provider->pkce = NULL; + provider->pkce = &oidc_pkce_none; return NULL; } return oidc_cfg_parse_is_valid_option(pool, arg, options); diff --git a/src/handle/request.c b/src/handle/request.c index 989dc5b9..90f18526 100644 --- a/src/handle/request.c +++ b/src/handle/request.c @@ -186,7 +186,7 @@ int oidc_request_authenticate_user(request_rec *r, oidc_cfg_t *c, oidc_provider_ if ((oidc_util_spaced_string_contains(r->pool, oidc_cfg_provider_response_type_get(provider), OIDC_PROTO_CODE) == TRUE) && - (oidc_cfg_provider_pkce_get(provider) != NULL)) { + (oidc_cfg_provider_pkce_get(provider) != &oidc_pkce_none)) { /* generate the code verifier value that correlates authorization requests and code exchange requests */ if (oidc_cfg_provider_pkce_get(provider)->state(r, &pkce_state) == FALSE) diff --git a/src/metadata.c b/src/metadata.c index ce69c733..5ff3c9e0 100644 --- a/src/metadata.c +++ b/src/metadata.c @@ -1334,9 +1334,7 @@ apr_byte_t oidc_metadata_conf_parse(request_rec *r, oidc_cfg_t *cfg, json_t *j_c /* get the PKCE method to use */ /* NB: pass "none" rather than NULL to allow fallback when the default in base is explicitly set to "none" */ oidc_util_json_object_get_string(r->pool, j_conf, OIDC_METADATA_PKCE_METHOD, &value, - oidc_cfg_provider_pkce_get(oidc_cfg_provider_get(cfg)) - ? oidc_cfg_provider_pkce_get(oidc_cfg_provider_get(cfg))->method - : OIDC_PKCE_METHOD_NONE); + oidc_cfg_provider_pkce_get(oidc_cfg_provider_get(cfg))->method); OIDC_METADATA_PROVIDER_SET(pkce, value, rv) /* see if we've got a custom DPoP mode */ diff --git a/src/proto/pkce.c b/src/proto/pkce.c index 57add344..cf7dc221 100644 --- a/src/proto/pkce.c +++ b/src/proto/pkce.c @@ -103,3 +103,8 @@ oidc_proto_pkce_t oidc_pkce_plain = {OIDC_PKCE_METHOD_PLAIN, oidc_proto_pkce_sta */ oidc_proto_pkce_t oidc_pkce_s256 = {OIDC_PKCE_METHOD_S256, oidc_proto_pkce_state_s256, oidc_proto_pkce_verifier_s256, oidc_proto_pkce_challenge_s256}; + +/* + * PKCE none + */ +oidc_proto_pkce_t oidc_pkce_none = {OIDC_PKCE_METHOD_NONE, NULL, NULL, NULL}; diff --git a/src/proto/proto.h b/src/proto/proto.h index ad7a3a74..f3a9f471 100644 --- a/src/proto/proto.h +++ b/src/proto/proto.h @@ -178,6 +178,7 @@ apr_byte_t oidc_proto_jwt_sign_and_serialize(request_rec *r, oidc_jwk_t *jwk, oi extern oidc_proto_pkce_t oidc_pkce_plain; extern oidc_proto_pkce_t oidc_pkce_s256; +extern oidc_proto_pkce_t oidc_pkce_none; const char *oidc_proto_state_get_pkce_state(oidc_proto_state_t *proto_state); void oidc_proto_state_set_pkce_state(oidc_proto_state_t *proto_state, const char *pkce_state); diff --git a/src/proto/request.c b/src/proto/request.c index 08ec8c34..3aac38a7 100644 --- a/src/proto/request.c +++ b/src/proto/request.c @@ -650,7 +650,7 @@ int oidc_proto_request_auth(request_rec *r, struct oidc_provider_t *provider, co apr_table_setn(params, OIDC_PROTO_NONCE, nonce); /* add PKCE code challenge if set */ - if ((code_challenge != NULL) && (oidc_cfg_provider_pkce_get(provider) != NULL)) { + if ((code_challenge != NULL) && (oidc_cfg_provider_pkce_get(provider) != &oidc_pkce_none)) { apr_table_setn(params, OIDC_PROTO_CODE_CHALLENGE, code_challenge); apr_table_setn(params, OIDC_PROTO_CODE_CHALLENGE_METHOD, oidc_cfg_provider_pkce_get(provider)->method); } diff --git a/src/proto/response.c b/src/proto/response.c index 99fbc96e..2f29582d 100644 --- a/src/proto/response.c +++ b/src/proto/response.c @@ -327,7 +327,7 @@ static apr_byte_t oidc_proto_resolve_code_and_validate_response(request_rec *r, char *refresh_token = NULL; char *code_verifier = NULL; - if (oidc_cfg_provider_pkce_get(provider) != NULL) + if (oidc_cfg_provider_pkce_get(provider) != &oidc_pkce_none) oidc_cfg_provider_pkce_get(provider)->verifier(r, oidc_proto_state_get_pkce_state(proto_state), &code_verifier);