From bef612ec00a4c8f3e504158fff1e3b4e1f7ecda7 Mon Sep 17 00:00:00 2001 From: CentOS Sources Date: May 10 2022 07:02:40 +0000 Subject: import mod_auth_openidc-2.3.7-11.module+el8.6.0+14082+b6f23e95 --- 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 new file mode 100644 index 0000000..4dadc5b --- /dev/null +++ b/SOURCES/0020-prevent-open-redirect-on-refresh-token-requests-rele.patch @@ -0,0 +1,453 @@ +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 new file mode 100644 index 0000000..f1da3a4 --- /dev/null +++ b/SOURCES/0021-prevent-XSS-and-open-redirect-on-OIDC-session-manage.patch @@ -0,0 +1,167 @@ +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 new file mode 100644 index 0000000..170e275 --- /dev/null +++ b/SOURCES/0022-replace-potentially-harmful-backslashes-with-forward.patch @@ -0,0 +1,42 @@ +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 new file mode 100644 index 0000000..24d6b1e --- /dev/null +++ b/SOURCES/0023-apply-OIDCRedirectURLsAllowed-setting-to-target_link.patch @@ -0,0 +1,306 @@ +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 new file mode 100644 index 0000000..36698f7 --- /dev/null +++ b/SOURCES/0024-use-encrypted-JWTs-for-storing-encrypted-cache-conte.patch @@ -0,0 +1,406 @@ +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 new file mode 100644 index 0000000..6a2cd33 --- /dev/null +++ b/SOURCES/0025-avoid-XSS-vulnerability-when-using-OIDCPreservePost-.patch @@ -0,0 +1,41 @@ +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 new file mode 100644 index 0000000..c7b6b45 --- /dev/null +++ b/SOURCES/0026-Add-a-function-to-escape-Javascript-characters.patch @@ -0,0 +1,149 @@ +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/SPECS/mod_auth_openidc.spec b/SPECS/mod_auth_openidc.spec index 6d55854..d327a73 100644 --- a/SPECS/mod_auth_openidc.spec +++ b/SPECS/mod_auth_openidc.spec @@ -15,7 +15,7 @@ Name: mod_auth_openidc Version: 2.3.7 -Release: 8%{?dist} +Release: 11%{?dist} Summary: OpenID Connect auth module for Apache HTTP Server Group: System Environment/Daemons @@ -42,7 +42,13 @@ 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 BuildRequires: gcc BuildRequires: httpd-devel @@ -82,6 +88,13 @@ an OpenID Connect Relying Party and/or OAuth 2.0 Resource Server. %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 @@ -134,6 +147,17 @@ install -m 700 -d $RPM_BUILD_ROOT%{httpd_pkg_cache_dir}/cache %dir %attr(0700, apache, apache) %{httpd_pkg_cache_dir}/cache %changelog +* Fri Jan 28 2022 Tomas Halman - 2.3.7-11 +- Resolves: rhbz#1987222 - CVE-2021-32792 XSS when using OIDCPreservePost On + +* Fri Jan 28 2022 Tomas Halman - 2.3.7-10 +- Resolves: rhbz#1987216 - CVE-2021-32791 hardcoded static IV and AAD with a + reused key in AES GCM encryption [rhel-8] (edit) + +* Fri Oct 29 2021 Tomas Halman - 2.3.7-9 +- Resolves: rhbz#2001853 - CVE-2021-39191 open redirect by supplying a crafted URL + in the target_link_uri parameter + * Tue Nov 17 2020 Jakub Hrozek - 2.3.7-8 - Resolves: rhbz#1823756 - Backport SameSite=None cookie from mod_auth_openidc upstream to support latest browsers