Prepare security bullets for Android: add issuer and change connection details.

This is the first of a series of CLs to expose the GetSecurityStyle()
explanations (currently only used in the DevTools Security panel overview,
untranslated) in the Android connection info popup (translated). It does two
things:

- Mention the issuer in the certificate bullet (for feature parity with the old
  Android connection info popup).
- Swap TLS connection settings values with their descriptions to avoid phrase
  substitution within a clause (for straightforward translation).

The relevant strings retain `translateable="false"` until a future CL, due to
non-trivial runtime logic about when to translate the strings.

BUG=587255, 657299
TEST=Open the DevTools Security panel, visit https://google.com, and verify that there is a green bullet stating "The connection to this site is using a valid, trusted server certificate issued by Google Internet Authority G2."

Review-Url: https://codereview.chromium.org/2951043002
Cr-Commit-Position: refs/heads/master@{#481806}

chrome/browser/ssl/security_state_tab_helper_browser_tests.cc

@@ -68,6 +68,8 @@ enum CertificateStatus { VALID_CERTIFICATE, INVALID_CERTIFICATE };
 const base::FilePath::CharType kDocRoot[] =
     FILE_PATH_LITERAL("chrome/test/data");
 
+const std::string kTestCertificateIssuerName = "Test Root CA";
+
 // Inject a script into every frame in the page. Used by tests that check for
 // visible password fields to wait for notifications about these
 // fields. Notifications about visible password fields are queued at the end of
@@ -166,11 +168,14 @@ void CheckSecureExplanations(
   ASSERT_EQ(cert_status == VALID_CERTIFICATE ? 2u : 1u,
             secure_explanations.size());
   if (cert_status == VALID_CERTIFICATE) {
+    ASSERT_EQ(kTestCertificateIssuerName,
+              expected_cert->issuer().GetDisplayName());
     EXPECT_EQ(l10n_util::GetStringUTF8(IDS_VALID_SERVER_CERTIFICATE),
               secure_explanations[0].summary);
-    EXPECT_EQ(
-        l10n_util::GetStringUTF8(IDS_VALID_SERVER_CERTIFICATE_DESCRIPTION),
-        secure_explanations[0].description);
+    EXPECT_EQ(l10n_util::GetStringFUTF8(
+                  IDS_VALID_SERVER_CERTIFICATE_DESCRIPTION,
+                  base::UTF8ToUTF16(kTestCertificateIssuerName)),
+              secure_explanations[0].description);
     net::X509Certificate* cert = browser->tab_strip_model()
                                      ->GetActiveWebContents()
                                      ->GetController()
@@ -2120,16 +2125,16 @@ IN_PROC_BROWSER_TEST_F(
   // Populate description string replacement with values corresponding
   // to test constants.
   std::vector<base::string16> description_replacements;
-  description_replacements.push_back(
-      l10n_util::GetStringUTF16(IDS_SSL_AN_OBSOLETE_PROTOCOL));
   description_replacements.push_back(base::ASCIIToUTF16("TLS 1.1"));
   description_replacements.push_back(
-      l10n_util::GetStringUTF16(IDS_SSL_A_STRONG_KEY_EXCHANGE));
+      l10n_util::GetStringUTF16(IDS_SSL_AN_OBSOLETE_PROTOCOL));
   description_replacements.push_back(base::ASCIIToUTF16("ECDHE_RSA"));
   description_replacements.push_back(
-      l10n_util::GetStringUTF16(IDS_SSL_AN_OBSOLETE_CIPHER));
+      l10n_util::GetStringUTF16(IDS_SSL_A_STRONG_KEY_EXCHANGE));
   description_replacements.push_back(
       base::ASCIIToUTF16("AES_128_CBC with HMAC-SHA1"));
+  description_replacements.push_back(
+      l10n_util::GetStringUTF16(IDS_SSL_AN_OBSOLETE_CIPHER));
   base::string16 obsolete_description = l10n_util::GetStringFUTF16(
       IDS_OBSOLETE_SSL_DESCRIPTION, description_replacements, nullptr);
 

components/security_state/content/content_utils.cc

@@ -112,19 +112,19 @@ void AddConnectionExplanation(
   str_id = (status & net::OBSOLETE_SSL_MASK_PROTOCOL)
                ? IDS_SSL_AN_OBSOLETE_PROTOCOL
                : IDS_SSL_A_STRONG_PROTOCOL;
-  description_replacements.push_back(l10n_util::GetStringUTF16(str_id));
   description_replacements.push_back(protocol_name);
+  description_replacements.push_back(l10n_util::GetStringUTF16(str_id));
 
   str_id = (status & net::OBSOLETE_SSL_MASK_KEY_EXCHANGE)
                ? IDS_SSL_AN_OBSOLETE_KEY_EXCHANGE
                : IDS_SSL_A_STRONG_KEY_EXCHANGE;
-  description_replacements.push_back(l10n_util::GetStringUTF16(str_id));
   description_replacements.push_back(key_exchange_name);
+  description_replacements.push_back(l10n_util::GetStringUTF16(str_id));
 
   str_id = (status & net::OBSOLETE_SSL_MASK_CIPHER) ? IDS_SSL_AN_OBSOLETE_CIPHER
                                                     : IDS_SSL_A_STRONG_CIPHER;
-  description_replacements.push_back(l10n_util::GetStringUTF16(str_id));
   description_replacements.push_back(cipher_name);
+  description_replacements.push_back(l10n_util::GetStringUTF16(str_id));
 
   security_style_explanations->info_explanations.push_back(
       content::SecurityStyleExplanation(
@@ -297,12 +297,26 @@ blink::WebSecurityStyle GetSecurityStyle(
   } else {
     // If the certificate does not have errors and is not using SHA1, then add
     // an explanation that the certificate is valid.
+
+    base::string16 issuer_name;
+    if (security_info.certificate) {
+      // This results in the empty string if there is no relevant display name.
+      issuer_name = base::UTF8ToUTF16(
+          security_info.certificate->issuer().GetDisplayName());
+    } else {
+      issuer_name = base::string16();
+    }
+    if (issuer_name.empty()) {
+      issuer_name.assign(
+          l10n_util::GetStringUTF16(IDS_PAGE_INFO_SECURITY_TAB_UNKNOWN_PARTY));
+    }
+
     if (!security_info.sha1_in_chain) {
       security_style_explanations->secure_explanations.push_back(
           content::SecurityStyleExplanation(
               l10n_util::GetStringUTF8(IDS_VALID_SERVER_CERTIFICATE),
-              l10n_util::GetStringUTF8(
-                  IDS_VALID_SERVER_CERTIFICATE_DESCRIPTION),
+              l10n_util::GetStringFUTF8(
+                  IDS_VALID_SERVER_CERTIFICATE_DESCRIPTION, issuer_name),
               !!security_info.certificate));
     }
   }

components/security_state/content/content_utils_unittest.cc

@@ -164,7 +164,7 @@ bool FindSecurityStyleExplanation(
   return false;
 }
 
-// Test that connection explanations are formated as expected. Note the strings
+// Test that connection explanations are formatted as expected. Note the strings
 // are not translated and so will be the same in any locale.
 TEST(SecurityStateContentUtilsTest, ConnectionExplanation) {
   // Test a modern configuration with a key exchange group.
@@ -185,9 +185,9 @@ TEST(SecurityStateContentUtilsTest, ConnectionExplanation) {
     ASSERT_TRUE(FindSecurityStyleExplanation(
         explanations.secure_explanations, "Secure connection", &explanation));
     EXPECT_EQ(
-        "The connection to this site is encrypted and authenticated using a "
-        "strong protocol (TLS 1.2), a strong key exchange (ECDHE_RSA with "
-        "X25519), and a strong cipher (CHACHA20_POLY1305).",
+        "The connection to this site is encrypted and authenticated using TLS "
+        "1.2 (a strong protocol), ECDHE_RSA with X25519 (a strong key "
+        "exchange), and CHACHA20_POLY1305 (a strong cipher).",
         explanation.description);
   }
 
@@ -201,9 +201,9 @@ TEST(SecurityStateContentUtilsTest, ConnectionExplanation) {
     ASSERT_TRUE(FindSecurityStyleExplanation(
         explanations.secure_explanations, "Secure connection", &explanation));
     EXPECT_EQ(
-        "The connection to this site is encrypted and authenticated using a "
-        "strong protocol (TLS 1.2), a strong key exchange (ECDHE_RSA), and a "
-        "strong cipher (CHACHA20_POLY1305).",
+        "The connection to this site is encrypted and authenticated using TLS "
+        "1.2 (a strong protocol), ECDHE_RSA (a strong key exchange), and "
+        "CHACHA20_POLY1305 (a strong cipher).",
         explanation.description);
   }
 
@@ -220,9 +220,38 @@ TEST(SecurityStateContentUtilsTest, ConnectionExplanation) {
     ASSERT_TRUE(FindSecurityStyleExplanation(
         explanations.secure_explanations, "Secure connection", &explanation));
     EXPECT_EQ(
-        "The connection to this site is encrypted and authenticated using a "
-        "strong protocol (TLS 1.3), a strong key exchange (X25519), and a "
-        "strong cipher (AES_128_GCM).",
+        "The connection to this site is encrypted and authenticated using TLS "
+        "1.3 (a strong protocol), X25519 (a strong key exchange), and "
+        "AES_128_GCM (a strong cipher).",
+        explanation.description);
+  }
+}
+
+// Test that obsolete connection explanations are formatted as expected.
+TEST(SecurityStateContentUtilsTest, ObsoleteConnectionExplanation) {
+  security_state::SecurityInfo security_info;
+  security_info.cert_status = net::CERT_STATUS_UNABLE_TO_CHECK_REVOCATION;
+  security_info.scheme_is_cryptographic = true;
+  net::SSLConnectionStatusSetCipherSuite(
+      0xc013 /* TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA */,
+      &security_info.connection_status);
+  net::SSLConnectionStatusSetVersion(net::SSL_CONNECTION_VERSION_TLS1_2,
+                                     &security_info.connection_status);
+  security_info.key_exchange_group = 29;  // X25519
+  security_info.obsolete_ssl_status =
+      net::ObsoleteSSLMask::OBSOLETE_SSL_MASK_CIPHER;
+
+  {
+    content::SecurityStyleExplanations explanations;
+    GetSecurityStyle(security_info, &explanations);
+    content::SecurityStyleExplanation explanation;
+    ASSERT_TRUE(FindSecurityStyleExplanation(explanations.info_explanations,
+                                             "Obsolete connection settings",
+                                             &explanation));
+    EXPECT_EQ(
+        "The connection to this site uses TLS 1.2 (a strong protocol), "
+        "ECDHE_RSA with X25519 (a strong key exchange), and AES_128_CBC with "
+        "HMAC-SHA1 (an obsolete cipher).",
         explanation.description);
   }
 }

components/security_state_strings.grdp

@@ -38,7 +38,7 @@
     Valid certificate
   </message>
   <message name="IDS_VALID_SERVER_CERTIFICATE_DESCRIPTION" desc="Description of a site that has a valid server certificate." translateable="false">
-    The connection to this site is using a valid, trusted server certificate.
+    The connection to this site is using a valid, trusted server certificate issued by <ph name="ISSUER">$1<ex>Let's Encrypt Authority X3</ex></ph>.
   </message>
   <message name="IDS_STRONG_SSL_SUMMARY" desc="Summary phrase for a site that uses a modern, secure TLS protocol and cipher." translateable="false">
     Secure connection
@@ -50,13 +50,13 @@
     Public-Key-Pinning was bypassed by a local root certificate.
   </message>
   <message name="IDS_STRONG_SSL_DESCRIPTION" desc="Description of a site that uses a modern, secure TLS protocol and cipher." translateable="false">
-    The connection to this site is encrypted and authenticated using a strong protocol (<ph name="PROTOCOL_VERSION">$1<ex>TLS 1.2</ex></ph>), a strong key exchange (<ph name="KEY_EXCHANGE">$2<ex>ECDHE_RSA</ex></ph>), and a strong cipher (<ph name="CIPHER_SUTE">$3<ex>AES_128_GCM</ex></ph>).
+    The connection to this site is encrypted and authenticated using <ph name="PROTOCOL_VERSION">$1<ex>TLS 1.2</ex></ph> (a strong protocol), <ph name="KEY_EXCHANGE">$2<ex>ECDHE_RSA</ex></ph> (a strong key exchange), and <ph name="CIPHER_SUTE">$3<ex>AES_128_GCM</ex></ph> (a strong cipher).
   </message>
   <message name="IDS_OBSOLETE_SSL_SUMMARY" desc="Summary phrase for a site that uses an outdated SSL settings (protocol, key exchange, or cipher)." translateable="false">
     Obsolete connection settings
   </message>
   <message name="IDS_OBSOLETE_SSL_DESCRIPTION" desc="Description of a site that uses an outdated TLS protocol or cipher." translateable="false">
-    The connection to this site uses <ph name="A_PROTOCOL">$1<ex>an obsolete protocol</ex></ph> (<ph name="PROTOCOL">$2<ex>TLS 1.0</ex></ph>), <ph name="A_KEY_EXCHANGE">$3<ex>an obsolete key exchange</ex></ph> (<ph name="KEY_EXCHANGE">$4<ex>ECDHE_RSA</ex></ph>), and <ph name="A_CIPHER">$5<ex>an obsolete cipher</ex></ph> (<ph name="CIPHER">$6<ex>AES_256_CBC with HMAC-SHA1</ex></ph>).
+    The connection to this site uses <ph name="PROTOCOL">$1<ex>TLS 1.0</ex></ph> (<ph name="A_PROTOCOL">$2<ex>an obsolete protocol</ex></ph>), <ph name="KEY_EXCHANGE">$3<ex>ECDHE_RSA</ex></ph> (<ph name="A_KEY_EXCHANGE">$4<ex>an obsolete key exchange</ex></ph>), and <ph name="CIPHER">$5<ex>AES_256_CBC with HMAC-SHA1</ex></ph> (<ph name="A_CIPHER">$6<ex>an obsolete cipher</ex></ph>).
   </message>
   <message name="IDS_CIPHER_WITH_MAC" desc="Description of an SSL cipher that contains a separate (bulk) cipher and MAC." translateable="false">
     <ph name="CIPHER">$1<ex>AES_256_CBC</ex></ph> with <ph name="MAC">$2<ex>HMAC-SHA1</ex></ph>