The documentation of mbedtls_pk_wrap_as_opaque is quite clear:
* \param handle Output: a PSA key handle.
* It's the caller's responsibility to call
* psa_destroy_key() on that handle after calling
* mbedtls_pk_free() on the PK context.
But the test failed to call psa_destroy_key().
While at it, also use PSA_DONE(): it ensures that if we fail to destroy the
key, we'll get an explicit error message about it without the need for
valgrind.
This is a preliminary to adding a valgrind-based test for constant-flow code:
we need to make sure the rest of the tests are fully valgrind-clean, which
they weren't.
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>
* development: (55 commits)
Log change as bugfix
Add changelog entry
Clarify updates to the persistent state in storage
With multiple applicable transparent drivers, the order is unspecified
Minor clarifications
Give some examples of purpsoses of pure-software transparent driver
Fix typos
Add a link to the PSA API specification
Explain locations vs lifetimes
Initialize key pointer in ecdh to NULL
Add buffer zeroization when ecp_write_key fails
Simplified key slot deletion
Style fixes
Use arc4random_buf instead of rand on NetBSD
Apply review feedback
Update open question section about public key storage
Remove the paragraph about declaring application needs
Change driver persistent data to a callback interface
Rework and expand key management in opaque drivers
Fix typos and copypasta
...
Since it is being dereferenced by free on exit it should be inited to NULL.
Also added a small test that would trigger the issue.
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
PSA Crypto was checking the byte length of a to-be-imported public ECP key
against the expected length for Weierstrass keys, forgetting that
Curve25519/Curve448 exists.
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
The convention from the TLS RFC is a bit unusual, so even if the test
function's introductory comment mentions that we're taking the RFC's
definition, it doesn't hurt to repeat it in crucial places.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
Passing a length of 0 to it is perfectly acceptable, the macro was designed to
handle it correctly.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
We only have a single integer available for two nested loops, but the loop
sizes are small enough compared to the integer's range that we can encode both
indexes. Since the integer is displayed in decimal in case of errors, use a
power of 10 to pack the two indexes together.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@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>
Everything works at the byte level, not bit level. Flipping the lsb is just
one convenient way to corrupt a byte, but don't really care about individual
bits.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@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>
The test function now depends on MBEDTLS_TEST_HOOKS, which is enabled by
config.py full, and since there are already components in all.sh exercising
the full config, this test function is sill exercised even with this new
dependency.
Since this is the first time a test function depends on MBEDTLS_TEST_HOOKS,
fix a bug in check-names.sh that wasn't apparent so far: headers from
library/*.h were not considered when looking for macro definitions. This
became apparent because MBEDTLS_STATIC_TESTABLE is defined in library/common.h
and started being used in library/ssl_msg.c, so was flagged as a likely typo.
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>
The ssl_tranform structure lacks some members accessed by this function when
CBC is not enabled.
This was found by test-ref-configs.pl and all.sh
test_when_no_ciphersuites_have_mac, so no need to add a new test.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
Started as copies of the AES block, then:
- for ARIA, just s/AES/ARIA/
- for Camellia, just s/AES/Camellia/
- for 3DES, s/AES/3DES/ then s/3DES_128_CBC/DES_EDE3_CBC/ then manually
subtract 8 to all plaintext lengths that were > 8. This accounts for the
fact that the block size of DES is 8 not 16.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
All started from a copy of the SHA256 block and modified as follows:
- for MD5, just s/SHA256/MD5/
- for SHA384, adapt the dependency line then s/SHA256/SHA384
- for SHA1, s/SHA256/SHA1/ then manually adapt the plaintext length for the
cases with "!trunc, B-1" and "!trunc, B", as the MAC length (20) is not a
multiple of the block size (16) for this hash
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
- plaintext length = 0 or 1
- plaintext length + MAC length = -1 or 0 mod block_size
(using the minimum plaintext length that works)
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
Compared to the previous approach of having the bad padding provided as input
to the testing function, this allows to test more kinds of incorrect data,
with less test cases in the .data file and more important no manually-generated
non-trivial data in the test case parameters, making it much easier to
complete the testing matrix.
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
They did not match their description, probably due to a botched manual
endianness conversion where the nibbles also got swapped.
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
Follow the PSA Crypto specification which was updated between 1.0 beta3
and 1.0.0.
Add corresponding test cases.
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
mbedtls_ecp_write_key is a mirror function to mbedtls_ecp_read_key, which
writes a private key back into a byte buffer in the correct format.
This is a helpful convenience function, since the byte order is defined
differently between Montgomery and Weierstrass curves. Since this difference
is accounted for in mbedtls_ecp_read_key, it made sense to add
mbedtls_ecp_write_key for the purpose of abstracting this away such that
psa_export_key doesn't need to take byte order into account.
Signed-off-by: Steven Cooreman <steven.cooreman@silabs.com>
Rename PSA_DH_GROUP_xxx to PSA_DH_FAMILY_xxx, also rename
PSA_KEY_TYPE_GET_GROUP to PSA_KEY_TYPE_DH_GET_FAMILY and rename
psa_dh_group_t to psa_dh_family_t. Old defines are provided in
include/crypto_compat.h for backward compatibility.
Signed-off-by: Paul Elliott <paul.elliott@arm.com>
Rename PSA_ECC_CURVE_xxx to PSA_ECC_FAMILY_xxx, also rename
PSA_KEY_TYPE_GET_CURVE to PSA_KEY_TYPE_ECC_GET_FAMILY and rename
psa_ecc_curve_t to psa_ecc_family_t. Old defines are provided in
include/crypto_compat.h for backward compatibility.
Signed-off-by: Paul Elliott <paul.elliott@arm.com>
Executed "./scripts/bump_version.sh --version 2.23.0 --so-crypto 5"
A symbol has been removed from the mbedcrypto library since the last
release:
mbedtls_ecc_group_to_psa ( enum mbedtls_ecp_group_id grpid,
size_t* bits )
This is an ABI break and we need to increase the SO version.
Signed-off-by: Janos Follath <janos.follath@arm.com>
The metadata tests depend on the corresponding feature because there
is no guarantee that the metadata is correct if the feature is
disabled. There are metadata test cases for some algorithms and key
types that are declared but not supported. These test cases are
present but can never run.
It is debatable whether having these test cases is a good thing in
case they become runnable in the future, or a bad thing because
they're dead code. We're working on detecting test cases that are
never executed for accidental reasons (e.g. typo in a dependency or
missing configuration on the CI), and having test cases that are
deliberately never executed messes this up. So remove these test
cases. If we do implement the corresponding feature, it'll be easy to
add the corresponding metadata test cases.
The features that had metadata tests but no implementations were:
* SHA-512/256 and SHA-512/224 (hypothetical dependency: MBEDTLS_SHA512_256)
* DSA (hypothetical dependency: MBEDTLS_DSA_C)
* SHA-3 and HMAC-SHA-3 (hypothetical dependency: MBEDTLS_SHA3_C)
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Rework mbedtls_test_unhexify to extend its scope of usage.
Return in error when the function detects an error instead
of calling mbedtls_exit().
Improve safety by checking the output buffer is not overrun.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
In test functions calling mbedtls_test_unhexify(), change the
type of the associated parameters from `char*` to `data_t`.
That way the `unhexify` operation is done by the test
framework and not by the unit test code.
Use for the new parameters of type data_t the name of the
local variable that use to store the `unhexify` version of
the `char*` parameter.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
In preparation of changing the type of some parameters
of mbedtls_ccm_star_encrypt_and_tag/auth_decrypt from
`char *` to `data_t` to get rid of the calls to
mbedtls_test_unhexify():
- Change the name of parameters and local variables to
clarify which ones are related to the outputs of the
library functions under test and which ones are
related to the expected values of those outputs.
- Use two different buffers to store the plain and cipher
text as expected by the library functions.
Signed-off-by: Ronald Cron <ronald.cron@arm.com>
In preparation of changing the type of some parameters
of aes_encrypt_ofb() from `char *` to `data_t` to get rid
of the calls to mbedtls_test_unhexify():
- Change the name of parameters and local variables to
clarify which ones are related to the outputs of the
library functions under test and which ones are
related to the expected values of those outputs.
- Add assertion on fragment_size parameter
Signed-off-by: Ronald Cron <ronald.cron@arm.com>