From aa9fbdc6bc11f4ca741a197d45c06064162e5d65 Mon Sep 17 00:00:00 2001 From: Per Nilsson Date: Tue, 7 Dec 2021 20:24:23 +0100 Subject: [PATCH 1/2] Fix supressed warnings via -w --- CMakeLists.txt | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index c3c35c12..190e8df4 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -26,9 +26,15 @@ # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. cmake_minimum_required (VERSION 3.5) +# policy CMP0025 is to get AppleClang identifier rather than Clang for both +# this matters since the apple compiler accepts different flags. cmake_policy(SET CMP0025 NEW) +cmake_policy(SET CMP0042 NEW) +cmake_policy(SET CMP0054 NEW) + include(CheckFunctionExists) -set (CMAKE_C_STANDARD 99) + +set (CMAKE_C_STANDARD 11) project (yubico-piv-tool) @@ -96,16 +102,14 @@ if(MSVC) else() find_package (PkgConfig REQUIRED) - set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -w") # -g -O2 - set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Wextra -Werror") + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Werror") + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wno-missing-braces") + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wformat -Wformat-security") set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wshadow") set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wwrite-strings") set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wmissing-prototypes") set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wbad-function-cast") - set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -pedantic") set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fstack-protector-all") - set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -std=c99") - set(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} -g2") set(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} -fno-omit-frame-pointer") endif() @@ -113,7 +117,7 @@ endif() # Use -Wshorten-64-to-32 if available. check_c_compiler_flag("-Wshorten-64-to-32" HAVE_SHORTEN_64_TO_32) if(HAVE_SHORTEN_64_TO_32) - set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wshorten-64-to-32") +# set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wshorten-64-to-32") endif() # Avoid https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66425 @@ -151,6 +155,7 @@ message(" Install prefix: ${CMAKE_PREFIX_PATH}") message(" Compiler: ${CMAKE_C_COMPILER}") message(" Compiler ID: ${CMAKE_C_COMPILER_ID}") message(" CFLAGS: ${CMAKE_C_FLAGS}") +message(" CFLAGS_DEBUG: ${CMAKE_C_FLAGS_DEBUG}") message(" CPPFLAGS: ${CMAKE_CXX_FLAGS}") message(" Warnings: ${WARN_FLAGS}") message(" Build type: ${CMAKE_BUILD_TYPE}") From f31435511881ef3534d737efdaa608f3398ebebd Mon Sep 17 00:00:00 2001 From: Per Nilsson Date: Tue, 7 Dec 2021 20:25:27 +0100 Subject: [PATCH 2/2] Fix all warnings that now occur --- common/util.c | 2 +- lib/tests/api.c | 39 ++++++++++++++++---------------- lib/tests/basic.c | 2 +- lib/tests/parse_key.c | 2 +- lib/ykpiv.c | 2 +- tool/tests/parse_name.c | 2 +- tool/tests/test_inout.c | 2 +- tool/yubico-piv-tool.c | 4 ++-- ykcs11/tests/ykcs11_tests.c | 24 +++++++++++--------- ykcs11/tests/ykcs11_tests_util.c | 23 +++---------------- ykcs11/ykcs11.c | 2 +- 11 files changed, 44 insertions(+), 60 deletions(-) diff --git a/common/util.c b/common/util.c index 5bdfa989..f92c6ae6 100644 --- a/common/util.c +++ b/common/util.c @@ -112,7 +112,7 @@ unsigned char get_algorithm(EVP_PKEY *key) { } } -char *string_parser(char *str_orig, char delimiter, char *str_found) { +static char *string_parser(char *str_orig, char delimiter, char *str_found) { char escape_char = '\\'; int f = 0; char *p = str_orig; diff --git a/lib/tests/api.c b/lib/tests/api.c index a3081e62..5bff9afa 100644 --- a/lib/tests/api.c +++ b/lib/tests/api.c @@ -44,8 +44,6 @@ #define dprintf(fd, ...) fprintf(stdout, __VA_ARGS__) #endif -int destruction_confirmed(void); - // only defined in libcheck 0.11+ (linux distros still shipping 0.10) #ifndef ck_assert_ptr_nonnull #define ck_assert_ptr_nonnull(a) ck_assert((a) != NULL) @@ -67,7 +65,23 @@ const uint8_t g_cert[] = { "0123456789ABCDEFGHIK0123456789ABCDEFGHIK0123456789ABCDEFGHIK0123456789ABCDEFGHIK" }; -void setup(void) { +#if HW_TESTS + +int destruction_confirmed(void) { + char *confirmed = getenv("YKPIV_ENV_HWTESTS_CONFIRMED"); + if (confirmed && confirmed[0] == '1') { +#ifdef _WIN32 + return 1; +#else + return system("../../../tools/confirm.sh") == 0; +#endif + } + // Use dprintf() to write directly to stdout, since cmake eats the standard stdout/stderr pointers. + dprintf(0, "\n***\n*** Hardware tests skipped.\n***\n\n"); + return 0; +} + +static void setup(void) { ykpiv_rc res; // Require user confirmation to continue, since this test suite will clear @@ -83,7 +97,7 @@ void setup(void) { ck_assert_int_eq(res, YKPIV_OK); } -void teardown(void) { +static void teardown(void) { ykpiv_rc res; // This is the expected case, if the allocator test ran, since it de-inits. @@ -97,7 +111,6 @@ void teardown(void) { ck_assert_int_eq(res, YKPIV_OK); } -#if HW_TESTS START_TEST(test_devicemodel) { ykpiv_rc res; ykpiv_devmodel model; @@ -919,21 +932,7 @@ START_TEST(test_pin_cache) { END_TEST #endif -int destruction_confirmed(void) { - char *confirmed = getenv("YKPIV_ENV_HWTESTS_CONFIRMED"); - if (confirmed && confirmed[0] == '1') { -#ifdef _WIN32 - return 1; -#else - return system("../../../tools/confirm.sh") == 0; -#endif - } - // Use dprintf() to write directly to stdout, since cmake eats the standard stdout/stderr pointers. - dprintf(0, "\n***\n*** Hardware tests skipped.\n***\n\n"); - return 0; -} - -Suite *test_suite(void) { +static Suite *test_suite(void) { Suite *s; TCase *tc; diff --git a/lib/tests/basic.c b/lib/tests/basic.c index 49b6f6b8..92ca92a3 100644 --- a/lib/tests/basic.c +++ b/lib/tests/basic.c @@ -70,7 +70,7 @@ START_TEST(test_strerror) { } END_TEST -Suite *basic_suite(void) { +static Suite *basic_suite(void) { Suite *s; TCase *tc; diff --git a/lib/tests/parse_key.c b/lib/tests/parse_key.c index 6f68c1f7..604180ac 100644 --- a/lib/tests/parse_key.c +++ b/lib/tests/parse_key.c @@ -74,7 +74,7 @@ START_TEST(test_parse_key) { } END_TEST -Suite *parsekey_suite(void) { +static Suite *parsekey_suite(void) { Suite *s; TCase *tc; diff --git a/lib/ykpiv.c b/lib/ykpiv.c index 18db7d35..dd8ffdb8 100644 --- a/lib/ykpiv.c +++ b/lib/ykpiv.c @@ -658,7 +658,7 @@ ykpiv_rc ykpiv_list_readers(ykpiv_state *state, char *readers, size_t *len) { } // Add errors that we should reconnect on here -bool ShouldReconnect(LONG rc) { +static bool ShouldReconnect(LONG rc) { switch(rc) { case SCARD_W_UNPOWERED_CARD: case SCARD_W_RESET_CARD: diff --git a/tool/tests/parse_name.c b/tool/tests/parse_name.c index 6a71afea..2f43243d 100644 --- a/tool/tests/parse_name.c +++ b/tool/tests/parse_name.c @@ -89,7 +89,7 @@ START_TEST(test_parse_name) { } END_TEST -Suite *test_suite(void) { +static Suite *test_suite(void) { Suite *s; TCase *tc; diff --git a/tool/tests/test_inout.c b/tool/tests/test_inout.c index cb895911..3a297eb2 100644 --- a/tool/tests/test_inout.c +++ b/tool/tests/test_inout.c @@ -74,7 +74,7 @@ START_TEST(test_inout) { } END_TEST -Suite *test_suite(void) { +static Suite *test_suite(void) { Suite *s; TCase *tc; diff --git a/tool/yubico-piv-tool.c b/tool/yubico-piv-tool.c index 36aa1961..9fde8f51 100644 --- a/tool/yubico-piv-tool.c +++ b/tool/yubico-piv-tool.c @@ -143,7 +143,7 @@ struct internal_key { size_t oid_len; }; -int yk_rsa_meth_sign(int dtype, const unsigned char *m, unsigned int m_length, +static int yk_rsa_meth_sign(int dtype, const unsigned char *m, unsigned int m_length, unsigned char *sigret, unsigned int *siglen, const RSA *rsa) { size_t yk_siglen = RSA_size(rsa); const RSA_METHOD *meth = RSA_get_method(rsa); @@ -164,7 +164,7 @@ int yk_rsa_meth_sign(int dtype, const unsigned char *m, unsigned int m_length, return 0; } -int yk_ec_meth_sign(int type, const unsigned char *dgst, int dlen, +static int yk_ec_meth_sign(int type, const unsigned char *dgst, int dlen, unsigned char *sig, unsigned int *siglen, const BIGNUM *kinv, const BIGNUM *r, EC_KEY *ec) { size_t yk_siglen = ECDSA_size(ec); diff --git a/ykcs11/tests/ykcs11_tests.c b/ykcs11/tests/ykcs11_tests.c index 3aad905f..e83e5119 100644 --- a/ykcs11/tests/ykcs11_tests.c +++ b/ykcs11/tests/ykcs11_tests.c @@ -79,35 +79,35 @@ static void get_functions() { } -static void init_connection() { - asrt(funcs->C_Initialize(NULL), CKR_OK, "INITIALIZE"); - CK_SLOT_ID pSlotList[16]; - CK_ULONG pulCount = 16; - asrt(funcs->C_GetSlotList(true, pSlotList, &pulCount), CKR_OK, "GETSLOTLIST"); -} - static void test_lib_info() { dprintf(0, "TEST START: test_lib_info()\n"); - const CK_CHAR_PTR MANUFACTURER_ID = "Yubico (www.yubico.com)"; - const CK_CHAR_PTR YKCS11_DESCRIPTION = "PKCS#11 PIV Library (SP-800-73)"; + const CK_CHAR_PTR MANUFACTURER_ID = (const CK_CHAR_PTR)"Yubico (www.yubico.com)"; + const CK_CHAR_PTR YKCS11_DESCRIPTION = (const CK_CHAR_PTR)"PKCS#11 PIV Library (SP-800-73)"; const CK_ULONG CRYPTOKI_VERSION_MAJ = 2; const CK_ULONG CRYPTOKI_VERSION_MIN = 40; CK_INFO info; asrt(funcs->C_Initialize(NULL), CKR_OK, "INITIALIZE"); asrt(funcs->C_GetInfo(&info), CKR_OK, "GET_INFO"); - asrt(strncmp(info.manufacturerID, MANUFACTURER_ID, strlen(MANUFACTURER_ID)), 0, "MANUFACTURER"); + asrt(strncmp((const char*)info.manufacturerID, (const char*)MANUFACTURER_ID, strlen((const char*)MANUFACTURER_ID)), 0, "MANUFACTURER"); asrt(info.cryptokiVersion.major, CRYPTOKI_VERSION_MAJ, "CK_MAJ"); asrt(info.cryptokiVersion.minor, CRYPTOKI_VERSION_MIN, "CK_MIN"); asrt(info.libraryVersion.major, YKCS11_VERSION_MAJOR, "LIB_MAJ"); asrt(info.libraryVersion.minor, ((YKCS11_VERSION_MINOR * 10) + YKCS11_VERSION_PATCH), "LIB_MIN"); - asrt(strncmp(info.libraryDescription, YKCS11_DESCRIPTION, strlen(YKCS11_DESCRIPTION)), 0, "LIB_DESC"); + asrt(strncmp((const char*)info.libraryDescription, (const char*)YKCS11_DESCRIPTION, strlen((const char*)YKCS11_DESCRIPTION)), 0, "LIB_DESC"); asrt(funcs->C_Finalize(NULL), CKR_OK, "FINALIZE"); dprintf(0, "TEST END: test_lib_info()\n"); } #if HW_TESTS +static void init_connection() { + asrt(funcs->C_Initialize(NULL), CKR_OK, "INITIALIZE"); + CK_SLOT_ID pSlotList[16]; + CK_ULONG pulCount = 16; + asrt(funcs->C_GetSlotList(true, pSlotList, &pulCount), CKR_OK, "GETSLOTLIST"); +} + static void test_initalize() { dprintf(0, "TEST START: test_initalize()\n"); asrt(funcs->C_Initialize(NULL), CKR_OK, "INITIALIZE"); @@ -842,6 +842,7 @@ static void test_digest() { #endif +#if HW_TESTS int destruction_confirmed(void) { #ifdef _WIN32 return 1; @@ -849,6 +850,7 @@ int destruction_confirmed(void) { return system("../../../tools/confirm.sh") == 0; #endif } +#endif int main(void) { diff --git a/ykcs11/tests/ykcs11_tests_util.c b/ykcs11/tests/ykcs11_tests_util.c index a43bd9de..72a3e3f5 100644 --- a/ykcs11/tests/ykcs11_tests_util.c +++ b/ykcs11/tests/ykcs11_tests_util.c @@ -715,10 +715,10 @@ void test_ec_ecdh_simple(CK_FUNCTION_LIST_PTR funcs, CK_SESSION_HANDLE session, asrt(funcs->C_DestroyObject(session, sk), CKR_OK, "DestroyObject"); asrt(funcs->C_Logout(session), CKR_OK, "Logout USER"); // Skip DER encoding - ptr = pointTemplate->pValue; - ptr += 2; + const unsigned char *ptr2 = pointTemplate->pValue; + ptr2 += 2; EC_KEY *pk = EC_KEY_new_by_curve_name(curve); - pk = o2i_ECPublicKey(&pk, &ptr, pointTemplate->ulValueLen - 2); + pk = o2i_ECPublicKey(&pk, &ptr2, pointTemplate->ulValueLen - 2); asrt(ECDH_compute_key(secret, sizeof(secret), EC_KEY_get0_public_key(pk), tmpkey, NULL), bits / 8, "ECDH_compute_key"); asrt(memcmp(secret, secret2, bits / 8), 0, "Compare secrets"); EC_KEY_free(pk); @@ -1016,23 +1016,6 @@ void test_rsa_sign_pss(CK_FUNCTION_LIST_PTR funcs, CK_SESSION_HANDLE session, CK asrt(funcs->C_Logout(session), CKR_OK, "Logout USER"); } -static int is_data_too_large(int enc_ret) { - if(enc_ret != -1) { - return 0; - } - - unsigned long err; - ERR_load_crypto_strings(); - err = ERR_get_error(); - //char err_str[128] = {0}; - //ERR_error_string_n(err, err_str, 128); - //printf("%ld %s\n", err, err_str); - if(err == 67534980) { // Error code for "data too large for modulus" - return 1; - } - return 0; -} - void test_rsa_decrypt(CK_FUNCTION_LIST_PTR funcs, CK_SESSION_HANDLE session, CK_OBJECT_HANDLE_PTR obj_pvtkey, CK_BYTE n_keys, RSA* rsak, CK_MECHANISM_TYPE mech_type, CK_ULONG padding) { CK_BYTE i, j; diff --git a/ykcs11/ykcs11.c b/ykcs11/ykcs11.c index 1252a2ed..854039af 100644 --- a/ykcs11/ykcs11.c +++ b/ykcs11/ykcs11.c @@ -3660,7 +3660,7 @@ CK_DEFINE_FUNCTION(CK_RV, C_DeriveKey)( locking.pfnLockMutex(session->slot->mutex); DBG("Deriving ECDH shared secret into object %u using slot %lx", PIV_SECRET_OBJ, slot); - ykpiv_rc rc = ykpiv_decipher_data(session->slot->piv_state, params->pPublicData, params->ulPublicDataLen, &buf, &len, algo, slot); + ykpiv_rc rc = ykpiv_decipher_data(session->slot->piv_state, params->pPublicData, params->ulPublicDataLen, buf, &len, algo, slot); if(rc != YKPIV_OK) { DBG("Failed to derive key in slot %lx: %s", slot, ykpiv_strerror(rc));