Commit Graph
33276 Commits
Author SHA1 Message Date
Mark Andrews bef5e723b2 Add CHANGES note for [GL #2460] 2021-02-09 12:30:14 +00:00
Mark Andrews 0a966315b2 Fix wrong length passed to isc_mem_put
If an invalid key name (e.g. "a..b") in a primaries list in named.conf
is specified the wrong size is passed to isc_mem_put resulting in the
returned memory being put on the wrong freed list.

    *** CID 316784:  Incorrect expression  (SIZEOF_MISMATCH)
    /bin/named/config.c: 636 in named_config_getname()
    630     	isc_buffer_constinit(&b, objstr, strlen(objstr));
    631     	isc_buffer_add(&b, strlen(objstr));
    632     	dns_fixedname_init(&fname);
    633     	result = dns_name_fromtext(dns_fixedname_name(&fname), &b, dns_rootname,
    634     				   0, NULL);
    635     	if (result != ISC_R_SUCCESS) {
       CID 316784:  Incorrect expression  (SIZEOF_MISMATCH)
       Passing argument "*namep" of type "dns_name_t *" and argument "8UL /* sizeof (*namep) */" to function "isc__mem_put" is suspicious.
    636     		isc_mem_put(mctx, *namep, sizeof(*namep));
    637     		*namep = NULL;
    638     		return (result);
    639     	}
    640     	dns_name_dup(dns_fixedname_name(&fname), mctx, *namep);
    641
2021-02-09 12:30:14 +00:00
Matthijs Mekking 5c0847e997 Merge branch '1810-refactor-ecdsa-eddsa-system-tests' into 'main'
Resolve "Refactor ecdsa and eddsa tests after testcrypto.sh changes"

Closes #1810

See merge request isc-projects/bind9!4645
2021-02-09 11:59:20 +00:00
Matthijs Mekking 51827ddcd3 Update copyrights for [#1810] 2021-02-09 11:59:08 +00:00
Matthijs Mekking 650b0d4691 Refactor ecdsa system test
Similar to eddsa system test.
2021-02-09 11:59:08 +00:00
Matthijs Mekking fd7d0f7968 Enable eddsa test
It should be fixed now.
2021-02-09 11:59:08 +00:00
Matthijs Mekking 8bf31d0592 Refactor eddsa system test
Test for Ed25519 and Ed448. If both algorithms are not supported, skip
test. If only one algorithm is supported, run test, skip the
unsupported algorithm. If both are supported, run test normally.

Create new ns3. This will test Ed448 specifically, while now ns2 only
tests Ed25519. This moves some files from ns2/ to ns3/.
2021-02-09 11:59:08 +00:00
Matthijs Mekking 572d7ec3b7 Fix testcrypto.sh
Testing Ed448 was actually testing Ed25519.
2021-02-09 11:59:08 +00:00
Michal Nowak e74187a056 Merge branch 'mnowak/drop-kyua-references-in-.gitlab-ci.yml' into 'main'
Remove remnant Kyua references

See merge request isc-projects/bind9!4638
2021-02-09 11:48:07 +00:00
Michal Nowak f557480078 Remove remnant Kyua references
Unit tests were ported from Kyua to Automake.  All references to Kyua
thus should be removed from the main branch.
2021-02-09 12:45:53 +01:00
Michal Nowak 566b65e513 Merge branch 'mnowak/check-asan-errors-in-configure' into 'main'
Check config.log for ASAN errors

See merge request isc-projects/bind9!4655
2021-02-09 11:02:20 +00:00
Michal Nowak 0db934d401 Check config.log for ASAN errors
./configure checks might produce a false negative error due to ASAN
errors and thus disable some options.
2021-02-09 11:56:08 +01:00
Matthijs Mekking 8dd1106bda Merge branch '2434-fetch-limit-serve-stale-follow-up' into 'main'
Resolve "Serve stale when fetch limits are hit" (follow-up)

Closes #2434

See merge request isc-projects/bind9!4654
2021-02-08 15:01:07 +00:00
Matthijs Mekking 00f575e7ef Adjust serve-stale test
The number of queries to use in the burst can be reduced, as we have
a very low fetch limit of 1.

The dig command in 'wait_for_fetchlimits()' should time out sooner as
we expect a SERVFAIL to be returned promptly.

Enabling serve-stale can be done before hitting fetch-limits. This
reduces the chance that the resolver queries time out and fetch count
is reset. The chance of that happening is already slim because
'resolver-query-timeout' is 10 seconds, but better to first let the
data become stale rather than doing that while attempting to resolve.
2021-02-08 15:17:09 +01:00
Matthijs Mekking 8bcd7fe69e Use stale on error also when unable to recurse
The 'query_usestale()' function was only called when in
'query_gotanswer()' and an unexpected error occurred. This may have
been "quota reached", and thus we were in some cases returning
stale data on fetch-limits (and if serve-stale enabled of course).

But we can also hit fetch-limits when recursing because we are
following a referral (in 'query_notfound()' and
'query_delegation_recurse()'). Here we should also check for using
stale data in case an error occurred.

Specifically don't check for using stale data when refetching a
zero TTL RRset from cache.

Move the setting of DNS_DBFIND_STALESTART into the 'query_usestale()'
function to avoid code duplication.
2021-02-08 15:17:09 +01:00
Mark Andrews a415424339 Merge branch '2469-cid-281461-untrusted-loop-bound' into 'main'
Resolve "CID 281461: untrusted loop bound"

Closes #2469

See merge request isc-projects/bind9!4642
2021-02-08 02:55:31 +00:00
Mark Andrews 2f946c831a Attempt to silence untrusted loop bound
Assign hit_len + key_len to len and test the result
rather than recomputing and letting the compiler simplify.

    213        isc_region_consume(&region, 2); /* hit length + algorithm */
        9. tainted_return_value: Function uint16_fromregion returns tainted data. [show details]
        10. tainted_data_transitive: Call to function uint16_fromregion with tainted argument *region.base returns tainted data.
        11. tainted_return_value: Function uint16_fromregion returns tainted data.
        12. tainted_data_transitive: Call to function uint16_fromregion with tainted argument *region.base returns tainted data.
        13. var_assign: Assigning: key_len = uint16_fromregion(&region), which taints key_len.
    214        key_len = uint16_fromregion(&region);
        14. lower_bounds: Casting narrower unsigned key_len to wider signed type int effectively tests its lower bound.
        15. Condition key_len == 0, taking false branch.
    215        if (key_len == 0) {
    216                RETERR(DNS_R_FORMERR);
    217        }
        16. Condition !!(_r->length >= _l), taking true branch.
        17. Condition !!(_r->length >= _l), taking true branch.
    218        isc_region_consume(&region, 2);
        18. lower_bounds: Casting narrower unsigned key_len to wider signed type int effectively tests its lower bound.
        19. Condition region.length < (unsigned int)(hit_len + key_len), taking false branch.
    219        if (region.length < (unsigned)(hit_len + key_len)) {
    220                RETERR(DNS_R_FORMERR);
    221        }
    222
        20. lower_bounds: Casting narrower unsigned key_len to wider signed type int effectively tests its lower bound.
        21. Condition _r != 0, taking false branch.
    223        RETERR(mem_tobuffer(target, rr.base, 4 + hit_len + key_len));
        22. lower_bounds: Casting narrower unsigned key_len to wider signed type int effectively tests its lower bound.
        23. var_assign_var: Compound assignment involving tainted variable 4 + hit_len + key_len to variable source->current taints source->current.
    224        isc_buffer_forward(source, 4 + hit_len + key_len);
    225
    226        dns_decompress_setmethods(dctx, DNS_COMPRESS_NONE);

    CID 281461 (#1 of 1): Untrusted loop bound (TAINTED_SCALAR)
        24. tainted_data: Using tainted variable source->active - source->current as a loop boundary.
    Ensure that tainted values are properly sanitized, by checking that their values are within a permissible range.
    227        while (isc_buffer_activelength(source) > 0) {
    228                dns_name_init(&name, NULL);
    229                RETERR(dns_name_fromwire(&name, source, dctx, options, target));
    230        }
2021-02-08 02:02:29 +00:00
Michal Nowak 64d5dad92a Merge branch 'mnowak/check-arm-pdf-validity' into 'main'
Check PDF file structure with QPDF

See merge request isc-projects/bind9!4620
2021-02-03 16:41:06 +00:00
Michal Nowak 359708b9d6 Check PDF file structure with QPDF
"qpdf --check" checks file structure of generated ARM PDF.
2021-02-03 17:39:58 +01:00
Matthijs Mekking 3648eb2936 Merge branch '2377-allow-a-records-below-an-_spf-label-as-a-check-names-exception' into 'main'
Resolve "Allow A records below an '_spf' label as a check-names exception"

Closes #2377

See merge request isc-projects/bind9!4529
2021-02-03 16:38:48 +00:00
Mark AndrewsandMatthijs Mekking 1294918702 Add release note entry 2021-02-03 16:24:44 +01:00
Mark AndrewsandMatthijs Mekking 2b5091ac17 Add CHANGES 2021-02-03 16:24:43 +01:00
Mark AndrewsandMatthijs Mekking a3b2b86e7f Check that A record is accepted with _spf label present 2021-02-03 16:23:20 +01:00
Mark AndrewsandMatthijs Mekking 63c16c8506 Allow A records below '_spf' labels as recommend by RFC7208 2021-02-03 16:23:20 +01:00
Matthijs Mekking 5b8c86606e Merge branch '2375-dnssec-policy-three-is-a-crowd-rollover-bug' into 'main'
Resolve "three is a crowd" dnssec-policy key rollover bug

Closes #2375

See merge request isc-projects/bind9!4541
2021-02-03 15:22:47 +00:00
Matthijs Mekking 189f5a3f28 Add kasp test todo for [#2375]
This bugfix has been manually verified but is missing a unit test.
Created GL #2471 to track this.
2021-02-03 15:35:06 +01:00
Matthijs Mekking 98ace6d97d Use NUM_KEYSTATES constant where appropriate
We use the number 4 a lot when working on key states. Better to use
the NUM_KEYSTATES constant instead.
2021-02-03 15:35:06 +01:00
Matthijs Mekking 7947f7f9c6 Add change and release note for [#2375]
News worthy.
2021-02-03 15:35:06 +01:00
Matthijs Mekking 189d9a2d21 Cleanup keymgr.c
Three small cleanups:

1. Remove an unused keystr/dst_key_format.
2. Initialize a dst_key_state_t state with NA.
3. Update false comment about local policy (local policy only adds
   barrier on transitions to the RUMOURED state, not the UNRETENTIVE
   state).
2021-02-03 15:35:06 +01:00
Matthijs Mekking 291bcc3721 Fix DS/DNSKEY hidden or chained functions
There was a bug in function 'keymgr_ds_hidden_or_chained()'.

The funcion 'keymgr_ds_hidden_or_chained()' implements (3e) of rule2
as defined in the "Flexible and Robust Key Rollover" paper. The rules
says: All DS records need to be in the HIDDEN state, or if it is not
there must be a key with its DNSKEY and KRRSIG in OMNIPRESENT, and
its DS in the same state as the key in question. In human langauge,
if all keys have their DS in HIDDEN state you can do what you want,
but if a DS record is available to some validators, there must be
a chain of trust for it.

Note that the barriers on transitions first check if the current
state is valid, and then if the next state is valid too. But
here we falsely updated the 'dnskey_omnipresent' (now 'dnskey_chained')
with the next state. The next state applies to 'key' not to the state
to be checked. Updating the state here leads to (true) always, because
the key that will move its state will match the falsely updated
expected state. This could lead to the assumption that Key 2 would be
a valid chain of trust for Key 1, while clearly the presence of any
DS is uncertain.

The fix here is to check if the DNSKEY and KRRSIG are in OMNIPRESENT
state for the key that does not have its DS in the HIDDEN state, and
only if that is not the case, ensure that there is a key with the same
algorithm, that provides a valid chain of trust, that is, has its
DNSKEY, KRRSIG, and DS in OMNIPRESENT state.

The changes in 'keymgr_dnskey_hidden_or_chained()' are only cosmetical,
renaming 'rrsig_omnipresent' to 'rrsig_chained' and removing the
redundant initialization of the DST_KEY_DNSKEY expected state to NA.
2021-02-03 15:34:36 +01:00
Matthijs Mekking 600915d1b2 Update keymgr_key_is_successor() calls
The previous commit changed the function definition of
'keymgr_key_is_successor()', this commit updates the code where
this function is called.

In 'keymgr_key_exists_with_state()' the logic is also updated slightly
to become more readable. First handle the easy cases:
- If the key does not match the state, continue with the next key.
- If we found a key with matching state, and there is no need to
  check the successor relationship, return (true).
- Otherwise check the successor relationship.

In 'keymgr_key_has_successor()' it is enough to check if a key has
a direct successor, so instead of calling 'keymgr_key_is_successor()',
we can just check 'keymgr_direct_dep()'.

In 'dns_keymgr_run()', we want to make sure that there is no
dependency on the keys before retiring excess keys, so replace
'keymgr_key_is_successor()' with 'keymgr_dep()'.
2021-02-03 15:34:36 +01:00
Matthijs Mekking cc38527b63 Implement Equation(2) of "Flexible Key Rollover"
So far the key manager could only deal with two keys in a rollover,
because it used a simplified version of the successor relationship
equation from "Flexible and Robust Key Rollover" paper. The simplified
version assumes only two keys take part in the key rollover and it
for that it is enough to check the direct relationship between two
keys (is key x the direct predecessor of key z and is key z the direct
successor of key x?).

But when a third key (or more keys) comes into the equation, the key
manager would assume that one key (or more) is redundant and removed
it from the zone prematurely.

Fix by implementing Equation(2) correctly, where we check for
dependencies on keys:

z ->T x: Dep(x, T) = ∅ ∧
         (x ∈ Dep(z, T) ∨
          ∃ y ∈ Dep(z, T)(y != z ∧ y ->T x ∧ DyKyRySy = DzKzRzSz))

This says: key z is a successor of key x if:
- key x depends on key z if z is a direct successor of x,
- or if there is another key y that depends on key z that has identical
  key states as key z and key y is a successor of key x.
- Also, key x may not have any other keys depending on it.

This is still a simplified version of Equation(2) (but at least much
better), because the paper allows for a set of keys to depend on a
key. This is defined as the set Dep(x, T). Keys in the set Dep(x, T)
have a dependency on key x for record type T. The BIND implementation
can only have one key in the set Dep(x, T). The function
'keymgr_dep()' stores this key in 'uint32_t *dep' if there is a
dependency.

There are two scenarios where multiple keys can depend on a single key:

1. Rolling keys is faster than the time required to finish the
   rollover procedure. This scenario is covered by the recursive
   implementation, and checking for a chain of direct dependencies
   will suffice.

2. Changing the policy, when a zone is requested to be signed with
   a different key length for example. BIND 9 will not mark successor
   relationships in this case, but tries to move towards the new
   policy. Since there is no successor relationship, the rules are
   even more strict, and the DNSSEC reconfiguration is actually slower
   than required.

Note: this commit breaks the build, because the function definition
of 'keymgr_key_is_successor' changed. This will be fixed in the
following commit.
2021-02-03 15:34:36 +01:00
Ondřej Surý de8058e828 Merge branch '2468-cid-318094-null-pointer-dereferences-reverse_inull' into 'main'
Resolve "CID 318094:  Null pointer dereferences  (REVERSE_INULL)"

Closes #2468

See merge request isc-projects/bind9!4641
2021-02-03 12:08:52 +00:00
Mark AndrewsandOndřej Surý 456d53d1fb Remove redundant 'version == NULL' check
*** CID 318094:  Null pointer dereferences  (REVERSE_INULL)
    /lib/dns/rbtdb.c: 1389 in newversion()
    1383     	version->xfrsize = rbtdb->current_version->xfrsize;
    1384     	RWUNLOCK(&rbtdb->current_version->rwlock, isc_rwlocktype_read);
    1385     	rbtdb->next_serial++;
    1386     	rbtdb->future_version = version;
    1387     	RBTDB_UNLOCK(&rbtdb->lock, isc_rwlocktype_write);
    1388
       CID 318094:  Null pointer dereferences  (REVERSE_INULL)
       Null-checking "version" suggests that it may be null, but it has already been dereferenced on all paths leading to the check.
    1389     	if (version == NULL) {
    1390     		return (result);
    1391     	}
    1392
    1393     	*versionp = version;
    1394
2021-02-03 13:06:27 +01:00
Ondřej Surý adf5051afc Merge branch '1144-dns-over-https-server' into 'main'
Resolve "Encrypted DNS - RFC 8484, DNS over HTTPS, DOH (also DoT comments)"

Closes #1144

See merge request isc-projects/bind9!4644
2021-02-03 12:01:47 +00:00
Evan HuntandOndřej Surý 91718fe4fb CHANGES, release notes 2021-02-03 12:06:17 +01:00
Ondřej Surý 0aacabc6dc Drop gcc:sid:i386 from GitLab CI
Building sid-i386 in Docker no longer works and we don't have a viable
alternative now, so dropping gcc:sid:i386 is our only option in this
very moment.
2021-02-03 12:06:17 +01:00
Evan HuntandOndřej Surý fe99484e14 support "tls ephemeral" with https 2021-02-03 12:06:17 +01:00
Evan HuntandOndřej Surý aa9d51c494 tls and http configuration code was unnecessarily complex
removed the isc_cfg_http_t and isc_cfg_tls_t structures
and the functions that loaded and accessed them; this can
be done using normal config parser functions.
2021-02-03 12:06:17 +01:00
Ondřej Surý 1cc24a2c8b Unit-test fixes and manual page updates for DoH configuration
This commit contains fixes to unit tests to make them work well on
various platforms (in particular ones shipping old versions of
OpenSSL) and for different configurations.

It also updates the generated manpage to include DoH configuration
options.
2021-02-03 12:06:17 +01:00
Artem BoldarievandOndřej Surý 08da09bc76 Initial support for DNS-over-HTTP(S)
This commit completes the support for DNS-over-HTTP(S) built on top of
nghttp2 and plugs it into the BIND. Support for both GET and POST
requests is present, as required by RFC8484.

Both encrypted (via TLS) and unencrypted HTTP/2 connections are
supported. The latter are mostly there for debugging/troubleshooting
purposes and for the means of encryption offloading to third-party
software (as might be desirable in some environments to simplify TLS
certificates management).
2021-02-03 12:06:17 +01:00
Witold KręcickiandOndřej Surý 7a96081360 nghttp2-based HTTP layer in netmgr
This commit includes work-in-progress implementation of
DNS-over-HTTP(S).

Server-side code remains mostly untested, and there is only support
for POST requests.
2021-02-03 12:06:17 +01:00
Witold KręcickiandOndřej Surý cdf9d21731 Add isc_mem_strndup() function
This commit adds an implementation of strndup() function which
allocates memory from the supplied isc_mem_t memory context.
2021-02-03 12:06:17 +01:00
Evan HuntandOndřej Surý 931ccd225f update ARM with "http" grammar
add a link to the http statement grammar and explanations and examples
for configuring DoH listeners.
2021-02-03 12:06:17 +01:00
Evan HuntandOndřej Surý 06951472dd Add parser support for DoH configuration options
This commit adds stub parser support and tests for:
- an "http" global option for HTTP/2 endpoint configuration.
- command line options to set http or https port numbers by
  specifying -p http=PORT or -p https=PORT.  (NOTE: this change
  only affects syntax; specifying HTTP and HTTPS ports on the
  command line currently has no effect.)
- named.conf options "http-port" and "https-port"
- HTTPSPORT environment variable for use when running tests.
2021-02-03 12:06:17 +01:00
Artem BoldarievandOndřej Surý 6b9a31989c Resurrect old TLS code
This commit resurrects the old TLS code from
8f73c70d23.

It also includes numerous stability fixes and support for
isc_nm_cancelread() for the TLS layer.

The code was resurrected to be used for DoH.
2021-02-03 12:06:17 +01:00
Michał Kępień 677cc94671 Merge branch '2448-tweak-sphinx-build-invocations' into 'main'
Tweak sphinx-build invocations

Closes #2448

See merge request isc-projects/bind9!4640
2021-02-03 10:57:51 +00:00
Michał Kępień 51479ed9a3 Make sphinx-build warnings fatal
In order to prevent documentation building issues from being glossed
over, pass the -W command line switch to all sphinx-build invocations.
This causes the latter to return with a non-zero exit code whenever any
Sphinx warnings are triggered.
2021-02-03 11:44:02 +01:00
Michał Kępień 84862e96c1 Address a Sphinx duplicate label warning
Both doc/man/ddns-confgen.rst and doc/man/tsig-keygen.rst include
bin/confgen/tsig-keygen.rst, which defines a "man_tsig-keygen" label.
This triggers the following warning when running sphinx-build with the
-W command line switch in the doc/man/ directory:

    ../../bin/confgen/tsig-keygen.rst:27: WARNING: duplicate label man_tsig-keygen, other instance in /tmp/bind9/doc/man/ddns-confgen.rst

Move the offending label from bin/confgen/tsig-keygen.rst to the proper
spot in doc/arm/manpages.rst to avoid effectively defining it twice in
different source documents while still allowing the relevant man page to
be referenced in the ARM.  Also rename that label so that it more
closely matches the content it points to.  As the label no longer
immediately precedes a section title in its new location, use
:ref:`Title <label>` syntax for the only reference to the
tsig-keygen/ddns-confgen man page in the ARM.
2021-02-03 11:44:02 +01:00
Michał Kępień 33db7ea16d Use separate sphinx-build cache directories
Simultaneously starting multiple sphinx-build instances with the -d
command line switch set to a common value (which is what happens when
e.g. "make -j6 doc" is run) causes intermittent problems which we failed
to notice before because they only trigger Sphinx warnings, not errors,
e.g.:

    WARNING: toctree contains ref to nonexisting file 'reference'

The message above is not triggered because doc/arm/reference.rst is
actually missing from disk at any point, but rather because a temporary
file created by one sphinx-build instance gets truncated by another one
working in parallel (the confusing message quoted above is logged
because of an overly broad "except" statement in Sphinx code).

Prevent this problem from being triggered by making each sphinx-build
process use its own dedicated cache directory.
2021-02-03 11:44:02 +01:00