Find reasons for the SSL common name invalid error.

chrome/browser/ssl/ssl_error_classification.h

Index: chrome/browser/ssl/ssl_error_classification.h
diff --git a/chrome/browser/ssl/ssl_error_classification.h b/chrome/browser/ssl/ssl_error_classification.h
index 87851ec60f72ee373063c7b963705c8ae21ee0a7..93effdcaa0f339a684037800b2aa198292a170e6 100644
--- a/chrome/browser/ssl/ssl_error_classification.h
+++ b/chrome/browser/ssl/ssl_error_classification.h
@@ -5,18 +5,23 @@
 #ifndef CHROME_BROWSER_SSL_SSL_ERROR_CLASSIFICATION_H_
 #define CHROME_BROWSER_SSL_SSL_ERROR_CLASSIFICATION_H_
 
+#include <string>
+#include <vector>
+
 #include "base/time/time.h"
 #include "net/cert/x509_certificate.h"
+#include "url/gurl.h"
 
 // This class calculates the severity scores for the different type of SSL
 // errors.
 class SSLErrorClassification {
  public:
   SSLErrorClassification(base::Time current_time,
+                         const GURL& url,
                          const net::X509Certificate& cert);
   ~SSLErrorClassification();
 
-  // This method checks whether the user clock is in the past or not.
+  // This method checks whether the system time is in the past.
   static bool IsUserClockInThePast(base::Time time_now);
 
   // This method checks whether the system time is too far in the future or
@@ -29,17 +34,57 @@ class SSLErrorClassification {
   // CERT_DATE_INVALID.
   float InvalidDateSeverityScore() const;
 
-  static void RecordUMAStatistics(bool overridable);
+  // A method which calculates the severity score when the ssl error is

[palmer, 2014/07/31 22:40:30]

// When the SSL error is |CERT_COMMON_NAME_INVALID|, returns
// a severity score between 0.0 and 1.0 (higher values being
// more severe).

(Does the implementation handle the case when the SSL error is not
|CERT_COMMON_NAME_INVALID|? Maybe just a DCHECK?)

[radhikabhar, 2014/08/01 23:06:57]

Done.

+  // CERT_COMMON_NAME_INVALID.
+  float InvalidCommonNameSeverityScore() const;
+
+  void RecordUMAStatistics(bool overridable, int cert_error);
   base::TimeDelta TimePassedSinceExpiry() const;
 
  private:
-  FRIEND_TEST_ALL_PREFIXES(SSLErrorClassification, TestDateInvalidScore);
+  FRIEND_TEST_ALL_PREFIXES(SSLErrorClassificationTest, TestDateInvalidScore);
+  FRIEND_TEST_ALL_PREFIXES(SSLErrorClassificationTest, TestNameMismatch);
+  FRIEND_TEST_ALL_PREFIXES(SSLErrorClassificationTest,
+                           TestHostNameHasKnownTLD);
+
+  typedef std::vector<std::string> Tokens;
+
+  // This method checks whether the hostname has a known Top Level Domain or
+  // not.
+  static bool IsHostNameKnownTLD(const std::string& host_name);

[palmer, 2014/07/31 22:40:30]

// Returns true if |hostname| has a known top-level domain.
static bool HasKnownTLD(const std::string& hostname);

(In technical English, "hostname" tends to be 1 word — like "nodename", but
unlike "domain name". English is weird.)

[radhikabhar, 2014/08/01 23:06:57]

Done.

+
+  // A certificate for bank.com does not work for www.bank.com and vice versa.
+  // This method checks whteher the error is caused because of "www" difference
+  // between the host name and any DNS name given in the CN or SAN fields of
+  // the SSL certificate.
+  bool IsWWWSubDomainMatch() const;

[palmer, 2014/07/31 22:40:30]

This should be more concise, and maybe illustrate it with examples.

// 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

[radhikabhar, 2014/08/01 23:06:57]

Done.

+
+  // 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 any of

[palmer, 2014/07/31 22:40:30]

"...is a subdomain of the |parent|." (I.e. remove "any of".)

[radhikabhar, 2014/08/01 23:06:57]

Done.

+  // the |parent|.
+  bool AnyNamesUnderName(const std::vector<Tokens>& potential_children,
+                         const Tokens& parent) const;
+
+  // This method checks whether the hostname is too broad for the scope of a
+  // wildcard certificate or not. For e.g. it returns true if the host name of
+  // the URL is "a.b.example.com" and the DNS name in the CN or SAN field of the
+  // SSL certificate is "*.example.com". But, it returns false for the hostname
+  // "b.example.com".
+  bool IsSubDomainOutsideWildcard(const Tokens& host_name_tokens) const;

[palmer, 2014/07/31 22:40:30]

// Returns true if |hostname| is too broad for the scope of a wildcard
// certificate. E.g.:
//
//     a.b.example.com ~ *.example.com --> true
//     b.example.com ~ *.example.com --> false
bool IsSubDomainOutsideWildcard(const Tokens& hostname);

[radhikabhar, 2014/08/01 23:06:57]

Done.

 
   float CalculateScoreTimePassedSinceExpiry() const;
 
+  std::vector<Tokens> GetTokenizedDNSNames(
+      std::vector<std::string>& dns_names) const;

[palmer, 2014/07/31 22:40:30]

|dns_names| can be a const &?

Can this function be static? (Does it need to access any internal state of
|this|?)

[radhikabhar, 2014/08/01 23:06:57]

Done.

+
   // This stores the current time.
   base::Time current_time_;
 
+  const GURL& request_url_;
+
   // This stores the certificate.
   const net::X509Certificate& cert_;
 };