Merge pull request #117 from gilles-peskine-arm/psa_error_code_coverage

PSA return status coverage script

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).

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

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

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

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 <psa/crypto.h>
+
+/** 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 */

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

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()