Skip to content

Commit

Permalink
Disable SHA-1 support for Local Anchors by default
Browse files Browse the repository at this point in the history
net::SSLConfig exposes a policy for configuring whether
or not to accept SHA-1 certificates issued by
locally-trusted (as opposed to publicly trusted) trust
anchors. The default for this policy was to accept these
certificates, while anything creating an
SSLConfigServiceManager under //components/ssl_config
would have these disabled by default, unless overridden
by preferences.

Change the default to be secure-by-default, as embedders
can supply an SSLConfigService that best reflects their
desired behaviours (if they do not wish the defaults) as
part of the URLRequestContext(Builder,Getter).

BUG=831240

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I0bcb3474458ca4e0f3e0a554054eec3046a103bb
Reviewed-on: https://chromium-review.googlesource.com/1005416
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Richard Coles <torne@chromium.org>
Commit-Queue: Ryan Sleevi <rsleevi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549910}
  • Loading branch information
sleevi authored and Commit Bot committed Apr 11, 2018
1 parent b802af6 commit e4fe0fb
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 10 deletions.
12 changes: 8 additions & 4 deletions android_webview/browser/net/aw_url_request_context_getter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -175,13 +175,17 @@ std::unique_ptr<net::URLRequestJobFactory> CreateJobFactory(
return job_factory;
}

// For Android WebView, do not enforce the policies outlined in
// For Android WebView, do not enforce policies that are not consistent with
// the underlying OS validator.
// This means not enforcing the Legacy Symantec PKI policies outlined in
// https://security.googleblog.com/2017/09/chromes-plan-to-distrust-symantec.html
// as those will be handled by Android itself on its own schedule. Otherwise,
// it returns the default SSLConfig.
// or disabling SHA-1 for locally-installed trust anchors.
class AwSSLConfigService : public net::SSLConfigService {
public:
AwSSLConfigService() { default_config_.symantec_enforcement_disabled = true; }
AwSSLConfigService() {
default_config_.symantec_enforcement_disabled = true;
default_config_.sha1_local_anchors_enabled = true;
}

void GetSSLConfig(net::SSLConfig* config) override {
*config = default_config_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,4 +99,19 @@ TEST_F(AwURLRequestContextGetterTest, SymantecPoliciesExempted) {
EXPECT_TRUE(config.symantec_enforcement_disabled);
}

// Tests that SHA-1 is still allowed for locally-installed trust anchors,
// including those in application manifests, as it should behave like
// the Android system.
TEST_F(AwURLRequestContextGetterTest, SHA1LocalAnchorsAllowed) {
net::URLRequestContext* context = getter_->GetURLRequestContext();
ASSERT_TRUE(context);
net::SSLConfigService* config_service = context->ssl_config_service();
ASSERT_TRUE(config_service);

net::SSLConfig config;
EXPECT_FALSE(config.sha1_local_anchors_enabled);
config_service->GetSSLConfig(&config);
EXPECT_TRUE(config.sha1_local_anchors_enabled);
}

} // namespace android_webview
Original file line number Diff line number Diff line change
Expand Up @@ -385,10 +385,10 @@ TEST_F(SSLConfigServiceManagerPrefTest, SHA1ForLocalAnchors) {
scoped_refptr<SSLConfigService> config_service(config_manager->Get());
ASSERT_TRUE(config_service);

// By default, SHA-1 local trust anchors should be enabled when not
// using any pref service.
// By default, SHA-1 local trust anchors should not be enabled when not
// not using any pref service.
SSLConfig config1;
EXPECT_TRUE(config1.sha1_local_anchors_enabled);
EXPECT_FALSE(config1.sha1_local_anchors_enabled);

// Using a pref service without any preference set should result in
// SHA-1 local trust anchors being disabled.
Expand Down
2 changes: 1 addition & 1 deletion net/ssl/ssl_config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ SSLConfig::CertAndStatus::~CertAndStatus() = default;
SSLConfig::SSLConfig()
: rev_checking_enabled(false),
rev_checking_required_local_anchors(false),
sha1_local_anchors_enabled(true),
sha1_local_anchors_enabled(false),
symantec_enforcement_disabled(false),
version_min(kDefaultSSLVersionMin),
version_max(kDefaultSSLVersionMax),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,8 @@
-SSLUITestWithClientCert.TestWSSClientCert

# https://bugs.chromium.org/p/chromium/issues/detail?id=755309
-SSLUITest.SHA1IsDefaultDisabled/0
-SSLUITest.SHA1IsDefaultDisabled/1
-SSLUITest.SHA1PrefsCanEnable/0
-SSLUITest.SHA1PrefsCanEnable/1
-SSLUITest.TestBadHTTPSDownload/0
-SSLUITest.TestBadHTTPSDownload/1
-SSLUITest.TestHTTPSOCSPOk/0
Expand Down

0 comments on commit e4fe0fb

Please sign in to comment.