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