Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2200)

Unified Diff: net/base/x509_certificate.cc

Issue 8362023: Disallow wildcards from matching top-level registry controlled domains during cert validation. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Require at least 3 host components for unknown/unrecognized domains Created 9 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | net/base/x509_certificate_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: net/base/x509_certificate.cc
diff --git a/net/base/x509_certificate.cc b/net/base/x509_certificate.cc
index 915838897e80ac7c5517c19357aa59f706969dbd..d405642ffc2d87b4ca7f61acdc822e9d45cfe589 100644
--- a/net/base/x509_certificate.cc
+++ b/net/base/x509_certificate.cc
@@ -27,6 +27,7 @@
#include "net/base/net_errors.h"
#include "net/base/net_util.h"
#include "net/base/pem_tokenizer.h"
+#include "net/base/registry_controlled_domain.h"
namespace net {
@@ -507,17 +508,29 @@ bool X509Certificate::VerifyHostname(
// |reference_domain| is the remainder of |host| after the leading host
// component is stripped off, but includes the leading dot e.g.
// "www.f.com" -> ".f.com".
- // If there is no meaningful domain part to |host| (e.g. it contains no dots)
- // then |reference_domain| will be empty.
+ // If there is no meaningful domain part to |host| (e.g. it contains no
+ // dots) then |reference_domain| will be empty.
base::StringPiece reference_host, reference_domain;
SplitOnChar(reference_name, '.', &reference_host, &reference_domain);
bool allow_wildcards = false;
if (!reference_domain.empty()) {
DCHECK(reference_domain.starts_with("."));
- // We required at least 3 components (i.e. 2 dots) as a basic protection
- // against too-broad wild-carding.
- // Also we don't attempt wildcard matching on a purely numerical hostname.
- allow_wildcards = reference_domain.rfind('.') != 0 &&
+
+ // Do not allow wildcards for registry controlled domains, so as to
+ // prevent accepting *.com or *.co.uk as valid presented names. For
+ // domains that are unknown (intranet hosts, new TLDs/gTLDs), require at
+ // least three components total - thus assuming all TLDs are
wtc 2011/10/21 23:44:51 Nit: remove "total".
+ // registry-controlled.
joth 2011/10/21 09:26:51 ah gotcha. Took me a little while to connect this
Chris Palmer 2011/10/21 17:28:36 Regarding *.intra: I understand this to be perfect
Ryan Sleevi 2011/10/21 22:56:35 joth: I mentioned this to Palmer yesterday, and is
+ size_t registry_length =
+ RegistryControlledDomainService::GetRegistryLength(reference_name,
+ true);
joth 2011/10/21 09:26:51 GetRegistryLength can return std::string::npos too
Ryan Sleevi 2011/10/21 22:56:35 Where do you see that? It's documented to return 0
joth 2011/10/24 09:43:39 http://codesearch.google.com/codesearch#OAMlx_jo-c
+ // Subtract 1 to account for the leading dot in |reference_domain|.
+ bool is_registry_controlled = registry_length != 0 &&
+ registry_length == (reference_domain.size() - 1);
+
+ // Additionally, do not attempt wildcard matching for purely numeric
+ // hostnames.
+ allow_wildcards = !is_registry_controlled &&
reference_name.find_first_not_of("0123456789.") != std::string::npos;
}
« no previous file with comments | « no previous file | net/base/x509_certificate_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698