Merge pull request #342 from Yubico/fix-cmake

Fix warnings that were supressed by -w

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,24 +102,22 @@ 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()
 
 # 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}")

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;

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;
 

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;
 

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;
 

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:

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;
 

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;
 

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

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,13 +842,15 @@ static void test_digest() {
 
 #endif
 
+#if HW_TESTS
 int destruction_confirmed(void) {
 #ifdef _WIN32
   return 1;
 #else
   return system("../../../tools/confirm.sh") == 0;
 #endif
 }
+#endif
 
 int main(void) {
 

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;

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