Michal Nowak [Mon, 24 Feb 2020 15:37:25 +0000 (15:37 +0000)]
Fix "pkcs11" system test
- Define the SLOT environment variable before starting the test. This
variable defaults to 0 and that does not work with SoftHSM 2.
- The system test expects the PIN environment variable to be set to
"1234" while bin/tests/prepare-softhsm2.sh sets it to "0000".
Update bin/tests/prepare-softhsm2.sh so that it sets the PIN to
"1234".
- Move contents of bin/tests/system/pkcs11/prereq.sh to
bin/tests/system/pkcs11/setup.sh as the former was creating a file
called "supported" that was getting removed by the latter before
bin/tests/system/pkcs11/tests.sh could access it.
Michał Kępień [Wed, 4 Mar 2020 11:41:01 +0000 (12:41 +0100)]
Fix cppcheck 1.90 warnings
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.
Michał Kępień [Wed, 4 Mar 2020 11:41:01 +0000 (12:41 +0100)]
Fix cppcheck 1.90 warning
cppcheck 1.90 reports the following issue for bin/named/query.c:
bin/named/query.c:6838:2: warning: %d in format string (no. 3) requires 'int' but the argument type is 'unsigned int'. [invalidPrintfArgType_sint]
snprintf(mbuf, sizeof(mbuf) - 1,
^
Tweak the format specifier for client->query.restarts to address the
problem.
Michał Kępień [Wed, 4 Mar 2020 11:41:01 +0000 (12:41 +0100)]
Fix cppcheck 1.90 warnings
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.
Michał Kępień [Wed, 16 Oct 2019 20:06:00 +0000 (22:06 +0200)]
Fix cppcheck 1.89 warnings
cppcheck 1.89 enabled certain value flow analysis mechanisms [1] which
trigger null pointer dereference false positives that were previously
not reported. It seems that cppcheck no longer treats at least some
REQUIRE() assertion failures as fatal, so add extra assertion macro
definitions to lib/isc/include/isc/util.h that are only used when the
CPPCHECK preprocessor macro is defined; these definitions make cppcheck
1.89 behave as expected.
There is an important requirement for these custom definitions to work:
cppcheck must properly treat abort() as a function which does not
return. In order for that to happen, the __GNUC__ macro must be set to
a high enough number (because system include directories are used and
system headers compile attributes away if __GNUC__ is not high enough).
__GNUC__ is thus set to the major version number of the GCC compiler
used, which is what that latter does itself during compilation.
Mark Andrews [Thu, 27 Feb 2020 06:35:18 +0000 (17:35 +1100)]
Call set_resigntime() in receive_secure_serial()
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.
Ondřej Surý [Tue, 25 Feb 2020 14:22:54 +0000 (15:22 +0100)]
Use pkg-config for --with-libxml2=auto/yes
The downstream distributors of BIND 9 (Debian in this case) are in
process of removing xml2-config command from the libxml2-dev package
(see Debian Bug #949056 for details). The removal of the script will
make BIND 9 to fail to build from the source when --with-libxml2=yes is
specified or not link with libxml2 when --with-libxml2=auto is specified
and then fail ABI changes (Debian Bug #949056).
When --with-libxml2=<path>, the script checks for <path>/bin/xml2-config
and uses the specified path to link with libxml2. This has been kept to
retain backwards compatibility with systems that does not ship
pkg-config.
Ondřej Surý [Thu, 8 Aug 2019 13:52:47 +0000 (15:52 +0200)]
Use standard PKCS#11 standard error codes instead of custom error codes
* CKR_CRYPTOKI_ALREADY_INITIALIZED: This value can only be returned by
`C_Initialize`. It means that the Cryptoki library has already been
initialized (by a previous call to `C_Initialize` which did not have a
matching `C_Finalize` call).
* CKR_FUNCTION_NOT_SUPPORTED: The requested function is not supported by this
Cryptoki library. Even unsupported functions in the Cryptoki API should have a
"stub" in the library; this stub should simply return the value
CKR_FUNCTION_NOT_SUPPORTED.
* CKR_LIBRARY_LOAD_FAILED: The Cryptoki library could not load a dependent
shared library.
Ondřej Surý [Wed, 29 May 2019 09:07:46 +0000 (11:07 +0200)]
Replace the OASIS PKCS#11 header file with one from p11-kit
The OASIS pkcs11.h header has a restrictive license. Replace the
pkcs11.h pkcs11f.h and pkcs11t.h headers with pkcs11.h from p11-kit.
For source distribution, the license for the OASIS headers itself
doesn't pose any licensing problem when combined with MPL license, but
it possibly creates problem for downstream distributors of BIND 9.
Mark Andrews [Fri, 21 Feb 2020 05:40:50 +0000 (21:40 -0800)]
Fix code to generate the test signatues.
* 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.
Michał Kępień [Thu, 20 Feb 2020 11:23:36 +0000 (12:23 +0100)]
Make a sed script in doc/arm/Makefile.in portable
BSD sed does not recognize \s as a whitespace matching token. Make the
sed script in doc/arm/Makefile.in which ensures GitLab identifiers are
not split across lines portable by replacing \s with [[:space:]].
Michał Kępień [Thu, 20 Feb 2020 10:51:58 +0000 (11:51 +0100)]
Increase lifetime of docs:sid:amd64 artifacts
Artifacts generated by the docs:sid:amd64 job need to be retained longer
than for other jobs as they are used for building bind.isc.org contents.
If these artifacts are removed too quickly, pipelines in the pages/bind
GitLab project start failing, preventing content updates from being
published. Increase lifetime of the relevant job artifacts to prevent
this from happening.
Michal Nowak [Wed, 12 Feb 2020 15:00:32 +0000 (15:00 +0000)]
Run Coverity Scan only when specific variables are present
Submissions to Coverity Scan should be limited to those originated from
release branches and only from a specific schedule which holds
COVERITY_SCAN_PROJECT_NAME and COVERITY_SCAN_TOKEN variables.
Ondřej Surý [Sat, 1 Feb 2020 09:48:20 +0000 (10:48 +0100)]
Convert all atomic operations in isc_rwlock to sequentially-consistent ordering
The memory ordering in the rwlock was all wrong, I am copying excerpts
from the https://en.cppreference.com/w/c/atomic/memory_order#Relaxed_ordering
for the convenience of the reader:
Relaxed ordering
Atomic operations tagged memory_order_relaxed are not synchronization
operations; they do not impose an order among concurrent memory
accesses. They only guarantee atomicity and modification order
consistency.
Sequentially-consistent ordering
Atomic operations tagged memory_order_seq_cst not only order memory
the same way as release/acquire ordering (everything that
happened-before a store in one thread becomes a visible side effect in
the thread that did a load), but also establish a single total
modification order of all atomic operations that are so tagged.
Which basically means that we had no or weak synchronization between
threads using the same variables in the rwlock structure. There should
not be a significant performance drop because the critical sections were
already protected by:
while(1) {
if (relaxed_atomic_operation) {
break;
}
LOCK(lock);
if (!relaxed_atomic_operation) {
WAIT(sem, lock);
}
UNLOCK(lock)l
}
I would add one more thing to "Don't do your own crypto, folks.":
- Also don't do your own locking, folks.
As part of this commit, I have also cleaned up the #ifdef spaghetti,
and fixed the isc_atomic API usage.
Mark Andrews [Mon, 10 Feb 2020 20:52:24 +0000 (07:52 +1100)]
Silence Coverity FORWARD_NULL warning
CID 1458400 (#1 of 1): Dereference after null check
(FORWARD_NULL) 14. var_deref_model: Passing null pointer
nxt->typebits to mem_tobuffer, which dereferences it. [show
details]
Mark Andrews [Mon, 10 Feb 2020 20:44:21 +0000 (07:44 +1100)]
Silence Coverity CHECKED_RETURN warnings
CID 1458403 (#1 of 1): Unchecked return value (CHECKED_RETURN)
8. check_return: Calling isc_socket_recv without checking
return value (as is done elsewhere 14 out of 17 times).
CID 1458402 (#1 of 1): Unchecked return value (CHECKED_RETURN)
2. check_return: Calling isc_socket_recv without checking
return value (as is done elsewhere 14 out of 17 times).
CID 1458401 (#1 of 1): Unchecked return value (CHECKED_RETURN)
6. check_return: Calling isc_socket_recv without checking
return value (as is done elsewhere 14 out of 17 times).
Ondřej Surý [Wed, 22 Jan 2020 09:16:22 +0000 (10:16 +0100)]
Cleanup support for specifying PKCS#11 engine as part of the label
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.