Properly reject zero length ALPN in commatxt_fromtext

ALPN are defined as 1*255OCTET in RFC 9460.  commatxt_fromtext was not
rejecting invalid inputs produces by missing a level of escaping
which where later caught be dns_rdata_fromwire on reception.

These inputs should have been rejected

	svcb in svcb 1 1.svcb alpn=\,abc
	svcb1 in svcb 1 1.svcb alpn=a\,\,abc

and generated 00 03 61 62 63 and 01 61 00 02 61 62 63 respectively.

The correct inputs to include commas in the alpn requires double
escaping.

	svcb in svcb 1 1.svcb alpn=\\,abc
	svcb1 in svcb 1 1.svcb alpn=a\\,\\,abc

and generate 04 2C 61 62 63 and 06 61 2C 2C 61 62 63 respectively.

(cherry picked from commit b51c9eb7975ad883fc6380347b9be56d4976e730)

diff --git a/lib/dns/rdata.c b/lib/dns/rdata.c
index 227dfbc795f6ed7ef29dabab4502a1f6fa17c0a9..5f0ab360920db98bd01da3a9c9396c26af950e4b 100644 (file)
--- a/lib/dns/rdata.c
+++ b/lib/dns/rdata.c
@@ -1639,21 +1639,26 @@ commatxt_fromtext(isc_textregion_t *source, bool comma, isc_buffer_t *target) {
 
        if (comma) {
                /*
-                * Disallow empty ALPN at start (",h1") or in the
-                * middle ("h1,,h2").
+                * Disallow empty ALPN at start (",h1" or "\,h1") or
+                * in the middle ("h1,,h2" or "h1\,\,h2").
                 */
-               if (s == source->base || (seen_comma && s == source->base + 1))
-               {
+               if ((t - tregion.base - 1) == 0) {
                        return (DNS_R_SYNTAX);
                }
+
+               /*
+                * Consume this ALPN and possible ending comma.
+                */
                isc_textregion_consume(source, s - source->base);
+
                /*
-                * Disallow empty ALPN at end ("h1,").
+                * Disallow empty ALPN at end ("h1," or "h1\,").
                 */
                if (seen_comma && source->length == 0) {
                        return (DNS_R_SYNTAX);
                }
        }
+
        *tregion.base = (unsigned char)(t - tregion.base - 1);
        isc_buffer_add(target, *tregion.base + 1);
        return (ISC_R_SUCCESS);

diff --git a/tests/dns/rdata_test.c b/tests/dns/rdata_test.c
index 0a9ec1c00a848c0049135f9303dab586206ca585..0d7c23d6aafea64ca8e798ec0e1ab91e30452236 100644 (file)
--- a/tests/dns/rdata_test.c
+++ b/tests/dns/rdata_test.c
@@ -2553,6 +2553,10 @@ ISC_RUN_TEST_IMPL(https_svcb) {
                TEXT_INVALID("2 svc.example.net. alpn=,h1"),
                TEXT_INVALID("2 svc.example.net. alpn=h1,"),
                TEXT_INVALID("2 svc.example.net. alpn=h1,,h2"),
+               /* empty alpn-id sub fields - RFC 1035 escaped commas */
+               TEXT_INVALID("2 svc.example.net. alpn=\\,abc"),
+               TEXT_INVALID("2 svc.example.net. alpn=abc\\,"),
+               TEXT_INVALID("2 svc.example.net. alpn=a\\,\\,abc"),
                /* mandatory */
                TEXT_VALID_LOOP(2, "2 svc.example.net. mandatory=alpn "
                                   "alpn=\"h2\""),