From 067bf229aaa080bb476533ec4ae97db543d620b6 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Fri, 15 Aug 2025 15:45:55 -0400 Subject: [PATCH 01/10] tlshd: is preferred over "config.h" Static checkers are having some difficulty finding the autoconf- generated config.h header. Signed-off-by: Chuck Lever --- src/tlshd/client.c | 2 +- src/tlshd/config.c | 2 +- src/tlshd/handshake.c | 2 +- src/tlshd/keyring.c | 2 +- src/tlshd/ktls.c | 2 +- src/tlshd/log.c | 2 +- src/tlshd/main.c | 2 +- src/tlshd/netlink.c | 2 +- src/tlshd/quic.c | 3 ++- src/tlshd/server.c | 2 +- 10 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/tlshd/client.c b/src/tlshd/client.c index 04d1b9f..7264162 100644 --- a/src/tlshd/client.c +++ b/src/tlshd/client.c @@ -20,7 +20,7 @@ * 02110-1301, USA. */ -#include "config.h" +#include #include #include diff --git a/src/tlshd/config.c b/src/tlshd/config.c index 4c54d37..029dbcc 100644 --- a/src/tlshd/config.c +++ b/src/tlshd/config.c @@ -18,7 +18,7 @@ * 02110-1301, USA. */ -#include "config.h" +#include #include #include diff --git a/src/tlshd/handshake.c b/src/tlshd/handshake.c index ed5c13b..22a2d4e 100644 --- a/src/tlshd/handshake.c +++ b/src/tlshd/handshake.c @@ -19,7 +19,7 @@ * 02110-1301, USA. */ -#include "config.h" +#include #include #include diff --git a/src/tlshd/keyring.c b/src/tlshd/keyring.c index 6aac02e..32f2d27 100644 --- a/src/tlshd/keyring.c +++ b/src/tlshd/keyring.c @@ -18,7 +18,7 @@ * 02110-1301, USA. */ -#include "config.h" +#include #include #include diff --git a/src/tlshd/ktls.c b/src/tlshd/ktls.c index 40a26a4..b2f931d 100644 --- a/src/tlshd/ktls.c +++ b/src/tlshd/ktls.c @@ -19,7 +19,7 @@ * 02110-1301, USA. */ -#include "config.h" +#include #include #include diff --git a/src/tlshd/log.c b/src/tlshd/log.c index 849027e..ad39d36 100644 --- a/src/tlshd/log.c +++ b/src/tlshd/log.c @@ -18,7 +18,7 @@ * 02110-1301, USA. */ -#include "config.h" +#include #include #include diff --git a/src/tlshd/main.c b/src/tlshd/main.c index 649fe3f..66d3966 100644 --- a/src/tlshd/main.c +++ b/src/tlshd/main.c @@ -19,7 +19,7 @@ * 02110-1301, USA. */ -#include "config.h" +#include #include #include diff --git a/src/tlshd/netlink.c b/src/tlshd/netlink.c index c08e5e5..4c5a068 100644 --- a/src/tlshd/netlink.c +++ b/src/tlshd/netlink.c @@ -18,7 +18,7 @@ * 02110-1301, USA. */ -#include "config.h" +#include #include #include diff --git a/src/tlshd/quic.c b/src/tlshd/quic.c index 36b8cee..1969871 100644 --- a/src/tlshd/quic.c +++ b/src/tlshd/quic.c @@ -18,6 +18,8 @@ * 02110-1301, USA. */ +#include + #include #include #include @@ -26,7 +28,6 @@ #include #include -#include "config.h" #include "tlshd.h" #ifdef HAVE_GNUTLS_QUIC diff --git a/src/tlshd/server.c b/src/tlshd/server.c index e80f2fd..91077a8 100644 --- a/src/tlshd/server.c +++ b/src/tlshd/server.c @@ -19,7 +19,7 @@ * 02110-1301, USA. */ -#include "config.h" +#include #include #include From bf3af3357663bfbaabbe9c13a1b993e18c90cf12 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Fri, 15 Aug 2025 16:31:56 -0400 Subject: [PATCH 02/10] tlshd: Address Cppcheck complaint in client.c src/tlshd/client.c:174:9: style: The scope of the variable 'ret' can be reduced. [variableScope] int i, ret; ^ Signed-off-by: Chuck Lever --- src/tlshd/client.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/tlshd/client.c b/src/tlshd/client.c index 7264162..20d0cf9 100644 --- a/src/tlshd/client.c +++ b/src/tlshd/client.c @@ -171,7 +171,7 @@ static void tlshd_x509_log_issuers(const gnutls_datum_t *req_ca_rdn, int nreqs) { char issuer_dn[256]; size_t len; - int i, ret; + int i; if (nreqs < 1) return; @@ -179,6 +179,8 @@ static void tlshd_x509_log_issuers(const gnutls_datum_t *req_ca_rdn, int nreqs) tlshd_log_debug("Server's trusted authorities:"); for (i = 0; i < nreqs; i++) { + int ret; + len = sizeof(issuer_dn); ret = gnutls_x509_rdn_get(&req_ca_rdn[i], issuer_dn, &len); if (ret >= 0) From f2b8954f6d7a59c16f181dc29b3d54becfea7ff0 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Fri, 15 Aug 2025 16:34:30 -0400 Subject: [PATCH 03/10] tlshd: Address Cppcheck complaints in handshake.c src/tlshd/handshake.c:67:3: warning: Assignment of function parameter has no effect outside the function. Did you forget dereferencing it? [uselessAssignmentPtrArg] saved = 0; ^ src/tlshd/handshake.c:67:9: style: Variable 'saved' is assigned a value that is never used. [unreadVariable] saved = 0; ^ Fixes: d1e87171d961 ("tlshd: Disable Nagle during handshakes on TCP sockets") Signed-off-by: Chuck Lever --- src/tlshd/handshake.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tlshd/handshake.c b/src/tlshd/handshake.c index 22a2d4e..5a28939 100644 --- a/src/tlshd/handshake.c +++ b/src/tlshd/handshake.c @@ -64,7 +64,7 @@ static void tlshd_save_nagle(gnutls_session_t session, int *saved) IPPROTO_TCP, TCP_NODELAY, saved, &len); if (ret < 0) { tlshd_log_perror("getsockopt (NODELAY)"); - saved = 0; + *saved = 0; return; } From dafd0ec2b47d7efe667153b0cad08f1a42c5b3ba Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Fri, 15 Aug 2025 16:13:13 -0400 Subject: [PATCH 04/10] tlshd: Address Cppcheck complaint in ktls.c src/tlshd/ktls.c:543:40: style: Parameter 'parms' can be declared as pointer to const [constParameterPointer] struct tlshd_handshake_parms *parms, Signed-off-by: Chuck Lever --- src/tlshd/ktls.c | 2 +- src/tlshd/tlshd.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tlshd/ktls.c b/src/tlshd/ktls.c index b2f931d..883256a 100644 --- a/src/tlshd/ktls.c +++ b/src/tlshd/ktls.c @@ -540,7 +540,7 @@ int tlshd_gnutls_priority_init(void) * Returns GNUTLS_E_SUCCESS on success, otherwise an error code. */ int tlshd_gnutls_priority_set(gnutls_session_t session, - struct tlshd_handshake_parms *parms, + const struct tlshd_handshake_parms *parms, unsigned int psk_len) { gnutls_priority_t priority = tlshd_gnutls_priority_x509; diff --git a/src/tlshd/tlshd.h b/src/tlshd/tlshd.h index a0dd47e..2857804 100644 --- a/src/tlshd/tlshd.h +++ b/src/tlshd/tlshd.h @@ -85,7 +85,7 @@ extern int tlshd_keyring_link_session(const char *keyring); extern unsigned int tlshd_initialize_ktls(gnutls_session_t session); extern int tlshd_gnutls_priority_init(void); extern int tlshd_gnutls_priority_set(gnutls_session_t session, - struct tlshd_handshake_parms *parms, + const struct tlshd_handshake_parms *parms, unsigned int psk_len); extern void tlshd_gnutls_priority_deinit(void); From 1ea46b5ec025e81a203b809c83df9f1ce3b0478d Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Fri, 15 Aug 2025 17:20:59 -0400 Subject: [PATCH 05/10] tlshd: Address Cppcheck complaint in main.c src/tlshd/main.c:59:25: style: Parameter 'progname' can be declared as pointer to const [constParameterPointer] static void usage(char *progname) ^ Signed-off-by: Chuck Lever --- src/tlshd/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tlshd/main.c b/src/tlshd/main.c index 66d3966..b570526 100644 --- a/src/tlshd/main.c +++ b/src/tlshd/main.c @@ -56,7 +56,7 @@ static const struct option longopts[] = { { NULL, 0, NULL, 0 } }; -static void usage(char *progname) +static void usage(const char *progname) { fprintf(stderr, "usage: %s [-chsv]\n", progname); } From 8945c126c5f97fd268fbff2b85874b3f284d5b74 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Fri, 15 Aug 2025 16:41:44 -0400 Subject: [PATCH 06/10] tlshd: Address Cppcheck complaints in netlink.c src/tlshd/netlink.c:483:6: style: The scope of the variable 'err' can be reduced. [variableScope] int err; ^ src/tlshd/netlink.c:287:8: style: Variable 'peername' can be declared as pointer to const [constVariablePointer] char *peername = NULL; ^ src/tlshd/netlink.c:394:19: style: Variable 'hdr' can be declared as pointer to const [constVariablePointer] struct nlmsghdr *hdr; ^ src/tlshd/netlink.c:503:19: style: Variable 'hdr' can be declared as pointer to const [constVariablePointer] struct nlmsghdr *hdr; ^ Signed-off-by: Chuck Lever --- src/tlshd/netlink.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/tlshd/netlink.c b/src/tlshd/netlink.c index 4c5a068..fbbee56 100644 --- a/src/tlshd/netlink.c +++ b/src/tlshd/netlink.c @@ -283,8 +283,8 @@ static int tlshd_genl_valid_handler(struct nl_msg *msg, void *arg) struct tlshd_handshake_parms *parms = arg; struct sockaddr_storage addr; struct sockaddr *sap = NULL; + const char *peername = NULL; socklen_t salen, optlen; - char *peername = NULL; int err; tlshd_log_debug("Parsing a valid netlink message\n"); @@ -390,8 +390,8 @@ static const struct tlshd_handshake_parms tlshd_default_handshake_parms = { */ int tlshd_genl_get_handshake_parms(struct tlshd_handshake_parms *parms) { + const struct nlmsghdr *hdr; int family_id, err, ret; - struct nlmsghdr *hdr; struct nl_sock *nls; struct nl_msg *msg; @@ -480,9 +480,10 @@ static int tlshd_genl_put_remote_peerids(struct nl_msg *msg, { key_serial_t peerid; guint i; - int err; for (i = 0; i < parms->remote_peerids->len; i++) { + int err; + peerid = g_array_index(parms->remote_peerids, key_serial_t, i); err = nla_put_s32(msg, HANDSHAKE_A_DONE_REMOTE_AUTH, peerid); if (err < 0) { @@ -500,7 +501,7 @@ static int tlshd_genl_put_remote_peerids(struct nl_msg *msg, */ void tlshd_genl_done(struct tlshd_handshake_parms *parms) { - struct nlmsghdr *hdr; + const struct nlmsghdr *hdr; struct nl_sock *nls; struct nl_msg *msg; int family_id, err; From 48afd22504fe930b1565e6b14982463be74c6934 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Fri, 15 Aug 2025 16:44:55 -0400 Subject: [PATCH 07/10] tlshd: Address Cppcheck complaints in quic.c src/tlshd/quic.c:110:14: style: The scope of the variable 'ret' can be reduced. [variableScope] int sockfd, ret, len = sizeof(secret); ^ src/tlshd/quic.c:387:61: style: Parameter 'conn' can be declared as pointer to const [constParameterPointer] static int quic_handshake_completed(struct tlshd_quic_conn *conn) ^ src/tlshd/quic.c:392:63: style: Parameter 'conn' can be declared as pointer to const [constParameterPointer] static int quic_handshake_crypto_data(struct tlshd_quic_conn *conn, uint8_t level, ^ Signed-off-by: Chuck Lever --- src/tlshd/quic.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/tlshd/quic.c b/src/tlshd/quic.c index 1969871..f19e1db 100644 --- a/src/tlshd/quic.c +++ b/src/tlshd/quic.c @@ -107,7 +107,7 @@ static int quic_secret_func(gnutls_session_t session, gnutls_record_encryption_l struct tlshd_quic_conn *conn = gnutls_session_get_ptr(session); gnutls_cipher_algorithm_t type = gnutls_cipher_get(session); struct quic_crypto_secret secret = {}; - int sockfd, ret, len = sizeof(secret); + int sockfd, len = sizeof(secret); if (conn->completed) return 0; @@ -135,6 +135,8 @@ static int quic_secret_func(gnutls_session_t session, gnutls_record_encryption_l } if (secret.level == QUIC_CRYPTO_APP) { if (conn->is_serv) { + int ret; + ret = gnutls_session_ticket_send(session, 1, 0); if (ret) { tlshd_log_gnutls_error(ret); @@ -384,13 +386,14 @@ static int quic_handshake_recvmsg(int sockfd, struct tlshd_quic_msg *msg) return ret; } -static int quic_handshake_completed(struct tlshd_quic_conn *conn) +static int quic_handshake_completed(const struct tlshd_quic_conn *conn) { return conn->completed || conn->errcode; } -static int quic_handshake_crypto_data(struct tlshd_quic_conn *conn, uint8_t level, - const uint8_t *data, size_t datalen) +static int quic_handshake_crypto_data(const struct tlshd_quic_conn *conn, + uint8_t level, const uint8_t *data, + size_t datalen) { gnutls_session_t session = conn->session; int ret; From 822997358263fc88556ffd8c475c259627520476 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Fri, 15 Aug 2025 16:13:13 -0400 Subject: [PATCH 08/10] tlshd: Address Cppcheck complaints in server.c src/tlshd/server.c:84:9: style: The scope of the variable 'ret' can be reduced. [variableScope] int i, ret; ^ Signed-off-by: Chuck Lever --- src/tlshd/server.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/tlshd/server.c b/src/tlshd/server.c index 91077a8..250f0ac 100644 --- a/src/tlshd/server.c +++ b/src/tlshd/server.c @@ -80,8 +80,7 @@ static void tlshd_x509_server_put_privkey(void) static void tlshd_x509_log_issuers(const gnutls_datum_t *req_ca_rdn, int nreqs) { char issuer_dn[256]; - size_t len; - int i, ret; + int i; if (nreqs < 1) return; @@ -89,6 +88,9 @@ static void tlshd_x509_log_issuers(const gnutls_datum_t *req_ca_rdn, int nreqs) tlshd_log_debug("Server's trusted authorities:"); for (i = 0; i < nreqs; i++) { + size_t len; + int ret; + len = sizeof(issuer_dn); ret = gnutls_x509_rdn_get(&req_ca_rdn[i], issuer_dn, &len); if (ret >= 0) From b0a3a427eafd1c2a756f55d6a2785be03de26e7b Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Thu, 14 Aug 2025 14:02:08 -0400 Subject: [PATCH 09/10] workflows: Add a static analysis workflow Start with a Cppchecker job. More can be added. Signed-off-by: Chuck Lever --- .github/workflows/static.yml | 47 ++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 .github/workflows/static.yml diff --git a/.github/workflows/static.yml b/.github/workflows/static.yml new file mode 100644 index 0000000..7e5a0c0 --- /dev/null +++ b/.github/workflows/static.yml @@ -0,0 +1,47 @@ +--- +name: Static analysis + +on: [push, pull_request, workflow_dispatch] + +jobs: + cppcheck: + runs-on: ubuntu-latest + permissions: read-all + + steps: + - uses: actions/checkout@v4 + + - name: Install build dependencies + run: | + sudo apt-get update + sudo apt-get -y install \ + build-essential \ + autoconf \ + automake \ + gnutls-dev \ + libkeyutils-dev \ + libnl-3-dev \ + libnl-genl-3-dev \ + libglib2.0-dev + + - name: Install tools + run: | + sudo apt-get install -y bear cppcheck + + - name: Configure + run: | + ./autogen.sh + ./configure --with-systemd + + - name: Generate compile commands + run: | + bear -- make + + - name: Run Cppcheck + run: | + echo "::group::Cppcheck Analysis" + cppcheck --enable=all -I. \ + --suppress=missingIncludeSystem \ + --suppress=unusedFunction \ + src/ + echo "::endgroup::" From 12ef0461327e1b881db993690406dd4685b29f58 Mon Sep 17 00:00:00 2001 From: Chuck Lever Date: Fri, 15 Aug 2025 17:37:03 -0400 Subject: [PATCH 10/10] workflows: Add lizard static analysis Signed-off-by: Chuck Lever --- .github/workflows/static.yml | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/.github/workflows/static.yml b/.github/workflows/static.yml index 7e5a0c0..9395d92 100644 --- a/.github/workflows/static.yml +++ b/.github/workflows/static.yml @@ -45,3 +45,38 @@ jobs: --suppress=unusedFunction \ src/ echo "::endgroup::" + + lizard: + runs-on: ubuntu-latest + permissions: read-all + + steps: + - uses: actions/checkout@v4 + + - name: Install build dependencies + run: | + sudo apt-get update + sudo apt-get -y install \ + build-essential \ + autoconf \ + automake \ + gnutls-dev \ + libkeyutils-dev \ + libnl-3-dev \ + libnl-genl-3-dev \ + libglib2.0-dev + + - name: Install tools + run: | + pip3 install lizard bandit[toml] + + - name: Configure + run: | + ./autogen.sh + ./configure --with-systemd + + - name: Run Lizard Complexity Analysis + run: | + echo "::group::Complexity Analysis" + lizard --CCN 15 src/ || true + echo "::endgroup::"