diff --git a/.gitignore b/.gitignore index 3513201..fa44962 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1 @@ -SOURCES/mod_auth_openidc-2.3.7.tar.gz +SOURCES/v2.4.9.4.tar.gz diff --git a/.mod_auth_openidc.metadata b/.mod_auth_openidc.metadata index 4a55f1a..8489896 100644 --- a/.mod_auth_openidc.metadata +++ b/.mod_auth_openidc.metadata @@ -1 +1 @@ -0f9d620dad066ae8c415a59055903da1bfa9a4bf SOURCES/mod_auth_openidc-2.3.7.tar.gz +47f8b949552c3d32f019c5cf785c4672dc0f8aae SOURCES/v2.4.9.4.tar.gz diff --git a/SOURCES/0002-Backport-of-improve-validation-of-the-post-logout-UR.patch b/SOURCES/0002-Backport-of-improve-validation-of-the-post-logout-UR.patch deleted file mode 100644 index 8b68923..0000000 --- a/SOURCES/0002-Backport-of-improve-validation-of-the-post-logout-UR.patch +++ /dev/null @@ -1,127 +0,0 @@ -From cb5560f016d4f8bbca40670c59898afafb8d0763 Mon Sep 17 00:00:00 2001 -From: Jakub Hrozek -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 - diff --git a/SOURCES/0003-Backport-of-Fix-open-redirect-starting-with-a-slash.patch b/SOURCES/0003-Backport-of-Fix-open-redirect-starting-with-a-slash.patch deleted file mode 100644 index 9d3c3ee..0000000 --- a/SOURCES/0003-Backport-of-Fix-open-redirect-starting-with-a-slash.patch +++ /dev/null @@ -1,31 +0,0 @@ -From ed041f8b5df58c4e612a0d0cbb920dc0b399b921 Mon Sep 17 00:00:00 2001 -From: Jakub Hrozek -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 - diff --git a/SOURCES/0004-Backport-of-Fix-open-redirect-starting-with-a-slash-.patch b/SOURCES/0004-Backport-of-Fix-open-redirect-starting-with-a-slash-.patch deleted file mode 100644 index dbd64c7..0000000 --- a/SOURCES/0004-Backport-of-Fix-open-redirect-starting-with-a-slash-.patch +++ /dev/null @@ -1,32 +0,0 @@ -From c21228a0f170c025d79625207dc94759f480418f Mon Sep 17 00:00:00 2001 -From: Jakub Hrozek -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 - diff --git a/SOURCES/0005-Fix-the-previous-backports.patch b/SOURCES/0005-Fix-the-previous-backports.patch deleted file mode 100644 index c3d0e2b..0000000 --- a/SOURCES/0005-Fix-the-previous-backports.patch +++ /dev/null @@ -1,61 +0,0 @@ -From a5c9f79516fd4097817ac75a37af3b191a3d1448 Mon Sep 17 00:00:00 2001 -From: Jakub Hrozek -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 - diff --git a/SOURCES/0006-add-OIDCStateMaxNumberOfCookies-to-limit-nr-of-state.patch b/SOURCES/0006-add-OIDCStateMaxNumberOfCookies-to-limit-nr-of-state.patch deleted file mode 100644 index 9918336..0000000 --- a/SOURCES/0006-add-OIDCStateMaxNumberOfCookies-to-limit-nr-of-state.patch +++ /dev/null @@ -1,254 +0,0 @@ -From 1bb55df94c0db8376f88a568c331802bd282f097 Mon Sep 17 00:00:00 2001 -From: Hans Zandbelt -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 -(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 - diff --git a/SOURCES/0007-set-boundaries-on-min-and-max-values-on-number-of-pa.patch b/SOURCES/0007-set-boundaries-on-min-and-max-values-on-number-of-pa.patch deleted file mode 100644 index e46448e..0000000 --- a/SOURCES/0007-set-boundaries-on-min-and-max-values-on-number-of-pa.patch +++ /dev/null @@ -1,118 +0,0 @@ -From 284537dfc0585e08cfc0702c89b241d8986c7236 Mon Sep 17 00:00:00 2001 -From: Hans Zandbelt -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 -(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 - diff --git a/SOURCES/0008-make-the-default-max-number-of-state-cookies-7-inste.patch b/SOURCES/0008-make-the-default-max-number-of-state-cookies-7-inste.patch deleted file mode 100644 index e9a161d..0000000 --- a/SOURCES/0008-make-the-default-max-number-of-state-cookies-7-inste.patch +++ /dev/null @@ -1,45 +0,0 @@ -From 623163348f74fc1bd019a676fa24af69dde79654 Mon Sep 17 00:00:00 2001 -From: Hans Zandbelt -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 -(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 - diff --git a/SOURCES/0009-don-t-return-content-with-503-see-331.patch b/SOURCES/0009-don-t-return-content-with-503-see-331.patch deleted file mode 100644 index c80525c..0000000 --- a/SOURCES/0009-don-t-return-content-with-503-see-331.patch +++ /dev/null @@ -1,59 +0,0 @@ -From 7f5666375a3351e9c37589456b6fb3c92ef987c0 Mon Sep 17 00:00:00 2001 -From: Hans Zandbelt -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 -(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 - diff --git a/SOURCES/0010-improve-auto-detection-of-XMLHttpRequests-via-Accept.patch b/SOURCES/0010-improve-auto-detection-of-XMLHttpRequests-via-Accept.patch deleted file mode 100644 index d0edbbc..0000000 --- a/SOURCES/0010-improve-auto-detection-of-XMLHttpRequests-via-Accept.patch +++ /dev/null @@ -1,305 +0,0 @@ -From 9a0827a18ec4d16cf9e79afede901194d6ef5cc6 Mon Sep 17 00:00:00 2001 -From: Hans Zandbelt -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 -(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 : "

"); - -- 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) { diff --git a/SOURCES/0011-oops-document-OIDCStateMaxNumberOfCookies-for-releas.patch b/SOURCES/0011-oops-document-OIDCStateMaxNumberOfCookies-for-releas.patch deleted file mode 100644 index 9979909..0000000 --- a/SOURCES/0011-oops-document-OIDCStateMaxNumberOfCookies-for-releas.patch +++ /dev/null @@ -1,33 +0,0 @@ -From 7f81371f55a4a28000675023ad949e217d22bea3 Mon Sep 17 00:00:00 2001 -From: Hans Zandbelt -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 -(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 []+ - -+# 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 -+ - ######################################################################################## - # - # Session Settings (only relevant in an OpenID Connect Relying Party setup) --- -2.26.2 - diff --git a/SOURCES/0012-optionally-delete-the-oldest-state-cookie-s-see-399.patch b/SOURCES/0012-optionally-delete-the-oldest-state-cookie-s-see-399.patch deleted file mode 100644 index 2da8779..0000000 --- a/SOURCES/0012-optionally-delete-the-oldest-state-cookie-s-see-399.patch +++ /dev/null @@ -1,285 +0,0 @@ -From 95baad3342aa7ef7172ad4922eb9f0d605dc469b Mon Sep 17 00:00:00 2001 -From: Hans Zandbelt -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 -(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 -+# -+# 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 [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 - diff --git a/SOURCES/0013-Allow-configuring-which-header-value-is-used-to-calc.patch b/SOURCES/0013-Allow-configuring-which-header-value-is-used-to-calc.patch deleted file mode 100644 index 2746022..0000000 --- a/SOURCES/0013-Allow-configuring-which-header-value-is-used-to-calc.patch +++ /dev/null @@ -1,240 +0,0 @@ -From 5a1f90847efce160631ee2a16d7b9d1da3496616 Mon Sep 17 00:00:00 2001 -From: Hiroyuki Wada -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 []+ - -+# 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 - diff --git a/SOURCES/0014-add-value-of-OIDC_SET_COOKIE_APPEND-env-var-to-Set-C.patch b/SOURCES/0014-add-value-of-OIDC_SET_COOKIE_APPEND-env-var-to-Set-C.patch deleted file mode 100644 index 5aa503a..0000000 --- a/SOURCES/0014-add-value-of-OIDC_SET_COOKIE_APPEND-env-var-to-Set-C.patch +++ /dev/null @@ -1,86 +0,0 @@ -From 0bd084eb058361517b64a2c10a46c332adc9aeea Mon Sep 17 00:00:00 2001 -From: Hans Zandbelt -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 -(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 - diff --git a/SOURCES/0015-pick-OIDC_SET_COOKIE_APPEND-over-ext-passed-in-to-oi.patch b/SOURCES/0015-pick-OIDC_SET_COOKIE_APPEND-over-ext-passed-in-to-oi.patch deleted file mode 100644 index 30f63c0..0000000 --- a/SOURCES/0015-pick-OIDC_SET_COOKIE_APPEND-over-ext-passed-in-to-oi.patch +++ /dev/null @@ -1,35 +0,0 @@ -From 914f700cd791d370cf363d408e938598023980dc Mon Sep 17 00:00:00 2001 -From: Hans Zandbelt -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 -(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 - diff --git a/SOURCES/0016-always-add-a-SameSite-value-to-the-Set-Cookie-header.patch b/SOURCES/0016-always-add-a-SameSite-value-to-the-Set-Cookie-header.patch deleted file mode 100644 index 307a3c9..0000000 --- a/SOURCES/0016-always-add-a-SameSite-value-to-the-Set-Cookie-header.patch +++ /dev/null @@ -1,95 +0,0 @@ -From 2c999448c87b286744ac9802cb8e4277d5c38b71 Mon Sep 17 00:00:00 2001 -From: Hans Zandbelt -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 -(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\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 - diff --git a/SOURCES/0017-fix-also-add-SameSite-None-to-by-value-session-cooki.patch b/SOURCES/0017-fix-also-add-SameSite-None-to-by-value-session-cooki.patch deleted file mode 100644 index d89f0e2..0000000 --- a/SOURCES/0017-fix-also-add-SameSite-None-to-by-value-session-cooki.patch +++ /dev/null @@ -1,42 +0,0 @@ -From ca43d64e722f80ed91871c9ea31fbc7660aa9147 Mon Sep 17 00:00:00 2001 -From: Hans Zandbelt -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 -(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 - diff --git a/SOURCES/0018-add-note-on-usage-of-OIDC_SET_COOKIE_APPEND-in-the-s.patch b/SOURCES/0018-add-note-on-usage-of-OIDC_SET_COOKIE_APPEND-in-the-s.patch deleted file mode 100644 index 2bb92d6..0000000 --- a/SOURCES/0018-add-note-on-usage-of-OIDC_SET_COOKIE_APPEND-in-the-s.patch +++ /dev/null @@ -1,32 +0,0 @@ -From d2f6572e93446d611fc66cf68d0b71cd13366d55 Mon Sep 17 00:00:00 2001 -From: Hans Zandbelt -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 -(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 - diff --git a/SOURCES/0019-add-SameSite-attribute-on-cookie-clearance-logout.patch b/SOURCES/0019-add-SameSite-attribute-on-cookie-clearance-logout.patch deleted file mode 100644 index 6950693..0000000 --- a/SOURCES/0019-add-SameSite-attribute-on-cookie-clearance-logout.patch +++ /dev/null @@ -1,204 +0,0 @@ -From b68ac577b465f54e7eb38e48a65e5cf4c88c74c5 Mon Sep 17 00:00:00 2001 -From: Hans Zandbelt -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 -(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\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 - diff --git a/SOURCES/0020-prevent-open-redirect-on-refresh-token-requests-rele.patch b/SOURCES/0020-prevent-open-redirect-on-refresh-token-requests-rele.patch deleted file mode 100644 index 4dadc5b..0000000 --- a/SOURCES/0020-prevent-open-redirect-on-refresh-token-requests-rele.patch +++ /dev/null @@ -1,453 +0,0 @@ -From e4d625b0f1116e50df2737e2738b4c77b35328bb Mon Sep 17 00:00:00 2001 -From: Hans Zandbelt -Date: Wed, 10 Jun 2020 18:14:24 +0200 -Subject: [PATCH 1/4] prevent open redirect on refresh token requests; release - 2.4.3 - -add new OIDCRedirectURLsAllowed primitive to handle post logout and -refresh-return-to validation; addresses #453; closes #466 - -Signed-off-by: Hans Zandbelt -(cherry picked from commit 8ea550f34ce51d8d41ba47843739c964407fa0ad) ---- - auth_openidc.conf | 9 ++ - src/config.c | 24 ++++- - src/mod_auth_openidc.c | 205 ++++++++++++++++++++++++----------------- - src/mod_auth_openidc.h | 1 + - src/util.c | 16 ++-- - 5 files changed, 166 insertions(+), 89 deletions(-) - -diff --git a/auth_openidc.conf b/auth_openidc.conf -index ce2fba7..87685f6 100644 ---- a/auth_openidc.conf -+++ b/auth_openidc.conf -@@ -784,3 +784,12 @@ - # 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] -+ -+# Define one or more regular expressions that specify URLs (or domains) allowed for post logout and -+# other redirects such as the "return_to" value on refresh token requests, e.g.: -+# OIDCRedirectURLsAllowed ^https://www.example.com ^https://(\w+).example.org ^https://example.net/app -+# or: -+# OIDCRedirectURLsAllowed ^https://www.example.com/logout$ ^https://www.example.com/app/return_to$ -+# When not defined, the default is to match the hostname in the URL redirected to against -+# the hostname in the current request. -+#OIDCRedirectURLsAllowed []+ -diff --git a/src/config.c b/src/config.c -index 588e1a3..b2c0da7 100644 ---- a/src/config.c -+++ b/src/config.c -@@ -264,6 +264,7 @@ - #define OIDCBlackListedClaims "OIDCBlackListedClaims" - #define OIDCOAuthServerMetadataURL "OIDCOAuthServerMetadataURL" - #define OIDCStateInputHeaders "OIDCStateInputHeaders" -+#define OIDCRedirectURLsAllowed "OIDCRedirectURLsAllowed" - - extern module AP_MODULE_DECLARE_DATA auth_openidc_module; - -@@ -1044,6 +1045,16 @@ static const char * oidc_set_state_input_headers_as(cmd_parms *cmd, void *m, - return OIDC_CONFIG_DIR_RV(cmd, rv); - } - -+static const char * oidc_set_redirect_urls_allowed(cmd_parms *cmd, void *m, -+ const char *arg) { -+ oidc_cfg *cfg = (oidc_cfg *) ap_get_module_config( -+ cmd->server->module_config, &auth_openidc_module); -+ if (cfg->redirect_urls_allowed == NULL) -+ cfg->redirect_urls_allowed = apr_hash_make(cmd->pool); -+ apr_hash_set(cfg->redirect_urls_allowed, arg, APR_HASH_KEY_STRING, arg); -+ return NULL; -+} -+ - /* - * create a new server config record with defaults - */ -@@ -1192,6 +1203,8 @@ void *oidc_create_server_config(apr_pool_t *pool, server_rec *svr) { - - c->state_input_headers = OIDC_DEFAULT_STATE_INPUT_HEADERS; - -+ c->redirect_urls_allowed = NULL; -+ - return c; - } - -@@ -1637,6 +1650,10 @@ void *oidc_merge_server_config(apr_pool_t *pool, void *BASE, void *ADD) { - add->state_input_headers != OIDC_DEFAULT_STATE_INPUT_HEADERS ? - add->state_input_headers : base->state_input_headers; - -+ c->redirect_urls_allowed = -+ add->redirect_urls_allowed != NULL ? -+ add->redirect_urls_allowed : base->redirect_urls_allowed; -+ - return c; - } - -@@ -2310,7 +2327,7 @@ void oidc_register_hooks(apr_pool_t *pool) { - ap_register_auth_provider(pool, AUTHZ_PROVIDER_GROUP, - OIDC_REQUIRE_CLAIM_NAME, "0", &oidc_authz_claim_provider, - AP_AUTH_INTERNAL_PER_CONF); --#ifdef USE_LIBJQ -+#ifdef USE_LIBJQ - ap_register_auth_provider(pool, AUTHZ_PROVIDER_GROUP, - OIDC_REQUIRE_CLAIMS_EXPR_NAME, "0", - &oidc_authz_claims_expr_provider, AP_AUTH_INTERNAL_PER_CONF); -@@ -2891,5 +2908,10 @@ const command_rec oidc_config_cmds[] = { - 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)."), -+ AP_INIT_ITERATE(OIDCRedirectURLsAllowed, -+ oidc_set_redirect_urls_allowed, -+ (void *) APR_OFFSETOF(oidc_cfg, redirect_urls_allowed), -+ RSRC_CONF|ACCESS_CONF|OR_AUTHCFG, -+ "Specify one or more regular expressions that define URLs allowed for post logout and other redirects."), - { NULL } - }; -diff --git a/src/mod_auth_openidc.c b/src/mod_auth_openidc.c -index 68fa2ce..215ed5e 100644 ---- a/src/mod_auth_openidc.c -+++ b/src/mod_auth_openidc.c -@@ -226,7 +226,8 @@ 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, oidc_cfg *c, const char *nonce) { -+static char *oidc_get_browser_state_hash(request_rec *r, oidc_cfg *c, -+ const char *nonce) { - - oidc_debug(r, "enter"); - -@@ -543,14 +544,13 @@ static apr_byte_t oidc_unsolicited_proto_state(request_rec *r, oidc_cfg *c, - oidc_jose_error_t err; - oidc_jwk_t *jwk = NULL; - if (oidc_util_create_symmetric_key(r, c->provider.client_secret, -- oidc_alg2keysize(alg), OIDC_JOSE_ALG_SHA256, -- TRUE, &jwk) == FALSE) -+ oidc_alg2keysize(alg), OIDC_JOSE_ALG_SHA256, TRUE, &jwk) == FALSE) - return FALSE; - - oidc_jwt_t *jwt = NULL; - if (oidc_jwt_parse(r->pool, state, &jwt, -- oidc_util_merge_symmetric_key(r->pool, c->private_keys, jwk), -- &err) == FALSE) { -+ oidc_util_merge_symmetric_key(r->pool, c->private_keys, jwk), &err) -+ == FALSE) { - oidc_error(r, - "could not parse JWT from state: invalid unsolicited response: %s", - oidc_jose_e2s(r->pool, err)); -@@ -600,8 +600,7 @@ static apr_byte_t oidc_unsolicited_proto_state(request_rec *r, oidc_cfg *c, - - char *target_link_uri = NULL; - oidc_jose_get_string(r->pool, jwt->payload.value.json, -- OIDC_CLAIM_TARGET_LINK_URI, -- FALSE, &target_link_uri, NULL); -+ OIDC_CLAIM_TARGET_LINK_URI, FALSE, &target_link_uri, NULL); - if (target_link_uri == NULL) { - if (c->default_sso_url == NULL) { - oidc_error(r, -@@ -1224,8 +1223,8 @@ static apr_byte_t oidc_refresh_access_token(request_rec *r, oidc_cfg *c, - - /* refresh the tokens by calling the token endpoint */ - if (oidc_proto_refresh_request(r, c, provider, refresh_token, &s_id_token, -- &s_access_token, &s_token_type, &expires_in, -- &s_refresh_token) == FALSE) { -+ &s_access_token, &s_token_type, &expires_in, &s_refresh_token) -+ == FALSE) { - oidc_error(r, "access_token could not be refreshed"); - return FALSE; - } -@@ -2286,8 +2285,8 @@ static int oidc_discovery(request_rec *r, oidc_cfg *cfg) { - char *javascript = NULL, *javascript_method = NULL; - char *html_head = - ""; -- if (oidc_post_preserve_javascript(r, NULL, &javascript, -- &javascript_method) == TRUE) -+ if (oidc_post_preserve_javascript(r, NULL, &javascript, &javascript_method) -+ == TRUE) - html_head = apr_psprintf(r->pool, "%s%s", html_head, javascript); - - /* now send the HTML contents to the user agent */ -@@ -2531,8 +2530,8 @@ static int oidc_handle_discovery_response(request_rec *r, oidc_cfg *c) { - } - - /* do open redirect prevention */ -- if (oidc_target_link_uri_matches_configuration(r, c, -- target_link_uri) == FALSE) { -+ if (oidc_target_link_uri_matches_configuration(r, c, target_link_uri) -+ == FALSE) { - return oidc_util_html_send_error(r, c->error_template, - "Invalid Request", - "\"target_link_uri\" parameter does not match configuration settings, aborting to prevent an open redirect.", -@@ -2587,7 +2586,8 @@ static int oidc_handle_discovery_response(request_rec *r, oidc_cfg *c) { - } - - /* got an account name as input, perform OP discovery with that */ -- if (oidc_proto_account_based_discovery(r, c, issuer, &issuer) == FALSE) { -+ if (oidc_proto_account_based_discovery(r, c, issuer, &issuer) -+ == FALSE) { - - /* something did not work out, show a user facing error */ - return oidc_util_html_send_error(r, c->error_template, -@@ -2687,66 +2687,84 @@ 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 ((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 it does not seem to be relative, i.e starting with '/': %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 = -- apr_psprintf(r->pool, -- "No hostname was parsed and starting with '//': %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 = -- apr_psprintf(r->pool, -- "No hostname was parsed and 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; -+static apr_byte_t oidc_validate_redirect_url(request_rec *r, oidc_cfg *c, -+ const char *url, char **err_str, char **err_desc) { -+ apr_uri_t uri; -+ const char *c_host = NULL; -+ apr_hash_index_t *hi = 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, "not a valid URL value: %s", url); -+ oidc_error(r, "%s: %s", *err_str, *err_desc); -+ return FALSE; -+ } -+ -+ if (c->redirect_urls_allowed != NULL) { -+ for (hi = apr_hash_first(NULL, c->redirect_urls_allowed); hi; hi = -+ apr_hash_next(hi)) { -+ apr_hash_this(hi, (const void**) &c_host, NULL, NULL); -+ if (oidc_util_regexp_first_match(r->pool, url, c_host, -+ NULL, err_str) == TRUE) -+ break; -+ } -+ if (hi == NULL) { -+ *err_str = apr_pstrdup(r->pool, "URL not allowed"); -+ *err_desc = -+ apr_psprintf(r->pool, -+ "value does not match the list of allowed redirect URLs: %s", -+ url); -+ oidc_error(r, "%s: %s", *err_str, *err_desc); -+ return FALSE; -+ } -+ } else if (uri.hostname != NULL) { -+ c_host = oidc_get_current_url_host(r); -+ if ((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, -+ "URL 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; -+ } -+ } -+ -+ 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 it does not seem to be relative, i.e starting with '/': %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 = apr_psprintf(r->pool, -+ "No hostname was parsed and starting with '//': %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 = apr_psprintf(r->pool, -+ "No hostname was parsed and 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 URL"); -+ *err_desc = -+ apr_psprintf(r->pool, -+ "URL value \"%s\" contains illegal \"\n\" or \"\r\" character(s)", -+ url); -+ oidc_error(r, "%s: %s", *err_str, *err_desc); -+ return FALSE; -+ } -+ -+ return TRUE; - } - - /* -@@ -2774,12 +2792,11 @@ static int oidc_handle_logout(request_rec *r, oidc_cfg *c, - } else { - - /* do input validation on the logout parameter value */ -- -- 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); -+ if (oidc_validate_redirect_url(r, c, url, &error_str, -+ &error_description) == FALSE) { -+ return oidc_util_html_send_error(r, c->error_template, error_str, -+ error_description, -+ HTTP_BAD_REQUEST); - } - } - -@@ -3027,6 +3044,8 @@ static int oidc_handle_refresh_token_request(request_rec *r, oidc_cfg *c, - char *return_to = NULL; - char *r_access_token = NULL; - char *error_code = NULL; -+ char *error_str = NULL; -+ char *error_description = NULL; - - /* get the command passed to the session management handler */ - oidc_util_get_request_parameter(r, OIDC_REDIRECT_URI_REQUEST_REFRESH, -@@ -3041,6 +3060,14 @@ static int oidc_handle_refresh_token_request(request_rec *r, oidc_cfg *c, - return HTTP_INTERNAL_SERVER_ERROR; - } - -+ /* do input validation on the return to parameter value */ -+ if (oidc_validate_redirect_url(r, c, return_to, &error_str, -+ &error_description) == FALSE) { -+ oidc_error(r, "return_to URL validation failed: %s: %s", error_str, -+ error_description); -+ return HTTP_INTERNAL_SERVER_ERROR; -+ } -+ - if (r_access_token == NULL) { - oidc_error(r, - "refresh token request handler called with no access_token parameter"); -@@ -3201,8 +3228,8 @@ static int oidc_handle_info_request(request_rec *r, oidc_cfg *c, - - /* get the current provider info */ - oidc_provider_t *provider = NULL; -- if (oidc_get_provider_from_session(r, c, session, -- &provider) == FALSE) -+ if (oidc_get_provider_from_session(r, c, session, &provider) -+ == FALSE) - return HTTP_INTERNAL_SERVER_ERROR; - - /* execute the actual refresh grant */ -@@ -3319,6 +3346,20 @@ int oidc_handle_redirect_uri_request(request_rec *r, oidc_cfg *c, - /* this is an authorization response from the OP using the Basic Client profile or a Hybrid flow*/ - return oidc_handle_redirect_authorization_response(r, c, session); - -+ /* -+ * -+ * Note that we are checking for logout *before* checking for a POST authorization response -+ * to handle backchannel POST-based logout -+ * -+ * so any POST to the Redirect URI that does not have a logout query parameter will be handled -+ * as an authorization response; alternatively we could assume that a POST response has no -+ * parameters -+ */ -+ } else if (oidc_util_request_has_parameter(r, -+ OIDC_REDIRECT_URI_REQUEST_LOGOUT)) { -+ /* handle logout */ -+ return oidc_handle_logout(r, c, session); -+ - } else if (oidc_proto_is_post_authorization_response(r, c)) { - - /* this is an authorization response using the fragment(+POST) response_mode with the Implicit Client profile */ -diff --git a/src/mod_auth_openidc.h b/src/mod_auth_openidc.h -index 6821d0c..ea79e6b 100644 ---- a/src/mod_auth_openidc.h -+++ b/src/mod_auth_openidc.h -@@ -414,6 +414,7 @@ typedef struct oidc_cfg { - - apr_byte_t state_input_headers; - -+ apr_hash_t *redirect_urls_allowed; - } oidc_cfg; - - int oidc_check_user_id(request_rec *r); -diff --git a/src/util.c b/src/util.c -index c1fa5f3..4b4e16b 100644 ---- a/src/util.c -+++ b/src/util.c -@@ -2201,14 +2201,18 @@ apr_byte_t oidc_util_regexp_first_match(apr_pool_t *pool, const char *input, - goto out; - } - -- if (pcre_get_substring(input, subStr, rc, OIDC_UTIL_REGEXP_MATCH_NR, -- &(psubStrMatchStr)) <= 0) { -- *error_str = apr_psprintf(pool, "pcre_get_substring failed (rc=%d)", -- rc); -- goto out; -+ if (output) { -+ -+ if (pcre_get_substring(input, subStr, rc, OIDC_UTIL_REGEXP_MATCH_NR, -+ &(psubStrMatchStr)) <= 0) { -+ *error_str = apr_psprintf(pool, "pcre_get_substring failed (rc=%d)", -+ rc); -+ goto out; -+ } -+ -+ *output = apr_pstrdup(pool, psubStrMatchStr); - } - -- *output = apr_pstrdup(pool, psubStrMatchStr); - rv = TRUE; - - out: --- -2.31.1 - diff --git a/SOURCES/0021-prevent-XSS-and-open-redirect-on-OIDC-session-manage.patch b/SOURCES/0021-prevent-XSS-and-open-redirect-on-OIDC-session-manage.patch deleted file mode 100644 index f1da3a4..0000000 --- a/SOURCES/0021-prevent-XSS-and-open-redirect-on-OIDC-session-manage.patch +++ /dev/null @@ -1,167 +0,0 @@ -From a1b8e7aa92e5e624a5f90bb736c307dae22230a1 Mon Sep 17 00:00:00 2001 -From: Hans Zandbelt -Date: Mon, 27 Jul 2020 19:35:29 +0200 -Subject: [PATCH 2/4] prevent XSS and open redirect on OIDC session - managemement OP iframe - -- apply OIDCRedirectURLsAllowed on the login_uri parameter -- thanks Andrew Brady -- bump to 2.4.4rc3 - -Signed-off-by: Hans Zandbelt -(cherry picked from commit 51d997899afea6ea454abda49bd4cd41aa7c0cdc) ---- - ChangeLog | 12 ++++++------ - auth_openidc.conf | 3 ++- - configure.ac | 2 +- - src/mod_auth_openidc.c | 21 +++++++++++++++++---- - 4 files changed, 26 insertions(+), 12 deletions(-) - -diff --git a/ChangeLog b/ChangeLog -index eba2ebc..075f98d 100644 ---- a/ChangeLog -+++ b/ChangeLog -@@ -86,7 +86,7 @@ - - bump to 2.3.5rc0 - - 04/27/2018 --- avoid crash when a relative logout URL parameter is passed in; thanks Vivien Delenne -+- avoid crash when a relative logout URL parameter is passed in; thanks Vivien Delenne - - release 2.3.4 - - 03/22/2018 -@@ -258,7 +258,7 @@ - - bump to 2.2.1rc6 - - 05/18/2017 --- fix parse function of OIDCRequestObject configuration option; thanks @suttod -+- fix parse function of OIDCRequestObject configuration option; thanks @suttod - - 05/17/2017 - - avoid crash when the X-Forwarded-Proto header is not correctly set by a reverse proxy in front of mod_auth_openidc -@@ -325,7 +325,7 @@ - - 02/20/2017 - - security fix: scrub headers for "AuthType oauth20" --- release 2.1.6 -+- release 2.1.6 - - 02/15/2017 - - improve logging of session max duration and session inactivity timeout -@@ -534,7 +534,7 @@ - - bump to 1.9.0rc3 - - 7/19/2016 --- add support for chunked session cookies; closes #153; thanks @glatzert -+- add support for chunked session cookies; closes #153; thanks @glatzert - - bump to 1.9.0rc2 - - 7/9/2016 -@@ -911,7 +911,7 @@ - - 1/1/2015 - - update copyright to 2015 --- use json_int_t (seconds) for "exp" and "iat" fields, instead of apr_time_t (microseconds) -+- use json_int_t (seconds) for "exp" and "iat" fields, instead of apr_time_t (microseconds) - - correct expiry debug printout - - bump to 1.7.2rc1 - -@@ -1191,7 +1191,7 @@ - - support using a Bearer token on client registration calls - - 4/22/2014 --- match request and response type -+- match request and response type - - check at_hash value on "token id_token" implicit flow - - use shared memory caching by default - - release 1.2 -diff --git a/auth_openidc.conf b/auth_openidc.conf -index 87685f6..75cdb8e 100644 ---- a/auth_openidc.conf -+++ b/auth_openidc.conf -@@ -786,7 +786,8 @@ - #OIDCStateInputHeaders [none|user-agent|x-forwarded-for|both] - - # Define one or more regular expressions that specify URLs (or domains) allowed for post logout and --# other redirects such as the "return_to" value on refresh token requests, e.g.: -+# other redirects such as the "return_to" value on refresh token requests, and the "login_uri" value -+# on session management based logins through the OP iframe, e.g.: - # OIDCRedirectURLsAllowed ^https://www.example.com ^https://(\w+).example.org ^https://example.net/app - # or: - # OIDCRedirectURLsAllowed ^https://www.example.com/logout$ ^https://www.example.com/app/return_to$ -diff --git a/configure.ac b/configure.ac -index ad5ba0e..c61d117 100644 ---- a/configure.ac -+++ b/configure.ac -@@ -91,7 +91,7 @@ HAVE_LIBJQ=0 - - AC_ARG_WITH(jq, - [ --with-jq=PATH location of your libjq installation]) -- -+ - if test -n "$with_jq" - then - JQ_CFLAGS="-I$with_jq/include" -diff --git a/src/mod_auth_openidc.c b/src/mod_auth_openidc.c -index 215ed5e..68fbca5 100644 ---- a/src/mod_auth_openidc.c -+++ b/src/mod_auth_openidc.c -@@ -2688,7 +2688,8 @@ static int oidc_handle_logout_request(request_rec *r, oidc_cfg *c, - } - - static apr_byte_t oidc_validate_redirect_url(request_rec *r, oidc_cfg *c, -- const char *url, char **err_str, char **err_desc) { -+ const char *url, apr_byte_t restrict_to_host, char **err_str, -+ char **err_desc) { - apr_uri_t uri; - const char *c_host = NULL; - apr_hash_index_t *hi = NULL; -@@ -2717,7 +2718,7 @@ static apr_byte_t oidc_validate_redirect_url(request_rec *r, oidc_cfg *c, - oidc_error(r, "%s: %s", *err_str, *err_desc); - return FALSE; - } -- } else if (uri.hostname != NULL) { -+ } else if ((uri.hostname != NULL) && (restrict_to_host == TRUE)) { - c_host = oidc_get_current_url_host(r); - if ((strstr(c_host, uri.hostname) == NULL) - || (strstr(uri.hostname, c_host) == NULL)) { -@@ -2792,7 +2793,7 @@ static int oidc_handle_logout(request_rec *r, oidc_cfg *c, - } else { - - /* do input validation on the logout parameter value */ -- if (oidc_validate_redirect_url(r, c, url, &error_str, -+ if (oidc_validate_redirect_url(r, c, url, TRUE, &error_str, - &error_description) == FALSE) { - return oidc_util_html_send_error(r, c->error_template, error_str, - error_description, -@@ -2948,6 +2949,18 @@ static int oidc_handle_session_management_iframe_rp(request_rec *r, oidc_cfg *c, - if (s_poll_interval == NULL) - s_poll_interval = "3000"; - -+ int poll_interval = s_poll_interval ? strtol(s_poll_interval, NULL, 10) : 0; -+ if ((poll_interval <= 0) || (poll_interval > 3600 * 24)) -+ poll_interval = 3000; -+ -+ char *login_uri = NULL, *error_str = NULL, *error_description = NULL; -+ oidc_util_get_request_parameter(r, "login_uri", &login_uri); -+ if ((login_uri != NULL) -+ && (oidc_validate_redirect_url(r, c, login_uri, FALSE, &error_str, -+ &error_description) == FALSE)) { -+ return HTTP_BAD_REQUEST; -+ } -+ - const char *redirect_uri = oidc_get_redirect_uri(r, c); - java_script = apr_psprintf(r->pool, java_script, origin, client_id, - session_state, op_iframe_id, s_poll_interval, redirect_uri, -@@ -3061,7 +3074,7 @@ static int oidc_handle_refresh_token_request(request_rec *r, oidc_cfg *c, - } - - /* do input validation on the return to parameter value */ -- if (oidc_validate_redirect_url(r, c, return_to, &error_str, -+ if (oidc_validate_redirect_url(r, c, return_to, TRUE, &error_str, - &error_description) == FALSE) { - oidc_error(r, "return_to URL validation failed: %s: %s", error_str, - error_description); --- -2.31.1 - diff --git a/SOURCES/0022-replace-potentially-harmful-backslashes-with-forward.patch b/SOURCES/0022-replace-potentially-harmful-backslashes-with-forward.patch deleted file mode 100644 index 170e275..0000000 --- a/SOURCES/0022-replace-potentially-harmful-backslashes-with-forward.patch +++ /dev/null @@ -1,42 +0,0 @@ -From 466f470265554e0e3ae27a6d82375456d2c133e6 Mon Sep 17 00:00:00 2001 -From: Hans Zandbelt -Date: Thu, 22 Jul 2021 15:32:12 +0200 -Subject: [PATCH 3/4] replace potentially harmful backslashes with forward - slashes when validating redirection URLs - -(cherry picked from commit 69cb206225c749b51db980d44dc268eee5623f2b) ---- - src/mod_auth_openidc.c | 12 +++++++++++- - 1 file changed, 11 insertions(+), 1 deletion(-) - -diff --git a/src/mod_auth_openidc.c b/src/mod_auth_openidc.c -index 68fbca5..c96af75 100644 ---- a/src/mod_auth_openidc.c -+++ b/src/mod_auth_openidc.c -@@ -2687,12 +2687,22 @@ static int oidc_handle_logout_request(request_rec *r, oidc_cfg *c, - return HTTP_MOVED_TEMPORARILY; - } - -+ -+#define OIDC_MAX_URL_LENGTH DEFAULT_LIMIT_REQUEST_LINE * 2 -+ - static apr_byte_t oidc_validate_redirect_url(request_rec *r, oidc_cfg *c, -- const char *url, apr_byte_t restrict_to_host, char **err_str, -+ const char *redirect_to_url, apr_byte_t restrict_to_host, char **err_str, - char **err_desc) { - apr_uri_t uri; - const char *c_host = NULL; - apr_hash_index_t *hi = NULL; -+ size_t i = 0; -+ char *url = apr_pstrndup(r->pool, redirect_to_url, OIDC_MAX_URL_LENGTH); -+ -+ // replace potentially harmful backslashes with forward slashes -+ for (i = 0; i < strlen(url); i++) -+ if (url[i] == '\\') -+ url[i] = '/'; - - if (apr_uri_parse(r->pool, url, &uri) != APR_SUCCESS) { - *err_str = apr_pstrdup(r->pool, "Malformed URL"); --- -2.31.1 - diff --git a/SOURCES/0023-apply-OIDCRedirectURLsAllowed-setting-to-target_link.patch b/SOURCES/0023-apply-OIDCRedirectURLsAllowed-setting-to-target_link.patch deleted file mode 100644 index 24d6b1e..0000000 --- a/SOURCES/0023-apply-OIDCRedirectURLsAllowed-setting-to-target_link.patch +++ /dev/null @@ -1,306 +0,0 @@ -From eac10ec54971ff021113f96c80deab9991faafc4 Mon Sep 17 00:00:00 2001 -From: Hans Zandbelt -Date: Fri, 3 Sep 2021 10:41:21 +0200 -Subject: [PATCH 4/4] apply OIDCRedirectURLsAllowed setting to target_link_uri - -closes #672; thanks @Meheni -release 2.4.9.4 - -Signed-off-by: Hans Zandbelt -(cherry picked from commit 03e6bfb446f4e3f27c003d30d6a433e5dd8e2b3d) ---- - AUTHORS | 31 +++++++ - auth_openidc.conf | 5 +- - src/mod_auth_openidc.c | 193 +++++++++++++++++++++-------------------- - 3 files changed, 135 insertions(+), 94 deletions(-) - -diff --git a/AUTHORS b/AUTHORS -index 1d91449..4f77eab 100644 ---- a/AUTHORS -+++ b/AUTHORS -@@ -46,3 +46,34 @@ reporting bugs, providing fixes, suggesting useful features or other: - timpuri - Eldar Zaitov - Gergan Penkov -+ Florian Weimer -+ Aaron Donovan -+ Hans Petter Bieker -+ archzone -+ Petteri Stenius -+ Lance Fannin -+ Ricardo Martin Camarero -+ Filip Vujicic -+ Janusz Ulanowski -+ Aimoto Norihito -+ Andy Lindeman -+ Stefan Wachter -+ Paolo Battino -+ absynth76 -+ Aaron Jones -+ Bryan Ingram -+ Tim Deisser -+ Peter Hurtenbach -+ Paul Spangler -+ Chris Pawling -+ Matthias Fleschütz -+ Harri Rautila -+ Tatsuhiko Yasumatsu -+ Adam Stadler -+ Steffen Greber -+ Iain Heggie -+ Dirk Kok -+ Meheni https://github.com/Meheni -+ -+ -+ -diff --git a/auth_openidc.conf b/auth_openidc.conf -index 75cdb8e..55febe5 100644 ---- a/auth_openidc.conf -+++ b/auth_openidc.conf -@@ -786,8 +786,9 @@ - #OIDCStateInputHeaders [none|user-agent|x-forwarded-for|both] - - # Define one or more regular expressions that specify URLs (or domains) allowed for post logout and --# other redirects such as the "return_to" value on refresh token requests, and the "login_uri" value --# on session management based logins through the OP iframe, e.g.: -+# other redirects such as the "return_to" value on refresh token requests, the "login_uri" value -+# on session management based logins through the OP iframe, and the "target_link_uri" parameter in -+# 3rd-party initiated logins, e.g.: - # OIDCRedirectURLsAllowed ^https://www.example.com ^https://(\w+).example.org ^https://example.net/app - # or: - # OIDCRedirectURLsAllowed ^https://www.example.com/logout$ ^https://www.example.com/app/return_to$ -diff --git a/src/mod_auth_openidc.c b/src/mod_auth_openidc.c -index c96af75..4cc7976 100644 ---- a/src/mod_auth_openidc.c -+++ b/src/mod_auth_openidc.c -@@ -2476,6 +2476,96 @@ static int oidc_target_link_uri_matches_configuration(request_rec *r, - return TRUE; - } - -+#define OIDC_MAX_URL_LENGTH 8192 * 2 -+ -+static apr_byte_t oidc_validate_redirect_url(request_rec *r, oidc_cfg *c, -+ const char *redirect_to_url, apr_byte_t restrict_to_host, char **err_str, -+ char **err_desc) { -+ apr_uri_t uri; -+ const char *c_host = NULL; -+ apr_hash_index_t *hi = NULL; -+ size_t i = 0; -+ char *url = apr_pstrndup(r->pool, redirect_to_url, OIDC_MAX_URL_LENGTH); -+ -+ // replace potentially harmful backslashes with forward slashes -+ for (i = 0; i < strlen(url); i++) -+ if (url[i] == '\\') -+ url[i] = '/'; -+ -+ if (apr_uri_parse(r->pool, url, &uri) != APR_SUCCESS) { -+ *err_str = apr_pstrdup(r->pool, "Malformed URL"); -+ *err_desc = apr_psprintf(r->pool, "not a valid URL value: %s", url); -+ oidc_error(r, "%s: %s", *err_str, *err_desc); -+ return FALSE; -+ } -+ -+ if (c->redirect_urls_allowed != NULL) { -+ for (hi = apr_hash_first(NULL, c->redirect_urls_allowed); hi; hi = -+ apr_hash_next(hi)) { -+ apr_hash_this(hi, (const void**) &c_host, NULL, NULL); -+ if (oidc_util_regexp_first_match(r->pool, url, c_host, -+ NULL, err_str) == TRUE) -+ break; -+ } -+ if (hi == NULL) { -+ *err_str = apr_pstrdup(r->pool, "URL not allowed"); -+ *err_desc = -+ apr_psprintf(r->pool, -+ "value does not match the list of allowed redirect URLs: %s", -+ url); -+ oidc_error(r, "%s: %s", *err_str, *err_desc); -+ return FALSE; -+ } -+ } else if ((uri.hostname != NULL) && (restrict_to_host == TRUE)) { -+ c_host = oidc_get_current_url_host(r); -+ if ((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, -+ "URL 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; -+ } -+ } -+ -+ 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 it does not seem to be relative, i.e starting with '/': %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 = apr_psprintf(r->pool, -+ "No hostname was parsed and starting with '//': %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 = apr_psprintf(r->pool, -+ "No hostname was parsed and 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 URL"); -+ *err_desc = -+ apr_psprintf(r->pool, -+ "URL value \"%s\" contains illegal \"\n\" or \"\r\" character(s)", -+ url); -+ oidc_error(r, "%s: %s", *err_str, *err_desc); -+ return FALSE; -+ } -+ -+ return TRUE; -+} -+ - /* - * handle a response from an IDP discovery page and/or handle 3rd-party initiated SSO - */ -@@ -2486,6 +2576,8 @@ static int oidc_handle_discovery_response(request_rec *r, oidc_cfg *c) { - *auth_request_params = NULL, *csrf_cookie, *csrf_query = NULL, - *user = NULL, *path_scopes; - oidc_provider_t *provider = NULL; -+ char *error_str = NULL; -+ char *error_description = NULL; - - oidc_util_get_request_parameter(r, OIDC_DISC_OP_PARAM, &issuer); - oidc_util_get_request_parameter(r, OIDC_DISC_USER_PARAM, &user); -@@ -2529,7 +2621,7 @@ static int oidc_handle_discovery_response(request_rec *r, oidc_cfg *c) { - target_link_uri = c->default_sso_url; - } - -- /* do open redirect prevention */ -+ /* do open redirect prevention, step 1 */ - if (oidc_target_link_uri_matches_configuration(r, c, target_link_uri) - == FALSE) { - return oidc_util_html_send_error(r, c->error_template, -@@ -2538,6 +2630,14 @@ static int oidc_handle_discovery_response(request_rec *r, oidc_cfg *c) { - HTTP_UNAUTHORIZED); - } - -+ /* do input validation on the target_link_uri parameter value, step 2 */ -+ if (oidc_validate_redirect_url(r, c, target_link_uri, TRUE, &error_str, -+ &error_description) == FALSE) { -+ return oidc_util_html_send_error(r, c->error_template, error_str, -+ error_description, -+ HTTP_UNAUTHORIZED); -+ } -+ - /* see if this is a static setup */ - if (c->metadata_dir == NULL) { - if ((oidc_provider_static_config(r, c, &provider) == TRUE) -@@ -2687,97 +2787,6 @@ static int oidc_handle_logout_request(request_rec *r, oidc_cfg *c, - return HTTP_MOVED_TEMPORARILY; - } - -- --#define OIDC_MAX_URL_LENGTH DEFAULT_LIMIT_REQUEST_LINE * 2 -- --static apr_byte_t oidc_validate_redirect_url(request_rec *r, oidc_cfg *c, -- const char *redirect_to_url, apr_byte_t restrict_to_host, char **err_str, -- char **err_desc) { -- apr_uri_t uri; -- const char *c_host = NULL; -- apr_hash_index_t *hi = NULL; -- size_t i = 0; -- char *url = apr_pstrndup(r->pool, redirect_to_url, OIDC_MAX_URL_LENGTH); -- -- // replace potentially harmful backslashes with forward slashes -- for (i = 0; i < strlen(url); i++) -- if (url[i] == '\\') -- url[i] = '/'; -- -- if (apr_uri_parse(r->pool, url, &uri) != APR_SUCCESS) { -- *err_str = apr_pstrdup(r->pool, "Malformed URL"); -- *err_desc = apr_psprintf(r->pool, "not a valid URL value: %s", url); -- oidc_error(r, "%s: %s", *err_str, *err_desc); -- return FALSE; -- } -- -- if (c->redirect_urls_allowed != NULL) { -- for (hi = apr_hash_first(NULL, c->redirect_urls_allowed); hi; hi = -- apr_hash_next(hi)) { -- apr_hash_this(hi, (const void**) &c_host, NULL, NULL); -- if (oidc_util_regexp_first_match(r->pool, url, c_host, -- NULL, err_str) == TRUE) -- break; -- } -- if (hi == NULL) { -- *err_str = apr_pstrdup(r->pool, "URL not allowed"); -- *err_desc = -- apr_psprintf(r->pool, -- "value does not match the list of allowed redirect URLs: %s", -- url); -- oidc_error(r, "%s: %s", *err_str, *err_desc); -- return FALSE; -- } -- } else if ((uri.hostname != NULL) && (restrict_to_host == TRUE)) { -- c_host = oidc_get_current_url_host(r); -- if ((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, -- "URL 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; -- } -- } -- -- 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 it does not seem to be relative, i.e starting with '/': %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 = apr_psprintf(r->pool, -- "No hostname was parsed and starting with '//': %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 = apr_psprintf(r->pool, -- "No hostname was parsed and 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 URL"); -- *err_desc = -- apr_psprintf(r->pool, -- "URL 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 - */ --- -2.31.1 - diff --git a/SOURCES/0024-use-encrypted-JWTs-for-storing-encrypted-cache-conte.patch b/SOURCES/0024-use-encrypted-JWTs-for-storing-encrypted-cache-conte.patch deleted file mode 100644 index 36698f7..0000000 --- a/SOURCES/0024-use-encrypted-JWTs-for-storing-encrypted-cache-conte.patch +++ /dev/null @@ -1,406 +0,0 @@ -From 3c4949e1436473644836f0c4634b809c7526c43a Mon Sep 17 00:00:00 2001 -From: Hans Zandbelt -Date: Thu, 10 Jun 2021 15:32:48 +0200 -Subject: [PATCH 1/3] use encrypted JWTs for storing encrypted cache contents - -- avoid using static AAD/IV; thanks @niebardzo -- bump to 2.4.9-dev - -Signed-off-by: Hans Zandbelt ---- - ChangeLog | 4 + - src/cache/common.c | 331 ++++---------------------------------- - -diff --git a/ChangeLog b/ChangeLog -index 075f98d..b4ab0a1 100644 ---- a/ChangeLog -+++ b/ChangeLog -@@ -1,3 +1,7 @@ -+06/10/2021 -+- use encrypted JWTs for storing encrypted cache contents and avoid using static AAD/IV; thanks @niebardzo -+- bump to 2.4.9-dev -+ - 09/03/2020 - - add SameSite attribute on cookie clearance / logout; thanks @v0gler - - bump to 2.4.4.1 -diff --git a/src/cache/common.c b/src/cache/common.c -index c33876a..e665370 100644 ---- a/src/cache/common.c -+++ b/src/cache/common.c -@@ -229,324 +229,59 @@ apr_byte_t oidc_cache_mutex_destroy(server_rec *s, oidc_cache_mutex_t *m) { - return rv; - } - --#define oidc_cache_crypto_openssl_error(r, fmt, ...) \ -- oidc_error(r, "%s: %s", apr_psprintf(r->pool, fmt, ##__VA_ARGS__), ERR_error_string(ERR_get_error(), NULL)) -- --#define OIDC_CACHE_CIPHER EVP_aes_256_gcm() --#define OIDC_CACHE_TAG_LEN 16 -- --#if (OPENSSL_VERSION_NUMBER >= 0x10100005L && !defined(LIBRESSL_VERSION_NUMBER)) --#define OIDC_CACHE_CRYPTO_GET_TAG EVP_CTRL_AEAD_GET_TAG --#define OIDC_CACHE_CRYPTO_SET_TAG EVP_CTRL_AEAD_SET_TAG --#define OIDC_CACHE_CRYPTO_SET_IVLEN EVP_CTRL_AEAD_SET_IVLEN --#else --#define OIDC_CACHE_CRYPTO_GET_TAG EVP_CTRL_GCM_GET_TAG --#define OIDC_CACHE_CRYPTO_SET_TAG EVP_CTRL_GCM_SET_TAG --#define OIDC_CACHE_CRYPTO_SET_IVLEN EVP_CTRL_GCM_SET_IVLEN --#endif -+#define OIDC_CACHE_CRYPTO_JSON_KEY "c" - - /* -- * AES GCM encrypt -+ * AES GCM encrypt using the crypto passphrase as symmetric key - */ --static int oidc_cache_crypto_encrypt_impl(request_rec *r, -- unsigned char *plaintext, int plaintext_len, const unsigned char *aad, -- int aad_len, unsigned char *key, const unsigned char *iv, int iv_len, -- unsigned char *ciphertext, const unsigned char *tag, int tag_len) { -- EVP_CIPHER_CTX *ctx; -- -- int len; -- -- int ciphertext_len; -- -- /* create and initialize the context */ -- if (!(ctx = EVP_CIPHER_CTX_new())) { -- oidc_cache_crypto_openssl_error(r, "EVP_CIPHER_CTX_new"); -- return -1; -- } -- -- /* initialize the encryption cipher */ -- if (!EVP_EncryptInit_ex(ctx, OIDC_CACHE_CIPHER, NULL, NULL, NULL)) { -- oidc_cache_crypto_openssl_error(r, "EVP_EncryptInit_ex"); -- return -1; -- } -- -- /* set IV length */ -- if (!EVP_CIPHER_CTX_ctrl(ctx, OIDC_CACHE_CRYPTO_SET_IVLEN, iv_len, NULL)) { -- oidc_cache_crypto_openssl_error(r, "EVP_CIPHER_CTX_ctrl"); -- return -1; -- } -- -- /* initialize key and IV */ -- if (!EVP_EncryptInit_ex(ctx, NULL, NULL, key, iv)) { -- oidc_cache_crypto_openssl_error(r, "EVP_EncryptInit_ex"); -- return -1; -- } -- -- /* provide AAD data */ -- if (!EVP_EncryptUpdate(ctx, NULL, &len, aad, aad_len)) { -- oidc_cache_crypto_openssl_error(r, "EVP_DecryptUpdate aad: aad_len=%d", -- aad_len); -- return -1; -- } -- -- /* provide the message to be encrypted and obtain the encrypted output */ -- if (!EVP_EncryptUpdate(ctx, ciphertext, &len, plaintext, plaintext_len)) { -- oidc_cache_crypto_openssl_error(r, "EVP_EncryptUpdate ciphertext"); -- return -1; -- } -- ciphertext_len = len; -- -- /* -- * finalize the encryption; normally ciphertext bytes may be written at -- * this stage, but this does not occur in GCM mode -- */ -- if (!EVP_EncryptFinal_ex(ctx, ciphertext + len, &len)) { -- oidc_cache_crypto_openssl_error(r, "EVP_EncryptFinal_ex"); -- return -1; -- } -- ciphertext_len += len; -- -- /* get the tag */ -- if (!EVP_CIPHER_CTX_ctrl(ctx, OIDC_CACHE_CRYPTO_GET_TAG, tag_len, -- (void *) tag)) { -- oidc_cache_crypto_openssl_error(r, "EVP_CIPHER_CTX_ctrl"); -- return -1; -- } -- -- /* clean up */ -- EVP_CIPHER_CTX_free(ctx); -- -- return ciphertext_len; --} -- --/* -- * AES GCM decrypt -- */ --static int oidc_cache_crypto_decrypt_impl(request_rec *r, -- unsigned char *ciphertext, int ciphertext_len, const unsigned char *aad, -- int aad_len, const unsigned char *tag, int tag_len, unsigned char *key, -- const unsigned char *iv, int iv_len, unsigned char *plaintext) { -- EVP_CIPHER_CTX *ctx; -- int len; -- int plaintext_len; -- int ret; -- -- /* create and initialize the context */ -- if (!(ctx = EVP_CIPHER_CTX_new())) { -- oidc_cache_crypto_openssl_error(r, "EVP_CIPHER_CTX_new"); -- return -1; -- } -- -- /* initialize the decryption cipher */ -- if (!EVP_DecryptInit_ex(ctx, OIDC_CACHE_CIPHER, NULL, NULL, NULL)) { -- oidc_cache_crypto_openssl_error(r, "EVP_DecryptInit_ex"); -- return -1; -- } -- -- /* set IV length */ -- if (!EVP_CIPHER_CTX_ctrl(ctx, OIDC_CACHE_CRYPTO_SET_IVLEN, iv_len, NULL)) { -- oidc_cache_crypto_openssl_error(r, "EVP_CIPHER_CTX_ctrl"); -- return -1; -- } -- -- /* initialize key and IV */ -- if (!EVP_DecryptInit_ex(ctx, NULL, NULL, key, iv)) { -- oidc_cache_crypto_openssl_error(r, "EVP_DecryptInit_ex"); -- return -1; -- } -- -- /* provide AAD data */ -- if (!EVP_DecryptUpdate(ctx, NULL, &len, aad, aad_len)) { -- oidc_cache_crypto_openssl_error(r, "EVP_DecryptUpdate aad: aad_len=%d", -- aad_len); -- return -1; -- } -- -- /* provide the message to be decrypted and obtain the plaintext output */ -- if (!EVP_DecryptUpdate(ctx, plaintext, &len, ciphertext, ciphertext_len)) { -- oidc_cache_crypto_openssl_error(r, "EVP_DecryptUpdate ciphertext"); -- return -1; -- } -- plaintext_len = len; -- -- /* set expected tag value; works in OpenSSL 1.0.1d and later */ -- if (!EVP_CIPHER_CTX_ctrl(ctx, OIDC_CACHE_CRYPTO_SET_TAG, tag_len, -- (void *) tag)) { -- oidc_cache_crypto_openssl_error(r, "EVP_CIPHER_CTX_ctrl"); -- return -1; -- } -+static apr_byte_t oidc_cache_crypto_encrypt(request_rec *r, const char *plaintext, const char *key, -+ char **result) { -+ apr_byte_t rv = FALSE; -+ json_t *json = NULL; - -- /* -- * finalize the decryption; a positive return value indicates success, -- * anything else is a failure - the plaintext is not trustworthy -- */ -- ret = EVP_DecryptFinal_ex(ctx, plaintext + len, &len); -+ json = json_object(); -+ json_object_set_new(json, OIDC_CACHE_CRYPTO_JSON_KEY, json_string(plaintext)); - -- /* clean up */ -- EVP_CIPHER_CTX_free(ctx); -- -- if (ret > 0) { -- /* success */ -- plaintext_len += len; -- return plaintext_len; -- } else { -- /* verify failed */ -- oidc_cache_crypto_openssl_error(r, "EVP_DecryptFinal_ex"); -- return -1; -- } --} -+ rv = oidc_util_jwt_create(r, (const char*) key, json, result); - --/* -- * static AAD value for encryption/decryption -- */ --static const unsigned char OIDC_CACHE_CRYPTO_GCM_AAD[] = { 0x4d, 0x23, 0xc3, -- 0xce, 0xc3, 0x34, 0xb4, 0x9b, 0xdb, 0x37, 0x0c, 0x43, 0x7f, 0xec, 0x78, -- 0xde }; -+ if (json) -+ json_decref(json); - --/* -- * static IV value for encryption/decryption -- */ --static const unsigned char OIDC_CACHE_CRYPTO_GCM_IV[] = { 0x00, 0x01, 0x02, -- 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, -- 0x0f }; -- --/* -- * AES GCM encrypt using the static AAD and IV -- */ --static int oidc_cache_crypto_encrypt(request_rec *r, const char *plaintext, -- unsigned char *key, char **result) { -- char *encoded = NULL, *p = NULL, *e_tag = NULL; -- unsigned char *ciphertext = NULL; -- int plaintext_len, ciphertext_len, encoded_len, e_tag_len; -- unsigned char tag[OIDC_CACHE_TAG_LEN]; -- -- /* allocate space for the ciphertext */ -- plaintext_len = strlen(plaintext) + 1; -- ciphertext = apr_pcalloc(r->pool, -- (plaintext_len + EVP_CIPHER_block_size(OIDC_CACHE_CIPHER))); -- -- ciphertext_len = oidc_cache_crypto_encrypt_impl(r, -- (unsigned char *) plaintext, plaintext_len, -- OIDC_CACHE_CRYPTO_GCM_AAD, sizeof(OIDC_CACHE_CRYPTO_GCM_AAD), key, -- OIDC_CACHE_CRYPTO_GCM_IV, sizeof(OIDC_CACHE_CRYPTO_GCM_IV), -- ciphertext, tag, sizeof(tag)); -- -- /* base64url encode the resulting ciphertext */ -- encoded_len = oidc_base64url_encode(r, &encoded, (const char *) ciphertext, -- ciphertext_len, 1); -- if (encoded_len > 0) { -- p = encoded; -- -- /* now allocated space for the concatenated base64url encoded ciphertext and tag */ -- encoded = apr_pcalloc(r->pool, -- encoded_len + 1 + OIDC_CACHE_TAG_LEN + 1); -- memcpy(encoded, p, encoded_len); -- p = encoded + encoded_len; -- *p = OIDC_CHAR_DOT; -- p++; -- -- /* base64url encode the tag and append it in the buffer */ -- e_tag_len = oidc_base64url_encode(r, &e_tag, (const char *) tag, -- OIDC_CACHE_TAG_LEN, 1); -- memcpy(p, e_tag, e_tag_len); -- encoded_len += e_tag_len + 1; -- -- /* make sure the result is \0 terminated */ -- encoded[encoded_len] = '\0'; -- -- *result = encoded; -- } -- -- return encoded_len; -+ return rv; - } - - /* -- * AES GCM decrypt using the static AAD and IV -+ * AES GCM decrypt using the crypto passphrase as symmetric key - */ --static int oidc_cache_crypto_decrypt(request_rec *r, const char *cache_value, -- unsigned char *key, unsigned char **plaintext) { -+static apr_byte_t oidc_cache_crypto_decrypt(request_rec *r, const char *cache_value, -+ const char *key, char **plaintext) { - -- int len = -1; -+ apr_byte_t rv = FALSE; -+ json_t *json = NULL; - -- /* grab the base64url-encoded tag after the "." */ -- char *encoded_tag = strstr(cache_value, "."); -- if (encoded_tag == NULL) { -- oidc_error(r, -- "corrupted cache value: no tag separator found in encrypted value"); -- return FALSE; -- } -+ rv = oidc_util_jwt_verify(r, (const char*) key, cache_value, &json); -+ if (rv == FALSE) -+ goto end; - -- /* make sure we don't modify the original string since it may be just a pointer into the cache (shm) */ -- cache_value = apr_pstrmemdup(r->pool, cache_value, -- strlen(cache_value) - strlen(encoded_tag)); -- encoded_tag++; -+ rv = oidc_json_object_get_string(r->pool, json, OIDC_CACHE_CRYPTO_JSON_KEY, plaintext, NULL); - -- /* base64url decode the ciphertext */ -- char *d_bytes = NULL; -- int d_len = oidc_base64url_decode(r->pool, &d_bytes, cache_value); -+ end: - -- /* base64url decode the tag */ -- char *t_bytes = NULL; -- int t_len = oidc_base64url_decode(r->pool, &t_bytes, encoded_tag); -+ if (json) -+ json_decref(json); - -- /* see if we're still good to go */ -- if ((d_len > 0) && (t_len > 0)) { -- -- /* allocated space for the plaintext */ -- *plaintext = apr_pcalloc(r->pool, -- (d_len + EVP_CIPHER_block_size(OIDC_CACHE_CIPHER) - 1)); -- -- /* decrypt the ciphertext providing the tag value */ -- -- len = oidc_cache_crypto_decrypt_impl(r, (unsigned char *) d_bytes, -- d_len, OIDC_CACHE_CRYPTO_GCM_AAD, -- sizeof(OIDC_CACHE_CRYPTO_GCM_AAD), (unsigned char *) t_bytes, -- t_len, key, OIDC_CACHE_CRYPTO_GCM_IV, -- sizeof(OIDC_CACHE_CRYPTO_GCM_IV), *plaintext); -- -- /* check the result and make sure it is \0 terminated */ -- if (len > -1) { -- (*plaintext)[len] = '\0'; -- } else { -- *plaintext = NULL; -- } -- -- } -- -- return len; --} -- --/* -- * hash the crypto passhphrase so it has enough key length for AES GCM 256 -- */ --static unsigned char *oidc_cache_hash_passphrase(request_rec *r, -- const char *passphrase) { -- -- unsigned char *key = NULL; -- unsigned int key_len = 0; -- oidc_jose_error_t err; -- -- if (oidc_jose_hash_bytes(r->pool, OIDC_JOSE_ALG_SHA256, -- (const unsigned char *) passphrase, strlen(passphrase), &key, -- &key_len, &err) == FALSE) { -- oidc_error(r, "oidc_jose_hash_bytes returned an error: %s", err.text); -- return NULL; -- } -- -- return key; -+ return rv; - } - - /* - * hash a cache key and a crypto passphrase so the result is suitable as an randomized cache key - */ --static char *oidc_cache_get_hashed_key(request_rec *r, const char *passphrase, -- const char *key) { -+static char* oidc_cache_get_hashed_key(request_rec *r, const char *passphrase, const char *key) { - char *input = apr_psprintf(r->pool, "%s:%s", passphrase, key); - char *output = NULL; -- if (oidc_util_hash_string_and_base64url_encode(r, OIDC_JOSE_ALG_SHA256, -- input, &output) == FALSE) { -- oidc_error(r, -- "oidc_util_hash_string_and_base64url_encode returned an error"); -+ if (oidc_util_hash_string_and_base64url_encode(r, OIDC_JOSE_ALG_SHA256, input, &output) -+ == FALSE) { -+ oidc_error(r, "oidc_util_hash_string_and_base64url_encode returned an error"); - return NULL; - } - return output; -@@ -588,9 +323,7 @@ apr_byte_t oidc_cache_get(request_rec *r, const char *section, const char *key, - goto out; - } - -- rc = (oidc_cache_crypto_decrypt(r, cache_value, -- oidc_cache_hash_passphrase(r, cfg->crypto_passphrase), -- (unsigned char **) value) > 0); -+ rc = oidc_cache_crypto_decrypt(r, cache_value, cfg->crypto_passphrase, value); - - out: - /* log the result */ -@@ -634,9 +367,7 @@ apr_byte_t oidc_cache_set(request_rec *r, const char *section, const char *key, - goto out; - - if (value != NULL) { -- if (oidc_cache_crypto_encrypt(r, value, -- oidc_cache_hash_passphrase(r, cfg->crypto_passphrase), -- &encoded) <= 0) -+ if (oidc_cache_crypto_encrypt(r, value, cfg->crypto_passphrase, &encoded) == FALSE) - goto out; - value = encoded; - } - diff --git a/SOURCES/0025-avoid-XSS-vulnerability-when-using-OIDCPreservePost-.patch b/SOURCES/0025-avoid-XSS-vulnerability-when-using-OIDCPreservePost-.patch deleted file mode 100644 index 6a2cd33..0000000 --- a/SOURCES/0025-avoid-XSS-vulnerability-when-using-OIDCPreservePost-.patch +++ /dev/null @@ -1,41 +0,0 @@ -From dad95a3ca050910d44ff346edead722e341417ef Mon Sep 17 00:00:00 2001 -From: Hans Zandbelt -Date: Fri, 25 Jun 2021 11:42:57 +0200 -Subject: [PATCH 2/3] avoid XSS vulnerability when using OIDCPreservePost On - -and supplying URLs that contain single quotes; thanks @oss-aimoto - -Signed-off-by: Hans Zandbelt ---- - ChangeLog | 4 ++++ - src/mod_auth_openidc.c | 2 +- - 2 files changed, 5 insertions(+), 1 deletion(-) - -diff --git a/ChangeLog b/ChangeLog -index b4ab0a1..7054f0b 100644 ---- a/ChangeLog -+++ b/ChangeLog -@@ -1,3 +1,7 @@ -+06/25/2021 -+- avoid XSS vulnerability when using OIDCPreservePost On and supplying URLs that contain single quotes -+ thanks @oss-aimoto -+ - 06/10/2021 - - use encrypted JWTs for storing encrypted cache contents and avoid using static AAD/IV; thanks @niebardzo - - bump to 2.4.9-dev -diff --git a/src/mod_auth_openidc.c b/src/mod_auth_openidc.c -index 4cc7976..ea84e5e 100644 ---- a/src/mod_auth_openidc.c -+++ b/src/mod_auth_openidc.c -@@ -519,7 +519,7 @@ static int oidc_request_post_preserved_restore(request_rec *r, - " input.type = \"hidden\";\n" - " document.forms[0].appendChild(input);\n" - " }\n" -- " document.forms[0].action = '%s';\n" -+ " document.forms[0].action = \"%s\";\n" - " document.forms[0].submit();\n" - " }\n" - " \n", method, original_url); --- -2.27.0 - diff --git a/SOURCES/0026-Add-a-function-to-escape-Javascript-characters.patch b/SOURCES/0026-Add-a-function-to-escape-Javascript-characters.patch deleted file mode 100644 index c7b6b45..0000000 --- a/SOURCES/0026-Add-a-function-to-escape-Javascript-characters.patch +++ /dev/null @@ -1,149 +0,0 @@ -From 93b81054d4d1ece64a6799cc50a65b0daeabf4d1 Mon Sep 17 00:00:00 2001 -From: AIMOTO NORIHITO -Date: Mon, 28 Jun 2021 13:05:52 +0900 -Subject: [PATCH 3/3] Add a function to escape Javascript characters - ---- - src/mod_auth_openidc.c | 6 ++-- - src/mod_auth_openidc.h | 1 + - src/util.c | 81 ++++++++++++++++++++++++++++++++++++++++++ - 3 files changed, 85 insertions(+), 3 deletions(-) - -diff --git a/src/mod_auth_openidc.c b/src/mod_auth_openidc.c -index ea84e5e..37743b3 100644 ---- a/src/mod_auth_openidc.c -+++ b/src/mod_auth_openidc.c -@@ -474,7 +474,7 @@ apr_byte_t oidc_post_preserve_javascript(request_rec *r, const char *location, - " \n", jmethod, json, - location ? - apr_psprintf(r->pool, "window.location='%s';\n", -- location) : -+ oidc_util_javascript_escape(r->pool, location)) : - ""); - if (location == NULL) { - if (javascript_method) -@@ -522,7 +522,7 @@ static int oidc_request_post_preserved_restore(request_rec *r, - " document.forms[0].action = \"%s\";\n" - " document.forms[0].submit();\n" - " }\n" -- " \n", method, original_url); -+ " \n", method, oidc_util_javascript_escape(r->pool, original_url)); - - const char *body = "

Restoring...

\n" - "
\n"; -@@ -1626,7 +1626,7 @@ static int oidc_session_redirect_parent_window_to_logout(request_rec *r, - char *java_script = apr_psprintf(r->pool, - " \n", oidc_get_redirect_uri(r, c)); -+ " \n", oidc_util_javascript_escape(r->pool, oidc_get_redirect_uri(r, c))); - - return oidc_util_html_send(r, "Redirecting...", java_script, NULL, NULL, - DONE); -diff --git a/src/mod_auth_openidc.h b/src/mod_auth_openidc.h -index ea79e6b..b88937a 100644 ---- a/src/mod_auth_openidc.h -+++ b/src/mod_auth_openidc.h -@@ -747,6 +747,7 @@ apr_byte_t oidc_json_object_get_string(apr_pool_t *pool, json_t *json, const cha - apr_byte_t oidc_json_object_get_int(apr_pool_t *pool, json_t *json, const char *name, int *value, const int default_value); - apr_byte_t oidc_json_object_get_bool(apr_pool_t *pool, json_t *json, const char *name, int *value, const int default_value); - char *oidc_util_html_escape(apr_pool_t *pool, const char *input); -+char *oidc_util_javascript_escape(apr_pool_t *pool, const char *input); - void oidc_util_table_add_query_encoded_params(apr_pool_t *pool, apr_table_t *table, const char *params); - apr_hash_t * oidc_util_merge_key_sets(apr_pool_t *pool, apr_hash_t *k1, apr_hash_t *k2); - apr_byte_t oidc_util_regexp_substitute(apr_pool_t *pool, const char *input, const char *regexp, const char *replace, char **output, char **error_str); -diff --git a/src/util.c b/src/util.c -index 4b4e16b..f98c824 100644 ---- a/src/util.c -+++ b/src/util.c -@@ -369,6 +369,87 @@ char *oidc_util_html_escape(apr_pool_t *pool, const char *s) { - return apr_pstrdup(pool, r); - } - -+/* -+ * JavaScript escape a string -+ */ -+char* oidc_util_javascript_escape(apr_pool_t *pool, const char *s) { -+ const char *cp; -+ char *output; -+ size_t outputlen; -+ int i; -+ -+ if (s == NULL) { -+ return NULL; -+ } -+ -+ outputlen = 0; -+ for (cp = s; *cp; cp++) { -+ switch (*cp) { -+ case '\'': -+ case '"': -+ case '\\': -+ case '/': -+ case 0x0D: -+ case 0x0A: -+ outputlen += 2; -+ break; -+ case '<': -+ case '>': -+ outputlen += 4; -+ break; -+ default: -+ outputlen += 1; -+ break; -+ } -+ } -+ -+ i = 0; -+ output = apr_palloc(pool, outputlen + 1); -+ for (cp = s; *cp; cp++) { -+ switch (*cp) { -+ case '\'': -+ (void)strcpy(&output[i], "\\'"); -+ i += 2; -+ break; -+ case '"': -+ (void)strcpy(&output[i], "\\\""); -+ i += 2; -+ break; -+ case '\\': -+ (void)strcpy(&output[i], "\\\\"); -+ i += 2; -+ break; -+ case '/': -+ (void)strcpy(&output[i], "\\/"); -+ i += 2; -+ break; -+ case 0x0D: -+ (void)strcpy(&output[i], "\\r"); -+ i += 2; -+ break; -+ case 0x0A: -+ (void)strcpy(&output[i], "\\n"); -+ i += 2; -+ break; -+ case '<': -+ (void)strcpy(&output[i], "\\x3c"); -+ i += 4; -+ break; -+ case '>': -+ (void)strcpy(&output[i], "\\x3e"); -+ i += 4; -+ break; -+ default: -+ output[i] = *cp; -+ i += 1; -+ break; -+ } -+ } -+ output[i] = '\0'; -+ return output; -+} -+ -+ - /* - * get the URL scheme that is currently being accessed - */ --- -2.27.0 - diff --git a/SOURCES/test-segfault.patch b/SOURCES/test-segfault.patch deleted file mode 100644 index 34bf7f0..0000000 --- a/SOURCES/test-segfault.patch +++ /dev/null @@ -1,167 +0,0 @@ -commit fe7dfb14c45262df3b15bda374b2ee390b43cfb4 -Author: John Dennis -Date: Tue Aug 14 18:08:56 2018 -0400 - - test_proto_authorization_request() segfault due to uninitialized value - - Many thanks to Florian Weimer 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 "\n\n \n \n 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 */ diff --git a/SPECS/mod_auth_openidc.spec b/SPECS/mod_auth_openidc.spec index d327a73..d805940 100644 --- a/SPECS/mod_auth_openidc.spec +++ b/SPECS/mod_auth_openidc.spec @@ -1,4 +1,4 @@ -%{!?_httpd_mmn: %{expand: %%global _httpd_mmn %%(cat %{_includedir}/httpd/.mmn || echo 0-0)}} +%{!?_httpd_mmn: %{expand: %%global _httpd_mmn %%(cat %{_includedir}/httpd/.mmn 2>/dev/null || echo 0-0)}} %{!?_httpd_moddir: %{expand: %%global _httpd_moddir %%{_libdir}/httpd/modules}} %{!?_httpd_confdir: %{expand: %%global _httpd_confdir %{_sysconfdir}/httpd/conf.d}} @@ -14,41 +14,13 @@ %global httpd_pkg_cache_dir /var/cache/httpd/mod_auth_openidc Name: mod_auth_openidc -Version: 2.3.7 -Release: 11%{?dist} +Version: 2.4.9.4 +Release: 1%{?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 -Patch20: 0020-prevent-open-redirect-on-refresh-token-requests-rele.patch -Patch21: 0021-prevent-XSS-and-open-redirect-on-OIDC-session-manage.patch -Patch22: 0022-replace-potentially-harmful-backslashes-with-forward.patch -Patch23: 0023-apply-OIDCRedirectURLsAllowed-setting-to-target_link.patch -Patch24: 0024-use-encrypted-JWTs-for-storing-encrypted-cache-conte.patch -Patch25: 0025-avoid-XSS-vulnerability-when-using-OIDCPreservePost-.patch -Patch26: 0026-Add-a-function-to-escape-Javascript-characters.patch +Source0: https://github.com/zmartzone/mod_auth_openidc/archive/v%{version}.tar.gz BuildRequires: gcc BuildRequires: httpd-devel @@ -69,32 +41,6 @@ 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 -%patch20 -p1 -%patch21 -p1 -%patch22 -p1 -%patch23 -p1 -%patch24 -p1 -%patch25 -p1 -%patch26 -p1 %build # workaround rpm-buildroot-usage @@ -104,9 +50,11 @@ autoreconf %configure \ --with-jq=/usr/lib64/ \ %{?_with_hiredis} \ - %{?_without_hiredis} + %{?_without_hiredis} \ + --with-apxs2=%{_httpd_apxs} -make %{?_smp_mflags} + +%{make_build} %check export MODULES_DIR=%{_httpd_moddir} @@ -147,6 +95,9 @@ install -m 700 -d $RPM_BUILD_ROOT%{httpd_pkg_cache_dir}/cache %dir %attr(0700, apache, apache) %{httpd_pkg_cache_dir}/cache %changelog +* Fri Apr 8 2022 Tomas Halman <thalman@redhat.com> - 2.4.9.4-1 +- Resolves: rhbz#2025368 - Rebase to new version + * Fri Jan 28 2022 Tomas Halman <thalman@redhat.com> - 2.3.7-11 - Resolves: rhbz#1987222 - CVE-2021-32792 XSS when using OIDCPreservePost On