diff --git a/CHANGES b/CHANGES index e9dd725cc5..795a7c1a5d 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,10 @@ +4154. [bug] A OPT record should be included with the FORMERR + response when there is a malformed EDNS option. + [RT #39647] + +4153. [bug] Check that non significant ECS bits are zero on + receipt. [RT #39647] + 4151. [bug] 'rndc flush' could cause a deadlock. [RT #39835] 4150. [bug] win32: listen-on-v6 { any; }; was not working. Apply diff --git a/bin/named/client.c b/bin/named/client.c index 26222d0a1d..54da0c2024 100644 --- a/bin/named/client.c +++ b/bin/named/client.c @@ -1660,6 +1660,8 @@ client_request(isc_task_t *task, isc_event_t *event) { * Parsing the request failed. Send a response * (typically FORMERR or SERVFAIL). */ + if (result == DNS_R_OPTERR) + (void)client_addopt(client); ns_client_error(client, result); goto cleanup; } diff --git a/bin/tests/system/formerr/badoptoption b/bin/tests/system/formerr/badoptoption new file mode 100644 index 0000000000..eec4129c82 --- /dev/null +++ b/bin/tests/system/formerr/badoptoption @@ -0,0 +1,3 @@ +00 00 00 00 00 01 00 00 00 00 00 01 +00 00 01 00 02 +00 00 29 10 00 00 00 00 00 00 04 00 08 00 00 diff --git a/bin/tests/system/formerr/clean.sh b/bin/tests/system/formerr/clean.sh index 4978c71b9f..da5df3ce95 100644 --- a/bin/tests/system/formerr/clean.sh +++ b/bin/tests/system/formerr/clean.sh @@ -12,6 +12,7 @@ # OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR # PERFORMANCE OF THIS SOFTWARE. +rm -f badoptoption.out rm -f nametoolong.out -rm -f twoquestions.out rm -f noquestions.out +rm -f twoquestions.out diff --git a/bin/tests/system/formerr/tests.sh b/bin/tests/system/formerr/tests.sh index ea6aca53ed..4ee4fd5f6b 100644 --- a/bin/tests/system/formerr/tests.sh +++ b/bin/tests/system/formerr/tests.sh @@ -44,6 +44,14 @@ then echo "I:failed"; status=`expr $status + 1`; fi +echo "I:bad OPT option" +$PERL formerr.pl -a 10.53.0.1 -p 5300 badoptoption > badoptoption.out +ans=`grep got: badoptoption.out` +if [ "${ans}" != "got: 00008001000100000000000100000100020000291000000000000000" ]; +then + echo "I:failed"; status=`expr $status + 1`; +fi + echo "I:exit status: $status" exit $status diff --git a/lib/dns/include/dns/result.h b/lib/dns/include/dns/result.h index 489737cf7c..7d11c2beb0 100644 --- a/lib/dns/include/dns/result.h +++ b/lib/dns/include/dns/result.h @@ -156,8 +156,9 @@ #define DNS_R_NTACOVERED (ISC_RESULTCLASS_DNS + 110) #define DNS_R_BADCDS (ISC_RESULTCLASS_DNS + 111) #define DNS_R_BADCDNSKEY (ISC_RESULTCLASS_DNS + 112) +#define DNS_R_OPTERR (ISC_RESULTCLASS_DNS + 113) -#define DNS_R_NRESULTS 113 /*%< Number of results */ +#define DNS_R_NRESULTS 114 /*%< Number of results */ /* * DNS wire format rcodes. diff --git a/lib/dns/rdata/generic/opt_41.c b/lib/dns/rdata/generic/opt_41.c index ba3fef001a..cc391c32e2 100644 --- a/lib/dns/rdata/generic/opt_41.c +++ b/lib/dns/rdata/generic/opt_41.c @@ -128,7 +128,7 @@ fromwire_opt(ARGS_FROMWIRE) { isc_uint8_t addrbytes; if (length < 4) - return (DNS_R_FORMERR); + return (DNS_R_OPTERR); family = uint16_fromregion(&sregion); isc_region_consume(&sregion, 2); addrlen = uint8_fromregion(&sregion); @@ -138,16 +138,23 @@ fromwire_opt(ARGS_FROMWIRE) { switch (family) { case 1: if (addrlen > 32U || scope > 32U) - return (DNS_R_FORMERR); + return (DNS_R_OPTERR); break; case 2: if (addrlen > 128U || scope > 128U) - return (DNS_R_FORMERR); + return (DNS_R_OPTERR); break; } addrbytes = (addrlen + 7) / 8; if (addrbytes + 4 != length) - return (DNS_R_FORMERR); + return (DNS_R_OPTERR); + + if (addrbytes != 0U && (addrlen % 8) != 0) { + isc_uint8_t bits = ~0 << (8 - (addrlen % 8)); + bits &= sregion.base[addrbytes - 1]; + if (bits != sregion.base[addrbytes - 1]) + return (DNS_R_OPTERR); + } isc_region_consume(&sregion, addrbytes); break; } @@ -156,7 +163,7 @@ fromwire_opt(ARGS_FROMWIRE) { * Request has zero length. Response is 32 bits. */ if (length != 0 && length != 4) - return (DNS_R_FORMERR); + return (DNS_R_OPTERR); isc_region_consume(&sregion, length); break; default: diff --git a/lib/dns/result.c b/lib/dns/result.c index 9387f97893..7be4f577ed 100644 --- a/lib/dns/result.c +++ b/lib/dns/result.c @@ -164,9 +164,10 @@ static const char *text[DNS_R_NRESULTS] = { "not dynamic", /*%< 108 DNS_R_NOTDYNAMIC */ "bad EUI", /*%< 109 DNS_R_BADEUI */ - "covered by negative trust anchor", /*%< 110 DNS_R_NTACOVERED */ + "covered by negative trust anchor", /*%< 110 DNS_R_NTACOVERED */ "bad CDS", /*%< 111 DNS_R_BADCSD */ - "bad CDNSKEY" /*%< 112 DNS_R_BADCDNSKEY */ + "bad CDNSKEY", /*%< 112 DNS_R_BADCDNSKEY */ + "malformed OPT option" /*%< 113 DNS_R_OPTERR */ }; static const char *rcode_text[DNS_R_NRCODERESULTS] = { @@ -242,6 +243,7 @@ dns_result_torcode(isc_result_t result) { */ return ((dns_rcode_t)((result) & 0xFFF)); } + /* * Try to supply an appropriate rcode. */ @@ -271,6 +273,7 @@ dns_result_torcode(isc_result_t result) { case DNS_R_TSIGERRORSET: case DNS_R_UNKNOWN: case DNS_R_NAMETOOLONG: + case DNS_R_OPTERR: rcode = dns_rcode_formerr; break; case DNS_R_DISALLOWED: