Expose TLS settings in the Security panel overview, and call out individual obsolete settings.

net/ssl/ssl_cipher_suite_names_unittest.cc

Index: net/ssl/ssl_cipher_suite_names_unittest.cc
diff --git a/net/ssl/ssl_cipher_suite_names_unittest.cc b/net/ssl/ssl_cipher_suite_names_unittest.cc
index cfa26e0a6b20c262084bb70e331861fd3ffd20b1..b96f80331a62865544737e366c06a80c5c903592 100644
--- a/net/ssl/ssl_cipher_suite_names_unittest.cc
+++ b/net/ssl/ssl_cipher_suite_names_unittest.cc
@@ -5,8 +5,21 @@
 #include "net/ssl/ssl_cipher_suite_names.h"
 
 #include "base/macros.h"
+#include "net/ssl/ssl_connection_status_flags.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+namespace {
+
+int MakeConnectionStatus(int version, uint16_t cipher_suite) {

[davidben, 2016/04/19 17:47:01]

Nit: Move this after line 25 and remove the net:: prefixes.

[lgarron, 2016/04/25 23:56:54]

Done.

+  int connection_status = 0;
+
+  net::SSLConnectionStatusSetVersion(version, &connection_status);
+  net::SSLConnectionStatusSetCipherSuite(cipher_suite, &connection_status);
+
+  return connection_status;
+}
+}
+
 namespace net {
 
 namespace {
@@ -69,40 +82,117 @@ TEST(CipherSuiteNamesTest, ParseSSLCipherStringFails) {
   }
 }
 
-TEST(CipherSuiteNamesTest, SecureCipherSuites) {
+TEST(CipherSuiteNamesTest, ObsoleteCipherSuites) {
   // Picked some random cipher suites.
-  EXPECT_FALSE(IsSecureTLSCipherSuite(0x0 /* TLS_NULL_WITH_NULL_NULL */));
-  EXPECT_FALSE(
-      IsSecureTLSCipherSuite(0x39 /* TLS_DHE_RSA_WITH_AES_256_CBC_SHA */));
-  EXPECT_FALSE(IsSecureTLSCipherSuite(
+  EXPECT_TRUE(IsObsoleteTLSCipherSuite(0x0 /* TLS_NULL_WITH_NULL_NULL */));
+  EXPECT_TRUE(
+      IsObsoleteTLSCipherSuite(0x39 /* TLS_DHE_RSA_WITH_AES_256_CBC_SHA */));
+  EXPECT_TRUE(IsObsoleteTLSCipherSuite(
       0xc5 /* TLS_DH_anon_WITH_CAMELLIA_256_CBC_SHA256 */));
-  EXPECT_FALSE(
-      IsSecureTLSCipherSuite(0xc00f /* TLS_ECDH_RSA_WITH_AES_256_CBC_SHA */));
-  EXPECT_FALSE(IsSecureTLSCipherSuite(
+  EXPECT_TRUE(
+      IsObsoleteTLSCipherSuite(0xc00f /* TLS_ECDH_RSA_WITH_AES_256_CBC_SHA */));
+  EXPECT_TRUE(IsObsoleteTLSCipherSuite(
       0xc083 /* TLS_DH_DSS_WITH_CAMELLIA_256_GCM_SHA384 */));
-  EXPECT_FALSE(
-      IsSecureTLSCipherSuite(0x9e /* TLS_DHE_RSA_WITH_AES_128_GCM_SHA256 */));
-  EXPECT_FALSE(
-      IsSecureTLSCipherSuite(0xc014 /* TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA */));
-  EXPECT_FALSE(
-      IsSecureTLSCipherSuite(0x9c /* TLS_RSA_WITH_AES_128_GCM_SHA256 */));
+  EXPECT_TRUE(
+      IsObsoleteTLSCipherSuite(0x9e /* TLS_DHE_RSA_WITH_AES_128_GCM_SHA256 */));
+  EXPECT_TRUE(IsObsoleteTLSCipherSuite(
+      0xc014 /* TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA */));
+  EXPECT_TRUE(
+      IsObsoleteTLSCipherSuite(0x9c /* TLS_RSA_WITH_AES_128_GCM_SHA256 */));
 
   // Non-existent cipher suite.
-  EXPECT_FALSE(IsSecureTLSCipherSuite(0xffff)) << "Doesn't exist!";
+  EXPECT_TRUE(IsObsoleteTLSCipherSuite(0xffff)) << "Doesn't exist!";
 
   // Secure ones.
-  EXPECT_TRUE(IsSecureTLSCipherSuite(
+  EXPECT_FALSE(IsObsoleteTLSCipherSuite(
       0xc02f /* TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 */));
-  EXPECT_TRUE(IsSecureTLSCipherSuite(
+  EXPECT_FALSE(IsObsoleteTLSCipherSuite(
       0xcc13 /* ECDHE_RSA_WITH_CHACHA20_POLY1305 (non-standard) */));
-  EXPECT_TRUE(IsSecureTLSCipherSuite(
+  EXPECT_FALSE(IsObsoleteTLSCipherSuite(
       0xcc14 /* ECDHE_ECDSA_WITH_CHACHA20_POLY1305 (non-standard) */));
-  EXPECT_TRUE(IsSecureTLSCipherSuite(
+  EXPECT_FALSE(IsObsoleteTLSCipherSuite(
       0xcca8 /* ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256 */));
-  EXPECT_TRUE(IsSecureTLSCipherSuite(
+  EXPECT_FALSE(IsObsoleteTLSCipherSuite(
       0xcca9 /* ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256 */));
 }
 
+TEST(CipherSuiteNamesTest, ObsoleteSSLStatusProtocol) {
+  // Modern cipher suite. Note that this can't actually appear with obsolete
+  // cipher suites in a real connection, but we're just trying to test that
+  // ObsoleteSSLStatus() can identify an obsolete protocol individually.
+  uint16_t cipher_suite = 0xc02f; /* TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 */

[davidben, 2016/04/19 17:47:01]

Nit: static const uint16_t kModernCipherSuite = ...;

(Both because it's a constant and so it's clear from reading the call sites that
this cipher is already modern.)

[lgarron, 2016/04/25 23:56:54]

Done. I'll also rename the vars below with that convention.

+
+  // Obsolete
+  EXPECT_EQ(OBSOLETE_SSL_MASK_PROTOCOL,
+            ObsoleteSSLStatus(MakeConnectionStatus(SSL_CONNECTION_VERSION_SSL2,
+                                                   cipher_suite)));
+  EXPECT_EQ(OBSOLETE_SSL_MASK_PROTOCOL,
+            ObsoleteSSLStatus(MakeConnectionStatus(SSL_CONNECTION_VERSION_SSL3,
+                                                   cipher_suite)));
+  EXPECT_EQ(OBSOLETE_SSL_MASK_PROTOCOL,
+            ObsoleteSSLStatus(MakeConnectionStatus(SSL_CONNECTION_VERSION_TLS1,
+                                                   cipher_suite)));
+  EXPECT_EQ(OBSOLETE_SSL_MASK_PROTOCOL,
+            ObsoleteSSLStatus(MakeConnectionStatus(
+                SSL_CONNECTION_VERSION_TLS1_1, cipher_suite)));
+
+  // Modern
+  EXPECT_EQ(OBSOLETE_SSL_NONE,
+            ObsoleteSSLStatus(MakeConnectionStatus(
+                SSL_CONNECTION_VERSION_TLS1_2, cipher_suite)));
+  EXPECT_EQ(OBSOLETE_SSL_NONE, ObsoleteSSLStatus(MakeConnectionStatus(
+                                   SSL_CONNECTION_VERSION_QUIC, cipher_suite)));
+}
+
+TEST(CipherSuiteNamesTest, ObsoleteSSLStatusProtocolAndCipherSuite) {
+  int version_obsolete = SSL_CONNECTION_VERSION_TLS1;
+  int version_modern = SSL_CONNECTION_VERSION_TLS1_2;
+
+  uint16_t cipher_suite_obsolete_ke_obsolete_cipher =
+      0xc5; /* TLS_DH_anon_WITH_CAMELLIA_256_CBC_SHA256 */

[davidben, 2016/04/19 17:47:01]

Nit: Let's do RSA with AES_128_CBC which is a much much more plausible input.
:-)

[lgarron, 2016/04/25 23:56:54]

Done.

+  uint16_t cipher_suite_obsolete_ke_modern_cipher =
+      0x9e; /* TLS_DHE_RSA_WITH_AES_128_GCM_SHA256 */
+  uint16_t cipher_suite_modern_ke_obsolete_cipher =
+      0xc014; /* TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA */
+  uint16_t cipher_suite_modern_ke_modern_cipher =
+      0xc02f; /* TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 */
+
+  // Bogus
+  EXPECT_EQ(
+      OBSOLETE_SSL_MASK_PROTOCOL | OBSOLETE_SSL_MASK_KEY_EXCHANGE |
+          OBSOLETE_SSL_MASK_CIPHER,
+      ObsoleteSSLStatus(MakeConnectionStatus(
+          SSL_CONNECTION_VERSION_UNKNOWN, 0x0 /* TLS_NULL_WITH_NULL_NULL */)));
+
+  // Cartesian combos

[estark, 2016/04/18 11:46:45]

optional nit: you might be able to use TEST_P with INSTANTIATE_TEST_CASE_P to
write this test more easily, here's an example:
https://code.google.com/p/chromium/codesearch#chromium/src/net/quic/quic_conn...

(I think the main advantage in this case is that it would be easier to review
for correctness.)

[lgarron, 2016/04/25 23:56:54]

INSTANTIATE_TEST_CASE_P looks nice, but it seems to be written with the
assumption that you're testing a variety of inputs for the *same* correct
behaviour. In this case, I would want to use gtest's `Combine()` functionality
with different expected outputs.
I could compute the expected output based on the input, but that feels like the
tail wagging the dog.
I don't see an idiomatic way to call INSTANTIATE_TEST_CASE_P on custom tuples of
input without basically replicating these calls, so I hope it's okay if I leave
them as-is?

+  // As above, some of these combinations can't happen in practice.
+  EXPECT_EQ(OBSOLETE_SSL_MASK_PROTOCOL | OBSOLETE_SSL_MASK_KEY_EXCHANGE |
+                OBSOLETE_SSL_MASK_CIPHER,
+            ObsoleteSSLStatus(MakeConnectionStatus(
+                version_obsolete, cipher_suite_obsolete_ke_obsolete_cipher)));
+  EXPECT_EQ(OBSOLETE_SSL_MASK_PROTOCOL | OBSOLETE_SSL_MASK_KEY_EXCHANGE,
+            ObsoleteSSLStatus(MakeConnectionStatus(
+                version_obsolete, cipher_suite_obsolete_ke_modern_cipher)));
+  EXPECT_EQ(OBSOLETE_SSL_MASK_PROTOCOL | OBSOLETE_SSL_MASK_CIPHER,
+            ObsoleteSSLStatus(MakeConnectionStatus(
+                version_obsolete, cipher_suite_modern_ke_obsolete_cipher)));
+  EXPECT_EQ(OBSOLETE_SSL_MASK_PROTOCOL,
+            ObsoleteSSLStatus(MakeConnectionStatus(
+                version_obsolete, cipher_suite_modern_ke_modern_cipher)));
+  EXPECT_EQ(OBSOLETE_SSL_MASK_KEY_EXCHANGE | OBSOLETE_SSL_MASK_CIPHER,
+            ObsoleteSSLStatus(MakeConnectionStatus(
+                version_modern, cipher_suite_obsolete_ke_obsolete_cipher)));
+  EXPECT_EQ(OBSOLETE_SSL_MASK_KEY_EXCHANGE,
+            ObsoleteSSLStatus(MakeConnectionStatus(
+                version_modern, cipher_suite_obsolete_ke_modern_cipher)));
+  EXPECT_EQ(OBSOLETE_SSL_MASK_CIPHER,
+            ObsoleteSSLStatus(MakeConnectionStatus(
+                version_modern, cipher_suite_modern_ke_obsolete_cipher)));
+  EXPECT_EQ(OBSOLETE_SSL_NONE,
+            ObsoleteSSLStatus(MakeConnectionStatus(
+                version_modern, cipher_suite_modern_ke_modern_cipher)));
+}
+
 TEST(CipherSuiteNamesTest, HTTP2CipherSuites) {
   // Picked some random cipher suites.
   EXPECT_FALSE(