From e349a19d3d5c85ab44b71d7e7402615e991d25fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20B=C3=A1rta?= Date: Mon, 15 Aug 2022 13:07:02 +0200 Subject: [PATCH 1/2] Fix make/cmake and includes. --- CMakeLists.txt | 13 ++++++------- Makefile | 18 ++++++++++++------ debian/rules | 3 ++- include/secutils/certstatus/certstatus.h | 1 + include/secutils/certstatus/crl_mgmt.h | 3 ++- include/secutils/certstatus/crls.h | 2 ++ include/secutils/credentials/cert.h | 3 +-- include/secutils/credentials/verify.h | 6 ++---- include/secutils/storage/files_dv.h | 2 ++ include/secutils/storage/files_icv.h | 1 + include/secutils/util/util.h | 3 ++- src/certstatus/certstatus.c | 8 ++++---- src/certstatus/crls.c | 2 +- src/certstatus/ocsp.c | 2 +- src/config/config.c | 2 +- src/config/config_update.c | 2 +- src/config/opt.c | 2 +- src/connections/conn.c | 2 +- src/connections/http.c | 3 ++- src/connections/tls.c | 2 +- src/credentials/cert.c | 2 +- src/credentials/credentials.c | 2 +- src/credentials/key.c | 2 +- src/credentials/store.c | 2 +- src/credentials/trusted.c | 2 +- src/credentials/verify.c | 5 ++--- src/crypto/crypto.c | 2 +- src/storage/files.c | 2 +- src/storage/files_dv.c | 2 +- src/storage/files_icv.c | 5 +++-- src/storage/uta_api.c | 2 +- src/util/extensions.c | 2 +- src/util/log.c | 4 ++-- src/util/util.c | 2 +- 34 files changed, 64 insertions(+), 52 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index cd52905..ebef363 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -43,13 +43,11 @@ set(SRC_EXAMPLE_DIR ${PROJECT_SOURCE_DIR}/test) find_package(OpenSSL REQUIRED) -if(DEFINED ENV{NDEBUG}) - add_definitions(-DNDEBUG=1 -O2) -else() - add_definitions(-g -O0) - add_definitions(-fsanitize=address -fsanitize=undefined -fno-sanitize-recover=all) - link_libraries("-fsanitize=address -fsanitize=undefined -fno-sanitize-recover=all") -endif() +# Add this to get some additional runtime checks. +# Warning: it's incompatible with tools like Valgrind and you have to add it to the app using this lib too. +# add_definitions("-fsanitize=address -fsanitize=undefined -fno-sanitize-recover=all") +# link_libraries("-fsanitize=address -fsanitize=undefined -fno-sanitize-recover=all") + add_definitions(-Wall -Woverflow -Wextra -Wswitch -Wmissing-prototypes -Wstrict-prototypes -Wformat -Wtype-limits -Wundef -Wconversion -Wno-shadow -Wno-conversion -Wno-sign-conversion -Wno-unused-parameter -Wno-sign-compare) # TODO enable -Wconversion -Wsign-conversion -Wsign-compare -Wunused-parameter add_definitions(-Wformat -Wformat-security -Wno-declaration-after-statement -Wno-vla) # -Wpointer-arith -pedantic -DPEDANTIC # -Werror @@ -75,6 +73,7 @@ set(SECUTIL_LIB_SRC ${SRC_DIR}/certstatus/certstatus.c ${SRC_DIR}/connections/conn.c ${SRC_DIR}/connections/http.c ${SRC_DIR}/connections/tls.c + ${SRC_DIR}/credentials/cert.c ${SRC_DIR}/credentials/credentials.c ${SRC_DIR}/credentials/key.c ${SRC_DIR}/credentials/store.c diff --git a/Makefile b/Makefile index d080f41..41ed47d 100644 --- a/Makefile +++ b/Makefile @@ -57,12 +57,16 @@ endif # Note: stuff for testing purposes should go here ################################################################################ -ifdef NDEBUG +ifdef DEBUG + DEBUG_FLAGS ?= -g -O0 +# Add this to get some additional runtime checks. +# Warning: it's incompatible with tools like Valgrind and you have to add it to the app using this lib too +# DEBUG_FLAGS += -fsanitize=address -fsanitize=undefined -fno-sanitize-recover=all +else DEBUG_FLAGS ?= -O2 override DEBUG_FLAGS += -DNDEBUG=1 -else - DEBUG_FLAGS ?= -g -O0 -fsanitize=address -fsanitize=undefined -fno-sanitize-recover=all # not every compiler(version) supports -Og endif + override CFLAGS += $(DEBUG_FLAGS) -Wall -Woverflow -Wextra -Wswitch -Wmissing-prototypes -Wstrict-prototypes -Wformat -Wtype-limits -Wundef -Wconversion override CFLAGS += -Wno-shadow -Wno-conversion -Wno-sign-conversion -Wno-sign-compare -Wno-unused-parameter # TODO clean up code and enable -Wshadow -Wconversion -Wsign-conversion -Wsign-compare -Wunused-parameter override CFLAGS += -Wformat -Wformat-security -Wno-declaration-after-statement -Wno-vla # -Wpointer-arith -pedantic -DPEDANTIC # -Werror @@ -150,11 +154,13 @@ ifeq ($(COV_ENABLED), 1) endif $(MAKE) COMPILE_TYPE=$(COMPILE_TYPE) build_only -util: +util: $(OUT_DIR)/$(OUTLIB) $(MAKE) -C util SECUTILS_USE_UTA="$(SECUTILS_USE_UTA)" \ CFLAGS="$(CFLAGS) $(LOCAL_CFLAGS)" LDFLAGS="$(LDFLAGS)" -build_all: build util +build_all: + $(MAKE) build + $(MAKE) util # Binary output target $(OUT_DIR)/$(OUTLIB).$(VERSION): $(OBJS) @@ -219,10 +225,10 @@ clean_uta: $(OUT_DIR)/$(OUTLIB)* $(OUT_DIR)/util/$(OUTBIN) $(OUT_DIR)/util/icvutil.o clean: + $(MAKE) -C util clean rm -rf $(OUT_DIR)/$(OUTLIB)* $(OUT_DIR)/util/$(OUTBIN) $(BUILDDIR) clean_all: clean clean_deb - $(MAKE) -C util clean rm -rf doc refman.pdf *.gcov reports doc: doc/html refman.pdf diff --git a/debian/rules b/debian/rules index f94832c..cb55fb2 100755 --- a/debian/rules +++ b/debian/rules @@ -25,7 +25,8 @@ override_dh_auto_clean: # adding compile flags as, defaults are commonly debug flags override_dh_auto_build: -# CFLAGS="-O2 -DNDEBUG" CXXFLAGS="-O2 -DNDEBUG" DEBUG_FLAGS="" LDFLAGS="" # can be used to avoid dependency on libasan and libubsan +# clear DEBUG_FLAGS so that the default debian options get applied + DEBUG_FLAGS="" \ OPENSSL_DIR=/usr CC=$(CC) CXX=$(CXX) AR=$(AR) \ dh_auto_build -- -j1 build_only util diff --git a/include/secutils/certstatus/certstatus.h b/include/secutils/certstatus/certstatus.h index eb7948d..f9ab7b7 100644 --- a/include/secutils/certstatus/certstatus.h +++ b/include/secutils/certstatus/certstatus.h @@ -17,6 +17,7 @@ #define SECUTILS_CERTSTATUS_H_ #include + #include "../util/log.h" #if OPENSSL_VERSION_NUMBER < 0x10101000L diff --git a/include/secutils/certstatus/crl_mgmt.h b/include/secutils/certstatus/crl_mgmt.h index ccf488b..5a59c18 100644 --- a/include/secutils/certstatus/crl_mgmt.h +++ b/include/secutils/certstatus/crl_mgmt.h @@ -17,7 +17,8 @@ #define SECUTILS_HEADER_CRL_MGMT_H #include -#include + +#include "../basic.h" #ifdef __cplusplus extern "C" { diff --git a/include/secutils/certstatus/crls.h b/include/secutils/certstatus/crls.h index 9b5e5d6..5c2ecd3 100644 --- a/include/secutils/certstatus/crls.h +++ b/include/secutils/certstatus/crls.h @@ -16,6 +16,8 @@ #ifndef SECUTILS_CRLS_H_ #define SECUTILS_CRLS_H_ +#include "../basic.h" + #include /*!***************************************************************************** diff --git a/include/secutils/credentials/cert.h b/include/secutils/credentials/cert.h index 1bf046a..fe72859 100644 --- a/include/secutils/credentials/cert.h +++ b/include/secutils/credentials/cert.h @@ -26,8 +26,7 @@ #include /* for strcmp, strlen */ #include "../basic.h" -#include "../operators.h" -# include "../util/log.h" +#include "../util/log.h" #include diff --git a/include/secutils/credentials/verify.h b/include/secutils/credentials/verify.h index c3bfbeb..bbf0ad3 100644 --- a/include/secutils/credentials/verify.h +++ b/include/secutils/credentials/verify.h @@ -74,15 +74,13 @@ bool verify_cb_cert(X509_STORE_CTX* store_ctx, X509* cert, int err); /*!***************************************************************************** * @brief attempt to verify certificate * - * @param ctx (optional) pointer to UTA context, unused * @param cert certificate to be verified * @param untrusted (optional) intermediate certs that may be useful for building * the chain of certificates between the cert and the trusted certs in the trust store * @param trust_store pointer to structure containing trusted (root) certs and further verification parameters * @note trust_store may contain CRLs loaded via STORE_load_crl_dir() - * @return < 0 on on verification error, 0 for invalid cert, 1 for vaild cert + * @return < 0 on on verification error, 0 for invalid cert, 1 for valid cert *******************************************************************************/ -int CREDENTIALS_verify_cert(OPTIONAL uta_ctx* ctx, X509* cert, - OPTIONAL const STACK_OF(X509) * untrusted, X509_STORE* trust_store); +int CREDENTIALS_verify_cert(X509* cert, OPTIONAL const STACK_OF(X509) * untrusted, X509_STORE* trust_store); #endif /* SECUTILS_VERIFY_H_ */ diff --git a/include/secutils/storage/files_dv.h b/include/secutils/storage/files_dv.h index bf87542..e00adcc 100644 --- a/include/secutils/storage/files_dv.h +++ b/include/secutils/storage/files_dv.h @@ -25,6 +25,8 @@ #include +#include "../util/util.h" + #include "../storage/uta_api.h" #define MAX_UTA_PASS_LEN (MAX_B64_CHARS_PER_BYTE * TA_OUTLEN + 1) #include "files.h" diff --git a/include/secutils/storage/files_icv.h b/include/secutils/storage/files_icv.h index f361e9b..7990fcd 100644 --- a/include/secutils/storage/files_icv.h +++ b/include/secutils/storage/files_icv.h @@ -20,6 +20,7 @@ #include "../storage/uta_api.h" #include +#include /*! * @brief (re-)protect integrity of file (of any type that allows appending text) with ICV derived via UTA diff --git a/include/secutils/util/util.h b/include/secutils/util/util.h index 11c82fc..24eea4f 100644 --- a/include/secutils/util/util.h +++ b/include/secutils/util/util.h @@ -30,11 +30,12 @@ # include # include "../basic.h" -# include "../operators.h" # include # include +# include "../storage/uta_api.h" + static const char *const UTIL_SECUTILS_NAME = "secutils"; /*!< short name of this library */ static const int UTIL_max_path_len = 512; /*!< max length of file path name */ diff --git a/src/certstatus/certstatus.c b/src/certstatus/certstatus.c index 5ee6c09..528cb0b 100644 --- a/src/certstatus/certstatus.c +++ b/src/certstatus/certstatus.c @@ -1,13 +1,13 @@ -/** +/** * @file certstatus.c -* +* * @brief Certificate status checking using CRLs and/or OCSP * * @copyright Copyright (c) Siemens Mobility GmbH, 2021 * * @author David von Oheimb * -* This work is licensed under the terms of the Apache Software License +* This work is licensed under the terms of the Apache Software License * 2.0. See the COPYING file in the top-level directory. * * SPDX-License-Identifier: Apache-2.0 @@ -28,7 +28,7 @@ # include #endif -#include +#include "secutils/operators.h" static unsigned int num_CDPs(const X509* cert) { diff --git a/src/certstatus/crls.c b/src/certstatus/crls.c index c9d545d..370e303 100644 --- a/src/certstatus/crls.c +++ b/src/certstatus/crls.c @@ -27,7 +27,7 @@ #include #include -#include +#include "secutils/operators.h" /* adapted from OpenSSL:crypto/x509/t_crl.c */ void UTIL_print_crl(OPTIONAL BIO* bio, OPTIONAL const X509_CRL* crl) diff --git a/src/certstatus/ocsp.c b/src/certstatus/ocsp.c index 5ff03a0..10da48e 100644 --- a/src/certstatus/ocsp.c +++ b/src/certstatus/ocsp.c @@ -29,7 +29,7 @@ # include # endif -# include +#include "secutils/operators.h" OCSP_RESPONSE* CONN_load_OCSP_http(const char* url, int timeout, const OCSP_REQUEST* req, diff --git a/src/config/config.c b/src/config/config.c index 6598a13..93c833e 100644 --- a/src/config/config.c +++ b/src/config/config.c @@ -22,7 +22,7 @@ #include #include -#include +#include "secutils/operators.h" /* adapted from OpenSSL:apps/include/apps.h */ static opt_t vpm_opts[] = { OPT_V_OPTIONS, OPT_END }; diff --git a/src/config/config_update.c b/src/config/config_update.c index 0d0d66b..58ab986 100644 --- a/src/config/config_update.c +++ b/src/config/config_update.c @@ -17,7 +17,7 @@ #include #include -#include +#include "secutils/operators.h" static void skip_space(char** p) diff --git a/src/config/opt.c b/src/config/opt.c index f41a454..ebd39ec 100644 --- a/src/config/opt.c +++ b/src/config/opt.c @@ -19,7 +19,7 @@ #include /* for strtoimax on Linux */ -#include +#include "secutils/operators.h" const char OPT_more_str[] = "-M"; const char OPT_section_str[] = "-S"; diff --git a/src/connections/conn.c b/src/connections/conn.c index 907587f..f61f2ba 100644 --- a/src/connections/conn.c +++ b/src/connections/conn.c @@ -21,7 +21,7 @@ # include #endif -#include +#include "secutils/operators.h" static const char* skip_scheme(const char* str) { diff --git a/src/connections/http.c b/src/connections/http.c index 3859e96..454992f 100644 --- a/src/connections/http.c +++ b/src/connections/http.c @@ -16,7 +16,6 @@ #if !defined(OPENSSL_NO_OCSP) && !defined(OPENSSL_NO_SOCK) # include -# include # include # include # ifndef SECUTILS_NO_TLS @@ -29,6 +28,8 @@ # endif # include +# include "secutils/operators.h" + /* TODO replace this all by new API in http.h of OpenSSL 3.0 */ static int REQ_CTX_i2d(OCSP_REQ_CTX* rctx, const char* content_type, diff --git a/src/connections/tls.c b/src/connections/tls.c index d611c1b..b141a41 100644 --- a/src/connections/tls.c +++ b/src/connections/tls.c @@ -29,7 +29,7 @@ #include #endif -#include +#include "secutils/operators.h" bool TLS_init(void) { diff --git a/src/credentials/cert.c b/src/credentials/cert.c index bc16638..6886d24 100644 --- a/src/credentials/cert.c +++ b/src/credentials/cert.c @@ -19,7 +19,7 @@ #include #include -#include +#include "secutils/operators.h" X509 *CERT_load(const char *file, OPTIONAL const char *source, diff --git a/src/credentials/credentials.c b/src/credentials/credentials.c index 58fdb64..df3218e 100644 --- a/src/credentials/credentials.c +++ b/src/credentials/credentials.c @@ -27,7 +27,7 @@ #include #include -#include +#include "secutils/operators.h" /* this type is part of the genCMPClient API */ struct credentials diff --git a/src/credentials/key.c b/src/credentials/key.c index c0f2314..d57e4e1 100644 --- a/src/credentials/key.c +++ b/src/credentials/key.c @@ -20,7 +20,7 @@ #include #include -#include +#include "secutils/operators.h" EVP_PKEY* KEY_new(const char* spec) { diff --git a/src/credentials/store.c b/src/credentials/store.c index 328ad07..339d558 100644 --- a/src/credentials/store.c +++ b/src/credentials/store.c @@ -26,7 +26,7 @@ #include #include -#include +#include "secutils/operators.h" typedef struct STORE_ex_st { diff --git a/src/credentials/trusted.c b/src/credentials/trusted.c index ef07eda..d862854 100644 --- a/src/credentials/trusted.c +++ b/src/credentials/trusted.c @@ -20,7 +20,7 @@ #include #include -#include +#include "secutils/operators.h" static const char* config_file(void) { diff --git a/src/credentials/verify.c b/src/credentials/verify.c index fcfb263..d2be773 100644 --- a/src/credentials/verify.c +++ b/src/credentials/verify.c @@ -23,7 +23,7 @@ #include #include -#include +#include "secutils/operators.h" bool STORE_CTX_tls_active(const X509_STORE_CTX* ctx) @@ -183,8 +183,7 @@ bool verify_cb_cert(X509_STORE_CTX* store_ctx, X509* cert, int err) return verify_cb != 0 and (*verify_cb)(0, store_ctx) != 0; } -int CREDENTIALS_verify_cert(OPTIONAL uta_ctx* uta_ctx, X509* cert, - OPTIONAL const STACK_OF(X509) * untrusted_certs, X509_STORE* trust_store) +int CREDENTIALS_verify_cert(X509* cert, OPTIONAL const STACK_OF(X509) * untrusted_certs, X509_STORE* trust_store) { int result = -1; X509_STORE_CTX* store_ctx = 0; diff --git a/src/crypto/crypto.c b/src/crypto/crypto.c index 5c6a8c8..da0b263 100644 --- a/src/crypto/crypto.c +++ b/src/crypto/crypto.c @@ -13,7 +13,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -#include +#include "secutils/operators.h" #include #include diff --git a/src/storage/files.c b/src/storage/files.c index 771b23f..bc90d96 100644 --- a/src/storage/files.c +++ b/src/storage/files.c @@ -33,7 +33,7 @@ _Pragma("GCC diagnostic ignored \"-Wdeprecated-declarations\"") #include # include -#include +#include "secutils/operators.h" static file_format_t adjust_format(const char* * file, file_format_t format, bool engine_ok) { diff --git a/src/storage/files_dv.c b/src/storage/files_dv.c index 576cf1e..01f6cba 100644 --- a/src/storage/files_dv.c +++ b/src/storage/files_dv.c @@ -28,7 +28,7 @@ static const char* const DV_SECTION = "dv"; /*! name of DVFILE section with DV s #include #include -#include +#include "secutils/operators.h" /* Get device-specific password (base64 encoded) */ static bool getBase64Password(OPTIONAL uta_ctx* ctx, const unsigned char* dv, char* pw) diff --git a/src/storage/files_icv.c b/src/storage/files_icv.c index 958e755..3f2b402 100644 --- a/src/storage/files_icv.c +++ b/src/storage/files_icv.c @@ -13,15 +13,16 @@ * SPDX-License-Identifier: Apache-2.0 */ +#include #include +#include #include #include #include #include -#include -#include +#include "secutils/operators.h" #define ICVLEN 16 diff --git a/src/storage/uta_api.c b/src/storage/uta_api.c index e2a910f..fc6c064 100644 --- a/src/storage/uta_api.c +++ b/src/storage/uta_api.c @@ -21,7 +21,7 @@ #include #include -#include +#include "secutils/operators.h" #if DVLEN not_eq UTA_LEN_DV_V1 #error DVLEN not_eq UTA_LEN_DV_V1 /* mismatch with uta.h */ diff --git a/src/util/extensions.c b/src/util/extensions.c index 4b5453b..14d2730 100644 --- a/src/util/extensions.c +++ b/src/util/extensions.c @@ -16,7 +16,7 @@ #include "util/extensions.h" #include "util/log.h" -#include +#include "secutils/operators.h" X509_EXTENSIONS* EXTENSIONS_new(void) diff --git a/src/util/log.c b/src/util/log.c index f8ff0fa..5b45bf7 100644 --- a/src/util/log.c +++ b/src/util/log.c @@ -16,12 +16,12 @@ #include #include -#include - #include #include +#include "secutils/operators.h" + // use the GNU-specific `strerror_r` // https://man7.org/linux/man-pages/man3/strerror.3.html : SYNOPSIS #define _GNU_SOURCE diff --git a/src/util/util.c b/src/util/util.c index 5c1d3c3..e793da9 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -24,7 +24,7 @@ #include -#include +#include "secutils/operators.h" int UTIL_atoint(const char* str) { From d6c3fbb1fc21de072e4af4d719b127e53c8c1922 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20B=C3=A1rta?= Date: Tue, 16 Aug 2022 19:14:04 +0200 Subject: [PATCH 2/2] Fix version number in CMakeLists.txt. --- CMakeLists.txt | 5 +++-- Makefile | 4 +--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index ebef363..6c6d476 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -25,8 +25,9 @@ if (NOT YOCTO_BUILD) endif() -if(NOT DEFINED SECUTILS_VERSION ) - set(SECUTILS_VERSION 0.9) +if(NOT DEFINED SECUTILS_VERSION) + # must be kept in sync with latest version in debian/changelog and with VERSION in Makefile + set(SECUTILS_VERSION 1.0) endif() set(INCLUDE_DIRS ${PROJECT_SOURCE_DIR}/include diff --git a/Makefile b/Makefile index 41ed47d..c654eea 100644 --- a/Makefile +++ b/Makefile @@ -18,10 +18,8 @@ ROOTFS ?= $(DESTDIR)$(prefix) OUT_DIR ?= . +# must be kept in sync with latest version in debian/changelog and with SECUTILS_VERSION in CMakeLists.txt VERSION=1.0 -# must be kept in sync with latest version in debian/changelog -# PACKAGENAME=libsecutils -# DIRNAME=$(PACKAGENAME)-$(VERSION) SHELL=bash # This is needed for supporting extended file name globbing