Skip to content

Commit

Permalink
apply OIDCRedirectURLsAllowed setting to target_link_uri
Browse files Browse the repository at this point in the history 
closes #672; thanks @Meheni
release 2.4.9.4

Signed-off-by: Hans Zandbelt <hans.zandbelt@zmartzone.eu>
  • Loading branch information
@zandbelt
zandbelt committed Sep 3, 2021
1 parent d6a9361 commit 03e6bfb
Show file tree
Hide file tree
Showing 5 changed files with 109 additions and 93 deletions.
2 changes: 2 additions & 0 deletions AUTHORS
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,5 +73,7 @@ reporting bugs, providing fixes, suggesting useful features or other:
Steffen Greber <https://github.com/codemaker219>
Iain Heggie <https://github.com/iainh>
Dirk Kok <https://github.com/Foxite>
Meheni https://github.com/Meheni



2 changes: 2 additions & 0 deletions ChangeLog
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
09/03/2021
- don't apply authz in discovery process; fixes 2.4.9.3
- apply OIDCRedirectURLsAllowed setting to target_link_uri; closes #672; thanks @Meheni
- release 2.4.9.4

08/26/2021
- don't apply authz to the redirect URI; fixes ac5686495a51bc93e257e42bfdc9c9c46252feb1
Expand Down
5 changes: 3 additions & 2 deletions auth_openidc.conf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -918,8 +918,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$
Expand Down
2 changes: 1 addition & 1 deletion configure.ac
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
AC_INIT([mod_auth_openidc],[2.4.9.3],[hans.zandbelt@zmartzone.eu])
AC_INIT([mod_auth_openidc],[2.4.9.4],[hans.zandbelt@zmartzone.eu])

AC_SUBST(NAMEVER, AC_PACKAGE_TARNAME()-AC_PACKAGE_VERSION())

Expand Down
191 changes: 101 additions & 90 deletions src/mod_auth_openidc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2457,6 +2457,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
*/
Expand All @@ -2467,6 +2557,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);
Expand Down Expand Up @@ -2510,7 +2602,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,
Expand All @@ -2519,6 +2611,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)
Expand Down Expand Up @@ -2947,95 +3047,6 @@ static int oidc_handle_logout_backchannel(request_rec *r, oidc_cfg *cfg) {
return rc;
}

#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;
}

/*
* perform (single) logout
Expand Down

0 comments on commit 03e6bfb

Please sign in to comment.