diff --git a/CMakeLists.txt b/CMakeLists.txt index 03b08b5f..eab33e0d 100755 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -247,6 +247,7 @@ if(NOT TARGET clean_all) COMMAND ${CMAKE_BUILD_TOOL} clean COMMAND find . -name "*.o" -o -regex "./libsecutils[0-9-].*" | xargs rm COMMAND rm -rf debian/{tmp,libsecutils,libsecutils-dev,libsecutils-bin} + COMMAND rm -rf ${CMAKE_CURRENT_BINARY_DIR}/src/${PROJECT_NAME}.pc COMMAND find . -type d -name "build" | xargs -r rm -r # cowardly not doing rm -r ${CMAKE_BINARY_DIR} diff --git a/README.md b/README.md index 5f21baae..9cc92116 100644 --- a/README.md +++ b/README.md @@ -48,11 +48,11 @@ also on a virtual machine or the Windows Subsystem for Linux ([WSL](https://docs and with MacOS. The following network and development tools are needed or recommended. -* Git (for getting the software, tested versions include 2.7.2, 2.11.0, 2.20, 2.34.1, 2.48.0) -* CMake (for using [`CMakeLists.txt`](CMakeLists.txt), tested versions include 3.18.4, 3.22.1, 3.27.7, 3.31.5) +* Git (for getting the software, tested versions include 2.7.2, 2.11.0, 2.20, 2.34.1, 2.48.0, 2.53.0) +* CMake (for using [`CMakeLists.txt`](CMakeLists.txt), tested versions include 3.18.4, 3.22.1, 3.27.7, 3.31.5, 4.3.0) * GNU make (tested versions include 3.81, 4.1, 4.2.1, 4.3) -* GNU C compiler (gcc, tested versions include 5.4.0, 7.3.0, 8.3.0, 10.2.1, 11.4.0, 12.2.0) - or clang (tested versions include 14.0.3, 17.0.3, 19.1.1) +* GNU C compiler (gcc, tested versions include 5.4.0, 7.3.0, 8.3.0, 10.2.1, 11.4.0, 12.2.0, 13.3.0) + or clang (tested versions include 14.0.3, 17.0.3, 19.1.1, 22.1.1) The following OSS components are used. * OpenSSL development edition, at least version 3.0. Tested, among others, diff --git a/src/libsecutils/include/secutils/certstatus/cdp_util.h b/src/libsecutils/include/secutils/certstatus/cdp_util.h index 4be309e2..c34215ae 100644 --- a/src/libsecutils/include/secutils/certstatus/cdp_util.h +++ b/src/libsecutils/include/secutils/certstatus/cdp_util.h @@ -5,8 +5,6 @@ * * @copyright Copyright (c) Siemens Mobility GmbH, 2021 * -* @author David von Oheimb -* * This work is licensed under the terms of the Apache Software License * 2.0. See the COPYING file in the top-level directory. * diff --git a/src/libsecutils/include/secutils/certstatus/crl_mgmt.h b/src/libsecutils/include/secutils/certstatus/crl_mgmt.h index a68fc7c3..1eac35fa 100644 --- a/src/libsecutils/include/secutils/certstatus/crl_mgmt.h +++ b/src/libsecutils/include/secutils/certstatus/crl_mgmt.h @@ -5,8 +5,6 @@ * * @copyright Copyright (c) Siemens Mobility GmbH, 2021 * -* @author David von Oheimb -* * This work is licensed under the terms of the Apache Software License * 2.0. See the COPYING file in the top-level directory. * diff --git a/src/libsecutils/include/secutils/certstatus/crls.h b/src/libsecutils/include/secutils/certstatus/crls.h index b9ec53d7..3ac31e70 100644 --- a/src/libsecutils/include/secutils/certstatus/crls.h +++ b/src/libsecutils/include/secutils/certstatus/crls.h @@ -42,6 +42,7 @@ bool CRL_check(const char *src, OPTIONAL X509_CRL *crl, OPTIONAL const X509_VERI * @brief retrieve CRL in DER format (ASN.1) from given CRL distribution point * * @param url the location of the CDP + * @note when given an https URL, uses trivial unauthenticated TLS * @param timeout number of seconds the HTTP transaction may take, or 0 for infinite or -1 for default * @param max_resp_len the maximal size of the response message, or 0 for the OpenSSL default: 100 kiB * @param desc description of the CRL to use for any error messages, or null @@ -65,6 +66,8 @@ int check_cert_crls(X509_STORE_CTX* ctx, OPTIONAL STACK_OF(X509_CRL) * crls, con /*! * @brief check the revocation status of the certificate at current error depth in ctx using CDPs + * @note This does not limit the use of URLs taken from CDP entries in the certificate. + * Thus any referenced online endpoints and local file locations may get accessed. * * @param ctx pointer to verification context structure including the cert to check * @return 1 on success, 0 on rejection (i.e., cert revoked), -2 on no CRL available, -1 on other error diff --git a/src/libsecutils/include/secutils/certstatus/ocsp.h b/src/libsecutils/include/secutils/certstatus/ocsp.h index 7cc164f4..fa500d80 100644 --- a/src/libsecutils/include/secutils/certstatus/ocsp.h +++ b/src/libsecutils/include/secutils/certstatus/ocsp.h @@ -32,6 +32,7 @@ * @brief obtain OCSP response from given OCSP responder * * @param url the location of the OCSP responder + * @note when given an https URL, uses trivial unauthenticated TLS * @param timeout number of seconds the HTTP transaction may take, or 0 for infinite or -1 for default * @param req the OCSP request to send * @param desc description of contents to use for any error messages, or null @@ -45,6 +46,7 @@ static const int OCSP_DEFAULT_TIMEOUT = 10; /* in seconds */ /*! * @brief check cert status using the given OCSP response * @note the OCSP response may be obtained using an OCSP request or OCSP stapling + * @note at this level, any nonce and cert ID relating request and response cannot be checked * * @param ts pointer to trust store containing verification parameters * @param untrusted a stack of certs that may be used for chain building, or null @@ -59,6 +61,10 @@ int check_ocsp_resp(X509_STORE* ts, STACK_OF(X509) *untrusted, /*! * @brief check cert revocation status via OCSP. * @note tries using any AIA entries (if enabled) then try any given fallback OCSP responder URLs, in the given order + * @note This does not limit the use of URLs taken from AIA entries in the certificate. + * Thus any referenced online endpoints and local file locations may get accessed. + * @note to avoid performance penalty on OCSP responders, does not use nonce for + * replay protection when retrieving OCSP response unless defined SECUTILS_OCSP_USE_NONCE * * @param ctx verification context containing verification parameters etc. * @param untrusted a stack of certs that may be used for chain building, or null diff --git a/src/libsecutils/include/secutils/connections/http.h b/src/libsecutils/include/secutils/connections/http.h index 1282cc1f..c95aadb7 100644 --- a/src/libsecutils/include/secutils/connections/http.h +++ b/src/libsecutils/include/secutils/connections/http.h @@ -26,6 +26,8 @@ * @brief obtain ASN.1 response from given server * * @param url the location of the server + * @note when given an https URL, uses trivial unauthenticated TLS - + * so use with care; this is okay for retrieving CRLs and OCSP responses * @param timeout number of seconds the HTTP transaction may take, or 0 for infinite * @param max_resp_len the maximal size of the response message, or 0 for the default: 100k * @param content_type the content type of the request, or null diff --git a/src/libsecutils/include/secutils/credentials/credentials.h b/src/libsecutils/include/secutils/credentials/credentials.h index 578847ef..b88d51e9 100644 --- a/src/libsecutils/include/secutils/credentials/credentials.h +++ b/src/libsecutils/include/secutils/credentials/credentials.h @@ -153,7 +153,8 @@ void CREDENTIALS_free(OPTIONAL CREDENTIALS* creds); /*!***************************************************************************** * @brief load asymmetric credentials from the given file(s) and optionally crypto engine - * @note If used, encryption indirectly also protects integrity&authenticity of file-based storage. + * @note If used, password-based decryption indirectly + * also checks integrity&authenticity of file-based storage. * * @param certs name of file holding certificate and optional chain, or null * @param key name of file holding private key or an engine key identifier, or null @@ -173,9 +174,9 @@ CREDENTIALS* CREDENTIALS_load(OPTIONAL const char* certs, OPTIONAL const char* k /*!***************************************************************************** - * @brief load asymmetric credentials from the given file(s) and optionally using DV-based integrity&authenticity - *protection - * @note If used, encryption indirectly also protects integrity&authenticity of file-based storage. + * @brief load asymmetric credentials from the given file(s), + * using DV-based decryption and integrity&authenticity check + * @note If SECUTILS_USE_UTA is not set, cannot derive secure password from DV * * @param certs name of file holding certificate and optional chain, or null * @param key name of file holding private key, or null @@ -191,7 +192,8 @@ CREDENTIALS* CREDENTIALS_load_dv(OPTIONAL const char* certs, OPTIONAL const char /*!***************************************************************************** * @brief save asymmetric credentials (except key held in crypto engine or provider) to the given file(s) - * @note If used, encryption indirectly also protects integrity&authenticity of file-based storage. + * @note If used, password-based encryption indirectly + * also protects integrity&authenticity of file-based storage. * * @param creds pointer to the credential structure to save * @param certs name of file to store certificates, or null. Any previous contents are overwritten. @@ -214,8 +216,9 @@ bool CREDENTIALS_save(const CREDENTIALS* creds, OPTIONAL const char* certs, OPTI /*!***************************************************************************** - * @brief save asymmetric credentials to the given file(s) - * @note If used, encryption indirectly also protects integrity&authenticity of file-based storage. + * @brief save asymmetric credentials to the given file(s), + * using DV-based encryption and integrity&authenticity protection + * @note If SECUTILS_USE_UTA is not set, cannot derive secure password from DV * * @param creds pointer to the credential structure to save * @param certs name of file to store certificates, or null. Any previous contents are overwritten. @@ -235,7 +238,7 @@ static const char* const CREDS_DIR_DEFAULT = "certs/creds"; /*!***************************************************************************** * @brief obtain credentials of the given component (based on its ID) from PKCS#12 file * @note file name is derived from CREDS_DIR_ENV (defaulting to CREDS_DIR_DEFAULT), cid, and extension ".p12". - * @note uses DV-based encryption, which also protects integrity&authenticity. + * @note does not use DV-based protection * * @param cid identifier of the component credentials * @return pointer to a new CREDENTIALS structure, or null on error @@ -245,7 +248,7 @@ CREDENTIALS* CREDENTIALS_get(component_creds_id cid); /*!***************************************************************************** * @brief store credentials of the given component (based on its ID) in PKCS#12 format * @note file name is derived from CREDS_DIR_ENV (defaulting to CREDS_DIR_DEFAULT), cid, and extension ".p12". - * @note uses DV-based encryption, which also protects integrity&authenticity. + * @note does not use DV-based protection * * @param cid identifier of the component credentials * @param creds credentials to store diff --git a/src/libsecutils/include/secutils/credentials/store.h b/src/libsecutils/include/secutils/credentials/store.h index 4d6f674e..6b92c07a 100644 --- a/src/libsecutils/include/secutils/credentials/store.h +++ b/src/libsecutils/include/secutils/credentials/store.h @@ -85,9 +85,13 @@ const char *STORE_get0_desc(OPTIONAL const X509_STORE *store); * @param stapling enable OCSP stapling, which makes sense only for TLS * @param crls (optional) provide a list of CRLS to be added to the store and enable CRL-based checks * @param use_CDP enable using HTTP CDP entries in certificates and enable CRL-based status checking + * @note The use of URLs taken from CDP entries in certificates is not limited. + * Thus any referenced online endpoints and local file locations may get accessed. * @param cdps (optional) provide fallback CDP URL(s) and enable CRL-based status checking * @param crls_timeout number of seconds fetching a CRL may take, or 0 for infinite or -1 for default (= 10) * @param use_AIA enable using AIA OCSP responder entries in certificates and enable OCSP-based status checking + * @note The use of URLs taken from AIA entries in certificates is not limited. + * Thus any referenced online endpoints and local file locations may get accessed. * @param ocsp (optional) provides fallback OCSP responder URL(s) and enable OCSP-based status checking * @param ocsp_timeout number of seconds getting an OCSP response may take, or 0 for infinite or -1 for default (= 10) * @return true on success, false on failure @@ -135,6 +139,9 @@ X509_CRL *STORE_fetch_crl(const X509_STORE *store, OPTIONAL const char *url, int /*! * @brief create or extend cert store structure with any given cert(s) * @note sets CREDENTIALS_print_cert_verify_cb() enabling diagnostic log output + * @note This turns X509_V_ERR_CERT_NOT_YET_VALID into just a warning, + * effectively accepting not yet valid certificates, which are considered OK here. + * * @note use in addition STORE_set_parameters() to enable certificate status checks * * @param store certificate store to be extended if not null @@ -157,6 +164,7 @@ X509_STORE* STORE_create(OPTIONAL X509_STORE* store, OPTIONAL const X509* cert, * @param vpm verification parameters, or null, governing if and how to check cert times, * depending on X509_V_FLAG_USE_CHECK_TIME and X509_V_FLAG_NO_CHECK_TIME * @param ctx (optional) pointer to UTA context for checking file integrity&authenticity using ICV + * @note if ctx is null or SECUTILS_USE_ICV not defined, no ICV-based check is done * @return true on success, false on error * @note For loaded certs their validity period and their CA flag are checked. * Failures are logged as a warning if vpm is null, otherwise as an error. @@ -177,6 +185,7 @@ bool STORE_load_more_check(X509_STORE** pstore, const char* file, * @param vpm verification parameters, or null, governing if and how to check cert times, * depending on X509_V_FLAG_USE_CHECK_TIME and X509_V_FLAG_NO_CHECK_TIME * @param ctx (optional) pointer to UTA context for checking file integrity&authenticity using ICV + * @note if ctx is null or SECUTILS_USE_ICV not defined, no ICV-based check is done * @return pointer to a new X509_STORE structure, or null on error * @note For loaded certs their validity period and their CA flag are checked. * Failures are logged as a warning if vpm is null, otherwise as an error. @@ -198,6 +207,7 @@ X509_STORE* STORE_load_check(const char* files, OPTIONAL const char* desc, * @param vpm verification parameters, or null, governing if and how to check cert times, * depending on X509_V_FLAG_USE_CHECK_TIME and X509_V_FLAG_NO_CHECK_TIME * @param ctx (optional) pointer to UTA context for checking file integrity&authenticity using ICV + * @note if ctx is null or SECUTILS_USE_ICV not defined, no ICV-based check is done * @note at least one valid certificate file must be found in all tested directories * @return true on success, false on error/failure * @note For loaded certs their validity period and their CA flag are checked. @@ -217,7 +227,8 @@ bool STORE_load_check_dir(X509_STORE** pstore, const char* trust_dir, * @param crl_dir directory where to search for CRLs * @param desc description of CRLs to use for error reporting, or null * @param recursive if true, use recursive search in subdirectories - * @param ctx pointer to UTA context for checking file integrity&authenticity using ICV, or null + * @param ctx (optional) pointer to UTA context for checking file integrity&authenticity using ICV + * @note if ctx is null or SECUTILS_USE_ICV not defined, no ICV-based check is done * @note at least one valid CRL file must be found in each visited directory * @return true on success, false on error/failure ******************************************************************************/ diff --git a/src/libsecutils/include/secutils/credentials/trusted.h b/src/libsecutils/include/secutils/credentials/trusted.h index 673b23c0..74d1ba56 100644 --- a/src/libsecutils/include/secutils/credentials/trusted.h +++ b/src/libsecutils/include/secutils/credentials/trusted.h @@ -37,6 +37,7 @@ static const char* const TRUST_CONFIG_ENTRY_CRLS = "crls"; * @param cid identifier of the component, which indicates the config file section to use, or TRUST_CONFIG_SECTION_DEFAULT if it is 0 * @param vpm OpenSSL certificate verification parameters to be taken over, or null for default * @param ctx (optional) pointer to UTA context for checking file integrity&authenticity using ICV + * @note if ctx is null or SECUTILS_USE_ICV not defined, no ICV-based check is done * @return pointer to a new CREDENTIALS structure, or null on error *******************************************************************************/ X509_STORE* CREDENTIALS_get_trust_store(component_creds_id cid, OPTIONAL X509_VERIFY_PARAM* vpm, OPTIONAL uta_ctx* ctx); diff --git a/src/libsecutils/include/secutils/credentials/verify.h b/src/libsecutils/include/secutils/credentials/verify.h index 8fda3003..5c86094c 100644 --- a/src/libsecutils/include/secutils/credentials/verify.h +++ b/src/libsecutils/include/secutils/credentials/verify.h @@ -46,6 +46,9 @@ bool STORE_CTX_tls_active(const X509_STORE_CTX* ctx); * @param ctx_x509 pointer to structure containing certificate verification options like trusted certs * @return 0 if and only if the cert verification is considered failed * + * @note This callback turns X509_V_ERR_CERT_NOT_YET_VALID into just a warning, + * effectively accepting not yet valid certificates, which are considered OK here. + * * @note OpenSSL's X509_verify_cert function calls this function * during certificate verification whenever a problem has been * found and on success at the end of the verification to give an opportunity @@ -78,6 +81,9 @@ bool verify_cb_cert(X509_STORE_CTX* store_ctx, X509* cert, int err); * @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 + * @note This function turns X509_V_ERR_CERT_NOT_YET_VALID into just a warning, + * effectively accepting not yet valid certificates, which are considered OK here. + * * @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 valid cert diff --git a/src/libsecutils/include/secutils/crypto/crypto.h b/src/libsecutils/include/secutils/crypto/crypto.h index ad60724e..95aea653 100644 --- a/src/libsecutils/include/secutils/crypto/crypto.h +++ b/src/libsecutils/include/secutils/crypto/crypto.h @@ -5,7 +5,7 @@ * * @copyright Copyright (c) Siemens Mobility GmbH, 2021 * -* @author David von Oheimb +* @author Martin Vitek * * This work is licensed under the terms of the Apache Software License * 2.0. See the COPYING file in the top-level directory. @@ -78,11 +78,11 @@ ssize_t AESGCM_decrypt(EVP_CIPHER_CTX* osslctx, uint8_t* plain_buff, size_t plai /*! - * @brief The function finalizes encryption and store tag (integrity protection value). + * @brief The function finalizes encryption and stores tag (integrity protection value). * - * @param osslctx pointer to context of decryption operation + * @param osslctx pointer to context of encryption operation * @param tag_buff pointer to buffer used to store tag - * @param tag_len size of resulting tag to be stored in tag_buff + * @param tag_len size of resulting tag to be stored in tag_buff, should be EVP_GCM_TLS_TAG_LEN * @note If tag_buff is set to null and tag_len is set to 0 then tag will not be generated * * @return true on success @@ -96,7 +96,7 @@ bool AESGCM_encrypt_final(EVP_CIPHER_CTX* osslctx, uint8_t* tag_buff, size_t tag * * @param osslctx pointer to context of decryption operation * @param tag pointer to tag - * @param tag_len size of tag + * @param tag_len size of tag, should be EVP_GCM_TLS_TAG_LEN * @note If tag_buff is set to null and tag_len is set 0 then authenticity will not be checked * @param plain pointer to plain text obtained by calling AESGCM_decrypt or null * @note on failure the function cleans plain text if parameter plain is not null diff --git a/src/libsecutils/include/secutils/operators.h b/src/libsecutils/include/secutils/operators.h index f8c9d985..8d45d89c 100644 --- a/src/libsecutils/include/secutils/operators.h +++ b/src/libsecutils/include/secutils/operators.h @@ -5,7 +5,7 @@ * * @copyright Copyright (c) Siemens Mobility GmbH, 2021 * -* @author David von Oheimb +* @author Martin Barta * * This work is licensed under the terms of the Apache Software License * 2.0. See the COPYING file in the top-level directory. diff --git a/src/libsecutils/include/secutils/storage/files_dv.h b/src/libsecutils/include/secutils/storage/files_dv.h index bf875420..8f9f4cae 100644 --- a/src/libsecutils/include/secutils/storage/files_dv.h +++ b/src/libsecutils/include/secutils/storage/files_dv.h @@ -31,6 +31,7 @@ /*! * @brief get file encryption password from UTA using derivation value based on file path name + * @note If SECUTILS_USE_UTA is not set, cannot derive secure password * * @param pass_buf place to store the derived password as base64 encoded string, must be at least MAX_UTA_PASS_LEN chars * long @@ -55,6 +56,7 @@ bool FILES_get_dv(const char* filename, unsigned char* dv_out); /*! * @brief store private key in given file and format, optionally using DV-based password + * @note If SECUTILS_USE_UTA is not set, cannot derive secure password from DV * * @param pkey private key to save * @param file (path) name of the output file. Any previous contents are overwritten. @@ -70,6 +72,7 @@ bool FILES_store_key_dv(const EVP_PKEY* pkey, const char* file, file_format_t fo /*! * @brief load private key from the given file or engine with flexible format, optionally using DV-based password + * @note If SECUTILS_USE_UTA is not set, cannot derive secure password from DV * * @param key file (path) name of the input file or engine key ID * @param file_format the format to try first when reading file contents @@ -85,6 +88,7 @@ EVP_PKEY* FILES_load_key_autofmt_dv(OPTIONAL const char* key, file_format_t file /*! * @brief load certificates from the given file with flexible format, optionally using DV-based password + * @note If SECUTILS_USE_UTA is not set, cannot derive secure password from DV * * @param file (path) name of the input file * @param format the format to try first when reading file contents diff --git a/src/libsecutils/include/secutils/storage/files_icv.h b/src/libsecutils/include/secutils/storage/files_icv.h index bf278e50..e43e2ea9 100644 --- a/src/libsecutils/include/secutils/storage/files_icv.h +++ b/src/libsecutils/include/secutils/storage/files_icv.h @@ -26,6 +26,7 @@ /*! * @brief (re-)protect integrity of file (of any type that allows appending text) with ICV derived via UTA * from the absolute path name of the file + * @note If SECUTILS_USE_UTA is not set, cannot derive secure ICV * * @param ctx pointer to UTA context, which typically is part of the libsecutils context, or null * @param file (path) name of the file to be protected @@ -37,6 +38,7 @@ bool FILES_protect_icv(OPTIONAL uta_ctx* ctx, const char* file); /*! * @brief (re-)protect integrity of file (of any type that allows appending text) with ICV derived via UTA * from the supplied path name of the file. If none is supplied, the absolute path is derived. + * @note If SECUTILS_USE_UTA is not set, cannot derive secure ICV * * @param ctx pointer to UTA context, which typically is part of the libsecutils context, or null * @param file (path) name of the file to be protected @@ -49,6 +51,7 @@ bool FILES_protect_icv_at(OPTIONAL uta_ctx* ctx, const char* file, const char* l /*! * @brief (re-)protect integrity of file, if it has suffix .pem, .crt, or .cnf (unless SECUTILS_NO_ICV), with ICV * derived via UTA + * @note If SECUTILS_USE_UTA is not set, cannot derive secure ICV * * @param file (path) name of the file to be protected * @param ctx pointer to UTA context, which typically is part of the libsecutils context, or null @@ -60,6 +63,7 @@ bool FILES_protect_icv_config_trusted(const char* file, OPTIONAL uta_ctx* ctx); /*! * @brief check integrity of file (of any type that allows appending text) using ICV derived via UTA * from the absolute path name of the file + * @note If SECUTILS_USE_UTA is not set, cannot derive secure ICV * * @param ctx pointer to UTA context, which typically is part of the libsecutils context, or null * @param file (path) name of the file to be checked @@ -71,6 +75,7 @@ bool FILES_check_icv(OPTIONAL uta_ctx* ctx, const char* file); /*! * @brief check integrity of file (of any type that allows appending text) using ICV derived via UTA * from the supplied path name of the file. If none is supplied, the absolute path is derived. + * @note If SECUTILS_USE_UTA is not set, cannot derive secure ICV * * @param ctx pointer to UTA context, which typically is part of the libsecutils context, or null * @param file (path) name of the file to be checked @@ -82,6 +87,7 @@ bool FILES_check_icv_at(OPTIONAL uta_ctx* ctx, const char* file, const char* loc /*! * @brief load a certificate from the given PEM file, checking ICV-based protection + * @note If SECUTILS_USE_UTA is not set, cannot derive secure ICV * * @param ctx pointer to UTA context, which typically is part of the libsecutils context, or null * @param file (path) name of the input file @@ -93,6 +99,7 @@ X509* FILES_load_cert_pem_icv(OPTIONAL uta_ctx* ctx, const char* file, OPTIONAL /*! * @brief store the given certificate in given PEM file, with ICV-based protection + * @note If SECUTILS_USE_UTA is not set, cannot derive secure ICV * * @param ctx pointer to UTA context, which typically is part of the libsecutils context, or null * @param cert certificate to save @@ -105,6 +112,7 @@ bool FILES_store_cert_pem_icv(OPTIONAL uta_ctx* ctx, const X509* cert, const cha /*! * @brief store the given certificate in given PEM file and optionally add ICV-based protection + * @note If SECUTILS_USE_UTA is not set, cannot derive secure ICV * * @param ctx pointer to UTA context, which typically is part of the libsecutils context, or null * @param cert certificate to save @@ -119,6 +127,7 @@ bool FILES_store_cert_pem(OPTIONAL uta_ctx* ctx, const X509* cert, const char* f /*! * @brief store the given CRL in given PEM file, with ICV-based protection + * @note If SECUTILS_USE_UTA is not set, cannot derive secure ICV * * @param ctx pointer to UTA context, which typically is part of the libsecutils context, or null * @param crl CRL to save diff --git a/src/libsecutils/include/secutils/util/util.h b/src/libsecutils/include/secutils/util/util.h index 5db947e4..2be7a343 100644 --- a/src/libsecutils/include/secutils/util/util.h +++ b/src/libsecutils/include/secutils/util/util.h @@ -359,7 +359,7 @@ bool UTIL_get_random(void *buf, size_t len); * is needed for the copied string. If the size_needed is null the size is not returned. * * @return number of bytes excluding the terminating NUL that have been copied to destination, - * -1 on error, e.g., the source is null or + * -1 on error, e.g., the source is null or the length is > INT_MAX or * the destination is not null and the buffer size is 0. */ int UTIL_safe_string_copy(const char *source, OPTIONAL char *destination, diff --git a/src/libsecutils/src/certstatus/cdp_util.c b/src/libsecutils/src/certstatus/cdp_util.c index 490ab040..2c57a8eb 100644 --- a/src/libsecutils/src/certstatus/cdp_util.c +++ b/src/libsecutils/src/certstatus/cdp_util.c @@ -5,8 +5,6 @@ * * @copyright Copyright (c) Siemens Mobility GmbH, 2021 * -* @author David von Oheimb -* * This work is licensed under the terms of the Apache Software License * 2.0. See the COPYING file in the top-level directory. * @@ -85,10 +83,14 @@ const char *CDP_get_uri_from_general_names( protocol prefixes, the distribution point has been found */ uri = GENERAL_NAME_get0_value(name_entry, >ype); - if(gtype == GEN_URI && ASN1_STRING_length(uri) > 7) - { - /* uri is of type ASN1_STRING */ + if (gtype == GEN_URI) { const char *asn1_uri = (const char *)ASN1_STRING_get0_data(uri); + int len = ASN1_STRING_length(uri); + + if (asn1_uri == NULL || len <= 6 || memchr(asn1_uri, '\0', (size_t)len) != NULL) { + LOG(FL_WARN, "ignoring CDP URI that is invalid, too short, or has embedded NUL byte"); + continue; + } if (CONN_IS_HTTP(asn1_uri) || CONN_IS_HTTPS(asn1_uri)) { return asn1_uri; diff --git a/src/libsecutils/src/certstatus/crl_mgmt.c b/src/libsecutils/src/certstatus/crl_mgmt.c index 88b859e0..5d74666b 100644 --- a/src/libsecutils/src/certstatus/crl_mgmt.c +++ b/src/libsecutils/src/certstatus/crl_mgmt.c @@ -5,8 +5,6 @@ * * @copyright Copyright (c) Siemens Mobility GmbH, 2021 * -* @author David von Oheimb -* * This work is licensed under the terms of the Apache Software License * 2.0. See the COPYING file in the top-level directory. * @@ -128,17 +126,19 @@ static X509_CRL *get_crl_from_cache(const char * cachefile) ASN1_TIME *now = ASN1_TIME_new(); ASN1_TIME_set(now, time(NULL)); const ASN1_TIME *lastUpdate = X509_CRL_get0_lastUpdate(crl); - const ASN1_TIME *nextUpdate = X509_CRL_get0_nextUpdate(crl); + const ASN1_TIME *nextUpdate = X509_CRL_get0_nextUpdate(crl); /* may be NULL */ CDP_get_x509_time(now, time_buf, sizeof(time_buf)); LOG(FL_TRACE, "current time: %s", time_buf); CDP_get_x509_time(lastUpdate, time_buf, sizeof(time_buf)); LOG(FL_TRACE, "CRL lastUpdate: %s", time_buf); - CDP_get_x509_time(nextUpdate, time_buf, sizeof(time_buf)); - LOG(FL_TRACE, "CRL nextUpdate: %s", time_buf); + if (nextUpdate != NULL) { + CDP_get_x509_time(nextUpdate, time_buf, sizeof(time_buf)); + LOG(FL_TRACE, "CRL nextUpdate: %s", time_buf); + } int isBeforeStart = ASN1_TIME_compare(now, lastUpdate) < 0; - int isAfterEnd = ASN1_TIME_compare(nextUpdate, now) <= 0; + int isAfterEnd = nextUpdate != NULL && ASN1_TIME_compare(nextUpdate, now) <= 0; int isAfterPublish = 0; int nextPublishIdx = X509_CRL_get_ext_by_NID(crl, NID_crl_next_publish, 0); @@ -152,7 +152,15 @@ static X509_CRL *get_crl_from_cache(const char * cachefile) #if OPENSSL_VERSION_NUMBER < 0x10101000L LOG(FL_ERR, "CRL nextPublish extension is present, but ASN1_TIME_set_string_X509 is not supported for OpenSSL version <1.1, sorry"); #else - const char *nextPublishString = (const char*)ASN1_STRING_get0_data(data); + const char *nextPublishString = (const char *)ASN1_STRING_get0_data(data); + int len = ASN1_STRING_length(data); + + if (nextPublishString == NULL || len <= 0 + || memchr(nextPublishString, '\0', (size_t)len) != NULL) { + LOG(FL_WARN, "ignoring nextPublishString that is invalid or has embedded NUL byte"); + goto ignore; + } + while (*nextPublishString && !((*nextPublishString) & 0xE0)) { ++nextPublishString; } @@ -165,6 +173,7 @@ static X509_CRL *get_crl_from_cache(const char * cachefile) LOG(FL_ERR, "CRL nextPublish extension is present, but time cannot be determined"); } ASN1_TIME_free(nextPublish); + ignore: ; #endif } } @@ -232,7 +241,8 @@ static X509_CRL *get_crl_by_download_or_from_cache(const CRLMGMT_DATA *data, } crl = CONN_load_crl_http(url, timeout, data->max_download_size, desc); - if (usecache && crl != NULL) { + if (usecache && crl != NULL + && X509_CRL_get0_nextUpdate(crl) != NULL /* otherwise would not know when to remove it again */) { put_crl_into_cache(crl, cachefile); } return crl; diff --git a/src/libsecutils/src/certstatus/crls.c b/src/libsecutils/src/certstatus/crls.c index 171b7865..a1cacd1f 100644 --- a/src/libsecutils/src/certstatus/crls.c +++ b/src/libsecutils/src/certstatus/crls.c @@ -297,9 +297,14 @@ static const char* get_dp_url(DIST_POINT* dp) { gen = sk_GENERAL_NAME_value(gens, i); uri = GENERAL_NAME_get0_value(gen, >ype); - if(gtype is_eq GEN_URI and ASN1_STRING_length(uri) > 6) - { - char* uptr = (char*)ASN1_STRING_get0_data(uri); + if (gtype is_eq GEN_URI) { + const char *uptr = (const char *)ASN1_STRING_get0_data(uri); + int len = ASN1_STRING_length(uri); + + if (uptr == NULL || len <= 6 || memchr(uptr, '\0', (size_t)len) != NULL) { + LOG(FL_WARN, "ignoring CDP URI that is invalid, too short, or has embedded NUL byte"); + continue; + } if (CONN_IS_HTTP(uptr) || HAS_CASE_PREFIX(uptr, "file:")) { return uptr; @@ -381,15 +386,16 @@ int check_cert_status_cdps(X509_STORE_CTX* ctx) int n_delta = sk_DIST_POINT_num(delta_crldp); STACK_OF(DIST_POINT) *crldp = use_CDP ? X509_get_ext_d2i(cert, NID_crl_distribution_points, 0, 0) : 0; + int n_base = sk_DIST_POINT_num(crldp); char* fallback_urls = OPENSSL_strdup(cdps->urls); int timeout = cdps->timeout; int i; int res = -2; /* no CRL available, thus inconclusive */ (void)ERR_set_mark(); - if(n_delta <= 0 and sk_DIST_POINT_num(crldp) <= 0) + if(n_base <= 0 and n_delta <= 0) { - LOG(FL_DEBUG, use_CDP ? "no HTTP CDP in cert" : "cert CDP use is disabled"); + LOG(FL_DEBUG, use_CDP ? "no CDP for base or delta CRLs in cert" : "cert CDP use is disabled"); } /* Try downloading any delta CRL */ @@ -397,7 +403,7 @@ int check_cert_status_cdps(X509_STORE_CTX* ctx) X509_CRL* delta_crl = 0; for(i = 0; delta_crl is_eq 0 and i < n_delta; i++) { - const char* url = get_dp_url(sk_DIST_POINT_value(crldp, i)); + const char* url = get_dp_url(sk_DIST_POINT_value(delta_crldp, i)); if(url not_eq 0) { LOG(FL_DEBUG, "trying to load delta CRL from CDP URL:", url); @@ -414,7 +420,7 @@ int check_cert_status_cdps(X509_STORE_CTX* ctx) /* Try downloading any base CRL */ /* TODO sufficient to use the first CRL found ? */ - for(i = 0; res < 0 and i < sk_DIST_POINT_num(crldp); i++) + for(i = 0; res < 0 and i < n_base; i++) { res = try_cdp(ctx, timeout, cert, get_dp_url(sk_DIST_POINT_value(crldp, i)), 0, delta_crl, nonfinal, "HTTP CDP entry in certificate"); diff --git a/src/libsecutils/src/certstatus/ocsp.c b/src/libsecutils/src/certstatus/ocsp.c index cb92532d..5c79d332 100644 --- a/src/libsecutils/src/certstatus/ocsp.c +++ b/src/libsecutils/src/certstatus/ocsp.c @@ -185,7 +185,6 @@ static OCSP_RESPONSE* get_ocsp_resp(X509* cert, X509* issuer, { OCSP_REQUEST* req = 0; OCSP_CERTID* id_copy, *id = 0; - int res; OCSP_RESPONSE* resp = 0; OCSP_BASICRESP* br = 0; int i; @@ -203,10 +202,12 @@ static OCSP_RESPONSE* get_ocsp_resp(X509* cert, X509* issuer, goto end; } id = 0; +#ifdef SECUTILS_OCSP_USE_NONCE if(not OCSP_request_add1_nonce(req, 0, -1)) { goto end; } +#endif /* Add any extensions to the request */ for(i = 0; i < sk_X509_EXTENSION_num(exts); i++) @@ -228,18 +229,23 @@ static OCSP_RESPONSE* get_ocsp_resp(X509* cert, X509* issuer, if((br = OCSP_response_get1_basic(resp)) is_eq 0) { LOG(FL_ERR, "error getting OCSP basic response"); - goto end; + goto err; } +#ifdef SECUTILS_OCSP_USE_NONCE + int res; if((res = OCSP_check_nonce(req, br)) <= 0) { LOG(FL_ERR, res is_eq -1 ? "no nonce in OCSP response" : "nonce verification error"); - goto end; + goto err; } - if(not OCSP_resp_find_status(br, id_copy, 0, 0, 0, 0, 0)) - { - LOG(FL_ERR, "no OCSP status found matching cert ID in request"); +#endif + if (OCSP_resp_find_status(br, id_copy, 0, 0, 0, 0, 0)) goto end; - } + LOG(FL_ERR, "no OCSP status found matching cert ID in request"); + + err: + OCSP_RESPONSE_free(resp); + resp = NULL; end: OCSP_CERTID_free(id); diff --git a/src/libsecutils/src/config/config.c b/src/libsecutils/src/config/config.c index 7e122f2c..58a43460 100644 --- a/src/libsecutils/src/config/config.c +++ b/src/libsecutils/src/config/config.c @@ -60,6 +60,8 @@ CONF* CONF_load_config(OPTIONAL ossl_unused uta_ctx* ctx, const char* file) if(0 not_eq file #ifdef SECUTILS_USE_ICV and FILES_check_icv(ctx, file) not_eq 0 +#else + and (ctx == NULL || (LOG(FL_ERR, "SECUTILS_USE_ICV not defined but UTA ctx is set"), false)) #endif ) { diff --git a/src/libsecutils/src/connections/http.c b/src/libsecutils/src/connections/http.c index 179a17a0..cf98ec7f 100644 --- a/src/libsecutils/src/connections/http.c +++ b/src/libsecutils/src/connections/http.c @@ -230,6 +230,7 @@ ASN1_VALUE* CONN_load_ASN1_http(const char* url, int req_timeout, LOG(FL_ERR, "error attaching trivial SSL context"); goto err; } + LOG(FL_WARN, "using trivial unauthenticated TLS for downloading %s via %s", desc, url); # else LOG(FL_ERR, "TLS is not enabled in this build"); goto err; diff --git a/src/libsecutils/src/connections/tls.c b/src/libsecutils/src/connections/tls.c index cb205a53..f8e3433d 100644 --- a/src/libsecutils/src/connections/tls.c +++ b/src/libsecutils/src/connections/tls.c @@ -320,6 +320,7 @@ SSL* TLS_connect(SSL_CTX* ctx, const char* host, OPTIONAL const char* port, int } /* link it with BIO connection */ SSL_set_bio(ssl, conn, conn); + conn = NULL; /* as ownership has been transferred */ if(not CONN_IS_IP_ADDR(hostaddr) and not SSL_set_tlsext_host_name(ssl, hostaddr)) @@ -336,7 +337,7 @@ SSL* TLS_connect(SSL_CTX* ctx, const char* host, OPTIONAL const char* port, int err: OPENSSL_free(host_str); - /* release the intermediate connection structure */ + /* release the connection structure (unless ownership transferred to ssl) */ CONN_free(conn); /* on error, release the SSL/TLS structure */ if(0 is_eq res) diff --git a/src/libsecutils/src/credentials/store.c b/src/libsecutils/src/credentials/store.c index 5d79e20c..ef0d03cc 100644 --- a/src/libsecutils/src/credentials/store.c +++ b/src/libsecutils/src/credentials/store.c @@ -234,6 +234,12 @@ bool STORE_load_more_check(X509_STORE** pstore, const char* file, LOG_err("null pointer argument"); goto err; } +#ifndef SECUTILS_USE_ICV + if (ctx != NULL) { + LOG(FL_ERR, "SECUTILS_USE_ICV not defined but UTA ctx is set"); + return false; + } +#endif #ifdef DEBUG LOG(FL_DEBUG, "Loading %s from file '%s'", desc not_eq 0 ? desc : "?", file); @@ -398,6 +404,12 @@ bool STORE_load_crl_dir(X509_STORE* pstore, const char* crl_dir, OPTIONAL const LOG(FL_ERR, "null pointer argument"); goto err; } +#ifndef SECUTILS_USE_ICV + if (ctx != NULL) { + LOG(FL_ERR, "SECUTILS_USE_ICV not defined but UTA ctx is set"); + return false; + } +#endif p_dir = opendir(crl_dir); if(0 is_eq p_dir) diff --git a/src/libsecutils/src/crypto/crypto.c b/src/libsecutils/src/crypto/crypto.c index 4b326d20..b8b5d753 100644 --- a/src/libsecutils/src/crypto/crypto.c +++ b/src/libsecutils/src/crypto/crypto.c @@ -5,7 +5,7 @@ * * @copyright Copyright (c) Siemens Mobility GmbH, 2021 * -* @author David von Oheimb +* @author Martin Vitek * * This work is licensed under the terms of the Apache Software License * 2.0. See the COPYING file in the top-level directory. @@ -93,8 +93,27 @@ ssize_t AESGCM_encrypt(EVP_CIPHER_CTX* osslctx, uint8_t* cipher_buff, size_t cip return ret; } - int cipher_len = cipher_buff_len; - if((EVP_EncryptUpdate(osslctx, cipher_buff, &cipher_len, plain, plain_len) not_eq 1) or ((size_t)cipher_len not_eq plain_len)) + if (cipher_buff_len > INT_MAX) { + LOG(FL_ERR, "cipher_buff_len > INT_MAX"); + return ret; + } + if (plain_len > INT_MAX) { + LOG(FL_ERR, "plain_len > INT_MAX"); + return ret; + } + int cipher_len = (int)cipher_buff_len; + int block_size = EVP_CIPHER_CTX_get_block_size(osslctx); + if (block_size <= 0) { + LOG(FL_ERR, "invalid cipher context (block size: %s)", block_size); + return ret; + } + int padding_len = (block_size - ((int)plain_len % block_size)) % block_size; + if ((int)plain_len > cipher_len - padding_len) { + LOG(FL_ERR, "cipher_buff_len = %zu too small, should be at least plain_len + padding_len = %zu", + cipher_buff_len, plain_len + padding_len); + return ret; + } + if((EVP_EncryptUpdate(osslctx, cipher_buff, &cipher_len, plain, (int)plain_len) not_eq 1) or ((size_t)cipher_len not_eq plain_len)) {/* The second condition - GCM is used, hence length(plain_text) == length(cipher_text) */ LOG(FL_ERR, "Encryption failed"); return ret; @@ -117,8 +136,22 @@ ssize_t AESGCM_decrypt(EVP_CIPHER_CTX* osslctx, uint8_t* plain_buff, size_t plai return ret; } - int plain_len = plain_buff_len; - if((EVP_DecryptUpdate(osslctx, plain_buff, &plain_len, cipher, cipher_len) not_eq 1) + if (plain_buff_len > INT_MAX) { + LOG(FL_ERR, "plain_buff_len > INT_MAX"); + return ret; + } + if (cipher_len > INT_MAX) { + LOG(FL_ERR, "cipher_len > INT_MAX"); + return ret; + } + if (plain_buff_len < cipher_len) { + LOG(FL_ERR, "plain_buff_len = %zu too small, should be at least cipher_len = %zu", + plain_buff_len, cipher_len); + return ret; + } + + int plain_len = (int)plain_buff_len; + if((EVP_DecryptUpdate(osslctx, plain_buff, &plain_len, cipher, (int)cipher_len) not_eq 1) or ((size_t)plain_len not_eq cipher_len)) {/* The second condition - GCM is used hence length(plain_text) == length(cipher_text) */ LOG(FL_ERR, "Decryption failed"); diff --git a/src/libsecutils/src/storage/files_dv.c b/src/libsecutils/src/storage/files_dv.c index 6025351e..81c7c690 100644 --- a/src/libsecutils/src/storage/files_dv.c +++ b/src/libsecutils/src/storage/files_dv.c @@ -63,7 +63,7 @@ static bool getBase64Password(OPTIONAL uta_ctx* ctx, const unsigned char* dv, ch #endif len = UTIL_base64_encode_to_buf(key, TA_OUTLEN, pw, MAX_B64_CHARS_PER_BYTE * TA_OUTLEN); - LOG(FL_TRACE, "DV-based password: %s", pw); + // better not do in productive builds: LOG(FL_TRACE, "DV-based password: %s", pw); return len >= 0; } @@ -264,7 +264,7 @@ static char* get_basename(const char* path) /** * @brief returns canonicalized absolute path * @param path path which should be canonicalized - * @param canonicalized + * @param pointer to buffer for canonicalized output, MUST be at least PATH_MAX characters long * @note the pointer has to be freed by OPENSSL_free() */ static char* get_canonicalized(const char* path, char* canonicalized) @@ -276,7 +276,8 @@ static char* get_canonicalized(const char* path, char* canonicalized) if((0 not_eq dir_part) and (0 not_eq name_part)) { - if(0 is_eq realpath(dir_part, canonicalized)) + if(0 is_eq realpath(dir_part, canonicalized) + || strlen(canonicalized) + 1 + strlen(name_part) >= PATH_MAX) { ret_val = 0; } diff --git a/src/libsecutils/src/storage/files_icv.c b/src/libsecutils/src/storage/files_icv.c index 4d129b14..ba8b37b8 100644 --- a/src/libsecutils/src/storage/files_icv.c +++ b/src/libsecutils/src/storage/files_icv.c @@ -36,6 +36,7 @@ /* * derive integrity protection hash for data with given len, using name as DV for key * optionally uses ctx pointer to UTA context, which typically is part of the libsecutils context + * if SECUTILS_USE_UTA is not set, cannot derive secure ICV * if returns true, hash value is placed in buf, which must be of size ICV_HEX_LEN+1 */ static bool calculate_icv_hex(OPTIONAL uta_ctx* ctx, const void* data, size_t len, const char* name, char* buf) @@ -221,9 +222,7 @@ bool FILES_protect_icv_config_trusted(const char* file, OPTIONAL uta_ctx* ctx) int str_len = strlen(file); const char* name_tail = file + str_len - (str_len >= ext_len ? ext_len : 0); if(0 is_eq strcmp(name_tail, ".pem") or 0 is_eq strcmp(name_tail, ".crt") -#ifdef SECUTILS_USE_ICV or 0 is_eq strcmp(name_tail, ".cnf") /* OpenSSL-style config file */ -#endif ) { LOG(FL_INFO, "making sure that file '%s' has an ICV", file); diff --git a/src/libsecutils/src/util/util.c b/src/libsecutils/src/util/util.c index ca2447b5..5e2305aa 100644 --- a/src/libsecutils/src/util/util.c +++ b/src/libsecutils/src/util/util.c @@ -357,14 +357,15 @@ int UTIL_safe_string_copy(const char *source, OPTIONAL char *destination, } const char *begin = source; - size_t needed = 1; /* record the buffer size that would be needed == strlen(source) */ - while (*source != 0) { + size_t needed = 1; /* record the buffer size that would be needed == strlen(source) + 1 */ + while (*source != '\0') { needed++; - if (destination != NULL && needed < destination_len) { - *destination++ = *source++; + if (destination != NULL && needed <= destination_len) { + *destination++ = *source; } + source++; } - if (destination != NULL) { + if (destination != NULL) { *destination = '\0'; } @@ -372,7 +373,12 @@ int UTIL_safe_string_copy(const char *source, OPTIONAL char *destination, *size_needed = needed; } - return source - begin; + if (destination != NULL && needed > destination_len) { + /* source was truncated to fit destination buffer */ + return -1; + } + + return source - begin > INT_MAX ? -1 : (int)(source - begin); } /* implementation of the function url_encode */ @@ -427,7 +433,7 @@ size_t UTIL_url_encode( size_t needed = 0; /* record the size needed */ for (is=0,id=0; source[is]!=0; ++is) { - if (unreserved[(int)source[is]]) { + if (unreserved[(unsigned char)source[is]]) { /* character is part of the unreserved ones */ ++needed; if (destination && destination_len >= needed) { diff --git a/src/util/icvutil.c b/src/util/icvutil.c index 12b0900e..6981677a 100644 --- a/src/util/icvutil.c +++ b/src/util/icvutil.c @@ -5,7 +5,7 @@ * * @copyright Copyright (c) Siemens Mobility GmbH, 2021 * -* @author David von Oheimb +* @author Raphael Lisicki * * This work is licensed under the terms of the Apache Software License * 2.0. See the COPYING file in the top-level directory. @@ -33,7 +33,7 @@ int main(int argc, char *argv[]) { return ret; } #else - LOG(FL_WARN, "Not using UTA lib because SECUTILS_USE_UTA was not defined"); + LOG(FL_WARN, "Not using UTA lib because SECUTILS_USE_UTA was not defined; cannot obtain secure ICV"); #endif const char *prog = argv[0];