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

import mod_auth_openidc-2.3.7-11.module+el8.6.0+14082+b6f23e95

Browse Source
This commit is contained in:
CentOS Sources 2022-05-10 03:02:40 -04:00 committed by Stepan Oksanichenko
parent 596714313c
commit 59d4e05cc1
8 changed files with 1590 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

453
SOURCES/0020-prevent-open-redirect-on-refresh-token-requests-rele.patch Normal file
View File

@ -0,0 +1,453 @@
From e4d625b0f1116e50df2737e2738b4c77b35328bb Mon Sep 17 00:00:00 2001
From: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
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 <hans.zandbelt@zmartzone.eu>
(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 [<regexp>]+
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 =
"<style type=\"text/css\">body {text-align: center}</style>";
- 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

167
SOURCES/0021-prevent-XSS-and-open-redirect-on-OIDC-session-manage.patch Normal file
View File

@ -0,0 +1,167 @@
From a1b8e7aa92e5e624a5f90bb736c307dae22230a1 Mon Sep 17 00:00:00 2001
From: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
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 <hans.zandbelt@zmartzone.eu>
(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

42
SOURCES/0022-replace-potentially-harmful-backslashes-with-forward.patch Normal file
View File

@ -0,0 +1,42 @@
From 466f470265554e0e3ae27a6d82375456d2c133e6 Mon Sep 17 00:00:00 2001
From: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
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

306
SOURCES/0023-apply-OIDCRedirectURLsAllowed-setting-to-target_link.patch Normal file
View File

@ -0,0 +1,306 @@
From eac10ec54971ff021113f96c80deab9991faafc4 Mon Sep 17 00:00:00 2001
From: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
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 <hans.zandbelt@zmartzone.eu>
(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 <https://github.com/timpuri>
Eldar Zaitov <https://github.com/kyprizel>
Gergan Penkov <https://github.com/gergan>
+ Florian Weimer <https://github.com/fweimer>
+ Aaron Donovan <https://github.com/amdonov>
+ Hans Petter Bieker <https://github.com/hpbieker>
+ archzone <https://github.com/archzone>
+ Petteri Stenius <https://github.com/psteniusubi>
+ Lance Fannin <lancekf08@gmail.com>
+ Ricardo Martin Camarero <https://github.com/rickyepoderi>
+ Filip Vujicic <https://github.com/FilipVujicic>
+ Janusz Ulanowski <https://github.com/janul>
+ Aimoto Norihito <https://github.com/oss-aimoto>
+ Andy Lindeman <https://github.com/alindeman>
+ Stefan Wachter <https://github.com/swachter>
+ Paolo Battino
+ absynth76 <https://github.com/absynth76>
+ Aaron Jones <https://github.com/wwaaron>
+ Bryan Ingram <https://github/bcingram>
+ Tim Deisser <https://github.com/deisser>
+ Peter Hurtenbach <https://github.com/Peter0x48>
+ Paul Spangler <https://github.com/spanglerco>
+ Chris Pawling <https://github.com/chris468>
+ Matthias Fleschütz <https://github.com/blindzero>
+ Harri Rautila <https://github.com/hrautila>
+ Tatsuhiko Yasumatsu <https://github.com/ty60>
+ Adam Stadler <https://github.com/tzfx>
+ Steffen Greber <https://github.com/codemaker219>
+ Iain Heggie <https://github.com/iainh>
+ Dirk Kok <https://github.com/Foxite>
+ 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

406
SOURCES/0024-use-encrypted-JWTs-for-storing-encrypted-cache-conte.patch Normal file
View File

@ -0,0 +1,406 @@
From 3c4949e1436473644836f0c4634b809c7526c43a Mon Sep 17 00:00:00 2001
From: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
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 <hans.zandbelt@zmartzone.eu>
---
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;
}

41
SOURCES/0025-avoid-XSS-vulnerability-when-using-OIDCPreservePost-.patch Normal file
View File

@ -0,0 +1,41 @@
From dad95a3ca050910d44ff346edead722e341417ef Mon Sep 17 00:00:00 2001
From: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
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 <hans.zandbelt@zmartzone.eu>
---
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"
" </script>\n", method, original_url);
--
2.27.0

149
SOURCES/0026-Add-a-function-to-escape-Javascript-characters.patch Normal file
View File

@ -0,0 +1,149 @@
From 93b81054d4d1ece64a6799cc50a65b0daeabf4d1 Mon Sep 17 00:00:00 2001
From: AIMOTO NORIHITO <aimoto@osstech.co.jp>
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,
" </script>\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"
- " </script>\n", method, original_url);
+ " </script>\n", method, oidc_util_javascript_escape(r->pool, original_url));
const char *body = " <p>Restoring...</p>\n"
" <form method=\"post\"></form>\n";
@@ -1626,7 +1626,7 @@ static int oidc_session_redirect_parent_window_to_logout(request_rec *r,
char *java_script = apr_psprintf(r->pool,
" <script type=\"text/javascript\">\n"
" window.top.location.href = '%s?session=logout';\n"
- " </script>\n", oidc_get_redirect_uri(r, c));
+ " </script>\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

28
SPECS/mod_auth_openidc.spec
View File

@ -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 <thalman@redhat.com> - 2.3.7-11
- Resolves: rhbz#1987222 - CVE-2021-32792 XSS when using OIDCPreservePost On
* Fri Jan 28 2022 Tomas Halman <thalman@redhat.com> - 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 <thalman@redhat.com> - 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 <jhrozek@redhat.com> - 2.3.7-8
- Resolves: rhbz#1823756 - Backport SameSite=None cookie from
mod_auth_openidc upstream to support latest browsers