Common Name Mismatch Handler For WWW Subdomain Mismatch case

chrome/browser/ssl/ssl_error_classification.h

 // Copyright 2014 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.
 
 #ifndef CHROME_BROWSER_SSL_SSL_ERROR_CLASSIFICATION_H_
 #define CHROME_BROWSER_SSL_SSL_ERROR_CLASSIFICATION_H_
 
 #include <string>
 #include <vector>
 
@@ skipping 54 matching lines @@
   // A function which calculates the severity score when the ssl error is
   // |CERT_AUTHORITY_INVALID|, returns a score between 0.0 and 1.0, higher
   // values being more severe, indicating how severe the certificate's
   // authority invalid error is.
   void InvalidAuthoritySeverityScore();
 
   void RecordUMAStatistics(bool overridable) const;
   void RecordCaptivePortalUMAStatistics(bool overridable) const;
   base::TimeDelta TimePassedSinceExpiry() const;
 
+  // Returns true if the site's hostname differs from one of the DNS
+  // names in the certificate (CN or SANs) only by the presence or
+  // absence of the single-label prefix "www". E.g.: (The first domain
+  // is hostname and the second domain is a DNS name in the certificate)
+  //
+  //     www.example.com ~ example.com -> true
+  //     example.com ~ www.example.com -> true
+  //     www.food.example.com ~ example.com -> false
+  //     mail.example.com ~ example.com -> false
+  static bool GetWWWSubDomainMatch(const std::string& host_name,

[Ryan Sleevi, 2015/08/07 00:14:11]

This is more of a nit, but it seems like this would be better clumped with the
remaining static functions on line 52.

From an API contract level, it makes me somewhat sad to see all of these as
static public, since it feels like it's revealing a lot of the internal details
just for unit tests. Makes me wonder whether it'd be better with just friending
the test fixture in the private section. You shouldn't have to do that in this
CL, though, because we should update the other public static methods
simultaneously.

[Bhanu Dev, 2015/08/07 22:28:48]

Done.

+                                   const std::vector<std::string>& dns_names,
+                                   std::string* www_match_host_name);
+
  private:
   FRIEND_TEST_ALL_PREFIXES(SSLErrorClassificationTest, TestDateInvalidScore);
   FRIEND_TEST_ALL_PREFIXES(SSLErrorClassificationTest, TestNameMismatch);
   FRIEND_TEST_ALL_PREFIXES(SSLErrorClassificationTest,
                            TestHostNameHasKnownTLD);
 
   typedef std::vector<std::string> Tokens;
 
   // Returns true if the hostname has a known Top Level Domain.
   static bool IsHostNameKnownTLD(const std::string& host_name);
 
-  // Returns true if the site's hostname differs from one of the DNS
-  // names in the certificate (CN or SANs) only by the presence or
-  // absence of the single-label prefix "www". E.g.:
-  //
-  //     www.example.com ~ example.com -> true
-  //     example.com ~ www.example.com -> true
-  //     www.food.example.com ~ example.com -> false
-  //     mail.example.com ~ example.com -> false
+  // Returns true if GetWWWSubDomainMatch finds a www mismatch.

[Ryan Sleevi, 2015/08/07 00:14:11]

Why have this function at all then? It seems like it can just be
replaced/inlined, isn't needed as a separate function.

[Bhanu Dev, 2015/08/07 22:28:48]

This method gets the hostname,dns names and calls the static method
GetWWWSubDomainMatch. I thought that it might not be good to have it inline with
definition in header file, tests also use this.

[Ryan Sleevi, 2015/08/07 22:31:11]

I didn't mean inlined in the .h file, I meant just delete this function entirely
and inline what this function does in the one place it's called. That is, now
that you have the static function, it's somewhat questionable whether or not
there's a benefit from having a small function like this that calls said static
function.

   bool IsWWWSubDomainMatch() const;
 
   // Returns true if |child| is a subdomain of any of the |potential_parents|.
   bool NameUnderAnyNames(const Tokens& child,
                         const std::vector<Tokens>& potential_parents) const;
 
   // Returns true if any of the |potential_children| is a subdomain of the
   // |parent|. The inverse case should be treated carefully as this is most
   // likely a MITM attack. We don't want foo.appspot.com to be able to MITM for
   // appspot.com.
@@ skipping 56 matching lines @@
   bool captive_portal_probe_completed_;
   // Did the captive portal probe receive an error or get a non-HTTP response?
   bool captive_portal_no_response_;
   // Was a captive portal detected?
   bool captive_portal_detected_;
 
   content::NotificationRegistrar registrar_;
 };
 
 #endif  // CHROME_BROWSER_SSL_SSL_ERROR_CLASSIFICATION_H_