Merge pull request #7197 from daverodgman/armclang-sha-warning
Enable -Werror in all.sh for armclang
diff --git a/include/mbedtls/check_config.h b/include/mbedtls/check_config.h
index ac374d2..654a845 100644
--- a/include/mbedtls/check_config.h
+++ b/include/mbedtls/check_config.h
@@ -712,41 +712,6 @@
#if defined(MBEDTLS_SHA512_ALT) || defined(MBEDTLS_SHA512_PROCESS_ALT)
#error "MBEDTLS_SHA512_*ALT can't be used with MBEDTLS_SHA512_USE_A64_CRYPTO_*"
#endif
-/*
- * Best performance comes from most recent compilers, with intrinsics and -O3.
- * Must compile with -march=armv8.2-a+sha3, but we can't detect armv8.2-a, and
- * can't always detect __ARM_FEATURE_SHA512 (notably clang 7-12).
- *
- * GCC < 8 won't work at all (lacks the sha512 instructions)
- * GCC >= 8 uses intrinsics, sets __ARM_FEATURE_SHA512
- *
- * Clang < 7 won't work at all (lacks the sha512 instructions)
- * Clang 7-12 don't have intrinsics (but we work around that with inline
- * assembler) or __ARM_FEATURE_SHA512
- * Clang == 13.0.0 same as clang 12 (only seen on macOS)
- * Clang >= 13.0.1 has __ARM_FEATURE_SHA512 and intrinsics
- */
-#if defined(__aarch64__) && !defined(__ARM_FEATURE_SHA512)
- /* Test Clang first, as it defines __GNUC__ */
-# if defined(__clang__)
-# if __clang_major__ < 7
-# error "A more recent Clang is required for MBEDTLS_SHA512_USE_A64_CRYPTO_*"
-# elif __clang_major__ < 13 || \
- (__clang_major__ == 13 && __clang_minor__ == 0 && __clang_patchlevel__ == 0)
- /* We implement the intrinsics with inline assembler, so don't error */
-# else
-# error "Must use minimum -march=armv8.2-a+sha3 for MBEDTLS_SHA512_USE_A64_CRYPTO_*"
-# endif
-# elif defined(__GNUC__)
-# if __GNUC__ < 8
-# error "A more recent GCC is required for MBEDTLS_SHA512_USE_A64_CRYPTO_*"
-# else
-# error "Must use minimum -march=armv8.2-a+sha3 for MBEDTLS_SHA512_USE_A64_CRYPTO_*"
-# endif
-# else
-# error "Only GCC and Clang supported for MBEDTLS_SHA512_USE_A64_CRYPTO_*"
-# endif
-#endif
#endif /* MBEDTLS_SHA512_USE_A64_CRYPTO_IF_PRESENT || MBEDTLS_SHA512_USE_A64_CRYPTO_ONLY */
@@ -767,9 +732,7 @@
#if defined(MBEDTLS_SHA256_ALT) || defined(MBEDTLS_SHA256_PROCESS_ALT)
#error "MBEDTLS_SHA256_*ALT can't be used with MBEDTLS_SHA256_USE_A64_CRYPTO_*"
#endif
-#if defined(__aarch64__) && !defined(__ARM_FEATURE_CRYPTO)
-#error "Must use minimum -march=armv8-a+crypto for MBEDTLS_SHA256_USE_A64_CRYPTO_*"
-#endif
+
#endif
#if defined(MBEDTLS_SHA256_USE_A64_CRYPTO_ONLY) && \
diff --git a/include/mbedtls/mbedtls_config.h b/include/mbedtls/mbedtls_config.h
index 5aff9c5..1995e54 100644
--- a/include/mbedtls/mbedtls_config.h
+++ b/include/mbedtls/mbedtls_config.h
@@ -3115,9 +3115,6 @@
* \note If MBEDTLS_SHA256_USE_A64_CRYPTO_IF_PRESENT is defined when building
* for a non-Aarch64 build it will be silently ignored.
*
- * \note The code uses Neon intrinsics, so \c CFLAGS must be set to a minimum
- * of \c -march=armv8-a+crypto.
- *
* \warning MBEDTLS_SHA256_USE_A64_CRYPTO_IF_PRESENT cannot be defined at the
* same time as MBEDTLS_SHA256_USE_A64_CRYPTO_ONLY.
*
@@ -3140,9 +3137,6 @@
* \note This allows builds with a smaller code size than with
* MBEDTLS_SHA256_USE_A64_CRYPTO_IF_PRESENT
*
- * \note The code uses Neon intrinsics, so \c CFLAGS must be set to a minimum
- * of \c -march=armv8-a+crypto.
- *
* \warning MBEDTLS_SHA256_USE_A64_CRYPTO_ONLY cannot be defined at the same
* time as MBEDTLS_SHA256_USE_A64_CRYPTO_IF_PRESENT.
*
@@ -3197,9 +3191,7 @@
* for a non-Aarch64 build it will be silently ignored.
*
* \note The code uses the SHA-512 Neon intrinsics, so requires GCC >= 8 or
- * Clang >= 7, and \c CFLAGS must be set to a minimum of
- * \c -march=armv8.2-a+sha3. An optimisation level of \c -O3 generates the
- * fastest code.
+ * Clang >= 7.
*
* \warning MBEDTLS_SHA512_USE_A64_CRYPTO_IF_PRESENT cannot be defined at the
* same time as MBEDTLS_SHA512_USE_A64_CRYPTO_ONLY.
@@ -3224,9 +3216,7 @@
* MBEDTLS_SHA512_USE_A64_CRYPTO_IF_PRESENT
*
* \note The code uses the SHA-512 Neon intrinsics, so requires GCC >= 8 or
- * Clang >= 7, and \c CFLAGS must be set to a minimum of
- * \c -march=armv8.2-a+sha3. An optimisation level of \c -O3 generates the
- * fastest code.
+ * Clang >= 7.
*
* \warning MBEDTLS_SHA512_USE_A64_CRYPTO_ONLY cannot be defined at the same
* time as MBEDTLS_SHA512_USE_A64_CRYPTO_IF_PRESENT.
diff --git a/library/sha256.c b/library/sha256.c
index cb09a71..23cd406 100644
--- a/library/sha256.c
+++ b/library/sha256.c
@@ -22,6 +22,23 @@
* http://csrc.nist.gov/publications/fips/fips180-2/fips180-2.pdf
*/
+#if defined(__aarch64__) && !defined(__ARM_FEATURE_CRYPTO) && \
+ defined(__clang__) && __clang_major__ < 18 && __clang_major__ > 3
+/* TODO: Re-consider above after https://reviews.llvm.org/D131064 merged.
+ *
+ * The intrinsic declaration are guarded by predefined ACLE macros in clang:
+ * these are normally only enabled by the -march option on the command line.
+ * By defining the macros ourselves we gain access to those declarations without
+ * requiring -march on the command line.
+ *
+ * `arm_neon.h` could be included by any header file, so we put these defines
+ * at the top of this file, before any includes.
+ */
+#define __ARM_FEATURE_CRYPTO 1
+#define NEED_TARGET_OPTIONS
+#endif /* __aarch64__ && __clang__ &&
+ !__ARM_FEATURE_CRYPTO && __clang_major__ < 18 && __clang_major__ > 3 */
+
#include "common.h"
#if defined(MBEDTLS_SHA256_C) || defined(MBEDTLS_SHA224_C)
@@ -37,6 +54,30 @@
#if defined(__aarch64__)
# if defined(MBEDTLS_SHA256_USE_A64_CRYPTO_IF_PRESENT) || \
defined(MBEDTLS_SHA256_USE_A64_CRYPTO_ONLY)
+/* *INDENT-OFF* */
+# if !defined(__ARM_FEATURE_CRYPTO) || defined(NEED_TARGET_OPTIONS)
+# if defined(__clang__)
+# if __clang_major__ < 4
+# error "A more recent Clang is required for MBEDTLS_SHA256_USE_A64_CRYPTO_*"
+# endif
+# pragma clang attribute push (__attribute__((target("crypto"))), apply_to=function)
+# define MBEDTLS_POP_TARGET_PRAGMA
+# elif defined(__GNUC__)
+ /* FIXME: GCC-5 annouce crypto extension, but some intrinsic are missed.
+ * Known miss intrinsic can be workaround.
+ */
+# if __GNUC__ < 6
+# error "A more recent GCC is required for MBEDTLS_SHA256_USE_A64_CRYPTO_*"
+# else
+# pragma GCC push_options
+# pragma GCC target ("arch=armv8-a+crypto")
+# define MBEDTLS_POP_TARGET_PRAGMA
+# endif
+# else
+# error "Only GCC and Clang supported for MBEDTLS_SHA256_USE_A64_CRYPTO_*"
+# endif
+# endif
+/* *INDENT-ON* */
# include <arm_neon.h>
# endif
# if defined(MBEDTLS_SHA256_USE_A64_CRYPTO_IF_PRESENT)
@@ -353,8 +394,16 @@
SHA256_BLOCK_SIZE) ? 0 : -1;
}
-#endif /* MBEDTLS_SHA256_USE_A64_CRYPTO_IF_PRESENT || MBEDTLS_SHA256_USE_A64_CRYPTO_ONLY */
+#if defined(MBEDTLS_POP_TARGET_PRAGMA)
+#if defined(__clang__)
+#pragma clang attribute pop
+#elif defined(__GNUC__)
+#pragma GCC pop_options
+#endif
+#undef MBEDTLS_POP_TARGET_PRAGMA
+#endif
+#endif /* MBEDTLS_SHA256_USE_A64_CRYPTO_IF_PRESENT || MBEDTLS_SHA256_USE_A64_CRYPTO_ONLY */
#if !defined(MBEDTLS_SHA256_USE_A64_CRYPTO_IF_PRESENT)
#define mbedtls_internal_sha256_process_many_c mbedtls_internal_sha256_process_many
diff --git a/library/sha512.c b/library/sha512.c
index efcbed4..bc92a8d 100644
--- a/library/sha512.c
+++ b/library/sha512.c
@@ -22,6 +22,26 @@
* http://csrc.nist.gov/publications/fips/fips180-2/fips180-2.pdf
*/
+#if defined(__aarch64__) && !defined(__ARM_FEATURE_SHA512) && \
+ defined(__clang__) && __clang_major__ < 18 && \
+ __clang_major__ >= 13 && __clang_minor__ > 0 && __clang_patchlevel__ > 0
+/* TODO: Re-consider above after https://reviews.llvm.org/D131064 merged.
+ *
+ * The intrinsic declaration are guarded by predefined ACLE macros in clang:
+ * these are normally only enabled by the -march option on the command line.
+ * By defining the macros ourselves we gain access to those declarations without
+ * requiring -march on the command line.
+ *
+ * `arm_neon.h` could be included by any header file, so we put these defines
+ * at the top of this file, before any includes.
+ */
+#define __ARM_FEATURE_SHA512 1
+#define NEED_TARGET_OPTIONS
+#endif /* __aarch64__ && __clang__ &&
+ !__ARM_FEATURE_SHA512 && __clang_major__ < 18 &&
+ __clang_major__ >= 13 && __clang_minor__ > 0 &&
+ __clang_patchlevel__ > 0 */
+
#include "common.h"
#if defined(MBEDTLS_SHA512_C) || defined(MBEDTLS_SHA384_C)
@@ -43,6 +63,47 @@
#if defined(__aarch64__)
# if defined(MBEDTLS_SHA512_USE_A64_CRYPTO_IF_PRESENT) || \
defined(MBEDTLS_SHA512_USE_A64_CRYPTO_ONLY)
+/* *INDENT-OFF* */
+/*
+ * Best performance comes from most recent compilers, with intrinsics and -O3.
+ * Must compile with -march=armv8.2-a+sha3, but we can't detect armv8.2-a, and
+ * can't always detect __ARM_FEATURE_SHA512 (notably clang 7-12).
+ *
+ * GCC < 8 won't work at all (lacks the sha512 instructions)
+ * GCC >= 8 uses intrinsics, sets __ARM_FEATURE_SHA512
+ *
+ * Clang < 7 won't work at all (lacks the sha512 instructions)
+ * Clang 7-12 don't have intrinsics (but we work around that with inline
+ * assembler) or __ARM_FEATURE_SHA512
+ * Clang == 13.0.0 same as clang 12 (only seen on macOS)
+ * Clang >= 13.0.1 has __ARM_FEATURE_SHA512 and intrinsics
+ */
+# if !defined(__ARM_FEATURE_SHA512) || defined(NEED_TARGET_OPTIONS)
+ /* Test Clang first, as it defines __GNUC__ */
+# if defined(__clang__)
+# if __clang_major__ < 7
+# error "A more recent Clang is required for MBEDTLS_SHA512_USE_A64_CRYPTO_*"
+# elif __clang_major__ < 13 || \
+ (__clang_major__ == 13 && __clang_minor__ == 0 && \
+ __clang_patchlevel__ == 0)
+ /* We implement the intrinsics with inline assembler, so don't error */
+# else
+# pragma clang attribute push (__attribute__((target("sha3"))), apply_to=function)
+# define MBEDTLS_POP_TARGET_PRAGMA
+# endif
+# elif defined(__GNUC__)
+# if __GNUC__ < 8
+# error "A more recent GCC is required for MBEDTLS_SHA512_USE_A64_CRYPTO_*"
+# else
+# pragma GCC push_options
+# pragma GCC target ("arch=armv8.2-a+sha3")
+# define MBEDTLS_POP_TARGET_PRAGMA
+# endif
+# else
+# error "Only GCC and Clang supported for MBEDTLS_SHA512_USE_A64_CRYPTO_*"
+# endif
+# endif
+/* *INDENT-ON* */
# include <arm_neon.h>
# endif
# if defined(MBEDTLS_SHA512_USE_A64_CRYPTO_IF_PRESENT)
@@ -516,6 +577,15 @@
SHA512_BLOCK_SIZE) ? 0 : -1;
}
+#if defined(MBEDTLS_POP_TARGET_PRAGMA)
+#if defined(__clang__)
+#pragma clang attribute pop
+#elif defined(__GNUC__)
+#pragma GCC pop_options
+#endif
+#undef MBEDTLS_POP_TARGET_PRAGMA
+#endif
+
#endif /* MBEDTLS_SHA512_USE_A64_CRYPTO_IF_PRESENT || MBEDTLS_SHA512_USE_A64_CRYPTO_ONLY */
diff --git a/tests/scripts/depends.py b/tests/scripts/depends.py
index 52ca412..581baad 100755
--- a/tests/scripts/depends.py
+++ b/tests/scripts/depends.py
@@ -23,7 +23,7 @@
This script can be divided into several steps:
First, include/mbedtls/mbedtls_config.h or a different config file passed
-in the arguments is parsed to extract any configuration options (collect_config_symbols).
+in the arguments is parsed to extract any configuration options (using config.py).
Then, test domains (groups of jobs, tests) are built based on predefined data
collected in the DomainData class. Here, each domain has five major traits:
@@ -65,6 +65,11 @@
import subprocess
import sys
import traceback
+from typing import Union
+
+# Add the Mbed TLS Python library directory to the module search path
+import scripts_path # pylint: disable=unused-import
+import config
class Colors: # pylint: disable=too-few-public-methods
"""Minimalistic support for colored output.
@@ -74,6 +79,7 @@
stop switches the text color back to the default."""
red = None
green = None
+ cyan = None
bold_red = None
bold_green = None
def __init__(self, options=None):
@@ -89,6 +95,7 @@
normal = '\033[0m'
self.red = ('\033[31m', normal)
self.green = ('\033[32m', normal)
+ self.cyan = ('\033[36m', normal)
self.bold_red = ('\033[1;31m', normal)
self.bold_green = ('\033[1;32m', normal)
NO_COLORS = Colors(None)
@@ -124,34 +131,38 @@
else:
shutil.copy(options.config_backup, options.config)
-def run_config_py(options, args):
- """Run scripts/config.py with the specified arguments."""
- cmd = ['scripts/config.py']
- if options.config != 'include/mbedtls/mbedtls_config.h':
- cmd += ['--file', options.config]
- cmd += args
- log_command(cmd)
- subprocess.check_call(cmd)
+def option_exists(conf, option):
+ return option in conf.settings
-def set_reference_config(options):
+def set_config_option_value(conf, option, colors, value: Union[bool, str]):
+ """Set/unset a configuration option, optionally specifying a value.
+value can be either True/False (set/unset config option), or a string,
+which will make a symbol defined with a certain value."""
+ if not option_exists(conf, option):
+ log_line('Symbol {} was not found in {}'.format(option, conf.filename), color=colors.red)
+ return False
+
+ if value is False:
+ log_command(['config.py', 'unset', option])
+ conf.unset(option)
+ elif value is True:
+ log_command(['config.py', 'set', option])
+ conf.set(option)
+ else:
+ log_command(['config.py', 'set', option, value])
+ conf.set(option, value)
+ return True
+
+def set_reference_config(conf, options, colors):
"""Change the library configuration file (mbedtls_config.h) to the reference state.
The reference state is the one from which the tested configurations are
derived."""
# Turn off options that are not relevant to the tests and slow them down.
- run_config_py(options, ['full'])
- run_config_py(options, ['unset', 'MBEDTLS_TEST_HOOKS'])
+ log_command(['config.py', 'full'])
+ conf.adapt(config.full_adapter)
+ set_config_option_value(conf, 'MBEDTLS_TEST_HOOKS', colors, False)
if options.unset_use_psa:
- run_config_py(options, ['unset', 'MBEDTLS_USE_PSA_CRYPTO'])
-
-def collect_config_symbols(options):
- """Read the list of settings from mbedtls_config.h.
-Return them in a generator."""
- with open(options.config, encoding="utf-8") as config_file:
- rx = re.compile(r'\s*(?://\s*)?#define\s+(\w+)\s*(?:$|/[/*])')
- for line in config_file:
- m = re.match(rx, line)
- if m:
- yield m.group(1)
+ set_config_option_value(conf, 'MBEDTLS_USE_PSA_CRYPTO', colors, False)
class Job:
"""A job builds the library in a specific configuration and runs some tests."""
@@ -179,19 +190,16 @@
elif what is False:
log_line(self.name + ' FAILED', color=colors.red)
else:
- log_line('starting ' + self.name)
+ log_line('starting ' + self.name, color=colors.cyan)
- def configure(self, options):
+ def configure(self, conf, options, colors):
'''Set library configuration options as required for the job.'''
- set_reference_config(options)
+ set_reference_config(conf, options, colors)
for key, value in sorted(self.config_settings.items()):
- if value is True:
- args = ['set', key]
- elif value is False:
- args = ['unset', key]
- else:
- args = ['set', key, value]
- run_config_py(options, args)
+ ret = set_config_option_value(conf, key, colors, value)
+ if ret is False:
+ return False
+ return True
def test(self, options):
'''Run the job's build and test commands.
@@ -382,11 +390,11 @@
return [symbol for symbol in self.all_config_symbols
if re.match(regexp, symbol)]
- def __init__(self, options):
+ def __init__(self, options, conf):
"""Gather data about the library and establish a list of domains to test."""
build_command = [options.make_command, 'CFLAGS=-Werror']
build_and_test = [build_command, [options.make_command, 'test']]
- self.all_config_symbols = set(collect_config_symbols(options))
+ self.all_config_symbols = set(conf.settings.keys())
# Find hash modules by name.
hash_symbols = self.config_symbols_matching(r'MBEDTLS_(MD|RIPEMD|SHA)[0-9]+_C\Z')
# Find elliptic curve enabling macros by name.
@@ -442,16 +450,19 @@
else:
return [self.jobs[name]]
-def run(options, job, colors=NO_COLORS):
+def run(options, job, conf, colors=NO_COLORS):
"""Run the specified job (a Job instance)."""
subprocess.check_call([options.make_command, 'clean'])
job.announce(colors, None)
- job.configure(options)
+ if not job.configure(conf, options, colors):
+ job.announce(colors, False)
+ return False
+ conf.write()
success = job.test(options)
job.announce(colors, success)
return success
-def run_tests(options, domain_data):
+def run_tests(options, domain_data, conf):
"""Run the desired jobs.
domain_data should be a DomainData instance that describes the available
domains and jobs.
@@ -467,7 +478,7 @@
backup_config(options)
try:
for job in jobs:
- success = run(options, job, colors=colors)
+ success = run(options, job, conf, colors=colors)
if not success:
if options.keep_going:
failures.append(job.name)
@@ -533,7 +544,9 @@
default=True)
options = parser.parse_args()
os.chdir(options.directory)
- domain_data = DomainData(options)
+ conf = config.ConfigFile(options.config)
+ domain_data = DomainData(options, conf)
+
if options.tasks is True:
options.tasks = sorted(domain_data.domains.keys())
if options.list:
@@ -542,7 +555,7 @@
print(domain_name)
sys.exit(0)
else:
- sys.exit(0 if run_tests(options, domain_data) else 1)
+ sys.exit(0 if run_tests(options, domain_data, conf) else 1)
except Exception: # pylint: disable=broad-except
traceback.print_exc()
sys.exit(3)
diff --git a/tests/suites/test_suite_psa_crypto.data b/tests/suites/test_suite_psa_crypto.data
index f340a7f..cfcdac1 100644
--- a/tests/suites/test_suite_psa_crypto.data
+++ b/tests/suites/test_suite_psa_crypto.data
@@ -4561,9 +4561,9 @@
depends_on:PSA_WANT_ALG_ECDSA:PSA_WANT_ALG_SHA_256:PSA_WANT_KEY_TYPE_ECC_KEY_PAIR:MBEDTLS_PK_PARSE_C:PSA_WANT_ECC_SECP_R1_256
interruptible_signverify_hash_edgecase_tests:PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_FAMILY_SECP_R1):"ab45435712649cb30bbddac49197eebf2740ffc7f874d9244c3460f54f322d3a":PSA_ALG_ECDSA(PSA_ALG_SHA_256):"9ac4335b469bbd791439248504dd0d49c71349a295fee5a1c68507f45a9e1c7b"
-PSA sign/vrfy hash int max ops tests: randomized ECDSA SECP256R1 SHA-256
+PSA sign/vrfy hash int ops tests: randomized ECDSA SECP256R1 SHA-256
depends_on:PSA_WANT_ALG_ECDSA:PSA_WANT_ALG_SHA_256:PSA_WANT_KEY_TYPE_ECC_KEY_PAIR:MBEDTLS_PK_PARSE_C:PSA_WANT_ECC_SECP_R1_256
-interruptible_signverify_hash_maxops_tests:PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_FAMILY_SECP_R1):"ab45435712649cb30bbddac49197eebf2740ffc7f874d9244c3460f54f322d3a":PSA_ALG_ECDSA(PSA_ALG_SHA_256):"9ac4335b469bbd791439248504dd0d49c71349a295fee5a1c68507f45a9e1c7b"
+interruptible_signverify_hash_ops_tests:PSA_KEY_TYPE_ECC_KEY_PAIR(PSA_ECC_FAMILY_SECP_R1):"ab45435712649cb30bbddac49197eebf2740ffc7f874d9244c3460f54f322d3a":PSA_ALG_ECDSA(PSA_ALG_SHA_256):"9ac4335b469bbd791439248504dd0d49c71349a295fee5a1c68507f45a9e1c7b"
PSA sign message: RSA PKCS#1 v1.5 SHA-256
depends_on:PSA_WANT_ALG_RSA_PKCS1V15_SIGN:PSA_WANT_ALG_SHA_256:PSA_WANT_KEY_TYPE_RSA_KEY_PAIR:MBEDTLS_PK_PARSE_C
diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function
index ab39fba..182443a 100644
--- a/tests/suites/test_suite_psa_crypto.function
+++ b/tests/suites/test_suite_psa_crypto.function
@@ -6662,6 +6662,12 @@
* 3. Test that the number of ops done prior to start and after abort is zero
* and that each successful stage completes some ops (this is not mandated by
* the PSA specification, but is currently the case).
+ *
+ * 4. Check that calling complete() when start() fails and complete()
+ * after completion results in a BAD_STATE error.
+ *
+ * 5. Check that calling start() again after start fails results in a BAD_STATE
+ * error.
*/
void sign_hash_fail_interruptible(int key_type_arg, data_t *key_data,
int alg_arg, data_t *input_data,
@@ -6718,6 +6724,15 @@
TEST_EQUAL(actual_status, expected_start_status);
if (expected_start_status != PSA_SUCCESS) {
+ /* Emulate poor application code, and call complete anyway, even though
+ * start failed. */
+ actual_status = psa_sign_hash_complete(&operation, signature,
+ signature_size,
+ &signature_length);
+
+ TEST_EQUAL(actual_status, PSA_ERROR_BAD_STATE);
+
+ /* Test that calling start again after failure also causes BAD_STATE. */
actual_status = psa_sign_hash_start(&operation, key, alg,
input_data->x, input_data->len);
@@ -6864,6 +6879,10 @@
*
* 3. Test the number of calls to psa_sign_hash_complete() required are as
* expected for different max_ops values.
+ *
+ * 4. Test that the number of ops done prior to starting signing and after abort
+ * is zero and that each successful signing stage completes some ops (this is
+ * not mandated by the PSA specification, but is currently the case).
*/
void sign_verify_hash_interruptible(int key_type_arg, data_t *key_data,
int alg_arg, data_t *input_data,
@@ -6879,6 +6898,8 @@
psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT;
psa_status_t status = PSA_OPERATION_INCOMPLETE;
uint32_t max_ops = max_ops_arg;
+ uint32_t num_ops = 0;
+ uint32_t num_ops_prior = 0;
size_t num_completes = 0;
size_t min_completes = 0;
size_t max_completes = 0;
@@ -6913,10 +6934,16 @@
interruptible_signverify_get_minmax_completes(max_ops, PSA_SUCCESS,
&min_completes, &max_completes);
+ num_ops_prior = psa_sign_hash_get_num_ops(&sign_operation);
+ TEST_ASSERT(num_ops_prior == 0);
+
/* Start performing the signature. */
PSA_ASSERT(psa_sign_hash_start(&sign_operation, key, alg,
input_data->x, input_data->len));
+ num_ops_prior = psa_sign_hash_get_num_ops(&sign_operation);
+ TEST_ASSERT(num_ops_prior == 0);
+
/* Continue performing the signature until complete. */
do {
@@ -6925,6 +6952,17 @@
&signature_length);
num_completes++;
+
+ if (status == PSA_SUCCESS || status == PSA_OPERATION_INCOMPLETE) {
+ num_ops = psa_sign_hash_get_num_ops(&sign_operation);
+ /* We are asserting here that every complete makes progress
+ * (completes some ops), which is true of the internal
+ * implementation and probably any implementation, however this is
+ * not mandated by the PSA specification. */
+ TEST_ASSERT(num_ops > num_ops_prior);
+
+ num_ops_prior = num_ops;
+ }
} while (status == PSA_OPERATION_INCOMPLETE);
TEST_ASSERT(status == PSA_SUCCESS);
@@ -6934,6 +6972,9 @@
PSA_ASSERT(psa_sign_hash_abort(&sign_operation));
+ num_ops = psa_sign_hash_get_num_ops(&sign_operation);
+ TEST_ASSERT(num_ops == 0);
+
/* Check that the signature length looks sensible. */
TEST_LE_U(signature_length, signature_size);
TEST_ASSERT(signature_length > 0);
@@ -7042,6 +7083,12 @@
* 3. Test that the number of ops done prior to start and after abort is zero
* and that each successful stage completes some ops (this is not mandated by
* the PSA specification, but is currently the case).
+ *
+ * 4. Test that calling psa_sign_hash_get_num_ops() multiple times between
+ * complete() calls does not alter the number of ops returned.
+ *
+ * 5. Test that after corrupting the hash, the verification detects an invalid
+ * signature.
*/
void verify_hash_interruptible(int key_type_arg, data_t *key_data,
int alg_arg, data_t *hash_data,
@@ -7126,6 +7173,25 @@
num_ops = psa_verify_hash_get_num_ops(&operation);
TEST_ASSERT(num_ops == 0);
+ if (hash_data->len != 0) {
+ /* Flip a bit in the hash and verify that the signature is now detected
+ * as invalid. Flip a bit at the beginning, not at the end, because
+ * ECDSA may ignore the last few bits of the input. */
+ hash_data->x[0] ^= 1;
+
+ /* Start verification. */
+ PSA_ASSERT(psa_verify_hash_start(&operation, key, alg,
+ hash_data->x, hash_data->len,
+ signature_data->x, signature_data->len));
+
+ /* Continue performing the signature until complete. */
+ do {
+ status = psa_verify_hash_complete(&operation);
+ } while (status == PSA_OPERATION_INCOMPLETE);
+
+ TEST_ASSERT(status == PSA_ERROR_INVALID_SIGNATURE);
+ }
+
exit:
psa_reset_key_attributes(&attributes);
psa_destroy_key(key);
@@ -7183,6 +7249,12 @@
* 3. Test that the number of ops done prior to start and after abort is zero
* and that each successful stage completes some ops (this is not mandated by
* the PSA specification, but is currently the case).
+ *
+ * 4. Check that calling complete() when start() fails and complete()
+ * after completion results in a BAD_STATE error.
+ *
+ * 5. Check that calling start() again after start fails results in a BAD_STATE
+ * error.
*/
void verify_hash_fail_interruptible(int key_type_arg, data_t *key_data,
int alg_arg, data_t *hash_data,
@@ -7235,6 +7307,13 @@
TEST_EQUAL(actual_status, expected_start_status);
if (expected_start_status != PSA_SUCCESS) {
+ /* Emulate poor application code, and call complete anyway, even though
+ * start failed. */
+ actual_status = psa_verify_hash_complete(&operation);
+
+ TEST_EQUAL(actual_status, PSA_ERROR_BAD_STATE);
+
+ /* Test that calling start again after failure also causes BAD_STATE. */
actual_status = psa_verify_hash_start(&operation, key, alg,
hash_data->x, hash_data->len,
signature_data->x,
@@ -7487,41 +7566,6 @@
TEST_LE_U(signature_size, PSA_SIGNATURE_MAX_SIZE);
ASSERT_ALLOC(signature, signature_size);
- /* --- Ensure changing the max ops mid operation works (operation should
- * complete successfully after setting max ops to unlimited --- */
- psa_interruptible_set_max_ops(1);
-
- PSA_ASSERT(psa_sign_hash_start(&sign_operation, key, alg,
- input_data->x, input_data->len));
-
- TEST_EQUAL(psa_sign_hash_complete(&sign_operation, signature,
- signature_size,
- &signature_length),
- PSA_OPERATION_INCOMPLETE);
-
- psa_interruptible_set_max_ops(PSA_INTERRUPTIBLE_MAX_OPS_UNLIMITED);
-
- PSA_ASSERT(psa_sign_hash_complete(&sign_operation, signature,
- signature_size,
- &signature_length));
-
- PSA_ASSERT(psa_sign_hash_abort(&sign_operation));
-
- psa_interruptible_set_max_ops(1);
-
- PSA_ASSERT(psa_verify_hash_start(&verify_operation, key, alg,
- input_data->x, input_data->len,
- signature, signature_length));
-
- TEST_EQUAL(psa_verify_hash_complete(&verify_operation),
- PSA_OPERATION_INCOMPLETE);
-
- psa_interruptible_set_max_ops(PSA_INTERRUPTIBLE_MAX_OPS_UNLIMITED);
-
- PSA_ASSERT(psa_verify_hash_complete(&verify_operation));
-
- PSA_ASSERT(psa_verify_hash_abort(&verify_operation));
-
/* --- Change function inputs mid run, to cause an error (sign only,
* verify passes all inputs to start. --- */
@@ -7602,15 +7646,21 @@
/* BEGIN_CASE depends_on:MBEDTLS_ECP_RESTARTABLE */
/**
- * interruptible_signverify_hash_maxops_tests() test intentions:
+ * interruptible_signverify_hash_ops_tests() test intentions:
*
* Note: This test can currently only handle ECDSA.
*
* 1. Test that setting max ops is reflected in both interruptible sign and
* verify hash
+ * 2. Test that changing the value of max_ops to unlimited during an operation
+ * causes that operation to complete in the next call.
+ *
+ * 3. Test that calling get_num_ops() between complete calls gives the same
+ * result as calling get_num_ops() once at the end of the operation.
*/
-void interruptible_signverify_hash_maxops_tests(int key_type_arg,
- data_t *key_data, int alg_arg, data_t *input_data)
+void interruptible_signverify_hash_ops_tests(int key_type_arg,
+ data_t *key_data, int alg_arg,
+ data_t *input_data)
{
mbedtls_svc_key_id_t key = MBEDTLS_SVC_KEY_ID_INIT;
psa_key_type_t key_type = key_type_arg;
@@ -7619,6 +7669,10 @@
size_t key_bits;
unsigned char *signature = NULL;
size_t signature_size;
+ size_t signature_length = 0xdeadbeef;
+ uint32_t num_ops = 0;
+ psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED;
+
psa_sign_hash_interruptible_operation_t sign_operation =
psa_sign_hash_interruptible_operation_init();
psa_verify_hash_interruptible_operation_t verify_operation =
@@ -7667,6 +7721,111 @@
TEST_EQUAL(psa_interruptible_get_max_ops(), 0xbeef);
+ /* --- Ensure changing the max ops mid operation works (operation should
+ * complete successfully after setting max ops to unlimited --- */
+ psa_interruptible_set_max_ops(1);
+
+ PSA_ASSERT(psa_sign_hash_start(&sign_operation, key, alg,
+ input_data->x, input_data->len));
+
+ TEST_EQUAL(psa_sign_hash_complete(&sign_operation, signature,
+ signature_size,
+ &signature_length),
+ PSA_OPERATION_INCOMPLETE);
+
+ psa_interruptible_set_max_ops(PSA_INTERRUPTIBLE_MAX_OPS_UNLIMITED);
+
+ PSA_ASSERT(psa_sign_hash_complete(&sign_operation, signature,
+ signature_size,
+ &signature_length));
+
+ PSA_ASSERT(psa_sign_hash_abort(&sign_operation));
+
+ psa_interruptible_set_max_ops(1);
+
+ PSA_ASSERT(psa_verify_hash_start(&verify_operation, key, alg,
+ input_data->x, input_data->len,
+ signature, signature_length));
+
+ TEST_EQUAL(psa_verify_hash_complete(&verify_operation),
+ PSA_OPERATION_INCOMPLETE);
+
+ psa_interruptible_set_max_ops(PSA_INTERRUPTIBLE_MAX_OPS_UNLIMITED);
+
+ PSA_ASSERT(psa_verify_hash_complete(&verify_operation));
+
+ PSA_ASSERT(psa_verify_hash_abort(&verify_operation));
+
+ /* --- Test that not calling get_num_ops inbetween complete calls does not
+ * result in lost ops. ---*/
+
+ psa_interruptible_set_max_ops(1);
+
+ PSA_ASSERT(psa_sign_hash_start(&sign_operation, key, alg,
+ input_data->x, input_data->len));
+
+ /* Continue performing the signature until complete. */
+ do {
+ status = psa_sign_hash_complete(&sign_operation, signature,
+ signature_size,
+ &signature_length);
+
+ num_ops = psa_sign_hash_get_num_ops(&sign_operation);
+
+ } while (status == PSA_OPERATION_INCOMPLETE);
+
+ PSA_ASSERT(status);
+
+ PSA_ASSERT(psa_sign_hash_abort(&sign_operation));
+
+ PSA_ASSERT(psa_sign_hash_start(&sign_operation, key, alg,
+ input_data->x, input_data->len));
+
+ /* Continue performing the signature until complete. */
+ do {
+ status = psa_sign_hash_complete(&sign_operation, signature,
+ signature_size,
+ &signature_length);
+ } while (status == PSA_OPERATION_INCOMPLETE);
+
+ PSA_ASSERT(status);
+
+ TEST_EQUAL(num_ops, psa_sign_hash_get_num_ops(&sign_operation));
+
+ PSA_ASSERT(psa_sign_hash_abort(&sign_operation));
+
+ PSA_ASSERT(psa_verify_hash_start(&verify_operation, key, alg,
+ input_data->x, input_data->len,
+ signature, signature_length));
+
+ /* Continue performing the verification until complete. */
+ do {
+ status = psa_verify_hash_complete(&verify_operation);
+
+ num_ops = psa_verify_hash_get_num_ops(&verify_operation);
+
+ } while (status == PSA_OPERATION_INCOMPLETE);
+
+ PSA_ASSERT(status);
+
+ PSA_ASSERT(psa_verify_hash_abort(&verify_operation));
+
+ PSA_ASSERT(psa_verify_hash_start(&verify_operation, key, alg,
+ input_data->x, input_data->len,
+ signature, signature_length));
+
+ /* Continue performing the verification until complete. */
+ do {
+ status = psa_verify_hash_complete(&verify_operation);
+
+ } while (status == PSA_OPERATION_INCOMPLETE);
+
+ PSA_ASSERT(status);
+
+ TEST_EQUAL(num_ops, psa_verify_hash_get_num_ops(&verify_operation));
+
+ PSA_ASSERT(psa_verify_hash_abort(&verify_operation));
+
exit:
/*
* Key attributes may have been returned by psa_get_key_attributes()