commit 1a668b39d5d2287f1fcdcf19a9d0e747b40f86ef
author David Benjamin <davidben@google.com> Thu Sep 02 23:00:28 2021 -0400
committer Adam Langley <agl@google.com> Fri Sep 03 17:15:05 2021 +0000
tree 161fb2f5f4f5fb8a378a6bf3267f870b6595d6a3
parent b49b78ef3f781b1d34c5b07204ee92d2dc70a2a7

Switch to the new, simpler WHATWG URL formulation.

In light of
https://groups.google.com/a/chromium.org/g/blink-dev/c/7QN5nxjwIfM/m/q9dw9MxoAwAJ,
the WHATWG URL parser is now more restrictive about which strings are
valid DNS names. The final component may not be numeric. Align the
ECHConfig validator with this.

Bug: 275
Change-Id: Iea2a3d9a7fee5bffc683da99274c54d60379be9e
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49225
Reviewed-by: Adam Langley <agl@google.com>

ssl/ssl_test.cc

diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index 60d820b..7ab5054 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -2119,44 +2119,39 @@
   EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span(
       "abcdefhijklmnopqrstuvwxyz-ABCDEFGHIJKLMNOPQRSTUVWXYZ-01234567899")));
 
-  // Inputs that parse as IPv4 addresses are rejected.
+  // Inputs with trailing numeric components are rejected.
   EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("127.0.0.1")));
-  EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("0177.0.0.01")));
-  EXPECT_FALSE(
-      ssl_is_valid_ech_public_name(str_to_span("0x7f.0x.0x.0x00000001")));
-  EXPECT_FALSE(
-      ssl_is_valid_ech_public_name(str_to_span("0XAB.0XCD.0XEF.0X01")));
-  EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("0.0.0.0")));
-  EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("255.255.255.255")));
-  // Out-of-bounds or overflowing components are not IP addresses.
-  EXPECT_TRUE(ssl_is_valid_ech_public_name(str_to_span("256.255.255.255")));
-  EXPECT_TRUE(ssl_is_valid_ech_public_name(str_to_span("255.0x100.255.255")));
-  EXPECT_TRUE(ssl_is_valid_ech_public_name(str_to_span("255.255.255.0400")));
+  EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("example.1")));
+  EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("example.01")));
+  EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("example.0x01")));
+  EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("example.0X01")));
+  // Leading zeros and values that overflow |uint32_t| are still rejected.
+  EXPECT_FALSE(ssl_is_valid_ech_public_name(
+      str_to_span("example.123456789000000000000000")));
+  EXPECT_FALSE(ssl_is_valid_ech_public_name(
+      str_to_span("example.012345678900000000000000")));
+  EXPECT_FALSE(ssl_is_valid_ech_public_name(
+      str_to_span("example.0x123456789abcdefABCDEF0")));
+  EXPECT_FALSE(ssl_is_valid_ech_public_name(
+      str_to_span("example.0x0123456789abcdefABCDEF")));
+  // Adding a non-digit or non-hex character makes it a valid DNS name again.
+  // Single-component numbers are rejected.
   EXPECT_TRUE(ssl_is_valid_ech_public_name(
-      str_to_span("255.255.255.0x100000000")));
-  // Invalid characters for the base are not IP addresses.
-  EXPECT_TRUE(ssl_is_valid_ech_public_name(str_to_span("12a.0.0.1")));
-  EXPECT_TRUE(ssl_is_valid_ech_public_name(str_to_span("0xg.0.0.1")));
-  EXPECT_TRUE(ssl_is_valid_ech_public_name(str_to_span("08.0.0.1")));
-  // Trailing components can be merged into a single component.
-  EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("127.0.1")));
-  EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("127.1")));
-  EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("2130706433")));
-  EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("0x7f000001")));
-  // Merged components must respect their limits.
-  EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("0.0.0.0xff")));
-  EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("0.0.0xffff")));
-  EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("0.0xffffff")));
-  EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("0xffffffff")));
-  EXPECT_TRUE(ssl_is_valid_ech_public_name(str_to_span("0.0.0.0x100")));
-  EXPECT_TRUE(ssl_is_valid_ech_public_name(str_to_span("0.0.0x10000")));
-  EXPECT_TRUE(ssl_is_valid_ech_public_name(str_to_span("0.0x1000000")));
-  EXPECT_TRUE(ssl_is_valid_ech_public_name(str_to_span("0x100000000")));
-  // Correctly handle overflow in decimal and octal.
-  EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("037777777777")));
-  EXPECT_TRUE(ssl_is_valid_ech_public_name(str_to_span("040000000000")));
-  EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("4294967295")));
-  EXPECT_TRUE(ssl_is_valid_ech_public_name(str_to_span("4294967296")));
+      str_to_span("example.1234567890a")));
+  EXPECT_TRUE(ssl_is_valid_ech_public_name(
+      str_to_span("example.01234567890a")));
+  EXPECT_TRUE(ssl_is_valid_ech_public_name(
+      str_to_span("example.0x123456789abcdefg")));
+  EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("1")));
+  EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("01")));
+  EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("0x01")));
+  EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("0X01")));
+  // Numbers with trailing dots are rejected. (They are already rejected by the
+  // LDH label rules, but the WHATWG URL parser additionally rejects them.)
+  EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("1.")));
+  EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("01.")));
+  EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("0x01.")));
+  EXPECT_FALSE(ssl_is_valid_ech_public_name(str_to_span("0X01.")));
 }
 
 // When using the built-in verifier, test that |SSL_get0_ech_name_override| is