Skip to content

Commit

Permalink
add warning/errors when configured hosts/domains do not match
Browse files Browse the repository at this point in the history
match the URL being accessed against the OIDCRedirectURI and
OIDCCookieDomain
  • Loading branch information
Hans Zandbelt committed Jun 5, 2014
1 parent e9857b4 commit 92f60fd
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 1 deletion.
1 change: 1 addition & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
6/5/2014
- do not set Secure cookies for plain HTTP
- add warning/errors when configured hosts/domains do not match

6/4/2014
- fix passing integer claims on non-Mac OS X systems
Expand Down
18 changes: 18 additions & 0 deletions src/config.c
Original file line number Diff line number Diff line change
Expand Up @@ -835,6 +835,24 @@ static int oidc_check_config_openid_openidc(server_rec *s, oidc_cfg *c) {
return oidc_check_config_error(s, "OIDCClientSecret");
}

apr_uri_t r_uri;
apr_uri_parse(s->process->pconf, c->redirect_uri, &r_uri);
if (apr_strnatcmp(r_uri.scheme, "https") != 0) {
ap_log_error(APLOG_MARK, APLOG_WARNING, 0, s,
"oidc_check_config_openid_openidc: the URL scheme (%s) of the configured OIDCRedirectURI SHOULD be \"https\" for security reasons (moreover: some Providers may reject non-HTTPS URLs)",
r_uri.scheme);
}

if (c->cookie_domain != NULL) {
char *p = strstr(r_uri.hostname, c->cookie_domain);
if ((p == NULL) || (apr_strnatcmp(c->cookie_domain, p) != 0)) {
ap_log_error(APLOG_MARK, APLOG_ERR, 0, s,
"oidc_check_config_openid_openidc: the domain (%s) configured in OIDCCookieDomain does not match the URL hostname (%s) of the configured OIDCRedirectURI (%s): setting \"state\" and \"session\" cookies will not work!",
c->cookie_domain, r_uri.hostname, c->redirect_uri);
return HTTP_INTERNAL_SERVER_ERROR;
}
}

return OK;
}

Expand Down
2 changes: 1 addition & 1 deletion src/metadata.c
Original file line number Diff line number Diff line change
Expand Up @@ -770,7 +770,7 @@ static apr_byte_t oidc_metadata_client_get(request_rec *r, oidc_cfg *cfg,
/* at this point we have no valid client metadata, see if there's a registration endpoint for this provider */
if (provider->registration_endpoint_url == NULL) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
"oidc_metadata_client_get: no (valid) client metadata exists and provider JSON object did not contain a (valid) \"registration_endpoint\" string");
"oidc_metadata_client_get: no (valid) client metadata exists for provider (%s) and provider JSON object did not contain a (valid) \"registration_endpoint\" string", issuer);
return FALSE;
}

Expand Down
25 changes: 25 additions & 0 deletions src/mod_auth_openidc.c
Original file line number Diff line number Diff line change
Expand Up @@ -1134,6 +1134,31 @@ static int oidc_authenticate_user(request_rec *r, oidc_cfg *c,
provider->response_type, "code token"))))
proto_state.nonce = NULL;

/*
* printout errors if Cookie settings are not going to work
*/
apr_uri_t o_uri;
apr_uri_t r_uri;
apr_uri_parse(r->pool, original_url, &o_uri);
apr_uri_parse(r->pool, c->redirect_uri, &r_uri);
if ( (apr_strnatcmp(o_uri.scheme, r_uri.scheme) != 0) && (apr_strnatcmp(r_uri.scheme, "https") == 0) ) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, "oidc_authenticate_user: the URL scheme (%s) of the configured OIDCRedirectURI does not match the URL scheme of the URL being accessed (%s): the \"state\" and \"session\" cookies will not be shared between the two!", r_uri.scheme, o_uri.scheme);
}

if (c->cookie_domain == NULL) {
if (apr_strnatcmp(o_uri.hostname, r_uri.hostname) != 0) {
char *p = strstr(o_uri.hostname, r_uri.hostname);
if ( (p == NULL) || (apr_strnatcmp(r_uri.hostname, p) != 0)) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, "oidc_authenticate_user: the URL hostname (%s) of the configured OIDCRedirectURI does not match the URL hostname of the URL being accessed (%s): the \"state\" and \"session\" cookies will not be shared between the two!", r_uri.hostname, o_uri.hostname);
}
}
} else {
char *p = strstr(o_uri.hostname, c->cookie_domain);
if ( (p == NULL) || (apr_strnatcmp(c->cookie_domain, p) != 0)) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, "oidc_authenticate_user: the domain (%s) configured in OIDCCookieDomain does not match the URL hostname (%s) of the URL being accessed (%s): setting \"state\" and \"session\" cookies will not work!!", c->cookie_domain, o_uri.hostname, original_url);
}
}

/* send off to the OpenID Connect Provider */
// TODO: maybe show intermediate/progress screen "redirecting to"
return oidc_proto_authorization_request(r, provider, c->redirect_uri, state, &proto_state);
Expand Down

0 comments on commit 92f60fd

Please sign in to comment.