From e69bdcbd32989a426536257a2f8072b9135b5d2c Mon Sep 17 00:00:00 2001 From: xvzcf Date: Tue, 28 Apr 2020 15:05:00 -0400 Subject: [PATCH] Broadened OQS_BUILD_TESTS to OQS_BUILD_ONLY_LIB and removed side-effecting asserts. (#741) --- .CMake/compiler_opts.cmake | 16 ++++++++------ .circleci/config.yml | 7 ++++--- CMakeLists.txt | 4 ++-- src/common/aes/aes_c.c | 6 +++--- src/common/aes/aes_ossl.c | 40 +++++++++++++++++------------------ src/common/common.h | 42 ++++++++++++++++++++++++++++++------- src/common/rand/rand.c | 8 +++---- src/common/sha2/sha2_ossl.c | 8 +++---- 8 files changed, 80 insertions(+), 51 deletions(-) diff --git a/.CMake/compiler_opts.cmake b/.CMake/compiler_opts.cmake index abe0abfe1..c0cb43582 100644 --- a/.CMake/compiler_opts.cmake +++ b/.CMake/compiler_opts.cmake @@ -5,9 +5,11 @@ if(CMAKE_C_COMPILER_ID MATCHES "Clang") add_compile_options(-Wpedantic) add_compile_options(-Wno-unused-command-line-argument) - set(THREADS_PREFER_PTHREAD_FLAG ON) - find_package(Threads REQUIRED) - set(OQS_USE_PTHREADS_IN_TESTS 1) + if(NOT OQS_BUILD_ONLY_LIB) + set(THREADS_PREFER_PTHREAD_FLAG ON) + find_package(Threads REQUIRED) + set(OQS_USE_PTHREADS_IN_TESTS 1) + endif() if(OQS_USE_CPU_EXTENSIONS) include(${CMAKE_CURRENT_LIST_DIR}/gcc_clang_intrinsics.cmake) @@ -57,9 +59,11 @@ elseif(CMAKE_C_COMPILER_ID STREQUAL "GNU") add_compile_options(-Wfloat-equal) add_compile_options(-Wwrite-strings) - set(THREADS_PREFER_PTHREAD_FLAG ON) - find_package(Threads REQUIRED) - set(OQS_USE_PTHREADS_IN_TESTS 1) + if(NOT OQS_BUILD_ONLY_LIB) + set(THREADS_PREFER_PTHREAD_FLAG ON) + find_package(Threads REQUIRED) + set(OQS_USE_PTHREADS_IN_TESTS 1) + endif() if(OQS_USE_CPU_EXTENSIONS) include(${CMAKE_CURRENT_LIST_DIR}/gcc_clang_intrinsics.cmake) diff --git a/.circleci/config.yml b/.circleci/config.yml index 0d66b78a9..74ef9beb6 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -101,7 +101,7 @@ jobs: <<: *oqsjob environment: IMAGE: openquantumsafe/ci-centos-8-amd64:latest - CONFIGURE_ARGS: -DOQS_PORTABLE_BUILD=OFF + CONFIGURE_ARGS: -DCMAKE_BUILD_TYPE=Release -DOQS_PORTABLE_BUILD=OFF SKIP_TESTS: style debian-buster-amd64: <<: *oqsjob @@ -121,7 +121,7 @@ jobs: debian-buster-armel: <<: *emulatedjob environment: - CONFIGURE_ARGS: -DOQS_ENABLE_SIG_SPHINCS=OFF # sig-sphincs exhausts memory on CircleCI servers + CONFIGURE_ARGS: -DCMAKE_BUILD_TYPE=Release -DOQS_ENABLE_SIG_SPHINCS=OFF # sig-sphincs exhausts memory on CircleCI servers ARCH: armel ubuntu-bionic-x86_64-gcc8: <<: *oqsjob @@ -133,7 +133,7 @@ jobs: environment: IMAGE: openquantumsafe/ci-ubuntu-bionic-x86_64:latest CC: gcc-7 - CONFIGURE_ARGS: -DOQS_USE_OPENSSL=OFF + CONFIGURE_ARGS: -DCMAKE_BUILD_TYPE=Release -DOQS_USE_OPENSSL=OFF ubuntu-bionic-x86_64-gcc7-shared: <<: *oqsjob environment: @@ -148,6 +148,7 @@ jobs: IMAGE: openquantumsafe/ci-ubuntu-bionic-x86_64:latest CC: clang-9 #TODO: Adding -DOQS_PORTABLE_BUILD=OFF causes kat_sig picnic to fail + CONFIGURE_ARGS: -DCMAKE_BUILD_TYPE=Release ubuntu-bionic-x86_64-asan: <<: *oqsjob environment: diff --git a/CMakeLists.txt b/CMakeLists.txt index 604333f3b..a27a8f4cd 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -36,7 +36,7 @@ endif() option(OQS_USE_CPU_EXTENSIONS "Enable compile and run-time support for CPU extensions such as AVX2, SSE, etc." ON) option(OQS_PORTABLE_BUILD "Ensure the resulting library is portable. This implies having run-time checks for CPU extensions." ON) -option(OQS_BUILD_TESTS "Build liboqs test programs." ON) +option(OQS_BUILD_ONLY_LIB "Build only liboqs and do not expose build targets for tests, documentation, and pretty-printing available." OFF) include(.CMake/compiler_opts.cmake) include(.CMake/alg_support.cmake) @@ -122,7 +122,7 @@ set(PUBLIC_HEADERS ${PUBLIC_HEADERS} ${PROJECT_BINARY_DIR}/include/oqs/oqsconfig include_directories(${PROJECT_BINARY_DIR}/include) add_subdirectory(src) -if(OQS_BUILD_TESTS) +if(NOT OQS_BUILD_ONLY_LIB) add_subdirectory(tests) find_package(Doxygen) diff --git a/src/common/aes/aes_c.c b/src/common/aes/aes_c.c index ae702c97b..012c8cd32 100644 --- a/src/common/aes/aes_c.c +++ b/src/common/aes/aes_c.c @@ -221,7 +221,7 @@ static void key_schedule_core(byte *a, int i) { // http://en.wikipedia.org/wiki/Rijndael_key_schedule#The_key_schedule void OQS_AES128_ECB_load_schedule(const uint8_t *key, void **_schedule, UNUSED int for_encryption) { *_schedule = malloc(16 * 11); - assert(*_schedule != NULL); + OQS_EXIT_IF_NULLPTR(*_schedule); uint8_t *schedule = (uint8_t *) *_schedule; int bytes = 16; // The count of how many bytes we've created so far int i = 1; // The rcon iteration value i is set to 1 @@ -257,7 +257,7 @@ void OQS_AES128_free_schedule(void *schedule) { // http://en.wikipedia.org/wiki/Rijndael_key_schedule#The_key_schedule void OQS_AES256_ECB_load_schedule(const uint8_t *key, void **_schedule, UNUSED int for_encryption) { *_schedule = malloc(16 * 15); - assert(*_schedule != NULL); + OQS_EXIT_IF_NULLPTR(*_schedule); uint8_t *schedule = (uint8_t *) *_schedule; int i = 0; // The count of how many iterations we've done uint8_t t[4]; // Temporary working area @@ -526,7 +526,7 @@ void OQS_AES256_CTR_sch(const uint8_t *iv, size_t iv_len, const void *schedule, memcpy(&ctr_be, &iv[12], 4); ctr = BE_TO_UINT32(ctr_be); } else { - assert(0); + exit(EXIT_FAILURE); } while (out_len >= 16) { ctr_be = UINT32_TO_BE(ctr); diff --git a/src/common/aes/aes_ossl.c b/src/common/aes/aes_ossl.c index 4d0c898d5..420c3efbd 100644 --- a/src/common/aes/aes_ossl.c +++ b/src/common/aes/aes_ossl.c @@ -15,15 +15,15 @@ struct key_schedule { void OQS_AES128_ECB_load_schedule(const uint8_t *key, void **schedule, int for_encryption) { *schedule = malloc(sizeof(struct key_schedule)); - assert(*schedule != NULL); + OQS_EXIT_IF_NULLPTR(*schedule); struct key_schedule *ks = (struct key_schedule *) *schedule; ks->for_ECB = 1; ks->ctx = EVP_CIPHER_CTX_new(); - assert(ks->ctx != NULL); + OQS_EXIT_IF_NULLPTR(ks->ctx); if (for_encryption) { - assert(1 == EVP_EncryptInit_ex(ks->ctx, EVP_aes_128_ecb(), NULL, key, NULL)); + OQS_OPENSSL_GUARD(EVP_EncryptInit_ex(ks->ctx, EVP_aes_128_ecb(), NULL, key, NULL)); } else { - assert(1 == EVP_DecryptInit_ex(ks->ctx, EVP_aes_128_ecb(), NULL, key, NULL)); + OQS_OPENSSL_GUARD(EVP_DecryptInit_ex(ks->ctx, EVP_aes_128_ecb(), NULL, key, NULL)); } EVP_CIPHER_CTX_set_padding(ks->ctx, 0); } @@ -57,40 +57,40 @@ void OQS_AES128_ECB_enc_sch(const uint8_t *plaintext, const size_t plaintext_len assert(plaintext_len % 16 == 0); int outlen; const struct key_schedule *ks = (const struct key_schedule *) schedule; - SIZE_T_TO_INT_OR_ABORT(plaintext_len, plaintext_len_int) - assert(1 == EVP_EncryptUpdate(ks->ctx, ciphertext, &outlen, plaintext, plaintext_len_int)); + SIZE_T_TO_INT_OR_EXIT(plaintext_len, plaintext_len_int) + OQS_OPENSSL_GUARD(EVP_EncryptUpdate(ks->ctx, ciphertext, &outlen, plaintext, plaintext_len_int)); assert(outlen == plaintext_len_int); - assert(1 == EVP_EncryptFinal_ex(ks->ctx, ciphertext, &outlen)); + OQS_OPENSSL_GUARD(EVP_EncryptFinal_ex(ks->ctx, ciphertext, &outlen)); } void OQS_AES128_ECB_dec_sch(const uint8_t *ciphertext, const size_t ciphertext_len, const void *schedule, uint8_t *plaintext) { assert(ciphertext_len % 16 == 0); int outlen; const struct key_schedule *ks = (const struct key_schedule *) schedule; - SIZE_T_TO_INT_OR_ABORT(ciphertext_len, ciphertext_len_int) - assert(1 == EVP_DecryptUpdate(ks->ctx, plaintext, &outlen, ciphertext, ciphertext_len_int)); + SIZE_T_TO_INT_OR_EXIT(ciphertext_len, ciphertext_len_int) + OQS_OPENSSL_GUARD(EVP_DecryptUpdate(ks->ctx, plaintext, &outlen, ciphertext, ciphertext_len_int)); assert(outlen == ciphertext_len_int); - assert(1 == EVP_DecryptFinal_ex(ks->ctx, plaintext, &outlen)); + OQS_OPENSSL_GUARD(EVP_DecryptFinal_ex(ks->ctx, plaintext, &outlen)); } void OQS_AES256_ECB_load_schedule(const uint8_t *key, void **schedule, int for_encryption) { *schedule = malloc(sizeof(struct key_schedule)); - assert(*schedule != NULL); + OQS_EXIT_IF_NULLPTR(*schedule); struct key_schedule *ks = (struct key_schedule *) *schedule; ks->for_ECB = 1; ks->ctx = EVP_CIPHER_CTX_new(); - assert(ks->ctx != NULL); + OQS_EXIT_IF_NULLPTR(ks->ctx); if (for_encryption) { - assert(1 == EVP_EncryptInit_ex(ks->ctx, EVP_aes_256_ecb(), NULL, key, NULL)); + OQS_OPENSSL_GUARD(EVP_EncryptInit_ex(ks->ctx, EVP_aes_256_ecb(), NULL, key, NULL)); } else { - assert(1 == EVP_DecryptInit_ex(ks->ctx, EVP_aes_256_ecb(), NULL, key, NULL)); + OQS_OPENSSL_GUARD(EVP_DecryptInit_ex(ks->ctx, EVP_aes_256_ecb(), NULL, key, NULL)); } EVP_CIPHER_CTX_set_padding(ks->ctx, 0); } void OQS_AES256_CTR_load_schedule(const uint8_t *key, void **schedule) { *schedule = malloc(sizeof(struct key_schedule)); - assert(*schedule != NULL); + OQS_EXIT_IF_NULLPTR(*schedule); struct key_schedule *ks = (struct key_schedule *) *schedule; ks->for_ECB = 0; ks->ctx = NULL; @@ -139,15 +139,15 @@ void OQS_AES256_CTR_sch(const uint8_t *iv, size_t iv_len, const void *schedule, } else if (iv_len == 16) { memcpy(iv_ctr, iv, 16); } else { - assert(0); + exit(EXIT_FAILURE); } const struct key_schedule *ks = (const struct key_schedule *) schedule; - assert(1 == EVP_EncryptInit_ex(ctr_ctx, EVP_aes_256_ctr(), NULL, ks->key, iv_ctr)); + OQS_OPENSSL_GUARD(EVP_EncryptInit_ex(ctr_ctx, EVP_aes_256_ctr(), NULL, ks->key, iv_ctr)); - SIZE_T_TO_INT_OR_ABORT(out_len, out_len_input_int) + SIZE_T_TO_INT_OR_EXIT(out_len, out_len_input_int) memset(out, 0, (size_t)out_len_input_int); int out_len_output; - assert(1 == EVP_EncryptUpdate(ctr_ctx, out, &out_len_output, out, out_len_input_int)); - assert(1 == EVP_EncryptFinal_ex(ctr_ctx, out + out_len_output, &out_len_output)); + OQS_OPENSSL_GUARD(EVP_EncryptUpdate(ctr_ctx, out, &out_len_output, out, out_len_input_int)); + OQS_OPENSSL_GUARD(EVP_EncryptFinal_ex(ctr_ctx, out + out_len_output, &out_len_output)); EVP_CIPHER_CTX_free(ctr_ctx); } diff --git a/src/common/common.h b/src/common/common.h index 9a7ab498e..c4d4389e8 100644 --- a/src/common/common.h +++ b/src/common/common.h @@ -17,19 +17,47 @@ extern "C" { #endif +/** + * Macro for terminating the program if x is + * a null pointer. + */ +#define OQS_EXIT_IF_NULLPTR(x) \ + do { \ + if ( (x) == (void*)0 ) \ + exit(EXIT_FAILURE); \ + } while (0) + +/** + * This macro is intended to replace those assert()s + * involving side-effecting statements in aes/aes_ossl.c. + * + * assert() becomes a no-op when -DNDEBUG is defined, + * which causes compilation failures when the statement + * being checked also results in side-effects. + * + * This is a temporary workaround until a better error + * handling strategy is developed. + */ +#define OQS_OPENSSL_GUARD(x) \ + do { \ + if( 1 != (x) ) { \ + exit(EXIT_FAILURE); \ + } \ + } while (0) + /** * Certain functions (such as OQS_randombytes_openssl in * src/rand/rand.c) take in a size_t parameter, but can * only handle values up to INT_MAX for those parameters. * This macro is a temporary workaround for such functions. */ -#define SIZE_T_TO_INT_OR_ABORT(size_t_var_name, int_var_name) \ - int int_var_name = 0; \ - if (size_t_var_name <= INT_MAX) { \ - int_var_name = (int)size_t_var_name; \ - } else { \ - abort(); \ - } +#define SIZE_T_TO_INT_OR_EXIT(size_t_var_name, int_var_name) \ + int int_var_name = 0; \ + if (size_t_var_name <= INT_MAX) { \ + int_var_name = (int)size_t_var_name; \ + } else { \ + exit(EXIT_FAILURE); \ + } /** * Defines which functions should be exposed outside the LibOQS library diff --git a/src/common/rand/rand.c b/src/common/rand/rand.c index 3a50dbf7e..f5a8f604b 100644 --- a/src/common/rand/rand.c +++ b/src/common/rand/rand.c @@ -2,10 +2,8 @@ #if defined(_WIN32) #include #include -#include #define strcasecmp _stricmp #else -#include #include #include #if defined(__APPLE__) @@ -15,6 +13,7 @@ #endif #endif #include +#include #include @@ -31,7 +30,6 @@ static void (*oqs_randombytes_algorithm)(uint8_t *, size_t) = &OQS_randombytes_o #else static void (*oqs_randombytes_algorithm)(uint8_t *, size_t) = &OQS_randombytes_system; #endif - OQS_API OQS_STATUS OQS_randombytes_switch_algorithm(const char *algorithm) { if (0 == strcasecmp(OQS_RAND_alg_system, algorithm)) { oqs_randombytes_algorithm = &OQS_randombytes_system; @@ -105,7 +103,7 @@ void OQS_randombytes_system(uint8_t *random_array, size_t bytes_to_read) { HCRYPTPROV hCryptProv; if (!CryptAcquireContext(&hCryptProv, NULL, NULL, PROV_RSA_FULL, CRYPT_VERIFYCONTEXT) || !CryptGenRandom(hCryptProv, (DWORD) bytes_to_read, random_array)) { - assert(0); // no other way to return an error; better fail than return bad random data + exit(EXIT_FAILURE); // better to fail than to return bad random data } CryptReleaseContext(hCryptProv, 0); } @@ -114,7 +112,7 @@ void OQS_randombytes_system(uint8_t *random_array, size_t bytes_to_read) { #ifdef OQS_USE_OPENSSL void OQS_randombytes_openssl(uint8_t *random_array, size_t bytes_to_read) { int rc; - SIZE_T_TO_INT_OR_ABORT(bytes_to_read, bytes_to_read_int) + SIZE_T_TO_INT_OR_EXIT(bytes_to_read, bytes_to_read_int) do { rc = RAND_bytes(random_array, bytes_to_read_int); } while (rc != 1); diff --git a/src/common/sha2/sha2_ossl.c b/src/common/sha2/sha2_ossl.c index 82e0517b2..9c826553f 100644 --- a/src/common/sha2/sha2_ossl.c +++ b/src/common/sha2/sha2_ossl.c @@ -3,8 +3,6 @@ * \brief Implementation of the OQS SHA2 API via calls to OpenSSL's SHA-2 functions */ -#include - #include #ifdef OQS_USE_SHA2_OPENSSL @@ -47,7 +45,7 @@ void OQS_SHA2_sha256_inc_init(OQS_SHA2_sha256_ctx *state) { EVP_MD_CTX *mdctx; const EVP_MD *md = NULL; md = EVP_sha256(); - assert(md != NULL); + OQS_EXIT_IF_NULLPTR(md); mdctx = EVP_MD_CTX_new(); EVP_DigestInit_ex(mdctx, md, NULL); state->ctx = mdctx; @@ -79,7 +77,7 @@ void OQS_SHA2_sha384_inc_init(OQS_SHA2_sha384_ctx *state) { EVP_MD_CTX *mdctx; const EVP_MD *md = NULL; md = EVP_sha384(); - assert(md != NULL); + OQS_EXIT_IF_NULLPTR(md); mdctx = EVP_MD_CTX_new(); EVP_DigestInit_ex(mdctx, md, NULL); state->ctx = mdctx; @@ -111,7 +109,7 @@ void OQS_SHA2_sha512_inc_init(OQS_SHA2_sha512_ctx *state) { EVP_MD_CTX *mdctx; const EVP_MD *md = NULL; md = EVP_sha512(); - assert(md != NULL); + OQS_EXIT_IF_NULLPTR(md); mdctx = EVP_MD_CTX_new(); EVP_DigestInit_ex(mdctx, md, NULL); state->ctx = mdctx;