From c0562f9c8359930797c6269f03b0db5d09e3547b Mon Sep 17 00:00:00 2001 From: Vita Batrla <34657903+batrla@users.noreply.github.com> Date: Mon, 17 Jan 2022 09:48:25 +0100 Subject: [PATCH] Logout endpoint should handle idP POST response (#84) When mod_auth_mellon sends logout request to the idP, it gets response back through HTTP GET request. That works fine, However, some idP implementations send the logout response through POST request. Logout response via POST is not understood by mod_auth_mellon and leads to 404 page. The mod_auth_mellon assumes that POST method in logout endpoint is always SOAP logout request. The suggested fix adds a check and POST handler for logout responses. Co-authored-by: Vita Batrla --- auth_mellon_handler.c | 60 +++++++++++++++++++++++++++++++++++++------ 1 file changed, 52 insertions(+), 8 deletions(-) diff --git a/auth_mellon_handler.c b/auth_mellon_handler.c index eaa0dca..7c62740 100644 --- a/auth_mellon_handler.c +++ b/auth_mellon_handler.c @@ -896,11 +896,15 @@ static int am_handle_invalidate_request(request_rec *r) * request_rec *r The logout response request. * LassoLogout *logout A LassoLogout object initiated with * the current session. + * input in POST case: url-decoded SAMLResponse parameter, + * in GET case: url-encoded query string. + * args Url-encoded parameters. * * Returns: * OK on success, or an error if any of the steps fail. */ -static int am_handle_logout_response(request_rec *r, LassoLogout *logout) +static int am_handle_logout_response_cmn(request_rec *r, LassoLogout *logout, + char *input, char *args) { gint res; int rc; @@ -908,7 +912,7 @@ static int am_handle_logout_response(request_rec *r, LassoLogout *logout) char *return_to; am_dir_cfg_rec *cfg = am_get_dir_cfg(r); - res = lasso_logout_process_response_msg(logout, r->args); + res = lasso_logout_process_response_msg(logout, input); am_diag_log_lasso_node(r, 0, LASSO_PROFILE(logout)->response, "SAML Response (%s):", __func__); #ifdef HAVE_lasso_profile_set_signature_verify_hint @@ -919,7 +923,7 @@ static int am_handle_logout_response(request_rec *r, LassoLogout *logout) APR_HASH_KEY_STRING)) { lasso_profile_set_signature_verify_hint(&logout->parent, LASSO_PROFILE_SIGNATURE_VERIFY_HINT_IGNORE); - res = lasso_logout_process_response_msg(logout, r->args); + res = lasso_logout_process_response_msg(logout, input); } } #endif @@ -946,7 +950,7 @@ static int am_handle_logout_response(request_rec *r, LassoLogout *logout) am_delete_request_session(r, session); } - return_to = am_extract_query_parameter(r->pool, r->args, "RelayState"); + return_to = am_extract_query_parameter(r->pool, args, "RelayState"); if(return_to == NULL) { AM_LOG_RERROR(APLOG_MARK, APLOG_ERR, 0, r, "No RelayState parameter to logout response handler." @@ -981,6 +985,35 @@ static int am_handle_logout_response(request_rec *r, LassoLogout *logout) return HTTP_SEE_OTHER; } +static int am_handle_logout_response_GET(request_rec *r, LassoLogout *logout) +{ + return am_handle_logout_response_cmn(r, logout, r->args, r->args); +} + +static int am_handle_logout_response_POST(request_rec *r, LassoLogout *logout, + char *post_data) +{ + int rc = 0; + char *saml_response; + + saml_response = am_extract_query_parameter(r->pool, post_data, + "SAMLResponse"); + + if (saml_response == NULL) { + AM_LOG_RERROR(APLOG_MARK, APLOG_ERR, rc, r, + "Could not find SAMLResponse field in POST data."); + return HTTP_BAD_REQUEST; + } + + rc = am_urldecode(saml_response); + if (rc != OK) { + AM_LOG_RERROR(APLOG_MARK, APLOG_ERR, rc, r, + "Could not urldecode SAMLResponse value."); + return rc; + } + return am_handle_logout_response_cmn(r, logout, saml_response, + post_data); +} /* This function initiates a logout request and sends it to the IdP. * @@ -1175,14 +1208,21 @@ static int am_handle_logout(request_rec *r) * - logout requests: The IdP sends a logout request to this service. * it can be either through HTTP-Redirect or SOAP. * - logout responses: We have sent a logout request to the IdP, and - * are receiving a response. + * are receiving a response either via GET or POST. * - We want to initiate a logout request. */ - /* First check for IdP-initiated SOAP logout request */ + /* + * First check for POST method, which could be either IdP-initiated SOAP + * logout request or POST logout response from IdP. + */ if ((r->args == NULL) && (r->method_number == M_POST)) { int rc; char *post_data; + const char *content_type; + + /* Check Content-Type */ + content_type = apr_table_get(r->headers_in, "Content-Type"); rc = am_read_post_data(r, &post_data, NULL); if (rc != OK) { @@ -1190,8 +1230,12 @@ static int am_handle_logout(request_rec *r) "Error reading POST data."); return HTTP_INTERNAL_SERVER_ERROR; } - return am_handle_logout_request(r, logout, post_data); + if (content_type != NULL && + am_has_header(r, content_type, "application/x-www-form-urlencoded")) + return am_handle_logout_response_POST(r, logout, post_data); + else + return am_handle_logout_request(r, logout, post_data); } else if(am_extract_query_parameter(r->pool, r->args, "SAMLRequest") != NULL) { /* SAMLRequest - logout request from the IdP. */ @@ -1200,7 +1244,7 @@ static int am_handle_logout(request_rec *r) } else if(am_extract_query_parameter(r->pool, r->args, "SAMLResponse") != NULL) { /* SAMLResponse - logout response from the IdP. */ - return am_handle_logout_response(r, logout); + return am_handle_logout_response_GET(r, logout); } else if(am_extract_query_parameter(r->pool, r->args, "ReturnTo") != NULL) {