Add a preference and command-line option to disable SSL/TLS cipher suites

chrome/browser/net/ssl_config_service_manager_pref.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
+#include "chrome/browser/net/ssl_config_service_manager.h"
 
+#include <algorithm>
+#include <string>
+#include <vector>
+
+#include "base/basictypes.h"
 #include "base/message_loop.h"
 #include "base/threading/thread.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/io_thread.h"
-#include "chrome/browser/net/ssl_config_service_manager.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/pref_names.h"
-#include "content/common/content_notification_types.h"
+#include "chrome/common/chrome_notification_types.h"

[battre, 2011/07/20 11:16:30]

nit: order

 #include "content/common/notification_details.h"
 #include "content/common/notification_source.h"
+#include "net/base/ssl_cipher_suite_names.h"
 #include "net/base/ssl_config_service.h"
 
+namespace {
+
+// Converts a ListValue of StringValues into a vector of strings. Any Values
+// which cannot be converted will be skipped.
+std::vector<std::string> ListValueToStringVector(const ListValue* value) {
+  std::vector<std::string> results;
+  results.reserve(value->GetSize());
+  std::string s;
+  for (ListValue::const_iterator it = value->begin(); it != value->end();
+       ++it) {
+    if (!(*it)->GetAsString(&s))
+      continue;
+    results.push_back(s);
+  }
+  return results;
+}
+
+// Parses a vector of cipher suite strings, returning a sorted vector
+// containing the underlying SSL/TLS cipher suites. Unrecognized/invalid
+// cipher suites will be ignored.
+std::vector<uint16> ParseCipherSuites(
+    const std::vector<std::string>& cipher_strings) {
+  std::vector<uint16> cipher_suites;
+  cipher_suites.reserve(cipher_strings.size());
+
+  for (std::vector<std::string>::const_iterator it = cipher_strings.begin();
+       it != cipher_strings.end(); ++it) {
+    uint16 cipher_suite = 0;
+    if (!net::ParseSSLCipherString(*it, &cipher_suite)) {
+      LOG(ERROR) << "Ignoring unrecognized or unparsable cipher suite: "
+                 << cipher_suite;
+      continue;
+    }
+    cipher_suites.push_back(cipher_suite);
+  }
+  std::sort(cipher_suites.begin(), cipher_suites.end());
+  return cipher_suites;
+}
+
+}  // namespace
+
 ////////////////////////////////////////////////////////////////////////////////
 //  SSLConfigServicePref
 
 // An SSLConfigService which stores a cached version of the current SSLConfig
 // prefs, which are updated by SSLConfigServiceManagerPref when the prefs
 // change.
 class SSLConfigServicePref : public net::SSLConfigService {
  public:
   SSLConfigServicePref() {}
   virtual ~SSLConfigServicePref() {}
@@ skipping 46 matching lines @@
   // Callback for preference changes.  This will post the changes to the IO
   // thread with SetNewSSLConfig.
   virtual void Observe(int type,
                        const NotificationSource& source,
                        const NotificationDetails& details);
 
   // Store SSL config settings in |config|, directly from the preferences. Must
   // only be called from UI thread.
   void GetSSLConfigFromPrefs(net::SSLConfig* config);
 
+  // Processes changes to the disabled cipher suites preference, updating the
+  // cached list of parsed SSL/TLS cipher suites that are disabled.
+  void OnDisabledCipherSuitesChange(PrefService* prefs);
+
+  PrefChangeRegistrar pref_change_registrar_;
+
   // 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_;
+
   scoped_refptr<SSLConfigServicePref> ssl_config_service_;
 
   DISALLOW_COPY_AND_ASSIGN(SSLConfigServiceManagerPref);
 };
 
 SSLConfigServiceManagerPref::SSLConfigServiceManagerPref(
     PrefService* local_state)
     : ssl_config_service_(new SSLConfigServicePref()) {
   DCHECK(local_state);
 
   RegisterPrefs(local_state);

[battre, 2011/07/20 11:16:30]

This introduces an interesting circular dependency. It surprises me that it
works.

Preferences can only be set if they have been registered. If the local_state_
PrefService is created, it calls browser::RegisterLocalState but as far as I can
see, browser::RegisterLocalState does not call
SSLConfigServiceManagerPref::RegisterPrefs. The CommandLinePrefStore that is
part of local_state_ will try to set preferences in local_state_ even though
they have not been registered, yet.

Did you try starting chrome with a command line flag for the
CipherSuiteBlacklist?

The common pattern is to put the registration in browser::RegisterLocalState. I
think I would prefer that, but it might require to modify some unit tests.

[Ryan Sleevi, 2011/07/21 07:05:12]

I was surprised by this as well, when I was reading code to understand how prefs
work. I believe it's a hold-over from when the SSLConfigServiceManager
alternatively returned a manager that dealt with the system vs one that dealt
with preferences.

I did run all the tests I suggested in the note, so it certainly worked and
without any exceptions/errors/DCHECKs. I suspect this may be due to some complex
interactions with the IO thread which results in an SSLConfigServiceManager
being created very early on, perhaps before preferences are loaded/read (or
perhaps a straight up race).

I've pulled out the registration into an explicit static method and added it, as
you suggested. I haven't run into any problems in doing so, but I'm hoping the
tryjob panel will catch anything if this is not the case.

 
   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);
 
+  OnDisabledCipherSuitesChange(local_state);
   // Initialize from UI thread.  This is okay as there shouldn't be anything on
   // the IO thread trying to access it yet.
   GetSSLConfigFromPrefs(&ssl_config_service_->cached_config_);
 }
 
 // static
 void SSLConfigServiceManagerPref::RegisterPrefs(PrefService* prefs) {
   net::SSLConfig default_config;
   if (!prefs->FindPreference(prefs::kCertRevocationCheckingEnabled)) {
     prefs->RegisterBooleanPref(prefs::kCertRevocationCheckingEnabled,
                                default_config.rev_checking_enabled);
   }
   if (!prefs->FindPreference(prefs::kSSL3Enabled)) {
     prefs->RegisterBooleanPref(prefs::kSSL3Enabled,
                                default_config.ssl3_enabled);
   }
   if (!prefs->FindPreference(prefs::kTLS1Enabled)) {
     prefs->RegisterBooleanPref(prefs::kTLS1Enabled,
                                default_config.tls1_enabled);
   }
+  if (!prefs->FindPreference(prefs::kCipherSuiteBlacklist)) {
+    prefs->RegisterListPref(prefs::kCipherSuiteBlacklist);
+  }
 }
 
 net::SSLConfigService* SSLConfigServiceManagerPref::Get() {
   return ssl_config_service_;
 }
 
 void SSLConfigServiceManagerPref::Observe(int type,
                                           const NotificationSource& source,
                                           const NotificationDetails& details) {
+  if (type == chrome::NOTIFICATION_PREF_CHANGED) {
+    std::string* pref_name_in = Details<std::string>(details).ptr();
+    PrefService* prefs = Source<PrefService>(source).ptr();
+    DCHECK(pref_name_in && prefs);
+    if (*pref_name_in == prefs::kCipherSuiteBlacklist)
+      OnDisabledCipherSuitesChange(prefs);
+  }
+
   base::Thread* io_thread = g_browser_process->io_thread();
   if (io_thread) {
     net::SSLConfig new_config;
     GetSSLConfigFromPrefs(&new_config);
 
     // Post a task to |io_loop| with the new configuration, so it can
     // update |cached_config_|.
     io_thread->message_loop()->PostTask(
         FROM_HERE,
         NewRunnableMethod(
             ssl_config_service_.get(),
             &SSLConfigServicePref::SetNewSSLConfig,
             new_config));
   }
 }
 
 void SSLConfigServiceManagerPref::GetSSLConfigFromPrefs(
     net::SSLConfig* config) {
   config->rev_checking_enabled = rev_checking_enabled_.GetValue();
   config->ssl3_enabled = ssl3_enabled_.GetValue();
   config->tls1_enabled = tls1_enabled_.GetValue();
+  config->disabled_cipher_suites =
+      disabled_cipher_suites_;

[battre, 2011/07/20 11:16:30]

nit: single line

   SSLConfigServicePref::SetSSLConfigFlags(config);
 }
 
+void SSLConfigServiceManagerPref::OnDisabledCipherSuitesChange(
+    PrefService* prefs) {
+  const ListValue* value = prefs->GetList(prefs::kCipherSuiteBlacklist);
+  disabled_cipher_suites_ = ParseCipherSuites(
+      ListValueToStringVector(value));

[battre, 2011/07/20 11:16:30]

nit: single line?

+}
+
 ////////////////////////////////////////////////////////////////////////////////
 //  SSLConfigServiceManager
 
 // static
 SSLConfigServiceManager* SSLConfigServiceManager::CreateDefaultManager(
     PrefService* local_state) {
   return new SSLConfigServiceManagerPref(local_state);
 }