Skip to content

Commit

Permalink
Introduce the ability to detect/block known interception certs
Browse files Browse the repository at this point in the history
This introduces a new net::Error and two net::CertStatuses that
can be used to signal a known interception certificate is either
detected or blocked (mutually exclusive). The blocking is
implemented via CRLSet, and can thus be provisioned both
in-binary and out-of-band (where dynamic CRLSet updates are
supported).

ERR_CERT_KNOWN_INTERCEPTION_BLOCKED will be set when a
known interception cert has been actively blocked. In addition,
the CertStatuses CERT_STATUS_REVOKED and
CERT_STATUS_KNOWN_INTERCEPTION_BLOCKED are set.

CERT_STATUS_KNOWN_INTERCEPTION_DETECTED will be set when
a known interception cert is detected in the verified chain. It
is a non-error status.

In this revision, if a known interception root is blocked, HSTS
does not cause the error to be fatal/non-overridable. This is
because the error is generated client side, not server-side, and
in the case of Known MITM, there's ambiguous signal of user
intent. Normally, locally-installed anchors would not be blocked
at all (thus not cause any HSTS errors), and this preserves that
element of local-device policy taking priority over the remote
server's preferences.

Because it's implemented via CRLSets, blocking certificates only
works on platforms that integrate CRLSets as part of path
building/verification. However, notification is implemented on
all platforms, as that runs after a certificate path has been
constructed.

Bug: 1014704
Change-Id: I7d20ae4366f04b02fc709a7d2c5e012bda4d0080
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1904545
Commit-Queue: Ryan Sleevi <rsleevi@chromium.org>
Reviewed-by: Richard Coles <torne@chromium.org>
Reviewed-by: Matt Mueller <mattm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#717407}
  • Loading branch information
sleevi authored and Commit Bot committed Nov 21, 2019
1 parent b7676d6 commit 54fe766
Show file tree
Hide file tree
Showing 34 changed files with 718 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ static int convertErrorCode(@NetError int netError) {

// The certificate errors are handled by onReceivedSslError
// and don't need to be reported here.
case NetError.ERR_CERT_KNOWN_INTERCEPTION_BLOCKED:
case NetError.ERR_CERT_COMMON_NAME_INVALID:
case NetError.ERR_CERT_DATE_INVALID:
case NetError.ERR_CERT_AUTHORITY_INVALID:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ public static SslError sslErrorFromNetErrorCode(
return new SslError(SslError.SSL_IDMISMATCH, cert, url);
case NetError.ERR_CERT_DATE_INVALID:
return new SslError(SslError.SSL_DATE_INVALID, cert, url);
case NetError.ERR_CERT_KNOWN_INTERCEPTION_BLOCKED:
case NetError.ERR_CERT_AUTHORITY_INVALID:
return new SslError(SslError.SSL_UNTRUSTED, cert, url);
default:
Expand Down
179 changes: 178 additions & 1 deletion chrome/browser/ssl/ssl_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include "base/test/scoped_command_line.h"
#include "base/test/scoped_feature_list.h"
#include "base/test/simple_test_clock.h"
#include "base/threading/scoped_blocking_call.h"
#include "base/threading/thread_restrictions.h"
#include "base/threading/thread_task_runner_handle.h"
#include "base/time/default_clock.h"
Expand Down Expand Up @@ -1753,6 +1754,182 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, SymantecEnforcementIsNotDisabled) {
->initial_ssl_config->symantec_enforcement_disabled);
}

// Test that CRLSets cause a certificate to be revoked and show an
// interstitial.
IN_PROC_BROWSER_TEST_F(SSLUITest, TestCRLSetRevoked) {
{
base::ScopedAllowBlockingForTesting allow_blocking;
ASSERT_TRUE(https_server_.Start());
}

std::string crl_set_bytes;
{
base::ScopedAllowBlockingForTesting allow_blocking;
base::ReadFileToString(
net::GetTestCertsDirectory().AppendASCII("crlset_by_leaf_spki.raw"),
&crl_set_bytes);
}
content::GetNetworkService()->UpdateCRLSet(
base::as_bytes(base::make_span(crl_set_bytes)));
content::FlushNetworkServiceInstanceForTesting();

ui_test_utils::NavigateToURL(browser(),
https_server_.GetURL("/ssl/google.html"));

WaitForInterstitial(browser()->tab_strip_model()->GetActiveWebContents());
ASSERT_NO_FATAL_FAILURE(ExpectSSLInterstitial(
browser()->tab_strip_model()->GetActiveWebContents()));

CheckAuthenticationBrokenState(
browser()->tab_strip_model()->GetActiveWebContents(),
net::CERT_STATUS_REVOKED, AuthState::SHOWING_INTERSTITIAL);
}

// Test that CRLSets configured to block MITM certificates cause the
// known interception interstitial.
IN_PROC_BROWSER_TEST_F(SSLUITest, TestCRLSetBlockedInterception) {
ASSERT_TRUE(https_server_.Start());

// Load a CRLSet that marks the root as a blocked MITM.
std::string crl_set_bytes;
{
base::ScopedAllowBlockingForTesting allow_blocking;
base::ReadFileToString(net::GetTestCertsDirectory().AppendASCII(
"crlset_blocked_interception_by_root.raw"),
&crl_set_bytes);
}
content::GetNetworkService()->UpdateCRLSet(
base::as_bytes(base::make_span(crl_set_bytes)));
content::FlushNetworkServiceInstanceForTesting();

ui_test_utils::NavigateToURL(browser(),
https_server_.GetURL("/ssl/google.html"));

WaitForInterstitial(browser()->tab_strip_model()->GetActiveWebContents());
ASSERT_NO_FATAL_FAILURE(ExpectSSLInterstitial(
browser()->tab_strip_model()->GetActiveWebContents()));

CheckAuthenticationBrokenState(
browser()->tab_strip_model()->GetActiveWebContents(),
net::CERT_STATUS_KNOWN_INTERCEPTION_BLOCKED | net::CERT_STATUS_REVOKED,
AuthState::SHOWING_INTERSTITIAL);

// Simulate clicking the learn more link.
SendInterstitialCommand(browser()->tab_strip_model()->GetActiveWebContents(),
security_interstitials::CMD_OPEN_HELP_CENTER);
EXPECT_EQ(browser()
->tab_strip_model()
->GetActiveWebContents()
->GetVisibleURL()
.ref(),
base::NumberToString(net::ERR_CERT_KNOWN_INTERCEPTION_BLOCKED));
}

// Test that CRLSets configured to identify known MITM certificates do not
// cause an interstitial unless the MITM certificate is blocked.
IN_PROC_BROWSER_TEST_F(SSLUITest, TestCRLSetKnownInterception) {
ASSERT_TRUE(https_server_.Start());

// Load a CRLSet that marks the root as a known MITM.
std::string crl_set_bytes;
{
base::ScopedAllowBlockingForTesting allow_blocking;
base::ReadFileToString(net::GetTestCertsDirectory().AppendASCII(
"crlset_known_interception_by_root.raw"),
&crl_set_bytes);
}
content::GetNetworkService()->UpdateCRLSet(
base::as_bytes(base::make_span(crl_set_bytes)));
content::FlushNetworkServiceInstanceForTesting();

// Navigate to the page. It should not cause an interstitial, but should
// allow for the display of additional information that interception is
// happening.
ui_test_utils::NavigateToURL(browser(),
https_server_.GetURL("/ssl/google.html"));
CheckAuthenticatedState(browser()->tab_strip_model()->GetActiveWebContents(),
AuthState::NONE);

content::NavigationEntry* entry = browser()
->tab_strip_model()
->GetActiveWebContents()
->GetController()
.GetVisibleEntry();
ASSERT_TRUE(entry);
EXPECT_TRUE(entry->GetSSL().cert_status &
net::CERT_STATUS_KNOWN_INTERCEPTION_DETECTED);
}

// While TestCRLSetBlockedInterception and TestCRLSetKnownInterception use
// a real CertVerifier in order to test that a real CRLSet is delivered and
// processed, testing HSTS requires with a MockCertVerifier so that the
// cert will match the intended hostname, and thus only fail because it's a
// blocked MITM certificate. This requires using a CertVerifierBrowserTest,
// which is not suitable for the previous tests because it does not test
// CRLSets.
class CRLSetInterceptionSSLUITest : public CertVerifierBrowserTest {
public:
CRLSetInterceptionSSLUITest()
: CertVerifierBrowserTest(),
https_server_(net::EmbeddedTestServer::TYPE_HTTPS) {}

void SetUpOnMainThread() override {
CertVerifierBrowserTest::SetUpOnMainThread();
SetHSTSForHostName(browser()->profile());

host_resolver()->AddRule("*", "127.0.0.1");
https_server_.AddDefaultHandlers(GetChromeTestDataDir());
}

protected:
net::EmbeddedTestServer https_server_;
};

IN_PROC_BROWSER_TEST_F(CRLSetInterceptionSSLUITest,
KnownInterceptionWorksOnHSTS) {
ASSERT_TRUE(https_server_.Start());

net::CertVerifyResult verify_result;
verify_result.verified_cert = https_server_.GetCertificate();
verify_result.cert_status =
net::CERT_STATUS_REVOKED | net::CERT_STATUS_KNOWN_INTERCEPTION_BLOCKED;

// Simulate verification returning that the certificate is a blocked
// interception certificate.
mock_cert_verifier()->AddResultForCertAndHost(
https_server_.GetCertificate(), kHstsTestHostName, verify_result,
net::ERR_CERT_KNOWN_INTERCEPTION_BLOCKED);

WebContents* tab = browser()->tab_strip_model()->GetActiveWebContents();

GURL::Replacements replacements;
replacements.SetHostStr(kHstsTestHostName);
GURL url =
https_server_.GetURL("/ssl/google.html").ReplaceComponents(replacements);

ui_test_utils::NavigateToURL(browser(), url);
WaitForInterstitial(tab);

CheckSecurityState(
tab,
net::CERT_STATUS_KNOWN_INTERCEPTION_BLOCKED | net::CERT_STATUS_REVOKED,
security_state::DANGEROUS, AuthState::SHOWING_INTERSTITIAL);

ASSERT_NO_FATAL_FAILURE(ExpectSSLInterstitial(tab));

// Expect there to be a proceed link, even for HSTS.
int result = security_interstitials::CMD_ERROR;
const std::string javascript = base::StringPrintf(
"domAutomationController.send("
"(document.querySelector(\"#proceed-link\") === null) "
"? (%d) : (%d))",
security_interstitials::CMD_TEXT_NOT_FOUND,
security_interstitials::CMD_TEXT_FOUND);
ASSERT_TRUE(content::ExecuteScriptAndExtractInt(tab->GetMainFrame(),
javascript, &result));
EXPECT_EQ(security_interstitials::CMD_TEXT_FOUND, result);
}

class CertificateTransparencySSLUITest : public CertVerifierBrowserTest {
public:
CertificateTransparencySSLUITest()
Expand Down Expand Up @@ -4326,7 +4503,7 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestLearnMoreLinkContainsErrorCode) {
->GetActiveWebContents()
->GetVisibleURL()
.ref(),
std::to_string(net::ERR_CERT_DATE_INVALID));
base::NumberToString(net::ERR_CERT_DATE_INVALID));
}

// Checks that interstitials are not used for subframe SSL errors. Regression
Expand Down
3 changes: 3 additions & 0 deletions chrome/browser/ssl/ssl_error_assistant.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ net::CertStatus MapToCertStatus(
case chrome_browser_ssl::DynamicInterstitial::
ERR_CERTIFICATE_TRANSPARENCY_REQUIRED:
return net::CERT_STATUS_CERTIFICATE_TRANSPARENCY_REQUIRED;
case chrome_browser_ssl::DynamicInterstitial::
ERR_CERT_KNOWN_INTERCEPTION_BLOCKED:
return net::CERT_STATUS_KNOWN_INTERCEPTION_BLOCKED;
case chrome_browser_ssl::DynamicInterstitial::ERR_CERT_SYMANTEC_LEGACY:
return net::CERT_STATUS_SYMANTEC_LEGACY;
case chrome_browser_ssl::DynamicInterstitial::ERR_CERT_REVOKED:
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/ssl/ssl_error_assistant.proto
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ message DynamicInterstitial {
ERR_CERT_NON_UNIQUE_NAME = 13;
ERR_CERTIFICATE_TRANSPARENCY_REQUIRED = 14;
ERR_CERT_SYMANTEC_LEGACY = 15;
ERR_CERT_KNOWN_INTERCEPTION_BLOCKED = 16;
};

// Sha256 hashes of the certificate's public key. If this field is not
Expand Down
1 change: 1 addition & 0 deletions components/domain_reliability/util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ const struct NetErrorMapping {
{net::ERR_CERT_COMMON_NAME_INVALID, "ssl.cert.name_invalid"},
{net::ERR_CERT_DATE_INVALID, "ssl.cert.date_invalid"},
{net::ERR_CERT_AUTHORITY_INVALID, "ssl.cert.authority_invalid"},
{net::ERR_CERT_KNOWN_INTERCEPTION_BLOCKED, "ssl.cert.authority_invalid"},
{net::ERR_CERT_REVOKED, "ssl.cert.revoked"},
{net::ERR_CERT_INVALID, "ssl.cert.invalid"},
{net::ERR_EMPTY_RESPONSE, "http.response.empty"},
Expand Down
2 changes: 2 additions & 0 deletions components/security_interstitials/content/cert_logger.proto
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ message CertLoggerRequest {
ERR_CERT_NON_UNIQUE_NAME = 13;
ERR_CERTIFICATE_TRANSPARENCY_REQUIRED = 14;
ERR_CERT_SYMANTEC_LEGACY = 15;
ERR_CERT_KNOWN_INTERCEPTION_BLOCKED = 16;
};

// Certificate errors encountered (if any) when validating this
Expand Down Expand Up @@ -182,6 +183,7 @@ message CertLoggerRequest {
STATUS_REV_CHECKING_ENABLED = 2;
STATUS_SHA1_SIGNATURE_PRESENT = 3;
STATUS_CT_COMPLIANCE_FAILED = 4;
STATUS_KNOWN_INTERCEPTION_DETECTED = 5;
}
// The non-error status results of validating the chain.
repeated CertStatus cert_status = 14;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ void AddCertStatusToReportErrors(
RENAME_CERT_STATUS(CERTIFICATE_TRANSPARENCY_REQUIRED,
CERTIFICATE_TRANSPARENCY_REQUIRED)
COPY_CERT_STATUS(SYMANTEC_LEGACY)
COPY_CERT_STATUS(KNOWN_INTERCEPTION_BLOCKED)

#undef RENAME_CERT_STATUS
#undef COPY_CERT_STATUS
Expand All @@ -74,6 +75,7 @@ void AddCertStatusToReportStatus(
COPY_CERT_STATUS(REV_CHECKING_ENABLED)
COPY_CERT_STATUS(SHA1_SIGNATURE_PRESENT)
COPY_CERT_STATUS(CT_COMPLIANCE_FAILED)
COPY_CERT_STATUS(KNOWN_INTERCEPTION_DETECTED)

#undef COPY_CERT_STATUS
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ ConnectionHelpUI::ConnectionHelpUI(content::WebUI* web_ui)
net::ERR_CERT_AUTHORITY_INVALID);
html_source->AddInteger("certWeakSignatureAlgorithm",
net::ERR_CERT_WEAK_SIGNATURE_ALGORITHM);
html_source->AddInteger("certKnownInterceptionBlocked",
net::ERR_CERT_KNOWN_INTERCEPTION_BLOCKED);

html_source->AddLocalizedString("connectionHelpTitle",
IDS_CONNECTION_HELP_TITLE);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ function setupEvents() {
case '#' + loadTimeData.getInteger('certCommonNameInvalid'):
case '#' + loadTimeData.getInteger('certAuthorityInvalid'):
case '#' + loadTimeData.getInteger('certWeakSignatureAlgorithm'):
case '#' + loadTimeData.getInteger('certKnownInterceptionBlocked'):
toggleHidden('details-certerror', 'details-certerror-button');
break;
case '#' + loadTimeData.getInteger('certExpired'):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ int IsCertErrorFatal(int cert_error) {
case net::ERR_CERT_VALIDITY_TOO_LONG:
case net::ERR_CERTIFICATE_TRANSPARENCY_REQUIRED:
case net::ERR_CERT_SYMANTEC_LEGACY:
case net::ERR_CERT_KNOWN_INTERCEPTION_BLOCKED:
return false;
case net::ERR_CERT_CONTAINS_ERRORS:
case net::ERR_CERT_REVOKED:
Expand Down
5 changes: 5 additions & 0 deletions components/ssl_errors/error_info.cc
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ ErrorInfo ErrorInfo::CreateError(ErrorType error_type,
IDS_CERT_ERROR_NOT_VALID_AT_THIS_TIME_DESCRIPTION);
}
break;
case CERT_KNOWN_INTERCEPTION_BLOCKED:
case CERT_AUTHORITY_INVALID:
case CERT_SYMANTEC_LEGACY:
details =
Expand Down Expand Up @@ -221,6 +222,8 @@ ErrorInfo::ErrorType ErrorInfo::NetErrorToErrorType(int net_error) {
return CERTIFICATE_TRANSPARENCY_REQUIRED;
case net::ERR_CERT_SYMANTEC_LEGACY:
return CERT_SYMANTEC_LEGACY;
case net::ERR_CERT_KNOWN_INTERCEPTION_BLOCKED:
return CERT_KNOWN_INTERCEPTION_BLOCKED;
default:
NOTREACHED();
return UNKNOWN;
Expand All @@ -247,6 +250,7 @@ void ErrorInfo::GetErrorsForCertStatus(
net::CERT_STATUS_VALIDITY_TOO_LONG,
net::CERT_STATUS_CERTIFICATE_TRANSPARENCY_REQUIRED,
net::CERT_STATUS_SYMANTEC_LEGACY,
net::CERT_STATUS_KNOWN_INTERCEPTION_BLOCKED,
};

const ErrorType kErrorTypes[] = {
Expand All @@ -263,6 +267,7 @@ void ErrorInfo::GetErrorsForCertStatus(
CERT_VALIDITY_TOO_LONG,
CERTIFICATE_TRANSPARENCY_REQUIRED,
CERT_SYMANTEC_LEGACY,
CERT_KNOWN_INTERCEPTION_BLOCKED,
};
DCHECK(base::size(kErrorFlags) == base::size(kErrorTypes));

Expand Down
1 change: 1 addition & 0 deletions components/ssl_errors/error_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ class ErrorInfo {
CERT_VALIDITY_TOO_LONG = 14,
CERTIFICATE_TRANSPARENCY_REQUIRED = 15,
CERT_SYMANTEC_LEGACY = 16,
CERT_KNOWN_INTERCEPTION_BLOCKED = 17,
END_OF_ENUM
};

Expand Down
3 changes: 3 additions & 0 deletions net/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -2520,12 +2520,15 @@ bundle_data("test_support_bundle_data") {
"data/ssl/certificates/common_name_only.pem",
"data/ssl/certificates/comodo-chain.pem",
"data/ssl/certificates/crit-codeSigning-chain.pem",
"data/ssl/certificates/crlset_blocked_interception_by_intermediate.raw",
"data/ssl/certificates/crlset_blocked_interception_by_root.raw",
"data/ssl/certificates/crlset_by_intermediate_serial.raw",
"data/ssl/certificates/crlset_by_leaf_spki.raw",
"data/ssl/certificates/crlset_by_leaf_subject_no_spki.raw",
"data/ssl/certificates/crlset_by_root_serial.raw",
"data/ssl/certificates/crlset_by_root_subject.raw",
"data/ssl/certificates/crlset_by_root_subject_no_spki.raw",
"data/ssl/certificates/crlset_known_interception_by_root.raw",
"data/ssl/certificates/cross-signed-leaf.pem",
"data/ssl/certificates/cross-signed-root-md5.pem",
"data/ssl/certificates/cross-signed-root-sha256.pem",
Expand Down
6 changes: 5 additions & 1 deletion net/base/net_error_list.h
Original file line number Diff line number Diff line change
Expand Up @@ -539,13 +539,17 @@ NET_ERROR(CERT_SYMANTEC_LEGACY, -215)
// -216 was QUIC_CERT_ROOT_NOT_KNOWN which has been renumbered to not be in the
// certificate error range.

// The certificate is known to be used for interception by an entity other
// the device owner.
NET_ERROR(CERT_KNOWN_INTERCEPTION_BLOCKED, -217)

// Add new certificate error codes here.
//
// Update the value of CERT_END whenever you add a new certificate error
// code.

// The value immediately past the last certificate error code.
NET_ERROR(CERT_END, -217)
NET_ERROR(CERT_END, -218)

// The URL is invalid.
NET_ERROR(INVALID_URL, -300)
Expand Down
3 changes: 2 additions & 1 deletion net/base/net_errors_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ TEST(NetErrorsTest, IsCertificateError) {
EXPECT_TRUE(IsCertificateError(ERR_CERT_WEAK_KEY));
EXPECT_TRUE(IsCertificateError(ERR_CERT_WEAK_SIGNATURE_ALGORITHM));
EXPECT_TRUE(IsCertificateError(ERR_SSL_PINNED_KEY_NOT_IN_CERT_CHAIN));
EXPECT_TRUE(IsCertificateError(ERR_CERT_KNOWN_INTERCEPTION_BLOCKED));

// Negative tests.
EXPECT_FALSE(IsCertificateError(ERR_SSL_PROTOCOL_ERROR));
Expand All @@ -41,7 +42,7 @@ TEST(NetErrorsTest, IsCertificateError) {

// Trigger a failure whenever ERR_CERT_END is changed, forcing developers to
// update this test.
EXPECT_EQ(ERR_CERT_END, -217)
EXPECT_EQ(ERR_CERT_END, -218)
<< "It looks like you added a new certificate error code ("
<< ErrorToString(ERR_CERT_END + 1)
<< ").\n"
Expand Down
Loading

0 comments on commit 54fe766

Please sign in to comment.