fix: usr: Properly detect private records before copying

We were triggering an assertion when trying to copy a private record
to a buffer for modifying.  Extend the private type detection and
copy the contents after we have rejected invalid private records.

Closes #5857

Merge branch '5857-properly-detect-private-records-before-copying' into 'main'

See merge request isc-projects/bind9!11816
This commit is contained in:
Mark Andrews 2026-07-02 11:10:32 +10:00
commit b378071e06
5 changed files with 71 additions and 13 deletions

View file

@ -1383,5 +1383,44 @@ test ${ttl2:-0} -eq 400 || ret=1
test "$ret" -eq 0 || echo_i "failed"
status=$((status + ret))
n=$((n + 1))
echo_i "Test oversized private record is properly ignored ($n)"
ret=0
zone=oversized-private-record
cat >ns3/oversized-private-record.db <<EOF
@ 0 IN SOA . . 0 0 0 0 0
@ 0 IN NS ns3
@ 0 IN NS ns7
ns3 0 IN A 10.53.0.3
ns7 0 IN A 10.53.0.7
; 1st and 3rd octets are zero to ensure length check is exercised
@ IN TYPE65534 \\# 262 ( 0041004141414141414141414141414141
4141414141414141414141414141414141
4141414141414141414141414141414141
4141414141414141414141414141414141
4141414141414141414141414141414141
4141414141414141414141414141414141
4141414141414141414141414141414141
4141414141414141414141414141414141
4141414141414141414141414141414141
4141414141414141414141414141414141
4141414141414141414141414141414141
4141414141414141414141414141414141
4141414141414141414141414141414141
4141414141414141414141414141414141
4141414141414141414141414141414141
41414141414141 )
EOF
$RNDCCMD 10.53.0.3 addzone $zone \
'{ type primary; file "oversized-private-record.db"; allow-transfer { any; }; };' || ret=1
$RNDCCMD 10.53.0.7 addzone $zone \
'{ type secondary; file "oversized-private-record.bk"; primaries { 10.53.0.3;}; dnssec-policy default; };' || ret=1
sleep 1
dig_with_opts @10.53.0.7 $zone TYPE65534 >dig.out.ns7.test$n.soa || ret=1
grep "status: NOERROR" dig.out.ns7.test$n.soa >/dev/null || ret=1
grep "TYPE65534.\\\\# 262" dig.out.ns7.test$n.soa >/dev/null || ret=1
test "$ret" -eq 0 || echo_i "failed"
status=$((status + ret))
echo_i "exit status: $status"
[ $status -eq 0 ] || exit 1

View file

@ -16,8 +16,15 @@
#include <isc/types.h>
#include <dns/db.h>
#include <dns/nsec.h>
#include <dns/types.h>
/*
* The private record has one extra byte, containing a 0, in front of
* an NSEC3PARAM record.
*/
#define DNS_PRIVATE_BUFFERSIZE (DNS_NSEC3PARAM_BUFFERSIZE + 1)
#pragma once
isc_result_t

View file

@ -32,6 +32,7 @@
#include <dns/fixedname.h>
#include <dns/nsec.h>
#include <dns/nsec3.h>
#include <dns/private.h>
#include <dns/rdata.h>
#include <dns/rdatalist.h>
#include <dns/rdataset.h>
@ -1064,7 +1065,7 @@ dns_nsec3param_deletechains(dns_db_t *db, dns_dbversion_t *ver,
dns_rdataset_t rdataset;
bool flag;
isc_result_t result = ISC_R_SUCCESS;
unsigned char buf[DNS_NSEC3PARAM_BUFFERSIZE + 1];
unsigned char buf[DNS_PRIVATE_BUFFERSIZE];
dns_name_t *origin = dns_zone_getorigin(zone);
dns_rdatatype_t privatetype = dns_zone_getprivatetype(zone);
@ -1123,20 +1124,31 @@ try_private:
DNS_RDATASET_FOREACH(&rdataset) {
dns_rdata_t rdata = DNS_RDATA_INIT;
dns_rdataset_current(&rdataset, &rdata);
INSIST(rdata.length <= sizeof(buf));
memmove(buf, rdata.data, rdata.length);
/*
* Private NSEC3 record length >= 6.
* <0(1), hash(1), flags(1), iterations(2), saltlen(1)>
* Filter out non-private records. Delete private records that
* were being used to creating NSEC3 chains and add private
* records to remove that chains that were in the process of
* being constructed. If nonsec is set and there is a record
* saying to go insecure preserve it.
*
* Private records have 6 <= length <= 261 (6 + saltlen) and
* the following structure:
*
* <0(1), hash(1), flags(1), iterations(2), saltlen(1),
* salt(0..255)>
*/
if (rdata.length < 6 || buf[0] != 0 ||
(buf[2] & DNS_NSEC3FLAG_REMOVE) != 0 ||
(nonsec && (buf[2] & DNS_NSEC3FLAG_NONSEC) != 0))
if (rdata.length < 6 || rdata.data[0] != 0 ||
rdata.data[5] + 6 != rdata.length ||
(rdata.data[2] & DNS_NSEC3FLAG_REMOVE) != 0 ||
(nonsec && (rdata.data[2] & DNS_NSEC3FLAG_NONSEC) != 0))
{
continue;
}
INSIST(rdata.length <= sizeof(buf));
memmove(buf, rdata.data, rdata.length);
dns_difftuple_create(diff->mctx, DNS_DIFFOP_DEL, origin, 0,
&rdata, &tuple);
CHECK(do_one_tuple(&tuple, db, ver, diff));

View file

@ -2323,7 +2323,7 @@ add_nsec3param_records(ns_client_t *client, dns_zone_t *zone, dns_db_t *db,
isc_result_t result = ISC_R_SUCCESS;
dns_difftuple_t *newtuple = NULL;
dns_rdata_t rdata = DNS_RDATA_INIT;
unsigned char buf[DNS_NSEC3PARAM_BUFFERSIZE + 1];
unsigned char buf[DNS_PRIVATE_BUFFERSIZE];
dns_diff_t temp_diff;
dns_diffop_t op;
bool flag;

View file

@ -74,7 +74,7 @@ make_signing(signing_testcase_t *testcase, dns_rdata_t *private,
static void
make_nsec3(nsec3_testcase_t *testcase, dns_rdata_t *private,
unsigned char *pbuf) {
unsigned char *pbuf, size_t pbufsize) {
isc_result_t result;
dns_rdata_nsec3param_t params;
dns_rdata_t nsec3param = DNS_RDATA_INIT;
@ -119,7 +119,7 @@ make_nsec3(nsec3_testcase_t *testcase, dns_rdata_t *private,
dns_rdata_init(private);
dns_nsec3param_toprivate(&nsec3param, private, privatetype, pbuf,
DNS_NSEC3PARAM_BUFFERSIZE + 1);
pbufsize);
}
/* convert private signing records to text */
@ -179,13 +179,13 @@ ISC_RUN_TEST_IMPL(private_nsec3_totext) {
UNUSED(state);
for (i = 0; i < ncases; i++) {
unsigned char data[DNS_NSEC3PARAM_BUFFERSIZE + 1];
unsigned char data[DNS_PRIVATE_BUFFERSIZE];
char output[BUFSIZ];
isc_buffer_t buf;
isc_buffer_init(&buf, output, sizeof(output));
make_nsec3(&testcases[i], &private, data);
make_nsec3(&testcases[i], &private, data, sizeof(data));
dns_private_totext(&private, &buf);
assert_string_equal(output, results[i]);
}