use PKCE S256 by default

disable by configuring "OIDCPKCEMethod none" Signed-off-by: Hans Zandbelt <[email protected]>
OpenIDC · Oct 31, 2023 · c5a72a6 · c5a72a6
1 parent f127c0f
commit c5a72a6
Show file tree

Hide file tree

Showing 6 changed files with 11 additions and 6 deletions.
diff --git 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
@@ -297,9 +297,9 @@
 #OIDCClientContact <contact>
 
 # 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
@@ -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
@@ -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
@@ -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
@@ -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);