From 9a0827a18ec4d16cf9e79afede901194d6ef5cc6 Mon Sep 17 00:00:00 2001 From: Hans Zandbelt Date: Thu, 23 Aug 2018 00:04:38 +0200 Subject: [PATCH 10/11] improve auto-detection of XMLHttpRequests via Accept header; see #331 version 2.3.8rc5 Signed-off-by: Hans Zandbelt (cherry picked from commit cff35bba8861ddb09d694ecfcd88d5fac1018453) --- src/mod_auth_openidc.c | 41 +++--- src/mod_auth_openidc.h | 6 +- src/util.c | 80 +++++++--- diff --git a/src/mod_auth_openidc.c b/src/mod_auth_openidc.c index e3817a9..897a449 100644 --- a/src/mod_auth_openidc.c +++ b/src/mod_auth_openidc.c @@ -805,8 +805,8 @@ static apr_byte_t oidc_restore_proto_state(request_rec *r, oidc_cfg *c, * set the state that is maintained between an authorization request and an authorization response * in a cookie in the browser that is cryptographically bound to that state */ -static int oidc_authorization_request_set_cookie(request_rec *r, - oidc_cfg *c, const char *state, oidc_proto_state_t *proto_state) { +static int oidc_authorization_request_set_cookie(request_rec *r, oidc_cfg *c, + const char *state, oidc_proto_state_t *proto_state) { /* * create a cookie consisting of 8 elements: * random value, original URL, original method, issuer, response_type, response_mod, prompt and timestamp @@ -818,7 +818,7 @@ static int oidc_authorization_request_set_cookie(request_rec *r, /* * clean expired state cookies to avoid pollution and optionally - * try to avoid the number of state cookies exceeding a max + * try to avoid the number of state cookies exceeding a max */ int number_of_cookies = oidc_clean_expired_state_cookies(r, c, NULL); int max_number_of_cookies = oidc_cfg_max_number_of_state_cookies(c); @@ -953,6 +953,26 @@ static void oidc_log_session_expires(request_rec *r, const char *msg, apr_time_sec(session_expires - apr_time_now())); } +/* + * see if this is a non-browser request + */ +static apr_byte_t oidc_is_xml_http_request(request_rec *r) { + + if ((oidc_util_hdr_in_x_requested_with_get(r) != NULL) + && (apr_strnatcasecmp(oidc_util_hdr_in_x_requested_with_get(r), + OIDC_HTTP_HDR_VAL_XML_HTTP_REQUEST) == 0)) + return TRUE; + + if ((oidc_util_hdr_in_accept_contains(r, OIDC_CONTENT_TYPE_TEXT_HTML) + == FALSE) && (oidc_util_hdr_in_accept_contains(r, + OIDC_CONTENT_TYPE_APP_XHTML_XML) == FALSE) + && (oidc_util_hdr_in_accept_contains(r, + OIDC_CONTENT_TYPE_ANY) == FALSE)) + return TRUE; + + return FALSE; +} + /* * find out which action we need to take when encountering an unauthenticated request */ @@ -982,9 +1002,7 @@ static int oidc_handle_unauthenticated_user(request_rec *r, oidc_cfg *c) { * won't redirect the user and thus avoid creating a state cookie * for a non-browser (= Javascript) call that will never return from the OP */ - if ((oidc_util_hdr_in_x_requested_with_get(r) != NULL) - && (apr_strnatcasecmp(oidc_util_hdr_in_x_requested_with_get(r), - OIDC_HTTP_HDR_VAL_XML_HTTP_REQUEST) == 0)) + if (oidc_is_xml_http_request(r) == TRUE) return HTTP_UNAUTHORIZED; } @@ -997,17 +1015,6 @@ static int oidc_handle_unauthenticated_user(request_rec *r, oidc_cfg *c) { oidc_dir_cfg_path_scope(r)); } -/* - * see if this is a non-browser request - */ -static apr_byte_t oidc_is_xml_http_request(request_rec *r) { - if ((oidc_util_hdr_in_x_requested_with_get(r) != NULL) - && (apr_strnatcasecmp(oidc_util_hdr_in_x_requested_with_get(r), - OIDC_HTTP_HDR_VAL_XML_HTTP_REQUEST) == 0)) - return TRUE; - return FALSE; -} - /* * check if maximum session duration was exceeded */ diff --git a/src/mod_auth_openidc.h b/src/mod_auth_openidc.h index 5162ed4..f89f392 100644 --- a/src/mod_auth_openidc.h +++ b/src/mod_auth_openidc.h @@ -537,7 +537,9 @@ apr_byte_t oidc_oauth_get_bearer_token(request_rec *r, const char **access_token #define OIDC_CONTENT_TYPE_JWT "application/jwt" #define OIDC_CONTENT_TYPE_FORM_ENCODED "application/x-www-form-urlencoded" #define OIDC_CONTENT_TYPE_IMAGE_PNG "image/png" -#define OIDC_CONTENT_TYPE_HTML "text/html" +#define OIDC_CONTENT_TYPE_TEXT_HTML "text/html" +#define OIDC_CONTENT_TYPE_APP_XHTML_XML "application/xhtml+xml" +#define OIDC_CONTENT_TYPE_ANY "*/*" #define OIDC_STR_SPACE " " #define OIDC_STR_EQUAL "=" @@ -560,6 +562,7 @@ apr_byte_t oidc_oauth_get_bearer_token(request_rec *r, const char **access_token #define OIDC_CHAR_FORWARD_SLASH '/' #define OIDC_CHAR_PIPE '|' #define OIDC_CHAR_AMP '&' +#define OIDC_CHAR_SEMI_COLON ';' #define OIDC_APP_INFO_REFRESH_TOKEN "refresh_token" #define OIDC_APP_INFO_ACCESS_TOKEN "access_token" @@ -787,6 +790,7 @@ const char *oidc_util_hdr_in_host_get(const request_rec *r); void oidc_util_hdr_out_location_set(const request_rec *r, const char *value); const char *oidc_util_hdr_out_location_get(const request_rec *r); void oidc_util_hdr_err_out_add(const request_rec *r, const char *name, const char *value); +apr_byte_t oidc_util_hdr_in_accept_contains(const request_rec *r, const char *needle); // oidc_metadata.c apr_byte_t oidc_metadata_provider_retrieve(request_rec *r, oidc_cfg *cfg, const char *issuer, const char *url, json_t **j_metadata, char **response); diff --git a/src/util.c b/src/util.c index 2fd79ec..67b2fc3 100644 --- a/src/util.c +++ b/src/util.c @@ -97,7 +97,7 @@ int oidc_base64url_encode(request_rec *r, char **dst, const char *src, enc_len--; if ((enc_len > 0) && (enc[enc_len - 1] == ',')) enc_len--; - if ((enc_len > 0) &&(enc[enc_len - 1] == ',')) + if ((enc_len > 0) && (enc[enc_len - 1] == ',')) enc_len--; enc[enc_len] = '\0'; } @@ -320,9 +320,9 @@ char *oidc_util_unescape_string(const request_rec *r, const char *str) { return NULL; } int counter = 0; - char *replaced = (char *)str; - while(str[counter] != '\0') { - if(str[counter] == '+') { + char *replaced = (char *) str; + while (str[counter] != '\0') { + if (str[counter] == '+') { replaced[counter] = ' '; } counter++; @@ -353,7 +353,7 @@ char *oidc_util_html_escape(apr_pool_t *pool, const char *s) { for (i = 0; i < strlen(s); i++) { for (n = 0; n < len; n++) { if (s[i] == chars[n]) { - m = (unsigned int)strlen(replace[n]); + m = (unsigned int) strlen(replace[n]); for (k = 0; k < m; k++) r[j + k] = replace[n][k]; j += m; @@ -530,12 +530,13 @@ const char *oidc_get_redirect_uri_iss(request_rec *r, oidc_cfg *cfg, const char *redirect_uri = oidc_get_redirect_uri(r, cfg); if (provider->issuer_specific_redirect_uri != 0) { redirect_uri = apr_psprintf(r->pool, "%s%s%s=%s", redirect_uri, - strchr(redirect_uri ? redirect_uri : "", OIDC_CHAR_QUERY) != NULL ? - OIDC_STR_AMP : - OIDC_STR_QUERY, - OIDC_PROTO_ISS, oidc_util_escape_string(r, provider->issuer)); -// OIDC_PROTO_CLIENT_ID, -// oidc_util_escape_string(r, provider->client_id)); + strchr(redirect_uri ? redirect_uri : "", + OIDC_CHAR_QUERY) != NULL ? + OIDC_STR_AMP : + OIDC_STR_QUERY, + OIDC_PROTO_ISS, oidc_util_escape_string(r, provider->issuer)); + // OIDC_PROTO_CLIENT_ID, + // oidc_util_escape_string(r, provider->client_id)); oidc_debug(r, "determined issuer specific redirect uri: %s", redirect_uri); } @@ -1346,8 +1347,8 @@ int oidc_util_html_send(request_rec *r, const char *title, on_load ? apr_psprintf(r->pool, " onload=\"%s()\"", on_load) : "", html_body ? html_body : "

"); - return oidc_util_http_send(r, html, strlen(html), OIDC_CONTENT_TYPE_HTML, - status_code); + return oidc_util_http_send(r, html, strlen(html), + OIDC_CONTENT_TYPE_TEXT_HTML, status_code); } static char *html_error_template_contents = NULL; @@ -1357,7 +1358,8 @@ static char *html_error_template_contents = NULL; * that is relative to the Apache root directory */ char *oidc_util_get_full_path(apr_pool_t *pool, const char *abs_or_rel_filename) { - return (abs_or_rel_filename) ? ap_server_root_relative(pool, abs_or_rel_filename) : NULL; + return (abs_or_rel_filename) ? + ap_server_root_relative(pool, abs_or_rel_filename) : NULL; } /* @@ -1389,7 +1391,7 @@ int oidc_util_html_send_error(request_rec *r, const char *html_template, description ? description : "")); return oidc_util_http_send(r, html, strlen(html), - OIDC_CONTENT_TYPE_HTML, status_code); + OIDC_CONTENT_TYPE_TEXT_HTML, status_code); } } @@ -1980,8 +1982,8 @@ void oidc_util_table_add_query_encoded_params(apr_pool_t *pool, * create a symmetric key from a client_secret */ apr_byte_t oidc_util_create_symmetric_key(request_rec *r, - const char *client_secret, unsigned int r_key_len, const char *hash_algo, - apr_byte_t set_kid, oidc_jwk_t **jwk) { + const char *client_secret, unsigned int r_key_len, + const char *hash_algo, apr_byte_t set_kid, oidc_jwk_t **jwk) { oidc_jose_error_t err; unsigned char *key = NULL; unsigned int key_len; @@ -2216,7 +2218,8 @@ static const char *oidc_util_hdr_in_get(const request_rec *r, const char *name) return value; } -static const char *oidc_util_hdr_in_get_left_most_only(const request_rec *r, const char *name, const char *separator) { +static const char *oidc_util_hdr_in_get_left_most_only(const request_rec *r, + const char *name, const char *separator) { char *last = NULL; const char *value = oidc_util_hdr_in_get(r, name); if (value) @@ -2224,6 +2227,29 @@ static const char *oidc_util_hdr_in_get_left_most_only(const request_rec *r, con return NULL; } +static apr_byte_t oidc_util_hdr_in_contains(const request_rec *r, + const char *name, const char *separator, const char postfix_separator, + const char *needle) { + char *ctx = NULL, *elem = NULL; + const char *value = oidc_util_hdr_in_get(r, name); + apr_byte_t rc = FALSE; + if (value) { + elem = apr_strtok(apr_pstrdup(r->pool, value), separator, &ctx); + while (elem != NULL) { + while (*elem == OIDC_CHAR_SPACE) + elem++; + if ((strncmp(elem, needle, strlen(needle)) == 0) + && ((elem[strlen(needle)] == '\0') + || (elem[strlen(needle)] == postfix_separator))) { + rc = TRUE; + break; + } + elem = apr_strtok(NULL, separator, &ctx); + } + } + return rc; +} + static void oidc_util_hdr_table_set(const request_rec *r, apr_table_t *table, const char *name, const char *value) { @@ -2288,7 +2314,8 @@ const char *oidc_util_hdr_in_user_agent_get(const request_rec *r) { } const char *oidc_util_hdr_in_x_forwarded_for_get(const request_rec *r) { - return oidc_util_hdr_in_get_left_most_only(r, OIDC_HTTP_HDR_X_FORWARDED_FOR, OIDC_STR_COMMA); + return oidc_util_hdr_in_get_left_most_only(r, OIDC_HTTP_HDR_X_FORWARDED_FOR, + OIDC_STR_COMMA); } const char *oidc_util_hdr_in_content_type_get(const request_rec *r) { @@ -2303,20 +2330,29 @@ const char *oidc_util_hdr_in_accept_get(const request_rec *r) { return oidc_util_hdr_in_get(r, OIDC_HTTP_HDR_ACCEPT); } +apr_byte_t oidc_util_hdr_in_accept_contains(const request_rec *r, + const char *needle) { + return oidc_util_hdr_in_contains(r, OIDC_HTTP_HDR_ACCEPT, OIDC_STR_COMMA, + OIDC_CHAR_SEMI_COLON, needle); +} + const char *oidc_util_hdr_in_authorization_get(const request_rec *r) { return oidc_util_hdr_in_get(r, OIDC_HTTP_HDR_AUTHORIZATION); } const char *oidc_util_hdr_in_x_forwarded_proto_get(const request_rec *r) { - return oidc_util_hdr_in_get_left_most_only(r, OIDC_HTTP_HDR_X_FORWARDED_PROTO, OIDC_STR_COMMA); + return oidc_util_hdr_in_get_left_most_only(r, + OIDC_HTTP_HDR_X_FORWARDED_PROTO, OIDC_STR_COMMA); } const char *oidc_util_hdr_in_x_forwarded_port_get(const request_rec *r) { - return oidc_util_hdr_in_get_left_most_only(r, OIDC_HTTP_HDR_X_FORWARDED_PORT, OIDC_STR_COMMA); + return oidc_util_hdr_in_get_left_most_only(r, + OIDC_HTTP_HDR_X_FORWARDED_PORT, OIDC_STR_COMMA); } const char *oidc_util_hdr_in_x_forwarded_host_get(const request_rec *r) { - return oidc_util_hdr_in_get_left_most_only(r, OIDC_HTTP_HDR_X_FORWARDED_HOST, OIDC_STR_COMMA); + return oidc_util_hdr_in_get_left_most_only(r, + OIDC_HTTP_HDR_X_FORWARDED_HOST, OIDC_STR_COMMA); } const char *oidc_util_hdr_in_host_get(const request_rec *r) {