HTTPS rdata parsing

Targeting draft-ietf-dnsop-svcb-httpssvc-03.

Bug: 1138620
Change-Id: I5198bb2aebff3d4aebfbc4e457965b0021991787
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2503490Reviewed-by: Dan McArdle <dmcardle@chromium.org>
Commit-Queue: Eric Orth <ericorth@chromium.org>
Cr-Commit-Position: refs/heads/master@{#822162}

net/dns/BUILD.gn

@@ -65,6 +65,7 @@ source_set("dns") {
       "host_resolver_mdns_task.h",
       "host_resolver_proc.cc",
       "host_resolver_proc.h",
+      "https_record_rdata.cc",
       "httpssvc_metrics.cc",
       "httpssvc_metrics.h",
       "mapped_host_resolver.cc",
@@ -313,6 +314,7 @@ source_set("dns_client") {
       "dns_client.h",
       "dns_response.h",
       "dns_transaction.h",
+      "https_record_rdata.h",
       "record_parsed.h",
       "record_rdata.h",
     ]
@@ -396,6 +398,7 @@ source_set("tests") {
     "dns_util_unittest.cc",
     "host_cache_unittest.cc",
     "host_resolver_manager_unittest.cc",
+    "https_record_rdata_unittest.cc",
     "httpssvc_metrics_unittest.cc",
     "mapped_host_resolver_unittest.cc",
     "record_parsed_unittest.cc",

net/dns/host_resolver_manager_unittest.cc

@@ -8838,8 +8838,8 @@ TEST_F(HostResolverManagerDnsTest, HttpsQuery) {
   const std::string kName = "https.test";
 
   MockDnsClientRuleList rules;
-  std::vector<DnsResourceRecord> records = {
-      BuildTestDnsRecord(kName, dns_protocol::kTypeHttps, "" /* rdata */)};
+  std::vector<DnsResourceRecord> records = {BuildTestDnsRecord(
+      kName, dns_protocol::kTypeHttps, "fake rdata" /* rdata */)};
   rules.emplace_back(kName, dns_protocol::kTypeHttps, false /* secure */,
                      MockDnsClientRule::Result(BuildTestDnsResponse(
                          kName, dns_protocol::kTypeHttps, records)),
@@ -9037,7 +9037,7 @@ TEST_F(HostResolverManagerDnsTest, HttpsQuery_MismatchedName) {
 
   MockDnsClientRuleList rules;
   std::vector<DnsResourceRecord> records = {BuildTestDnsRecord(
-      "different.test", dns_protocol::kTypeHttps, "" /* rdata */)};
+      "different.test", dns_protocol::kTypeHttps, "fake rdata" /* rdata */)};
   rules.emplace_back(kName, dns_protocol::kTypeHttps, false /* secure */,
                      MockDnsClientRule::Result(BuildTestDnsResponse(
                          kName, dns_protocol::kTypeHttps, records)),
@@ -9101,8 +9101,8 @@ TEST_F(HostResolverManagerDnsTest, HttpsDnsQuery) {
   const std::string kName = "https.test";
 
   MockDnsClientRuleList rules;
-  std::vector<DnsResourceRecord> records = {
-      BuildTestDnsRecord(kName, dns_protocol::kTypeHttps, "" /* rdata */)};
+  std::vector<DnsResourceRecord> records = {BuildTestDnsRecord(
+      kName, dns_protocol::kTypeHttps, "fake rdata" /* rdata */)};
   rules.emplace_back(kName, dns_protocol::kTypeHttps, false /* secure */,
                      MockDnsClientRule::Result(BuildTestDnsResponse(
                          kName, dns_protocol::kTypeHttps, records)),
@@ -9134,8 +9134,8 @@ TEST_F(HostResolverManagerDnsTest, HttpsInAddressQuery) {
                                {"DnsHttpssvcExperimentDomains", kName}});
 
   MockDnsClientRuleList rules;
-  std::vector<DnsResourceRecord> records = {
-      BuildTestDnsRecord(kName, dns_protocol::kTypeHttps, "" /* rdata */)};
+  std::vector<DnsResourceRecord> records = {BuildTestDnsRecord(
+      kName, dns_protocol::kTypeHttps, "fake rdata" /* rdata */)};
   rules.emplace_back(kName, dns_protocol::kTypeHttps, true /* secure */,
                      MockDnsClientRule::Result(BuildTestDnsResponse(
                          kName, dns_protocol::kTypeHttps, records)),
@@ -9207,8 +9207,8 @@ TEST_F(HostResolverManagerDnsTest, HttpsInAddressQuery_HttpsOnly) {
                                {"DnsHttpssvcExperimentDomains", kName}});
 
   MockDnsClientRuleList rules;
-  std::vector<DnsResourceRecord> records = {
-      BuildTestDnsRecord(kName, dns_protocol::kTypeHttps, "" /* rdata */)};
+  std::vector<DnsResourceRecord> records = {BuildTestDnsRecord(
+      kName, dns_protocol::kTypeHttps, "fake rdata" /* rdata */)};
   rules.emplace_back(kName, dns_protocol::kTypeHttps, true /* secure */,
                      MockDnsClientRule::Result(BuildTestDnsResponse(
                          kName, dns_protocol::kTypeHttps, records)),
@@ -9245,8 +9245,8 @@ TEST_F(HostResolverManagerDnsTest, HttpsInAddressQuery_AddressError) {
                                {"DnsHttpssvcExperimentDomains", kName}});
 
   MockDnsClientRuleList rules;
-  std::vector<DnsResourceRecord> records = {
-      BuildTestDnsRecord(kName, dns_protocol::kTypeHttps, "" /* rdata */)};
+  std::vector<DnsResourceRecord> records = {BuildTestDnsRecord(
+      kName, dns_protocol::kTypeHttps, "fake rdata" /* rdata */)};
   rules.emplace_back(kName, dns_protocol::kTypeHttps, true /* secure */,
                      MockDnsClientRule::Result(BuildTestDnsResponse(
                          kName, dns_protocol::kTypeHttps, records)),
@@ -9353,8 +9353,8 @@ TEST_F(HostResolverManagerDnsTest, HttpsInAddressQuery_HttpsLast) {
                                {"DnsHttpssvcExperimentDomains", kName}});
 
   MockDnsClientRuleList rules;
-  std::vector<DnsResourceRecord> records = {
-      BuildTestDnsRecord(kName, dns_protocol::kTypeHttps, "" /* rdata */)};
+  std::vector<DnsResourceRecord> records = {BuildTestDnsRecord(
+      kName, dns_protocol::kTypeHttps, "fake rdata" /* rdata */)};
   rules.emplace_back(kName, dns_protocol::kTypeHttps, true /* secure */,
                      MockDnsClientRule::Result(BuildTestDnsResponse(
                          kName, dns_protocol::kTypeHttps, records)),
@@ -9398,8 +9398,8 @@ TEST_F(HostResolverManagerDnsTest, HttpsInAddressQuery_AddressesLast) {
                                {"DnsHttpssvcExperimentDomains", kName}});
 
   MockDnsClientRuleList rules;
-  std::vector<DnsResourceRecord> records = {
-      BuildTestDnsRecord(kName, dns_protocol::kTypeHttps, "" /* rdata */)};
+  std::vector<DnsResourceRecord> records = {BuildTestDnsRecord(
+      kName, dns_protocol::kTypeHttps, "fake rdata" /* rdata */)};
   rules.emplace_back(kName, dns_protocol::kTypeHttps, true /* secure */,
                      MockDnsClientRule::Result(BuildTestDnsResponse(
                          kName, dns_protocol::kTypeHttps, records)),

net/dns/https_record_rdata.h

+// Copyright 2020 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 NET_DNS_HTTPS_RECORD_RDATA_H_
+#define NET_DNS_HTTPS_RECORD_RDATA_H_
+
+#include <stdint.h>
+#include <map>
+#include <memory>
+#include <string>
+#include <vector>
+
+#include "base/optional.h"
+#include "base/strings/string_piece.h"
+#include "net/base/ip_address.h"
+#include "net/base/net_export.h"
+#include "net/dns/public/dns_protocol.h"
+#include "net/dns/record_rdata.h"
+
+namespace net {
+
+class AliasFormHttpsRecordRdata;
+class ServiceFormHttpsRecordRdata;
+
+class NET_EXPORT_PRIVATE HttpsRecordRdata : public RecordRdata {
+ public:
+  static const uint16_t kType = dns_protocol::kTypeHttps;
+
+  static std::unique_ptr<HttpsRecordRdata> Parse(base::StringPiece data);
+
+  HttpsRecordRdata(const HttpsRecordRdata& rdata) = delete;
+  HttpsRecordRdata& operator=(const HttpsRecordRdata& rdata) = delete;
+
+  ~HttpsRecordRdata() override;
+
+  bool IsEqual(const RecordRdata* other) const override;
+  virtual bool IsEqual(const HttpsRecordRdata* other) const = 0;
+  uint16_t Type() const override;
+
+  virtual bool IsAlias() const = 0;
+  AliasFormHttpsRecordRdata* AsAliasForm();
+  const AliasFormHttpsRecordRdata* AsAliasForm() const;
+  ServiceFormHttpsRecordRdata* AsServiceForm();
+  const ServiceFormHttpsRecordRdata* AsServiceForm() const;
+
+ protected:
+  HttpsRecordRdata() = default;
+};
+
+class NET_EXPORT_PRIVATE AliasFormHttpsRecordRdata : public HttpsRecordRdata {
+ public:
+  explicit AliasFormHttpsRecordRdata(std::string alias_name);
+  static std::unique_ptr<AliasFormHttpsRecordRdata> Parse(
+      base::StringPiece data);
+
+  bool IsEqual(const HttpsRecordRdata* other) const override;
+  bool IsAlias() const override;
+
+  base::StringPiece alias_name() { return alias_name_; }
+
+ private:
+  AliasFormHttpsRecordRdata() = default;
+
+  const std::string alias_name_;
+};
+
+class NET_EXPORT_PRIVATE ServiceFormHttpsRecordRdata : public HttpsRecordRdata {
+ public:
+  ServiceFormHttpsRecordRdata(uint16_t priority,
+                              std::string service_name,
+                              std::vector<std::string> alpn_ids,
+                              bool default_alpn,
+                              base::Optional<uint16_t> port,
+                              std::vector<IPAddress> ipv4_hint,
+                              std::string ech_config,
+                              std::vector<IPAddress> ipv6_hint,
+                              std::map<uint16_t, std::string> unparsed_params);
+  static std::unique_ptr<ServiceFormHttpsRecordRdata> Parse(
+      base::StringPiece data);
+
+  ~ServiceFormHttpsRecordRdata() override;
+
+  bool IsEqual(const HttpsRecordRdata* other) const override;
+  bool IsAlias() const override;
+
+  uint16_t priority() { return priority_; }
+  base::StringPiece service_name() { return service_name_; }
+  const std::vector<std::string>& alpn_ids() { return alpn_ids_; }
+  bool default_alpn() { return default_alpn_; }
+  base::Optional<uint16_t> port() { return port_; }
+  const std::vector<IPAddress>& ipv4_hint() { return ipv4_hint_; }
+  base::StringPiece ech_config() { return ech_config_; }
+  const std::vector<IPAddress>& ipv6_hint() { return ipv6_hint_; }
+  const std::map<uint16_t, std::string>& unparsed_params() {
+    return unparsed_params_;
+  }
+
+ private:
+  const uint16_t priority_;
+  const std::string service_name_;
+
+  // Supported service parameters.
+  const std::vector<std::string> alpn_ids_;
+  const bool default_alpn_;
+  const base::Optional<uint16_t> port_;
+  const std::vector<IPAddress> ipv4_hint_;
+  const std::string ech_config_;
+  const std::vector<IPAddress> ipv6_hint_;
+
+  const std::map<uint16_t, std::string> unparsed_params_;
+};
+
+}  // namespace net
+
+#endif  // NET_DNS_HTTPS_RECORD_RDATA_H_

net/dns/https_record_rdata_unittest.cc

+// Copyright 2020 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 "net/dns/https_record_rdata.h"
+
+#include <map>
+#include <memory>
+#include <string>
+#include <vector>
+
+#include "base/strings/string_piece.h"
+#include "net/base/ip_address.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace net {
+namespace {
+
+TEST(HttpsRecordRdataTest, ParsesAlias) {
+  const char kRdata[] =
+      // Priority: 0 for alias record
+      "\000\000"
+      // Alias name: chromium.org
+      "\010chromium\003org\000";
+
+  std::unique_ptr<HttpsRecordRdata> rdata =
+      HttpsRecordRdata::Parse(base::StringPiece(kRdata, sizeof(kRdata) - 1));
+  ASSERT_TRUE(rdata);
+
+  AliasFormHttpsRecordRdata expected("chromium.org");
+  EXPECT_TRUE(rdata->IsEqual(&expected));
+
+  EXPECT_TRUE(rdata->IsAlias());
+  AliasFormHttpsRecordRdata* alias_rdata = rdata->AsAliasForm();
+  ASSERT_TRUE(alias_rdata);
+  EXPECT_EQ(alias_rdata->alias_name(), "chromium.org");
+}
+
+TEST(HttpsRecordRdataTest, ParsesService) {
+  const char kRdata[] =
+      // Priority: 1
+      "\000\001"
+      // Service name: chromium.org
+      "\010chromium\003org\000"
+      // alpn=foo,bar
+      "\000\001\000\010\003foo\003bar"
+      // no-default-alpn
+      "\000\002\000\000"
+      // port=46
+      "\000\003\000\002\000\056"
+      // ipv4hint=8.8.8.8
+      "\000\004\000\004\010\010\010\010"
+      // echconfig=hello
+      "\000\005\000\005hello"
+      // ipv6hint=2001:4860:4860::8888
+      "\000\006\000\020\x20\x01\x48\x60\x48\x60\x00\x00\x00\x00\x00\x00\x00\x00"
+      "\x88\x88"
+      // Unknown key7=foo
+      "\000\007\000\003foo";
+
+  std::unique_ptr<HttpsRecordRdata> rdata =
+      HttpsRecordRdata::Parse(base::StringPiece(kRdata, sizeof(kRdata) - 1));
+  ASSERT_TRUE(rdata);
+
+  IPAddress expected_ipv6;
+  ASSERT_TRUE(expected_ipv6.AssignFromIPLiteral("2001:4860:4860::8888"));
+  ServiceFormHttpsRecordRdata expected(
+      1 /* priority */, "chromium.org",
+      std::vector<std::string>({"foo", "bar"}) /* alpn_ids */,
+      false /* default_alpn */, base::Optional<uint16_t>(46) /* port */,
+      std::vector<IPAddress>({IPAddress(8, 8, 8, 8)}) /* ipv4_hint */,
+      "hello" /* ech_config */,
+      std::vector<IPAddress>({expected_ipv6}) /* ipv6_hint */,
+      std::map<uint16_t, std::string>({{7, "foo"}}) /* unparsed_params */);
+  EXPECT_TRUE(rdata->IsEqual(&expected));
+
+  EXPECT_FALSE(rdata->IsAlias());
+  ServiceFormHttpsRecordRdata* service_rdata = rdata->AsServiceForm();
+  ASSERT_TRUE(service_rdata);
+  EXPECT_EQ(service_rdata->priority(), 1);
+  EXPECT_EQ(service_rdata->service_name(), "chromium.org");
+  EXPECT_THAT(service_rdata->alpn_ids(), testing::ElementsAre("foo", "bar"));
+  EXPECT_FALSE(service_rdata->default_alpn());
+  EXPECT_THAT(service_rdata->port(), testing::Optional(46));
+  EXPECT_THAT(service_rdata->ipv4_hint(),
+              testing::ElementsAre(IPAddress(8, 8, 8, 8)));
+  EXPECT_EQ(service_rdata->ech_config(), "hello");
+  EXPECT_THAT(service_rdata->ipv6_hint(), testing::ElementsAre(expected_ipv6));
+  EXPECT_THAT(service_rdata->unparsed_params(),
+              testing::ElementsAre(testing::Pair(7, "foo")));
+}
+
+TEST(HttpsRecordRdataTest, RejectCorruptRdata) {
+  const char kRdata[] =
+      // Priority: 5
+      "\000\005"
+      // Service name: chromium.org
+      "\010chromium\003org\000"
+      // Malformed alpn
+      "\000\001\000\005hi";
+
+  std::unique_ptr<HttpsRecordRdata> rdata =
+      HttpsRecordRdata::Parse(base::StringPiece(kRdata, sizeof(kRdata) - 1));
+  EXPECT_FALSE(rdata);
+}
+
+}  // namespace
+}  // namespace net

net/dns/public/dns_protocol.h

@@ -187,6 +187,16 @@ static const uint16_t kFlagAA = 0x400;  // Authoritative Answer - response flag.
 static const uint16_t kFlagRD = 0x100;  // Recursion Desired - query flag.
 static const uint16_t kFlagTC = 0x200;  // Truncated - server flag.
 
+// SVCB/HTTPS ServiceParamKey
+//
+// IANA registration pending. Values from draft-ietf-dnsop-svcb-httpssvc-03.
+static constexpr uint16_t kHttpsServiceParamKeyAlpn = 1;
+static constexpr uint16_t kHttpsServiceParamKeyNoDefaultAlpn = 2;
+static constexpr uint16_t kHttpsServiceParamKeyPort = 3;
+static constexpr uint16_t kHttpsServiceParamKeyIpv4Hint = 4;
+static constexpr uint16_t kHttpsServiceParamKeyEchConfig = 5;
+static constexpr uint16_t kHttpsServiceParamKeyIpv6Hint = 6;
+
 }  // namespace dns_protocol
 
 }  // namespace net

net/dns/record_rdata.cc

@@ -25,6 +25,9 @@ static const size_t kSrvRecordMinimumSize = 6;
 static constexpr size_t kIntegrityMinimumSize =
     sizeof(uint16_t) + IntegrityRecordRdata::kDigestLen;
 
+// Minimal HTTPS rdata is 2 octets priority + 1 octet empty name.
+static constexpr size_t kHttpsRdataMinimumSize = 3;
+
 bool RecordRdata::HasValidSize(const base::StringPiece& data, uint16_t type) {
   switch (type) {
     case dns_protocol::kTypeSRV:
@@ -36,8 +39,7 @@ bool RecordRdata::HasValidSize(const base::StringPiece& data, uint16_t type) {
     case dns_protocol::kExperimentalTypeIntegrity:
       return data.size() >= kIntegrityMinimumSize;
     case dns_protocol::kTypeHttps:
-      // TODO(crbug.com/1138620): Implement actual size minimum.
-      return data.size() == 0;
+      return data.size() >= kHttpsRdataMinimumSize;
     case dns_protocol::kTypeCNAME:
     case dns_protocol::kTypePTR:
     case dns_protocol::kTypeTXT: