From: Pieter Lexis <pieter.lexis@powerdns.com>
Date: Mon, 15 Mar 2021 12:00:18 +0000 (+0100)
Subject: SVCB: bring parsing in line with draft-03
X-Git-Tag: dnsdist-1.6.0-rc1~33^2~7
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e701f9d4e9d8e225b396ad3949b9398f37c4b72d;p=thirdparty%2Fpdns.git

SVCB: bring parsing in line with draft-03
---

diff --git a/pdns/dnslabeltext.rl b/pdns/dnslabeltext.rl
index 3eabf72748..3731345739 100644
--- a/pdns/dnslabeltext.rl
+++ b/pdns/dnslabeltext.rl
@@ -240,12 +240,12 @@ size_t parseRFC1035CharString(const std::string &in, std::string &val) {
   return counter;
 }
 
-size_t parseSVCBValueList(const std::string &in, std::vector<std::string> &val) {
+size_t parseSVCBValueListFromParsedRFC1035CharString(const std::string &in, std::vector<std::string> &val) {
   val.clear();
   const char *p = in.c_str();
   const char *pe = p + in.size();
   int cs = 0;
-  uint8_t escaped_octet = 0;
+  const char* eof = pe;
   // Keeps track of how many chars we read from the source string
   size_t counter=0;
 
@@ -254,24 +254,18 @@ size_t parseSVCBValueList(const std::string &in, std::vector<std::string> &val)
 
 %%{
   machine dns_text_to_value_list;
+  alphtype unsigned char;
 
-  action doEscapedNumber {
-    escaped_octet *= 10;
-    escaped_octet += fc-'0';
+  action addToVal {
+    tmp += fc;
     counter++;
   }
 
-  action doneEscapedNumber {
-    tmp += escaped_octet;
-    escaped_octet = 0;
-  }
-
-  action addToVal {
+  action addToValNoIncrement {
     tmp += fc;
-    counter++;
   }
 
-  action handleComma {
+  action addToVector {
     val.push_back(tmp);
     tmp.clear();
     counter++;
@@ -282,24 +276,13 @@ size_t parseSVCBValueList(const std::string &in, std::vector<std::string> &val)
   }
 
   # generated rules, define required actions
-  DIGIT = 0x30..0x39;
-  DQUOTE = "\"";
-  HTAB = "\t";
-  SP = " ";
-  WSP = (SP | HTAB)@addToVal;
-  non_special = "!" | 0x23..0x27 | 0x2a..0x2b | 0x2d..0x3a | 0x3c..0x5b | 0x5d..0x7e;
-  non_digit = 0x21..0x2f | 0x3a..0x7e;
-  dec_octet = ( ( "0" | "1" ) DIGIT{2} ) | ( "2" ( ( 0x30..0x34 DIGIT ) | ( "5" 0x30..0x35 ) ) );
-  escaped = '\\'@incrementCounter ( non_digit$addToVal | dec_octet$doEscapedNumber@doneEscapedNumber );
-  contiguous = ( non_special$addToVal | escaped )+;
-  comma = ',';
-  quoted_sepped = ( contiguous | ('\\'? WSP) )* (comma@handleComma ( contiguous | ('\\'? WSP) )+ )*;
-  unquoted_sepped = (contiguous (comma@handleComma contiguous)*);
-  quoted = DQUOTE@incrementCounter quoted_sepped DQUOTE@incrementCounter;
-  char_string = (quoted | unquoted_sepped);
+  OCTET = 0x00..0xff;
+  item_allowed = 0x00..0x2b | 0x2d..0x5b | 0x5d..0xff;
+  escaped_item = ( item_allowed$addToVal | '\\,'$incrementCounter@addToValNoIncrement | '\\\\'$incrementCounter@addToValNoIncrement )+;
+  comma_separated = ( escaped_item%addToVector ( ","@incrementCounter escaped_item%addToVector )* )?;
 
   # instantiate machine rules
-  main := char_string;
+  main := comma_separated;
   write data;
   write init;
 }%%
@@ -310,9 +293,6 @@ size_t parseSVCBValueList(const std::string &in, std::vector<std::string> &val)
   (void) dns_text_to_value_list_en_main;
   %% write exec;
 
-  // Add the last-parsed value to val
-  // ideally, we'd use a transition as well, but too many hours were wasted trying that
-  val.push_back(tmp);
   return counter;
 }
 
diff --git a/pdns/misc.cc b/pdns/misc.cc
index 21179f67fb..899e32ed6d 100644
--- a/pdns/misc.cc
+++ b/pdns/misc.cc
@@ -1633,3 +1633,10 @@ std::string makeLuaString(const std::string& in)
 
   return str.str();
 }
+
+size_t parseSVCBValueList(const std::string &in, vector<std::string> &val) {
+  std::string parsed;
+  auto ret = parseRFC1035CharString(in, parsed);
+  parseSVCBValueListFromParsedRFC1035CharString(parsed, val);
+  return ret;
+};
diff --git a/pdns/misc.hh b/pdns/misc.hh
index be78521df7..ee04ebb72b 100644
--- a/pdns/misc.hh
+++ b/pdns/misc.hh
@@ -624,6 +624,7 @@ DNSName reverseNameFromIP(const ComboAddress& ip);
 
 std::string getCarbonHostName();
 size_t parseRFC1035CharString(const std::string &in, std::string &val); // from ragel
-size_t parseSVCBValueList(const std::string &in, vector<std::string> &val); // from ragel
+size_t parseSVCBValueListFromParsedRFC1035CharString(const std::string &in, vector<std::string> &val); // from ragel
+size_t parseSVCBValueList(const std::string &in, vector<std::string> &val);
 
 std::string makeLuaString(const std::string& in);
diff --git a/pdns/rcpgenerator.cc b/pdns/rcpgenerator.cc
index 3210231160..84a468f123 100644
--- a/pdns/rcpgenerator.cc
+++ b/pdns/rcpgenerator.cc
@@ -760,7 +760,7 @@ void RecordTextWriter::xfrSVCBValueList(const vector<string> &val) {
       shouldQuote = true;
     }
     string tmp = txtEscape(v);
-    boost::replace_all(tmp, ",", "\\,");
+    boost::replace_all(tmp, ",", "\\\\,");
     escaped.push_back(tmp);
   }
   if (shouldQuote) {
diff --git a/pdns/test-dnsrecords_cc.cc b/pdns/test-dnsrecords_cc.cc
index 9404e2fc14..9094d24ee8 100644
--- a/pdns/test-dnsrecords_cc.cc
+++ b/pdns/test-dnsrecords_cc.cc
@@ -232,9 +232,9 @@ BOOST_AUTO_TEST_CASE(test_record_types) {
      // IPv4hint is quoted
      (CASE_L(QType::SVCB, "16 foo.powerdns.org. alpn=h2,h3 mandatory=alpn ipv4hint=\"192.0.2.1\"", "16 foo.powerdns.org. mandatory=alpn alpn=h2,h3 ipv4hint=192.0.2.1", "\0\x10\3foo\x08powerdns\x03org\x00\x00\x00\x00\x02\x00\x01\x00\x01\x00\x06\x02h2\x02h3\x00\x04\x00\x04\xc0\x00\x02\x01"))
      // Escaped ALPN value
-     (CASE_S(QType::SVCB, "1 foo.powerdns.org. alpn=h3\\,cool,h2", "\0\x01\3foo\x08powerdns\x03org\x00\x00\x01\x00\x0b\x07h3,cool\x02h2"))
+     (CASE_S(QType::SVCB, R"FOO(1 foo.powerdns.org. alpn=h3\\,cool,h2)FOO", "\0\x01\3foo\x08powerdns\x03org\x00\x00\x01\x00\x0b\x07h3,cool\x02h2"))
      // Escaped _and_ spaced ALPN value
-     (CASE_S(QType::SVCB, "1 foo.powerdns.org. alpn=\"h3\\,co ol,h2\"", "\0\x01\3foo\x08powerdns\x03org\x00\x00\x01\x00\x0c\x08h3,co ol\x02h2"))
+     (CASE_S(QType::SVCB, R"FOO(1 foo.powerdns.org. alpn="h3\\,co ol,h2")FOO", "\0\x01\3foo\x08powerdns\x03org\x00\x00\x01\x00\x0c\x08h3,co ol\x02h2"))
 
      (CASE_S(QType::SPF, "\"v=spf1 a:mail.rec.test ~all\"", "\x1bv=spf1 a:mail.rec.test ~all"))
      (CASE_S(QType::EUI48, "00-11-22-33-44-55", "\x00\x11\x22\x33\x44\x55"))
diff --git a/pdns/test-misc_hh.cc b/pdns/test-misc_hh.cc
index 213d6a34d4..8cd24a857d 100644
--- a/pdns/test-misc_hh.cc
+++ b/pdns/test-misc_hh.cc
@@ -287,12 +287,39 @@ BOOST_AUTO_TEST_CASE(test_parseRFC1035CharString)
   amount = parseRFC1035CharString(in, out);
   BOOST_CHECK_EQUAL(amount, in.size());
   BOOST_CHECK_EQUAL(out, expected);
+
+  // From draft-ietf-dnsop-svcb-https-03
+  expected = R"FOO(part1,part2,part3\,part4\\)FOO";
+  in = R"FOO("part1,part2,part3\\,part4\\\\)FOO";
+  amount = parseRFC1035CharString(in, out);
+  BOOST_CHECK_EQUAL(amount, in.size());
+  BOOST_CHECK_EQUAL(out, expected);
+
+  in = R"FOO(part1\,\p\a\r\t2\044part3\092,part4\092\\)FOO";
+  amount = parseRFC1035CharString(in, out);
+  BOOST_CHECK_EQUAL(amount, in.size());
+  BOOST_CHECK_EQUAL(out, expected);
 }
 
 BOOST_AUTO_TEST_CASE(test_parseSVCBValueList)
 {
   vector<string> out;
 
+  // From draft-ietf-dnsop-svcb-https-03
+  vector<string> expected = {"part1", "part2", "part3,part4\\"};
+  parseSVCBValueList(R"FOO("part1,part2,part3\\,part4\\\\)FOO", out);
+  BOOST_CHECK_EQUAL(out.size(), expected.size());
+  BOOST_CHECK_EQUAL(out[0], expected[0]);
+  BOOST_CHECK_EQUAL(out[1], expected[1]);
+  BOOST_CHECK_EQUAL(out[2], expected[2]);
+
+  parseSVCBValueList(R"FOO(part1\,\p\a\r\t2\044part3\092,part4\092\\)FOO", out);
+  BOOST_CHECK_EQUAL(out.size(), expected.size());
+  BOOST_CHECK_EQUAL(out[0], expected[0]);
+  BOOST_CHECK_EQUAL(out[1], expected[1]);
+  BOOST_CHECK_EQUAL(out[2], expected[2]);
+
+  // Our tests
   parseSVCBValueList("foobar123", out);
   BOOST_CHECK_EQUAL(out.size(), 1);
   BOOST_CHECK_EQUAL(out[0], "foobar123");
@@ -314,11 +341,11 @@ BOOST_AUTO_TEST_CASE(test_parseSVCBValueList)
   BOOST_CHECK_EQUAL(out[0], "foobar123");
   BOOST_CHECK_EQUAL(out[1], "bazquux456");
 
-  parseSVCBValueList("foobar123\\,bazquux456", out);
+  parseSVCBValueList(R"FOO(foobar123\\,bazquux456)FOO", out);
   BOOST_CHECK_EQUAL(out.size(), 1);
   BOOST_CHECK_EQUAL(out[0], "foobar123,bazquux456");
 
-  parseSVCBValueList("foobar123\\044bazquux456", out);
+  parseSVCBValueList(R"FOO(foobar123\\\044bazquux456)FOO", out);
   BOOST_CHECK_EQUAL(out.size(), 1);
   BOOST_CHECK_EQUAL(out[0], "foobar123,bazquux456");
 
@@ -332,11 +359,11 @@ BOOST_AUTO_TEST_CASE(test_parseSVCBValueList)
   BOOST_CHECK_EQUAL(out[0], "foobar123");
   BOOST_CHECK_EQUAL(out[1], "bazquux456");
 
-  parseSVCBValueList("\"foobar123\\,bazquux456\"", out);
+  parseSVCBValueList(R"FOO("foobar123\\,bazquux456")FOO", out);
   BOOST_CHECK_EQUAL(out.size(), 1);
   BOOST_CHECK_EQUAL(out[0], "foobar123,bazquux456");
 
-  parseSVCBValueList("\"foobar123\\044bazquux456\"", out);
+  parseSVCBValueList(R"FOO("foobar123\\\044bazquux456")FOO", out);
   BOOST_CHECK_EQUAL(out.size(), 1);
   BOOST_CHECK_EQUAL(out[0], "foobar123,bazquux456");
 
@@ -355,11 +382,11 @@ BOOST_AUTO_TEST_CASE(test_parseSVCBValueList)
   BOOST_CHECK_EQUAL(out[0], "foobar123");
   BOOST_CHECK_EQUAL(out[1], "baz quux456");
 
-  parseSVCBValueList("\"foobar123 blabla bla\\,baz quux456\"", out);
+  parseSVCBValueList(R"FOO("foobar123 blabla bla\\,baz quux456")FOO", out);
   BOOST_CHECK_EQUAL(out.size(), 1);
   BOOST_CHECK_EQUAL(out[0], "foobar123 blabla bla,baz quux456");
 
-  parseSVCBValueList("\"foobar123 blabla bla\\044baz quux456\"", out);
+  parseSVCBValueList(R"FOO("foobar123 blabla bla\\\044baz quux456")FOO", out);
   BOOST_CHECK_EQUAL(out.size(), 1);
   BOOST_CHECK_EQUAL(out[0], "foobar123 blabla bla,baz quux456");
 }