Commit graph

2638 commits

Author SHA1 Message Date
Ronald Cron 71016a9ea7 psa: Rename psa_key_file_id_t to mbedtls_svc_key_id_t
With PSA crypto v1.0.0, a volatile key identifier may
contain a owner identifier but no file is associated
to it. Thus rename the type psa_key_file_id_t to
mbedtls_svc_key_id_t to avoid a direct link with a
file when a key identifier involves an owner
identifier.

The new type name is prefixed by mbedtls to highlight
that the type is specific to Mbed TLS implementation
and not defined in the PSA Cryptography API
specification.

The svc in the type name stands for service as this
is the key identifier type from the point of view of
the service providing the Cryptography services.
The service can be completely provided by the present
library or partially in case of a multi-client service.

As a consequence rename as well:
. MBEDTLS_PSA_CRYPTO_KEY_FILE_ID_ENCODES_OWNER to
  MBEDTLS_PSA_CRYPTO_KEY_ID_ENCODES_OWNER
. PSA_KEY_ID_INIT to MBEDTLS_SVC_KEY_ID_INIT
. PSA_KEY_FILE_GET_KEY_ID to MBEDTLS_SVC_KEY_ID_GET_KEY_ID
. psa_key_file_id_make to mbedtls_svc_key_id_make

Signed-off-by: Ronald Cron <ronald.cron@arm.com>
2020-09-02 14:27:07 +02:00
Ronald Cron 039a98b598 Define always psa_key_id_t as defined in PSA crypto spec
Define always psa_key_id_t as defined in the PSA
Cryptography API specification independently of
whether the MBEDTLS_PSA_CRYPTO_KEY_FILE_ID_ENCODES_OWNER
configuration file is set or not.

As a consequence, get rid of `psa_app_key_id_t` that is
not needed anymore.

Signed-off-by: Ronald Cron <ronald.cron@arm.com>
2020-09-02 14:27:07 +02:00
Ronald Cron 27238fcbd8 psa: Use psa_key_file_id_t as the key id type
The purpose of this commit and the following is for
psa_key_id_t to always be as defined by the PSA
Cryptography API specification.

Currently psa_key_id_t departs from its specification
definition when MBEDTLS_PSA_CRYPTO_KEY_FILE_ID_ENCODES_OWNER
configuration flag is set. In that configuration, it is set
to be equal to psa_key_file_id_t which in that configuration
encodes an owner identifier along the key identifier.

Type psa_key_file_id_t was meant to be the key identifier type
used throughout the library code. If
MBEDTLS_PSA_CRYPTO_KEY_FILE_ID_ENCODES_OWNER is set it
includes both a key and owner identifier, otherwise it is
equal to psa_key_id_t.

It has not been the key identifier type throughout the
library so far because when the PSA Cryptography
specification was developped the library Doxygen
documentation was used to generate the PSA Cryptography API
specification thus the need to use psa_key_id_t and not
psa_key_file_id_t.

As this constraint does not hold anymore, move
to psa_key_file_id_t as the key identifier type throughout
the library code.

By the way, this commit updates the key identifier
initialization in the tests to be compatible with a
composit key identifier. A psa_key_id_make()
inline function is introduced to initialize key
identifiers (composit ot not) at runtime.

Signed-off-by: Ronald Cron <ronald.cron@arm.com>
2020-09-02 14:27:07 +02:00
Janos Follath 17ffc5da8d Bump version to Mbed TLS 2.24.0
Executed "./scripts/bump_version.sh --version 2.24.0"

Signed-off-by: Janos Follath <janos.follath@arm.com>
2020-08-26 16:22:57 +01:00
Janos Follath d2ce916b58 Merge branch 'development-restricted' 2020-08-26 14:15:34 +01:00
Gilles Peskine d4b9133850
Merge pull request #3611 from gilles-peskine-arm/psa-coverity-cleanups-202008
Minor fixes in PSA code and tests
2020-08-26 13:18:27 +02:00
Gilles Peskine 9e4d4387f0
Merge pull request #3433 from raoulstrackx/raoul/verify_crl_without_time
Always revoke certificate on CRL
2020-08-26 12:56:11 +02:00
Gilles Peskine a2e518daf5 Fix the documentation of has_even_parity
The documentation had the boolean meaning of the return value inverted.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-08-26 12:14:37 +02:00
Gilles Peskine ed9fbc6443 Clearer function name for parity check
Return a name that more clearly returns nonzero=true=good, 0=bad. We'd
normally expect check_xxx to return 0=pass, nonzero=fail so
check_parity was a bad name.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-08-26 11:16:50 +02:00
Gilles Peskine 6c75152b9f Explain the purpose of check_parity
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-08-26 10:24:26 +02:00
Gilles Peskine 34f063ca47 Add missing cleanup to hash multipart operation tests
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-08-26 10:24:13 +02:00
Gilles Peskine 29c4a6cf9f Add negative tests for MAC verification
Add negative tests for psa_mac_verify_finish: too large, too small, or
a changed byte.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-08-26 00:16:03 +02:00
Gilles Peskine 090e16cb8b Don't destroy the key during a MAC verification operation
An early draft of the PSA crypto specification required multipart
operations to keep working after destroying the key. This is no longer
the case: instead, now, operations are guaranteed to fail. Mbed TLS
does not comply yet, and still allows the operation to keep going.
Stop testing Mbed TLS's non-compliant behavior.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-08-26 00:16:03 +02:00
Gilles Peskine 8b356b5652 Test other output sizes for psa_mac_sign_finish
Test psa_mac_sign_finish with a smaller or larger buffer.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-08-26 00:16:03 +02:00
Gilles Peskine 5e65cec5e8 Simplify output bounds check in mac_sign test
Rely on Asan to detect a potential buffer overflow, instead of doing a
manual check. This makes the code simpler and Asan can detect
underflows as well as overflows.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-08-26 00:16:03 +02:00
Gilles Peskine 3d404d677e Test PSA_MAC_FINAL_SIZE in mac_sign exactly
We expect PSA_MAC_FINAL_SIZE to be exact in this implementation, so
check it here.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-08-26 00:16:03 +02:00
Gilles Peskine cd65f4ccac Add empty-output-buffer test cases for single-part hash functions
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-08-26 00:11:23 +02:00
Gilles Peskine e92c68a878 Note that a failure in cleanup is intentional
In the cleanup code for persistent_key_load_key_from_storage(), we
only attempt to reopen the key so that it will be deleted if it exists
at that point. It's intentional that we do nothing if psa_open_key()
fails here.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-08-26 00:11:23 +02:00
Gilles Peskine 64f13ef6ab Add missing cleanup to some multipart operation tests
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-08-26 00:11:23 +02:00
Gilles Peskine a09713c795 test cleanup: Annotate file removal after a failed creation
Let static analyzers know that it's ok if remove() fails here.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-08-25 22:50:18 +02:00
Gilles Peskine 14613bcd75 Fix parity tests to actually fail the test on error
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-08-25 22:30:31 +02:00
Janos Follath d4ac4e037b
Merge pull request #736 from mpg/cf-varpos-copy-dev-restricted
Constant-flow copy of HMAC from variable position
2020-08-25 14:35:55 +01:00
Manuel Pégourié-Gonnard 04b7488411 Fix potential use of uninitialised variable
If any of the TEST_ASSERT()s that are before the call to
mbedtls_pk_warp_as_opaque() failed, when reaching the exit label
psa_destroy_key() would be called with an uninitialized argument.

Found by Clang.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-08-25 10:45:51 +02:00
Gilles Peskine 0f343ac87f
Merge pull request #3528 from gufe44/helpers-redirect-restore-output
Fix bug in redirection of unit test outputs
2020-08-24 10:45:08 +02:00
Manuel Pégourié-Gonnard 8a79b9b68c Fix "unused function" warning in some configs
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-08-24 10:29:30 +02:00
gufe44 067f6e01f1 Fix bug in redirection of unit test outputs
Avoid replacing handle. stdout is defined as a macro on several platforms.

Signed-off-by: gufe44 <gu981@protonmail.com>
2020-08-21 08:32:26 +02:00
Dan Handley abccfc1684 Merge development into development-restricted
* development:
  Update copyright notices to use Linux Foundation guidance
  Undef ASSERT before defining it to ensure that no previous definition has sneaked in through included files.
  Add ChangeLog entry for X.509 CN-type vulnerability
  Improve documentation of cn in x509_crt_verify()
  Fix comparison between different name types
  Add test: DNS names should not match IP addresses
  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
  ssl-opt.sh: remove old buildbot-specific condition
  ssl-opt.sh: add proxy to all DTLS tests

Signed-off-by: Dan Handley <dan.handley@arm.com>
2020-08-20 11:07:12 +01:00
Manuel Pégourié-Gonnard feb0396d20 Fix memory leak in test_suite_x509write with PSA crypto
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>
2020-08-20 10:21:32 +02:00
Manuel Pégourié-Gonnard 7fe2c5f086 Add mbedtls_ssl_cf_memcpy_offset() with tests
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>
2020-08-19 11:56:02 +02:00
Bence Szépkúti 1e14827beb Update copyright notices to use Linux Foundation guidance
As a result, the copyright of contributors other than Arm is now
acknowledged, and the years of publishing are no longer tracked in the
source files.

Also remove the now-redundant lines declaring that the files are part of
MbedTLS.

This commit was generated using the following script:

# ========================
#!/bin/sh

# Find files
find '(' -path './.git' -o -path './3rdparty' ')' -prune -o -type f -print | xargs sed -bi '

# Replace copyright attribution line
s/Copyright.*Arm.*/Copyright The Mbed TLS Contributors/I

# Remove redundant declaration and the preceding line
$!N
/This file is part of Mbed TLS/Id
P
D
'
# ========================

Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>
2020-08-19 10:35:41 +02:00
Raoul Strackx a4e86141f1 Always revoke certificate on CRL
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>
2020-08-17 09:05:03 +02:00
Gilles Peskine 71f4fa13bb
Merge pull request #721 from gilles-peskine-arm/x509parse_crl-empty_entry-development
Fix buffer overflow in x509_get_entries (oss-fuzz 24123)
2020-08-14 23:22:31 +02:00
Manuel Pégourié-Gonnard 8ca03a7b68
Merge pull request #3554 from mpg/x509-verify-non-dns-san-dev
X509 verify non-DNS SANs
2020-08-14 11:32:22 +02:00
Manuel Pégourié-Gonnard c4af324a4b Merge branch 'development' into development-restricted
* 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
  ...
2020-08-14 10:11:21 +02:00
Manuel Pégourié-Gonnard 7d2a4d873f Add test: DNS names should not match IP addresses
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-08-11 10:23:52 +02:00
Manuel Pégourié-Gonnard eaa0739143
Merge pull request #716 from mpg/ct-varlen-hmac
Add constant-flow variable-length HMAC function
2020-08-10 12:40:33 +02:00
Gilles Peskine 210bea38f3 Add the decomposition of the base case as a comment
Put the base good case first, then the bad cases derived from it.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
2020-08-06 17:12:03 +02:00
Steven Cooreman d4867877f1 Initialize key pointer in ecdh to NULL
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>
2020-08-05 17:38:47 +02:00
Steven Cooreman 3fa684ed91 Allow importing Montgomery public keys in PSA Crypto
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>
2020-07-30 15:35:14 +02:00
Manuel Pégourié-Gonnard ea6a740923
Merge pull request #717 from mpg/non-etm-cbc-negative-testing
Add negative tests for non-EtM CBC decryption
2020-07-22 13:33:49 +02:00
Manuel Pégourié-Gonnard e55653f085 Improve comments about padlen convention
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>
2020-07-22 11:42:57 +02:00
Manuel Pégourié-Gonnard c3219006ff Fix suboptimal use of ASSER_ALLOC()
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>
2020-07-22 11:31:19 +02:00
Manuel Pégourié-Gonnard ca8287cbaf Use test_set_step() in loop in cf_hmac test
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>
2020-07-22 11:31:19 +02:00
Manuel Pégourié-Gonnard ed0e86428d Factor repeated condition to its own macro
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-07-22 11:31:19 +02:00
Manuel Pégourié-Gonnard 9670a59230 Start testing cf_hmac() for constant flow
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>
2020-07-22 11:31:18 +02:00
Manuel Pégourié-Gonnard b51f04466f Fix misleading comment in test function
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>
2020-07-21 10:40:25 +02:00
Manuel Pégourié-Gonnard 864abbff4e Rework how lengths are expressed in CBC test
This is hopefully more readable in the .data file.

Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-07-21 10:37:14 +02:00
Gilles Peskine b2281e1cf0 x509parse_crl: more negative test cases
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>
2020-07-16 18:18:22 +02:00
Manuel Pégourié-Gonnard 4adc04a8a3 Give a constant a name in test function
Signed-off-by: Manuel Pégourié-Gonnard <manuel.pegourie-gonnard@arm.com>
2020-07-16 10:00:48 +02:00
Manuel Pégourié-Gonnard 65a6fa3e26 Make cf_hmac() STATIC_TESTABLE
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>
2020-07-15 12:26:21 +02:00