Commit Graph
8173 Commits
Author SHA1 Message Date
Mark Andrews d329d47bae Prevent existing catalog zone entries being incorrectly deleted
After receiving a new version of a catalog zone it is required
to merge it with the old version.

The algorithm walks through the new version's hash table and applies
the following logic:

1. If an entry from the new version does not exist in the old
   version, then it's a new entry, add the entry to the `toadd` hash
   table.
2. If the zone does not exist in the set of configured zones, because
   it was deleted via rndc delzone or it was removed from another
   catalog zone instance, then add into to the `toadd` hash table to
   be reinstantiated.
3. If an entry from the new version also exists in the old version,
   but is modified, then add the entry to the `tomod` hash table, then
   remove it from the old version's hash table.
4. If an entry from the new version also exists in the old version and
   is the same (unmodified) then just remove it from the old version's
   hash table.

The algorithm then deletes all the remaining zones which still exist
in the old version's hash table (because only the ones that don't
exist in the new version should now remain there), then adds the ones
that were added to the `toadd`, and modifies the ones that were added
to the `tomod`, completing the merge.

During a recent refactoring, the part when the entry should be
removed from the old version's hash table on condition (4.) above
was accidentally omitted, so the unmodified zones were remaining
in the old version's hash table and consequently being deleted.

(cherry picked from commit 63145fb1d3)
2021-10-28 00:04:44 +11:00
Ondřej Surý 40fa423fd3 Add missing DNS_ZONEFLG_DUMPING to setmodtime()
It was found, that the original commit adding the setmodtime() was
incompletely squashed and there was double check for
DNS_ZONEFLG_NEEDDUMP instead of check for DNS_ZONEFLG_NEEDDUMP and
DNS_ZONEFLG_DUMPING.

Change the duplicate check to DNS_ZONEFLG_DUMPING.

(cherry picked from commit 55ac6b7394)
2021-10-21 12:33:24 +02:00
Ondřej Surý 093cd31ae2 Update the source code formatting using clang-format-13
clang-format-13 fixed some of the formatting that clang-format-12 got
wrong.  Update the formatting.

(cherry picked from commit ed95f9fba3)
2021-10-12 11:31:55 +02:00
Mark Andrews 187787c723 Fix cleanup of signature buffer in dns_dnssec_signmessage
A NULL pointer could be freed if error handling occured.

(cherry picked from commit 0378c05ba0)
2021-10-12 10:19:08 +11:00
Aram Sargsyan 311074f51e Handle a missing zone when reloading a catalog zone
Previously a missing/deleted zone which was referenced by a catalog
zone was causing a crash when doing a reload.

This commit will make `named` to ignore the fact that the zone is
missing, and make sure to restore it later on.

(cherry picked from commit 94a5712801)
2021-09-30 20:15:19 +00:00
Mark AndrewsandOndřej Surý 21771f5295 Pause the dbiterator before calling dns_db_find
zone.c:integrity_checks() acquires a read lock while iterating the
zone database, and calls zone_check_mx() which acquires another
read lock. If another thread tries to acquire a write lock in the
meantime, it can deadlock. Calling dns_dbiterator_pause() to release
the first read lock prevents this.

(cherry picked from commit 4e1faa35d5)
2021-09-29 18:04:14 +02:00
Evan Hunt e9d62d2442 map files over 2GB could not be loaded
- fixed a size comparison using "signed int" that failed if the file
  size was more than 2GB, since that was treated as a negative number.
- incidentally renamed deserialize32() to just deserialize(). we no
  longer have separate 32 and 64 bit rbtdb implementations.
2021-09-01 00:43:54 -07:00
Evan Hunt ddc677ae64 rename dns_zone_master and dns_zone_slave
dns_zone_master and dns_zone_slave are renamed as dns_zone_primary
and dns_zone_secondary.

(cherry picked from commit 916760ae46)
2021-08-30 11:58:29 -07:00
Petr ŠpačekandEvan Hunt b70a2c2d07 increase MAPAPI
bump the map zonefile version number to avoid an assertion
failure when loading map files from versions of BIND prior to
the most recent change to the in-memory structure of zone
databases.

(cherry picked from commit 4a68c7be22)
2021-08-28 07:45:39 -07:00
Diego FronzaandEvan Hunt f262860d5d Replace literal 255 with a more descriptive macro name
More details on thread:
https://gitlab.isc.org/isc-projects/bind9/-/merge_requests/291#note_12186

(cherry picked from commit bd0cc048d1)
2021-08-25 15:57:35 -07:00
Diego FronzaandEvan Hunt caa4af08d4 Avoid an empty block under if condition
This commit doesn't change the logic flow from previous code, it only
makes the code more readable and consistent.

More details on thread:
https://gitlab.isc.org/isc-projects/bind9/-/merge_requests/291#note_12185

(cherry picked from commit 90e724af45)
2021-08-25 15:57:35 -07:00
Diego FronzaandEvan Hunt 7af2573219 Remove redundant function 'newchain'
The removed function 'newchain(a, b)' was almost the same as calling
!chain_equal(a, b), varying only in the amount of data compared
in the non-fixed-length data portion of given chain nodes.

A third argument 'data_size' has been introduced into 'chain_equal'
function in order to allow it to know how many bytes to compare in the
variable-length data portion of the chain nodes.

A helper function 'chain_length(e)' has been introduced to allow
easy calculation of the total length of the non-fixed-length data part
of chain nodes.

Check the thread below for more details:
https://gitlab.isc.org/isc-projects/bind9/-/merge_requests/291#note_12184

(cherry picked from commit 37f42d19a1)
2021-08-25 15:57:35 -07:00
Diego FronzaandEvan Hunt 063bc21543 Use sizeof instead of arbitrary number to iterate fixed size array
More details on thread:
https://gitlab.isc.org/isc-projects/bind9/-/merge_requests/291#note_12180

(cherry picked from commit 6a12e37382)
2021-08-25 15:57:34 -07:00
Diego FronzaandEvan Hunt 9954c2da91 Initialize buffers with { 0 } instead of memset
More details on threads:
https://gitlab.isc.org/isc-projects/bind9/-/merge_requests/291#note_12178
https://gitlab.isc.org/isc-projects/bind9/-/merge_requests/291#note_12181

(cherry picked from commit 87e970474f)
2021-08-25 15:57:34 -07:00
Diego FronzaandEvan Hunt 9f5c643769 Minor refactoring on function match_nsec3
The logic for matching a set of nsec3 objects against an nsec3param
object was moved to a specific function.

For more details check thread:
https://gitlab.isc.org/isc-projects/bind9/-/merge_requests/291#note_12176

(cherry picked from commit 98d1e40143)
2021-08-25 15:57:34 -07:00
Diego FronzaandEvan Hunt ebb827c1d5 Minor refactoring/cleanup
This commit moves the warning message to the logical block where it
belongs better.

For more details check thread comment:
https://gitlab.isc.org/isc-projects/bind9/merge_requests/291#note_12167

(cherry picked from commit 966c06a9e6)
2021-08-25 15:57:34 -07:00
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 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
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 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 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
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
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 8538c762cb Fix unchecked returns of dns_name_fromtext 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