From 0a5ed1e3507430a92a8cfe68f39ee4f9ec828da1 Mon Sep 17 00:00:00 2001 From: Chris Hofstaedtler Date: Tue, 27 May 2025 22:04:34 +0200 Subject: [PATCH] SVCB: allow parsing generic key without value --- pdns/rcpgenerator.cc | 38 +++++++++++++-- pdns/test-rcpgenerator_cc.cc | 89 ++++++++++++++++++++++++++++++++++-- pdns/test-svc_records_cc.cc | 3 ++ 3 files changed, 122 insertions(+), 8 deletions(-) diff --git a/pdns/rcpgenerator.cc b/pdns/rcpgenerator.cc index 10afab9bc0..c3ec2c20d7 100644 --- a/pdns/rcpgenerator.cc +++ b/pdns/rcpgenerator.cc @@ -362,10 +362,7 @@ void RecordTextReader::xfrSvcParamKeyVals(set& val) // NOLINT(readabil throw RecordTextException(e.what()); } - if (key != SvcParam::no_default_alpn) { - if (d_pos == d_end || d_string.at(d_pos) != '=') { - throw RecordTextException("expected '=' after " + k); - } + if (d_pos != d_end && d_string.at(d_pos) == '=') { d_pos++; // Now on the first character after '=' if (d_pos == d_end || d_string.at(d_pos) == ' ') { throw RecordTextException("expected value after " + k + "="); @@ -374,7 +371,7 @@ void RecordTextReader::xfrSvcParamKeyVals(set& val) // NOLINT(readabil switch (key) { case SvcParam::no_default_alpn: - if (d_pos != d_end && d_string.at(d_pos) == '=') { + if (d_pos != d_end && d_string.at(d_pos) != ' ') { throw RecordTextException(k + " key can not have values"); } val.insert(SvcParam(key)); @@ -406,6 +403,9 @@ void RecordTextReader::xfrSvcParamKeyVals(set& val) // NOLINT(readabil hints.push_back(ComboAddress(v)); } } + if (!doAuto && hints.empty()) { + throw RecordTextException("value is required for SVC Param " + k); + } try { auto p = SvcParam(key, std::move(hints)); p.setAutoHint(doAuto); @@ -425,6 +425,9 @@ void RecordTextReader::xfrSvcParamKeyVals(set& val) // NOLINT(readabil while (spos < v.length()) { len = v.at(spos); spos += 1; + if (len == 0) { + throw RecordTextException("ALPN values cannot be empty strings"); + } if (len > v.length() - spos) { throw RecordTextException("Length of ALPN value goes over total length of alpn SVC Param"); } @@ -434,6 +437,14 @@ void RecordTextReader::xfrSvcParamKeyVals(set& val) // NOLINT(readabil } else { xfrSVCBValueList(value); } + if (value.empty()) { + throw RecordTextException("value is required for SVC Param " + k); + } + for (const auto &alpn_value : value) { + if (alpn_value.empty()) { + throw RecordTextException("ALPN values cannot be empty strings"); + } + } val.insert(SvcParam(key, std::move(value))); break; } @@ -441,6 +452,9 @@ void RecordTextReader::xfrSvcParamKeyVals(set& val) // NOLINT(readabil if (generic) { string v; xfrRFC1035CharString(v); + if (v.empty()) { + throw RecordTextException("value is required for SVC Param " + k); + } if (v.length() % 2 != 0) { throw RecordTextException("Wrong number of bytes in SVC Param " + k); } @@ -455,6 +469,9 @@ void RecordTextReader::xfrSvcParamKeyVals(set& val) // NOLINT(readabil } vector parts; xfrSVCBValueList(parts); + if (parts.empty()) { + throw RecordTextException("value is required for SVC Param " + k); + } set values(parts.begin(), parts.end()); val.insert(SvcParam(key, std::move(values))); break; @@ -472,6 +489,9 @@ void RecordTextReader::xfrSvcParamKeyVals(set& val) // NOLINT(readabil } else { string portstring; xfrRFC1035CharString(portstring); + if (portstring.empty()) { + throw RecordTextException("value is required for SVC Param " + k); + } try { pdns::checked_stoi_into(port, portstring); } catch (const std::exception &e) { @@ -498,12 +518,20 @@ void RecordTextReader::xfrSvcParamKeyVals(set& val) // NOLINT(readabil d_pos++; } } + if (value.empty()) { + throw RecordTextException("value is required for SVC Param " + k); + } val.insert(SvcParam(key, value)); break; } default: { string value; xfrRFC1035CharString(value); + if (!generic && value.empty()) { + // for generic format, we do not know. + // Known keys which forbid having a value need to implement a switch case, above. + throw RecordTextException("value is required for SVC Param " + k); + } val.insert(SvcParam(key, value)); break; } diff --git a/pdns/test-rcpgenerator_cc.cc b/pdns/test-rcpgenerator_cc.cc index ba8357d6ad..02960d9fcc 100644 --- a/pdns/test-rcpgenerator_cc.cc +++ b/pdns/test-rcpgenerator_cc.cc @@ -92,6 +92,14 @@ BOOST_AUTO_TEST_CASE(test_xfrSvcParamKeyVals_alpn) { source="key1=\\002h2\\002h3foobar"; // extra data RecordTextReader rtr5(source); BOOST_CHECK_THROW(rtr5.xfrSvcParamKeyVals(v), RecordTextException); + + source="key1"; // no data + RecordTextReader rtr6(source); + BOOST_CHECK_THROW(rtr6.xfrSvcParamKeyVals(v), RecordTextException); + + source="key1=\\000"; // no data + RecordTextReader rtr7(source); + BOOST_CHECK_THROW(rtr7.xfrSvcParamKeyVals(v), RecordTextException); } BOOST_AUTO_TEST_CASE(test_xfrSvcParamKeyVals_mandatory) { @@ -180,12 +188,37 @@ BOOST_AUTO_TEST_CASE(test_xfrSvcParamKeyVals_no_default_alpn) { // Generic v.clear(); - source = "key2"; - RecordTextReader rtr3(source); + RecordTextReader rtr3("key2"); rtr3.xfrSvcParamKeyVals(v); BOOST_CHECK_EQUAL(v.size(), 1U); k = v.begin()->getKey(); BOOST_CHECK(k == SvcParam::no_default_alpn); + + v.clear(); + RecordTextReader rtr4("key2="); + BOOST_CHECK_THROW(rtr4.xfrSvcParamKeyVals(v), RecordTextException); + + v.clear(); + RecordTextReader rtr5("key2 ipv4hint=1.2.3.4"); + rtr5.xfrSvcParamKeyVals(v); + BOOST_CHECK_EQUAL(v.size(), 2U); + k = v.begin()->getKey(); + BOOST_CHECK(k == SvcParam::no_default_alpn); + + v.clear(); + RecordTextReader rtr6("ipv4hint=1.2.3.4 key2"); + rtr6.xfrSvcParamKeyVals(v); + BOOST_CHECK_EQUAL(v.size(), 2U); + k = v.begin()->getKey(); + BOOST_CHECK(k == SvcParam::no_default_alpn); + + v.clear(); + RecordTextReader rtr7("key2=port=123 ipv4hint=1.2.3.4"); + BOOST_CHECK_THROW(rtr7.xfrSvcParamKeyVals(v), RecordTextException); + + v.clear(); + RecordTextReader rtr8("key2=port=123"); + BOOST_CHECK_THROW(rtr8.xfrSvcParamKeyVals(v), RecordTextException); } BOOST_AUTO_TEST_CASE(test_xfrSvcParamKeyVals_ipv4hint) { @@ -263,6 +296,23 @@ BOOST_AUTO_TEST_CASE(test_xfrSvcParamKeyVals_ipv4hint) { source = "key4=\\192\\000\\222"; // Wrong number of octets RecordTextReader rtr7(source); BOOST_CHECK_THROW(rtr7.xfrSvcParamKeyVals(v), RecordTextException); + + v.clear(); + RecordTextReader rtr8("key4="); // must have a value + BOOST_CHECK_THROW(rtr8.xfrSvcParamKeyVals(v), RecordTextException); + + v.clear(); + RecordTextReader rtr9("ipv4hint="); // must have a value + BOOST_CHECK_THROW(rtr9.xfrSvcParamKeyVals(v), RecordTextException); + + v.clear(); + RecordTextReader rtr10("ipv4hint=auto"); // special value + rtr10.xfrSvcParamKeyVals(v); + BOOST_CHECK_EQUAL(v.size(), 1U); + k = v.begin()->getKey(); + BOOST_CHECK(k == SvcParam::ipv4hint); + BOOST_CHECK_EQUAL(v.begin()->getIPHints().size(), 0U); + BOOST_CHECK_EQUAL(v.begin()->getAutoHint(), true); } BOOST_AUTO_TEST_CASE(test_xfrSvcParamKeyVals_ipv6hint) { @@ -339,6 +389,14 @@ BOOST_AUTO_TEST_CASE(test_xfrSvcParamKeyVals_ipv6hint) { source = "key6=\\040\\001\\015\\270\\000\\123\\000\\000\\000\\000\\000\\000\\000\\000\\000"; // wrong number of octets RecordTextReader rtr7(source); BOOST_CHECK_THROW(rtr7.xfrSvcParamKeyVals(v), RecordTextException); + + v.clear(); + RecordTextReader rtr8("key6="); // must have a value + BOOST_CHECK_THROW(rtr8.xfrSvcParamKeyVals(v), RecordTextException); + + v.clear(); + RecordTextReader rtr9("ipv6hint="); // must have a value + BOOST_CHECK_THROW(rtr9.xfrSvcParamKeyVals(v), RecordTextException); } BOOST_AUTO_TEST_CASE(test_xfrSvcParamKeyVals_port) { @@ -412,7 +470,10 @@ BOOST_AUTO_TEST_CASE(test_xfrSvcParamKeyVals_generic) { v.clear(); RecordTextReader rtr3("key666"); - BOOST_CHECK_THROW(rtr3.xfrSvcParamKeyVals(v), RecordTextException); + BOOST_CHECK_NO_THROW(rtr3.xfrSvcParamKeyVals(v)); + BOOST_CHECK_EQUAL(v.size(), 1U); + k = v.begin()->getKey(); + BOOST_CHECK(k == SvcParam::keyFromString("key666")); v.clear(); source = "key666=\"blablabla\""; @@ -490,6 +551,20 @@ BOOST_AUTO_TEST_CASE(test_xfrSvcParamKeyVals_multiple) { RecordTextWriter rtw(target); rtw.xfrSvcParamKeyVals(v); BOOST_CHECK_EQUAL(target, "mandatory=alpn alpn=h2,h3 ipv4hint=192.0.2.1,192.0.2.2 ech=\"dG90YWxseSBib2d1cyBlY2hjb25maWcgdmFsdWU=\" ipv6hint=2001:db8::1 key666=\"foobar\""); + + v.clear(); + RecordTextReader rtr2("mandatory=alpn key666"); // generic key without value at the end of the string + BOOST_CHECK_NO_THROW(rtr2.xfrSvcParamKeyVals(v)); + BOOST_CHECK_EQUAL(v.size(), 2U); + + v.clear(); + RecordTextReader rtr3("key666 key677=\"foo\" mandatory=alpn"); // generic key without value -not- at the end of the string + BOOST_CHECK_NO_THROW(rtr3.xfrSvcParamKeyVals(v)); + BOOST_CHECK_EQUAL(v.size(), 3U); + + v.clear(); + RecordTextReader rtr4("mandatory= key666"); // non-generic key without value -not- at the end of the string + BOOST_CHECK_THROW(rtr4.xfrSvcParamKeyVals(v), RecordTextException); } BOOST_AUTO_TEST_CASE(test_xfrSvcParamKeyVals_ech) { @@ -519,6 +594,14 @@ BOOST_AUTO_TEST_CASE(test_xfrSvcParamKeyVals_ech) { BOOST_CHECK(k == SvcParam::ech); val = v.begin()->getECH(); BOOST_CHECK_EQUAL(val, "echconfig"); + + v.clear(); + RecordTextReader rtr3("key5"); + BOOST_CHECK_THROW(rtr3.xfrSvcParamKeyVals(v), RecordTextException); + + v.clear(); + RecordTextReader rtr4("ech=\"\""); + BOOST_CHECK_THROW(rtr4.xfrSvcParamKeyVals(v), RecordTextException); } BOOST_AUTO_TEST_CASE(test_xfrNodeOrLocatorID) { diff --git a/pdns/test-svc_records_cc.cc b/pdns/test-svc_records_cc.cc index bb7f08d2ef..920b1083a6 100644 --- a/pdns/test-svc_records_cc.cc +++ b/pdns/test-svc_records_cc.cc @@ -53,6 +53,9 @@ BOOST_AUTO_TEST_CASE(test_SvcParam_keyFromString) { k = SvcParam::keyFromString("key666"); BOOST_CHECK(k == 666); + k = SvcParam::keyFromString("key00666"); + BOOST_CHECK(k == 666); + BOOST_CHECK_THROW(SvcParam::keyFromString("MANDATORY"), std::invalid_argument); } -- 2.47.2