Commit 3deab63e authored by Matt Menke's avatar Matt Menke Committed by Commit Bot

Fix SSLPKPBrowserTest.SendPKPReport/SendPKPReportServerHangs.

In https://chromium-review.googlesource.com/c/chromium/src/+/1794305,
I deleted the in-process NetworkService path, assuming that the
NetworkServiceTest interface would work with an in-process
NetworkService, and that we weren't running in that configuration
anyways. I was wrong on both counts.

This CL brings back the old behavior, adding a comment that it's not
threadsafe, since it's running code on the IO thread, which should
be run on the NetworkService's thread.

Bug: 1007610
Change-Id: I5ccb029d0c3590d42fa22a35be019eea767ffc8b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1825565
Commit-Queue: Matt Menke <mmenke@chromium.org>
Reviewed-by: default avatarEmily Stark <estark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#700877}
parent d0bc1e37
......@@ -125,6 +125,7 @@
#include "content/public/browser/web_contents.h"
#include "content/public/browser/web_contents_observer.h"
#include "content/public/common/content_switches.h"
#include "content/public/common/network_service_util.h"
#include "content/public/common/page_state.h"
#include "content/public/common/service_names.mojom.h"
#include "content/public/common/url_constants.h"
......@@ -5768,6 +5769,9 @@ class SSLUITestNoCert : public SSLUITest,
// Checks that a newly-added certificate authority is usable immediately.
IN_PROC_BROWSER_TEST_F(SSLUITestNoCert, NewCertificateAuthority) {
if (!content::IsOutOfProcessNetworkService())
return;
ASSERT_TRUE(https_server_.Start());
WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents();
......@@ -7552,12 +7556,17 @@ class SSLPKPBrowserTest : public CertVerifierBrowserTest {
}
void TearDownOnMainThread() override {
mojo::ScopedAllowSyncCallForTesting allow_sync_call;
if (content::IsOutOfProcessNetworkService()) {
mojo::ScopedAllowSyncCallForTesting allow_sync_call;
network::mojom::NetworkServiceTestPtr network_service_test;
content::GetSystemConnector()->BindInterface(
content::mojom::kNetworkServiceName, &network_service_test);
network_service_test->SetTransportSecurityStateSource(0);
network::mojom::NetworkServiceTestPtr network_service_test;
content::GetSystemConnector()->BindInterface(
content::mojom::kNetworkServiceName, &network_service_test);
network_service_test->SetTransportSecurityStateSource(0);
} else {
RunOnIOThreadBlocking(base::BindOnce(
&SSLPKPBrowserTest::CleanUpOnIOThread, base::Unretained(this)));
}
CertVerifierBrowserTest::TearDownOnMainThread();
}
......@@ -7567,11 +7576,22 @@ class SSLPKPBrowserTest : public CertVerifierBrowserTest {
content::BrowserContext::GetDefaultStoragePartition(
browser()->profile());
partition->GetNetworkContext()->EnableStaticKeyPinningForTesting();
partition->FlushNetworkInterfaceForTesting();
network::mojom::NetworkServiceTestPtr network_service_test;
content::GetSystemConnector()->BindInterface(
content::mojom::kNetworkServiceName, &network_service_test);
network_service_test->SetTransportSecurityStateSource(reporting_port);
if (content::IsOutOfProcessNetworkService()) {
network::mojom::NetworkServiceTestPtr network_service_test;
content::GetSystemConnector()->BindInterface(
content::mojom::kNetworkServiceName, &network_service_test);
network_service_test->SetTransportSecurityStateSource(reporting_port);
} else {
// TODO(https://crbug.com/1008175): This code is not threadsafe, as the
// network stack does not run on the IO thread. Ideally, the
// NetworkServiceTest object would be set up in-process on the network
// service's thread, and this path would be removed.
RunOnIOThreadBlocking(base::BindOnce(
&SSLPKPBrowserTest::SetTransportSecurityStateSourceOnIO,
base::Unretained(this), reporting_port));
}
}
private:
......@@ -7581,6 +7601,18 @@ class SSLPKPBrowserTest : public CertVerifierBrowserTest {
std::move(task), run_loop.QuitClosure());
run_loop.Run();
}
void SetTransportSecurityStateSourceOnIO(int reporting_port) {
transport_security_state_source_ =
std::make_unique<net::ScopedTransportSecurityStateSource>(
reporting_port);
}
void CleanUpOnIOThread() { transport_security_state_source_.reset(); }
// Only used when NetworkService is disabled. Accessed on IO thread.
std::unique_ptr<net::ScopedTransportSecurityStateSource>
transport_security_state_source_;
};
// Test case where a PKP report is sent.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment