The name 'isdelegation()' was confusing. This function is not checking
whether this message is a delegation, but whether the denial of
existence proofs in this message is a proof of a referral to an
unsigned zone.
The name 'is_unsecure_referral()' is more appropriate.
(cherry picked from commit e0f09bb374)
The isdelegation() was changed to return an isc_result_t because the
idea was to have a separate return value DNS_R_NSEC3ITERRANGE to signal
to the caller we could not verify the proof because of too many
iterations in the NSEC3 record, or perhaps ISC_R_UNEXPECTED for a more
generic cause that verification was not done.
But this would make error handling more fragile and all we care about
is whether we can reliably say the NS bit was not set.
If we can not reliably say so, we have to treat it as an insecure
referrral.
Since the answer is either yes or no, we can revert back to returning
a boolean value.
(cherry picked from commit 3ac1bb1c39)
Pass the SSU table through the update event struct from
send_update() to update_action() instead of reading it from the
zone twice. If rndc reconfig changed the zone's update policy
between the two reads (e.g., from allow-update to update-policy),
send_update() would skip the maxbytype allocation but
update_action() would see a non-NULL ssutable, triggering
INSIST(ssutable == NULL || maxbytype != NULL) and crashing named.
The ssutable reference is now taken once in send_update() and
transferred to update_action() via the event struct, ensuring
both functions see the same value.
(cherry picked from commit c172416559)
A helper macro that returns the current value of a pointer and sets
it to NULL in one expression, useful for transferring ownership in
designated initializers.
(cherry picked from commit 0f3be0beb8)
Move the write to fctx->vresult after LOCK(&fctx->lock). The field was
being set before acquiring the lock, but dns_resolver_logfetch() reads
it under the same lock from another thread.
(cherry picked from commit a2bd833909)
The previous code was incorrectly clearing errno after calling
strtol but before testing the result rather than clearing it and
then calling strtol so that changes to errno can be correctly
determined.
(cherry picked from commit d3ffa1f007)
Starting from OpenSSL 4 the the X509_get_subject_name() function
returns a 'const' pointer to a name instead of a regular pointer.
Duplicate the name before operating on it, then free it.
(cherry picked from commit 336c523b79)
In many places we only create a validator if the RRset has too low
trust (the RRset is pending validation, or could not be validated
before). This check was missing prior to validating negative response
data.
(cherry picked from commit 6ca67f65cd)
If we already marked an rdataset as secure (or it has even stronger
trust), there is no need to cryptographically verify it again.
(cherry picked from commit 0ec08c2120)
When looking up an NSEC3 as part of an insecurity proof, check the
number of iterations. If this is too high, treat the answer as insecure
by marking the answer with trust level "answer", indicating that they
did not validate, but could be cached as insecure.
(cherry picked from commit 988040a5e0)
A stale answer or SERVFAIL could have been served in case of multiple
upstream failures when following the CNAME chains. This has been fixed.
(cherry picked from commit d46277b398)
NSEC3 hashes are required to fit within a single DNS label. Since there
are 5 bits per label byte without pad characters, the maximum hash size
is floor(63*5/8) (39 bytes).
This patch enforces this maximum length for unknown algorithms, while
strictly enforcing the exact expected digest length for known algorithms
like SHA-1.
(cherry picked from commit 3801d0ebbf)
When .next_length is longer than NSEC3_MAX_HASH_LENGTH, it causes a
harmless out-of-bound read of the isdelegation() stack. This patch
fixes the issue by skipping NSEC3 records with an oversized hash length
during validation.
(cherry picked from commit 67b4fb56e4)
In dst_gssapi_acceptctx(), the gnamebuf could leak a little bit of
memory if dns_name_fromtext() would theoretically fail. This would
require a Kerberos principal with invalid DNS name.
(cherry picked from commit 3ad87f1ad6)
uint16_tobuffer was used instead of uint8_tobuffer when adding the
scheme to the buffer. This produced a record that was one octet
too long. This has been fixed.
(cherry picked from commit 3180e50459)
C23 now has qualifier preserving standard functions for strchr,
bsearch, strpbrk, strrchr, strstr, memchr. There where a few places
where the return value was not assigned to a const qualified pointer.
These have been fixed.
(cherry picked from commit af379e10cc)
Fix incorrect length checks in the towire_*() methods for BRID and HHIT
records to prevent assertion failures when trying to serve short
records.
(cherry picked from commit 14e299995f)
While building on uclibc this error is thrown:
In file included from ./include/dns/log.h:20,
from callbacks.c:19:
../../lib/isc/include/isc/log.h:141:9: error: unknown type name ‘off_t’
141 | off_t maximum_size;
| ^~~~~
This is due to missing include unistd.h, so let's add it on top of
isc/log.h
Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
(cherry picked from commit 0e43f62c12)
When switching from NSEC3 opt-out to NSEC, add NSEC records if we saw an
RR. This corrects a mistake in style cleanups done in commit
308ab1b4a5.
(cherry picked from commit 6f285bff6a)
when a query for type ANY triggers a delegation response, all
additional data was omitted from the response, including
mandatory glue. this has been corrected.
(cherry picked from commit aa13e62355)
dns_rbtnodechain_prev requires the current node to still be valid
which was not always the case after dereference_iter_node was called.
Move the call to dereference_iter_node to after the dns_rbtnodechain_prev
to preserve the node.
(cherry picked from commit b677d31fca)
previously, there were over 40 separate definitions of CHECK macros, of
which most used "goto cleanup", and the rest "goto failure" or "goto
out". there were another 10 definitions of RETERR, of which most were
identical to CHECK, but some simply returned a result code instead of
jumping to a cleanup label.
this has now been standardized throughout the code base: RETERR is for
returning an error code in the case of an error, and CHECK is for jumping
to a cleanup tag, which is now always called "cleanup". both macros are
defined in isc/util.h.
(cherry picked from commit 25c9fb54da)
(cherry picked from commit 52bba5cc34)
When looking for a signing key in select_signing_key(), the result code
indicating unsupported algorithm would abort the search. Instead, skip
such keys and continue searching for the right key.
Co-Authored-By: Aram Sargsyan <aram@isc.org>
Co-Authored-By: Petr Menšík <pemensik@redhat.com>
(cherry picked from commit a94a7c1a1e)
Since we are using system routines for randomness, there's no point
in spending time and run the statistical suite for testing PRNG.
(cherry picked from commit 90b3def5e9)
The selfsigned_dnskey() function currently returns boolean. There
was a recent change to make it return a isc_result_t error code,
which is implicitly converted to bool, which is obviously an error.
If instead of the result code we return true/false, it still doesn't
indicate the error to the caller that has happened before.
Change the function to return isc_result_t, and change the caller
routine to process the new return type.
It was discovered in an upcoming academic paper that a xoshiro128**
internal state can be recovered by an external 3rd party allowing to
predict UDP ports and DNS IDs in the outgoing queries. This could lead
to an attacker spoofing the DNS answers with great efficiency and
poisoning the DNS cache.
Change the internal random generator to system CSPRNG with buffering to
avoid excessive syscalls.
Thanks Omer Ben Simhon and Amit Klein of Hebrew University of Jerusalem
for responsibly reporting this to us. Very cool research!
(cherry picked from commit cffcab9d5f)
To prevent spoofed unsigned DNAME responses being accepted retry
response with unsigned DNAMEs over TCP if the response is not TSIG
signed or there isn't a good DNS CLIENT COOKIE.
To prevent test failures, this required adding TCP support to the
ans3 and ans4 servers in the chain system test.
(cherry picked from commit 2e40705c06)
Use the owner name of the NS record as the bailwick apex name
when determining which additional records to cache, rather than
the name of the delegating zone (or a parent thereof).
(cherry picked from commit a41054e9e6)
To prevent certain spoofing attacks, a new check has been added
to the existing rules for whether NS data can be cached: the owner
name of the NS RRset must be an ancestor of the name being queried.
(cherry picked from commit fa153f791f)
If a matching but cryptographically invalid key was encountered during
the DNSSEC validation, the key would be just skipped and not counted
towards validation failures. Treat such DNSSEC keys as hard failures
and fail the DNSSEC validation immediatelly instead of continuing the
DNSSEC validation with the next DNSKEYs in the RRset.
Co-authored-by: Matthijs Mekking <matthijs@isc.org>
(cherry picked from commit f00117a422)
The code to test whether to store the RRSIGs on DNS_R_UNCHANGED
with CD=1 was failing because the comparison methods of the two
rdatatset instances were not compatible. Move the testing into
dns_db_addrdataset(), and request it by setting the DNS_ADD_EQUALOK
option. If the option is set and the old and new rrsets compare
as equal, dns_db_addrdataset() returns ISC_R_SUCCESS instead of
DNS_R_UNCHANGED.
(cherry picked from commit b954a1df43)
Do not insert a NUL into ip_str too early as the full value
is needed later. Only insert the NUL immediately before
displaying just the prefix string.
(cherry picked from commit 283da99f02)
