From 86c7f1ed14be8a1994cb93d6353e530d52803787 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 18 Jul 2025 11:32:22 +0200 Subject: [PATCH] libnm-core: honor secrets flags when serializing WireGuard peers to D-Bus If "flags" indicate that only secrets should be serialized and a peer doesn't contain any secrets, skip it. Otherwise the function would return a non-empty result when the connection contains no secret, which causes issues later in the agent manager. Fixes: e148ec07d5d3 ('libnm: add NMWireGuardPeer and libnm support for peers') https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/2244 --- src/libnm-core-impl/nm-setting-wireguard.c | 19 +- src/libnm-core-impl/tests/test-setting.c | 219 +++++++++++++++++++++ 2 files changed, 237 insertions(+), 1 deletion(-) diff --git a/src/libnm-core-impl/nm-setting-wireguard.c b/src/libnm-core-impl/nm-setting-wireguard.c index a03090865f..9a3241884c 100644 --- a/src/libnm-core-impl/nm-setting-wireguard.c +++ b/src/libnm-core-impl/nm-setting-wireguard.c @@ -1477,6 +1477,7 @@ peers_to_dbus(_NM_SETT_INFO_PROP_TO_DBUS_FCN_ARGS _nm_nil) for (i_peer = 0; i_peer < n_peers; i_peer++) { const NMWireGuardPeer *peer = _peers_get(priv, i_peer)->peer; GVariantBuilder builder; + gboolean has_secrets = FALSE; if (!peer->public_key) continue; @@ -1496,11 +1497,13 @@ peers_to_dbus(_NM_SETT_INFO_PROP_TO_DBUS_FCN_ARGS _nm_nil) g_variant_new_string(nm_sock_addr_endpoint_get_endpoint(peer->endpoint))); if (_nm_connection_serialize_secrets(flags, peer->preshared_key_flags) - && peer->preshared_key) + && peer->preshared_key) { g_variant_builder_add(&builder, "{sv}", NM_WIREGUARD_PEER_ATTR_PRESHARED_KEY, g_variant_new_string(peer->preshared_key)); + has_secrets = TRUE; + } if (_nm_connection_serialize_non_secret(flags) && peer->preshared_key_flags != NM_SETTING_SECRET_FLAG_NOT_REQUIRED) @@ -1546,6 +1549,20 @@ peers_to_dbus(_NM_SETT_INFO_PROP_TO_DBUS_FCN_ARGS _nm_nil) g_variant_new_strv(strv, peer->allowed_ips->len)); } + if (NM_FLAGS_ANY(flags, + NM_CONNECTION_SERIALIZE_ONLY_SECRETS + | NM_CONNECTION_SERIALIZE_WITH_SECRETS_AGENT_OWNED + | NM_CONNECTION_SERIALIZE_WITH_SECRETS_SYSTEM_OWNED + | NM_CONNECTION_SERIALIZE_WITH_SECRETS_NOT_SAVED) + && !_nm_connection_serialize_non_secret(flags)) { + /* The flags indicate that only secrets must be serialized and this + * peer doesn't contain any. Skip the peer. */ + if (!has_secrets) { + g_variant_builder_clear(&builder); + continue; + } + } + if (!any_peers) { g_variant_builder_init(&peers_builder, G_VARIANT_TYPE("aa{sv}")); any_peers = TRUE; diff --git a/src/libnm-core-impl/tests/test-setting.c b/src/libnm-core-impl/tests/test-setting.c index ee0fa1586c..f3070c1730 100644 --- a/src/libnm-core-impl/tests/test-setting.c +++ b/src/libnm-core-impl/tests/test-setting.c @@ -5560,6 +5560,223 @@ test_bond_meta(void) /*****************************************************************************/ +static void +check_wg_setting_str(NMSetting *s_wg, + const char *exp_all, + const char *exp_nonsec, + const char *exp_sec) +{ + gs_unref_variant GVariant *dict_all = NULL; + gs_unref_variant GVariant *dict_nonsec = NULL; + gs_unref_variant GVariant *dict_sec = NULL; + gs_free char *str_all = NULL; + gs_free char *str_nonsec = NULL; + gs_free char *str_sec = NULL; + + dict_all = _nm_setting_to_dbus(s_wg, NULL, NM_CONNECTION_SERIALIZE_ALL, NULL); + dict_nonsec = _nm_setting_to_dbus(s_wg, NULL, NM_CONNECTION_SERIALIZE_WITH_NON_SECRET, NULL); + dict_sec = _nm_setting_to_dbus(s_wg, NULL, NM_CONNECTION_SERIALIZE_ONLY_SECRETS, NULL); + + str_all = g_variant_print(dict_all, TRUE); + str_nonsec = g_variant_print(dict_nonsec, TRUE); + str_sec = g_variant_print(dict_sec, TRUE); + + g_assert_cmpstr(exp_all, ==, str_all); + g_assert_cmpstr(exp_nonsec, ==, str_nonsec); + g_assert_cmpstr(exp_sec, ==, str_sec); +} + +static void +test_wireguard_to_dbus(void) +{ + gs_unref_object NMSetting *s_wg = NULL; + nm_auto_unref_wgpeer NMWireGuardPeer *peer1 = NULL; + nm_auto_unref_wgpeer NMWireGuardPeer *peer2 = NULL; + gs_unref_variant GVariant *dict_all = NULL; + gs_unref_variant GVariant *dict_non_secret = NULL; + gs_unref_variant GVariant *dict_only_secrets = NULL; + gs_free char *dict_str = NULL; + const char *test_private_key = "cFoJbK9bSrYrQrjFQGgqsWTO4IUIX0+rsaqNeCw2IWM="; + const char *test_public_key1 = "OMhgSum5+NamArI/LTp1mCZQD+CbzZxtOuvDC/RaGWU="; + const char *test_public_key2 = "2S7mA0vEMethVGG0qBm4T5EXbcQ2WYHOuP14Seb7jEM="; + const char *test_preshared_key = "yFGq76ej4lNI0pLLu36L0DgJMxWs4HmH5qNDNOt8AmM="; + + /* Test case 1: Minimal WireGuard setting without peers or private key */ + s_wg = nm_setting_wireguard_new(); + g_object_set(s_wg, + NM_SETTING_WIREGUARD_LISTEN_PORT, + 51820U, + NM_SETTING_WIREGUARD_FWMARK, + 42U, + NULL); + + check_wg_setting_str(s_wg, + /* clang-format off */ + /* all */ + "{'fwmark': , 'listen-port': }", + /* non secrets */ + "{'fwmark': , 'listen-port': }", + /* secrets */ + "@a{sv} {}" + /* clang-format on */ + ); + g_clear_object(&s_wg); + + /* Test case 2: WireGuard setting with private key, no peers */ + s_wg = nm_setting_wireguard_new(); + g_object_set(s_wg, + NM_SETTING_WIREGUARD_PRIVATE_KEY, + test_private_key, + NM_SETTING_WIREGUARD_PRIVATE_KEY_FLAGS, + NM_SETTING_SECRET_FLAG_NONE, + NM_SETTING_WIREGUARD_LISTEN_PORT, + 51820U, + NM_SETTING_WIREGUARD_FWMARK, + 42U, + NULL); + + check_wg_setting_str(s_wg, + /* clang-format off */ + /* all */ + "{" + "'fwmark': , " + "'listen-port': , " + "'private-key': <'cFoJbK9bSrYrQrjFQGgqsWTO4IUIX0+rsaqNeCw2IWM='>" + "}", + /* non secrets */ + "{" + "'fwmark': , " + "'listen-port': " + "}", + /* secrets */ + "{" + "'private-key': <'cFoJbK9bSrYrQrjFQGgqsWTO4IUIX0+rsaqNeCw2IWM='>" + "}" + /* clang-format on */ + ); + g_clear_object(&s_wg); + + /* Test case 3: WireGuard setting with peers (no PSK) */ + s_wg = nm_setting_wireguard_new(); + g_object_set(s_wg, + NM_SETTING_WIREGUARD_PRIVATE_KEY, + test_private_key, + NM_SETTING_WIREGUARD_PRIVATE_KEY_FLAGS, + NM_SETTING_SECRET_FLAG_NONE, + NM_SETTING_WIREGUARD_LISTEN_PORT, + 51820U, + NULL); + peer1 = nm_wireguard_peer_new(); + nm_wireguard_peer_set_public_key(peer1, test_public_key1, FALSE); + nm_wireguard_peer_set_endpoint(peer1, "192.168.1.1:51820", FALSE); + nm_wireguard_peer_append_allowed_ip(peer1, "10.0.0.0/8", FALSE); + nm_setting_wireguard_append_peer(NM_SETTING_WIREGUARD(s_wg), peer1); + + check_wg_setting_str(s_wg, + /* clang-format off */ + /* all */ + "{" + "'listen-port': , " + "'peers': <[{" + "'public-key': <'OMhgSum5+NamArI/LTp1mCZQD+CbzZxtOuvDC/RaGWU='>, " + "'endpoint': <'192.168.1.1:51820'>, " + "'allowed-ips': <['10.0.0.0/8']>" + "}]>, " + "'private-key': <'cFoJbK9bSrYrQrjFQGgqsWTO4IUIX0+rsaqNeCw2IWM='>" + "}", + /* non secrets */ + "{" + "'listen-port': , " + "'peers': <[{" + "'public-key': <'OMhgSum5+NamArI/LTp1mCZQD+CbzZxtOuvDC/RaGWU='>, " + "'endpoint': <'192.168.1.1:51820'>, " + "'allowed-ips': <['10.0.0.0/8']>" + "}]>" + "}", + /* secrets */ + "{" + "'private-key': <'cFoJbK9bSrYrQrjFQGgqsWTO4IUIX0+rsaqNeCw2IWM='>" + "}" + /* clang-format on */ + ); + g_clear_object(&s_wg); + nm_clear_pointer(&peer1, nm_wireguard_peer_unref); + + /* Test case 4: WireGuard setting with peers, one has PSK */ + s_wg = nm_setting_wireguard_new(); + g_object_set(s_wg, + NM_SETTING_WIREGUARD_PRIVATE_KEY, + test_private_key, + NM_SETTING_WIREGUARD_PRIVATE_KEY_FLAGS, + NM_SETTING_SECRET_FLAG_NONE, + NM_SETTING_WIREGUARD_LISTEN_PORT, + 51820U, + NULL); + + /* Peer without PSK */ + peer1 = nm_wireguard_peer_new(); + nm_wireguard_peer_set_public_key(peer1, test_public_key1, FALSE); + nm_wireguard_peer_set_endpoint(peer1, "192.168.1.1:51820", FALSE); + nm_wireguard_peer_append_allowed_ip(peer1, "10.0.0.0/8", FALSE); + nm_setting_wireguard_append_peer(NM_SETTING_WIREGUARD(s_wg), peer1); + + /* Peer with PSK */ + peer2 = nm_wireguard_peer_new(); + nm_wireguard_peer_set_public_key(peer2, test_public_key2, FALSE); + nm_wireguard_peer_set_endpoint(peer2, "192.168.2.1:51820", FALSE); + nm_wireguard_peer_append_allowed_ip(peer2, "172.16.0.0/12", FALSE); + nm_wireguard_peer_set_preshared_key(peer2, test_preshared_key, FALSE); + nm_wireguard_peer_set_preshared_key_flags(peer2, NM_SETTING_SECRET_FLAG_NONE); + nm_setting_wireguard_append_peer(NM_SETTING_WIREGUARD(s_wg), peer2); + + check_wg_setting_str(s_wg, + /* clang-format off */ + /* all */ + "{" + "'listen-port': , " + "'peers': <[{" + "'public-key': <'OMhgSum5+NamArI/LTp1mCZQD+CbzZxtOuvDC/RaGWU='>, " + "'endpoint': <'192.168.1.1:51820'>, " + "'allowed-ips': <['10.0.0.0/8']>" + "}, {" + "'public-key': <'2S7mA0vEMethVGG0qBm4T5EXbcQ2WYHOuP14Seb7jEM='>, " + "'endpoint': <'192.168.2.1:51820'>, " + "'preshared-key': <'yFGq76ej4lNI0pLLu36L0DgJMxWs4HmH5qNDNOt8AmM='>, " + "'preshared-key-flags': , " + "'allowed-ips': <['172.16.0.0/12']>" + "}]>, " + "'private-key': <'cFoJbK9bSrYrQrjFQGgqsWTO4IUIX0+rsaqNeCw2IWM='>" + "}", + /* non secrets */ + "{" + "'listen-port': , " + "'peers': <[{" + "'public-key': <'OMhgSum5+NamArI/LTp1mCZQD+CbzZxtOuvDC/RaGWU='>, " + "'endpoint': <'192.168.1.1:51820'>, " + "'allowed-ips': <['10.0.0.0/8']>" + "}, {" + "'public-key': <'2S7mA0vEMethVGG0qBm4T5EXbcQ2WYHOuP14Seb7jEM='>, " + "'endpoint': <'192.168.2.1:51820'>, " + "'preshared-key-flags': , " + "'allowed-ips': <['172.16.0.0/12']>" + "}]>" + "}", + /* secrets */ + "{" + "'peers': <[{" + "'public-key': <'2S7mA0vEMethVGG0qBm4T5EXbcQ2WYHOuP14Seb7jEM='>, " + "'preshared-key': <'yFGq76ej4lNI0pLLu36L0DgJMxWs4HmH5qNDNOt8AmM='>" + "}]>, " + "'private-key': <'cFoJbK9bSrYrQrjFQGgqsWTO4IUIX0+rsaqNeCw2IWM='>}" + /* clang-format on */ + ); + g_clear_object(&s_wg); + nm_clear_pointer(&peer1, nm_wireguard_peer_unref); + nm_clear_pointer(&peer2, nm_wireguard_peer_unref); +} + +/*****************************************************************************/ + NMTST_DEFINE(); int @@ -5688,5 +5905,7 @@ main(int argc, char **argv) g_test_add_func("/libnm/test_bond_meta", test_bond_meta); + g_test_add_func("/libnm/test_wireguard_to_dbus", test_wireguard_to_dbus); + return g_test_run(); }