Add back prefs::kSSL3Enabled and prefs::kTLS1Enabled, but control

the preferences with the command-line options via the
CommandLinePrefStore. This allows us to control the preferences
via the PolicyPrefStores or any other pref store in the future.

Patch originally by wtc@chromium.org at http://codereview.chromium.org/7776002

R=wtc@chromium.org
BUG=102019
TEST=none


Review URL: http://codereview.chromium.org/8402019

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@108073 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 151 insertions, 36 deletions

diff --git a/chrome/browser/net/ssl_config_service_manager_pref.cc b/chrome/browser/net/ssl_config_service_manager_pref.cc
index ffb1394..af3683b 100644
--- a/chrome/browser/net/ssl_config_service_manager_pref.cc
+++ b/chrome/browser/net/ssl_config_service_manager_pref.cc
@@ -9,12 +9,10 @@
 
 #include "base/basictypes.h"
 #include "base/bind.h"
-#include "base/command_line.h"
 #include "chrome/browser/prefs/pref_change_registrar.h"
 #include "chrome/browser/prefs/pref_member.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/common/chrome_notification_types.h"
-#include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_details.h"
@@ -138,6 +136,8 @@ class SSLConfigServiceManagerPref
 
   // The prefs (should only be accessed from UI thread)
   BooleanPrefMember rev_checking_enabled_;
+  BooleanPrefMember ssl3_enabled_;
+  BooleanPrefMember tls1_enabled_;
 
   // The cached list of disabled SSL cipher suites.
   std::vector<uint16> disabled_cipher_suites_;
@@ -154,6 +154,8 @@ SSLConfigServiceManagerPref::SSLConfigServiceManagerPref(
 
   rev_checking_enabled_.Init(prefs::kCertRevocationCheckingEnabled,
                              local_state, this);
+  ssl3_enabled_.Init(prefs::kSSL3Enabled, local_state, this);
+  tls1_enabled_.Init(prefs::kTLS1Enabled, local_state, this);
   pref_change_registrar_.Init(local_state);
   pref_change_registrar_.Add(prefs::kCipherSuiteBlacklist, this);
 
@@ -168,7 +170,16 @@ void SSLConfigServiceManagerPref::RegisterPrefs(PrefService* prefs) {
   net::SSLConfig default_config;
   prefs->RegisterBooleanPref(prefs::kCertRevocationCheckingEnabled,
                              default_config.rev_checking_enabled);
+  prefs->RegisterBooleanPref(prefs::kSSL3Enabled,
+                             default_config.ssl3_enabled);
+  prefs->RegisterBooleanPref(prefs::kTLS1Enabled,
+                             default_config.tls1_enabled);
   prefs->RegisterListPref(prefs::kCipherSuiteBlacklist);
+  // The Options menu used to allow changing the ssl.ssl3.enabled and
+  // ssl.tls1.enabled preferences, so some users' Local State may have
+  // these preferences.  Remove them from Local State.
+  prefs->ClearPref(prefs::kSSL3Enabled);
+  prefs->ClearPref(prefs::kTLS1Enabled);
 }
 
 net::SSLConfigService* SSLConfigServiceManagerPref::Get() {
@@ -205,12 +216,8 @@ void SSLConfigServiceManagerPref::Observe(
 void SSLConfigServiceManagerPref::GetSSLConfigFromPrefs(
     net::SSLConfig* config) {
   config->rev_checking_enabled = rev_checking_enabled_.GetValue();
-
-  config->ssl3_enabled =
-      !CommandLine::ForCurrentProcess()->HasSwitch(switches::kDisableSSL3);
-  config->tls1_enabled =
-      !CommandLine::ForCurrentProcess()->HasSwitch(switches::kDisableTLS1);
-
+  config->ssl3_enabled = ssl3_enabled_.GetValue();
+  config->tls1_enabled = tls1_enabled_.GetValue();
   config->disabled_cipher_suites = disabled_cipher_suites_;
   SSLConfigServicePref::SetSSLConfigFlags(config);
 }
diff --git a/chrome/browser/net/ssl_config_service_manager_pref_unittest.cc b/chrome/browser/net/ssl_config_service_manager_pref_unittest.cc
index 0d3e906..525335b 100644
--- a/chrome/browser/net/ssl_config_service_manager_pref_unittest.cc
+++ b/chrome/browser/net/ssl_config_service_manager_pref_unittest.cc
@@ -4,9 +4,14 @@
 
 #include "chrome/browser/net/pref_proxy_config_service.h"
 
+#include "base/command_line.h"
+#include "base/memory/ref_counted.h"
 #include "base/message_loop.h"
 #include "base/values.h"
 #include "chrome/browser/net/ssl_config_service_manager.h"
+#include "chrome/browser/prefs/pref_service_mock_builder.h"
+#include "chrome/browser/prefs/testing_pref_store.h"
+#include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_pref_service.h"
 #include "content/test/test_browser_thread.h"
@@ -20,37 +25,24 @@ using net::SSLConfigService;
 
 class SSLConfigServiceManagerPrefTest : public testing::Test {
  public:
-  SSLConfigServiceManagerPrefTest() {}
-
-  virtual void SetUp() {
-    message_loop_.reset(new MessageLoop());
-    ui_thread_.reset(
-        new content::TestBrowserThread(BrowserThread::UI, message_loop_.get()));
-    io_thread_.reset(
-        new content::TestBrowserThread(BrowserThread::IO, message_loop_.get()));
-    pref_service_.reset(new TestingPrefService());
-    SSLConfigServiceManager::RegisterPrefs(pref_service_.get());
-  }
-
-  virtual void TearDown() {
-    pref_service_.reset();
-    io_thread_.reset();
-    ui_thread_.reset();
-    message_loop_.reset();
-  }
+  SSLConfigServiceManagerPrefTest()
+      : ui_thread_(BrowserThread::UI, &message_loop_),
+        io_thread_(BrowserThread::IO, &message_loop_) {}
 
  protected:
-  scoped_ptr<MessageLoop> message_loop_;
-  scoped_ptr<content::TestBrowserThread> ui_thread_;
-  scoped_ptr<content::TestBrowserThread> io_thread_;
-  scoped_ptr<TestingPrefService> pref_service_;
+  MessageLoop message_loop_;
+  content::TestBrowserThread ui_thread_;
+  content::TestBrowserThread io_thread_;
 };
 
 // Test that cipher suites can be disabled. "Good" refers to the fact that
 // every value is expected to be successfully parsed into a cipher suite.
 TEST_F(SSLConfigServiceManagerPrefTest, GoodDisabledCipherSuites) {
+  TestingPrefService pref_service;
+  SSLConfigServiceManager::RegisterPrefs(&pref_service);
+
   scoped_ptr<SSLConfigServiceManager> config_manager(
-      SSLConfigServiceManager::CreateDefaultManager(pref_service_.get()));
+      SSLConfigServiceManager::CreateDefaultManager(&pref_service));
   ASSERT_TRUE(config_manager.get());
   scoped_refptr<SSLConfigService> config_service(config_manager->Get());
   ASSERT_TRUE(config_service.get());
@@ -62,11 +54,11 @@ TEST_F(SSLConfigServiceManagerPrefTest, GoodDisabledCipherSuites) {
   ListValue* list_value = new ListValue();
   list_value->Append(Value::CreateStringValue("0x0004"));
   list_value->Append(Value::CreateStringValue("0x0005"));
-  pref_service_->SetUserPref(prefs::kCipherSuiteBlacklist, list_value);
+  pref_service.SetUserPref(prefs::kCipherSuiteBlacklist, list_value);
 
   // Pump the message loop to notify the SSLConfigServiceManagerPref that the
   // preferences changed.
-  message_loop_->RunAllPending();
+  message_loop_.RunAllPending();
 
   SSLConfig config;
   config_service->GetSSLConfig(&config);
@@ -81,8 +73,11 @@ TEST_F(SSLConfigServiceManagerPrefTest, GoodDisabledCipherSuites) {
 // there are one or more non-cipher suite strings in the preference. They
 // should be ignored.
 TEST_F(SSLConfigServiceManagerPrefTest, BadDisabledCipherSuites) {
+  TestingPrefService pref_service;
+  SSLConfigServiceManager::RegisterPrefs(&pref_service);
+
   scoped_ptr<SSLConfigServiceManager> config_manager(
-      SSLConfigServiceManager::CreateDefaultManager(pref_service_.get()));
+      SSLConfigServiceManager::CreateDefaultManager(&pref_service));
   ASSERT_TRUE(config_manager.get());
   scoped_refptr<SSLConfigService> config_service(config_manager->Get());
   ASSERT_TRUE(config_service.get());
@@ -96,11 +91,11 @@ TEST_F(SSLConfigServiceManagerPrefTest, BadDisabledCipherSuites) {
   list_value->Append(Value::CreateStringValue("TLS_NOT_WITH_A_CIPHER_SUITE"));
   list_value->Append(Value::CreateStringValue("0x0005"));
   list_value->Append(Value::CreateStringValue("0xBEEFY"));
-  pref_service_->SetUserPref(prefs::kCipherSuiteBlacklist, list_value);
+  pref_service.SetUserPref(prefs::kCipherSuiteBlacklist, list_value);
 
   // Pump the message loop to notify the SSLConfigServiceManagerPref that the
   // preferences changed.
-  message_loop_->RunAllPending();
+  message_loop_.RunAllPending();
 
   SSLConfig config;
   config_service->GetSSLConfig(&config);
@@ -110,3 +105,110 @@ TEST_F(SSLConfigServiceManagerPrefTest, BadDisabledCipherSuites) {
   EXPECT_EQ(0x0004, config.disabled_cipher_suites[0]);
   EXPECT_EQ(0x0005, config.disabled_cipher_suites[1]);
 }
+
+// Test that existing user settings for TLS1.0/SSL3.0 are both ignored and
+// cleared from user preferences.
+TEST_F(SSLConfigServiceManagerPrefTest, IgnoreLegacySSLSettings) {
+  scoped_refptr<TestingPrefStore> user_prefs(new TestingPrefStore());
+
+  // SSL3.0 and TLS1.0 used to be user-definable prefs. They are now used as
+  // command-line options. Ensure any existing user prefs are ignored in
+  // favour of the command-line flags.
+  user_prefs->SetBoolean(prefs::kSSL3Enabled, false);
+  user_prefs->SetBoolean(prefs::kTLS1Enabled, false);
+
+  // Ensure the preferences exist initially.
+  bool is_ssl3_enabled = true;
+  EXPECT_TRUE(user_prefs->GetBoolean(prefs::kSSL3Enabled, &is_ssl3_enabled));
+  EXPECT_FALSE(is_ssl3_enabled);
+
+  bool is_tls1_enabled = true;
+  EXPECT_TRUE(user_prefs->GetBoolean(prefs::kTLS1Enabled, &is_tls1_enabled));
+  EXPECT_FALSE(is_tls1_enabled);
+
+  PrefServiceMockBuilder builder;
+  builder.WithUserPrefs(user_prefs.get());
+  scoped_ptr<PrefService> pref_service(builder.Create());
+
+  SSLConfigServiceManager::RegisterPrefs(pref_service.get());
+
+  scoped_ptr<SSLConfigServiceManager> config_manager(
+      SSLConfigServiceManager::CreateDefaultManager(pref_service.get()));
+  ASSERT_TRUE(config_manager.get());
+  scoped_refptr<SSLConfigService> config_service(config_manager->Get());
+  ASSERT_TRUE(config_service.get());
+
+  SSLConfig ssl_config;
+  config_service->GetSSLConfig(&ssl_config);
+  // The default value in the absence of command-line options is that both
+  // protocols are enabled.
+  EXPECT_TRUE(ssl_config.ssl3_enabled);
+  EXPECT_TRUE(ssl_config.tls1_enabled);
+
+  // The existing user settings should be removed from the pref_service.
+  EXPECT_FALSE(pref_service->HasPrefPath(prefs::kSSL3Enabled));
+  EXPECT_FALSE(pref_service->HasPrefPath(prefs::kTLS1Enabled));
+
+  // Explicitly double-check the settings are not in the user preference
+  // store.
+  EXPECT_FALSE(user_prefs->GetBoolean(prefs::kSSL3Enabled, &is_ssl3_enabled));
+  EXPECT_FALSE(user_prefs->GetBoolean(prefs::kTLS1Enabled, &is_tls1_enabled));
+}
+
+// Test that command-line settings for TLS1.0/SSL3.0 are respected, that they
+// disregard any existing user preferences, and that they do not persist to
+// the user preferences files.
+TEST_F(SSLConfigServiceManagerPrefTest, CommandLineOverridesUserPrefs) {
+  scoped_refptr<TestingPrefStore> user_prefs(new TestingPrefStore());
+
+  // Explicitly enable SSL3.0/TLS1.0 in the user preferences, to mirror the
+  // more common legacy file.
+  user_prefs->SetBoolean(prefs::kSSL3Enabled, true);
+  user_prefs->SetBoolean(prefs::kTLS1Enabled, true);
+
+  // Ensure the preferences exist initially.
+  bool is_ssl3_enabled = false;
+  EXPECT_TRUE(user_prefs->GetBoolean(prefs::kSSL3Enabled, &is_ssl3_enabled));
+  EXPECT_TRUE(is_ssl3_enabled);
+
+  bool is_tls1_enabled = false;
+  EXPECT_TRUE(user_prefs->GetBoolean(prefs::kTLS1Enabled, &is_tls1_enabled));
+  EXPECT_TRUE(is_tls1_enabled);
+
+  CommandLine command_line(CommandLine::NO_PROGRAM);
+  command_line.AppendSwitch(switches::kDisableSSL3);
+  command_line.AppendSwitch(switches::kDisableTLS1);
+
+  PrefServiceMockBuilder builder;
+  builder.WithUserPrefs(user_prefs.get());
+  builder.WithCommandLine(&command_line);
+  scoped_ptr<PrefService> pref_service(builder.Create());
+
+  SSLConfigServiceManager::RegisterPrefs(pref_service.get());
+
+  scoped_ptr<SSLConfigServiceManager> config_manager(
+      SSLConfigServiceManager::CreateDefaultManager(pref_service.get()));
+  ASSERT_TRUE(config_manager.get());
+  scoped_refptr<SSLConfigService> config_service(config_manager->Get());
+  ASSERT_TRUE(config_service.get());
+
+  SSLConfig ssl_config;
+  config_service->GetSSLConfig(&ssl_config);
+  // Command-line flags to disable should override the user preferences to
+  // enable.
+  EXPECT_FALSE(ssl_config.ssl3_enabled);
+  EXPECT_FALSE(ssl_config.tls1_enabled);
+
+  // Explicitly double-check the settings are not in the user preference
+  // store.
+  const PrefService::Preference* ssl3_enabled_pref =
+      pref_service->FindPreference(prefs::kSSL3Enabled);
+  EXPECT_FALSE(ssl3_enabled_pref->IsUserModifiable());
+
+  const PrefService::Preference* tls1_enabled_pref =
+      pref_service->FindPreference(prefs::kTLS1Enabled);
+  EXPECT_FALSE(tls1_enabled_pref->IsUserModifiable());
+
+  EXPECT_FALSE(user_prefs->GetBoolean(prefs::kSSL3Enabled, &is_ssl3_enabled));
+  EXPECT_FALSE(user_prefs->GetBoolean(prefs::kTLS1Enabled, &is_tls1_enabled));
+}
diff --git a/chrome/browser/prefs/command_line_pref_store.cc b/chrome/browser/prefs/command_line_pref_store.cc
index 021be74..af4f39d 100644
--- a/chrome/browser/prefs/command_line_pref_store.cc
+++ b/chrome/browser/prefs/command_line_pref_store.cc
@@ -42,6 +42,8 @@ const CommandLinePrefStore::BooleanSwitchToPreferenceMapEntry
         prefs::kWebKitAllowDisplayingInsecureContent, false },
       { switches::kAllowCrossOriginAuthPrompt,
         prefs::kAllowCrossOriginAuthPrompt, true },
+      { switches::kDisableSSL3, prefs::kSSL3Enabled, false },
+      { switches::kDisableTLS1, prefs::kTLS1Enabled, false },
 };
 
 CommandLinePrefStore::CommandLinePrefStore(const CommandLine* command_line)
diff --git a/chrome/common/pref_names.cc b/chrome/common/pref_names.cc
index 5486c31..7c3011d 100644
--- a/chrome/common/pref_names.cc
+++ b/chrome/common/pref_names.cc
@@ -870,6 +870,8 @@ const char kProfileInfoCache[] = "profile.info_cache";
 
 // Prefs for SSLConfigServicePref.
 const char kCertRevocationCheckingEnabled[] = "ssl.rev_checking.enabled";
+const char kSSL3Enabled[] = "ssl.ssl3.enabled";
+const char kTLS1Enabled[] = "ssl.tls1.enabled";
 const char kCipherSuiteBlacklist[] = "ssl.cipher_suites.blacklist";
 
 // The metrics client GUID and session ID.
diff --git a/chrome/common/pref_names.h b/chrome/common/pref_names.h
index eaf4347..7c956063 100644
--- a/chrome/common/pref_names.h
+++ b/chrome/common/pref_names.h
@@ -308,6 +308,8 @@ extern const char kPasswordsUseLocalProfileId[];
 
 // Local state prefs. Please add Profile prefs above instead.
 extern const char kCertRevocationCheckingEnabled[];
+extern const char kSSL3Enabled[];
+extern const char kTLS1Enabled[];
 extern const char kCipherSuiteBlacklist[];
 
 extern const char kMetricsClientID[];