diff --git a/include/psa/crypto.h b/include/psa/crypto.h index fd325b3e9..71bad3b7a 100644 --- a/include/psa/crypto.h +++ b/include/psa/crypto.h @@ -2004,6 +2004,14 @@ psa_status_t psa_cipher_update(psa_cipher_operation_t *operation, * * \retval #PSA_SUCCESS * Success. + * \retval #PSA_ERROR_INVALID_ARGUMENT + * The total input size passed to this operation is not valid for + * this particular algorithm. For example, the algorithm is a based + * on block cipher and requires a whole number of blocks, but the + * total input size is not a multiple of the block size. + * \retval #PSA_ERROR_INVALID_PADDING + * This is a decryption operation for an algorithm that includes + * padding, and the ciphertext does not contain valid padding. * \retval #PSA_ERROR_BAD_STATE * The operation state is not valid (not set up, IV required but * not set, or already completed). diff --git a/include/psa/crypto_values.h b/include/psa/crypto_values.h index b53e1c769..fc0f9637f 100644 --- a/include/psa/crypto_values.h +++ b/include/psa/crypto_values.h @@ -149,7 +149,7 @@ * * \warning If a function returns this error, it is undetermined * whether the requested action has completed or not. Implementations - * should return #PSA_SUCCESS on successful completion whenver + * should return #PSA_SUCCESS on successful completion whenever * possible, however functions may return #PSA_ERROR_COMMUNICATION_FAILURE * if the requested action was completed successfully in an external * cryptoprocessor but there was a breakdown of communication before diff --git a/tests/.gitignore b/tests/.gitignore index 3c9b0cf25..fbbd0dfe2 100644 --- a/tests/.gitignore +++ b/tests/.gitignore @@ -7,3 +7,5 @@ data_files/mpi_write data_files/hmac_drbg_seed data_files/ctr_drbg_seed data_files/entropy_seed + +/instrument_record_status.h diff --git a/tests/Makefile b/tests/Makefile index 4eb914231..f7505b602 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -28,6 +28,10 @@ ifdef DEBUG LOCAL_CFLAGS += -g3 endif +ifdef RECORD_PSA_STATUS_COVERAGE_LOG +LOCAL_CFLAGS += -Werror -DRECORD_PSA_STATUS_COVERAGE_LOG +endif + # if we're running on Windows, build for Windows ifdef WINDOWS WINDOWS_BUILD=1 @@ -163,3 +167,9 @@ endif endef $(foreach app, $(APPS), $(foreach file, $(wildcard *.h), \ $(eval $(call copy_header_to_target,$(app),$(file))))) + +ifdef RECORD_PSA_STATUS_COVERAGE_LOG +$(BINARIES): instrument_record_status.h +instrument_record_status.h: ../include/psa/crypto.h Makefile + sed <../include/psa/crypto.h >$@ -n 's/^psa_status_t \([A-Za-z0-9_]*\)(.*/#define \1(...) RECORD_STATUS("\1", \1(__VA_ARGS__))/p' +endif diff --git a/tests/psa_crypto_helpers.h b/tests/psa_crypto_helpers.h index 3780d161a..19303de57 100644 --- a/tests/psa_crypto_helpers.h +++ b/tests/psa_crypto_helpers.h @@ -72,4 +72,59 @@ static void test_helper_psa_done( int line, const char *file ) */ #define PSA_DONE( ) test_helper_psa_done( __LINE__, __FILE__ ) + + +#if defined(RECORD_PSA_STATUS_COVERAGE_LOG) +#include + +/** Name of the file where return statuses are logged by #RECORD_STATUS. */ +#define STATUS_LOG_FILE_NAME "statuses.log" + +static psa_status_t record_status( psa_status_t status, + const char *func, + const char *file, int line, + const char *expr ) +{ + /* We open the log file on first use. + * We never close the log file, so the record_status feature is not + * compatible with resource leak detectors such as Asan. + */ + static FILE *log; + if( log == NULL ) + log = fopen( STATUS_LOG_FILE_NAME, "a" ); + fprintf( log, "%d:%s:%s:%d:%s\n", (int) status, func, file, line, expr ); + return( status ); +} + +/** Return value logging wrapper macro. + * + * Evaluate \p expr. Write a line recording its value to the log file + * #STATUS_LOG_FILE_NAME and return the value. The line is a colon-separated + * list of fields: + * ``` + * value of expr:string:__FILE__:__LINE__:expr + * ``` + * + * The test code does not call this macro explicitly because that would + * be very invasive. Instead, we instrument the source code by defining + * a bunch of wrapper macros like + * ``` + * #define psa_crypto_init() RECORD_STATUS("psa_crypto_init", psa_crypto_init()) + * ``` + * These macro definitions must be present in `instrument_record_status.h` + * when building the test suites. + * + * \param string A string, normally a function name. + * \param expr An expression to evaluate, normally a call of the function + * whose name is in \p string. This expression must return + * a value of type #psa_status_t. + * \return The value of \p expr. + */ +#define RECORD_STATUS( string, expr ) \ + record_status( ( expr ), string, __FILE__, __LINE__, #expr ) + +#include "instrument_record_status.h" + +#endif /* defined(RECORD_PSA_STATUS_COVERAGE_LOG) */ + #endif /* PSA_CRYPTO_HELPERS_H */ diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 20458af2c..e3a8c0e31 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -629,6 +629,16 @@ component_test_everest () { make test } +component_test_psa_collect_statuses () { + msg "build+test: psa_collect_statuses" # ~30s + scripts/config.pl full + scripts/config.pl unset MBEDTLS_MEMORY_BUFFER_ALLOC_C # slow and irrelevant + record_status tests/scripts/psa_collect_statuses.py + # Check that psa_crypto_init() succeeded at least once + record_status grep -q '^0:psa_crypto_init:' tests/statuses.log + rm -f tests/statuses.log +} + component_test_full_cmake_clang () { msg "build: cmake, full config, clang" # ~ 50s scripts/config.pl full diff --git a/tests/scripts/psa_collect_statuses.py b/tests/scripts/psa_collect_statuses.py new file mode 100755 index 000000000..e38beeac3 --- /dev/null +++ b/tests/scripts/psa_collect_statuses.py @@ -0,0 +1,125 @@ +#!/usr/bin/env python3 +"""Describe the test coverage of PSA functions in terms of return statuses. + +1. Build Mbed Crypto with -DRECORD_PSA_STATUS_COVERAGE_LOG +2. Run psa_collect_statuses.py + +The output is a series of line of the form "psa_foo PSA_ERROR_XXX". Each +function/status combination appears only once. + +This script must be run from the top of an Mbed Crypto source tree. +The build command is "make -DRECORD_PSA_STATUS_COVERAGE_LOG", which is +only supported with make (as opposed to CMake or other build methods). +""" + +import argparse +import os +import subprocess +import sys + +DEFAULT_STATUS_LOG_FILE = 'tests/statuses.log' +DEFAULT_PSA_CONSTANT_NAMES = 'programs/psa/psa_constant_names' + +class Statuses: + """Information about observed return statues of API functions.""" + + def __init__(self): + self.functions = {} + self.codes = set() + self.status_names = {} + + def collect_log(self, log_file_name): + """Read logs from RECORD_PSA_STATUS_COVERAGE_LOG. + + Read logs produced by running Mbed Crypto test suites built with + -DRECORD_PSA_STATUS_COVERAGE_LOG. + """ + with open(log_file_name) as log: + for line in log: + value, function, tail = line.split(':', 2) + if function not in self.functions: + self.functions[function] = {} + fdata = self.functions[function] + if value not in self.functions[function]: + fdata[value] = [] + fdata[value].append(tail) + self.codes.add(int(value)) + + def get_constant_names(self, psa_constant_names): + """Run psa_constant_names to obtain names for observed numerical values.""" + values = [str(value) for value in self.codes] + cmd = [psa_constant_names, 'status'] + values + output = subprocess.check_output(cmd).decode('ascii') + for value, name in zip(values, output.rstrip().split('\n')): + self.status_names[value] = name + + def report(self): + """Report observed return values for each function. + + The report is a series of line of the form "psa_foo PSA_ERROR_XXX". + """ + for function in sorted(self.functions.keys()): + fdata = self.functions[function] + names = [self.status_names[value] for value in fdata.keys()] + for name in sorted(names): + sys.stdout.write('{} {}\n'.format(function, name)) + +def collect_status_logs(options): + """Build and run unit tests and report observed function return statuses. + + Build Mbed Crypto with -DRECORD_PSA_STATUS_COVERAGE_LOG, run the + test suites and display information about observed return statuses. + """ + rebuilt = False + if not options.use_existing_log and os.path.exists(options.log_file): + os.remove(options.log_file) + if not os.path.exists(options.log_file): + if options.clean_before: + subprocess.check_call(['make', 'clean'], + cwd='tests', + stdout=sys.stderr) + with open(os.devnull, 'w') as devnull: + make_q_ret = subprocess.call(['make', '-q', 'lib', 'tests'], + stdout=devnull, stderr=devnull) + if make_q_ret != 0: + subprocess.check_call(['make', 'RECORD_PSA_STATUS_COVERAGE_LOG=1'], + stdout=sys.stderr) + rebuilt = True + subprocess.check_call(['make', 'test'], + stdout=sys.stderr) + data = Statuses() + data.collect_log(options.log_file) + data.get_constant_names(options.psa_constant_names) + if rebuilt and options.clean_after: + subprocess.check_call(['make', 'clean'], + cwd='tests', + stdout=sys.stderr) + return data + +def main(): + parser = argparse.ArgumentParser(description=globals()['__doc__']) + parser.add_argument('--clean-after', + action='store_true', + help='Run "make clean" after rebuilding') + parser.add_argument('--clean-before', + action='store_true', + help='Run "make clean" before regenerating the log file)') + parser.add_argument('--log-file', metavar='FILE', + default=DEFAULT_STATUS_LOG_FILE, + help='Log file location (default: {})'.format( + DEFAULT_STATUS_LOG_FILE + )) + parser.add_argument('--psa-constant-names', metavar='PROGRAM', + default=DEFAULT_PSA_CONSTANT_NAMES, + help='Path to psa_constant_names (default: {})'.format( + DEFAULT_PSA_CONSTANT_NAMES + )) + parser.add_argument('--use-existing-log', '-e', + action='store_true', + help='Don\'t regenerate the log file if it exists') + options = parser.parse_args() + data = collect_status_logs(options) + data.report() + +if __name__ == '__main__': + main()