From 5a1f90847efce160631ee2a16d7b9d1da3496616 Mon Sep 17 00:00:00 2001

From: Hiroyuki Wada <h2-wada@nri.co.jp>

Date: Tue, 21 Apr 2020 20:29:25 +0900

Subject: [PATCH 13/13] Allow configuring which header value is used to

 calculate the fingerprint of the state during authentication

(cherry picked from commit 2a97eced569ebbff82fc0370c4f741d04ba7cb13)

---

 auth_openidc.conf      |  4 ++++

 src/config.c           | 25 +++++++++++++++++++++++++

 src/mod_auth_openidc.c | 30 +++++++++++++++++-------------

 src/mod_auth_openidc.h |  5 +++++

 src/parse.c            | 33 +++++++++++++++++++++++++++++++++

 src/parse.h            |  1 +

 6 files changed, 85 insertions(+), 13 deletions(-)

diff --git a/auth_openidc.conf b/auth_openidc.conf

index 33cea64..4012df3 100644

--- a/auth_openidc.conf

+++ b/auth_openidc.conf

@@ -774,3 +774,7 @@

 # When not defined no claims are whitelisted and all claims are stored except when blacklisted with OIDCBlackListedClaims.

 #OIDCWhiteListedClaims [<claim>]+

+# Defines whether the value of the User-Agent and X-Forwarded-For headers will be used as the input

+# for calculating the fingerprint of the state during authentication.

+# When not defined the default "both" is used.

+#OIDCStateInputHeaders [none|user-agent|x-forwarded-for|both]

diff --git a/src/config.c b/src/config.c

index 8e56716..588e1a3 100644

--- a/src/config.c

+++ b/src/config.c

@@ -166,6 +166,8 @@

 #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 */

 #define OIDC_DEFAULT_PROVIDER_ISSUER_SPECIFIC_REDIRECT_URI 0

+/* default setting for calculating the fingerprint of the state from request headers during authentication */

+#define OIDC_DEFAULT_STATE_INPUT_HEADERS OIDC_STATE_INPUT_HEADERS_USER_AGENT | OIDC_STATE_INPUT_HEADERS_X_FORWARDED_FOR

 #define OIDCProviderMetadataURL              "OIDCProviderMetadataURL"

 #define OIDCProviderIssuer                   "OIDCProviderIssuer"

@@ -261,6 +263,7 @@

 #define OIDCProviderAuthRequestMethod        "OIDCProviderAuthRequestMethod"

 #define OIDCBlackListedClaims                "OIDCBlackListedClaims"

 #define OIDCOAuthServerMetadataURL           "OIDCOAuthServerMetadataURL"

+#define OIDCStateInputHeaders                  "OIDCStateInputHeaders"

 extern module AP_MODULE_DECLARE_DATA auth_openidc_module;

@@ -1030,6 +1033,17 @@ int oidc_cfg_delete_oldest_state_cookies(oidc_cfg *cfg) {

 	return cfg->delete_oldest_state_cookies;

 }

+/*

+ * define which header we use for calculating the fingerprint of the state during authentication

+ */

+static const char * oidc_set_state_input_headers_as(cmd_parms *cmd, void *m,

+		const char *arg) {

+	oidc_cfg *cfg = (oidc_cfg *) ap_get_module_config(

+			cmd->server->module_config, &auth_openidc_module);

+	const char *rv = oidc_parse_set_state_input_headers_as(cmd->pool, arg, &cfg->state_input_headers);

+	return OIDC_CONFIG_DIR_RV(cmd, rv);

+}

+

 /*

  * create a new server config record with defaults

  */

@@ -1176,6 +1190,8 @@ void *oidc_create_server_config(apr_pool_t *pool, server_rec *svr) {

 	c->provider.issuer_specific_redirect_uri =

 			OIDC_DEFAULT_PROVIDER_ISSUER_SPECIFIC_REDIRECT_URI;

+	c->state_input_headers = OIDC_DEFAULT_STATE_INPUT_HEADERS;

+

 	return c;

 }

@@ -1617,6 +1633,10 @@ void *oidc_merge_server_config(apr_pool_t *pool, void *BASE, void *ADD) {

 					add->provider.issuer_specific_redirect_uri :

 					base->provider.issuer_specific_redirect_uri;

+	c->state_input_headers =

+			add->state_input_headers != OIDC_DEFAULT_STATE_INPUT_HEADERS ?

+					add->state_input_headers : base->state_input_headers;

+

 	return c;

 }

@@ -2866,5 +2886,10 @@ const command_rec oidc_config_cmds[] = {

 				(void*)APR_OFFSETOF(oidc_cfg, oauth.metadata_url),

 				RSRC_CONF,

 				"Authorization Server metadata URL."),

+		AP_INIT_TAKE123(OIDCStateInputHeaders,

+				oidc_set_state_input_headers_as,

+				NULL,

+				RSRC_CONF,

+				"Specify header name which is used as the input for calculating the fingerprint of the state during authentication; must be one of \"none\", \"user-agent\", \"x-forwarded-for\" or \"both\" (default)."),

 		{ NULL }

 };

diff --git a/src/mod_auth_openidc.c b/src/mod_auth_openidc.c

index 8740e02..38558d2 100644

--- a/src/mod_auth_openidc.c

+++ b/src/mod_auth_openidc.c

@@ -226,7 +226,7 @@ void oidc_strip_cookies(request_rec *r) {

 /*

  * calculates a hash value based on request fingerprint plus a provided nonce string.

  */

-static char *oidc_get_browser_state_hash(request_rec *r, const char *nonce) {

+static char *oidc_get_browser_state_hash(request_rec *r, oidc_cfg *c, const char *nonce) {

 	oidc_debug(r, "enter");

@@ -238,17 +238,21 @@ static char *oidc_get_browser_state_hash(request_rec *r, const char *nonce) {

 	/* Initialize the hash context */

 	apr_sha1_init(&sha1;;

-	/* get the X-FORWARDED-FOR header value  */

-	value = oidc_util_hdr_in_x_forwarded_for_get(r);

-	/* if we have a value for this header, concat it to the hash input */

-	if (value != NULL)

-		apr_sha1_update(&sha1, value, strlen(value));

+	if (c->state_input_headers & OIDC_STATE_INPUT_HEADERS_X_FORWARDED_FOR) {

+		/* get the X-FORWARDED-FOR header value  */

+		value = oidc_util_hdr_in_x_forwarded_for_get(r);

+		/* if we have a value for this header, concat it to the hash input */

+		if (value != NULL)

+			apr_sha1_update(&sha1, value, strlen(value));

+	}

-	/* get the USER-AGENT header value  */

-	value = oidc_util_hdr_in_user_agent_get(r);

-	/* if we have a value for this header, concat it to the hash input */

-	if (value != NULL)

-		apr_sha1_update(&sha1, value, strlen(value));

+	if (c->state_input_headers & OIDC_STATE_INPUT_HEADERS_USER_AGENT) {

+		/* get the USER-AGENT header value  */

+		value = oidc_util_hdr_in_user_agent_get(r);

+		/* if we have a value for this header, concat it to the hash input */

+		if (value != NULL)

+			apr_sha1_update(&sha1, value, strlen(value));

+	}

 	/* get the remote client IP address or host name */

 	/*

@@ -821,7 +825,7 @@ static apr_byte_t oidc_restore_proto_state(request_rec *r, oidc_cfg *c,

 	const char *nonce = oidc_proto_state_get_nonce(*proto_state);

 	/* calculate the hash of the browser fingerprint concatenated with the nonce */

-	char *calc = oidc_get_browser_state_hash(r, nonce);

+	char *calc = oidc_get_browser_state_hash(r, c, nonce);

 	/* compare the calculated hash with the value provided in the authorization response */

 	if (apr_strnatcmp(calc, state) != 0) {

 		oidc_error(r,

@@ -2345,7 +2349,7 @@ static int oidc_authenticate_user(request_rec *r, oidc_cfg *c,

 		oidc_proto_state_set_pkce_state(proto_state, pkce_state);

 	/* get a hash value that fingerprints the browser concatenated with the random input */

-	char *state = oidc_get_browser_state_hash(r, nonce);

+	char *state = oidc_get_browser_state_hash(r, c, nonce);

 	/*

 	 * create state that restores the context when the authorization response comes in

diff --git a/src/mod_auth_openidc.h b/src/mod_auth_openidc.h

index c3a0a23..fada56d 100644

--- a/src/mod_auth_openidc.h

+++ b/src/mod_auth_openidc.h

@@ -222,6 +222,9 @@ APLOG_USE_MODULE(auth_openidc);

 #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

+

 typedef apr_byte_t (*oidc_proto_pkce_state)(request_rec *r, char **state);

 typedef apr_byte_t (*oidc_proto_pkce_challenge)(request_rec *r, const char *state, char **code_challenge);

 typedef apr_byte_t (*oidc_proto_pkce_verifier)(request_rec *r, const char *state, char **code_verifier);

@@ -403,6 +406,8 @@ typedef struct oidc_cfg {

 	apr_hash_t *black_listed_claims;

 	apr_hash_t *white_listed_claims;

+	apr_byte_t state_input_headers;

+

 } oidc_cfg;

 int oidc_check_user_id(request_rec *r);

diff --git a/src/parse.c b/src/parse.c

index 2d09584..a0cedcb 100644

--- a/src/parse.c

+++ b/src/parse.c

@@ -1254,3 +1254,36 @@ const char *oidc_parse_max_number_of_state_cookies(apr_pool_t *pool,

 		rv = oidc_parse_boolean(pool, arg2, bool_value);

 	return rv;

 }

+

+#define OIDC_STATE_INPUT_HEADERS_AS_BOTH            "both"

+#define OIDC_STATE_INPUT_HEADERS_AS_USER_AGENT      "user-agent"

+#define OIDC_STATE_INPUT_HEADERS_AS_X_FORWARDED_FOR "x-forwarded-for"

+#define OIDC_STATE_INPUT_HEADERS_AS_NONE            "none"

+

+/*

+ * parse a "set state input headers as" value from the provided string

+ */

+const char *oidc_parse_set_state_input_headers_as(apr_pool_t *pool, const char *arg,

+		apr_byte_t *state_input_headers) {

+	static char *options[] = {

+			OIDC_STATE_INPUT_HEADERS_AS_BOTH,

+			OIDC_STATE_INPUT_HEADERS_AS_USER_AGENT,

+			OIDC_STATE_INPUT_HEADERS_AS_X_FORWARDED_FOR,

+			OIDC_STATE_INPUT_HEADERS_AS_NONE,

+			NULL };

+	const char *rv = oidc_valid_string_option(pool, arg, options);

+	if (rv != NULL)

+		return rv;

+

+	if (apr_strnatcmp(arg, OIDC_STATE_INPUT_HEADERS_AS_BOTH) == 0) {

+		*state_input_headers = OIDC_STATE_INPUT_HEADERS_USER_AGENT | OIDC_STATE_INPUT_HEADERS_X_FORWARDED_FOR;

+	} else if (apr_strnatcmp(arg, OIDC_STATE_INPUT_HEADERS_AS_USER_AGENT) == 0) {

+		*state_input_headers = OIDC_STATE_INPUT_HEADERS_USER_AGENT;

+	} else if (apr_strnatcmp(arg, OIDC_STATE_INPUT_HEADERS_AS_X_FORWARDED_FOR) == 0) {

+		*state_input_headers = OIDC_STATE_INPUT_HEADERS_X_FORWARDED_FOR;

+	} else if (apr_strnatcmp(arg, OIDC_STATE_INPUT_HEADERS_AS_NONE) == 0) {

+		*state_input_headers = 0;

+	}

+

+	return NULL;

+}

diff --git a/src/parse.h b/src/parse.h

index bdf5651..c4301a3 100644

--- a/src/parse.h

+++ b/src/parse.h

@@ -118,6 +118,7 @@ const char *oidc_parse_token_binding_policy(apr_pool_t *pool, const char *arg, i

 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, const char *arg1, const char *arg2, int *int_value, int *bool_value);

+const char *oidc_parse_set_state_input_headers_as(apr_pool_t *pool, const char *arg, apr_byte_t *state_input_headers);

 typedef const char *(*oidc_valid_int_function_t)(apr_pool_t *, int);

 typedef const char *(*oidc_valid_function_t)(apr_pool_t *, const char *);

-- 

2.26.2