Validate origins when generating subdomain tokens

tools/origin_trials/validate_subdomain_origin/check_subdomain_origin.cc

+// Copyright (c) 2016 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.
+
+#include <stddef.h>
+#include <stdint.h>
+
+#include <iostream>
+#include <string>
+
+#include "base/command_line.h"
+#include "base/memory/ptr_util.h"
+#include "base/strings/string_piece.h"
+#include "base/strings/string_util.h"
+#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
+#include "net/base/url_util.h"
+#include "url/gurl.h"
+
+using net::registry_controlled_domains::GetCanonicalHostRegistryLength;
+using net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES;
+using net::registry_controlled_domains::EXCLUDE_UNKNOWN_REGISTRIES;
+using net::registry_controlled_domains::INCLUDE_UNKNOWN_REGISTRIES;
+
+// Return codes used by this utility.
+static const int kStatusValid = 0;
+static const int kStatusInvalidOrigin = 1;
+static const int kStatusInPublicSuffixList = 2;
+static const int kStatusError = 3;
+
+void PrintHelp() {
+  std::cerr
+      << "Usage:\n"
+         "  validate_subdomain_origin <origin>\n"
+         "    Checks that the origin can be used in a token that matches\n"
+         "    subdomains, returning 0 when the origin is valid for such use.\n"
+         "    The origin may be specified as an url (e.g. "
+         "'https://example.com'),\n"
+         "    or as bare hostname (e.g. 'example.com').\n";
+}
+
+int CheckOrigin(const base::CommandLine::StringType origin) {
+  base::StringPiece host;
+  std::unique_ptr<std::string> canon_host = nullptr;
+
+  // Validate the origin, which may be provided as an url (with scheme prefix),
+  // or just as a hostname. Regardless of format, if the origin is identified
+  // as an IP address, that is valid for subdomain tokens.
+  GURL gurl(origin);
+  if (gurl.is_valid()) {
+    if (gurl.HostIsIPAddress()) {
+      return kStatusValid;
+    }
+    host = gurl.host_piece();
+  } else {
+    // Check if it looks like an url, that is not valid

[iclelland, 2016/11/02 15:25:47]

nit: drop the comma

[chasej, 2016/11/03 19:23:40]

Done.

+    if (base::StartsWith(origin, "http",

[iclelland, 2016/11/02 15:25:47]

Less of a nit: Why does this utility care whether it's an http prefix
specifically? I could imagine requiring 'https', and failing on 'http','ftp',
'gopher', 'mailto', or anything else, but it seems strange to only fail on http.

More broadly, though, does this utility even need to worry about that? It it's
not a secure origin, then the generate_token.py script will refuse to generate
it for its own reasons. This utility could easily be reused in other contexts
where secure origin isn't a requirement; it probably shouldn't be trying to be
enforce it.

[iclelland, 2016/11/02 15:28:55]

Actually, I misunderstood -- I tried to get this to trigger by using various
protocols, and it doesn't. I see that it only comes into play when the origin
given can't be parsed as a full URL, such as when a hostname is passed.

Unfortunately, this means that it also fails to pass legitimate domains, such as
"httpd.apache.org", because of the "http" prefix.

Is this test actually required, or is it just trying to provide helpful errors
for the operator? If we don't need it, I'd suggest getting rid of it and letting
bad URLs fail on their own.

[chasej, 2016/11/03 19:23:40]

I was trying to address the scenario where GURL.is_valid() returns false, and
the origin should be considered invalid. CanonicalizeHost() doesn't catch many
of those scenarios (e.g. it simply escapes invalid domain chars).

Upon further testing, there's other inputs that I would consider invalid for
token generation purposes, which GURL will happily report as valid.

I'm now thinking that the utility should give on trying to check for malformed
(or otherwise "invalid" origins). Instead, it is up to the caller to ensure that
the origin meets any other validity rules. This utility will simply check if the
origin can be used for subdomain matching. With that approach, I could get rid
of the check for the http|https prefix entirely.

Thoughts?

[iclelland, 2016/11/03 19:47:55]

I agree that if we can drop this check, then we should. The calling script
already has logic to make sure that the string is shaped like some sort of
origin or hostname.

+                         base::CompareCase::INSENSITIVE_ASCII)) {
+      std::cerr << "validate_subdomain_origin: Invalid url - " << origin
+                << std::endl;
+      return kStatusInvalidOrigin;
+    }
+    // Doesn't look like an url, try the origin as a hostname
+    url::CanonHostInfo host_info;
+    canon_host = base::MakeUnique<std::string>(
+        net::CanonicalizeHost(origin, &host_info));
+    if (canon_host->empty()) {
+      std::cerr << "validate_subdomain_origin: Invalid origin - " << origin
+                << std::endl;
+      return kStatusInvalidOrigin;
+    }
+    if (host_info.IsIPAddress()) {
+      return kStatusValid;
+    }
+    host.set(canon_host->c_str());

[iclelland, 2016/11/02 15:25:47]

What happens to this string after this block ends? canon_host is free()d, but
I'm not sure if that invalidates the pointer returned by c_str(), which is just
copied into host.ptr_ by the set() method.

[chasej, 2016/11/03 19:23:40]

canon_host not is declared in this block - it's scoped to the entire function.
That's why it's assigned via MakeUnique. My expectation is that the string lives
until the end of the function, and thus it's safe to assign to host inside the
block.

[iclelland, 2016/11/03 19:47:55]

Ahh, you're right; that makes sense. Thanks.

+  }
+
+  size_t registry_length = GetCanonicalHostRegistryLength(
+      host, INCLUDE_UNKNOWN_REGISTRIES, INCLUDE_PRIVATE_REGISTRIES);
+
+  if (registry_length > 0) {
+    // Host has at least one subcomponent (e.g. a.b), and the host is not just
+    // a registry (e.g. co.uk).
+    return kStatusValid;
+  }
+
+  // If registry length is 0, then the host may be a registry, or it has no
+  // subcomponents. If there are subcomponents, the host must be a registry,
+  // which makes it invalid.
+  if (host.find('.') != std::string::npos) {
+    std::cerr << "validate_subdomain_origin: Origin in Public Suffix List - "
+              << host << std::endl;
+    return kStatusInPublicSuffixList;
+  }
+
+  // There are no subcomponents, but still don't know if this a registry
+  // (e.g. host = "com"), or a private/internal address (e.g. host = "bar").
+  // Test by adding a subcomponent, and re-checking the registry. In this case,
+  // exclude unknown registries. That means that "prefix.com" will match the
+  // "com" registry, and return a non-zero length. Conversely, "prefix.bar" will
+  // not match any known registry, and return a zero length.
+  std::string test_host("prefix.");
+  test_host.append(host.as_string());
+
+  size_t test_registry_length = GetCanonicalHostRegistryLength(
+      test_host, EXCLUDE_UNKNOWN_REGISTRIES, INCLUDE_PRIVATE_REGISTRIES);
+  if (test_registry_length > 0) {
+    std::cerr << "validate_subdomain_origin: Origin in Public Suffix List - "

[iclelland, 2016/11/02 15:25:47]

Nit -- I'd prefer a '-v' or '-q' option to control whether there is any output
at all, and be consistent with outputting a message whether or not the domain
passes.

If I could have a pony, I'd have the utility print a message when called with no
parameters (by someone just using it from the command line) and have an option
for the script to make it silent, so the script can decide on the format for the
error output, if any.

[chasej, 2016/11/03 19:23:40]

Done. You can have a pony, but just this one time ;-)

+              << host << std::endl;
+    return kStatusInPublicSuffixList;
+  }
+
+  return kStatusValid;
+}
+
+int main(int argc, const char* argv[]) {
+  base::CommandLine::Init(argc, argv);
+  const base::CommandLine& parsed_command_line =
+      *base::CommandLine::ForCurrentProcess();
+
+  const base::CommandLine::StringVector& args = parsed_command_line.GetArgs();
+  if (args.size() == 1) {
+    return CheckOrigin(args[0]);
+  }
+
+  PrintHelp();
+  return kStatusError;
+}