It was failing to set the key in the ENCRYPT direction before encrypting.
This just happened to work for GCM and CCM.
After re-encrypting, compare the length to the expected ciphertext
length not the plaintext length. Again this just happens to work for
GCM and CCM since they do not perform any kind of padding.
* public/pr/2148:
Add ChangeLog entry for unused bits in bitstrings
Improve docs for ASN.1 bitstrings and their usage
Add tests for (named) bitstring to suite_asn1write
Fix ASN1 bitstring writing
Add a test case for doing an ECDH calculation by calling
mbedtls_ecdh_get_params on both keys, with keys belonging to
different groups. This should fail, but currently passes.
tests/Makefile had some unused warnings disabled unnecessarily, which
test-ref-configs.pl was turning back on. We don't need to disable these warnings
so I'm turning them back on.
Dependent on configured options, not all of the helper functions were being
used, which was leading to warning of unused functions with Clang.
To avoid any complex compile time options, or adding more logic to
generate_test_code.py to screen out unused functions, those functions which were
provoking the warning were changed to remove static, remove them from file
scope, and expose them to the linker.
Since the AD too long is a limitation on Mbed TLS,
HW accelerators may support this. Run the test for AD too long,
only if `MBEDTLS_CCM_ALT` is not defined.
Addresses comment in #1996.
This commit introduces variants test-ca_utf8.crt,
test-ca_printablestring.crt and test-ca_uppercase.crt
of tests/data_files/test-ca.crt which differ from
test-ca.crt in their choice of string encoding and
upper and lower case letters in the DN field. These
changes should be immaterial to the recovation check,
and three tests are added that crl.pem, which applies
to test-ca.crt, is also considered as applying to
test-ca_*.crt.
The test files were generated using PR #1641 which
- adds a build instruction for test-ca.crt to
tests/data_files/Makefile which allows easy
change of the subject DN.
- changes the default string format from `PrintableString`
to `UTF8String`.
Specifically:
- `test-ca_utf8.crt` was generated by running
`rm test-ca.crt && make test-ca.crt`
on PR #1641.
- `test-ca_uppercase.crt`, too, was generated by running
`rm test-ca.crt && make test-ca.crt`
on PR #1641, after modifying the subject DN line in the build
instruction for `test-ca.crt` in `tests/data_files/Makefile`.
- `test-ca_printable.crt` is a copy of `test-ca.crt`
because at the time of this commit, `PrintableString` is
still the default string format.
Primality tests have to deal with different distribution when generating
primes and when validating primes.
These new tests are testing if mbedtls_mpi_is_prime() is working
properly in the latter setting.
The new tests involve pseudoprimes with maximum number of
non-witnesses. The non-witnesses were generated by printing them
from mpi_miller_rabin(). The pseudoprimes were generated by the
following function:
void gen_monier( mbedtls_mpi* res, int nbits )
{
mbedtls_mpi p_2x_plus_1, p_4x_plus_1, x, tmp;
mbedtls_mpi_init( &p_2x_plus_1 );
mbedtls_mpi_init( &p_4x_plus_1 );
mbedtls_mpi_init( &x ); mbedtls_mpi_init( &tmp );
do
{
mbedtls_mpi_gen_prime( &p_2x_plus_1, nbits >> 1, 0,
rnd_std_rand, NULL );
mbedtls_mpi_sub_int( &x, &p_2x_plus_1, 1 );
mbedtls_mpi_div_int( &x, &tmp, &x, 2 );
if( mbedtls_mpi_get_bit( &x, 0 ) == 0 )
continue;
mbedtls_mpi_mul_int( &p_4x_plus_1, &x, 4 );
mbedtls_mpi_add_int( &p_4x_plus_1, &p_4x_plus_1, 1 );
if( mbedtls_mpi_is_prime( &p_4x_plus_1, rnd_std_rand,
NULL ) == 0 )
break;
} while( 1 );
mbedtls_mpi_mul_mpi( res, &p_2x_plus_1, &p_4x_plus_1 );
}
Functional tests for various payload sizes and output buffer sizes.
When the padding is bad or the plaintext is too large for the output
buffer, verify that function writes some outputs. This doesn't
validate that the implementation is time-constant, but it at least
validates that it doesn't just return early without outputting anything.
Deprecate mbedtls_ctr_drbg_update (which returns void) in favor of a
new function mbedtls_ctr_drbg_update_ret which reports error. The old
function is not officially marked as deprecated in this branch because
this is a stable maintenance branch.
Add pk_write test cases where the ASN.1 INTEGER encoding of the
private value does not have the mandatory size for the OCTET STRING
that contains the value.
ec_256_long_prv.pem is a random secp256r1 private key, selected so
that the private value is >= 2^255, i.e. the top bit of the first byte
is set (which would cause the INTEGER encoding to have an extra
leading 0 byte).
ec_521_short_prv.pem is a random secp521r1 private key, selected so
that the private value is < 2^518, i.e. the first byte is zero and the
top bit of the second byte is 0 (which would cause the INTEGER
encoding to have one less 0 byte at the start).
Address review comments:
1. add `mbedtls_cipher_init()` after freeing context, in test code
2. style comments
3. set `ctx->iv_size = 0` in case `IV == NULL && iv_len == 0`
This PR fixes multiple issues in the source code to address issues raised by
tests/scripts/check-files.py. Specifically:
* incorrect file permissions
* missing newline at the end of files
* trailing whitespace
* Tabs present
* TODOs in the souce code
The relevant ASN.1 definitions for a PKCS#8 encoded Elliptic Curve key are:
PrivateKeyInfo ::= SEQUENCE {
version Version,
privateKeyAlgorithm PrivateKeyAlgorithmIdentifier,
privateKey PrivateKey,
attributes [0] IMPLICIT Attributes OPTIONAL
}
AlgorithmIdentifier ::= SEQUENCE {
algorithm OBJECT IDENTIFIER,
parameters ANY DEFINED BY algorithm OPTIONAL
}
ECParameters ::= CHOICE {
namedCurve OBJECT IDENTIFIER
-- implicitCurve NULL
-- specifiedCurve SpecifiedECDomain
}
ECPrivateKey ::= SEQUENCE {
version INTEGER { ecPrivkeyVer1(1) } (ecPrivkeyVer1),
privateKey OCTET STRING,
parameters [0] ECParameters {{ NamedCurve }} OPTIONAL,
publicKey [1] BIT STRING OPTIONAL
}
Because of the two optional fields, there are 4 possible variants that need to
be parsed: no optional fields, only parameters, only public key, and both
optional fields. Previously mbedTLS was unable to parse keys with "only
parameters". Also, only "only public key" was tested. There was a test for "no
optional fields", but it was labelled incorrectly as SEC.1 and not run because
of a great renaming mixup.
Conflict resolution:
* ChangeLog
* tests/data_files/Makefile: concurrent additions, order irrelevant
* tests/data_files/test-ca.opensslconf: concurrent additions, order irrelevant
* tests/scripts/all.sh: one comment change conflicted with a code
addition. In addition some of the additions in the
iotssl-1381-x509-verify-refactor-restricted branch need support for
keep-going mode, this will be added in a subsequent commit.
The 'critical' boolean can be set to false in two ways:
- by leaving it implicit (test data generated by openssl)
- by explicitly setting it to false (generated by hand)
This covers all lines added in the previous commit. Coverage was tested using:
make CFLAGS='--coverage -g3 -O0'
(cd tests && ./test_suite_x509parse)
make lcov
firefox Coverage/index.html # then visual check
Test data was generated by taking a copy of tests/data_files/crl-idp.pem,
encoding it as hex, and then manually changing the values of some bytes to
achieve the desired errors, using https://lapo.it/asn1js/ for help in locating
the desired bytes.
Found by running:
CC=clang cmake -D CMAKE_BUILD_TYPE="Check"
tests/scripts/depend-pkalgs.pl
(Also tested with same command but CC=gcc)
Another PR will address improving all.sh and/or the depend-xxx.pl scripts
themselves to catch this kind of thing.
Our current behaviour is a bit inconsistent here:
- when the bad signature is made by a trusted CA, we stop here and don't
include the trusted CA in the chain (don't call vrfy on it)
- otherwise, we just add NOT_TRUSTED to the flags but keep building the chain
and call vrfy on the upper certs
This ensures that the callback can actually clear that flag, and that it is
seen by the callback at the right level. This flag is not set at the same
place than others, and this difference will get bigger in the upcoming
refactor, so let's ensure we don't break anything here.
When a trusted CA is rolling its root keys, it could happen that for some
users the list of trusted roots contains two versions of the same CA with the
same name but different keys. Currently this is supported but wasn't tested.
Note: the intermediate file test-ca-alt.csr is commited on purpose, as not
commiting intermediate files causes make to regenerate files that we don't
want it to touch.
As we accept EE certs that are explicitly trusted (in the list of trusted
roots) and usually look for parent by subject, and in the future we might want
to avoid checking the self-signature on trusted certs, there could a risk that we
incorrectly accept a cert that looks like a trusted root except it doesn't
have the same key. This test ensures this will never happen.
The tests cover chains of length 0, 1 and 2, with one error, located at any of
the available levels in the chain. This exercises all three call sites of
f_vrfy (two in verify_top, one in verify_child). Chains of greater length
would not cover any new code path or behaviour that I can see.
So far there was no test ensuring that the flags passed to the vrfy callback
are correct (ie the flags for the current certificate, not including those of
the parent).
Actual tests case making use of that test function will be added in the next
commit.
We have code to skip them but didn't have explicit tests ensuring they are
(the corresponding branch was never taken).
While at it, remove extra copy of the chain in server10*.crt, which was
duplicated for no reason.
This shows inconsistencies in how flags are handled when callback fails:
- sometimes the flags set by the callback are transmitted, sometimes not
- when the cert if not trusted, sometimes BADCERT_NOT_TRUSTED is set,
sometimes not
This adds coverage for 9 lines and 9 branches. Now all lines related to
callback failure are covered.
Now all checks related to profile are covered in:
- verify_with_profile()
- verify_child()
- verify_top()
(that's 10 lines that were previously not covered)
Leaving aside profile enforcement in CRLs for now, as the focus is on
preparing to refactor cert verification.
Previously flags was left to whatever value it had before. It's cleaner to
make sure it has a definite value, and all bits set looks like the safest way
for when it went very wrong.
Extend the pkparse test suite with the newly created keys
encrypted using PKCS#8 with PKCS#5 v2.0 with PRF being
SHA224, 256, 384 and 512.
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
Some unit tests for pbkdf2_hmac() have results longer than
99bytes when represented in hexadecimal form.
For this reason extend the result array to accommodate
longer strings.
At the same time make memset() parametric to avoid
bugs in the future.
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
Test vectors for SHA224,256,384 and 512 have been
generated using Python's hashlib module by the
following oneliner:
import binascii, hashlib
binascii.hexlify(hashlib.pbkdf2_hmac(ALGO, binascii.unhexlify('PASSWORD'), binascii.unhexlify('SALT'), ITER, KEYLEN)))
where ALGO was 'sha224', 'sha256', 'sha384' and 'sha512'
respectively.
Values for PASSWORD, SALT, ITER and KEYLEN were copied from the
existent test vectors for SHA1.
For SHA256 we also have two test vectors coming from RFC7914 Sec 11.
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
Currently only SHA1 is supported as PRF algorithm for PBKDF2
(PKCS#5 v2.0).
This means that keys encrypted and authenticated using
another algorithm of the SHA family cannot be decrypted.
This deficiency has become particularly incumbent now that
PKIs created with OpenSSL1.1 are encrypting keys using
hmacSHA256 by default (OpenSSL1.0 used PKCS#5 v1.0 by default
and even if v2 was forced, it would still use hmacSHA1).
Enable support for all the digest algorithms of the SHA
family for PKCS#5 v2.0.
Signed-off-by: Antonio Quartulli <antonio@openvpn.net>
This change fixes a problem in the tests pk_rsa_alt() and
pk_rsa_overflow() from test_suite_pk.function that would cause a
segmentation fault. The problem is that these tests are only designed
to run in computers where the SIZE_MAX > UINT_MAX.
Change function in tests named mbedtls_entropy_func to
mbedtls_test_entropy_func to avoid getting error from the linker when
calling the mbedtls_entropy_func elsewhere.
The _ext suffix suggests "new arguments", but the new functions have
the same arguments. Use _ret instead, to convey that the difference is
that the new functions return a value.
Conflict resolution:
* ChangeLog: put the new entries in their rightful place.
* library/x509write_crt.c: the change in development was whitespace
only, so use the one from the iotssl-1251 feature branch.
1) `mbedtls_rsa_import_raw` used an uninitialized return
value when it was called without any input parameters.
While not sensible, this is allowed and should be a
succeeding no-op.
2) The MPI test for prime generation missed a return value
check for a call to `mbedtls_mpi_shift_r`. This is neither
critical nor new but should be fixed.
3) Both the RSA keygeneration example program and the
RSA test suites contained code initializing an RSA context
after a potentially failing call to CTR DRBG initialization,
leaving the corresponding RSA context free call in the
cleanup section of the respective function orphaned.
While this defect existed before, Coverity picked up on
it again because of newly introduced MPI's that were
also wrongly initialized only after the call to CTR DRBG
init. The commit fixes both the old and the new issue
by moving the initializtion of both the RSA context and
all MPI's prior to the first potentially failing call.
* development:
Timing self test: shorten redundant tests
Timing self test: increased duration
Timing self test: increased tolerance
Timing unit tests: more protection against infinite loops
Unit test for mbedtls_timing_hardclock
New timing unit tests
selftest: allow excluding a subset of the tests
selftest: allow running a subset of the tests
selftest: refactor to separate the list of tests from the logic
Timing self test: print some diagnosis information
mbedtls_timing_get_timer: don't use uninitialized memory
timing interface documentation: minor clarifications
Timing: fix mbedtls_set_alarm(0) on Unix/POSIX
* public/pr/1136:
Timing self test: shorten redundant tests
Timing self test: increased duration
Timing self test: increased tolerance
Timing unit tests: more protection against infinite loops
Unit test for mbedtls_timing_hardclock
New timing unit tests
selftest: allow excluding a subset of the tests
selftest: allow running a subset of the tests
selftest: refactor to separate the list of tests from the logic
Timing self test: print some diagnosis information
mbedtls_timing_get_timer: don't use uninitialized memory
timing interface documentation: minor clarifications
Timing: fix mbedtls_set_alarm(0) on Unix/POSIX
* Correct order of sections in ChangeLog
* Restore unintentionally removed whitespace and
formatting improvements.
* Consistently rename MBEDTLS_ERR_RSA_EXPORT_UNSUPPORTED
to MBEDTLS_ERR_RSA_UNSUPPORTED_OPERATION in rsa.h
documentation.
During the work on the RSA change the issue was brought up,
and a fix was provided on development, that some RSA tests
use CTR DRBG and depend on the presence of a strong entropy
source to succeed. The RSA work introduced more tests using
CTR DRBG, and the dependency needs to be added for them, too.
If timing_timer_simple fails because it detects that timers are likely
to never expire (e.g. going backward or not incrementing), skip all
tests that rely on timers.
Do test mbedtls_timing_hardclock. We can't reliably test much about
it, but at least test that it doesn't crash, isn't constant, and
doesn't look completely random.
New set of unit tests for the timing module, instead of just running
the selftest function.
The selftest function sometimes fails on a heavily loaded
machine (such as a typical continuous integration system). Because of
the all-in-one nature of the test and because the exact load pattern
can be hard to reproduce, it is difficult to diagnose failures of CI
runs with selftest. The new tests are more separated and I strove to
point out potential failure modes in comments.
* mbedtls_timing_hardclock: not tested. This function gives so few
guarantees that there isn't much to test, and it is hard to test
reliably because clock cycles don't easily relate to time in any
remotely portable way. This function isn't used in the library
anyway, it's only there for benchmark programs.
* mbedtls_timing_get_timer: tested by setting a timer and verifying
that it reaches its target, and by verifying that a timer started
later than another always has a smaller elapsed time.
* mbedtls_set_alarm: tested by setting an alarm, busy-waiting for it
and measuring the elapsed time with a timer.
* mbedtls_timing_set_delay, mbedtls_timing_get_delay: tested by
setting a delay object and watching it go through its two delay
values, using a timer to check that the delays are passed at the
expected time.
The tests pass under light to moderate load, but some of them can be
defeated with sufficiently heavy load. This is unavoidable since the
test process to be effectively suspended for any length of time,
making us think that a timer has gone on for too long.
* development:
Don't split error code description across multiple lines
Register new error code in error.h
Move deprecation to separate section in ChangeLog
Extend scope of ERR_RSA_UNSUPPORTED_OPERATION error code
Adapt RSA test suite
Adapt ChangeLog
Deprecate usage of RSA primitives with wrong key type
* restricted/pr/397:
Don't split error code description across multiple lines
Register new error code in error.h
Move deprecation to separate section in ChangeLog
Extend scope of ERR_RSA_UNSUPPORTED_OPERATION error code
Adapt RSA test suite
Adapt ChangeLog
Deprecate usage of RSA primitives with wrong key type
For a key of size 8N+1, check that the first byte after applying the
public key operation is 0 (it could have been 1 instead). The code was
incorrectly doing a no-op check instead, which led to invalid
signatures being accepted. Not a security flaw, since you would need the
private key to craft such an invalid signature, but a bug nonetheless.
