From 4bae382789d66df391800749fe6442d5cbebdf22 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 8 Jan 2025 14:20:30 -0800 Subject: [PATCH 1/3] Refactor: libcrmcommon: Use gnutls_transport_set_int() And gnutls_transport_get_int(). As of GnuTLS 3.1.9, we no longer have to use the _ptr() functions. The _int() functions are a simpler alternative, assuming TCP is the transport layer. As of this commit, there's no longer any real benefit to keeping pcmk__tls_get_client_sock(), except to provide a layer of abstraction so that GnuTLS library functions are called in fewer places. https://gnutls.org/manual/html_node/Setting-up-the-transport-layer.html Ref T967 Signed-off-by: Reid Wahl --- include/crm/common/tls_internal.h | 4 ++-- lib/common/tls.c | 10 ++-------- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/include/crm/common/tls_internal.h b/include/crm/common/tls_internal.h index f1cd517e7c7..0a113feb07e 100644 --- a/include/crm/common/tls_internal.h +++ b/include/crm/common/tls_internal.h @@ -84,8 +84,8 @@ int pcmk__init_tls_dh(gnutls_dh_params_t *dh_params); * \internal * \brief Initialize a new TLS session * - * \param[in] tls A TLS environment object - * \param[in] csock Connected socket for TLS session + * \param[in] tls TLS environment object + * \param[in] csock Connected TCP socket for TLS session * * \return Pointer to newly created session object, or NULL on error */ diff --git a/lib/common/tls.c b/lib/common/tls.c index 9d9652643c4..6328370525c 100644 --- a/lib/common/tls.c +++ b/lib/common/tls.c @@ -14,8 +14,6 @@ #include #include -#include // gpointer, GPOINTER_TO_INT(), GINT_TO_POINTER() - #include static char * @@ -349,8 +347,7 @@ pcmk__new_tls_session(pcmk__tls_t *tls, int csock) goto error; } - gnutls_transport_set_ptr(session, - (gnutls_transport_ptr_t) GINT_TO_POINTER(csock)); + gnutls_transport_set_int(session, csock); /* gnutls does not make this easy */ if (tls->cred_type == GNUTLS_CRD_ANON && tls->server) { @@ -417,12 +414,9 @@ pcmk__new_tls_session(pcmk__tls_t *tls, int csock) int pcmk__tls_get_client_sock(const pcmk__remote_t *remote) { - gpointer sock_ptr = NULL; - pcmk__assert((remote != NULL) && (remote->tls_session != NULL)); - sock_ptr = (gpointer) gnutls_transport_get_ptr(remote->tls_session); - return GPOINTER_TO_INT(sock_ptr); + return gnutls_transport_get_int(remote->tls_session); } int From 73526d8e01457f8ea2f3d77c802b7e6f07d7ce9d Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 8 Jan 2025 14:28:33 -0800 Subject: [PATCH 2/3] Refactor: libcrmcommon: Drop calls to gnutls_global_init()/deinit() Except in deprecated crm_gnutls_global_init(). As of GnuTLS 3.3.0, the GnuTLS library is initialized on load, and it's no longer necessary to call these functions explicitly. https://www.gnutls.org/manual/html_node/Initialization.html https://www.gnutls.org/manual/html_node/Core-TLS-API.html#gnutls_005fglobal_005finit Ref T967 Signed-off-by: Reid Wahl --- lib/common/tls.c | 10 ---------- lib/common/utils.c | 4 +++- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/lib/common/tls.c b/lib/common/tls.c index 6328370525c..dbf34898975 100644 --- a/lib/common/tls.c +++ b/lib/common/tls.c @@ -173,8 +173,6 @@ pcmk__free_tls(pcmk__tls_t *tls) free(tls); tls = NULL; - - gnutls_global_deinit(); } int @@ -190,14 +188,6 @@ pcmk__init_tls(pcmk__tls_t **tls, bool server, gnutls_credentials_type_t cred_ty signal(SIGPIPE, SIG_IGN); - /* gnutls_global_init is safe to call multiple times, but we have to call - * gnutls_global_deinit the same number of times for that function to do - * anything. - * - * FIXME: When we can use gnutls >= 3.3.0, we don't have to call - * gnutls_global_init anymore. - */ - gnutls_global_init(); gnutls_global_set_log_level(8); gnutls_global_set_log_function(_gnutls_log_func); diff --git a/lib/common/utils.c b/lib/common/utils.c index f49f5c0b1bb..b1079a7740f 100644 --- a/lib/common/utils.c +++ b/lib/common/utils.c @@ -1,5 +1,5 @@ /* - * Copyright 2004-2024 the Pacemaker project contributors + * Copyright 2004-2025 the Pacemaker project contributors * * The version control history for this file may have further details. * @@ -445,6 +445,8 @@ pcmk__timeout_ms2s(guint timeout_ms) // Deprecated functions kept only for backward API compatibility // LCOV_EXCL_START +#include // gnutls_global_init(), etc. + #include static void From 939fd116824b78c4ff0010ba4f0c8eda1180de64 Mon Sep 17 00:00:00 2001 From: Reid Wahl Date: Wed, 8 Jan 2025 14:56:42 -0800 Subject: [PATCH 3/3] Refactor: libcrmcommon: Use gnutls_session_set_verify_cert() Instead of calling gnutls_certificate_set_verify_function() with the custom callback verify_peer_cert(). gnutls_session_set_verify_cert() is available since GnuTLS 3.4.6. It sets a verify function for the entire session, overriding any verify function set for a particular certificate (for example, using gnutls_certificate_set_verify_function()). For our purposes, each session has a unique certificate anyway, so the effect is the same either way. gnutls_session_set_verify_cert() sets up a verify callback function automatically, using hostname and flags parameters. At the time of this commit, it's called auto_verify_cb(); it calls gnutls_certificate_verify_peers() or a related function and returns 0 on success or GNUTLS_E_CERTIFICATE_ERROR or GNUTLS_E_CERTIFICATE_VERIFICATION_ERROR on error. * Our verify_peer_cert() function passes NULL to gnutls_certificate_verify_peers3() to disable hostname verification. Accordingly, we pass NULL to gnutls_session_set_verify_cert(). * We don't currently override the default verify flags (which would have required a call to gnutls_certificate_set_verify_flags()). So we pass 0 for the flags argument here, which says to use the defaults. There will be changes in the output upon error, as we lose our custom error processing from verify_peer_cert(), but that seems acceptable. Closes T967 Signed-off-by: Reid Wahl --- lib/common/tls.c | 46 ++-------------------------------------------- 1 file changed, 2 insertions(+), 44 deletions(-) diff --git a/lib/common/tls.c b/lib/common/tls.c index dbf34898975..2bdd2aab927 100644 --- a/lib/common/tls.c +++ b/lib/common/tls.c @@ -99,44 +99,6 @@ tls_load_x509_data(pcmk__tls_t *tls) return pcmk_rc_ok; } -/*! - * \internal - * \brief Verify a peer's certificate - * - * \return 0 if the certificate is trusted and the gnutls handshake should - * continue, -1 otherwise - */ -static int -verify_peer_cert(gnutls_session_t session) -{ - int rc; - int type; - unsigned int status; - gnutls_datum_t out; - - /* NULL = no hostname comparison will be performed */ - rc = gnutls_certificate_verify_peers3(session, NULL, &status); - - /* Success means it was able to perform the verification. We still have - * to check status to see whether the cert is valid or not. - */ - if (rc != GNUTLS_E_SUCCESS) { - crm_err("Failed to verify peer certificate: %s", gnutls_strerror(rc)); - return -1; - } - - if (status == 0) { - /* The certificate is trusted. */ - return 0; - } - - type = gnutls_certificate_type_get(session); - gnutls_certificate_verification_status_print(status, type, &out, 0); - crm_err("Peer certificate invalid: %s", out.data); - gnutls_free(out.data); - return GNUTLS_E_CERTIFICATE_VERIFICATION_ERROR; -} - static void _gnutls_log_func(int level, const char *msg) { @@ -368,12 +330,8 @@ pcmk__new_tls_session(pcmk__tls_t *tls, int csock) gnutls_certificate_server_set_request(session, GNUTLS_CERT_REQUIRE); } - /* Register a function to verify the peer's certificate. - * - * FIXME: When we can require gnutls >= 3.4.6, remove verify_peer_cert - * and use gnutls_session_set_verify_cert instead. - */ - gnutls_certificate_set_verify_function(tls->credentials.cert, verify_peer_cert); + // Register a function to verify the peer's certificate + gnutls_session_set_verify_cert(session, NULL, 0); } return session;