From 7c2dd5890f945006d838089220c92e66cc1fba66 Mon Sep 17 00:00:00 2001 From: Darryl Green Date: Thu, 1 Mar 2018 14:53:49 +0000 Subject: [PATCH 1/6] Add script for ABI compatibility checking --- scripts/abi_check.py | 233 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 233 insertions(+) create mode 100755 scripts/abi_check.py diff --git a/scripts/abi_check.py b/scripts/abi_check.py new file mode 100755 index 000000000..0f063a3f3 --- /dev/null +++ b/scripts/abi_check.py @@ -0,0 +1,233 @@ +#!/usr/bin/env python3 + +# This script is a small wrapper around the abi-compliance-checker and +# abi-dumper tools, applying them to compare the ABI and API of the library +# files from two different Git revisions within an Mbed TLS repository. +# The results of the comparison are formatted as HTML and stored at +# a configurable location. Returns 0 on success, 1 on ABI/API non-compliance, +# and 2 if there is an error while running the script. +# Note: must be run from Mbed TLS root. + +import os +import sys +import traceback +import shutil +import subprocess +import argparse +import logging +import tempfile + + +class AbiChecker(object): + + def __init__(self, report_dir, old_rev, new_rev, keep_all_reports): + self.repo_path = "." + self.log = None + self.setup_logger() + self.report_dir = os.path.abspath(report_dir) + self.keep_all_reports = keep_all_reports + self.should_keep_report_dir = os.path.isdir(self.report_dir) + self.old_rev = old_rev + self.new_rev = new_rev + self.mbedtls_modules = ["libmbedcrypto", "libmbedtls", "libmbedx509"] + self.old_dumps = {} + self.new_dumps = {} + self.git_command = "git" + self.make_command = "make" + + def check_repo_path(self): + if not __file__ == os.path.join(".", "scripts", "abi_check.py"): + raise Exception("Must be run from Mbed TLS root") + + def setup_logger(self): + self.log = logging.getLogger() + self.log.setLevel(logging.INFO) + self.log.addHandler(logging.StreamHandler()) + + def check_abi_tools_are_installed(self): + for command in ["abi-dumper", "abi-compliance-checker"]: + if not shutil.which(command): + raise Exception("{} not installed, aborting".format(command)) + + def get_clean_worktree_for_git_revision(self, git_rev): + self.log.info( + "Checking out git worktree for revision {}".format(git_rev) + ) + git_worktree_path = tempfile.mkdtemp() + worktree_process = subprocess.Popen( + [self.git_command, "worktree", "add", git_worktree_path, git_rev], + cwd=self.repo_path, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT + ) + worktree_output, _ = worktree_process.communicate() + self.log.info(worktree_output.decode("utf-8")) + if worktree_process.returncode != 0: + raise Exception("Checking out worktree failed, aborting") + return git_worktree_path + + def build_shared_libraries(self, git_worktree_path): + my_environment = os.environ.copy() + my_environment["CFLAGS"] = "-g -Og" + my_environment["SHARED"] = "1" + make_process = subprocess.Popen( + self.make_command, + env=my_environment, + cwd=git_worktree_path, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT + ) + make_output, _ = make_process.communicate() + self.log.info(make_output.decode("utf-8")) + if make_process.returncode != 0: + raise Exception("make failed, aborting") + + def get_abi_dumps_from_shared_libraries(self, git_ref, git_worktree_path): + abi_dumps = {} + for mbed_module in self.mbedtls_modules: + output_path = os.path.join( + self.report_dir, "{}-{}.dump".format(mbed_module, git_ref) + ) + abi_dump_command = [ + "abi-dumper", + os.path.join( + git_worktree_path, "library", mbed_module + ".so"), + "-o", output_path, + "-lver", git_ref + ] + abi_dump_process = subprocess.Popen( + abi_dump_command, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT + ) + abi_dump_output, _ = abi_dump_process.communicate() + self.log.info(abi_dump_output.decode("utf-8")) + if abi_dump_process.returncode != 0: + raise Exception("abi-dumper failed, aborting") + abi_dumps[mbed_module] = output_path + return abi_dumps + + def cleanup_worktree(self, git_worktree_path): + shutil.rmtree(git_worktree_path) + worktree_process = subprocess.Popen( + [self.git_command, "worktree", "prune"], + cwd=self.repo_path, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT + ) + worktree_output, _ = worktree_process.communicate() + self.log.info(worktree_output.decode("utf-8")) + if worktree_process.returncode != 0: + raise Exception("Worktree cleanup failed, aborting") + + def get_abi_dump_for_ref(self, git_rev): + git_worktree_path = self.get_clean_worktree_for_git_revision(git_rev) + self.build_shared_libraries(git_worktree_path) + abi_dumps = self.get_abi_dumps_from_shared_libraries( + git_rev, git_worktree_path + ) + self.cleanup_worktree(git_worktree_path) + return abi_dumps + + def get_abi_compatibility_report(self): + compatibility_report = "" + compliance_return_code = 0 + for mbed_module in self.mbedtls_modules: + output_path = os.path.join( + self.report_dir, "{}-{}-{}.html".format( + mbed_module, self.old_rev, self.new_rev + ) + ) + abi_compliance_command = [ + "abi-compliance-checker", + "-l", mbed_module, + "-old", self.old_dumps[mbed_module], + "-new", self.new_dumps[mbed_module], + "-strict", + "-report-path", output_path + ] + abi_compliance_process = subprocess.Popen( + abi_compliance_command, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT + ) + abi_compliance_output, _ = abi_compliance_process.communicate() + self.log.info(abi_compliance_output.decode("utf-8")) + if abi_compliance_process.returncode == 0: + compatibility_report += ( + "No compatibility issues for {}\n".format(mbed_module) + ) + if not self.keep_all_reports: + os.remove(output_path) + elif abi_compliance_process.returncode == 1: + compliance_return_code = 1 + self.should_keep_report_dir = True + compatibility_report += ( + "Compatibility issues found for {}, " + "for details see {}\n".format(mbed_module, output_path) + ) + else: + raise Exception( + "abi-compliance-checker failed with a return code of {}," + " aborting".format(abi_compliance_process.returncode) + ) + os.remove(self.old_dumps[mbed_module]) + os.remove(self.new_dumps[mbed_module]) + if not self.should_keep_report_dir and not self.keep_all_reports: + os.rmdir(self.report_dir) + self.log.info(compatibility_report) + return compliance_return_code + + def check_for_abi_changes(self): + self.check_repo_path() + self.check_abi_tools_are_installed() + self.old_dumps = self.get_abi_dump_for_ref(self.old_rev) + self.new_dumps = self.get_abi_dump_for_ref(self.new_rev) + return self.get_abi_compatibility_report() + + +def run_main(): + try: + parser = argparse.ArgumentParser( + description=( + "This script is a small wrapper around the " + "abi-compliance-checker and abi-dumper tools, applying them " + "to compare the ABI and API of the library files from two " + "different Git revisions within an Mbed TLS repository." + " The results of the comparison are formatted as HTML and" + " stored at a configurable location. Returns 0 on success, " + "1 on ABI/API non-compliance, and 2 if there is an error " + "while running the script. # Note: must be run from " + "Mbed TLS root." + ) + ) + parser.add_argument( + "-r", "--report_dir", type=str, default="reports", + help="directory where reports are stored, default is reports", + ) + parser.add_argument( + "-k", "--keep_all_reports", action="store_true", + help="keep all reports, even if there are no compatibility issues", + ) + parser.add_argument( + "-o", "--old_rev", type=str, help="revision for old version", + required=True + ) + parser.add_argument( + "-n", "--new_rev", type=str, help="revision for new version", + required=True + ) + abi_args = parser.parse_args() + abi_check = AbiChecker( + abi_args.report_dir, abi_args.old_rev, + abi_args.new_rev, abi_args.keep_all_reports + ) + return_code = abi_check.check_for_abi_changes() + sys.exit(return_code) + except Exception as error: + traceback.print_exc(error) + sys.exit(2) + + +if __name__ == "__main__": + run_main() From 127c5affce7b419afc9eb9f4a5c37ecdf3498e67 Mon Sep 17 00:00:00 2001 From: Darryl Green Date: Mon, 12 Mar 2018 15:44:31 +0000 Subject: [PATCH 2/6] Add copyright to abi_check script --- scripts/abi_check.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/scripts/abi_check.py b/scripts/abi_check.py index 0f063a3f3..f9fb7f65d 100755 --- a/scripts/abi_check.py +++ b/scripts/abi_check.py @@ -1,5 +1,11 @@ #!/usr/bin/env python3 - +# +# This file is part of mbed TLS (https://tls.mbed.org) +# +# Copyright (c) 2018, Arm Limited, All Rights Reserved +# +# Purpose +# # This script is a small wrapper around the abi-compliance-checker and # abi-dumper tools, applying them to compare the ABI and API of the library # files from two different Git revisions within an Mbed TLS repository. From a6f430f5778c606b15e16dc5843d5519c78a3ae3 Mon Sep 17 00:00:00 2001 From: Darryl Green Date: Thu, 15 Mar 2018 10:12:06 +0000 Subject: [PATCH 3/6] Fix current directory check --- scripts/abi_check.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/scripts/abi_check.py b/scripts/abi_check.py index f9fb7f65d..98d8be422 100755 --- a/scripts/abi_check.py +++ b/scripts/abi_check.py @@ -1,6 +1,6 @@ #!/usr/bin/env python3 # -# This file is part of mbed TLS (https://tls.mbed.org) +# This file is part of Mbed TLS (https://tls.mbed.org) # # Copyright (c) 2018, Arm Limited, All Rights Reserved # @@ -42,7 +42,9 @@ class AbiChecker(object): self.make_command = "make" def check_repo_path(self): - if not __file__ == os.path.join(".", "scripts", "abi_check.py"): + current_dir = os.path.realpath('.') + root_dir = os.path.dirname(os.path.dirname(os.path.realpath(__file__))) + if current_dir != root_dir: raise Exception("Must be run from Mbed TLS root") def setup_logger(self): @@ -230,8 +232,8 @@ def run_main(): ) return_code = abi_check.check_for_abi_changes() sys.exit(return_code) - except Exception as error: - traceback.print_exc(error) + except Exception: + traceback.print_exc() sys.exit(2) From 7869680e41e09e2aa1d24529099b86e08acfe1e3 Mon Sep 17 00:00:00 2001 From: Darryl Green Date: Fri, 6 Apr 2018 11:23:22 +0100 Subject: [PATCH 4/6] Updated abi_check.py docstrings --- scripts/abi_check.py | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/scripts/abi_check.py b/scripts/abi_check.py index 98d8be422..14250d2b9 100755 --- a/scripts/abi_check.py +++ b/scripts/abi_check.py @@ -1,18 +1,19 @@ #!/usr/bin/env python3 -# -# This file is part of Mbed TLS (https://tls.mbed.org) -# -# Copyright (c) 2018, Arm Limited, All Rights Reserved -# -# Purpose -# -# This script is a small wrapper around the abi-compliance-checker and -# abi-dumper tools, applying them to compare the ABI and API of the library -# files from two different Git revisions within an Mbed TLS repository. -# The results of the comparison are formatted as HTML and stored at -# a configurable location. Returns 0 on success, 1 on ABI/API non-compliance, -# and 2 if there is an error while running the script. -# Note: must be run from Mbed TLS root. +""" +This file is part of Mbed TLS (https://tls.mbed.org) + +Copyright (c) 2018, Arm Limited, All Rights Reserved + +Purpose + +This script is a small wrapper around the abi-compliance-checker and +abi-dumper tools, applying them to compare the ABI and API of the library +files from two different Git revisions within an Mbed TLS repository. +The results of the comparison are formatted as HTML and stored at +a configurable location. Returns 0 on success, 1 on ABI/API non-compliance, +and 2 if there is an error while running the script. +Note: requires Python 3, must be run from Mbed TLS root. +""" import os import sys @@ -205,8 +206,8 @@ def run_main(): " The results of the comparison are formatted as HTML and" " stored at a configurable location. Returns 0 on success, " "1 on ABI/API non-compliance, and 2 if there is an error " - "while running the script. # Note: must be run from " - "Mbed TLS root." + "while running the script. Note: requires Python 3, " + "must be run from Mbed TLS root." ) ) parser.add_argument( From 418527b041e2c147ed604221d7b58d1143e953ff Mon Sep 17 00:00:00 2001 From: Darryl Green Date: Mon, 16 Apr 2018 12:02:29 +0100 Subject: [PATCH 5/6] Fix minor issues with command line options --- scripts/abi_check.py | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/scripts/abi_check.py b/scripts/abi_check.py index 14250d2b9..8f9cd0f43 100755 --- a/scripts/abi_check.py +++ b/scripts/abi_check.py @@ -12,7 +12,7 @@ files from two different Git revisions within an Mbed TLS repository. The results of the comparison are formatted as HTML and stored at a configurable location. Returns 0 on success, 1 on ABI/API non-compliance, and 2 if there is an error while running the script. -Note: requires Python 3, must be run from Mbed TLS root. +Note: must be run from Mbed TLS root. """ import os @@ -199,31 +199,30 @@ def run_main(): try: parser = argparse.ArgumentParser( description=( - "This script is a small wrapper around the " - "abi-compliance-checker and abi-dumper tools, applying them " - "to compare the ABI and API of the library files from two " - "different Git revisions within an Mbed TLS repository." - " The results of the comparison are formatted as HTML and" - " stored at a configurable location. Returns 0 on success, " - "1 on ABI/API non-compliance, and 2 if there is an error " - "while running the script. Note: requires Python 3, " - "must be run from Mbed TLS root." + """This script is a small wrapper around the + abi-compliance-checker and abi-dumper tools, applying them + to compare the ABI and API of the library files from two + different Git revisions within an Mbed TLS repository. + The results of the comparison are formatted as HTML and stored + at a configurable location. Returns 0 on success, 1 on ABI/API + non-compliance, and 2 if there is an error while running the + script. Note: must be run from Mbed TLS root.""" ) ) parser.add_argument( - "-r", "--report_dir", type=str, default="reports", + "-r", "--report-dir", type=str, default="reports", help="directory where reports are stored, default is reports", ) parser.add_argument( - "-k", "--keep_all_reports", action="store_true", + "-k", "--keep-all-reports", action="store_true", help="keep all reports, even if there are no compatibility issues", ) parser.add_argument( - "-o", "--old_rev", type=str, help="revision for old version", + "-o", "--old-rev", type=str, help="revision for old version", required=True ) parser.add_argument( - "-n", "--new_rev", type=str, help="revision for new version", + "-n", "--new-rev", type=str, help="revision for new version", required=True ) abi_args = parser.parse_args() From 1a925bc0aab288f9911f4f1dfd76672c8c40c1be Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Mon, 14 May 2018 13:58:22 +0100 Subject: [PATCH 6/6] Initial prototype and demonstrator for parameter validation Adds a new configurable option for the parameter validation level. --- include/mbedtls/aes.h | 11 +++++++++++ include/mbedtls/config.h | 19 +++++++++++++++++++ library/aes.c | 18 ++++++++++-------- tests/suites/test_suite_aes.function | 17 +++++++++++++++++ tests/suites/test_suite_aes.rest.data | 4 ++++ 5 files changed, 61 insertions(+), 8 deletions(-) diff --git a/include/mbedtls/aes.h b/include/mbedtls/aes.h index e0fc238d7..e7b95105f 100644 --- a/include/mbedtls/aes.h +++ b/include/mbedtls/aes.h @@ -56,6 +56,17 @@ /* Error codes in range 0x0023-0x0025 */ #define MBEDTLS_ERR_AES_FEATURE_UNAVAILABLE -0x0023 /**< Feature not available. For example, an unsupported AES key size. */ #define MBEDTLS_ERR_AES_HW_ACCEL_FAILED -0x0025 /**< AES hardware accelerator failed. */ +#define MBEDTLS_ERR_AES_BAD_INPUT_DATA -0x0027 /**< Invalid +input data. */ + +#if defined( MBEDTLS_CHECK_PARAMS ) +#define MBEDTLS_AES_VALIDATE( cond ) do{ if( !(cond) ) \ + return MBEDTLS_ERR_AES_BAD_INPUT_DATA; \ + } while(0); +#else +/* No validation of parameters will be performed */ +#define MBEDTLS_AES_VALIDATE( cond) +#endif #if ( defined(__ARMCC_VERSION) || defined(_MSC_VER) ) && \ !defined(inline) && !defined(__cplusplus) diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 7c9acb230..dff75ae02 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -221,6 +221,25 @@ */ //#define MBEDTLS_DEPRECATED_REMOVED +/** + * \def MBEDTLS_PARAM_VALIDATION_LEVEL + * + * The defined parameter validation level for the library. This configuration + * controls whether the library validates parameters passed to it. + * + * Application code that deals with 3rd party input may wish to enable such + * validation, whilst code on closed systems, such as embedded systems, where + * the input is controlled and predictable, may wish to disable it entirely to + * reduce the code size of the library. + * + * When the symbol is not defined, no parameter validation except that required + * to ensure the integrity or security of the library are performed. + * + * When the symbol is defined, all parameters will be validated, and an error + * code returned where appropriate. + */ +#define MBEDTLS_CHECK_PARAMS + /* \} name SECTION: System support */ /** diff --git a/library/aes.c b/library/aes.c index b0aea0091..dff424ba6 100644 --- a/library/aes.c +++ b/library/aes.c @@ -531,14 +531,7 @@ int mbedtls_aes_setkey_enc( mbedtls_aes_context *ctx, const unsigned char *key, unsigned int i; uint32_t *RK; -#if !defined(MBEDTLS_AES_ROM_TABLES) - if( aes_init_done == 0 ) - { - aes_gen_tables(); - aes_init_done = 1; - - } -#endif + MBEDTLS_AES_VALIDATE( ctx != NULL && key != NULL ); switch( keybits ) { @@ -548,6 +541,15 @@ int mbedtls_aes_setkey_enc( mbedtls_aes_context *ctx, const unsigned char *key, default : return( MBEDTLS_ERR_AES_INVALID_KEY_LENGTH ); } +#if !defined(MBEDTLS_AES_ROM_TABLES) + if( aes_init_done == 0 ) + { + aes_gen_tables(); + aes_init_done = 1; + + } +#endif + #if defined(MBEDTLS_PADLOCK_C) && defined(MBEDTLS_PADLOCK_ALIGN16) if( aes_padlock_ace == -1 ) aes_padlock_ace = mbedtls_padlock_has_support( MBEDTLS_PADLOCK_ACE ); diff --git a/tests/suites/test_suite_aes.function b/tests/suites/test_suite_aes.function index c5f0eaac9..513c14524 100644 --- a/tests/suites/test_suite_aes.function +++ b/tests/suites/test_suite_aes.function @@ -289,6 +289,23 @@ exit: } /* END_CASE */ +/* BEGIN_CASE */ +void aes_invalid_param( ) +{ + mbedtls_aes_context dummy_ctx; + const unsigned char key[] = { 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 }; + + /* mbedtls_aes_setkey_enc() */ + TEST_ASSERT( mbedtls_aes_setkey_enc( NULL, key, 128 ) + == MBEDTLS_ERR_AES_BAD_INPUT_DATA ); + TEST_ASSERT( mbedtls_aes_setkey_enc( &dummy_ctx, NULL, 128 ) + == MBEDTLS_ERR_AES_BAD_INPUT_DATA ); + +exit: + return; +} +/* END_CASE */ + /* BEGIN_CASE depends_on:MBEDTLS_SELF_TEST */ void aes_selftest() { diff --git a/tests/suites/test_suite_aes.rest.data b/tests/suites/test_suite_aes.rest.data index bbb222f10..3ec916ded 100644 --- a/tests/suites/test_suite_aes.rest.data +++ b/tests/suites/test_suite_aes.rest.data @@ -10,6 +10,10 @@ aes_encrypt_cbc:"000000000000000000000000000000000000000000000000000000000000000 AES-256-CBC Decrypt (Invalid input length) aes_decrypt_cbc:"0000000000000000000000000000000000000000000000000000000000000000":"00000000000000000000000000000000":"623a52fcea5d443e48d9181ab32c74":"":MBEDTLS_ERR_AES_INVALID_INPUT_LENGTH +AES - Invalid parameters +depends_on:MBEDTLS_CHECK_PARAMS +aes_invalid_param: + AES Selftest depends_on:MBEDTLS_SELF_TEST aes_selftest: