Merge pull request #5540 from gilles-peskine-arm/check_config-chachapoly-2.28
Backport 2.28: Add check_config checks for AEAD
diff --git a/ChangeLog.d/PSA-test-suites-NOT-using-UID-0.txt b/ChangeLog.d/PSA-test-suites-NOT-using-UID-0.txt
new file mode 100644
index 0000000..9acbb0a
--- /dev/null
+++ b/ChangeLog.d/PSA-test-suites-NOT-using-UID-0.txt
@@ -0,0 +1,3 @@
+Bugfix
+ * Fix unit tests that used 0 as the file UID. This failed on some
+ implementations of PSA ITS. Fixes #3838.
diff --git a/ChangeLog.d/raw-agreement-destroy-missing.txt b/ChangeLog.d/raw-agreement-destroy-missing.txt
new file mode 100644
index 0000000..7342b8c
--- /dev/null
+++ b/ChangeLog.d/raw-agreement-destroy-missing.txt
@@ -0,0 +1,3 @@
+Bugfix
+ * Add missing key slot destruction calls when a raw key agreement or
+ a public key export fails in ssl_write_client_key_exchange.
diff --git a/ChangeLog.d/use-psa-ecdhe-curve.txt b/ChangeLog.d/use-psa-ecdhe-curve.txt
new file mode 100644
index 0000000..cc432bd
--- /dev/null
+++ b/ChangeLog.d/use-psa-ecdhe-curve.txt
@@ -0,0 +1,7 @@
+Bugfix
+ * Fix a bug in (D)TLS curve negotiation: when MBEDTLS_USE_PSA_CRYPTO was
+ enabled and an ECDHE-ECDSA or ECDHE-RSA key exchange was used, the
+ client would fail to check that the curve selected by the server for
+ ECDHE was indeed one that was offered. As a result, the client would
+ accept any curve that it supported, even if that curve was not allowed
+ according to its configuration.
diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h
index 6913dc0..f50cf9f 100644
--- a/include/mbedtls/ssl_internal.h
+++ b/include/mbedtls/ssl_internal.h
@@ -1112,6 +1112,7 @@
#if defined(MBEDTLS_ECP_C)
int mbedtls_ssl_check_curve( const mbedtls_ssl_context *ssl, mbedtls_ecp_group_id grp_id );
+int mbedtls_ssl_check_curve_tls_id( const mbedtls_ssl_context *ssl, uint16_t tls_id );
#endif
#if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED)
diff --git a/library/psa_its_file.c b/library/psa_its_file.c
index c4782cd..f058720 100644
--- a/library/psa_its_file.c
+++ b/library/psa_its_file.c
@@ -184,6 +184,11 @@
const void *p_data,
psa_storage_create_flags_t create_flags )
{
+ if( uid == 0 )
+ {
+ return( PSA_ERROR_INVALID_HANDLE );
+ }
+
psa_status_t status = PSA_ERROR_STORAGE_FAILURE;
char filename[PSA_ITS_STORAGE_FILENAME_LENGTH];
FILE *stream = NULL;
diff --git a/library/ssl_cli.c b/library/ssl_cli.c
index b87879c..c674506 100644
--- a/library/ssl_cli.c
+++ b/library/ssl_cli.c
@@ -2703,6 +2703,10 @@
tls_id <<= 8;
tls_id |= *(*p)++;
+ /* Check it's a curve we offered */
+ if( mbedtls_ssl_check_curve_tls_id( ssl, tls_id ) != 0 )
+ return( MBEDTLS_ERR_SSL_BAD_HS_SERVER_KEY_EXCHANGE );
+
/* Convert EC group to PSA key type. */
if( ( handshake->ecdh_psa_type =
mbedtls_psa_parse_tls_ecc_group( tls_id, &ecdh_bits ) ) == 0 )
@@ -3718,7 +3722,8 @@
if( ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_RSA ||
ciphersuite_info->key_exchange == MBEDTLS_KEY_EXCHANGE_ECDHE_ECDSA )
{
- psa_status_t status;
+ psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED;
+ psa_status_t destruction_status = PSA_ERROR_CORRUPTION_DETECTED;
psa_key_attributes_t key_attributes;
mbedtls_ssl_handshake_params *handshake = ssl->handshake;
@@ -3761,13 +3766,19 @@
own_pubkey, sizeof( own_pubkey ),
&own_pubkey_len );
if( status != PSA_SUCCESS )
+ {
+ psa_destroy_key( handshake->ecdh_psa_privkey );
+ handshake->ecdh_psa_privkey = MBEDTLS_SVC_KEY_ID_INIT;
return( MBEDTLS_ERR_SSL_HW_ACCEL_FAILED );
+ }
if( mbedtls_psa_tls_psa_ec_to_ecpoint( own_pubkey,
own_pubkey_len,
&own_pubkey_ecpoint,
&own_pubkey_ecpoint_len ) != 0 )
{
+ psa_destroy_key( handshake->ecdh_psa_privkey );
+ handshake->ecdh_psa_privkey = MBEDTLS_SVC_KEY_ID_INIT;
return( MBEDTLS_ERR_SSL_HW_ACCEL_FAILED );
}
@@ -3787,13 +3798,12 @@
ssl->handshake->premaster,
sizeof( ssl->handshake->premaster ),
&ssl->handshake->pmslen );
- if( status != PSA_SUCCESS )
- return( MBEDTLS_ERR_SSL_HW_ACCEL_FAILED );
- status = psa_destroy_key( handshake->ecdh_psa_privkey );
- if( status != PSA_SUCCESS )
- return( MBEDTLS_ERR_SSL_HW_ACCEL_FAILED );
+ destruction_status = psa_destroy_key( handshake->ecdh_psa_privkey );
handshake->ecdh_psa_privkey = MBEDTLS_SVC_KEY_ID_INIT;
+
+ if( status != PSA_SUCCESS || destruction_status != PSA_SUCCESS )
+ return( MBEDTLS_ERR_SSL_HW_ACCEL_FAILED );
}
else
#endif /* MBEDTLS_USE_PSA_CRYPTO &&
diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index c7265f1..bd0eb10 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -7326,6 +7326,18 @@
return( -1 );
}
+
+/*
+ * Same as mbedtls_ssl_check_curve() but takes a TLS ID for the curve.
+ */
+int mbedtls_ssl_check_curve_tls_id( const mbedtls_ssl_context *ssl, uint16_t tls_id )
+{
+ const mbedtls_ecp_curve_info *curve_info =
+ mbedtls_ecp_curve_info_from_tls_id( tls_id );
+ if( curve_info == NULL )
+ return( -1 );
+ return( mbedtls_ssl_check_curve( ssl, curve_info->grp_id ) );
+}
#endif /* MBEDTLS_ECP_C */
#if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED)
diff --git a/scripts/ci.requirements.txt b/scripts/ci.requirements.txt
index 18b40ec..338b14f 100644
--- a/scripts/ci.requirements.txt
+++ b/scripts/ci.requirements.txt
@@ -1,5 +1,9 @@
# Python package requirements for Mbed TLS testing.
+# Any package used by a script in this repository must be listed here
+# or in one of the included files. Normally there should be a minimum
+# version constraint; the CI will test with the minimum version.
+
# Use a known version of Pylint, because new versions tend to add warnings
# that could start rejecting our code.
# 2.4.4 is the version in Ubuntu 20.04. It supports Python >=3.5.
diff --git a/scripts/maintainer.requirements.txt b/scripts/maintainer.requirements.txt
index b149921..8734140 100644
--- a/scripts/maintainer.requirements.txt
+++ b/scripts/maintainer.requirements.txt
@@ -1,4 +1,5 @@
-# Python packages that are only useful to Mbed TLS maintainers.
+# Python packages that are not used by any script in this repository,
+# but are likely to be useful to Mbed TLS maintainers.
-r ci.requirements.txt
diff --git a/tests/scripts/analyze_outcomes.py b/tests/scripts/analyze_outcomes.py
index 73f16bd..d06a059 100755
--- a/tests/scripts/analyze_outcomes.py
+++ b/tests/scripts/analyze_outcomes.py
@@ -7,7 +7,6 @@
"""
import argparse
-import re
import sys
import traceback
@@ -51,29 +50,9 @@
"""
return len(self.successes) + len(self.failures)
-class TestDescriptions(check_test_cases.TestDescriptionExplorer):
- """Collect the available test cases."""
-
- def __init__(self):
- super().__init__()
- self.descriptions = set()
-
- def process_test_case(self, _per_file_state,
- file_name, _line_number, description):
- """Record an available test case."""
- base_name = re.sub(r'\.[^.]*$', '', re.sub(r'.*/', '', file_name))
- key = ';'.join([base_name, description.decode('utf-8')])
- self.descriptions.add(key)
-
-def collect_available_test_cases():
- """Collect the available test cases."""
- explorer = TestDescriptions()
- explorer.walk_all()
- return sorted(explorer.descriptions)
-
def analyze_coverage(results, outcomes):
"""Check that all available test cases are executed at least once."""
- available = collect_available_test_cases()
+ available = check_test_cases.collect_available_test_cases()
for key in available:
hits = outcomes[key].hits() if key in outcomes else 0
if hits == 0:
diff --git a/tests/scripts/check_test_cases.py b/tests/scripts/check_test_cases.py
index fe11f20..c9f5e11 100755
--- a/tests/scripts/check_test_cases.py
+++ b/tests/scripts/check_test_cases.py
@@ -134,6 +134,26 @@
if os.path.exists(ssl_opt_sh):
self.walk_ssl_opt_sh(ssl_opt_sh)
+class TestDescriptions(TestDescriptionExplorer):
+ """Collect the available test cases."""
+
+ def __init__(self):
+ super().__init__()
+ self.descriptions = set()
+
+ def process_test_case(self, _per_file_state,
+ file_name, _line_number, description):
+ """Record an available test case."""
+ base_name = re.sub(r'\.[^.]*$', '', re.sub(r'.*/', '', file_name))
+ key = ';'.join([base_name, description.decode('utf-8')])
+ self.descriptions.add(key)
+
+def collect_available_test_cases():
+ """Collect the available test cases."""
+ explorer = TestDescriptions()
+ explorer.walk_all()
+ return sorted(explorer.descriptions)
+
class DescriptionChecker(TestDescriptionExplorer):
"""Check all test case descriptions.
@@ -173,6 +193,9 @@
def main():
parser = argparse.ArgumentParser(description=__doc__)
+ parser.add_argument('--list-all',
+ action='store_true',
+ help='List all test cases, without doing checks')
parser.add_argument('--quiet', '-q',
action='store_true',
help='Hide warnings')
@@ -180,6 +203,10 @@
action='store_false', dest='quiet',
help='Show warnings (default: on; undoes --quiet)')
options = parser.parse_args()
+ if options.list_all:
+ descriptions = collect_available_test_cases()
+ sys.stdout.write('\n'.join(descriptions + ['']))
+ return
results = Results(options)
checker = DescriptionChecker(results)
checker.walk_all()
diff --git a/tests/suites/test_suite_psa_its.data b/tests/suites/test_suite_psa_its.data
index 9057a1a..06aed07 100644
--- a/tests/suites/test_suite_psa_its.data
+++ b/tests/suites/test_suite_psa_its.data
@@ -1,71 +1,74 @@
Set/get/remove 0 bytes
-set_get_remove:0:0:""
+set_get_remove:1:0:""
Set/get/remove 42 bytes
-set_get_remove:0:0:"000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f20212223242526272829"
+set_get_remove:1:0:"000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f20212223242526272829"
Set/get/remove 1000 bytes
-set_get_remove:0:0:"6a07ecfcc7c7bfe0129d56d2dcf2955a12845b9e6e0034b0ed7226764261c6222a07b9f654deb682130eb1cd07ed298324e60a46f9c76c8a5a0be000c69e93dd81054ca21fbc6190cef7745e9d5436f70e20e10cbf111d1d40c9ceb83be108775199d81abaf0fecfe30eaa08e7ed82517cba939de4449f7ac5c730fcbbf56e691640b0129db0e178045dd2034262de9138873d9bdca57685146a3d516ff13c29e6628a00097435a8e10fef7faff62d2963c303a93793e2211d8604556fec08cd59c0f5bd1f22eea64be13e88b3f454781e83fe6e771d3d81eb2fbe2021e276f42a93db5343d767d854115e74f5e129a8036b1e81aced9872709d515e00bcf2098ccdee23006b0e836b27dc8aaf30f53fe58a31a6408abb79b13098c22e262a98040f9b09809a3b43bd42eb01cf1d17bbc8b4dfe51fa6573d4d8741943e3ae71a649e194c1218f2e20556c7d8cfe8c64d8cc1aa94531fbf638768c7d19b3c079299cf4f26ed3f964efb8fd23d82b4157a51f46da11156c74e2d6e2fd788869ebb52429e12a82da2ba083e2e74565026162f29ca22582da72a2698e7c5d958b919bc2cdfe12f50364ccfed30efd5cd120a7d5f196b2bd7f911bb44d5871eb3dedcd70ece7faf464988f9fe361f23d7244b1e08bee921d0f28bdb4912675809d099876d4d15b7d13ece356e1f2a5dce64feb3d6749a07a4f2b7721190e17a9ab2966e48b6d25187070b81eb45b1c44608b2f0e175958ba57fcf1b2cd145eea5fd4de858d157ddac69dfbb5d5d6f0c1691b0fae5a143b6e58cdf5000f28d74b3322670ed11e740c828c7bfad4e2f392012da3ac931ea26ed15fd003e604071f5900c6e1329d021805d50da9f1e732a49bcc292d9f8e07737cfd59442e8d7aaa813b18183a68e22bf6b4519545dd7d2d519db3652be4131bad4f4b0625dbaa749e979f6ee8c1b97803cb50a2fa20dc883eac932a824b777b226e15294de6a80be3ddef41478fe18172d64407a004de6bae18bc60e90c902c1cbb0e1633395b42391f5011be0d480541987609b0cd8d902ea29f86f73e7362340119323eb0ea4f672b70d6e9a9df5235f9f1965f5cb0c2998c5a7f4754e83eeda5d95fefbbaaa0875fe37b7ca461e7281cc5479162627c5a709b45fd9ddcde4dfb40659e1d70fa7361d9fc7de24f9b8b13259423fdae4dbb98d691db687467a5a7eb027a4a0552a03e430ac8a32de0c30160ba60a036d6b9db2d6182193283337b92e7438dc5d6eb4fa00200d8efa9127f1c3a32ac8e202262773aaa5a965c6b8035b2e5706c32a55511560429ddf1df4ac34076b7eedd9cf94b6915a894fdd9084ffe3db0e7040f382c3cd04f0484595de95865c36b6bf20f46a78cdfb37228acbeb218de798b9586f6d99a0cbae47e80d"
+set_get_remove:1:0:"6a07ecfcc7c7bfe0129d56d2dcf2955a12845b9e6e0034b0ed7226764261c6222a07b9f654deb682130eb1cd07ed298324e60a46f9c76c8a5a0be000c69e93dd81054ca21fbc6190cef7745e9d5436f70e20e10cbf111d1d40c9ceb83be108775199d81abaf0fecfe30eaa08e7ed82517cba939de4449f7ac5c730fcbbf56e691640b0129db0e178045dd2034262de9138873d9bdca57685146a3d516ff13c29e6628a00097435a8e10fef7faff62d2963c303a93793e2211d8604556fec08cd59c0f5bd1f22eea64be13e88b3f454781e83fe6e771d3d81eb2fbe2021e276f42a93db5343d767d854115e74f5e129a8036b1e81aced9872709d515e00bcf2098ccdee23006b0e836b27dc8aaf30f53fe58a31a6408abb79b13098c22e262a98040f9b09809a3b43bd42eb01cf1d17bbc8b4dfe51fa6573d4d8741943e3ae71a649e194c1218f2e20556c7d8cfe8c64d8cc1aa94531fbf638768c7d19b3c079299cf4f26ed3f964efb8fd23d82b4157a51f46da11156c74e2d6e2fd788869ebb52429e12a82da2ba083e2e74565026162f29ca22582da72a2698e7c5d958b919bc2cdfe12f50364ccfed30efd5cd120a7d5f196b2bd7f911bb44d5871eb3dedcd70ece7faf464988f9fe361f23d7244b1e08bee921d0f28bdb4912675809d099876d4d15b7d13ece356e1f2a5dce64feb3d6749a07a4f2b7721190e17a9ab2966e48b6d25187070b81eb45b1c44608b2f0e175958ba57fcf1b2cd145eea5fd4de858d157ddac69dfbb5d5d6f0c1691b0fae5a143b6e58cdf5000f28d74b3322670ed11e740c828c7bfad4e2f392012da3ac931ea26ed15fd003e604071f5900c6e1329d021805d50da9f1e732a49bcc292d9f8e07737cfd59442e8d7aaa813b18183a68e22bf6b4519545dd7d2d519db3652be4131bad4f4b0625dbaa749e979f6ee8c1b97803cb50a2fa20dc883eac932a824b777b226e15294de6a80be3ddef41478fe18172d64407a004de6bae18bc60e90c902c1cbb0e1633395b42391f5011be0d480541987609b0cd8d902ea29f86f73e7362340119323eb0ea4f672b70d6e9a9df5235f9f1965f5cb0c2998c5a7f4754e83eeda5d95fefbbaaa0875fe37b7ca461e7281cc5479162627c5a709b45fd9ddcde4dfb40659e1d70fa7361d9fc7de24f9b8b13259423fdae4dbb98d691db687467a5a7eb027a4a0552a03e430ac8a32de0c30160ba60a036d6b9db2d6182193283337b92e7438dc5d6eb4fa00200d8efa9127f1c3a32ac8e202262773aaa5a965c6b8035b2e5706c32a55511560429ddf1df4ac34076b7eedd9cf94b6915a894fdd9084ffe3db0e7040f382c3cd04f0484595de95865c36b6bf20f46a78cdfb37228acbeb218de798b9586f6d99a0cbae47e80d"
Set/get/remove with flags
-set_get_remove:0:0x12345678:"abcdef"
+set_get_remove:1:0x12345678:"abcdef"
Overwrite 0 -> 3
-set_overwrite:0:0x12345678:"":0x01020304:"abcdef"
+set_overwrite:1:0x12345678:"":0x01020304:"abcdef"
Overwrite 3 -> 0
-set_overwrite:0:0x12345678:"abcdef":0x01020304:""
+set_overwrite:1:0x12345678:"abcdef":0x01020304:""
Overwrite 3 -> 3
-set_overwrite:0:0x12345678:"123456":0x01020304:"abcdef"
+set_overwrite:1:0x12345678:"123456":0x01020304:"abcdef"
Overwrite 3 -> 18
-set_overwrite:0:0x12345678:"abcdef":0x01020304:"404142434445464748494a4b4c4d4e4f5051"
+set_overwrite:1:0x12345678:"abcdef":0x01020304:"404142434445464748494a4b4c4d4e4f5051"
Overwrite 18 -> 3
-set_overwrite:0:0x12345678:"404142434445464748494a4b4c4d4e4f5051":0x01020304:"abcdef"
+set_overwrite:1:0x12345678:"404142434445464748494a4b4c4d4e4f5051":0x01020304:"abcdef"
Multiple files
-set_multiple:0:5
+set_multiple:1:5
+
+Set UID 0
+set_fail:0:"40414243444546474849":PSA_ERROR_INVALID_HANDLE
Non-existent file
-nonexistent:0:0
+nonexistent:1:0
Removed file
-nonexistent:0:1
+nonexistent:1:1
Get 0 bytes of 10 at 10
-get_at:0:"40414243444546474849":10:0:PSA_SUCCESS
+get_at:1:"40414243444546474849":10:0:PSA_SUCCESS
Get 1 byte of 10 at 9
-get_at:0:"40414243444546474849":9:1:PSA_SUCCESS
+get_at:1:"40414243444546474849":9:1:PSA_SUCCESS
Get 0 bytes of 10 at 0
-get_at:0:"40414243444546474849":0:0:PSA_SUCCESS
+get_at:1:"40414243444546474849":0:0:PSA_SUCCESS
Get 1 byte of 10 at 0
-get_at:0:"40414243444546474849":0:1:PSA_SUCCESS
+get_at:1:"40414243444546474849":0:1:PSA_SUCCESS
Get 2 bytes of 10 at 1
-get_at:0:"40414243444546474849":1:2:PSA_SUCCESS
+get_at:1:"40414243444546474849":1:2:PSA_SUCCESS
Get 1 byte of 10 at 10: out of range
-get_at:0:"40414243444546474849":10:1:PSA_ERROR_INVALID_ARGUMENT
+get_at:1:"40414243444546474849":10:1:PSA_ERROR_INVALID_ARGUMENT
Get 1 byte of 10 at 11: out of range
-get_at:0:"40414243444546474849":11:1:PSA_ERROR_INVALID_ARGUMENT
+get_at:1:"40414243444546474849":11:1:PSA_ERROR_INVALID_ARGUMENT
Get 0 bytes of 10 at 11: out of range
-get_at:0:"40414243444546474849":11:0:PSA_ERROR_INVALID_ARGUMENT
+get_at:1:"40414243444546474849":11:0:PSA_ERROR_INVALID_ARGUMENT
Get -1 byte of 10 at 10: out of range
-get_at:0:"40414243444546474849":10:-1:PSA_ERROR_INVALID_ARGUMENT
+get_at:1:"40414243444546474849":10:-1:PSA_ERROR_INVALID_ARGUMENT
Get 1 byte of 10 at -1: out of range
-get_at:0:"40414243444546474849":-1:1:PSA_ERROR_INVALID_ARGUMENT
+get_at:1:"40414243444546474849":-1:1:PSA_ERROR_INVALID_ARGUMENT
Overwrite ITS header magic
-get_fail:0:"40414243444546474849":1:0:PSA_ERROR_DATA_CORRUPT
+get_fail:1:"40414243444546474849":1:0:PSA_ERROR_DATA_CORRUPT
Truncate ITS header
-get_fail:0:"40414243444546474849":0:1:PSA_ERROR_DATA_CORRUPT
+get_fail:1:"40414243444546474849":0:1:PSA_ERROR_DATA_CORRUPT
diff --git a/tests/suites/test_suite_psa_its.function b/tests/suites/test_suite_psa_its.function
index fb9ce07..e16c050 100644
--- a/tests/suites/test_suite_psa_its.function
+++ b/tests/suites/test_suite_psa_its.function
@@ -285,3 +285,16 @@
cleanup( );
}
/* END_CASE */
+
+/* BEGIN_CASE */
+void set_fail( int uid_arg, data_t *data,
+ int expected_status )
+{
+ psa_storage_uid_t uid = uid_arg;
+ TEST_ASSERT( psa_its_set_wrap( uid, data->len, data->x, 0 ) ==
+ (psa_status_t) expected_status );
+
+ exit:
+ cleanup( );
+}
+/* END_CASE */