From c5a72a6d3495ad59adb25a025d736cc2c70d5255 Mon Sep 17 00:00:00 2001 From: Hans Zandbelt Date: Tue, 31 Oct 2023 21:41:22 +0100 Subject: [PATCH] use PKCE S256 by default disable by configuring "OIDCPKCEMethod none" Signed-off-by: Hans Zandbelt --- ChangeLog | 1 + auth_openidc.conf | 4 ++-- src/config.c | 8 +++++--- src/parse.c | 1 + src/parse.h | 1 + src/proto.c | 2 +- 6 files changed, 11 insertions(+), 6 deletions(-) diff --git a/ChangeLog b/ChangeLog index c5c360d4..5aa2275b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -4,6 +4,7 @@ - remove obsolete support for Token Binding https://www.rfc-editor.org/rfc/rfc8471.html (id_token, access_token, session cookie) - use only the User-Agent header as input for the state browser fingerprinting by default (no X-Forwarded-For) as cloud environments increasingly use dynamic proxy IPs in front +- use PKCE S256 by default 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 1ec49743..9738e1f3 100644 --- a/auth_openidc.conf +++ b/auth_openidc.conf @@ -297,9 +297,9 @@ #OIDCClientContact # The PKCE method used (this serves as default value for discovered OPs too) -# When not defined PKCE is not used. +# When not defined S256 is used. # NB: this can be overridden on a per-OP basis in the .conf file using the key: pkce_method -#OIDCPKCEMethod [plain|S256|referred_tb] +#OIDCPKCEMethod [plain|S256|none] # (used only in dynamic client registration) # Define the Client JWKs URL (e.g. https://localhost/protected/?jwks=rsa)") that will be diff --git a/src/config.c b/src/config.c index 7851e1ee..568ccb76 100644 --- a/src/config.c +++ b/src/config.c @@ -771,6 +771,8 @@ 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_NONE) == 0) { + *type = NULL; } return NULL; @@ -1565,7 +1567,7 @@ static void oidc_cfg_provider_init(oidc_provider_t *provider) { provider->session_max_duration = OIDC_DEFAULT_SESSION_MAX_DURATION; provider->auth_request_params = NULL; provider->logout_request_params = NULL; - provider->pkce = NULL; + provider->pkce = &oidc_pkce_s256; provider->client_jwks_uri = NULL; provider->client_keys = NULL; @@ -1702,7 +1704,7 @@ static void oidc_merge_provider_config(apr_pool_t *pool, oidc_provider_t *dst, dst->logout_request_params = add->logout_request_params != NULL ? add->logout_request_params : base->logout_request_params; - dst->pkce = add->pkce != NULL ? add->pkce : base->pkce; + dst->pkce = add->pkce != &oidc_pkce_s256 ? add->pkce : base->pkce; dst->client_jwks_uri = add->client_jwks_uri != NULL ? @@ -3422,7 +3424,7 @@ const command_rec oidc_config_cmds[] = { oidc_set_pkce_method, (void *)APR_OFFSETOF(oidc_cfg, provider.pkce), RSRC_CONF, - "The RFC 7636 PCKE mode used; must be one of \"plain\", \"S256\" or \"referred_tb\""), + "The RFC 7636 PCKE mode used; must be one of \"plain\" or \"S256\""), AP_INIT_TAKE1(OIDCClientID, oidc_set_string_slot, diff --git a/src/parse.c b/src/parse.c index 398990a3..f8e96f0b 100644 --- a/src/parse.c +++ b/src/parse.c @@ -443,6 +443,7 @@ 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_NONE, NULL }; return oidc_valid_string_option(pool, arg, options); } diff --git a/src/parse.h b/src/parse.h index 2e779c18..591b27cd 100644 --- a/src/parse.h +++ b/src/parse.h @@ -62,6 +62,7 @@ #define OIDC_PKCE_METHOD_PLAIN "plain" #define OIDC_PKCE_METHOD_S256 "S256" +#define OIDC_PKCE_METHOD_NONE "none" #define OIDC_ENDPOINT_AUTH_CLIENT_SECRET_BASIC "client_secret_basic" diff --git a/src/proto.c b/src/proto.c index 9765346a..72f54706 100644 --- a/src/proto.c +++ b/src/proto.c @@ -698,7 +698,7 @@ int oidc_proto_authorization_request(request_rec *r, apr_table_setn(params, OIDC_PROTO_NONCE, nonce); /* add PKCE code challenge if set */ - if (code_challenge != NULL) { + if ((code_challenge != NULL) && (provider->pkce != NULL)) { apr_table_setn(params, OIDC_PROTO_CODE_CHALLENGE, code_challenge); apr_table_setn(params, OIDC_PROTO_CODE_CHALLENGE_METHOD, provider->pkce->method);