The test function mbedtls_mpi_lt_mpi_ct did not initialize ret in test
code. If there was a bug in library code whereby the library function
mbedtls_mpi_lt_mpi_ct() did not set ret when it should, we might have
missed it if ret happened to contain the expected value. So initialize
ret to a value that we never expect.
In Mbed TLS 2.7.17, the lack of initialization also caused Valgrind to
fail on a Clang 3.8 build with -O1 or more (not with -O0). As far as I
can tell, this is an instance of a known bug/feature in Clang which
sometimes generates code that contains a conditional jump based on
memory which is not initialized at the C level. This is not really a
bug in Clang as a C compiler since the code has the same behavior
whether the branch is taken or not, and therefore the branch is not
observable at the C level. However, the branch on C-uninitialized
memory causes a false positive from Valgrind. Here are some reports of
this Clang behavior:
* https://lists.llvm.org/pipermail/llvm-dev/2016-November/107428.html
* https://bugs.llvm.org/show_bug.cgi?id=32604
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
If test_fail is called multiple times in the same test case, report
the location of the first failure, not the last one.
With this change, you no longer need to take care in tests that use
auxiliary functions not to fail in the main function if the auxiliary
function has failed.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
RFC5280 does not state that the `revocationDate` should be checked.
In addition, when no time source is available (i.e., when MBEDTLS_HAVE_TIME_DATE is not defined), `mbedtls_x509_time_is_past` always returns 0. This results in the CRL not being checked at all.
https://tools.ietf.org/html/rfc5280
Signed-off-by: Raoul Strackx <raoul.strackx@fortanix.com>
Currently the new component in all.sh fails because
mbedtls_ssl_cf_memcpy_offset() is not actually constant flow - this is on
purpose to be able to verify that the new test works.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
The tests are supposed to be failing now (in all.sh component
test_memsan_constant_flow), but they don't as apparently MemSan doesn't
complain when the src argument of memcpy() is uninitialized, see
https://github.com/google/sanitizers/issues/1296
The next commit will add an option to test constant flow with valgrind, which
will hopefully correctly flag the current non-constant-flow implementation.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
* mbedtls-2.7: (28 commits)
A different approach of signed-to-unsigned comparison
Update the copy of tests/data_files/server2-sha256.crt in certs.c
Fix bug in redirection of unit test outputs
Backport e2k support to mbedtls-2.7
Don't forget to free G, P, Q, ctr_drbg, and entropy
Regenerate server2-sha256.crt with a PrintableString issuer
Regenerate test client certificates with a PrintableString issuer
cert_write: support all hash algorithms
compat.sh: stop using allow_sha1
compat.sh: quit using SHA-1 certificates
compat.sh: enable CBC-SHA-2 suites for GnuTLS
Fix license header in pre-commit hook
Update copyright notices to use Linux Foundation guidance
Fix building on NetBSD 9.0
Remove obsolete buildbot reference in compat.sh
Fix misuse of printf in shell script
Fix added proxy command when IPv6 is used
Simplify test syntax
Fix logic error in setting client port
ssl-opt.sh: include test name in log files
...
The test certificate used for clients in compat.sh, cert_sha256.crt,
had the issuer ON and CN encoded as UTF8String, but the corresponding
CA certificate test-ca_cat12.crt had them encoded as PrintableString.
The strings matched, which is sufficient according to RFC 5280 §7.1
and RFC 4518 §2.1. However, GnuTLS 3.4.10 requires the strings to have
the same encoding, so it did not accept that the certificate issued by
UTF8String "PolarSSL Test CA" was validly issued by the
PrintableString "PolarSSL Test CA" CA.
ebc1f40aa0, merged via
https://github.com/ARMmbed/mbedtls/pull/1641 and released in Mbed TLS
2.14, updated these certificates.
4f928c0f37 merged, via
https://github.com/ARMmbed/mbedtls/pull/2418 fixed this in the 2.7 LTS
branch for the SHA-1 certificate which was used at the time. The
present commit applies the same fix for the SHA-256 certificate that
is now in use.
For uniformity, this commit regenerates all the cert_*.crt.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Add a few more negative test cases for mbedtls_x509_crl_parse.
The test data is manually adapted from the existing positive test case
"X509 CRL ASN1 (TBSCertList, sig present)" which decomposes as
305c
3047 tbsCertList TBSCertList
020100 version INTEGER OPTIONAL
300d signatureAlgorithm AlgorithmIdentifier
06092a864886f70d01010e
0500
300f issuer Name
310d300b0603550403130441424344
170c303930313031303030303030 thisUpdate Time
3014 revokedCertificates
3012 entry 1
8202abcd userCertificate CertificateSerialNumber
170c303831323331323335393539 revocationDate Time
300d signatureAlgorithm AlgorithmIdentifier
06092a864886f70d01010e
0500
03020001 signatureValue BIT STRING
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Currently this breaks all.sh component test_memsan_constant_flow, just as
expected, as the current implementation is not constant flow.
This will be fixed in the next commit.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
This option allows to test the constant-flow nature of selected code, using
MemSan and the fundamental observation behind ctgrind that the set of
operations allowed on undefined memory by dynamic analysers is the same as the
set of operations allowed on secret data to avoid leaking it to a local
attacker via side channels, namely, any operation except branching and
dereferencing.
(This isn't the full story, as on some CPUs some instructions have variable
execution depending on the inputs, most notably division and on some cores
multiplication. However, testing that no branch or memory access depends on
secret data is already a good start.)
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
The dummy implementation is not constant-flow at all for now, it's just
here as a starting point and a support for developing the tests and putting
the infrastructure in place.
Depending on the implementation strategy, there might be various corner cases
depending on where the lengths fall relative to block boundaries. So it seems
safer to just test all possible lengths in a given range than to use only a
few randomly-chosen values.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
While at it, declare deps on ENTROPY as well.
A non-regression test will be added in a follow-up commit.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
An earlier botched backport (d56ca658ab)
had the wrong name for a variable and a missing header inclusion.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
When parsing a certificate with the basic constraints extension
the max_pathlen that was read from it was incremented regardless
of its value. However, if the max_pathlen is equal to INT_MAX (which
is highly unlikely), an undefined behaviour would occur.
This commit adds a check to ensure that such value is not accepted
as valid. Relevant tests for INT_MAX and INT_MAX-1 are also introduced.
Certificates added in this commit were generated using the
test_suite_x509write, function test_x509_crt_check. Input data taken
from the "Certificate write check Server1 SHA1" test case, so the generated
files are like the "server1.crt", but with the "is_ca" field set to 1 and
max_pathlen as described by the file name.
Signed-off-by: Andrzej Kurek <andrzej.kurek@arm.com>
Signed-off-by: Piotr Nowicki <piotr.nowicki@arm.com>
The identifiers of the unmet dependencies of a test case are
stored in a buffer of fixed size that can be potentially too
small to store all the unmet dependencies. Indicate in test
report if some unmet dependencies are missing.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Fix potential buffer overflow when tracking the unmet dependencies
of a test case. The identifiers of unmet dependencies are stored
in an array of fixed size. Ensure that we don't overrun the array.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Use size_t for some variables that are array indices.
Use unsigned for some variables that are counts of "small" things.
This is a backport of commit 3c1c8ea3e7.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
Warn about a gotcha that caused a bug in development.
Ensure that it's at least zeroed out, rather than uninitialized, in
non-verbose mode.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
(Only the top-level ones, ie, for each call to eg asn1_get_mpi(), ensure
there's at least one test case that makes this call fail in one way, but don't
test the various ways to make asn1_get_mpi fail - that should be covered
elsewhere.)
- the new checks added by the previous commits needed exercising
- existing tests sometimes had wrong descriptions or where passing for the
wrong reason (eg with the "length mismatch" test, the function actually
failed before reaching the length check)
- while at it, add tests for the rest as well
The valid minimal-size key was generated with:
openssl genrsa 128 2>/dev/null | openssl rsa -outform der 2>/dev/null | xxd -p
