Detect when the disabled SSL/TLS cipher suites change within an SSLConfig

BUG=58831
TEST=net_unittests --gtest_filter=SSLConfigServiceTest.*

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

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

3 files changed, 128 insertions, 11 deletions

diff --git a/net/base/ssl_config_service.cc b/net/base/ssl_config_service.cc
index f3a09f0..82c9add 100644
--- a/net/base/ssl_config_service.cc
+++ b/net/base/ssl_config_service.cc
@@ -109,11 +109,19 @@ void SSLConfigService::SetSSLConfigFlags(SSLConfig* ssl_config) {
 
 void SSLConfigService::ProcessConfigUpdate(const SSLConfig& orig_config,
                                            const SSLConfig& new_config) {
-  if (orig_config.rev_checking_enabled != new_config.rev_checking_enabled ||
-      orig_config.ssl3_enabled != new_config.ssl3_enabled ||
-      orig_config.tls1_enabled != new_config.tls1_enabled) {
-    FOR_EACH_OBSERVER(Observer, observer_list_, OnSSLConfigChanged());
+  bool config_changed =
+      (orig_config.rev_checking_enabled != new_config.rev_checking_enabled) ||
+      (orig_config.ssl3_enabled != new_config.ssl3_enabled) ||
+      (orig_config.tls1_enabled != new_config.tls1_enabled) ||
+      (orig_config.disabled_cipher_suites.size() !=
+       new_config.disabled_cipher_suites.size());
+  if (!config_changed) {
+    config_changed = (orig_config.disabled_cipher_suites !=
+        new_config.disabled_cipher_suites);
   }
+
+  if (config_changed)
+    FOR_EACH_OBSERVER(Observer, observer_list_, OnSSLConfigChanged());
 }
 
 // static
diff --git a/net/base/ssl_config_service.h b/net/base/ssl_config_service.h
index ab84aad..98acc1e 100644
--- a/net/base/ssl_config_service.h
+++ b/net/base/ssl_config_service.h
@@ -42,17 +42,18 @@ struct NET_API SSLConfig {
   // True if we'll do async checks for certificate provenance using DNS.
   bool dns_cert_provenance_checking_enabled;
 
-  // Cipher suites which should be explicitly prevented from being used in
-  // addition to those disabled by the net built-in policy -- by default, all
-  // cipher suites supported by the underlying SSL implementation will be
-  // enabled except for:
+  // Presorted list of cipher suites which should be explicitly prevented from
+  // being used in addition to those disabled by the net built-in policy.
+  //
+  // By default, all cipher suites supported by the underlying SSL
+  // implementation will be enabled except for:
   // - Null encryption cipher suites.
   // - Weak cipher suites: < 80 bits of security strength.
   // - FORTEZZA cipher suites (obsolete).
   // - IDEA cipher suites (RFC 5469 explains why).
   // - Anonymous cipher suites.
   // The ciphers listed in |disabled_cipher_suites| will be removed in addition
-  // to the above statically defined disable list.
+  // to the above list.
   //
   // Though cipher suites are sent in TLS as "uint8 CipherSuite[2]", in
   // big-endian form, they should be declared in host byte order, with the
@@ -60,7 +61,7 @@ struct NET_API SSLConfig {
   // Ex: To disable TLS_RSA_WITH_RC4_128_MD5, specify 0x0004, while to
   // disable TLS_ECDH_ECDSA_WITH_RC4_128_SHA, specify 0xC002.
   //
-  // TODO(rsleevi): Not implemented when using Schannel.
+  // Note: Not implemented when using Schannel/SSLClientSocketWin.
   std::vector<uint16> disabled_cipher_suites;
 
   bool cached_info_enabled;  // True if TLS cached info extension is enabled.
@@ -118,6 +119,7 @@ class NET_API SSLConfigService
     //     rev_checking_enabled
     //     ssl3_enabled
     //     tls1_enabled
+    //     disabled_cipher_suites
     virtual void OnSSLConfigChanged() = 0;
 
    protected:
diff --git a/net/base/ssl_config_service_unittest.cc b/net/base/ssl_config_service_unittest.cc
index 528cf99..67036bb 100644
--- a/net/base/ssl_config_service_unittest.cc
+++ b/net/base/ssl_config_service_unittest.cc
@@ -3,15 +3,53 @@
 // found in the LICENSE file.
 
 #include "net/base/ssl_config_service.h"
+
+#include <vector>
+
+#include "base/basictypes.h"
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+namespace net {
+
 namespace {
 
 bool IsFalseStartIncompatible(const std::string& hostname) {
-  return net::SSLConfigService::IsKnownFalseStartIncompatibleServer(
+  return SSLConfigService::IsKnownFalseStartIncompatibleServer(
       hostname);
 }
 
+class MockSSLConfigService : public SSLConfigService {
+ public:
+  explicit MockSSLConfigService(const SSLConfig& config) : config_(config) {}
+
+  // SSLConfigService implementation
+  virtual void GetSSLConfig(SSLConfig* config) {
+    *config = config_;
+  }
+
+  // Sets the SSLConfig to be returned by GetSSLConfig and processes any
+  // updates.
+  void SetSSLConfig(const SSLConfig& config) {
+    SSLConfig old_config = config_;
+    config_ = config;
+    ProcessConfigUpdate(old_config, config_);
+  }
+
+ private:
+  virtual ~MockSSLConfigService() {}
+
+  SSLConfig config_;
+};
+
+class MockSSLConfigServiceObserver : public SSLConfigService::Observer {
+ public:
+  MockSSLConfigServiceObserver() {}
+  virtual ~MockSSLConfigServiceObserver() {}
+
+  MOCK_METHOD0(OnSSLConfigChanged, void());
+};
+
 }  // namespace
 
 TEST(SSLConfigServiceTest, FalseStartDisabledHosts) {
@@ -28,3 +66,72 @@ TEST(SSLConfigServiceTest, FalseStartDisabledDomains) {
   EXPECT_FALSE(IsFalseStartIncompatible("yodleea.com"));
   EXPECT_FALSE(IsFalseStartIncompatible("yodlee.org"));
 }
+
+TEST(SSLConfigServiceTest, NoChangesWontNotifyObservers) {
+  SSLConfig initial_config;
+  initial_config.rev_checking_enabled = true;
+  initial_config.ssl3_enabled = true;
+  initial_config.tls1_enabled = true;
+
+  scoped_refptr<MockSSLConfigService> mock_service(
+      new MockSSLConfigService(initial_config));
+  MockSSLConfigServiceObserver observer;
+  mock_service->AddObserver(&observer);
+
+  EXPECT_CALL(observer, OnSSLConfigChanged()).Times(0);
+  mock_service->SetSSLConfig(initial_config);
+
+  mock_service->RemoveObserver(&observer);
+}
+
+TEST(SSLConfigServiceTest, ConfigUpdatesNotifyObservers) {
+  SSLConfig initial_config;
+  initial_config.rev_checking_enabled = true;
+  initial_config.ssl3_enabled = true;
+  initial_config.tls1_enabled = true;
+
+  scoped_refptr<MockSSLConfigService> mock_service(
+      new MockSSLConfigService(initial_config));
+  MockSSLConfigServiceObserver observer;
+  mock_service->AddObserver(&observer);
+
+  // Test that the basic boolean preferences trigger updates.
+  initial_config.rev_checking_enabled = false;
+  EXPECT_CALL(observer, OnSSLConfigChanged()).Times(1);
+  mock_service->SetSSLConfig(initial_config);
+
+  initial_config.ssl3_enabled = false;
+  EXPECT_CALL(observer, OnSSLConfigChanged()).Times(1);
+  mock_service->SetSSLConfig(initial_config);
+
+  initial_config.tls1_enabled = false;
+  EXPECT_CALL(observer, OnSSLConfigChanged()).Times(1);
+  mock_service->SetSSLConfig(initial_config);
+
+  // Test that disabling certain cipher suites triggers an update.
+  std::vector<uint16> disabled_ciphers;
+  disabled_ciphers.push_back(0x0004u);
+  disabled_ciphers.push_back(0xBEEFu);
+  disabled_ciphers.push_back(0xDEADu);
+  initial_config.disabled_cipher_suites = disabled_ciphers;
+  EXPECT_CALL(observer, OnSSLConfigChanged()).Times(1);
+  mock_service->SetSSLConfig(initial_config);
+
+  // Ensure that changing a disabled cipher suite, while still maintaining
+  // sorted order, triggers an update.
+  disabled_ciphers[1] = 0xCAFEu;
+  initial_config.disabled_cipher_suites = disabled_ciphers;
+  EXPECT_CALL(observer, OnSSLConfigChanged()).Times(1);
+  mock_service->SetSSLConfig(initial_config);
+
+  // Ensure that removing a disabled cipher suite, while still keeping some
+  // cipher suites disabled, triggers an update.
+  disabled_ciphers.pop_back();
+  initial_config.disabled_cipher_suites = disabled_ciphers;
+  EXPECT_CALL(observer, OnSSLConfigChanged()).Times(1);
+  mock_service->SetSSLConfig(initial_config);
+
+  mock_service->RemoveObserver(&observer);
+}
+
+}  // namespace net