Fix incorrect Origin and Cookie headers for POST redirects (fixes issue #2806)

This change also adds unit test coverage for cross-origin POST redirects.
This commit is contained in:
Marshall Greenblatt 2020-09-09 15:59:33 -04:00
parent 42f517ec69
commit 88faf1023a
2 changed files with 221 additions and 45 deletions

View File

@ -20,6 +20,7 @@
#include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents.h"
#include "mojo/public/cpp/base/big_buffer.h" #include "mojo/public/cpp/base/big_buffer.h"
#include "net/http/http_status_code.h" #include "net/http/http_status_code.h"
#include "net/url_request/redirect_util.h"
#include "net/url_request/url_request.h" #include "net/url_request/url_request.h"
#include "services/network/public/cpp/cors/cors.h" #include "services/network/public/cpp/cors/cors.h"
#include "services/network/public/cpp/features.h" #include "services/network/public/cpp/features.h"
@ -884,27 +885,24 @@ void InterceptedRequest::ContinueToBeforeRedirect(
if (proxied_client_binding_) if (proxied_client_binding_)
proxied_client_binding_.ResumeIncomingMethodCallProcessing(); proxied_client_binding_.ResumeIncomingMethodCallProcessing();
const auto original_url = request_.url;
const auto original_method = request_.method;
net::RedirectInfo new_redirect_info = redirect_info;
if (redirect_url.is_valid()) { if (redirect_url.is_valid()) {
net::RedirectInfo new_redirect_info = redirect_info;
new_redirect_info.new_url = redirect_url; new_redirect_info.new_url = redirect_url;
target_client_->OnReceiveRedirect(new_redirect_info, new_redirect_info.new_site_for_cookies =
std::move(current_response_)); net::SiteForCookies::FromUrl(redirect_url);
request_.url = redirect_url;
} else {
target_client_->OnReceiveRedirect(redirect_info,
std::move(current_response_));
request_.url = redirect_info.new_url;
} }
// If request_ changes from POST to GET, strip POST headers. target_client_->OnReceiveRedirect(new_redirect_info,
const bool post_to_get = std::move(current_response_));
request_.method == "POST" &&
redirect_info.new_method == net::HttpRequestHeaders::kGetMethod;
request_.method = redirect_info.new_method; request_.url = new_redirect_info.new_url;
request_.site_for_cookies = redirect_info.new_site_for_cookies; request_.method = new_redirect_info.new_method;
request_.referrer = GURL(redirect_info.new_referrer); request_.site_for_cookies = new_redirect_info.new_site_for_cookies;
request_.referrer_policy = redirect_info.new_referrer_policy; request_.referrer = GURL(new_redirect_info.new_referrer);
request_.referrer_policy = new_redirect_info.new_referrer_policy;
if (request_.trusted_params) { if (request_.trusted_params) {
request_.trusted_params->isolation_info = request_.trusted_params->isolation_info =
@ -912,15 +910,21 @@ void InterceptedRequest::ContinueToBeforeRedirect(
url::Origin::Create(request_.url)); url::Origin::Create(request_.url));
} }
// The request method can be changed to "GET". In this case we need to // Remove existing Cookie headers. They may be re-added after Restart().
// reset the request body manually, and strip the POST headers. const std::vector<std::string> remove_headers{
if (request_.method == net::HttpRequestHeaders::kGetMethod) { net::HttpRequestHeaders::kCookie};
request_.request_body = nullptr;
if (post_to_get) { // Use common logic for sanitizing request headers including Origin and
request_.headers.RemoveHeader(net::HttpRequestHeaders::kContentLength); // Content-*.
request_.headers.RemoveHeader(net::HttpRequestHeaders::kContentType); bool should_clear_upload;
} net::RedirectUtil::UpdateHttpRequest(original_url, original_method,
new_redirect_info,
base::make_optional(remove_headers),
/*modified_headers=*/base::nullopt,
&request_.headers, &should_clear_upload);
if (should_clear_upload) {
request_.request_body = nullptr;
} }
} }

View File

@ -1101,7 +1101,7 @@ CORS_TEST_FETCH_ALL(WithHeader, true)
namespace { namespace {
enum class RedirectGetMode { enum class RedirectMode {
MODE_302, MODE_302,
MODE_307, MODE_307,
}; };
@ -1120,12 +1120,12 @@ struct RedirectGetResource : CookieResource {
} }
}; };
void SetupRedirectGetResponse(RedirectGetMode mode, void SetupRedirectResponse(RedirectMode mode,
const std::string& redirect_url, const std::string& redirect_url,
CefRefPtr<CefResponse> response) { CefRefPtr<CefResponse> response) {
if (mode == RedirectGetMode::MODE_302) if (mode == RedirectMode::MODE_302)
response->SetStatus(302); response->SetStatus(302);
else if (mode == RedirectGetMode::MODE_307) else if (mode == RedirectMode::MODE_307)
response->SetStatus(307); response->SetStatus(307);
else else
NOTREACHED(); NOTREACHED();
@ -1135,7 +1135,7 @@ void SetupRedirectGetResponse(RedirectGetMode mode,
} }
// Test redirect requests. // Test redirect requests.
void SetupRedirectGetRequest(RedirectGetMode mode, void SetupRedirectGetRequest(RedirectMode mode,
CookieTestSetup* setup, CookieTestSetup* setup,
const std::string& test_name, const std::string& test_name,
HandlerType main_handler, HandlerType main_handler,
@ -1152,7 +1152,7 @@ void SetupRedirectGetRequest(RedirectGetMode mode,
// Expect a single main request that results in a redirect. // Expect a single main request that results in a redirect.
const std::string& redirect_url = redirect_resource->GetPathURL(); const std::string& redirect_url = redirect_resource->GetPathURL();
main_resource->Init(main_handler, base_path, kMimeTypeHtml, std::string()); main_resource->Init(main_handler, base_path, kMimeTypeHtml, std::string());
SetupRedirectGetResponse(mode, redirect_url, main_resource->response); SetupRedirectResponse(mode, redirect_url, main_resource->response);
SetupCookieExpectations(setup, main_resource, redirect_resource); SetupCookieExpectations(setup, main_resource, redirect_resource);
@ -1163,19 +1163,19 @@ void SetupRedirectGetRequest(RedirectGetMode mode,
} // namespace } // namespace
// Test redirect GET requests with different origin combinations. // Test redirect GET requests with different origin combinations.
#define CORS_TEST_REDIRECT_GET(test_name, mode, handler_main, \ #define CORS_TEST_REDIRECT_GET(test_name, mode, handler_main, \
handler_redirect) \ handler_redirect) \
TEST(CorsTest, RedirectGet##test_name) { \ TEST(CorsTest, RedirectGet##test_name) { \
CookieTestSetup setup; \ CookieTestSetup setup; \
CookieResource resource_main; \ CookieResource resource_main; \
RedirectGetResource resource_redirect; \ RedirectGetResource resource_redirect; \
SetupRedirectGetRequest( \ SetupRedirectGetRequest( \
RedirectGetMode::mode, &setup, "CorsTest.RedirectGet" #test_name, \ RedirectMode::mode, &setup, "CorsTest.RedirectGet" #test_name, \
HandlerType::handler_main, &resource_main, \ HandlerType::handler_main, &resource_main, \
HandlerType::handler_redirect, &resource_redirect); \ HandlerType::handler_redirect, &resource_redirect); \
CefRefPtr<CorsTestHandler> handler = new CorsTestHandler(&setup); \ CefRefPtr<CorsTestHandler> handler = new CorsTestHandler(&setup); \
handler->ExecuteTest(); \ handler->ExecuteTest(); \
ReleaseAndWaitForDestructor(handler); \ ReleaseAndWaitForDestructor(handler); \
} }
// Test all redirect GET combinations (same and cross-origin). // Test all redirect GET combinations (same and cross-origin).
@ -1216,3 +1216,175 @@ void SetupRedirectGetRequest(RedirectGetMode mode,
// Redirect GET requests. // Redirect GET requests.
CORS_TEST_REDIRECT_GET_ALL(302, MODE_302) CORS_TEST_REDIRECT_GET_ALL(302, MODE_302)
CORS_TEST_REDIRECT_GET_ALL(307, MODE_307) CORS_TEST_REDIRECT_GET_ALL(307, MODE_307)
namespace {
struct PostResource : CookieResource {
PostResource() {}
bool expect_downgrade_to_get = false;
bool was_redirected = false;
std::string main_origin;
bool is_cross_origin;
void InitOrigin(HandlerType main_handler) {
// Origin is always "null" for non-HTTP(S) schemes.
// This should only be "null" for non-standard schemes, but Blink is likely
// using SchemeIsHTTPOrHTTPS() when submitting the form request.
main_origin = main_handler == HandlerType::CUSTOM_NONSTANDARD_SCHEME ||
main_handler == HandlerType::CUSTOM_STANDARD_SCHEME
? "null"
: GetOrigin(main_handler);
// True if the request is considered cross-origin. Any requests between
// non-standard schemes are considered cross-origin (due to the "null"
// origin).
is_cross_origin = main_handler != handler ||
(main_handler == HandlerType::CUSTOM_NONSTANDARD_SCHEME &&
handler == main_handler);
}
bool VerifyRequest(CefRefPtr<CefRequest> request) const override {
if (!CookieResource::VerifyRequest(request))
return false;
// The "Origin" header should be present if the request is POST, and was not
// redirected cross-origin.
std::string expected_origin;
if (!expect_downgrade_to_get) {
if (was_redirected && is_cross_origin) {
// Always "null" for cross-origin redirects.
expected_origin = "null";
} else {
expected_origin = main_origin;
}
}
const std::string& origin = request->GetHeaderByName("Origin");
EXPECT_STREQ(expected_origin.c_str(), origin.c_str()) << GetPathURL();
if (expected_origin != origin)
return false;
const std::string& method = request->GetMethod();
const bool has_post_data = request->GetPostData() != nullptr;
if (expect_downgrade_to_get) {
EXPECT_FALSE(has_post_data) << GetPathURL();
EXPECT_STREQ("GET", method.c_str()) << GetPathURL();
return !has_post_data && method == "GET";
} else {
EXPECT_TRUE(has_post_data) << GetPathURL();
EXPECT_STREQ("POST", method.c_str()) << GetPathURL();
return has_post_data && method == "POST";
}
}
};
std::string GetPostFormHtml(const std::string& submit_url) {
return "<html><body>"
"<form id=\"f\" action=\"" +
submit_url +
"\" method=\"post\">"
"<input type=\"hidden\" name=\"n\" value=\"v\"></form>"
"<script>document.getElementById('f').submit();</script>"
"</body></html>";
}
// Test redirect requests.
void SetupRedirectPostRequest(RedirectMode mode,
CookieTestSetup* setup,
const std::string& test_name,
HandlerType main_handler,
CookieResource* main_resource,
PostResource* submit_resource,
HandlerType redirect_handler,
PostResource* redirect_resource) {
const std::string& base_path = "/" + test_name;
// Expect a single redirect request that sends SuccessMsg.
redirect_resource->Init(redirect_handler, base_path + ".redirect.html",
kMimeTypeHtml, GetDefaultSuccessMsgHtml());
redirect_resource->InitOrigin(main_handler);
redirect_resource->expected_success_query_ct = 1;
// 302 redirects will downgrade POST requests to GET.
redirect_resource->expect_downgrade_to_get = mode == RedirectMode::MODE_302;
redirect_resource->was_redirected = true;
// Expect a single submit request that redirects the response.
const std::string& redirect_url = redirect_resource->GetPathURL();
submit_resource->Init(main_handler, base_path + ".submit.html", kMimeTypeHtml,
std::string());
submit_resource->InitOrigin(main_handler);
SetupRedirectResponse(mode, redirect_url, submit_resource->response);
// Expect a single main request that submits the form.
const std::string& submit_url = submit_resource->GetPathURL();
main_resource->Init(main_handler, base_path, kMimeTypeHtml,
GetPostFormHtml(submit_url));
SetupCookieExpectations(setup, main_resource, submit_resource);
SetupCookieExpectations(setup, main_resource, redirect_resource);
setup->AddResource(main_resource);
setup->AddResource(submit_resource);
setup->AddResource(redirect_resource);
}
} // namespace
// Test redirect GET requests with different origin combinations.
#define CORS_TEST_REDIRECT_POST(test_name, mode, handler_main, \
handler_redirect) \
TEST(CorsTest, RedirectPost##test_name) { \
CookieTestSetup setup; \
CookieResource resource_main; \
PostResource resource_submit, resource_redirect; \
SetupRedirectPostRequest( \
RedirectMode::mode, &setup, "CorsTest.RedirectPost" #test_name, \
HandlerType::handler_main, &resource_main, &resource_submit, \
HandlerType::handler_redirect, &resource_redirect); \
CefRefPtr<CorsTestHandler> handler = new CorsTestHandler(&setup); \
handler->ExecuteTest(); \
ReleaseAndWaitForDestructor(handler); \
}
// Test all redirect GET combinations (same and cross-origin).
#define CORS_TEST_REDIRECT_POST_ALL(name, mode) \
CORS_TEST_REDIRECT_POST(name##ServerToServer, mode, SERVER, SERVER) \
CORS_TEST_REDIRECT_POST(name##ServerToHttpScheme, mode, SERVER, HTTP_SCHEME) \
CORS_TEST_REDIRECT_POST(name##ServerToCustomStandardScheme, mode, SERVER, \
CUSTOM_STANDARD_SCHEME) \
CORS_TEST_REDIRECT_POST(name##ServerToCustomNonStandardScheme, mode, SERVER, \
CUSTOM_NONSTANDARD_SCHEME) \
CORS_TEST_REDIRECT_POST(name##HttpSchemeToServer, mode, HTTP_SCHEME, SERVER) \
CORS_TEST_REDIRECT_POST(name##HttpSchemeToHttpScheme, mode, HTTP_SCHEME, \
HTTP_SCHEME) \
CORS_TEST_REDIRECT_POST(name##HttpSchemeToCustomStandardScheme, mode, \
HTTP_SCHEME, CUSTOM_STANDARD_SCHEME) \
CORS_TEST_REDIRECT_POST(name##HttpSchemeToCustomNonStandardScheme, mode, \
HTTP_SCHEME, CUSTOM_NONSTANDARD_SCHEME) \
CORS_TEST_REDIRECT_POST(name##CustomStandardSchemeToServer, mode, \
CUSTOM_STANDARD_SCHEME, SERVER) \
CORS_TEST_REDIRECT_POST(name##CustomStandardSchemeToHttpScheme, mode, \
CUSTOM_STANDARD_SCHEME, HTTP_SCHEME) \
CORS_TEST_REDIRECT_POST(name##CustomStandardSchemeToCustomStandardScheme, \
mode, CUSTOM_STANDARD_SCHEME, \
CUSTOM_STANDARD_SCHEME) \
CORS_TEST_REDIRECT_POST(name##CustomStandardSchemeToCustomNonStandardScheme, \
mode, CUSTOM_STANDARD_SCHEME, \
CUSTOM_NONSTANDARD_SCHEME) \
CORS_TEST_REDIRECT_POST(name##CustomNonStandardSchemeToServer, mode, \
CUSTOM_NONSTANDARD_SCHEME, SERVER) \
CORS_TEST_REDIRECT_POST(name##CustomNonStandardSchemeToHttpScheme, mode, \
CUSTOM_NONSTANDARD_SCHEME, HTTP_SCHEME) \
CORS_TEST_REDIRECT_POST(name##CustomNonStandardSchemeToCustomStandardScheme, \
mode, CUSTOM_NONSTANDARD_SCHEME, \
CUSTOM_STANDARD_SCHEME) \
CORS_TEST_REDIRECT_POST( \
name##CustomNonStandardSchemeToCustomNonStandardScheme, mode, \
CUSTOM_NONSTANDARD_SCHEME, CUSTOM_NONSTANDARD_SCHEME)
// Redirect GET requests.
CORS_TEST_REDIRECT_POST_ALL(302, MODE_302)
CORS_TEST_REDIRECT_POST_ALL(307, MODE_307)