rpms/mod_auth_openidc
6
0
Fork 0
Code Issues Pull Requests Releases Activity

import mod_auth_openidc-2.3.7-8.module+el8.4.0+9707+f2438af7

Browse Source
This commit is contained in:
CentOS Sources 2021-02-02 06:38:32 +00:00 committed by Andrew Lukoshko
commit d3f3f93435
22 changed files with 2482 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
.gitignore vendored Normal file
View File

@ -0,0 +1 @@
SOURCES/mod_auth_openidc-2.3.7.tar.gz

1
.mod_auth_openidc.metadata Normal file
View File

@ -0,0 +1 @@
0f9d620dad066ae8c415a59055903da1bfa9a4bf SOURCES/mod_auth_openidc-2.3.7.tar.gz

127
SOURCES/0002-Backport-of-improve-validation-of-the-post-logout-UR.patch Normal file
View File

@ -0,0 +1,127 @@
From cb5560f016d4f8bbca40670c59898afafb8d0763 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek@redhat.com>
Date: Sun, 10 May 2020 19:56:53 +0200
Subject: [PATCH] Backport of improve validation of the post-logout URL
---
src/mod_auth_openidc.c | 90 +++++++++++++++++++++++++-----------------
1 file changed, 53 insertions(+), 37 deletions(-)
diff --git a/src/mod_auth_openidc.c b/src/mod_auth_openidc.c
index eaaec3c..e86c61e 100644
--- a/src/mod_auth_openidc.c
+++ b/src/mod_auth_openidc.c
@@ -2563,6 +2563,52 @@ static int oidc_handle_logout_request(request_rec *r, oidc_cfg *c,
return HTTP_MOVED_TEMPORARILY;
}
+static apr_byte_t oidc_validate_post_logout_url(request_rec *r, const char *url, char **err_str, char **err_desc) {
+ apr_uri_t uri;
+ const char *c_host = NULL;
+
+ if (apr_uri_parse(r->pool, url, &uri) != APR_SUCCESS) {
+ *err_str = apr_pstrdup(r->pool, "Malformed URL");
+ *err_desc = apr_psprintf(r->pool, "Logout URL malformed: %s", url);
+ oidc_error(r, "%s: %s", *err_str, *err_desc);
+ return FALSE;
+ }
+
+ c_host = oidc_get_current_url_host(r);
+ if ((uri.hostname != NULL)
+ && ((strstr(c_host, uri.hostname) == NULL)
+ || (strstr(uri.hostname, c_host) == NULL))) {
+ *err_str = apr_pstrdup(r->pool, "Invalid Request");
+ *err_desc =
+ apr_psprintf(r->pool,
+ "logout value \"%s\" does not match the hostname of the current request \"%s\"",
+ apr_uri_unparse(r->pool, &uri, 0), c_host);
+ oidc_error(r, "%s: %s", *err_str, *err_desc);
+ return FALSE;
+ } else if (strstr(url, "/") != url) {
+ *err_str = apr_pstrdup(r->pool, "Malformed URL");
+ *err_desc =
+ apr_psprintf(r->pool,
+ "No hostname was parsed and it does not seem to be relative, i.e starting with '/': %s",
+ url);
+ oidc_error(r, "%s: %s", *err_str, *err_desc);
+ return FALSE;
+ }
+
+ /* validate the URL to prevent HTTP header splitting */
+ if (((strstr(url, "\n") != NULL) || strstr(url, "\r") != NULL)) {
+ *err_str = apr_pstrdup(r->pool, "Invalid Request");
+ *err_desc =
+ apr_psprintf(r->pool,
+ "logout value \"%s\" contains illegal \"\n\" or \"\r\" character(s)",
+ url);
+ oidc_error(r, "%s: %s", *err_str, *err_desc);
+ return FALSE;
+ }
+
+ return TRUE;
+}
+
/*
* perform (single) logout
*/
@@ -2571,6 +2617,8 @@ static int oidc_handle_logout(request_rec *r, oidc_cfg *c,
/* pickup the command or URL where the user wants to go after logout */
char *url = NULL;
+ char *error_str = NULL;
+ char *error_description = NULL;
oidc_util_get_request_parameter(r, OIDC_REDIRECT_URI_REQUEST_LOGOUT, &url);
oidc_debug(r, "enter (url=%s)", url);
@@ -2587,43 +2635,11 @@ static int oidc_handle_logout(request_rec *r, oidc_cfg *c,
/* do input validation on the logout parameter value */
- const char *error_description = NULL;
- apr_uri_t uri;
-
- if (apr_uri_parse(r->pool, url, &uri) != APR_SUCCESS) {
- const char *error_description = apr_psprintf(r->pool,
- "Logout URL malformed: %s", url);
- oidc_error(r, "%s", error_description);
- return oidc_util_html_send_error(r, c->error_template,
- "Malformed URL", error_description,
- HTTP_INTERNAL_SERVER_ERROR);
-
- }
-
- const char *c_host = oidc_get_current_url_host(r);
- if ((uri.hostname != NULL)
- && ((strstr(c_host, uri.hostname) == NULL)
- || (strstr(uri.hostname, c_host) == NULL))) {
- error_description =
- apr_psprintf(r->pool,
- "logout value \"%s\" does not match the hostname of the current request \"%s\"",
- apr_uri_unparse(r->pool, &uri, 0), c_host);
- oidc_error(r, "%s", error_description);
- return oidc_util_html_send_error(r, c->error_template,
- "Invalid Request", error_description,
- HTTP_INTERNAL_SERVER_ERROR);
- }
-
- /* validate the URL to prevent HTTP header splitting */
- if (((strstr(url, "\n") != NULL) || strstr(url, "\r") != NULL)) {
- error_description =
- apr_psprintf(r->pool,
- "logout value \"%s\" contains illegal \"\n\" or \"\r\" character(s)",
- url);
- oidc_error(r, "%s", error_description);
- return oidc_util_html_send_error(r, c->error_template,
- "Invalid Request", error_description,
- HTTP_INTERNAL_SERVER_ERROR);
+ if (oidc_validate_post_logout_url(r, url, &error_str,
+ &error_description) == FALSE) {
+ return oidc_util_html_send_error(r, c->error_template, error_str,
+ error_description,
+ HTTP_BAD_REQUEST);
}
}
--
2.21.3

31
SOURCES/0003-Backport-of-Fix-open-redirect-starting-with-a-slash.patch Normal file
View File

@ -0,0 +1,31 @@
From ed041f8b5df58c4e612a0d0cbb920dc0b399b921 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek@redhat.com>
Date: Sun, 10 May 2020 20:00:49 +0200
Subject: [PATCH 3/3] Backport of Fix open redirect starting with a slash
---
src/mod_auth_openidc.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/src/mod_auth_openidc.c b/src/mod_auth_openidc.c
index e86c61e..3c6efb4 100644
--- a/src/mod_auth_openidc.c
+++ b/src/mod_auth_openidc.c
@@ -2604,6 +2604,14 @@ static apr_byte_t oidc_validate_post_logout_url(request_rec *r, const char *url,
url);
oidc_error(r, "%s: %s", *err_str, *err_desc);
return FALSE;
+ } else if ((uri.hostname == NULL) && (strstr(url, "//") == url)) {
+ *err_str = apr_pstrdup(r->pool, "Malformed URL");
+ *err_desc =
+ apr_psprintf(r->pool,
+ "No hostname was parsed and starting with '//': %s",
+ url);
+ oidc_error(r, "%s: %s", *err_str, *err_desc);
+ return FALSE;
}
return TRUE;
--
2.21.3

32
SOURCES/0004-Backport-of-Fix-open-redirect-starting-with-a-slash-.patch Normal file
View File

@ -0,0 +1,32 @@
From c21228a0f170c025d79625207dc94759f480418f Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek@redhat.com>
Date: Sun, 10 May 2020 20:02:23 +0200
Subject: [PATCH 4/4] Backport of Fix open redirect starting with a slash and a
backslash
---
src/mod_auth_openidc.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/src/mod_auth_openidc.c b/src/mod_auth_openidc.c
index 3c6efb4..e16d500 100644
--- a/src/mod_auth_openidc.c
+++ b/src/mod_auth_openidc.c
@@ -2612,6 +2612,14 @@ static apr_byte_t oidc_validate_post_logout_url(request_rec *r, const char *url,
url);
oidc_error(r, "%s: %s", *err_str, *err_desc);
return FALSE;
+ } else if ((uri.hostname == NULL) && (strstr(url, "/\\") == url)) {
+ *err_str = apr_pstrdup(r->pool, "Malformed URL");
+ *err_desc =
+ apr_psprintf(r->pool,
+ "No hostname was parsed and starting with '/\\': %s",
+ url);
+ oidc_error(r, "%s: %s", *err_str, *err_desc);
+ return FALSE;
}
return TRUE;
--
2.21.3

61
SOURCES/0005-Fix-the-previous-backports.patch Normal file
View File

@ -0,0 +1,61 @@
From a5c9f79516fd4097817ac75a37af3b191a3d1448 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek <jhrozek@redhat.com>
Date: Mon, 1 Jun 2020 21:47:28 +0200
Subject: [PATCH] Fix the previous backports
---
src/mod_auth_openidc.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/src/mod_auth_openidc.c b/src/mod_auth_openidc.c
index e16d500..74f206b 100644
--- a/src/mod_auth_openidc.c
+++ b/src/mod_auth_openidc.c
@@ -2585,7 +2585,7 @@ static apr_byte_t oidc_validate_post_logout_url(request_rec *r, const char *url,
apr_uri_unparse(r->pool, &uri, 0), c_host);
oidc_error(r, "%s: %s", *err_str, *err_desc);
return FALSE;
- } else if (strstr(url, "/") != url) {
+ } else if ((uri.hostname == NULL) && (strstr(url, "/") != url)) {
*err_str = apr_pstrdup(r->pool, "Malformed URL");
*err_desc =
apr_psprintf(r->pool,
@@ -2593,17 +2593,6 @@ static apr_byte_t oidc_validate_post_logout_url(request_rec *r, const char *url,
url);
oidc_error(r, "%s: %s", *err_str, *err_desc);
return FALSE;
- }
-
- /* validate the URL to prevent HTTP header splitting */
- if (((strstr(url, "\n") != NULL) || strstr(url, "\r") != NULL)) {
- *err_str = apr_pstrdup(r->pool, "Invalid Request");
- *err_desc =
- apr_psprintf(r->pool,
- "logout value \"%s\" contains illegal \"\n\" or \"\r\" character(s)",
- url);
- oidc_error(r, "%s: %s", *err_str, *err_desc);
- return FALSE;
} else if ((uri.hostname == NULL) && (strstr(url, "//") == url)) {
*err_str = apr_pstrdup(r->pool, "Malformed URL");
*err_desc =
@@ -2622,6 +2611,17 @@ static apr_byte_t oidc_validate_post_logout_url(request_rec *r, const char *url,
return FALSE;
}
+ /* validate the URL to prevent HTTP header splitting */
+ if (((strstr(url, "\n") != NULL) || strstr(url, "\r") != NULL)) {
+ *err_str = apr_pstrdup(r->pool, "Invalid Request");
+ *err_desc =
+ apr_psprintf(r->pool,
+ "logout value \"%s\" contains illegal \"\n\" or \"\r\" character(s)",
+ url);
+ oidc_error(r, "%s: %s", *err_str, *err_desc);
+ return FALSE;
+ }
+
return TRUE;
}
--
2.21.3

254
SOURCES/0006-add-OIDCStateMaxNumberOfCookies-to-limit-nr-of-state.patch Normal file
View File

@ -0,0 +1,254 @@
From 1bb55df94c0db8376f88a568c331802bd282f097 Mon Sep 17 00:00:00 2001
From: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
Date: Wed, 1 Aug 2018 11:41:57 +0200
Subject: [PATCH 06/11] add OIDCStateMaxNumberOfCookies to limit nr of state
cookies; see #331
Signed-off-by: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
(cherry picked from commit 5041e0f679f03525f106eaf0edc7c7d245b1d89c)
---
ChangeLog | 3 +++
src/config.c | 19 +++++++++++++
src/mod_auth_openidc.c | 61 ++++++++++++++++++++++++++++++++++--------
src/mod_auth_openidc.h | 2 ++
4 files changed, 74 insertions(+), 11 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index af9380e..b6ac513 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,6 @@
+08/01/2018
+- add option to set an upper limit to the number of concurrent state cookies via OIDCStateMaxNumberOfCookies; see #331
+
07/06/2018
- abort when string length for remote user name substitution is larger than 255 characters
- release 2.3.7
diff --git a/src/config.c b/src/config.c
index 0185cff..2fd63ea 100644
--- a/src/config.c
+++ b/src/config.c
@@ -104,6 +104,8 @@
#define OIDC_DEFAULT_SESSION_CLIENT_COOKIE_CHUNK_SIZE 4000
/* timeout in seconds after which state expires */
#define OIDC_DEFAULT_STATE_TIMEOUT 300
+/* maximum number of parallel state cookies; 0 means unlimited, until the browser or server gives up */
+#define OIDC_DEFAULT_MAX_NUMBER_OF_STATE_COOKIES 0
/* default session inactivity timeout */
#define OIDC_DEFAULT_SESSION_INACTIVITY_TIMEOUT 300
/* default session max duration */
@@ -227,6 +229,7 @@
#define OIDCHTTPTimeoutLong "OIDCHTTPTimeoutLong"
#define OIDCHTTPTimeoutShort "OIDCHTTPTimeoutShort"
#define OIDCStateTimeout "OIDCStateTimeout"
+#define OIDCStateMaxNumberOfCookies "OIDCStateMaxNumberOfCookies"
#define OIDCSessionInactivityTimeout "OIDCSessionInactivityTimeout"
#define OIDCMetadataDir "OIDCMetadataDir"
#define OIDCSessionCacheFallbackToCookie "OIDCSessionCacheFallbackToCookie"
@@ -994,6 +997,12 @@ static const char *oidc_set_client_auth_bearer_token(cmd_parms *cmd,
return NULL;
}
+int oidc_cfg_max_number_of_state_cookies(oidc_cfg *cfg) {
+ if (cfg->max_number_of_state_cookies == OIDC_CONFIG_POS_INT_UNSET)
+ return OIDC_DEFAULT_MAX_NUMBER_OF_STATE_COOKIES;
+ return cfg->max_number_of_state_cookies;
+}
+
/*
* create a new server config record with defaults
*/
@@ -1102,6 +1111,7 @@ void *oidc_create_server_config(apr_pool_t *pool, server_rec *svr) {
c->http_timeout_long = OIDC_DEFAULT_HTTP_TIMEOUT_LONG;
c->http_timeout_short = OIDC_DEFAULT_HTTP_TIMEOUT_SHORT;
c->state_timeout = OIDC_DEFAULT_STATE_TIMEOUT;
+ c->max_number_of_state_cookies = OIDC_CONFIG_POS_INT_UNSET;
c->session_inactivity_timeout = OIDC_DEFAULT_SESSION_INACTIVITY_TIMEOUT;
c->cookie_domain = NULL;
@@ -1416,6 +1426,10 @@ void *oidc_merge_server_config(apr_pool_t *pool, void *BASE, void *ADD) {
c->state_timeout =
add->state_timeout != OIDC_DEFAULT_STATE_TIMEOUT ?
add->state_timeout : base->state_timeout;
+ c->max_number_of_state_cookies =
+ add->max_number_of_state_cookies != OIDC_CONFIG_POS_INT_UNSET ?
+ add->max_number_of_state_cookies :
+ base->max_number_of_state_cookies;
c->session_inactivity_timeout =
add->session_inactivity_timeout
!= OIDC_DEFAULT_SESSION_INACTIVITY_TIMEOUT ?
@@ -2627,6 +2641,11 @@ const command_rec oidc_config_cmds[] = {
(void*)APR_OFFSETOF(oidc_cfg, state_timeout),
RSRC_CONF,
"Time to live in seconds for state parameter (cq. interval in which the authorization request and the corresponding response need to be completed)."),
+ AP_INIT_TAKE1(OIDCStateMaxNumberOfCookies,
+ oidc_set_int_slot,
+ (void*)APR_OFFSETOF(oidc_cfg, max_number_of_state_cookies),
+ RSRC_CONF,
+ "Maximun number of parallel state cookies i.e. outstanding authorization requests."),
AP_INIT_TAKE1(OIDCSessionInactivityTimeout,
oidc_set_session_inactivity_timeout,
(void*)APR_OFFSETOF(oidc_cfg, session_inactivity_timeout),
diff --git a/src/mod_auth_openidc.c b/src/mod_auth_openidc.c
index 74f206b..c0f65c6 100644
--- a/src/mod_auth_openidc.c
+++ b/src/mod_auth_openidc.c
@@ -686,8 +686,14 @@ static apr_byte_t oidc_unsolicited_proto_state(request_rec *r, oidc_cfg *c,
return TRUE;
}
-static void oidc_clean_expired_state_cookies(request_rec *r, oidc_cfg *c,
+/*
+ * clean state cookies that have expired i.e. for outstanding requests that will never return
+ * successfully and return the number of remaining valid cookies/outstanding-requests while
+ * doing so
+ */
+static int oidc_clean_expired_state_cookies(request_rec *r, oidc_cfg *c,
const char *currentCookieName) {
+ int number_of_valid_state_cookies = 0;
char *cookie, *tokenizerCtx;
char *cookies = apr_pstrdup(r->pool, oidc_util_hdr_in_cookie_get(r));
if (cookies != NULL) {
@@ -715,6 +721,8 @@ static void oidc_clean_expired_state_cookies(request_rec *r, oidc_cfg *c,
cookieName);
oidc_util_set_cookie(r, cookieName, "", 0,
NULL);
+ } else {
+ number_of_valid_state_cookies++;
}
oidc_proto_state_destroy(proto_state);
}
@@ -724,6 +732,7 @@ static void oidc_clean_expired_state_cookies(request_rec *r, oidc_cfg *c,
cookie = apr_strtok(NULL, OIDC_STR_SEMI_COLON, &tokenizerCtx);
}
}
+ return number_of_valid_state_cookies;
}
/*
@@ -796,7 +805,7 @@ static apr_byte_t oidc_restore_proto_state(request_rec *r, oidc_cfg *c,
* set the state that is maintained between an authorization request and an authorization response
* in a cookie in the browser that is cryptographically bound to that state
*/
-static apr_byte_t oidc_authorization_request_set_cookie(request_rec *r,
+static int oidc_authorization_request_set_cookie(request_rec *r,
oidc_cfg *c, const char *state, oidc_proto_state_t *proto_state) {
/*
* create a cookie consisting of 8 elements:
@@ -805,10 +814,32 @@ static apr_byte_t oidc_authorization_request_set_cookie(request_rec *r,
*/
char *cookieValue = oidc_proto_state_to_cookie(r, c, proto_state);
if (cookieValue == NULL)
- return FALSE;
+ return HTTP_INTERNAL_SERVER_ERROR;
- /* clean expired state cookies to avoid pollution */
- oidc_clean_expired_state_cookies(r, c, NULL);
+ /*
+ * clean expired state cookies to avoid pollution and optionally
+ * try to avoid the number of state cookies exceeding a max
+ */
+ int number_of_cookies = oidc_clean_expired_state_cookies(r, c, NULL);
+ int max_number_of_cookies = oidc_cfg_max_number_of_state_cookies(c);
+ if ((max_number_of_cookies > 0)
+ && (number_of_cookies >= max_number_of_cookies)) {
+ oidc_warn(r,
+ "the number of existing, valid state cookies (%d) has exceeded the limit (%d), no additional authorization request + state cookie can be generated, aborting the request",
+ number_of_cookies, max_number_of_cookies);
+ /*
+ * TODO: the html_send code below caters for the case that there's a user behind a
+ * browser generating this request, rather than a piece of XHR code; how would an
+ * XHR client handle this?
+ */
+
+ return oidc_util_html_send_error(r, c->error_template,
+ "Too Many Outstanding Requests",
+ apr_psprintf(r->pool,
+ "No authentication request could be generated since there are too many outstanding authentication requests already; you may have to wait up to %d seconds to be able to create a new request",
+ c->state_timeout),
+ HTTP_SERVICE_UNAVAILABLE);
+ }
/* assemble the cookie name for the state cookie */
const char *cookieName = oidc_get_state_cookie_name(r, state);
@@ -817,9 +848,7 @@ static apr_byte_t oidc_authorization_request_set_cookie(request_rec *r,
oidc_util_set_cookie(r, cookieName, cookieValue, -1,
c->cookie_same_site ? OIDC_COOKIE_EXT_SAME_SITE_LAX : NULL);
- //free(s_value);
-
- return TRUE;
+ return HTTP_OK;
}
/*
@@ -2245,12 +2274,19 @@ static int oidc_authenticate_user(request_rec *r, oidc_cfg *c,
/* get a hash value that fingerprints the browser concatenated with the random input */
char *state = oidc_get_browser_state_hash(r, nonce);
- /* create state that restores the context when the authorization response comes in; cryptographically bind it to the browser */
- if (oidc_authorization_request_set_cookie(r, c, state, proto_state) == FALSE)
- return HTTP_INTERNAL_SERVER_ERROR;
+ /*
+ * create state that restores the context when the authorization response comes in
+ * and cryptographically bind it to the browser
+ */
+ int rc = oidc_authorization_request_set_cookie(r, c, state, proto_state);
+ if (rc != HTTP_OK) {
+ oidc_proto_state_destroy(proto_state);
+ return rc;
+ }
/*
* printout errors if Cookie settings are not going to work
+ * TODO: separate this code out into its own function
*/
apr_uri_t o_uri;
memset(&o_uri, 0, sizeof(apr_uri_t));
@@ -2263,6 +2299,7 @@ static int oidc_authenticate_user(request_rec *r, oidc_cfg *c,
oidc_error(r,
"the URL scheme (%s) of the configured " OIDCRedirectURI " does not match the URL scheme of the URL being accessed (%s): the \"state\" and \"session\" cookies will not be shared between the two!",
r_uri.scheme, o_uri.scheme);
+ oidc_proto_state_destroy(proto_state);
return HTTP_INTERNAL_SERVER_ERROR;
}
@@ -2273,6 +2310,7 @@ static int oidc_authenticate_user(request_rec *r, oidc_cfg *c,
oidc_error(r,
"the URL hostname (%s) of the configured " OIDCRedirectURI " does not match the URL hostname of the URL being accessed (%s): the \"state\" and \"session\" cookies will not be shared between the two!",
r_uri.hostname, o_uri.hostname);
+ oidc_proto_state_destroy(proto_state);
return HTTP_INTERNAL_SERVER_ERROR;
}
}
@@ -2281,6 +2319,7 @@ static int oidc_authenticate_user(request_rec *r, oidc_cfg *c,
oidc_error(r,
"the domain (%s) configured in " OIDCCookieDomain " does not match the URL hostname (%s) of the URL being accessed (%s): setting \"state\" and \"session\" cookies will not work!!",
c->cookie_domain, o_uri.hostname, original_url);
+ oidc_proto_state_destroy(proto_state);
return HTTP_INTERNAL_SERVER_ERROR;
}
}
diff --git a/src/mod_auth_openidc.h b/src/mod_auth_openidc.h
index 48bb8fe..5162ed4 100644
--- a/src/mod_auth_openidc.h
+++ b/src/mod_auth_openidc.h
@@ -379,6 +379,7 @@ typedef struct oidc_cfg {
int http_timeout_long;
int http_timeout_short;
int state_timeout;
+ int max_number_of_state_cookies;
int session_inactivity_timeout;
int session_cache_fallback_to_cookie;
@@ -686,6 +687,7 @@ int oidc_cfg_cache_encrypt(request_rec *r);
int oidc_cfg_session_cache_fallback_to_cookie(request_rec *r);
const char *oidc_parse_pkce_type(apr_pool_t *pool, const char *arg, oidc_proto_pkce_t **type);
const char *oidc_cfg_claim_prefix(request_rec *r);
+int oidc_cfg_max_number_of_state_cookies(oidc_cfg *cfg);
// oidc_util.c
int oidc_strnenvcmp(const char *a, const char *b, int len);
--
2.26.2

118
SOURCES/0007-set-boundaries-on-min-and-max-values-on-number-of-pa.patch Normal file
View File

@ -0,0 +1,118 @@
From 284537dfc0585e08cfc0702c89b241d8986c7236 Mon Sep 17 00:00:00 2001
From: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
Date: Fri, 3 Aug 2018 12:22:45 +0200
Subject: [PATCH 07/11] set boundaries on min and max values on number of
parallel state cookies
Signed-off-by: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
(cherry picked from commit b8c53d7e0439f190afe0c6eeb2e2e12e881c65ac)
---
src/config.c | 17 ++++++++++++++++-
src/parse.c | 31 +++++++++++++++++++++++++++++++
src/parse.h | 2 ++
3 files changed, 49 insertions(+), 1 deletion(-)
diff --git a/src/config.c b/src/config.c
index 2fd63ea..c793818 100644
--- a/src/config.c
+++ b/src/config.c
@@ -997,6 +997,21 @@ static const char *oidc_set_client_auth_bearer_token(cmd_parms *cmd,
return NULL;
}
+/*
+ * set the maximun number of parallel state cookies
+ */
+static const char *oidc_set_max_number_of_state_cookies(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);
+ const char *rv = oidc_parse_max_number_of_state_cookies(cmd->pool, arg,
+ &cfg->max_number_of_state_cookies);
+ return OIDC_CONFIG_DIR_RV(cmd, rv);
+}
+
+/*
+ * return the maximun number of parallel state cookies
+ */
int oidc_cfg_max_number_of_state_cookies(oidc_cfg *cfg) {
if (cfg->max_number_of_state_cookies == OIDC_CONFIG_POS_INT_UNSET)
return OIDC_DEFAULT_MAX_NUMBER_OF_STATE_COOKIES;
@@ -2642,7 +2657,7 @@ const command_rec oidc_config_cmds[] = {
RSRC_CONF,
"Time to live in seconds for state parameter (cq. interval in which the authorization request and the corresponding response need to be completed)."),
AP_INIT_TAKE1(OIDCStateMaxNumberOfCookies,
- oidc_set_int_slot,
+ oidc_set_max_number_of_state_cookies,
(void*)APR_OFFSETOF(oidc_cfg, max_number_of_state_cookies),
RSRC_CONF,
"Maximun number of parallel state cookies i.e. outstanding authorization requests."),
diff --git a/src/parse.c b/src/parse.c
index 9d3763c..0f986fd 100644
--- a/src/parse.c
+++ b/src/parse.c
@@ -530,6 +530,28 @@ const char *oidc_valid_session_max_duration(apr_pool_t *pool, int v) {
return NULL;
}
+#define OIDC_MAX_NUMBER_OF_STATE_COOKIES_MIN 0
+#define OIDC_MAX_NUMBER_OF_STATE_COOKIES_MAX 255
+
+/*
+ * check the maximum number of parallel state cookies
+ */
+const char *oidc_valid_max_number_of_state_cookies(apr_pool_t *pool, int v) {
+ if (v == 0) {
+ return NULL;
+ }
+ if (v < OIDC_MAX_NUMBER_OF_STATE_COOKIES_MIN) {
+ return apr_psprintf(pool, "maximum must not be less than %d",
+ OIDC_MAX_NUMBER_OF_STATE_COOKIES_MIN);
+ }
+ if (v > OIDC_MAX_NUMBER_OF_STATE_COOKIES_MAX) {
+ return apr_psprintf(pool, "maximum must not be greater than %d",
+ OIDC_MAX_NUMBER_OF_STATE_COOKIES_MAX);
+ }
+ return NULL;
+}
+
+
/*
* parse a session max duration value from the provided string
*/
@@ -1218,3 +1240,12 @@ const char *oidc_parse_auth_request_method(apr_pool_t *pool, const char *arg,
return NULL;
}
+
+/*
+ * parse the maximum number of parallel state cookies
+ */
+const char *oidc_parse_max_number_of_state_cookies(apr_pool_t *pool,
+ const char *arg, int *int_value) {
+ return oidc_parse_int_valid(pool, arg, int_value,
+ oidc_valid_max_number_of_state_cookies);
+}
diff --git a/src/parse.h b/src/parse.h
index 853e98f..6355db4 100644
--- a/src/parse.h
+++ b/src/parse.h
@@ -90,6 +90,7 @@ 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);
const char *oidc_parse_int(apr_pool_t *pool, const char *arg, int *int_value);
const char *oidc_parse_boolean(apr_pool_t *pool, const char *arg, int *bool_value);
@@ -116,6 +117,7 @@ const char *oidc_parse_info_hook_data(apr_pool_t *pool, const char *arg, apr_has
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, const char *arg, int *int_value);
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

45
SOURCES/0008-make-the-default-max-number-of-state-cookies-7-inste.patch Normal file
View File

@ -0,0 +1,45 @@
From 623163348f74fc1bd019a676fa24af69dde79654 Mon Sep 17 00:00:00 2001
From: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
Date: Fri, 3 Aug 2018 21:41:34 +0200
Subject: [PATCH 08/11] make the default max number of state cookies 7 instead
of unlimited
bump to 2.3.8rc1
Signed-off-by: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
(cherry picked from commit 6616372af77df04a9b0b197e759790ecf3f2399a)
---
ChangeLog | 5 ++++-
src/config.c | 2 +-
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index b6ac513..27f45be 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,8 @@
-08/01/2018
+
+08/03/2018
- add option to set an upper limit to the number of concurrent state cookies via OIDCStateMaxNumberOfCookies; see #331
+- make the default maximum number of parallel state cookies 7 instead of unlimited; see #331
+- bump o 2.3.8rc1
07/06/2018
- abort when string length for remote user name substitution is larger than 255 characters
diff --git a/src/config.c b/src/config.c
index c793818..6fa6227 100644
--- a/src/config.c
+++ b/src/config.c
@@ -105,7 +105,7 @@
/* timeout in seconds after which state expires */
#define OIDC_DEFAULT_STATE_TIMEOUT 300
/* maximum number of parallel state cookies; 0 means unlimited, until the browser or server gives up */
-#define OIDC_DEFAULT_MAX_NUMBER_OF_STATE_COOKIES 0
+#define OIDC_DEFAULT_MAX_NUMBER_OF_STATE_COOKIES 7
/* default session inactivity timeout */
#define OIDC_DEFAULT_SESSION_INACTIVITY_TIMEOUT 300
/* default session max duration */
--
2.26.2

59
SOURCES/0009-don-t-return-content-with-503-see-331.patch Normal file
View File

@ -0,0 +1,59 @@
From 7f5666375a3351e9c37589456b6fb3c92ef987c0 Mon Sep 17 00:00:00 2001
From: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
Date: Sat, 4 Aug 2018 08:55:33 +0200
Subject: [PATCH 09/11] don't return content with 503; see #331
since it turns the HTTP 503 status code into a 200 which we don't prefer
for XHR clients; users will see Apache specific readable text
Signed-off-by: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
(cherry picked from commit 9e98f1a042fa14d6b0892638a0d87c2b951837b6)
---
ChangeLog | 4 +++-
src/mod_auth_openidc.c | 8 ++++++++
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/ChangeLog b/ChangeLog
index 27f45be..dfe4bd6 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,8 +1,10 @@
+08/04/2018
+- don't return content with 503 since it will turn the HTTP status code into a 200; see #331
08/03/2018
- add option to set an upper limit to the number of concurrent state cookies via OIDCStateMaxNumberOfCookies; see #331
- make the default maximum number of parallel state cookies 7 instead of unlimited; see #331
-- bump o 2.3.8rc1
+- bump to 2.3.8rc1
07/06/2018
- abort when string length for remote user name substitution is larger than 255 characters
diff --git a/src/mod_auth_openidc.c b/src/mod_auth_openidc.c
index c0f65c6..e3817a9 100644
--- a/src/mod_auth_openidc.c
+++ b/src/mod_auth_openidc.c
@@ -833,12 +833,20 @@ static int oidc_authorization_request_set_cookie(request_rec *r,
* XHR client handle this?
*/
+ /*
+ * it appears that sending content with a 503 turns the HTTP status code
+ * into a 200 so we'll avoid that for now: the user will see Apache specific
+ * readable text anyway
+ *
return oidc_util_html_send_error(r, c->error_template,
"Too Many Outstanding Requests",
apr_psprintf(r->pool,
"No authentication request could be generated since there are too many outstanding authentication requests already; you may have to wait up to %d seconds to be able to create a new request",
c->state_timeout),
HTTP_SERVICE_UNAVAILABLE);
+ */
+
+ return HTTP_SERVICE_UNAVAILABLE;
}
/* assemble the cookie name for the state cookie */
--
2.26.2

305
SOURCES/0010-improve-auto-detection-of-XMLHttpRequests-via-Accept.patch Normal file
View File

@ -0,0 +1,305 @@
From 9a0827a18ec4d16cf9e79afede901194d6ef5cc6 Mon Sep 17 00:00:00 2001
From: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
Date: Thu, 23 Aug 2018 00:04:38 +0200
Subject: [PATCH 10/11] improve auto-detection of XMLHttpRequests via Accept
header; see #331
version 2.3.8rc5
Signed-off-by: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
(cherry picked from commit cff35bba8861ddb09d694ecfcd88d5fac1018453)
---
src/mod_auth_openidc.c | 41 +++---
src/mod_auth_openidc.h | 6 +-
src/util.c | 80 +++++++---
diff --git a/src/mod_auth_openidc.c b/src/mod_auth_openidc.c
index e3817a9..897a449 100644
--- a/src/mod_auth_openidc.c
+++ b/src/mod_auth_openidc.c
@@ -805,8 +805,8 @@ static apr_byte_t oidc_restore_proto_state(request_rec *r, oidc_cfg *c,
* set the state that is maintained between an authorization request and an authorization response
* in a cookie in the browser that is cryptographically bound to that state
*/
-static int oidc_authorization_request_set_cookie(request_rec *r,
- oidc_cfg *c, const char *state, oidc_proto_state_t *proto_state) {
+static int oidc_authorization_request_set_cookie(request_rec *r, oidc_cfg *c,
+ const char *state, oidc_proto_state_t *proto_state) {
/*
* create a cookie consisting of 8 elements:
* random value, original URL, original method, issuer, response_type, response_mod, prompt and timestamp
@@ -818,7 +818,7 @@ static int oidc_authorization_request_set_cookie(request_rec *r,
/*
* clean expired state cookies to avoid pollution and optionally
- * try to avoid the number of state cookies exceeding a max
+ * try to avoid the number of state cookies exceeding a max
*/
int number_of_cookies = oidc_clean_expired_state_cookies(r, c, NULL);
int max_number_of_cookies = oidc_cfg_max_number_of_state_cookies(c);
@@ -953,6 +953,26 @@ static void oidc_log_session_expires(request_rec *r, const char *msg,
apr_time_sec(session_expires - apr_time_now()));
}
+/*
+ * see if this is a non-browser request
+ */
+static apr_byte_t oidc_is_xml_http_request(request_rec *r) {
+
+ if ((oidc_util_hdr_in_x_requested_with_get(r) != NULL)
+ && (apr_strnatcasecmp(oidc_util_hdr_in_x_requested_with_get(r),
+ OIDC_HTTP_HDR_VAL_XML_HTTP_REQUEST) == 0))
+ return TRUE;
+
+ if ((oidc_util_hdr_in_accept_contains(r, OIDC_CONTENT_TYPE_TEXT_HTML)
+ == FALSE) && (oidc_util_hdr_in_accept_contains(r,
+ OIDC_CONTENT_TYPE_APP_XHTML_XML) == FALSE)
+ && (oidc_util_hdr_in_accept_contains(r,
+ OIDC_CONTENT_TYPE_ANY) == FALSE))
+ return TRUE;
+
+ return FALSE;
+}
+
/*
* find out which action we need to take when encountering an unauthenticated request
*/
@@ -982,9 +1002,7 @@ static int oidc_handle_unauthenticated_user(request_rec *r, oidc_cfg *c) {
* won't redirect the user and thus avoid creating a state cookie
* for a non-browser (= Javascript) call that will never return from the OP
*/
- if ((oidc_util_hdr_in_x_requested_with_get(r) != NULL)
- && (apr_strnatcasecmp(oidc_util_hdr_in_x_requested_with_get(r),
- OIDC_HTTP_HDR_VAL_XML_HTTP_REQUEST) == 0))
+ if (oidc_is_xml_http_request(r) == TRUE)
return HTTP_UNAUTHORIZED;
}
@@ -997,17 +1015,6 @@ static int oidc_handle_unauthenticated_user(request_rec *r, oidc_cfg *c) {
oidc_dir_cfg_path_scope(r));
}
-/*
- * see if this is a non-browser request
- */
-static apr_byte_t oidc_is_xml_http_request(request_rec *r) {
- if ((oidc_util_hdr_in_x_requested_with_get(r) != NULL)
- && (apr_strnatcasecmp(oidc_util_hdr_in_x_requested_with_get(r),
- OIDC_HTTP_HDR_VAL_XML_HTTP_REQUEST) == 0))
- return TRUE;
- return FALSE;
-}
-
/*
* check if maximum session duration was exceeded
*/
diff --git a/src/mod_auth_openidc.h b/src/mod_auth_openidc.h
index 5162ed4..f89f392 100644
--- a/src/mod_auth_openidc.h
+++ b/src/mod_auth_openidc.h
@@ -537,7 +537,9 @@ apr_byte_t oidc_oauth_get_bearer_token(request_rec *r, const char **access_token
#define OIDC_CONTENT_TYPE_JWT "application/jwt"
#define OIDC_CONTENT_TYPE_FORM_ENCODED "application/x-www-form-urlencoded"
#define OIDC_CONTENT_TYPE_IMAGE_PNG "image/png"
-#define OIDC_CONTENT_TYPE_HTML "text/html"
+#define OIDC_CONTENT_TYPE_TEXT_HTML "text/html"
+#define OIDC_CONTENT_TYPE_APP_XHTML_XML "application/xhtml+xml"
+#define OIDC_CONTENT_TYPE_ANY "*/*"
#define OIDC_STR_SPACE " "
#define OIDC_STR_EQUAL "="
@@ -560,6 +562,7 @@ apr_byte_t oidc_oauth_get_bearer_token(request_rec *r, const char **access_token
#define OIDC_CHAR_FORWARD_SLASH '/'
#define OIDC_CHAR_PIPE '|'
#define OIDC_CHAR_AMP '&'
+#define OIDC_CHAR_SEMI_COLON ';'
#define OIDC_APP_INFO_REFRESH_TOKEN "refresh_token"
#define OIDC_APP_INFO_ACCESS_TOKEN "access_token"
@@ -787,6 +790,7 @@ const char *oidc_util_hdr_in_host_get(const request_rec *r);
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);
// oidc_metadata.c
apr_byte_t oidc_metadata_provider_retrieve(request_rec *r, oidc_cfg *cfg, const char *issuer, const char *url, json_t **j_metadata, char **response);
diff --git a/src/util.c b/src/util.c
index 2fd79ec..67b2fc3 100644
--- a/src/util.c
+++ b/src/util.c
@@ -97,7 +97,7 @@ int oidc_base64url_encode(request_rec *r, char **dst, const char *src,
enc_len--;
if ((enc_len > 0) && (enc[enc_len - 1] == ','))
enc_len--;
- if ((enc_len > 0) &&(enc[enc_len - 1] == ','))
+ if ((enc_len > 0) && (enc[enc_len - 1] == ','))
enc_len--;
enc[enc_len] = '\0';
}
@@ -320,9 +320,9 @@ char *oidc_util_unescape_string(const request_rec *r, const char *str) {
return NULL;
}
int counter = 0;
- char *replaced = (char *)str;
- while(str[counter] != '\0') {
- if(str[counter] == '+') {
+ char *replaced = (char *) str;
+ while (str[counter] != '\0') {
+ if (str[counter] == '+') {
replaced[counter] = ' ';
}
counter++;
@@ -353,7 +353,7 @@ char *oidc_util_html_escape(apr_pool_t *pool, const char *s) {
for (i = 0; i < strlen(s); i++) {
for (n = 0; n < len; n++) {
if (s[i] == chars[n]) {
- m = (unsigned int)strlen(replace[n]);
+ m = (unsigned int) strlen(replace[n]);
for (k = 0; k < m; k++)
r[j + k] = replace[n][k];
j += m;
@@ -530,12 +530,13 @@ const char *oidc_get_redirect_uri_iss(request_rec *r, oidc_cfg *cfg,
const char *redirect_uri = oidc_get_redirect_uri(r, cfg);
if (provider->issuer_specific_redirect_uri != 0) {
redirect_uri = apr_psprintf(r->pool, "%s%s%s=%s", redirect_uri,
- strchr(redirect_uri ? redirect_uri : "", OIDC_CHAR_QUERY) != NULL ?
- OIDC_STR_AMP :
- OIDC_STR_QUERY,
- OIDC_PROTO_ISS, oidc_util_escape_string(r, provider->issuer));
-// OIDC_PROTO_CLIENT_ID,
-// oidc_util_escape_string(r, provider->client_id));
+ strchr(redirect_uri ? redirect_uri : "",
+ OIDC_CHAR_QUERY) != NULL ?
+ OIDC_STR_AMP :
+ OIDC_STR_QUERY,
+ OIDC_PROTO_ISS, oidc_util_escape_string(r, provider->issuer));
+ // OIDC_PROTO_CLIENT_ID,
+ // oidc_util_escape_string(r, provider->client_id));
oidc_debug(r, "determined issuer specific redirect uri: %s",
redirect_uri);
}
@@ -1346,8 +1347,8 @@ int oidc_util_html_send(request_rec *r, const char *title,
on_load ? apr_psprintf(r->pool, " onload=\"%s()\"", on_load) : "",
html_body ? html_body : "<p></p>");
- return oidc_util_http_send(r, html, strlen(html), OIDC_CONTENT_TYPE_HTML,
- status_code);
+ return oidc_util_http_send(r, html, strlen(html),
+ OIDC_CONTENT_TYPE_TEXT_HTML, status_code);
}
static char *html_error_template_contents = NULL;
@@ -1357,7 +1358,8 @@ static char *html_error_template_contents = NULL;
* that is relative to the Apache root directory
*/
char *oidc_util_get_full_path(apr_pool_t *pool, const char *abs_or_rel_filename) {
- return (abs_or_rel_filename) ? ap_server_root_relative(pool, abs_or_rel_filename) : NULL;
+ return (abs_or_rel_filename) ?
+ ap_server_root_relative(pool, abs_or_rel_filename) : NULL;
}
/*
@@ -1389,7 +1391,7 @@ int oidc_util_html_send_error(request_rec *r, const char *html_template,
description ? description : ""));
return oidc_util_http_send(r, html, strlen(html),
- OIDC_CONTENT_TYPE_HTML, status_code);
+ OIDC_CONTENT_TYPE_TEXT_HTML, status_code);
}
}
@@ -1980,8 +1982,8 @@ void oidc_util_table_add_query_encoded_params(apr_pool_t *pool,
* create a symmetric key from a client_secret
*/
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) {
+ const char *client_secret, unsigned int r_key_len,
+ const char *hash_algo, apr_byte_t set_kid, oidc_jwk_t **jwk) {
oidc_jose_error_t err;
unsigned char *key = NULL;
unsigned int key_len;
@@ -2216,7 +2218,8 @@ static const char *oidc_util_hdr_in_get(const request_rec *r, const char *name)
return value;
}
-static const char *oidc_util_hdr_in_get_left_most_only(const request_rec *r, const char *name, const char *separator) {
+static const char *oidc_util_hdr_in_get_left_most_only(const request_rec *r,
+ const char *name, const char *separator) {
char *last = NULL;
const char *value = oidc_util_hdr_in_get(r, name);
if (value)
@@ -2224,6 +2227,29 @@ static const char *oidc_util_hdr_in_get_left_most_only(const request_rec *r, con
return NULL;
}
+static apr_byte_t oidc_util_hdr_in_contains(const request_rec *r,
+ const char *name, const char *separator, const char postfix_separator,
+ const char *needle) {
+ char *ctx = NULL, *elem = NULL;
+ const char *value = oidc_util_hdr_in_get(r, name);
+ apr_byte_t rc = FALSE;
+ if (value) {
+ elem = apr_strtok(apr_pstrdup(r->pool, value), separator, &ctx);
+ while (elem != NULL) {
+ while (*elem == OIDC_CHAR_SPACE)
+ elem++;
+ if ((strncmp(elem, needle, strlen(needle)) == 0)
+ && ((elem[strlen(needle)] == '\0')
+ || (elem[strlen(needle)] == postfix_separator))) {
+ rc = TRUE;
+ break;
+ }
+ elem = apr_strtok(NULL, separator, &ctx);
+ }
+ }
+ return rc;
+}
+
static void oidc_util_hdr_table_set(const request_rec *r, apr_table_t *table,
const char *name, const char *value) {
@@ -2288,7 +2314,8 @@ const char *oidc_util_hdr_in_user_agent_get(const request_rec *r) {
}
const char *oidc_util_hdr_in_x_forwarded_for_get(const request_rec *r) {
- return oidc_util_hdr_in_get_left_most_only(r, OIDC_HTTP_HDR_X_FORWARDED_FOR, OIDC_STR_COMMA);
+ return oidc_util_hdr_in_get_left_most_only(r, OIDC_HTTP_HDR_X_FORWARDED_FOR,
+ OIDC_STR_COMMA);
}
const char *oidc_util_hdr_in_content_type_get(const request_rec *r) {
@@ -2303,20 +2330,29 @@ const char *oidc_util_hdr_in_accept_get(const request_rec *r) {
return oidc_util_hdr_in_get(r, OIDC_HTTP_HDR_ACCEPT);
}
+apr_byte_t oidc_util_hdr_in_accept_contains(const request_rec *r,
+ const char *needle) {
+ return oidc_util_hdr_in_contains(r, OIDC_HTTP_HDR_ACCEPT, OIDC_STR_COMMA,
+ OIDC_CHAR_SEMI_COLON, needle);
+}
+
const char *oidc_util_hdr_in_authorization_get(const request_rec *r) {
return oidc_util_hdr_in_get(r, OIDC_HTTP_HDR_AUTHORIZATION);
}
const char *oidc_util_hdr_in_x_forwarded_proto_get(const request_rec *r) {
- return oidc_util_hdr_in_get_left_most_only(r, OIDC_HTTP_HDR_X_FORWARDED_PROTO, OIDC_STR_COMMA);
+ return oidc_util_hdr_in_get_left_most_only(r,
+ OIDC_HTTP_HDR_X_FORWARDED_PROTO, OIDC_STR_COMMA);
}
const char *oidc_util_hdr_in_x_forwarded_port_get(const request_rec *r) {
- return oidc_util_hdr_in_get_left_most_only(r, OIDC_HTTP_HDR_X_FORWARDED_PORT, OIDC_STR_COMMA);
+ return oidc_util_hdr_in_get_left_most_only(r,
+ OIDC_HTTP_HDR_X_FORWARDED_PORT, OIDC_STR_COMMA);
}
const char *oidc_util_hdr_in_x_forwarded_host_get(const request_rec *r) {
- return oidc_util_hdr_in_get_left_most_only(r, OIDC_HTTP_HDR_X_FORWARDED_HOST, OIDC_STR_COMMA);
+ return oidc_util_hdr_in_get_left_most_only(r,
+ OIDC_HTTP_HDR_X_FORWARDED_HOST, OIDC_STR_COMMA);
}
const char *oidc_util_hdr_in_host_get(const request_rec *r) {

33
SOURCES/0011-oops-document-OIDCStateMaxNumberOfCookies-for-releas.patch Normal file
View File

@ -0,0 +1,33 @@
From 7f81371f55a4a28000675023ad949e217d22bea3 Mon Sep 17 00:00:00 2001
From: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
Date: Wed, 12 Sep 2018 20:05:34 +0200
Subject: [PATCH 11/11] oops: document OIDCStateMaxNumberOfCookies for release
2.3.8
Signed-off-by: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
(cherry picked from commit 406a4915d3747d4cfdc2283f287a99ba1c7b446a)
---
auth_openidc.conf | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/auth_openidc.conf b/auth_openidc.conf
index eb83c24..48dd027 100644
--- a/auth_openidc.conf
+++ b/auth_openidc.conf
@@ -446,6 +446,13 @@
# When not defined, no cookies are stripped.
#OIDCStripCookies [<cookie-name>]+
+# Specify the maximum number of state cookies i.e. the maximum number of parallel outstanding
+# authentication requests. See: https://github.com/zmartzone/mod_auth_openidc/issues/331
+# Setting this to 0 means unlimited, until the browser or server gives up which is the
+# behavior of mod_auth_openidc < 2.3.8, which did not have this configuration option.
+# When not defined, the default is 7.
+#OIDCStateMaxNumberOfCookies <number>
+
########################################################################################
#
# Session Settings (only relevant in an OpenID Connect Relying Party setup)
--
2.26.2

285
SOURCES/0012-optionally-delete-the-oldest-state-cookie-s-see-399.patch Normal file
View File

@ -0,0 +1,285 @@
From 95baad3342aa7ef7172ad4922eb9f0d605dc469b Mon Sep 17 00:00:00 2001
From: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
Date: Thu, 6 Dec 2018 14:55:44 +0100
Subject: [PATCH 12/13] optionally delete the oldest state cookie(s); see #399
bump to 2.3.10rc3
Signed-off-by: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
(cherry picked from commit 46758a75eef8e3c91f2917fc7c6302136eb18809)
---
auth_openidc.conf | 8 +++--
src/config.c | 27 +++++++++++++----
src/mod_auth_openidc.c | 66 +++++++++++++++++++++++++++++++++++++++---
src/mod_auth_openidc.h | 2 ++
src/parse.c | 9 ++++--
src/parse.h | 2 +-
6 files changed, 100 insertions(+), 14 deletions(-)
diff --git a/auth_openidc.conf b/auth_openidc.conf
index 48dd027..33cea64 100644
--- a/auth_openidc.conf
+++ b/auth_openidc.conf
@@ -450,8 +450,12 @@
# authentication requests. See: https://github.com/zmartzone/mod_auth_openidc/issues/331
# Setting this to 0 means unlimited, until the browser or server gives up which is the
# behavior of mod_auth_openidc < 2.3.8, which did not have this configuration option.
-# When not defined, the default is 7.
-#OIDCStateMaxNumberOfCookies <number>
+#
+# The optional second boolean parameter if the oldest state cookie(s) will be deleted,
+# even if still valid; see #399.
+#
+# When not defined, the default is 7 and "false", thus the oldest cookie(s) will not be deleted.
+#OIDCStateMaxNumberOfCookies <number> [false|true]
########################################################################################
#
diff --git a/src/config.c b/src/config.c
index 6fa6227..8e56716 100644
--- a/src/config.c
+++ b/src/config.c
@@ -106,6 +106,8 @@
#define OIDC_DEFAULT_STATE_TIMEOUT 300
/* maximum number of parallel state cookies; 0 means unlimited, until the browser or server gives up */
#define OIDC_DEFAULT_MAX_NUMBER_OF_STATE_COOKIES 7
+/* default setting for deleting the oldest state cookies */
+#define OIDC_DEFAULT_DELETE_OLDEST_STATE_COOKIES 0
/* default session inactivity timeout */
#define OIDC_DEFAULT_SESSION_INACTIVITY_TIMEOUT 300
/* default session max duration */
@@ -1001,11 +1003,12 @@ static const char *oidc_set_client_auth_bearer_token(cmd_parms *cmd,
* set the maximun number of parallel state cookies
*/
static const char *oidc_set_max_number_of_state_cookies(cmd_parms *cmd,
- void *struct_ptr, const char *arg) {
+ void *struct_ptr, const char *arg1, const char *arg2) {
oidc_cfg *cfg = (oidc_cfg *) ap_get_module_config(
cmd->server->module_config, &auth_openidc_module);
- const char *rv = oidc_parse_max_number_of_state_cookies(cmd->pool, arg,
- &cfg->max_number_of_state_cookies);
+ const char *rv = oidc_parse_max_number_of_state_cookies(cmd->pool, arg1,
+ arg2, &cfg->max_number_of_state_cookies,
+ &cfg->delete_oldest_state_cookies);
return OIDC_CONFIG_DIR_RV(cmd, rv);
}
@@ -1018,6 +1021,15 @@ int oidc_cfg_max_number_of_state_cookies(oidc_cfg *cfg) {
return cfg->max_number_of_state_cookies;
}
+/*
+ * return the number of oldest state cookies that need to be deleted
+ */
+int oidc_cfg_delete_oldest_state_cookies(oidc_cfg *cfg) {
+ if (cfg->delete_oldest_state_cookies == OIDC_CONFIG_POS_INT_UNSET)
+ return OIDC_DEFAULT_DELETE_OLDEST_STATE_COOKIES;
+ return cfg->delete_oldest_state_cookies;
+}
+
/*
* create a new server config record with defaults
*/
@@ -1127,6 +1139,7 @@ void *oidc_create_server_config(apr_pool_t *pool, server_rec *svr) {
c->http_timeout_short = OIDC_DEFAULT_HTTP_TIMEOUT_SHORT;
c->state_timeout = OIDC_DEFAULT_STATE_TIMEOUT;
c->max_number_of_state_cookies = OIDC_CONFIG_POS_INT_UNSET;
+ c->delete_oldest_state_cookies = OIDC_CONFIG_POS_INT_UNSET;
c->session_inactivity_timeout = OIDC_DEFAULT_SESSION_INACTIVITY_TIMEOUT;
c->cookie_domain = NULL;
@@ -1445,6 +1458,10 @@ void *oidc_merge_server_config(apr_pool_t *pool, void *BASE, void *ADD) {
add->max_number_of_state_cookies != OIDC_CONFIG_POS_INT_UNSET ?
add->max_number_of_state_cookies :
base->max_number_of_state_cookies;
+ c->delete_oldest_state_cookies =
+ add->delete_oldest_state_cookies != OIDC_CONFIG_POS_INT_UNSET ?
+ add->delete_oldest_state_cookies :
+ base->delete_oldest_state_cookies;
c->session_inactivity_timeout =
add->session_inactivity_timeout
!= OIDC_DEFAULT_SESSION_INACTIVITY_TIMEOUT ?
@@ -2656,11 +2673,11 @@ const command_rec oidc_config_cmds[] = {
(void*)APR_OFFSETOF(oidc_cfg, state_timeout),
RSRC_CONF,
"Time to live in seconds for state parameter (cq. interval in which the authorization request and the corresponding response need to be completed)."),
- AP_INIT_TAKE1(OIDCStateMaxNumberOfCookies,
+ AP_INIT_TAKE12(OIDCStateMaxNumberOfCookies,
oidc_set_max_number_of_state_cookies,
(void*)APR_OFFSETOF(oidc_cfg, max_number_of_state_cookies),
RSRC_CONF,
- "Maximun number of parallel state cookies i.e. outstanding authorization requests."),
+ "Maximun number of parallel state cookies i.e. outstanding authorization requests and whether to delete the oldest cookie(s)."),
AP_INIT_TAKE1(OIDCSessionInactivityTimeout,
oidc_set_session_inactivity_timeout,
(void*)APR_OFFSETOF(oidc_cfg, session_inactivity_timeout),
diff --git a/src/mod_auth_openidc.c b/src/mod_auth_openidc.c
index 897a449..8740e02 100644
--- a/src/mod_auth_openidc.c
+++ b/src/mod_auth_openidc.c
@@ -686,15 +686,53 @@ static apr_byte_t oidc_unsolicited_proto_state(request_rec *r, oidc_cfg *c,
return TRUE;
}
+typedef struct oidc_state_cookies_t {
+ char *name;
+ apr_time_t timestamp;
+ struct oidc_state_cookies_t *next;
+} oidc_state_cookies_t;
+
+static int oidc_delete_oldest_state_cookies(request_rec *r,
+ int number_of_valid_state_cookies, int max_number_of_state_cookies,
+ oidc_state_cookies_t *first) {
+ oidc_state_cookies_t *cur = NULL, *prev = NULL, *prev_oldest = NULL,
+ *oldest = NULL;
+ while (number_of_valid_state_cookies >= max_number_of_state_cookies) {
+ oldest = first;
+ prev_oldest = NULL;
+ prev = first;
+ cur = first->next;
+ while (cur) {
+ if ((cur->timestamp < oldest->timestamp)) {
+ oldest = cur;
+ prev_oldest = prev;
+ }
+ prev = cur;
+ cur = cur->next;
+ }
+ oidc_warn(r,
+ "deleting oldest state cookie: %s (time until expiry " APR_TIME_T_FMT " seconds)",
+ oldest->name, apr_time_sec(oldest->timestamp - apr_time_now()));
+ oidc_util_set_cookie(r, oldest->name, "", 0, NULL);
+ if (prev_oldest)
+ prev_oldest->next = oldest->next;
+ else
+ first = first->next;
+ number_of_valid_state_cookies--;
+ }
+ return number_of_valid_state_cookies;
+}
+
/*
* clean state cookies that have expired i.e. for outstanding requests that will never return
* successfully and return the number of remaining valid cookies/outstanding-requests while
* doing so
*/
static int oidc_clean_expired_state_cookies(request_rec *r, oidc_cfg *c,
- const char *currentCookieName) {
+ const char *currentCookieName, int delete_oldest) {
int number_of_valid_state_cookies = 0;
- char *cookie, *tokenizerCtx;
+ oidc_state_cookies_t *first = NULL, *last = NULL;
+ char *cookie, *tokenizerCtx = NULL;
char *cookies = apr_pstrdup(r->pool, oidc_util_hdr_in_cookie_get(r));
if (cookies != NULL) {
cookie = apr_strtok(cookies, OIDC_STR_SEMI_COLON, &tokenizerCtx);
@@ -722,6 +760,18 @@ static int oidc_clean_expired_state_cookies(request_rec *r, oidc_cfg *c,
oidc_util_set_cookie(r, cookieName, "", 0,
NULL);
} else {
+ if (first == NULL) {
+ first = apr_pcalloc(r->pool,
+ sizeof(oidc_state_cookies_t));
+ last = first;
+ } else {
+ last->next = apr_pcalloc(r->pool,
+ sizeof(oidc_state_cookies_t));
+ last = last->next;
+ }
+ last->name = cookieName;
+ last->timestamp = ts;
+ last->next = NULL;
number_of_valid_state_cookies++;
}
oidc_proto_state_destroy(proto_state);
@@ -732,6 +782,12 @@ static int oidc_clean_expired_state_cookies(request_rec *r, oidc_cfg *c,
cookie = apr_strtok(NULL, OIDC_STR_SEMI_COLON, &tokenizerCtx);
}
}
+
+ if (delete_oldest > 0)
+ number_of_valid_state_cookies = oidc_delete_oldest_state_cookies(r,
+ number_of_valid_state_cookies, c->max_number_of_state_cookies,
+ first);
+
return number_of_valid_state_cookies;
}
@@ -746,7 +802,7 @@ static apr_byte_t oidc_restore_proto_state(request_rec *r, oidc_cfg *c,
const char *cookieName = oidc_get_state_cookie_name(r, state);
/* clean expired state cookies to avoid pollution */
- oidc_clean_expired_state_cookies(r, c, cookieName);
+ oidc_clean_expired_state_cookies(r, c, cookieName, FALSE);
/* get the state cookie value first */
char *cookieValue = oidc_util_get_cookie(r, cookieName);
@@ -820,10 +876,12 @@ static int oidc_authorization_request_set_cookie(request_rec *r, oidc_cfg *c,
* clean expired state cookies to avoid pollution and optionally
* try to avoid the number of state cookies exceeding a max
*/
- int number_of_cookies = oidc_clean_expired_state_cookies(r, c, NULL);
+ int number_of_cookies = oidc_clean_expired_state_cookies(r, c, NULL,
+ oidc_cfg_delete_oldest_state_cookies(c));
int max_number_of_cookies = oidc_cfg_max_number_of_state_cookies(c);
if ((max_number_of_cookies > 0)
&& (number_of_cookies >= max_number_of_cookies)) {
+
oidc_warn(r,
"the number of existing, valid state cookies (%d) has exceeded the limit (%d), no additional authorization request + state cookie can be generated, aborting the request",
number_of_cookies, max_number_of_cookies);
diff --git a/src/mod_auth_openidc.h b/src/mod_auth_openidc.h
index f89f392..c3a0a23 100644
--- a/src/mod_auth_openidc.h
+++ b/src/mod_auth_openidc.h
@@ -380,6 +380,7 @@ typedef struct oidc_cfg {
int http_timeout_short;
int state_timeout;
int max_number_of_state_cookies;
+ int delete_oldest_state_cookies;
int session_inactivity_timeout;
int session_cache_fallback_to_cookie;
@@ -691,6 +692,7 @@ int oidc_cfg_session_cache_fallback_to_cookie(request_rec *r);
const char *oidc_parse_pkce_type(apr_pool_t *pool, const char *arg, oidc_proto_pkce_t **type);
const char *oidc_cfg_claim_prefix(request_rec *r);
int oidc_cfg_max_number_of_state_cookies(oidc_cfg *cfg);
+int oidc_cfg_delete_oldest_state_cookies(oidc_cfg *cfg);
// oidc_util.c
int oidc_strnenvcmp(const char *a, const char *b, int len);
diff --git a/src/parse.c b/src/parse.c
index 0f986fd..2d09584 100644
--- a/src/parse.c
+++ b/src/parse.c
@@ -1245,7 +1245,12 @@ const char *oidc_parse_auth_request_method(apr_pool_t *pool, const char *arg,
* parse the maximum number of parallel state cookies
*/
const char *oidc_parse_max_number_of_state_cookies(apr_pool_t *pool,
- const char *arg, int *int_value) {
- return oidc_parse_int_valid(pool, arg, int_value,
+ const char *arg1, const char *arg2, int *int_value, int *bool_value) {
+ const char *rv = NULL;
+
+ rv = oidc_parse_int_valid(pool, arg1, int_value,
oidc_valid_max_number_of_state_cookies);
+ if ((rv == NULL) && (arg2 != NULL))
+ rv = oidc_parse_boolean(pool, arg2, bool_value);
+ return rv;
}
diff --git a/src/parse.h b/src/parse.h
index 6355db4..bdf5651 100644
--- a/src/parse.h
+++ b/src/parse.h
@@ -117,7 +117,7 @@ const char *oidc_parse_info_hook_data(apr_pool_t *pool, const char *arg, apr_has
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, const char *arg, int *int_value);
+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);
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

240
SOURCES/0013-Allow-configuring-which-header-value-is-used-to-calc.patch Normal file
View File

@ -0,0 +1,240 @@
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

86
SOURCES/0014-add-value-of-OIDC_SET_COOKIE_APPEND-env-var-to-Set-C.patch Normal file
View File

@ -0,0 +1,86 @@
From 0bd084eb058361517b64a2c10a46c332adc9aeea Mon Sep 17 00:00:00 2001
From: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
Date: Wed, 15 Jan 2020 17:58:53 +0100
Subject: [PATCH 14/19] add value of OIDC_SET_COOKIE_APPEND env var to
Set-Cookie headers
- useful for handling changing/upcoming SameSite behaviors across
different browsers, e.g.:
SetEnvIf User-Agent ".*IOS.*" OIDC_SET_COOKIE_APPEND=SameSite=None
- bump to 2.4.1rc4
Signed-off-by: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
(cherry picked from commit a326dbe843a755124ecee883db52dcdc26284c26)
---
ChangeLog | 5 +++++
src/util.c | 27 +++++++++++++++++++++++++++
2 files changed, 32 insertions(+)
diff --git a/ChangeLog b/ChangeLog
index dfe4bd6..fc7c5ae 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+01/15/2020
+- add value of OIDC_SET_COOKIE_APPEND env var to Set-Cookie headers
+ useful for handling changing/upcoming SameSite behaviors across different browsers, e.g.:
+ SetEnvIf User-Agent ".*IOS.*" OIDC_SET_COOKIE_APPEND=SameSite=None
+
08/04/2018
- don't return content with 503 since it will turn the HTTP status code into a 200; see #331
diff --git a/src/util.c b/src/util.c
index 67b2fc3..993718e 100644
--- a/src/util.c
+++ b/src/util.c
@@ -914,6 +914,27 @@ static char *oidc_util_get_cookie_path(request_rec *r) {
#define OIDC_COOKIE_MAX_SIZE 4093
+#define OIDC_SET_COOKIE_APPEND_ENV_VAR "OIDC_SET_COOKIE_APPEND"
+
+const char *oidc_util_set_cookie_append_value(request_rec *r, oidc_cfg *c) {
+ const char *env_var_value = NULL;
+
+ if (r->subprocess_env != NULL)
+ env_var_value = apr_table_get(r->subprocess_env,
+ OIDC_SET_COOKIE_APPEND_ENV_VAR);
+
+ if (env_var_value == NULL) {
+ oidc_debug(r, "no cookie append environment variable %s found",
+ OIDC_SET_COOKIE_APPEND_ENV_VAR);
+ return NULL;
+ }
+
+ oidc_debug(r, "cookie append environment variable %s=%s found",
+ OIDC_SET_COOKIE_APPEND_ENV_VAR, env_var_value);
+
+ return env_var_value;
+}
+
/*
* set a cookie in the HTTP response headers
*/
@@ -923,6 +944,7 @@ void oidc_util_set_cookie(request_rec *r, const char *cookieName,
oidc_cfg *c = ap_get_module_config(r->server->module_config,
&auth_openidc_module);
char *headerString, *expiresString = NULL;
+ const char *appendString = NULL;
/* see if we need to clear the cookie */
if (apr_strnatcmp(cookieValue, "") == 0)
@@ -961,6 +983,11 @@ void oidc_util_set_cookie(request_rec *r, const char *cookieName,
if (ext != NULL)
headerString = apr_psprintf(r->pool, "%s; %s", headerString, ext);
+ appendString = oidc_util_set_cookie_append_value(r, c);
+ if (appendString != NULL)
+ headerString = apr_psprintf(r->pool, "%s; %s", headerString,
+ appendString);
+
/* sanity check on overall cookie value size */
if (strlen(headerString) > OIDC_COOKIE_MAX_SIZE) {
oidc_warn(r,
--
2.26.2

35
SOURCES/0015-pick-OIDC_SET_COOKIE_APPEND-over-ext-passed-in-to-oi.patch Normal file
View File

@ -0,0 +1,35 @@
From 914f700cd791d370cf363d408e938598023980dc Mon Sep 17 00:00:00 2001
From: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
Date: Sun, 19 Jan 2020 16:00:31 +0100
Subject: [PATCH 15/19] pick OIDC_SET_COOKIE_APPEND over ext passed in to
oidc_util_set_cookie
Signed-off-by: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
(cherry picked from commit 5aa73817172acbb9e86287a54bc4532af7e394ee)
---
src/util.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/util.c b/src/util.c
index 993718e..c1fa5f3 100644
--- a/src/util.c
+++ b/src/util.c
@@ -980,13 +980,12 @@ void oidc_util_set_cookie(request_rec *r, const char *cookieName,
headerString = apr_psprintf(r->pool, "%s; %s", headerString,
OIDC_COOKIE_FLAG_HTTP_ONLY);
- if (ext != NULL)
- headerString = apr_psprintf(r->pool, "%s; %s", headerString, ext);
-
appendString = oidc_util_set_cookie_append_value(r, c);
if (appendString != NULL)
headerString = apr_psprintf(r->pool, "%s; %s", headerString,
appendString);
+ else if (ext != NULL)
+ headerString = apr_psprintf(r->pool, "%s; %s", headerString, ext);
/* sanity check on overall cookie value size */
if (strlen(headerString) > OIDC_COOKIE_MAX_SIZE) {
--
2.26.2

95
SOURCES/0016-always-add-a-SameSite-value-to-the-Set-Cookie-header.patch Normal file
View File

@ -0,0 +1,95 @@
From 2c999448c87b286744ac9802cb8e4277d5c38b71 Mon Sep 17 00:00:00 2001
From: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
Date: Wed, 29 Jan 2020 13:27:44 +0100
Subject: [PATCH 16/19] always add a SameSite value to the Set-Cookie header
- to satisfy upcoming Chrome/Firefox changes
this can be overridden by using, e.g.:
SetEnvIf User-Agent ".*IOS.*" OIDC_SET_COOKIE_APPEND=;
- release 2.4.1rc6
Signed-off-by: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
(cherry picked from commit 3b4770f49cc67b9b0ae8732e9908895683ea556c)
---
ChangeLog | 5 +++++
src/mod_auth_openidc.c | 10 +++++++---
src/mod_auth_openidc.h | 1 +
src/session.c | 2 +-
4 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index fc7c5ae..b67f764 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+01/29/2020
+- always add a SameSite value to the Set-Cookie header to satisfy upcoming Chrome/Firefox changes
+ this can be overridden by using, e.g.:
+ SetEnvIf User-Agent ".*IOS.*" OIDC_SET_COOKIE_APPEND=;
+
01/15/2020
- add value of OIDC_SET_COOKIE_APPEND env var to Set-Cookie headers
useful for handling changing/upcoming SameSite behaviors across different browsers, e.g.:
diff --git a/src/mod_auth_openidc.c b/src/mod_auth_openidc.c
index 38558d2..0d2b37c 100644
--- a/src/mod_auth_openidc.c
+++ b/src/mod_auth_openidc.c
@@ -916,7 +916,9 @@ static int oidc_authorization_request_set_cookie(request_rec *r, oidc_cfg *c,
/* set it as a cookie */
oidc_util_set_cookie(r, cookieName, cookieValue, -1,
- c->cookie_same_site ? OIDC_COOKIE_EXT_SAME_SITE_LAX : NULL);
+ c->cookie_same_site ?
+ OIDC_COOKIE_EXT_SAME_SITE_LAX :
+ OIDC_COOKIE_EXT_SAME_SITE_NONE);
return HTTP_OK;
}
@@ -2183,7 +2185,7 @@ static int oidc_discovery(request_rec *r, oidc_cfg *cfg) {
oidc_util_set_cookie(r, OIDC_CSRF_NAME, csrf, -1,
cfg->cookie_same_site ?
OIDC_COOKIE_EXT_SAME_SITE_STRICT :
- NULL);
+ OIDC_COOKIE_EXT_SAME_SITE_NONE);
/* see if we need to preserve POST parameters through Javascript/HTML5 storage */
if (oidc_post_preserve_javascript(r, url, NULL, NULL) == TRUE)
@@ -2276,7 +2278,9 @@ static int oidc_discovery(request_rec *r, oidc_cfg *cfg) {
s = apr_psprintf(r->pool, "%s</form>\n", s);
oidc_util_set_cookie(r, OIDC_CSRF_NAME, csrf, -1,
- cfg->cookie_same_site ? OIDC_COOKIE_EXT_SAME_SITE_STRICT : NULL);
+ cfg->cookie_same_site ?
+ OIDC_COOKIE_EXT_SAME_SITE_STRICT :
+ OIDC_COOKIE_EXT_SAME_SITE_NONE);
char *javascript = NULL, *javascript_method = NULL;
char *html_head =
diff --git a/src/mod_auth_openidc.h b/src/mod_auth_openidc.h
index fada56d..5f1a79a 100644
--- a/src/mod_auth_openidc.h
+++ b/src/mod_auth_openidc.h
@@ -213,6 +213,7 @@ APLOG_USE_MODULE(auth_openidc);
#define OIDC_COOKIE_EXT_SAME_SITE_LAX "SameSite=Lax"
#define OIDC_COOKIE_EXT_SAME_SITE_STRICT "SameSite=Strict"
+#define OIDC_COOKIE_EXT_SAME_SITE_NONE "SameSite=None"
/* https://tools.ietf.org/html/draft-ietf-tokbind-ttrp-01 */
#define OIDC_TB_CFG_PROVIDED_ENV_VAR "Sec-Provided-Token-Binding-ID"
diff --git a/src/session.c b/src/session.c
index 1c6e118..cd9ccb8 100644
--- a/src/session.c
+++ b/src/session.c
@@ -204,7 +204,7 @@ static apr_byte_t oidc_session_save_cache(request_rec *r, oidc_session_t *z,
(first_time ?
OIDC_COOKIE_EXT_SAME_SITE_LAX :
OIDC_COOKIE_EXT_SAME_SITE_STRICT) :
- NULL);
+ OIDC_COOKIE_EXT_SAME_SITE_NONE);
} else {
/* clear the cookie */
--
2.26.2

42
SOURCES/0017-fix-also-add-SameSite-None-to-by-value-session-cooki.patch Normal file
View File

@ -0,0 +1,42 @@
From ca43d64e722f80ed91871c9ea31fbc7660aa9147 Mon Sep 17 00:00:00 2001
From: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
Date: Mon, 3 Feb 2020 10:34:10 +0100
Subject: [PATCH 17/19] fix: also add SameSite=None to by-value session cookies
bump to 2.4.2rc0
Signed-off-by: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
(cherry picked from commit f6798246abc8fd8f865db313439882ac9f5771f3)
---
ChangeLog | 4 ++++
src/session.c | 2 +-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/ChangeLog b/ChangeLog
index b67f764..3db7110 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+02/03/2020
+- fix: also add SameSite=None to by-value session cookies
+- bump to 2.4.2rc0
+
01/29/2020
- always add a SameSite value to the Set-Cookie header to satisfy upcoming Chrome/Firefox changes
this can be overridden by using, e.g.:
diff --git a/src/session.c b/src/session.c
index cd9ccb8..e7194bd 100644
--- a/src/session.c
+++ b/src/session.c
@@ -249,7 +249,7 @@ static apr_byte_t oidc_session_save_cookie(request_rec *r, oidc_session_t *z,
(first_time ?
OIDC_COOKIE_EXT_SAME_SITE_LAX :
OIDC_COOKIE_EXT_SAME_SITE_STRICT) :
- NULL);
+ OIDC_COOKIE_EXT_SAME_SITE_NONE);
return TRUE;
}
--
2.26.2

32
SOURCES/0018-add-note-on-usage-of-OIDC_SET_COOKIE_APPEND-in-the-s.patch Normal file
View File

@ -0,0 +1,32 @@
From d2f6572e93446d611fc66cf68d0b71cd13366d55 Mon Sep 17 00:00:00 2001
From: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
Date: Thu, 30 Jul 2020 10:10:04 +0200
Subject: [PATCH 18/19] add note on usage of OIDC_SET_COOKIE_APPEND in the
sample config/doc
Signed-off-by: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
(cherry picked from commit bcbdd1993e7449446cb34df696826bd8bc9d2977)
---
auth_openidc.conf | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/auth_openidc.conf b/auth_openidc.conf
index 4012df3..ce2fba7 100644
--- a/auth_openidc.conf
+++ b/auth_openidc.conf
@@ -431,6 +431,12 @@
# state cookie: Lax
# session cookie: first time set Lax, updates (e.g. after inactivity timeout) Strict
# x_csrf discovery: Strict:
+#
+# The default `SameSite=None` cookie appendix on `Set-Cookie` response headers can be
+# conditionally overridden using an environment variable in the Apache config as in:
+# SetEnvIf User-Agent ".*IOS.*" OIDC_SET_COOKIE_APPEND=;
+# (since version 2.4.1)
+#
# When not defined the default is Off.
#OIDCCookieSameSite [On|Off]
--
2.26.2

204
SOURCES/0019-add-SameSite-attribute-on-cookie-clearance-logout.patch Normal file
View File

@ -0,0 +1,204 @@
From b68ac577b465f54e7eb38e48a65e5cf4c88c74c5 Mon Sep 17 00:00:00 2001
From: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
Date: Thu, 3 Sep 2020 16:52:30 +0200
Subject: [PATCH 19/19] add SameSite attribute on cookie clearance / logout
bump to 2.4.4.1; thanks @v0gler
Signed-off-by: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
(cherry picked from commit 314000d179c2d08af6897725f37980e1f0891aa6)
---
ChangeLog | 4 ++++
src/mod_auth_openidc.c | 42 +++++++++++++++++++++++-------------------
src/mod_auth_openidc.h | 5 +++++
src/session.c | 16 +++++++++-------
4 files changed, 41 insertions(+), 26 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 3db7110..eba2ebc 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+09/03/2020
+- add SameSite attribute on cookie clearance / logout; thanks @v0gler
+- bump to 2.4.4.1
+
02/03/2020
- fix: also add SameSite=None to by-value session cookies
- bump to 2.4.2rc0
diff --git a/src/mod_auth_openidc.c b/src/mod_auth_openidc.c
index 0d2b37c..68fa2ce 100644
--- a/src/mod_auth_openidc.c
+++ b/src/mod_auth_openidc.c
@@ -717,7 +717,8 @@ static int oidc_delete_oldest_state_cookies(request_rec *r,
oidc_warn(r,
"deleting oldest state cookie: %s (time until expiry " APR_TIME_T_FMT " seconds)",
oldest->name, apr_time_sec(oldest->timestamp - apr_time_now()));
- oidc_util_set_cookie(r, oldest->name, "", 0, NULL);
+ oidc_util_set_cookie(r, oldest->name, "", 0,
+ OIDC_COOKIE_EXT_SAME_SITE_NONE);
if (prev_oldest)
prev_oldest->next = oldest->next;
else
@@ -762,7 +763,7 @@ static int oidc_clean_expired_state_cookies(request_rec *r, oidc_cfg *c,
oidc_error(r, "state (%s) has expired",
cookieName);
oidc_util_set_cookie(r, cookieName, "", 0,
- NULL);
+ OIDC_COOKIE_EXT_SAME_SITE_NONE);
} else {
if (first == NULL) {
first = apr_pcalloc(r->pool,
@@ -779,6 +780,12 @@ static int oidc_clean_expired_state_cookies(request_rec *r, oidc_cfg *c,
number_of_valid_state_cookies++;
}
oidc_proto_state_destroy(proto_state);
+ } else {
+ oidc_warn(r,
+ "state cookie could not be retrieved/decoded, deleting: %s",
+ cookieName);
+ oidc_util_set_cookie(r, cookieName, "", 0,
+ OIDC_COOKIE_EXT_SAME_SITE_NONE);
}
}
}
@@ -816,7 +823,7 @@ static apr_byte_t oidc_restore_proto_state(request_rec *r, oidc_cfg *c,
}
/* clear state cookie because we don't need it anymore */
- oidc_util_set_cookie(r, cookieName, "", 0, NULL);
+ oidc_util_set_cookie(r, cookieName, "", 0, OIDC_COOKIE_EXT_SAME_SITE_NONE);
*proto_state = oidc_proto_state_from_cookie(r, c, cookieValue);
if (*proto_state == NULL)
@@ -916,9 +923,7 @@ static int oidc_authorization_request_set_cookie(request_rec *r, oidc_cfg *c,
/* set it as a cookie */
oidc_util_set_cookie(r, cookieName, cookieValue, -1,
- c->cookie_same_site ?
- OIDC_COOKIE_EXT_SAME_SITE_LAX :
- OIDC_COOKIE_EXT_SAME_SITE_NONE);
+ OIDC_COOKIE_SAMESITE_LAX(c));
return HTTP_OK;
}
@@ -2183,9 +2188,7 @@ static int oidc_discovery(request_rec *r, oidc_cfg *cfg) {
/* set CSRF cookie */
oidc_util_set_cookie(r, OIDC_CSRF_NAME, csrf, -1,
- cfg->cookie_same_site ?
- OIDC_COOKIE_EXT_SAME_SITE_STRICT :
- OIDC_COOKIE_EXT_SAME_SITE_NONE);
+ OIDC_COOKIE_SAMESITE_STRICT(cfg));
/* see if we need to preserve POST parameters through Javascript/HTML5 storage */
if (oidc_post_preserve_javascript(r, url, NULL, NULL) == TRUE)
@@ -2278,9 +2281,7 @@ static int oidc_discovery(request_rec *r, oidc_cfg *cfg) {
s = apr_psprintf(r->pool, "%s</form>\n", s);
oidc_util_set_cookie(r, OIDC_CSRF_NAME, csrf, -1,
- cfg->cookie_same_site ?
- OIDC_COOKIE_EXT_SAME_SITE_STRICT :
- OIDC_COOKIE_EXT_SAME_SITE_NONE);
+ OIDC_COOKIE_SAMESITE_STRICT(cfg));
char *javascript = NULL, *javascript_method = NULL;
char *html_head =
@@ -2501,7 +2502,8 @@ static int oidc_handle_discovery_response(request_rec *r, oidc_cfg *c) {
if (csrf_cookie) {
/* clean CSRF cookie */
- oidc_util_set_cookie(r, OIDC_CSRF_NAME, "", 0, NULL);
+ oidc_util_set_cookie(r, OIDC_CSRF_NAME, "", 0,
+ OIDC_COOKIE_EXT_SAME_SITE_NONE);
/* compare CSRF cookie value with query parameter value */
if ((csrf_query == NULL)
@@ -2639,12 +2641,12 @@ static int oidc_handle_logout_request(request_rec *r, oidc_cfg *c,
oidc_debug(r, "enter (url=%s)", url);
- /* if there's no remote_user then there's no (stored) session to kill */
- if (session->remote_user != NULL) {
-
- /* remove session state (cq. cache entry and cookie) */
- oidc_session_kill(r, session);
- }
+ /*
+ * remove session state (cq. cache entry and cookie)
+ * always clear the session cookie because the cookie may be not sent (but still in the browser)
+ * due to SameSite policies
+ */
+ oidc_session_kill(r, session);
/* see if this is the OP calling us */
if (oidc_is_front_channel_logout(url)) {
@@ -2661,6 +2663,8 @@ static int oidc_handle_logout_request(request_rec *r, oidc_cfg *c,
const char *accept = oidc_util_hdr_in_accept_get(r);
if ((apr_strnatcmp(url, OIDC_IMG_STYLE_LOGOUT_PARAM_VALUE) == 0)
|| ((accept) && strstr(accept, OIDC_CONTENT_TYPE_IMAGE_PNG))) {
+ // terminate with DONE instead of OK
+ // to avoid Apache returning auth/authz error 401 for the redirect URI
return oidc_util_http_send(r,
(const char *) &oidc_transparent_pixel,
sizeof(oidc_transparent_pixel), OIDC_CONTENT_TYPE_IMAGE_PNG,
diff --git a/src/mod_auth_openidc.h b/src/mod_auth_openidc.h
index 5f1a79a..6821d0c 100644
--- a/src/mod_auth_openidc.h
+++ b/src/mod_auth_openidc.h
@@ -215,6 +215,11 @@ APLOG_USE_MODULE(auth_openidc);
#define OIDC_COOKIE_EXT_SAME_SITE_STRICT "SameSite=Strict"
#define OIDC_COOKIE_EXT_SAME_SITE_NONE "SameSite=None"
+#define OIDC_COOKIE_SAMESITE_STRICT(c) \
+ c->cookie_same_site ? OIDC_COOKIE_EXT_SAME_SITE_STRICT : OIDC_COOKIE_EXT_SAME_SITE_NONE
+#define OIDC_COOKIE_SAMESITE_LAX(c) \
+ c->cookie_same_site ? OIDC_COOKIE_EXT_SAME_SITE_LAX : OIDC_COOKIE_EXT_SAME_SITE_NONE
+
/* https://tools.ietf.org/html/draft-ietf-tokbind-ttrp-01 */
#define OIDC_TB_CFG_PROVIDED_ENV_VAR "Sec-Provided-Token-Binding-ID"
diff --git a/src/session.c b/src/session.c
index e7194bd..539ea21 100644
--- a/src/session.c
+++ b/src/session.c
@@ -155,7 +155,7 @@ static apr_byte_t oidc_session_load_cache(request_rec *r, oidc_session_t *z) {
/* delete the session cookie */
oidc_util_set_cookie(r, oidc_cfg_dir_cookie(r), "", 0,
- NULL);
+ OIDC_COOKIE_EXT_SAME_SITE_NONE);
/* delete the cache entry */
rc = oidc_cache_set_session(r, z->uuid, NULL, 0);
/* clear the session */
@@ -208,7 +208,8 @@ static apr_byte_t oidc_session_save_cache(request_rec *r, oidc_session_t *z,
} else {
/* clear the cookie */
- oidc_util_set_cookie(r, oidc_cfg_dir_cookie(r), "", 0, NULL);
+ oidc_util_set_cookie(r, oidc_cfg_dir_cookie(r), "", 0,
+ OIDC_COOKIE_EXT_SAME_SITE_NONE);
/* remove the session from the cache */
rc = oidc_cache_set_session(r, z->uuid, NULL, 0);
@@ -245,11 +246,12 @@ static apr_byte_t oidc_session_save_cookie(request_rec *r, oidc_session_t *z,
oidc_util_set_chunked_cookie(r, oidc_cfg_dir_cookie(r), cookieValue,
c->persistent_session_cookie ? z->expiry : -1,
c->session_cookie_chunk_size,
- c->cookie_same_site ?
- (first_time ?
- OIDC_COOKIE_EXT_SAME_SITE_LAX :
- OIDC_COOKIE_EXT_SAME_SITE_STRICT) :
- OIDC_COOKIE_EXT_SAME_SITE_NONE);
+ (z->state == NULL) ? OIDC_COOKIE_EXT_SAME_SITE_NONE :
+ c->cookie_same_site ?
+ (first_time ?
+ OIDC_COOKIE_EXT_SAME_SITE_LAX :
+ OIDC_COOKIE_EXT_SAME_SITE_STRICT) :
+ OIDC_COOKIE_EXT_SAME_SITE_NONE);
return TRUE;
}
--
2.26.2

167
SOURCES/test-segfault.patch Normal file
View File

@ -0,0 +1,167 @@
commit fe7dfb14c45262df3b15bda374b2ee390b43cfb4
Author: John Dennis <jdennis@redhat.com>
Date: Tue Aug 14 18:08:56 2018 -0400
test_proto_authorization_request() segfault due to uninitialized value
Many thanks to Florian Weimer <fweimer@redhat.com> for his assistence
in helping diagnose the problem.
In test_proto_authorization_request() it creates a provider object but
fails to fully initialize it. In particular it fails to initialize
auth_request_method to OIDC_AUTH_REQUEST_METHOD_GET.
The function oidc_proto_authorization_request() in the file
src/proto.c has a weak test for provider->auth_request_method on line
646.
if (provider->auth_request_method == OIDC_AUTH_REQUEST_METHOD_POST) {
/* construct a HTML POST auto-submit page with the authorization request parameters */
} else {
/* construct the full authorization request URL */
}
The assumption is provider->auth_request_method must be one of
OIDC_AUTH_REQUEST_METHOD_GET or OIDC_AUTH_REQUEST_METHOD_POST but if
the provider struct is not properly initialized it could be any random
value. Therefore the fact provider->auth_request_method does not equal
OIDC_AUTH_REQUEST_METHOD_POST does not imply it's
OIDC_AUTH_REQUEST_METHOD_GET. The test would also be a problem if
OIDC_AUTH_REQUEST_METHOD ever added a new enumerated value.
The defined values for OIDC_AUTH_REQUEST_METHOD are:
define OIDC_AUTH_REQUEST_METHOD_GET 0
define OIDC_AUTH_REQUEST_METHOD_POST 1
So what the test on line src/proto.c:646 is really saying is this:
if provider->auth_request_method != 1 then use the GET method.
The unit test works most of the time because it's unlikely the
unitialized auth_request_method member will be exactly 1. Except on
some architectures it is (e.g. s390x). Of course what's on the stack
is influenced by a variety of factors (all unknown).
The segfault occurs because oidc_proto_authorization_request() takes
the OIDC_AUTH_REQUEST_METHOD_POST branch and calls
oidc_proto_html_post() which then operates on more uninitialized
data. Specfically request->connection->bucket_alloc is
NULL. Fortunately the request object was intialized to zero via
apr_pcalloc() so at least bucket_alloc will consistetnly be NULL.
Here is the stack trace:
Program received signal SIGSEGV, Segmentation fault.
0x00007ffff6b9f67a in apr_bucket_alloc () from /lib64/libaprutil-1.so.0
(gdb) bt
from /lib64/libaprutil-1.so.0
data=0x6adfe0 "<!DOCTYPE HTML PUBLIC \"-//W3C//DTD HTML 4.01//EN\" \"http://www.w3.org/TR/html4/strict.dtd\">\n<html>\n <head>\n <meta http-equiv=\"Content-Type\" content=\"text/html; charset=utf-8\">\n <title>Submitting"...,
data_len=825, content_type=0x47311a "text/html", success_rvalue=-2)
at src/util.c:1307
title=0x46bf28 "Submitting...", html_head=0x0,
on_load=0x46bf0d "document.forms[0].submit()",
html_body=0x6add40 " <p>Submitting Authentication Request...</p>\n <form method=\"post\" action=\"https://idp.example.com/authorize\">\n <p>\n <input type=\"hidden\" name=\"response_type\" value=\"code\">\n <input"..., status_code=-2) at src/util.c:1349
url=0x465758 "https://idp.example.com/authorize", params=0x6acf30)
at src/proto.c:544
provider=0x7fffffffd260, login_hint=0x0,
redirect_uri=0x465790 "https://www.example.com/protected/",
state=0x4657b3 "12345", proto_state=0x68e5f0, id_token_hint=0x0,
nge=0x0, auth_request_params=0x0, path_scope=0x0) at src/proto.c:650
This patch does the following:
1) Initializes the provider struct created on the stack in
test_proto_authorization_request to zero so it's at least
initialized to known consistent values.
2) Initializes provider.auth_request_method to
OIDC_AUTH_REQUEST_METHOD_GET.
3) Initializes request->connection->bucket_alloc via
apr_bucket_alloc_create() so if one does enter that code it won't
segfault.
It's easy to verify the above observations.
If you explicitly intialize provider.auth_request_method to
OIDC_AUTH_REQUEST_METHOD_POST in test_proto_authorization_request()
instead of allowing it to be a random stack value you will segfault
every time. However if you initialize request->connection->bucket_alloc
the segfault goes away and the unit test correctly reports the error,
e.g.
Failed: # test_proto_authorization_request: error in oidc_proto_authorization_request (1): result "0" != expected "1"
WARNING: This patch does not address the test in proto.c:646. That
test should be augmented to assure only valid enumerated values are
operated on and if the enumerated value is not valid it should return
an error.
Note: The above was fixed in the following commit.
Signed-off-by: John Dennis <jdennis@redhat.com>
diff --git a/test/test.c b/test/test.c
index 16f09b5..87d3700 100755
--- a/test/test.c
+++ b/test/test.c
@@ -1019,6 +1019,9 @@ static char *test_proto_validate_code(request_rec *r) {
static char * test_proto_authorization_request(request_rec *r) {
oidc_provider_t provider;
+
+ memset(&provider, 0, sizeof(provider));
+
provider.issuer = "https://idp.example.com";
provider.authorization_endpoint_url = "https://idp.example.com/authorize";
provider.scope = "openid";
@@ -1028,6 +1031,8 @@ static char * test_proto_authorization_request(request_rec *r) {
provider.auth_request_params = NULL;
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/";
const char *state = "12345";
@@ -1260,6 +1265,7 @@ static request_rec * test_setup(apr_pool_t *pool) {
sizeof(struct process_rec));
request->server->process->pool = request->pool;
request->connection = apr_pcalloc(request->pool, sizeof(struct conn_rec));
+ request->connection->bucket_alloc = apr_bucket_alloc_create(request->pool);
request->connection->local_addr = apr_pcalloc(request->pool,
sizeof(apr_sockaddr_t));
commit aca77a82c1ce2f1ec8f363066ffbc480b3bd75c8
Author: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
Date: Wed Aug 15 07:47:57 2018 +0200
add sanity check on provider->auth_request_method; closes #382
thanks @jdennis; bump to 2.3.8rc4
Signed-off-by: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
diff --git a/src/proto.c b/src/proto.c
index e9dbc99..ac7696a 100644
--- a/src/proto.c
+++ b/src/proto.c
@@ -649,7 +649,7 @@ int oidc_proto_authorization_request(request_rec *r,
rv = oidc_proto_html_post(r, provider->authorization_endpoint_url,
params);
- } else {
+ } else if (provider->auth_request_method == OIDC_AUTH_REQUEST_METHOD_GET) {
/* construct the full authorization request URL */
authorization_request = oidc_util_http_query_encoded_url(r,
@@ -666,6 +666,10 @@ int oidc_proto_authorization_request(request_rec *r,
/* and tell Apache to return an HTTP Redirect (302) message */
rv = HTTP_MOVED_TEMPORARILY;
}
+ } else {
+ oidc_error(r, "provider->auth_request_method set to wrong value: %d",
+ provider->auth_request_method);
+ return HTTP_INTERNAL_SERVER_ERROR;
}
/* add a referred token binding request for the provider if enabled */

229
SPECS/mod_auth_openidc.spec Normal file
View File

@ -0,0 +1,229 @@
%{!?_httpd_mmn: %{expand: %%global _httpd_mmn %%(cat %{_includedir}/httpd/.mmn || echo 0-0)}}
%{!?_httpd_moddir: %{expand: %%global _httpd_moddir %%{_libdir}/httpd/modules}}
%{!?_httpd_confdir: %{expand: %%global _httpd_confdir %{_sysconfdir}/httpd/conf.d}}
# Optionally build with hiredis if --with hiredis is passed
%{!?_with_hiredis: %{!?_without_hiredis: %global _without_hiredis --without-hiredis}}
# It is an error if both or neither required options exist.
%{?_with_hiredis: %{?_without_hiredis: %{error: both _with_hiredis and _without_hiredis}}}
%{!?_with_hiredis: %{!?_without_hiredis: %{error: neither _with_hiredis nor _without_hiredis}}}
# /etc/httpd/conf.d with httpd < 2.4 and defined as /etc/httpd/conf.modules.d with httpd >= 2.4
%{!?_httpd_modconfdir: %{expand: %%global _httpd_modconfdir %%{_sysconfdir}/httpd/conf.d}}
%global httpd_pkg_cache_dir /var/cache/httpd/mod_auth_openidc
Name: mod_auth_openidc
Version: 2.3.7
Release: 8%{?dist}
Summary: OpenID Connect auth module for Apache HTTP Server
Group: System Environment/Daemons
License: ASL 2.0
URL: https://github.com/zmartzone/mod_auth_openidc
Source0: https://github.com/zmartzone/mod_auth_openidc/releases/download/v%{version}/mod_auth_openidc-%{version}.tar.gz
Patch1: test-segfault.patch
Patch2: 0002-Backport-of-improve-validation-of-the-post-logout-UR.patch
Patch3: 0003-Backport-of-Fix-open-redirect-starting-with-a-slash.patch
Patch4: 0004-Backport-of-Fix-open-redirect-starting-with-a-slash-.patch
Patch5: 0005-Fix-the-previous-backports.patch
Patch6: 0006-add-OIDCStateMaxNumberOfCookies-to-limit-nr-of-state.patch
Patch7: 0007-set-boundaries-on-min-and-max-values-on-number-of-pa.patch
Patch8: 0008-make-the-default-max-number-of-state-cookies-7-inste.patch
Patch9: 0009-don-t-return-content-with-503-see-331.patch
Patch10: 0010-improve-auto-detection-of-XMLHttpRequests-via-Accept.patch
Patch11: 0011-oops-document-OIDCStateMaxNumberOfCookies-for-releas.patch
Patch12: 0012-optionally-delete-the-oldest-state-cookie-s-see-399.patch
Patch13: 0013-Allow-configuring-which-header-value-is-used-to-calc.patch
Patch14: 0014-add-value-of-OIDC_SET_COOKIE_APPEND-env-var-to-Set-C.patch
Patch15: 0015-pick-OIDC_SET_COOKIE_APPEND-over-ext-passed-in-to-oi.patch
Patch16: 0016-always-add-a-SameSite-value-to-the-Set-Cookie-header.patch
Patch17: 0017-fix-also-add-SameSite-None-to-by-value-session-cooki.patch
Patch18: 0018-add-note-on-usage-of-OIDC_SET_COOKIE_APPEND-in-the-s.patch
Patch19: 0019-add-SameSite-attribute-on-cookie-clearance-logout.patch
BuildRequires: gcc
BuildRequires: httpd-devel
BuildRequires: openssl-devel
BuildRequires: curl-devel
BuildRequires: jansson-devel
BuildRequires: pcre-devel
BuildRequires: autoconf
BuildRequires: automake
BuildRequires: cjose-devel
BuildRequires: jq-devel
%{?_with_hiredis:BuildRequires: hiredis-devel}
Requires: httpd-mmn = %{_httpd_mmn}
%description
This module enables an Apache 2.x web server to operate as
an OpenID Connect Relying Party and/or OAuth 2.0 Resource Server.
%prep
%setup -q
%patch1 -p1
%patch2 -p1
%patch3 -p1
%patch4 -p1
%patch5 -p1
%patch6 -p1
%patch7 -p1
%patch8 -p1
%patch9 -p1
%patch10 -p1
%patch11 -p1
%patch12 -p1
%patch13 -p1
%patch14 -p1
%patch15 -p1
%patch16 -p1
%patch17 -p1
%patch18 -p1
%patch19 -p1
%build
# workaround rpm-buildroot-usage
export MODULES_DIR=%{_httpd_moddir}
export APXS2_OPTS='-S LIBEXECDIR=${MODULES_DIR}'
autoreconf
%configure \
--with-jq=/usr/lib64/ \
%{?_with_hiredis} \
%{?_without_hiredis}
make %{?_smp_mflags}
%check
export MODULES_DIR=%{_httpd_moddir}
export APXS2_OPTS='-S LIBEXECDIR=${MODULES_DIR}'
make test
%install
mkdir -p $RPM_BUILD_ROOT%{_httpd_moddir}
make install MODULES_DIR=$RPM_BUILD_ROOT%{_httpd_moddir}
install -m 755 -d $RPM_BUILD_ROOT%{_httpd_modconfdir}
echo 'LoadModule auth_openidc_module modules/mod_auth_openidc.so' > \
$RPM_BUILD_ROOT%{_httpd_modconfdir}/10-auth_openidc.conf
install -m 755 -d $RPM_BUILD_ROOT%{_httpd_confdir}
install -m 644 auth_openidc.conf $RPM_BUILD_ROOT%{_httpd_confdir}
# Adjust httpd cache location in install config file
sed -i 's!/var/cache/apache2/!/var/cache/httpd/!' $RPM_BUILD_ROOT%{_httpd_confdir}/auth_openidc.conf
install -m 700 -d $RPM_BUILD_ROOT%{httpd_pkg_cache_dir}
install -m 700 -d $RPM_BUILD_ROOT%{httpd_pkg_cache_dir}/metadata
install -m 700 -d $RPM_BUILD_ROOT%{httpd_pkg_cache_dir}/cache
%files
%if 0%{?rhel} && 0%{?rhel} < 7
%doc LICENSE.txt
%else
%license LICENSE.txt
%endif
%doc ChangeLog
%doc AUTHORS
%doc README.md
%{_httpd_moddir}/mod_auth_openidc.so
%config(noreplace) %{_httpd_modconfdir}/10-auth_openidc.conf
%config(noreplace) %{_httpd_confdir}/auth_openidc.conf
%dir %attr(0700, apache, apache) %{httpd_pkg_cache_dir}
%dir %attr(0700, apache, apache) %{httpd_pkg_cache_dir}/metadata
%dir %attr(0700, apache, apache) %{httpd_pkg_cache_dir}/cache
%changelog
* Tue Nov 17 2020 Jakub Hrozek <jhrozek@redhat.com> - 2.3.7-8
- Resolves: rhbz#1823756 - Backport SameSite=None cookie from
mod_auth_openidc upstream to support latest browsers
* Tue Nov 17 2020 Jakub Hrozek <jhrozek@redhat.com> - 2.3.7-7
- Resolves: rhbz#1897992 - OIDCStateInputHeaders &
OIDCStateMaxNumberOfCookies in existing
mod_auth_openidc version
- Backport the OIDCStateMaxNumberOfCookies option
- Configure which header value is used to calculate the fingerprint of
the auth state
* Sun May 10 2020 Jakub Hrozek <jhrozek@redhat.com> - 2.3.7-6
- Fix the previous backport
- Related: rhbz#1805749 - CVE-2019-14857 mod_auth_openidc:2.3/mod_auth_openidc:
Open redirect in logout url when using URLs with
leading slashes
- Related: rhbz#1805068 - CVE-2019-20479 mod_auth_openidc:2.3/mod_auth_openidc:
open redirect issue exists in URLs with slash and
backslash
* Sun May 10 2020 Jakub Hrozek <jhrozek@redhat.com> - 2.3.7-5
- Resolves: rhbz#1805749 - CVE-2019-14857 mod_auth_openidc:2.3/mod_auth_openidc:
Open redirect in logout url when using URLs with
leading slashes
- Resolves: rhbz#1805068 - CVE-2019-20479 mod_auth_openidc:2.3/mod_auth_openidc:
open redirect issue exists in URLs with slash and
backslash
* Thu Aug 16 2018 <jdennis@redhat.com> - 2.3.7-3
- Resolves: rhbz# 1614977 - fix unit test segfault,
the problem was not limited exclusively to s390x, but s390x provoked it.
* Fri Aug 10 2018 <jdennis@redhat.com> - 2.3.7-2
- disable running check on s390x
* Wed Aug 1 2018 <jdennis@redhat.com> - 2.3.7-1
- upgrade to upstream 2.3.7
* Fri Jul 13 2018 Fedora Release Engineering <releng@fedoraproject.org> - 2.3.5-2
- Rebuilt for https://fedoraproject.org/wiki/Fedora_29_Mass_Rebuild
* Wed May 23 2018 Patrick Uiterwijk <patrick@puiterwijk.org> - 2.3.5-1
- Rebase to 2.3.5
* Fri Feb 09 2018 Igor Gnatenko <ignatenkobrain@fedoraproject.org> - 1.8.10.1-7
- Escape macros in %%changelog
* Thu Feb 08 2018 Fedora Release Engineering <releng@fedoraproject.org> - 1.8.10.1-6
- Rebuilt for https://fedoraproject.org/wiki/Fedora_28_Mass_Rebuild
* Thu Aug 03 2017 Fedora Release Engineering <releng@fedoraproject.org> - 1.8.10.1-5
- Rebuilt for https://fedoraproject.org/wiki/Fedora_27_Binutils_Mass_Rebuild
* Wed Jul 26 2017 Fedora Release Engineering <releng@fedoraproject.org> - 1.8.10.1-4
- Rebuilt for https://fedoraproject.org/wiki/Fedora_27_Mass_Rebuild
* Sat Feb 18 2017 John Dennis <jdennis@redhat.com> - 1.8.10.1-3
- Resolves: #1423956 fails to build with openssl 1.1.x
Also rolls up all fixes to jose library before the change over to cjose
* Fri Feb 10 2017 Fedora Release Engineering <releng@fedoraproject.org> - 1.8.10.1-2
- Rebuilt for https://fedoraproject.org/wiki/Fedora_26_Mass_Rebuild
* Tue Jul 12 2016 John Dennis <jdennis@redhat.com> - 1.8.10.1-1
- Upgrade to new upstream
See /usr/share/doc/mod_auth_openidc/ChangeLog for details
* Tue Mar 29 2016 John Dennis <jdennis@redhat.com> - 1.8.8-4
- Add %%check to run test
* Wed Mar 23 2016 John Dennis <jdennis@redhat.com> - 1.8.8-3
- Make building with redis support optional (defaults to without)
* Mon Mar 21 2016 John Dennis <jdennis@redhat.com> - 1.8.8-2
- Add missing unpackaged files/directories
Add to doc: README.md, DISCLAIMER, AUTHORS
Add to httpd/conf.d: auth_openidc.conf
Add to /var/cache: /var/cache/httpd/mod_auth_openidc/cache
/var/cache/httpd/mod_auth_openidc/metadata
* Thu Mar 10 2016 Jan Pazdziora <jpazdziora@redhat.com> 1.8.8-1
- Update to 1.8.8 (#1316528)
* Thu Feb 04 2016 Fedora Release Engineering <releng@fedoraproject.org> - 1.8.7-2
- Rebuilt for https://fedoraproject.org/wiki/Fedora_24_Mass_Rebuild
* Sat Jan 09 2016 Fedora Release Monitoring <release-monitoring@fedoraproject.org> - 1.8.7-1
- Update to 1.8.7 (#1297080)
* Sat Nov 07 2015 Jan Pazdziora <jpazdziora@redhat.com> 1.8.6-1
- Initial packaging for Fedora 23.