Commit Graph

13136 Commits

Author SHA1 Message Date
Matthijs Mekking
c499478321 Clear dnssec-sign stats for removed keys
Clear the key slots for dnssec-sign statistics for keys that are
removed. This way, the number of slots will stabilize to the maximum
key usage in a zone and will not grow every time a key rollover is
triggered.

(cherry picked from commit de15e07800)
2021-08-24 09:51:45 +02:00
Matthijs Mekking
df6fb95621 Grow dnssec-sign statistics instead of rotating
We have introduced dnssec-sign statistics to the zone statistics. This
introduced an operational issue because when using zone-statistics
full, the memory usage was going through the roof. We fixed this by
by allocating just four key slots per zone. If a zone exceeds the
number of keys for example through a key rollover, the keys will be
rotated out on a FIFO basis.

This works for most cases, and fixes the immediate problem of high
memory usage, but if you sign your zone with many, many keys, or are
sign with a ZSK/KSK double algorithm strategy you may experience weird
statistics. A better strategy is to grow the number of key slots per
zone on key rollover events.

That is what this commit is doing: instead of rotating the four slots
to track sign statistics, named now grows the number of key slots
during a key rollover (or via some other method that introduces new
keys).

(cherry picked from commit d9cca81d50)
2021-08-24 09:51:45 +02:00
Matthijs Mekking
4a1987a380 Add a function isc_stats_resize
Add a new function to resize the number of counters in a statistics
counter structure. This will be needed when we keep track of DNSSEC
sign statistics and new keys are introduced due to a rollover.

(cherry picked from commit 9acce8a82a)
2021-08-24 09:51:45 +02:00
Matthijs Mekking
4f08beb1de Add stats unit test
Add a simple stats unit test that tests the existing library functions
isc_stats_ncounters, isc_stats_increment, isc_stats_decrement,
isc_stats_set, and isc_stats_update_if_greater.

(manually picked from commit 0bac9c7c5c)
2021-08-24 09:27:38 +02:00
Matthijs Mekking
db18004d69 Migrate a single key to CSK with dnssec-policy
When migrating keys to dnssec-policy, if a zone has only one key,
assume it is going to be a CSK.

(cherry picked from commit 3ea953512a)
2021-08-23 10:36:42 +02:00
Mark Andrews
7d3d7cacf9 Reject zero length ALPN elements in fromwire
(cherry picked from commit 8833d90292)
2021-08-19 18:59:29 +10:00
Mark Andrews
c9858fa078 Check that ALPN is present when NO-DEFAULT-ALPN is present in fromwire
(cherry picked from commit 2f51bb2d93)
2021-08-19 17:32:32 +10:00
Michal Nowak
ae370e3e61 Fix typos in lib/isc/trampoline_p.h 2021-08-19 07:20:15 +02:00
Ondřej Surý
607f8d114e Disable the Path MTU Discover on UDP Sockets
Instead of disabling the fragmentation on the UDP sockets, we now
disable the Path MTU Discovery by setting IP(V6)_MTU_DISCOVER socket
option to IP_PMTUDISC_OMIT on Linux and disabling IP(V6)_DONTFRAG socket
option on FreeBSD.  This option sets DF=0 in the IP header and also
ignores the Path MTU Discovery.

As additional mitigation on Linux, we recommend setting
net.ipv4.ip_no_pmtu_disc to Mode 3:

    Mode 3 is a hardend pmtu discover mode. The kernel will only accept
    fragmentation-needed errors if the underlying protocol can verify
    them besides a plain socket lookup. Current protocols for which pmtu
    events will be honored are TCP, SCTP and DCCP as they verify
    e.g. the sequence number or the association. This mode should not be
    enabled globally but is only intended to secure e.g. name servers in
    namespaces where TCP path mtu must still work but path MTU
    information of other protocols should be discarded. If enabled
    globally this mode could break other protocols.
2021-08-19 07:20:15 +02:00
Evan Hunt
15996f0cb1 ns_client_error() could assert if rcode was overridden to NOERROR
The client->rcode_override was originally created to force the server
to send SERVFAIL in some cases when it would normally have sent FORMERR.

More recently, it was used in a3ba95116e
commit (part of GL #2790) to force the sending of a TC=1 NOERROR
response, triggering a retry via TCP, when a UDP packet could not be
sent due to ISC_R_MAXSIZE.

This ran afoul of a pre-existing INSIST in ns_client_error() when
RRL was in use. the INSIST was based on the assumption that
ns_client_error() could never result in a non-error rcode. as
that assumption is no longer valid, the INSIST has been removed.
2021-08-19 07:20:15 +02:00
Mark Andrews
e3c22ec53a Check that the hostname of the server is legal
(cherry picked from commit f46a0c27df)
2021-08-18 16:54:31 +10:00
Mark Andrews
20cb00e1a5 add tests for string and qstring
(cherry picked from commit 26b22a1445)
2021-08-18 16:54:31 +10:00
Mark Andrews
2db56ffbd8 Add unit test for keypair
(cherry picked from commit a6357d8b5c)
2021-08-18 15:00:08 +10:00
Mark Andrews
c0c38eeb36 Add invalid test vectors
(cherry picked from commit bc21015438)
2021-08-18 14:59:29 +10:00
Mark Andrews
4fd35998e6 add text vs unknown test vectors
(cherry picked from commit 3e459b4808)
2021-08-18 14:59:29 +10:00
Mark Andrews
c7130b36fc Parse and print HTTPS and SVCB records
(cherry picked from commit 36f34a3e79)
2021-08-18 14:59:29 +10:00
Mark Andrews
cc93d10c82 Add support for parsing <tag>[=<value>]
where <value> may be a quoted string.  Previously quoted string
only supported opening quotes at the start of the string.

(cherry picked from commit 42c22670b3)
2021-08-18 14:59:29 +10:00
Mark Andrews
24e5e3ffd6 Make whether to follow additional data records generic
Adds dns_rdatatype_followadditional() and
DNS_RDATATYPEATTR_FOLLOWADDITIONAL

(cherry picked from commit f0265b8fa6)
2021-08-18 14:59:20 +10:00
Matthijs Mekking
5688bd31e3 Don't use stale nodes when looking up a zonecut
When looking up a zonecut in cache, we use 'dns_rbt_findnode' to find
the closest matching node. This function however does not take into
account stale nodes. When we do find a stale node and use it, this
has implications for subsequent lookups. For example, this may break
QNAME minimization because we are using a deeper zonecut than we should
have.

Check the header for staleness and if so, and stale entries are not
accepted, look for the deepest zonecut from this node up.

(cherry picked from commit bc448fb3b1)
2021-08-16 19:32:16 +02:00
Matthijs Mekking
5c23ec44bd Add extra checks for !ANCIENT(header)
There are some occurrences where we check if a header exists in the
rbtdb. These cases require that the header is also not marked as
ancient (aka ready for cleanup). These cases involve finding certain
data in cache.

(cherry picked from commit e2d4896864)
2021-08-16 16:42:41 +02:00
Mark Andrews
42856b25bd Don't freeze / thaw non-explict in-view zones
(cherry picked from commit dcdd9a403a)
2021-08-12 04:19:44 +00:00
Matthijs Mekking
4fec33fd20 Fix bug in dst_key_copymetadata
When copying metadata from one dst_key to another, when the source
dst_key has a boolean metadata unset, the destination dst_key will
have a numeric metadata unset instead.

This means that if a key has KSK or ZSK unset, we may be clearing the
Predecessor or Successor metadata in the destination dst_key.

(cherry picked from commit 94bb545087)
2021-08-11 15:18:10 +02:00
Mark Andrews
da13526669 Order the diff from dns_db_diffx so that deletes proceed adds
for the same rdataset.  This allows the diff when passed to
dns_diff_apply to succeed.

(cherry picked from commit 76453961bd)
2021-07-23 09:20:25 +10:00
Mark Andrews
37f6b31017 Record load time when a inline zone file has been touched
(cherry picked from commit 194e47cb0d)
2021-07-23 07:49:21 +10:00
Ondřej Surý
bceda720e4 Reduce the nodelock count for both cache and regular rbtdb
Increasing the nodelock count had major impact on the memory footprint
in scenarios where multiple rbtdb structure would be created like
hosting many zones in a single server.

This reverts commit 0344684385 and sets
the nodelock count to previously used values.
2021-07-21 17:03:33 +02:00
Mark Andrews
350605a3cc dns_rdata_tostruct() should reject rdata with DNS_RDATA_UPDATE set
(cherry picked from commit e97249e012)
2021-07-21 12:40:47 +10:00
Mark Andrews
498de906fa Check opcode of messages returned by dns_request_getresponse
(cherry picked from commit ed4e00713f)
2021-07-21 12:40:47 +10:00
Michał Kępień
f81c8e3e73 Tweak query_addds() comments to avoid confusion
It has been noticed that commit f88c90f47f
did not only fix NSEC record handling in signed, insecure delegations
prepared using both wildcard expansion and CNAME chaining - it also
inadvertently fixed DS record handling in signed, secure delegations
of that flavor.  This is because the 'rdataset' variable in the relevant
location in query_addds() can be either a DS RRset or an NSEC RRset.
Update a code comment in query_addds() to avoid confusion.

Update the comments describing the purpose of query_addds() so that they
also mention NSEC(3) records.

(cherry picked from commit 29d8d35869)
2021-07-16 07:24:34 +02:00
Mark Andrews
5d9dced395 zone->requeststats_on was not being set at the correct point
(cherry picked from commit 616896d735)
2021-07-16 14:13:49 +10:00
Matthijs Mekking
65f58d68f0 Relax zone_cdscheck function
If we have a CDS or CDNSKEY we at least need to have a DNSKEY with the
same algorithm published and signing the CDS RRset. Same for CDNSKEY
of course.

This relaxes the zone_cdscheck function, because before the CDS or
CDNSKEY had to match a DNSKEY, now only the algorithm has to match.

This allows a provider in a multisigner model to update the CDS/CDNSKEY
RRset in the zone that is served by the other provider.

(cherry picked from commit 577bf913b9)
2021-07-15 09:26:16 +02:00
Evan Hunt
312c78809a document isc__trampoline
Added some header file documentation to the isc__trampoline
implementation in trampoline_p.h.
2021-07-14 10:56:42 -07:00
Ondřej Surý
c546545d32 Disable setting the thread affinity
It was discovered that setting the thread affinity on both the netmgr
and netthread threads lead to inconsistent recursive performance because
sometimes the netmgr and netthread threads would compete over single
resource and sometimes not.

Removing setting the affinity causes a slight dip in the authoritative
performance around 5% (the measured range was from 3.8% to 7.8%), but
the recursive performance is now consistently good.

(cherry picked from commit a9e6a7ae57)
2021-07-13 15:47:13 +02:00
Matthijs Mekking
ed4358da37 Fix leak in checkds code
In 'checkds_send_toaddr' there is a goto bug that causes the TSIG key
and DNS message to not be detached. Remove the offending goto statement.

(cherry picked from commit b676163933)
2021-07-13 11:20:24 +02:00
Mark Andrews
928af4c424 Reset errcnt at the start of each subtest
(cherry picked from commit 3945c289bb)
2021-07-12 13:57:34 +10:00
Mark Andrews
8538c762cb Fix unchecked returns of dns_name_fromtext 2021-07-12 02:40:25 +00:00
Mark Andrews
bcaf23dd27 Fix unchecked return of isc_rwlock_lock and isc_rwlock_unlock 2021-07-12 02:40:25 +00:00
Mark Andrews
5f82841098 Silence untrusted loop bound on nsec3param.iterations
630
   	    1. tainted_argument: Calling function dns_rdata_tostruct taints argument nsec3param.iterations. [show details]
    631        result = dns_rdata_tostruct(nsec3rdata, &nsec3param, NULL);
   	    2. Condition !!(result == 0), taking true branch.
   	    3. Condition !!(result == 0), taking true branch.
    632        RUNTIME_CHECK(result == ISC_R_SUCCESS);
    633
    634        dns_fixedname_init(&fixed);

            CID 281425 (#1 of 1): Untrusted loop bound (TAINTED_SCALAR)
            4. tainted_data: Passing tainted expression nsec3param.iterations to dns_nsec3_hashname, which uses it as a loop boundary. [show details]
   	    Ensure that tainted values are properly sanitized, by checking that their values are within a permissible range.
    635        result = dns_nsec3_hashname(&fixed, rawhash, &rhsize, vctx->origin,
    636                                    vctx->origin, nsec3param.hash,
    637                                    nsec3param.iterations, nsec3param.salt,
    638                                    nsec3param.salt_length);

(cherry picked from commit c5e1c35e45)
2021-07-12 12:16:29 +10:00
Mark Andrews
ac34c3f552 Silence tainted scalar on rdlen
2042        ttl = isc_buffer_getuint32(&j->it.source);
    	    13. tainted_data_transitive: Call to function isc_buffer_getuint16 with tainted argument *j->it.source.base returns tainted data. [show details]
    	    14. var_assign: Assigning: rdlen = isc_buffer_getuint16(&j->it.source), which taints rdlen.
    2043        rdlen = isc_buffer_getuint16(&j->it.source);
    2044
    2045        /*
    2046         * Parse the rdata.
    2047         */
    	    15. Condition j->it.source.used - j->it.source.current != rdlen, taking false branch.
    2048        if (isc_buffer_remaininglength(&j->it.source) != rdlen) {
    2049                FAIL(DNS_R_FORMERR);
    2050        }
    	    16. var_assign_var: Assigning: j->it.source.active = j->it.source.current + rdlen. Both are now tainted.
    2051        isc_buffer_setactive(&j->it.source, rdlen);
    2052        dns_rdata_reset(&j->it.rdata);
    	    17. lower_bounds: Checking lower bounds of unsigned scalar j->it.source.active by taking the true branch of j->it.source.active > j->it.source.current.

    CID 316506 (#1 of 1): Untrusted loop bound (TAINTED_SCALAR)
    18. tainted_data: Passing tainted expression j->it.source.active to dns_rdata_fromwire, which uses it as a loop boundary. [show details]
    	    Ensure that tainted values are properly sanitized, by checking that their values are within a permissible range.
    2053        CHECK(dns_rdata_fromwire(&j->it.rdata, rdclass, rdtype, &j->it.source,
    2054                                 &j->it.dctx, 0, &j->it.target));

(cherry picked from commit f0fdca90f2)
2021-07-12 10:45:42 +10:00
Mark Andrews
b212d29a71 Silence use of tainted scalar
2607
            43. tainted_argument: Calling function journal_read_xhdr taints argument xhdr.size. [show details]
    2608                        result = journal_read_xhdr(j1, &xhdr);
            44. Condition rewrite, taking true branch.
            45. Condition result == 29, taking false branch.
    2609                        if (rewrite && result == ISC_R_NOMORE) {
    2610                                break;
    2611                        }
            46. Condition result != 0, taking false branch.
    2612                        CHECK(result);
    2613
            47. var_assign_var: Assigning: size = xhdr.size. Both are now tainted.
    2614                        size = xhdr.size;

            CID 331088 (#3 of 3): Untrusted allocation size (TAINTED_SCALAR)
            48. tainted_data: Passing tainted expression size to isc__mem_get, which uses it as an allocation size. [show details]
            Ensure that tainted values are properly sanitized, by checking that their values are within a permissible range.
    2615                        buf = isc_mem_get(mctx, size);

(cherry picked from commit 83fd38dd2c)
2021-07-12 10:45:42 +10:00
Matthijs Mekking
7e9fb5deda Fix CID 332468: Memory - illegal accesses (UNINIT)
*** CID 332468:  Memory - illegal accesses  (UNINIT)
/lib/dns/zone.c: 6613 in dns_zone_getdnsseckeys()
6607                 ISC_LIST_UNLINK(dnskeys, k1, link);
6608                 ISC_LIST_APPEND(*keys, k1, link);
6609             }
6610         }
6611     6612     failure:
>>>     CID 332468:  Memory - illegal accesses  (UNINIT)
>>>     Using uninitialized value "keyset.methods" when calling
>>>     "dns_rdataset_isassociated".
6613         if (dns_rdataset_isassociated(&keyset)) {
6614             dns_rdataset_disassociate(&keyset);
6615         }
6616         if (node != NULL) {
6617             dns_db_detachnode(db, &node);
6618         }

Fix by initializing the 'keyset' with 'dns_rdataset_init'.
2021-07-01 14:59:00 +02:00
Matthijs Mekking
e814422e23 Fix windows build
The checkds feature added new functions that required no specific
additional changes for Windows (because the Windows support has been
dropped), but for 9.16 we still need to define them in libdns.def.in.
2021-07-01 14:48:47 +02:00
Matthijs Mekking
dd92a7d5e3 Protect dst key metadata with lock
The DST key metadata can be written by several threads in parralel.
Protect the dst_key_get* and dst_key_set* functions with a mutex.

(cherry picked from commit 39df3f0475)
2021-07-01 14:48:47 +02:00
Matthijs Mekking
099a548340 Replace zone keyflock with zonemgr keymgmt
The old approach where each zone structure has its own mutex that
a thread needs to obtain multiple locks to do safe keyfile I/O
operations lead to a race condition ending in a possible deadlock.

Consider a zone in two views. Each such zone is stored in a separate
zone structure. A thread that needs to read or write the key files for
this zone needs to obtain both mutexes in seperate structures. If
another thread is working on the same zone in a different view, they
race to get the locks. It would be possible that thread1 grabs the
lock of the zone in view1, while thread2 wins the race for the lock
of the zone in view2. Now both threads try to get the other lock,  both
of them are already locked.

Ideally, when a thread wants to do key file operations, it only needs
to lock a single mutex. This commit introduces a key management hash
table, stored in the zonemgr structure. Each time a zone is being
managed, an object is added to the hash table (and removed when the
zone is being released). This object is identified by the zone name
and contains a mutex that needs to be locked prior to reading or
writing key files.

(cherry-picked from commit ef4619366d49efd46f9fae5f75c4a67c246ba2e6)

(cherry picked from commit 28c5179904)
2021-07-01 14:48:46 +02:00
Matthijs Mekking
d565dd6190 Add checkds code
Similar to notify, add code to send and keep track of checkds requests.

On every zone_rekey event, we will check the DS at parental agents
(but we will only actually query parental agents if theree is a DS
scheduled to be published/withdrawn).

On a zone_rekey event, we will first clear the ongoing checkds requests.
Reset the counter, to avoid continuing KSK rollover premature.

This has the risk that if zone_rekey events happen too soon after each
other, there are redundant DS queries to the parental agents. But
if TTLs and the configured durations in the dnssec-policy are sane (as
in not ridiculous short) the chance of this happening is low.

Update: Remove the TLS bits as this is not supported in 9.16

(cherry picked from commit f7872dbd20)
2021-07-01 14:48:23 +02:00
Matthijs Mekking
70cee781a1 Add checkds log notice
When the checkds published/withdrawn is activated, log a notice. Can
be used for testing, but also operationally useful.

(cherry picked from commit 1a50554963)
2021-07-01 14:48:23 +02:00
Matthijs Mekking
96d4f99a8f Add key metadata for DS published/withdrawn
In order to keep track of how many parents have the DS for a given key
published or withdrawn, keep a counter.

(cherry picked from commit 6e2c24be7c)
2021-07-01 14:48:23 +02:00
Matthijs Mekking
850aed0219 Add helpful function 'dns_zone_getdnsseckeys'
This code gathers DNSSEC keys from key files and from the DNSKEY RRset.
It is used for the 'rndc dnssec -status' command, but will also be
needed for "checkds". Turn it into a function.

(cherry picked from commit 40331a20c4)
2021-07-01 14:48:23 +02:00
Matthijs Mekking
9c0e252e2b Add "parental-source[-v6]" config option
Similar to "notify-source" and "transfer-source", add options to
set the source address when querying parental agents for DS records.

(manually picked from commit 2872d6a12e)
2021-07-01 14:48:23 +02:00
Matthijs Mekking
884750b66d Add dst_key_role function
Change the static function 'get_ksk_zsk' to a library function that
can be used to determine the role of a dst_key. Add checks if the
boolean parameters to store the role are not NULL. Rename to
'dst_key_role'.

(cherry picked from commit c9b7f62767)
2021-07-01 14:48:23 +02:00
Matthijs Mekking
63582dc778 Parse "parental-agents" configuration
Parse the new "parental-agents" configuration and store it in the zone
structure.

(cherry picked from commit 6f92d4b9a5)
2021-07-01 14:48:23 +02:00