0

Introduce a strong restriction to disallow redirects in Pepper

This patch will improve the interoperability on flash.
As Firefox has a restriction to disallow 307/308 POST redirects
for requests from the flash plugin, we will follow the same
approach, but will have a stronger restriction to allow only
GET and HEAD.

Bug: 332023
Change-Id: I17924315f1770c61b8cffca64f897e6be2e09cda
TEST: browser_tests --gtest_filter="*PPAPI*.URLLoader*"
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2026839
Reviewed-by: Raymes Khoury <raymes@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Cr-Commit-Position: refs/heads/master@{#739799}
This commit is contained in:
Takashi Toyoshima
2020-02-10 06:46:38 +00:00
committed by Commit Bot
parent 6d1511d16f
commit eab4ff72b7
6 changed files with 208 additions and 1 deletions

@ -11,6 +11,7 @@
#include "base/optional.h"
#include "base/path_service.h"
#include "base/stl_util.h"
#include "base/test/scoped_feature_list.h"
#include "base/test/test_timeouts.h"
#include "base/threading/thread_restrictions.h"
#include "build/build_config.h"
@ -33,6 +34,7 @@
#include "content/public/browser/network_service_instance.h"
#include "content/public/browser/storage_partition.h"
#include "content/public/browser/web_contents.h"
#include "content/public/common/content_features.h"
#include "content/public/common/content_switches.h"
#include "content/public/common/network_service_util.h"
#include "content/public/common/url_constants.h"
@ -1390,6 +1392,8 @@ TEST_PPAPI_NACL(HostResolverPrivate_ResolveIPv4)
LIST_TEST(URLLoader_UntrustedHttpRequests) \
LIST_TEST(URLLoader_FollowURLRedirect) \
LIST_TEST(URLLoader_AuditURLRedirect) \
LIST_TEST(URLLoader_RestrictURLRedirectCommon) \
LIST_TEST(URLLoader_RestrictURLRedirectEnabled) \
LIST_TEST(URLLoader_AbortCalls) \
LIST_TEST(URLLoader_UntendedLoad) \
LIST_TEST(URLLoader_PrefetchBufferThreshold) \
@ -1422,6 +1426,28 @@ IN_PROC_BROWSER_TEST_F(OutOfProcessPPAPITest, URLLoader3) {
IN_PROC_BROWSER_TEST_F(OutOfProcessPPAPITest, URLLoaderTrusted) {
RUN_URLLOADER_TRUSTED_SUBTESTS;
}
class OutOfProcessWithoutPepperCrossOriginRestrictionPPAPITest
: public OutOfProcessPPAPITest {
public:
OutOfProcessWithoutPepperCrossOriginRestrictionPPAPITest() {
scoped_feature_list_.InitAndDisableFeature(
features::kPepperCrossOriginRedirectRestriction);
}
private:
base::test::ScopedFeatureList scoped_feature_list_;
};
IN_PROC_BROWSER_TEST_F(OutOfProcessWithoutPepperCrossOriginRestrictionPPAPITest,
URLLoaderRestrictURLRedirectDisabled) {
// This test verifies if the restriction in the pepper_url_loader_host.cc
// can be managed via base::FeatureList, and does not need to run with various
// NaCl sandbox modes.
RunTestViaHTTP(LIST_TEST(URLLoader_RestrictURLRedirectCommon)
LIST_TEST(URLLoader_RestrictURLRedirectDisabled));
}
IN_PROC_BROWSER_TEST_F(PPAPINaClNewlibTest, MAYBE_PPAPI_NACL(URLLoader0)) {
RUN_URLLOADER_SUBTESTS_0;
}

@ -409,6 +409,10 @@ const base::Feature kPepper3DImageChromium {
#endif
};
// Kill-switch to introduce a compatibility breaking restriction.
const base::Feature kPepperCrossOriginRedirectRestriction{
"PepperCrossOriginRedirectRestriction", base::FEATURE_ENABLED_BY_DEFAULT};
// Whether we should composite a PLSA even if it means losing lcd text.
const base::Feature kPreferCompositingToLCDText = {
"PreferCompositingToLCDText", base::FEATURE_DISABLED_BY_DEFAULT};

@ -96,6 +96,7 @@ CONTENT_EXPORT extern const base::Feature kPassiveDocumentWheelEventListeners;
CONTENT_EXPORT extern const base::Feature kPassiveEventListenersDueToFling;
CONTENT_EXPORT extern const base::Feature kPeriodicBackgroundSync;
CONTENT_EXPORT extern const base::Feature kPepper3DImageChromium;
CONTENT_EXPORT extern const base::Feature kPepperCrossOriginRedirectRestriction;
CONTENT_EXPORT extern const base::Feature kPreferCompositingToLCDText;
CONTENT_EXPORT extern const base::Feature kPrioritizeBootstrapTasks;
CONTENT_EXPORT extern const base::Feature kProactivelySwapBrowsingInstance;

@ -6,11 +6,15 @@
#include <stddef.h>
#include "base/feature_list.h"
#include "base/strings/string_util.h"
#include "content/public/common/content_features.h"
#include "content/renderer/pepper/pepper_plugin_instance_impl.h"
#include "content/renderer/pepper/renderer_ppapi_host_impl.h"
#include "content/renderer/pepper/url_request_info_util.h"
#include "content/renderer/pepper/url_response_info_util.h"
#include "net/base/net_errors.h"
#include "net/http/http_request_headers.h"
#include "ppapi/c/pp_errors.h"
#include "ppapi/host/dispatch_host_message.h"
#include "ppapi/host/host_message_context.h"
@ -124,6 +128,24 @@ bool PepperURLLoaderHost::WillFollowRedirect(
const WebURL& new_url,
const WebURLResponse& redirect_response) {
DCHECK(out_of_order_replies_.empty());
if (base::FeatureList::IsEnabled(
features::kPepperCrossOriginRedirectRestriction)) {
// Follows the Firefox approach
// (https://bugzilla.mozilla.org/show_bug.cgi?id=1436241) to disallow
// cross-origin 307/308 POST redirects for requests from plugins. But we try
// allowing only GET and HEAD methods rather than disallowing POST.
// See http://crbug.com/332023 for details.
int status = redirect_response.HttpStatusCode();
if ((status == 307 || status == 308)) {
std::string method = base::ToUpperASCII(request_data_.method);
// method can be an empty string for default behavior, GET.
if (!method.empty() && method != net::HttpRequestHeaders::kGetMethod &&
method != net::HttpRequestHeaders::kHeadMethod) {
return false;
}
}
}
if (!request_data_.follow_redirects) {
SaveResponse(redirect_response);
SetDefersLoading(true);

@ -99,7 +99,7 @@ bool TestURLLoader::Init() {
*
* Here is the environment:
*
* 1. TestServer.py only accepts one open connection at the time.
* 1. net::EmbeddedTestServer only accepts one open connection at the time.
* 2. HTTP socket pool keeps sockets open for several seconds after last use
* (hoping that there will be another request that could reuse the connection).
* 3. HTTP socket pool is separated by host/port and privacy mode (which is
@ -140,6 +140,9 @@ void TestURLLoader::RunTests(const std::string& filter) {
RUN_CALLBACK_TEST(TestURLLoader, TrustedHttpRequests, filter);
RUN_CALLBACK_TEST(TestURLLoader, FollowURLRedirect, filter);
RUN_CALLBACK_TEST(TestURLLoader, AuditURLRedirect, filter);
RUN_CALLBACK_TEST(TestURLLoader, RestrictURLRedirectCommon, filter);
RUN_CALLBACK_TEST(TestURLLoader, RestrictURLRedirectEnabled, filter);
RUN_CALLBACK_TEST(TestURLLoader, RestrictURLRedirectDisabled, filter);
RUN_CALLBACK_TEST(TestURLLoader, AbortCalls, filter);
RUN_CALLBACK_TEST(TestURLLoader, UntendedLoad, filter);
RUN_CALLBACK_TEST(TestURLLoader, PrefetchBufferThreshold, filter);
@ -222,6 +225,17 @@ std::string TestURLLoader::LoadAndCompareBody(
PASS();
}
std::string TestURLLoader::LoadAndFail(const pp::URLRequestInfo& request) {
TestCompletionCallback callback(instance_->pp_instance(), callback_type());
pp::URLLoader loader(instance_);
callback.WaitForResult(loader.Open(request, callback.GetCallback()));
CHECK_CALLBACK_BEHAVIOR(callback);
ASSERT_EQ(PP_ERROR_FAILED, callback.result());
PASS();
}
int32_t TestURLLoader::OpenFileSystem(pp::FileSystem* file_system,
std::string* message) {
TestCompletionCallback callback(instance_->pp_instance(), callback_type());
@ -784,6 +798,142 @@ std::string TestURLLoader::TestAuditURLRedirect() {
PASS();
}
// This test checks if the redirect restriction does not block acceptable cases
// of 307/308 GET and HEAD.
std::string TestURLLoader::TestRestrictURLRedirectCommon() {
std::string url = GetReachableAbsoluteURL("test_url_loader_data/hello.txt");
std::string redirect_307_prefix("/server-redirect-307?");
{
// Default method is GET and will follow the redirect.
pp::URLRequestInfo request_for_default_307(instance_);
request_for_default_307.SetURL(redirect_307_prefix.append(url));
std::string result_for_default_307 =
LoadAndCompareBody(request_for_default_307, "hello\n");
if (!result_for_default_307.empty())
return result_for_default_307;
}
{
// GET will follow the redirect.
pp::URLRequestInfo request_for_get_307(instance_);
request_for_get_307.SetURL(redirect_307_prefix.append(url));
request_for_get_307.SetMethod("GET");
std::string result_for_get_307 =
LoadAndCompareBody(request_for_get_307, "hello\n");
if (!result_for_get_307.empty())
return result_for_get_307;
}
{
// HEAD will follow the redirect.
pp::URLRequestInfo request_for_head_307(instance_);
request_for_head_307.SetURL(redirect_307_prefix.append(url));
request_for_head_307.SetMethod("HEAD");
std::string result_for_head_307 =
LoadAndCompareBody(request_for_head_307, "");
if (!result_for_head_307.empty())
return result_for_head_307;
}
std::string redirect_308_prefix("/server-redirect-308?");
{
// Default method is GET and will follow the redirect.
pp::URLRequestInfo request_for_default_308(instance_);
request_for_default_308.SetURL(redirect_308_prefix.append(url));
std::string result_for_default_308 =
LoadAndCompareBody(request_for_default_308, "hello\n");
if (!result_for_default_308.empty())
return result_for_default_308;
}
{
// GET will follow the redirect.
pp::URLRequestInfo request_for_get_308(instance_);
request_for_get_308.SetURL(redirect_308_prefix.append(url));
request_for_get_308.SetMethod("GET");
std::string result_for_get_308 =
LoadAndCompareBody(request_for_get_308, "hello\n");
if (!result_for_get_308.empty())
return result_for_get_308;
}
{
// HEAD will follow the redirect.
pp::URLRequestInfo request_for_head_308(instance_);
request_for_head_308.SetURL(redirect_308_prefix.append(url));
request_for_head_308.SetMethod("HEAD");
std::string result_for_head_308 =
LoadAndCompareBody(request_for_head_308, "");
if (!result_for_head_308.empty())
return result_for_head_308;
}
PASS();
}
// This test checks if the redirect restriction blocks the restricted cases of
// 307/308 POST.
std::string TestURLLoader::TestRestrictURLRedirectEnabled() {
std::string url = GetReachableAbsoluteURL("test_url_loader_data/hello.txt");
{
// POST will be blocked and fail.
std::string redirect_307_prefix("/server-redirect-307?");
pp::URLRequestInfo request_for_post_307(instance_);
request_for_post_307.SetURL(redirect_307_prefix.append(url));
request_for_post_307.SetMethod("POST");
std::string result_for_post_307 = LoadAndFail(request_for_post_307);
if (!result_for_post_307.empty())
return result_for_post_307;
}
{
// POST will be blocked and fail.
pp::URLRequestInfo request_for_post_308(instance_);
std::string redirect_308_prefix("/server-redirect-308?");
request_for_post_308.SetURL(redirect_308_prefix.append(url));
request_for_post_308.SetMethod("POST");
std::string result_for_post_308 = LoadAndFail(request_for_post_308);
if (!result_for_post_308.empty())
return result_for_post_308;
}
PASS();
}
// This test checks if the redirect restriction does not block the restricted
// cases if the restriction is disabled.
std::string TestURLLoader::TestRestrictURLRedirectDisabled() {
std::string url = GetReachableAbsoluteURL("test_url_loader_data/hello.txt");
{
// POST will not be blocked, but follow the redirect.
std::string redirect_307_prefix("/server-redirect-307?");
pp::URLRequestInfo request_for_post_307(instance_);
request_for_post_307.SetURL(redirect_307_prefix.append(url));
request_for_post_307.SetMethod("POST");
std::string result_for_post_307 =
LoadAndCompareBody(request_for_post_307, "hello\n");
if (!result_for_post_307.empty())
return result_for_post_307;
}
{
// POST will not be blocked, but follow the redirect.
pp::URLRequestInfo request_for_post_308(instance_);
std::string redirect_308_prefix("/server-redirect-308?");
request_for_post_308.SetURL(redirect_308_prefix.append(url));
request_for_post_308.SetMethod("POST");
std::string result_for_post_308 =
LoadAndCompareBody(request_for_post_308, "hello\n");
if (!result_for_post_308.empty())
return result_for_post_308;
}
PASS();
}
std::string TestURLLoader::TestAbortCalls() {
pp::URLRequestInfo request(instance_);
request.SetURL("test_url_loader_data/hello.txt");

@ -35,6 +35,7 @@ class TestURLLoader : public TestCase {
std::string* body);
std::string LoadAndCompareBody(const pp::URLRequestInfo& request,
const std::string& expected_body);
std::string LoadAndFail(const pp::URLRequestInfo& request);
int32_t OpenFileSystem(pp::FileSystem* file_system, std::string* message);
int32_t PrepareFileForPost(const pp::FileRef& file_ref,
const std::string& data,
@ -76,6 +77,9 @@ class TestURLLoader : public TestCase {
std::string TestTrustedHttpRequests();
std::string TestFollowURLRedirect();
std::string TestAuditURLRedirect();
std::string TestRestrictURLRedirectCommon();
std::string TestRestrictURLRedirectEnabled();
std::string TestRestrictURLRedirectDisabled();
std::string TestAbortCalls();
std::string TestUntendedLoad();
std::string TestPrefetchBufferThreshold();