Our vcxproj files set the WarningLevel to Level3, which is too verbose
for a code that needs to be portable. That basically leads to ignoring
all the errors that MSVC produces. This commits downgrades the
WarningLevel to Level1 and enables treating warnings as errors for
Release builds. For the Debug builds the WarningLevel got upgraded to
Level4, and treating warnings as errors is explicitly disabled.
We should eventually make the code clean of all MSVC warnings, but it's
a long way to go for Level4, so it's more reasonable to start at Level1.
For reference[1], these are the warning levels as described by MSVC
documentation:
* /W0 suppresses all warnings. It's equivalent to /w.
* /W1 displays level 1 (severe) warnings. /W1 is the default setting
in the command-line compiler.
* /W2 displays level 1 and level 2 (significant) warnings.
* /W3 displays level 1, level 2, and level 3 (production quality)
warnings. /W3 is the default setting in the IDE.
* /W4 displays level 1, level 2, and level 3 warnings, and all level 4
(informational) warnings that aren't off by default. We recommend
that you use this option to provide lint-like warnings. For a new
project, it may be best to use /W4 in all compilations. This option
helps ensure the fewest possible hard-to-find code defects.
* /Wall displays all warnings displayed by /W4 and all other warnings
that /W4 doesn't include — for example, warnings that are off by
default.
* /WX treats all compiler warnings as errors. For a new project, it
may be best to use /WX in all compilations; resolving all warnings
ensures the fewest possible hard-to-find code defects.
1. https://docs.microsoft.com/en-us/cpp/build/reference/compiler-option-warning-level?view=vs-2019
(cherry picked from commit 789d253e3d)
BIND wasn't honoring option "deny-answer-aliases" when configured to
forward queries.
Before the fix it was possible for nameservers listed in "forwarders"
option to return CNAME answers pointing to unrelated domains of the
original query, which could be used as a vector for rebinding attacks.
The fix ensures that BIND apply filters even if configured as a forwarder
instance.
The 3 warnings reported are:
os.c:872:7: warning: Although the value stored to 'ptr' is used in the enclosing expression, the value is never actually read from 'ptr'
if ((ptr = strtok_r(command, " \t", &last)) == NULL) {
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
--
rpz.c:1117:10: warning: Although the value stored to 'zbits' is used in the enclosing expression, the value is never actually read from 'zbits'
return (zbits &= x);
^ ~
1 warning generated.
--
openssleddsa_link.c:532:10: warning: Although the value stored to 'err' is used in the enclosing expression, the value is never actually read from 'err'
while ((err = ERR_get_error()) != 0) {
^ ~~~~~~~~~~~~~~~
1 warning generated.
(cherry picked from commit 262f087bcf)
Bumped the version file and added release line in CHANGES.
API files:
- lib/bind9/api:
No changes because only changes in comments.
- lib/dns/api:
Increment LIBINTERFACE because of the added field structure in
dns_struct_update.
- lib/isc/api:
Increment LIBINTERFACE because of the PKCS#11 replacement.
- lib/isccc/api:
No changes because no source code changes.
- lib/isccfg/api:
Increment LIBREVISION because of minor source code changes.
- lib/lwres/api:
No changes because no source code changes.
I decided no changes to README.md or the release notes were necessary.
Perflab graphs look sane.
On Windows, C11 localtime_r() and gmtime_r() functions are not
available. While localtime() and gmtime() functions are already thread
safe because they use Thread Local Storage, it's quite ugly to #ifdef
around every localtime_r() and gmtime_r() usage to make the usage also
thread-safe on POSIX platforms.
The commit adds wrappers around Windows localtime_s() and gmtime_s()
functions.
NOTE: The implementation of localtime_s and gmtime_s in Microsoft CRT
are incompatible with the C standard since it has reversed parameter
order and errno_t return type.
(cherry picked from commit 08f4c7d6c0)
some empty conditional branches which contained a semicolon were
"fixed" by clang-format to contain nothing. add comments to prevent this.
(cherry picked from commit 735be3b816)
Save 'i' to 'locknum' and use that rather than using
'header->node->locknum' when performing the deferred
unlock as 'header->node->locknum' can theoretically be
different to 'i'.
(cherry picked from commit 8dd8d48c9f)
cppcheck 1.90 reports the following false positives for
lib/dns/tests/rbt_serialize_test.c:
lib/dns/tests/rbt_serialize_test.c:412:12: warning: Either the condition 'base!=NULL' is redundant or there is pointer arithmetic with NULL pointer. [nullPointerArithmeticRedundantCheck]
p = base + (r % filesize);
^
lib/dns/tests/rbt_serialize_test.c:407:20: note: Assuming that condition 'base!=NULL' is not redundant
assert_true(base != NULL && base != MAP_FAILED);
^
lib/dns/tests/rbt_serialize_test.c:405:14: note: Assignment 'base=mmap(NULL,filesize,PROT_READ|PROT_WRITE,0|MAP_PRIVATE,fd,0)', assigned value is 0
base = mmap(NULL, filesize, PROT_READ|PROT_WRITE,
^
lib/dns/tests/rbt_serialize_test.c:412:12: note: Null pointer addition
p = base + (r % filesize);
^
lib/dns/tests/rbt_serialize_test.c:413:12: warning: Either the condition 'base!=NULL' is redundant or there is pointer arithmetic with NULL pointer. [nullPointerArithmeticRedundantCheck]
q = base + filesize;
^
lib/dns/tests/rbt_serialize_test.c:407:20: note: Assuming that condition 'base!=NULL' is not redundant
assert_true(base != NULL && base != MAP_FAILED);
^
lib/dns/tests/rbt_serialize_test.c:405:14: note: Assignment 'base=mmap(NULL,filesize,PROT_READ|PROT_WRITE,0|MAP_PRIVATE,fd,0)', assigned value is 0
base = mmap(NULL, filesize, PROT_READ|PROT_WRITE,
^
lib/dns/tests/rbt_serialize_test.c:413:12: note: Null pointer addition
q = base + filesize;
^
This is caused by cppcheck not understanding how cmocka's assert_true()
macro works. The problem being reported is a false positive: if mmap()
fails, the lines flagged by cppcheck will never be reached. Address the
problem by suppressing nullPointerArithmeticRedundantCheck warnings for
the affected lines.
cppcheck 1.90 reports some false positives for lib/dns/client.c:
lib/dns/client.c:1431:2: warning: Either the condition 'rctx==((void*)0)' is redundant or there is possible null pointer dereference: rctx. [nullPointerRedundantCheck]
rctx->rdataset = rdataset;
^
lib/dns/client.c:1416:11: note: Assuming that condition 'rctx==((void*)0)' is not redundant
if (rctx == NULL)
^
lib/dns/client.c:1415:9: note: Assignment 'rctx=isc__mem_get(mctx,sizeof(*rctx),"lib/dns/client.c",1415)', assigned value is 0
rctx = isc_mem_get(mctx, sizeof(*rctx));
^
lib/dns/client.c:1431:2: note: Null pointer dereference
rctx->rdataset = rdataset;
^
lib/dns/client.c:1438:2: warning: Either the condition 'rctx==((void*)0)' is redundant or there is possible null pointer dereference: rctx. [nullPointerRedundantCheck]
rctx->sigrdataset = sigrdataset;
^
lib/dns/client.c:1416:11: note: Assuming that condition 'rctx==((void*)0)' is not redundant
if (rctx == NULL)
^
lib/dns/client.c:1415:9: note: Assignment 'rctx=isc__mem_get(mctx,sizeof(*rctx),"lib/dns/client.c",1415)', assigned value is 0
rctx = isc_mem_get(mctx, sizeof(*rctx));
^
lib/dns/client.c:1438:2: note: Null pointer dereference
rctx->sigrdataset = sigrdataset;
^
lib/dns/client.c:1445:2: warning: Either the condition 'rctx==((void*)0)' is redundant or there is possible null pointer dereference: rctx. [nullPointerRedundantCheck]
rctx->client = client;
^
lib/dns/client.c:1416:11: note: Assuming that condition 'rctx==((void*)0)' is not redundant
if (rctx == NULL)
^
lib/dns/client.c:1415:9: note: Assignment 'rctx=isc__mem_get(mctx,sizeof(*rctx),"lib/dns/client.c",1415)', assigned value is 0
rctx = isc_mem_get(mctx, sizeof(*rctx));
^
lib/dns/client.c:1445:2: note: Null pointer dereference
rctx->client = client;
^
lib/dns/client.c:1827:2: warning: Either the condition 'ctx==((void*)0)' is redundant or there is possible null pointer dereference: ctx. [nullPointerRedundantCheck]
ctx->client = client;
^
lib/dns/client.c:1815:10: note: Assuming that condition 'ctx==((void*)0)' is not redundant
if (ctx == NULL)
^
lib/dns/client.c:1814:8: note: Assignment 'ctx=isc__mem_get(client->mctx,sizeof(*ctx),"lib/dns/client.c",1814)', assigned value is 0
ctx = isc_mem_get(client->mctx, sizeof(*ctx));
^
lib/dns/client.c:1827:2: note: Null pointer dereference
ctx->client = client;
^
All of them are caused by cppcheck not recognizing the relationship
between isc_mem_get() returning NULL and the result variable being set
to ISC_R_NOMEMORY (with a subsequent jump to a cleanup section).
Move "goto cleanup;" statements into error handling branches to prevent
cppcheck from generating these warnings.
zone_needdump() could potentially not call zone_settimer() so
explitly call zone_settimer() as zone->resigntime could have
gone backward.
(cherry picked from commit 5ec57f31b0)
With RRSIG records no longer being signed with the full
sig-validity-interval we need to ensure the zone->resigntime
as it may need to be set to a earlier time.
(cherry picked from commit 5d1611afdc)
* ctx needs to be destroyed before it is regenerated.
* emit the name of the signature to be replaced.
* cleanup memory before asserting so post longjump doesn't detect a
memory leak.
* comment code.
(cherry picked from commit 3a8c8a2a31)
The code for specifying OpenSSL PKCS#11 engine as part of the label
(e.g. -l "pkcs11:token=..." instead of -E pkcs11 -l "token=...")
was non-functional. This commit just cleans the related code.
(cherry picked from commit a5c87d9d18)
- Add quotes before and after zone name when generating "addzone"
input so avoid "unexpected token" errors.
- Use a hex digest for zone filenames when the zone or view name
contains a slash.
- Test with a domain name containing a slash.
- Incidentally added 'catzhash.py' to contrib/scripts to generate
hash labels for catalog zones, as it was needed to write the test.
(cherry picked from commit dba0163dac)
Update the API files.
- lib/dns:
- struct resolver has added elements, this is an interface change
and thus LIBINTERFACE is incremented, and LIBREVISION is reset.
- Since this also means an interface change since the last public
release, also reset LIBAGE.
- lib/isc:
- The library source code changed, so increment LIBREVISION.
- lib/isccfg:
- The library source code changed, so increment LIBREVISION.
Update other files:
- No changes needed to the README, this is a small bugfix release.
- Fix a bad version xml:id in the release notes.
The `rndc signing -clear` command cleans up the private-type records
that keep track of zone signing activity, but before this change it
did not tell the secondary servers that the zone has changed.
(cherry picked from commit f3f7b7df5d)
Function dns_view_findzonecut in view.c wasn't correctly handling
classes other than IN (chaos, hesiod, etc) whenever the name being
looked up wasn't in cache or in any of the configured zone views' database.
That resulted in a NULL fname being used in resolver.c:4900, which
in turn was triggering abort.
(cherry picked from commit 85555f29d7)
This is a bug I encountered when trying to schedule an algorithm
rollover. My plan, for a zone whose maximum TTL is 48h, was to sign
with the new algorithm and schedule a change of CDS records for more
than 48 hours in the future, roughly like this:
$ dnssec-keygen -a 13 -fk -Psync now+50h $zone
$ dnssec-keygen -a 13 $zone
$ dnssec-settime -Dsync now+50h $zone_ksk_old
However the algorithm 13 CDS was published immediately, which could
have made the zone bogus.
To reveal the bug using the `smartsign` test, this change just adds a
KSK with all its times in the future, so it should not affect the
existing checks at all. But the final check (that there are no CDS or
CDSNSKEY records after -Dsync) fails with the old `syncpublish()`
logic, because the future key's sync records appear early. With the
new `syncpublish()` logic the future key does not affect the test, as
expected, and it now passes.
(cherry picked from commit 4227b7969b)
(cherry picked from commit 2db5a2539a)
Add is_leaf and send_to_prune_tree to make the logic easier
to understand in cleanup_dead_nodes and decrement_reference.
(cherry picked from commit c6efc0e50f)
Ondřej Surý
Diego FronzaandOndřej Surý
Tinderbox UserandMatthijs Mekking
Mark Andrews
Michał Kępień
Tony FinchandMark Andrews