From 343b99f891137b2601d56851527cda3595372543 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 3 Oct 2018 00:59:17 +0200 Subject: [PATCH 01/10] acd/tests: skip NAcd tests under valgrind Under valgrind, we cannot create an NAcd instance. --10916-- WARNING: unhandled amd64-linux syscall: 321 --10916-- You may be able to write your own handler. --10916-- Read the file README_MISSING_SYSCALL_OR_IOCTL. --10916-- Nevertheless we consider this a bug. Please report --10916-- it at http://valgrind.org/support/bug_reports.html. This limitation already poses a problem, because running NetworkManager under valgrind might fail. However, for tests it doesn't matter and we can just skip them. --- src/devices/tests/test-acd.c | 48 ++++++++++++++++++++++++++++++++++++ tools/run-nm-test.sh | 7 ++++-- 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/src/devices/tests/test-acd.c b/src/devices/tests/test-acd.c index 6f4ba8d8fd..28910338ea 100644 --- a/src/devices/tests/test-acd.c +++ b/src/devices/tests/test-acd.c @@ -20,6 +20,8 @@ #include "nm-default.h" +#include "n-acd/src/n-acd.h" + #include "devices/nm-acd-manager.h" #include "platform/tests/test-common.h" @@ -31,6 +33,46 @@ #define ADDR3 0x03030303 #define ADDR4 0x04040404 +/*****************************************************************************/ + +static gboolean +_skip_acd_test_check (void) +{ + NAcd *acd; + NAcdConfig *config; + const guint8 hwaddr[ETH_ALEN] = { 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 }; + int r; + static int skip = -1; + + if (skip == -1) { + r = n_acd_config_new (&config); + g_assert (r == 0); + + n_acd_config_set_ifindex (config, 1); + n_acd_config_set_transport (config, N_ACD_TRANSPORT_ETHERNET); + n_acd_config_set_mac (config, hwaddr, sizeof (hwaddr)); + + r = n_acd_new (&acd, config); + n_acd_config_free (config); + if (r == 0) + n_acd_unref (acd); + + skip = (r != 0); + } + return skip; +} + +#define _skip_acd_test() \ + ({ \ + gboolean _skip = _skip_acd_test_check (); \ + \ + if (_skip) \ + g_test_skip ("Cannot create NAcd. Running under valgind?"); \ + _skip; \ + }) + +/*****************************************************************************/ + typedef struct { int ifindex0; int ifindex1; @@ -79,6 +121,9 @@ test_acd_common (test_fixture *fixture, TestInfo *info) .user_data_destroy = (GDestroyNotify) g_main_loop_unref, }; + if (_skip_acd_test ()) + return; + /* first, try with a short waittime. We hope that this is long enough * to successfully complete the test. Only if that's not the case, we * assume the computer is currently busy (high load) and we retry with @@ -156,6 +201,9 @@ test_acd_announce (test_fixture *fixture, gconstpointer user_data) NMAcdManager *manager; GMainLoop *loop; + if (_skip_acd_test ()) + return; + manager = nm_acd_manager_new (fixture->ifindex0, fixture->hwaddr0, fixture->hwaddr0_len, diff --git a/tools/run-nm-test.sh b/tools/run-nm-test.sh index 6b96a139d8..6e7d02c845 100755 --- a/tools/run-nm-test.sh +++ b/tools/run-nm-test.sh @@ -294,8 +294,11 @@ fi if [ $HAS_ERRORS -eq 0 ]; then # valgrind doesn't support setns syscall and spams the logfile. # hack around it... - if [ "$TEST_NAME" = 'test-link-linux' -a -z "$(sed -e '/^--[0-9]\+-- WARNING: unhandled .* syscall: /,/^--[0-9]\+-- it at http.*\.$/d' "$LOGFILE")" ]; then - HAS_ERRORS=1 + if [ "$TEST_NAME" = 'test-link-linux' -o \ + "$TEST_NAME" = 'test-acd' ]; then + if [ -z "$(sed -e '/^--[0-9]\+-- WARNING: unhandled .* syscall: /,/^--[0-9]\+-- it at http.*\.$/d' "$LOGFILE")" ]; then + HAS_ERRORS=1 + fi fi fi From ba491a6674d322371ed1c989afc1ecee79a63c1a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 21 Sep 2018 15:45:59 +0200 Subject: [PATCH 02/10] shared: add nm_strndup_a() helper --- shared/nm-utils/nm-shared-utils.h | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/shared/nm-utils/nm-shared-utils.h b/shared/nm-utils/nm-shared-utils.h index 97e030b005..968e375c4d 100644 --- a/shared/nm-utils/nm-shared-utils.h +++ b/shared/nm-utils/nm-shared-utils.h @@ -236,6 +236,37 @@ nm_memdup (gconstpointer data, gsize size) return p; } +/* Similar to g_strndup(), however, if the string (including the terminating + * NUL char) fits into alloca_maxlen, this will alloca() the memory. + * + * It's a mix of strndup() and strndupa(), but deciding based on @alloca_maxlen + * which one to use. + * + * In case malloc() is necessary, @out_str_free will be set (this string + * must be freed afterwards). It is permissible to pass %NULL as @out_str_free, + * if you ensure that len < alloca_maxlen. */ +#define nm_strndup_a(alloca_maxlen, str, len, out_str_free) \ + ({ \ + const gsize _alloca_maxlen = (alloca_maxlen); \ + const char *const _str = (str); \ + const gsize _len = (len); \ + char **const _out_str_free = (out_str_free); \ + char *_s; \ + \ + if ( _out_str_free \ + && _len >= _alloca_maxlen) { \ + _s = g_malloc (_len + 1); \ + *_out_str_free = _s; \ + } else { \ + g_assert (_len < _alloca_maxlen); \ + _s = g_alloca (_len + 1); \ + } \ + if (_len > 0) \ + strncpy (_s, _str, _len); \ + _s[_len] = '\0'; \ + _s; \ + }) + /*****************************************************************************/ extern const void *const _NM_PTRARRAY_EMPTY[1]; From d060b7b3798033d3109a3cbf664d1b44f36c394e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 27 Sep 2018 14:20:02 +0200 Subject: [PATCH 03/10] shared: avoid invoking g_free() with NULL from gs_free cleanup attribute In general, it's fine to pass %NULL to g_free(). However, consider: char * foo (void) { gs_free char *value = NULL; value = g_strdup ("hi"); return g_steal_pointer (&value); } gs_free, gs_local_free(), and g_steal_pointer() are all inlinable. Here the compiler can easily recognize that we always pass %NULL to g_free(). But with the previous implementation, the compiler would not omit the call to g_free(). Similar patterns happen all over the place: gboolean baz (void) { gs_free char *value = NULL; if (!some_check ()) return FALSE; value = get_value (); if (!value) return FALSE; return TRUE; } in this example, g_free() is only required after setting @value to non-NULL. Note that this does increase the binary side a bit (4k for libnm, 8k for NetworkManager, with "-O2"). --- shared/nm-utils/nm-macros-internal.h | 10 +++++----- shared/nm-utils/nm-secret-utils.h | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/shared/nm-utils/nm-macros-internal.h b/shared/nm-utils/nm-macros-internal.h index b8b945342a..d78f087414 100644 --- a/shared/nm-utils/nm-macros-internal.h +++ b/shared/nm-utils/nm-macros-internal.h @@ -99,7 +99,7 @@ static inline void name (Type *v) \ * Call g_free() on a variable location when it goes out of scope. */ #define gs_free nm_auto(gs_local_free) -NM_AUTO_DEFINE_FCN_VOID (void *, gs_local_free, g_free) +NM_AUTO_DEFINE_FCN_VOID0 (void *, gs_local_free, g_free) /** * gs_unref_object: @@ -160,7 +160,7 @@ NM_AUTO_DEFINE_FCN0 (GHashTable *, gs_local_hashtable_unref, g_hash_table_unref) * of scope. */ #define gs_free_slist nm_auto(gs_local_free_slist) -NM_AUTO_DEFINE_FCN (GSList *, gs_local_free_slist, g_slist_free) +NM_AUTO_DEFINE_FCN0 (GSList *, gs_local_free_slist, g_slist_free) /** * gs_unref_bytes: @@ -178,7 +178,7 @@ NM_AUTO_DEFINE_FCN0 (GBytes *, gs_local_bytes_unref, g_bytes_unref) * Call g_strfreev() on a variable location when it goes out of scope. */ #define gs_strfreev nm_auto(gs_local_strfreev) -NM_AUTO_DEFINE_FCN (char **, gs_local_strfreev, g_strfreev) +NM_AUTO_DEFINE_FCN0 (char **, gs_local_strfreev, g_strfreev) /** * gs_free_error: @@ -222,7 +222,7 @@ static inline int nm_close (int fd); * However, let's never mix them. To free malloc'ed memory, always use * free() or nm_auto_free. */ -NM_AUTO_DEFINE_FCN_VOID (void *, _nm_auto_free_impl, free) +NM_AUTO_DEFINE_FCN_VOID0 (void *, _nm_auto_free_impl, free) #define nm_auto_free nm_auto(_nm_auto_free_impl) NM_AUTO_DEFINE_FCN0 (GVariantIter *, _nm_auto_free_variant_iter, g_variant_iter_free) @@ -231,7 +231,7 @@ NM_AUTO_DEFINE_FCN0 (GVariantIter *, _nm_auto_free_variant_iter, g_variant_iter_ NM_AUTO_DEFINE_FCN0 (GVariantBuilder *, _nm_auto_unref_variant_builder, g_variant_builder_unref) #define nm_auto_unref_variant_builder nm_auto(_nm_auto_unref_variant_builder) -NM_AUTO_DEFINE_FCN (GList *, _nm_auto_free_list, g_list_free) +NM_AUTO_DEFINE_FCN0 (GList *, _nm_auto_free_list, g_list_free) #define nm_auto_free_list nm_auto(_nm_auto_free_list) NM_AUTO_DEFINE_FCN0 (GChecksum *, _nm_auto_checksum_free, g_checksum_free) diff --git a/shared/nm-utils/nm-secret-utils.h b/shared/nm-utils/nm-secret-utils.h index 21a3c1ba11..9df31afea1 100644 --- a/shared/nm-utils/nm-secret-utils.h +++ b/shared/nm-utils/nm-secret-utils.h @@ -43,7 +43,7 @@ nm_free_secret (char *secret) } } -NM_AUTO_DEFINE_FCN (char *, _nm_auto_free_secret, nm_free_secret) +NM_AUTO_DEFINE_FCN0 (char *, _nm_auto_free_secret, nm_free_secret) /** * nm_auto_free_secret: * From 5345cac15118975fd1fa37d0ca1a04b44a2c91e3 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 27 Sep 2018 13:51:55 +0200 Subject: [PATCH 04/10] core: add code comment to nm_utils_read_link_absolute() and minor cleanup --- src/nm-core-utils.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/nm-core-utils.c b/src/nm-core-utils.c index 97866256d8..99fa8c9523 100644 --- a/src/nm-core-utils.c +++ b/src/nm-core-utils.c @@ -1114,13 +1114,17 @@ nm_utils_read_link_absolute (const char *link_file, GError **error) return ln; dirname = g_path_get_dirname (link_file); - if (!g_path_is_absolute (link_file)) { - gs_free char *dirname_rel = dirname; + if (!g_path_is_absolute (dirname)) { gs_free char *current_dir = g_get_current_dir (); - dirname = g_build_filename (current_dir, dirname_rel, NULL); - } - ln_abs = g_build_filename (dirname, ln, NULL); + /* @link_file argument was not an absolute path in the first place. + * That actually may be a bug, because the CWD is not well defined + * in most cases. Anyway, apparently we were able to load the file + * even from a relative path. So, when making the link absolute, we + * also need to prepend the CWD. */ + ln_abs = g_build_filename (current_dir, dirname, ln, NULL); + } else + ln_abs = g_build_filename (dirname, ln, NULL); g_free (dirname); g_free (ln); return ln_abs; From e886e5364eba64b2a143f02930b646e3d9d30619 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 2 Oct 2018 22:42:39 +0200 Subject: [PATCH 05/10] keyfile/tests: refactor loading of keyfiles in tests --- .../plugins/keyfile/tests/test-keyfile.c | 345 +++--------------- 1 file changed, 50 insertions(+), 295 deletions(-) diff --git a/src/settings/plugins/keyfile/tests/test-keyfile.c b/src/settings/plugins/keyfile/tests/test-keyfile.c index b4c6b1e2ce..5970a26b60 100644 --- a/src/settings/plugins/keyfile/tests/test-keyfile.c +++ b/src/settings/plugins/keyfile/tests/test-keyfile.c @@ -65,37 +65,31 @@ check_ip_route (NMSettingIPConfig *config, int idx, const char *destination, int g_assert_cmpint (nm_ip_route_get_metric (route), ==, metric); } -static NMConnection * -keyfile_read_connection_from_file (const char *filename) -{ - gs_free_error GError *error = NULL; - NMConnection *connection; - - g_assert (filename); - - connection = nms_keyfile_reader_from_file (filename, &error); - g_assert_no_error (error); - - nmtst_assert_connection_verifies_without_normalization (connection); - - return connection; -} +#define keyfile_read_connection_from_file(full_filename) \ +({ \ + gs_free_error GError *_error = NULL; \ + NMConnection *_connection; \ + \ + g_assert (full_filename && full_filename[0] == '/'); \ + \ + _connection = nms_keyfile_reader_from_file (full_filename, \ + (nmtst_get_rand_int () % 2) ? &_error : NULL); \ + nmtst_assert_success (_connection, _error); \ + nmtst_assert_connection_verifies_without_normalization (_connection); \ + \ + _connection; \ +}) static void assert_reread (NMConnection *connection, gboolean normalize_connection, const char *testfile) { gs_unref_object NMConnection *reread = NULL; gs_unref_object NMConnection *connection_clone = NULL; - GError *error = NULL; - GError **p_error = (nmtst_get_rand_int () % 2) ? &error : NULL; NMSettingConnection *s_con; g_assert (NM_IS_CONNECTION (connection)); - g_assert (testfile && testfile[0]); - reread = nms_keyfile_reader_from_file (testfile, p_error); - g_assert_no_error (error); - g_assert (NM_IS_CONNECTION (reread)); + reread = keyfile_read_connection_from_file (testfile); if ( !normalize_connection && (s_con = nm_connection_get_setting_connection (connection)) @@ -229,10 +223,8 @@ test_read_valid_wired_connection (void) NMSettingIPConfig *s_ip4; NMSettingIPConfig *s_ip6; NMIPRoute *route; - gs_free_error GError *error = NULL; const char *mac; char expected_mac_address[ETH_ALEN] = { 0x00, 0x11, 0x22, 0x33, 0x44, 0x55 }; - gboolean success; NMTST_EXPECT_NM_INFO ("*ipv4.addresses:*semicolon at the end*addresses1*"); NMTST_EXPECT_NM_INFO ("*ipv4.addresses:*semicolon at the end*addresses2*"); @@ -249,16 +241,9 @@ test_read_valid_wired_connection (void) NMTST_EXPECT_NM_INFO ("*ipv6.address*semicolon at the end*address7*"); NMTST_EXPECT_NM_INFO ("*ipv6.routes*semicolon at the end*routes1*"); NMTST_EXPECT_NM_INFO ("*ipv6.route*semicolon at the end*route6*"); - connection = nms_keyfile_reader_from_file (TEST_KEYFILES_DIR "/Test_Wired_Connection", &error); - g_assert_no_error (error); + connection = keyfile_read_connection_from_file (TEST_KEYFILES_DIR "/Test_Wired_Connection"); g_test_assert_expected_messages (); - g_assert (connection); - success = nm_connection_verify (connection, &error); - g_assert_no_error (error); - g_assert (success); - - /* ===== CONNECTION SETTING ===== */ s_con = nm_connection_get_setting_connection (connection); g_assert (s_con); g_assert_cmpstr (nm_setting_connection_get_id (s_con), ==, "Test Wired Connection"); @@ -266,7 +251,6 @@ test_read_valid_wired_connection (void) g_assert_cmpuint (nm_setting_connection_get_timestamp (s_con), ==, 6654332); g_assert (nm_setting_connection_get_autoconnect (s_con)); - /* ===== WIRED SETTING ===== */ s_wired = nm_connection_get_setting_wired (connection); g_assert (s_wired); @@ -275,7 +259,6 @@ test_read_valid_wired_connection (void) g_assert (nm_utils_hwaddr_matches (mac, -1, expected_mac_address, sizeof (expected_mac_address))); g_assert_cmpint (nm_setting_wired_get_mtu (s_wired), ==, 1400); - /* ===== IPv4 SETTING ===== */ s_ip4 = nm_connection_get_setting_ip4_config (connection); g_assert (s_ip4); g_assert_cmpstr (nm_setting_ip_config_get_method (s_ip4), ==, NM_SETTING_IP4_CONFIG_METHOD_MANUAL); @@ -324,7 +307,6 @@ test_read_valid_wired_connection (void) nmtst_assert_route_attribute_boolean (route, NM_IP_ROUTE_ATTRIBUTE_LOCK_CWND, TRUE); nmtst_assert_route_attribute_string (route, NM_IP_ROUTE_ATTRIBUTE_SRC, "7.7.7.7"); - /* ===== IPv6 SETTING ===== */ s_ip6 = nm_connection_get_setting_ip6_config (connection); g_assert (s_ip6); @@ -535,32 +517,22 @@ test_read_ip6_wired_connection (void) NMSettingWired *s_wired; NMSettingIPConfig *s_ip4; NMSettingIPConfig *s_ip6; - gs_free_error GError *error = NULL; - gboolean success; - connection = nms_keyfile_reader_from_file (TEST_KEYFILES_DIR "/Test_Wired_Connection_IP6", NULL); - g_assert (connection); - success = nm_connection_verify (connection, &error); - g_assert_no_error (error); - g_assert (success); + connection = keyfile_read_connection_from_file (TEST_KEYFILES_DIR "/Test_Wired_Connection_IP6"); - /* ===== CONNECTION SETTING ===== */ s_con = nm_connection_get_setting_connection (connection); g_assert (s_con); g_assert_cmpstr (nm_setting_connection_get_id (s_con), ==, "Test Wired Connection IP6"); g_assert_cmpstr (nm_setting_connection_get_uuid (s_con), ==, "4e80a56d-c99f-4aad-a6dd-b449bc398c57"); - /* ===== WIRED SETTING ===== */ s_wired = nm_connection_get_setting_wired (connection); g_assert (s_wired); - /* ===== IPv4 SETTING ===== */ s_ip4 = nm_connection_get_setting_ip4_config (connection); g_assert (s_ip4); g_assert_cmpstr (nm_setting_ip_config_get_method (s_ip4), ==, NM_SETTING_IP4_CONFIG_METHOD_DISABLED); g_assert_cmpint (nm_setting_ip_config_get_num_addresses (s_ip4), ==, 0); - /* ===== IPv6 SETTING ===== */ s_ip6 = nm_connection_get_setting_ip6_config (connection); g_assert (s_ip6); g_assert_cmpstr (nm_setting_ip_config_get_method (s_ip6), ==, NM_SETTING_IP6_CONFIG_METHOD_MANUAL); @@ -638,28 +610,20 @@ test_read_wired_mac_case (void) gs_unref_object NMConnection *connection = NULL; NMSettingConnection *s_con; NMSettingWired *s_wired; - gs_free_error GError *error = NULL; const char *mac; char expected_mac_address[ETH_ALEN] = { 0x00, 0x11, 0xaa, 0xbb, 0xcc, 0x55 }; - gboolean success; NMTST_EXPECT_NM_INFO ("*ipv4.addresses*semicolon at the end*addresses1*"); NMTST_EXPECT_NM_INFO ("*ipv4.addresses*semicolon at the end*addresses2*"); NMTST_EXPECT_NM_INFO ("*ipv6.routes*semicolon at the end*routes1*"); - connection = nms_keyfile_reader_from_file (TEST_KEYFILES_DIR "/Test_Wired_Connection_MAC_Case", NULL); + connection = keyfile_read_connection_from_file (TEST_KEYFILES_DIR "/Test_Wired_Connection_MAC_Case"); g_test_assert_expected_messages (); - g_assert (connection); - success = nm_connection_verify (connection, &error); - g_assert_no_error (error); - g_assert (success); - /* ===== CONNECTION SETTING ===== */ s_con = nm_connection_get_setting_connection (connection); g_assert (s_con); g_assert_cmpstr (nm_setting_connection_get_id (s_con), ==, "Test Wired Connection MAC Case"); g_assert_cmpstr (nm_setting_connection_get_uuid (s_con), ==, "4e80a56d-c99f-4aad-a6dd-b449bc398c57"); - /* ===== WIRED SETTING ===== */ s_wired = nm_connection_get_setting_wired (connection); g_assert (s_wired); mac = nm_setting_wired_get_mac_address (s_wired); @@ -672,29 +636,19 @@ test_read_mac_old_format (void) { gs_unref_object NMConnection *connection = NULL; NMSettingWired *s_wired; - gs_free_error GError *error = NULL; - gboolean success; const char *mac; char expected_mac[ETH_ALEN] = { 0x00, 0x11, 0xaa, 0xbb, 0xcc, 0x55 }; char expected_cloned_mac[ETH_ALEN] = { 0x00, 0x16, 0xaa, 0xbb, 0xcc, 0xfe }; - connection = nms_keyfile_reader_from_file (TEST_KEYFILES_DIR "/Test_MAC_Old_Format", &error); - g_assert_no_error (error); - g_assert (connection); - - success = nm_connection_verify (connection, &error); - g_assert_no_error (error); - g_assert (success); + connection = keyfile_read_connection_from_file (TEST_KEYFILES_DIR "/Test_MAC_Old_Format"); s_wired = nm_connection_get_setting_wired (connection); g_assert (s_wired); - /* MAC address */ mac = nm_setting_wired_get_mac_address (s_wired); g_assert (mac); g_assert (nm_utils_hwaddr_matches (mac, -1, expected_mac, ETH_ALEN)); - /* Cloned MAC address */ mac = nm_setting_wired_get_cloned_mac_address (s_wired); g_assert (mac); g_assert (nm_utils_hwaddr_matches (mac, -1, expected_cloned_mac, ETH_ALEN)); @@ -705,20 +659,12 @@ test_read_mac_ib_old_format (void) { gs_unref_object NMConnection *connection = NULL; NMSettingInfiniband *s_ib; - gs_free_error GError *error = NULL; - gboolean success; const char *mac; guint8 expected_mac[INFINIBAND_ALEN] = { 0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, - 0x77, 0x88, 0x99, 0x01, 0x12, 0x23, 0x34, 0x45, 0x56, 0x67, 0x78, 0x89, - 0x90 }; + 0x77, 0x88, 0x99, 0x01, 0x12, 0x23, 0x34, 0x45, 0x56, 0x67, 0x78, 0x89, + 0x90 }; - connection = nms_keyfile_reader_from_file (TEST_KEYFILES_DIR "/Test_MAC_IB_Old_Format", &error); - g_assert_no_error (error); - g_assert (connection); - - success = nm_connection_verify (connection, &error); - g_assert_no_error (error); - g_assert (success); + connection = keyfile_read_connection_from_file (TEST_KEYFILES_DIR "/Test_MAC_IB_Old_Format"); s_ib = nm_connection_get_setting_infiniband (connection); g_assert (s_ib); @@ -736,18 +682,11 @@ test_read_valid_wireless_connection (void) NMSettingConnection *s_con; NMSettingWireless *s_wireless; NMSettingIPConfig *s_ip4; - gs_free_error GError *error = NULL; const char *bssid; const guint8 expected_bssid[ETH_ALEN] = { 0x00, 0x1a, 0x33, 0x44, 0x99, 0x82 }; - gboolean success; - connection = nms_keyfile_reader_from_file (TEST_KEYFILES_DIR "/Test_Wireless_Connection", NULL); - g_assert (connection); - success = nm_connection_verify (connection, &error); - g_assert_no_error (error); - g_assert (success); + connection = keyfile_read_connection_from_file (TEST_KEYFILES_DIR "/Test_Wireless_Connection"); - /* ===== CONNECTION SETTING ===== */ s_con = nm_connection_get_setting_connection (connection); g_assert (s_con); g_assert_cmpstr (nm_setting_connection_get_id (s_con), ==, "Test Wireless Connection"); @@ -755,14 +694,12 @@ test_read_valid_wireless_connection (void) g_assert_cmpuint (nm_setting_connection_get_timestamp (s_con), ==, 1226604314); g_assert (nm_setting_connection_get_autoconnect (s_con) == FALSE); - /* ===== WIRELESS SETTING ===== */ s_wireless = nm_connection_get_setting_wireless (connection); g_assert (s_wireless); bssid = nm_setting_wireless_get_bssid (s_wireless); g_assert (bssid); g_assert (nm_utils_hwaddr_matches (bssid, -1, expected_bssid, sizeof (expected_bssid))); - /* ===== IPv4 SETTING ===== */ s_ip4 = nm_connection_get_setting_ip4_config (connection); g_assert (s_ip4); g_assert_cmpstr (nm_setting_ip_config_get_method (s_ip4), ==, NM_SETTING_IP4_CONFIG_METHOD_AUTO); @@ -838,24 +775,19 @@ test_read_string_ssid (void) { gs_unref_object NMConnection *connection = NULL; NMSettingWireless *s_wireless; - gs_free_error GError *error = NULL; GBytes *ssid; const guint8 *ssid_data; gsize ssid_len; const char *expected_ssid = "blah blah ssid 1234"; - gboolean success; - connection = nms_keyfile_reader_from_file (TEST_KEYFILES_DIR "/Test_String_SSID", NULL); - g_assert (connection); - success = nm_connection_verify (connection, &error); - g_assert_no_error (error); - g_assert (success); + connection = keyfile_read_connection_from_file (TEST_KEYFILES_DIR "/Test_String_SSID"); - /* ===== WIRELESS SETTING ===== */ s_wireless = nm_connection_get_setting_wireless (connection); g_assert (s_wireless); + ssid = nm_setting_wireless_get_ssid (s_wireless); g_assert (ssid); + ssid_data = g_bytes_get_data (ssid, &ssid_len); g_assert_cmpmem (ssid_data, ssid_len, expected_ssid, strlen (expected_ssid)); } @@ -922,22 +854,13 @@ test_read_intlist_ssid (void) { gs_unref_object NMConnection *connection = NULL; NMSettingWireless *s_wifi; - gs_free_error GError *error = NULL; - gboolean success; GBytes *ssid; const guint8 *ssid_data; gsize ssid_len; const char *expected_ssid = "blah1234"; - connection = nms_keyfile_reader_from_file (TEST_KEYFILES_DIR "/Test_Intlist_SSID", &error); - g_assert_no_error (error); - g_assert (connection); + connection = keyfile_read_connection_from_file (TEST_KEYFILES_DIR "/Test_Intlist_SSID"); - success = nm_connection_verify (connection, &error); - g_assert_no_error (error); - g_assert (success); - - /* SSID */ s_wifi = nm_connection_get_setting_wireless (connection); g_assert (s_wifi); @@ -1015,16 +938,10 @@ test_read_intlike_ssid (void) { gs_unref_object NMConnection *connection = NULL; NMSettingWireless *s_wifi; - gs_free_error GError *error = NULL; - gboolean success; GBytes *ssid; const char *expected_ssid = "101"; - connection = nms_keyfile_reader_from_file (TEST_KEYFILES_DIR "/Test_Intlike_SSID", &error); - nmtst_assert_success (connection, error); - - success = nm_connection_verify (connection, &error); - nmtst_assert_success (success, error); + connection = keyfile_read_connection_from_file (TEST_KEYFILES_DIR "/Test_Intlike_SSID"); s_wifi = nm_connection_get_setting_wireless (connection); g_assert (s_wifi); @@ -1039,16 +956,10 @@ test_read_intlike_ssid_2 (void) { gs_unref_object NMConnection *connection = NULL; NMSettingWireless *s_wifi; - gs_free_error GError *error = NULL; - gboolean success; GBytes *ssid; const char *expected_ssid = "11;12;13;"; - connection = nms_keyfile_reader_from_file (TEST_KEYFILES_DIR "/Test_Intlike_SSID_2", &error); - nmtst_assert_success (connection, error); - - success = nm_connection_verify (connection, &error); - nmtst_assert_success (success, error); + connection = keyfile_read_connection_from_file (TEST_KEYFILES_DIR "/Test_Intlike_SSID_2"); s_wifi = nm_connection_get_setting_wireless (connection); g_assert (s_wifi); @@ -1183,24 +1094,16 @@ test_read_bt_dun_connection (void) NMSettingBluetooth *s_bluetooth; NMSettingSerial *s_serial; NMSettingGsm *s_gsm; - gs_free_error GError *error = NULL; const char *bdaddr; const guint8 expected_bdaddr[ETH_ALEN] = { 0x00, 0x11, 0x22, 0x33, 0x44, 0x55 }; - gboolean success; - connection = nms_keyfile_reader_from_file (TEST_KEYFILES_DIR "/ATT_Data_Connect_BT", NULL); - g_assert (connection); - success = nm_connection_verify (connection, &error); - g_assert_no_error (error); - g_assert (success); + connection = keyfile_read_connection_from_file (TEST_KEYFILES_DIR "/ATT_Data_Connect_BT"); - /* ===== CONNECTION SETTING ===== */ s_con = nm_connection_get_setting_connection (connection); g_assert (s_con); g_assert_cmpstr (nm_setting_connection_get_id (s_con), ==, "AT&T Data Connect BT"); g_assert_cmpstr (nm_setting_connection_get_uuid (s_con), ==, "089130ab-ce28-46e4-ad77-d44869b03d19"); - /* ===== BLUETOOTH SETTING ===== */ s_bluetooth = nm_connection_get_setting_bluetooth (connection); g_assert (s_bluetooth); bdaddr = nm_setting_bluetooth_get_bdaddr (s_bluetooth); @@ -1208,14 +1111,12 @@ test_read_bt_dun_connection (void) g_assert (nm_utils_hwaddr_matches (bdaddr, -1, expected_bdaddr, sizeof (expected_bdaddr))); g_assert_cmpstr (nm_setting_bluetooth_get_connection_type (s_bluetooth), ==, NM_SETTING_BLUETOOTH_TYPE_DUN); - /* ===== GSM SETTING ===== */ s_gsm = nm_connection_get_setting_gsm (connection); g_assert (s_gsm); g_assert_cmpstr (nm_setting_gsm_get_apn (s_gsm), ==, "ISP.CINGULAR"); g_assert_cmpstr (nm_setting_gsm_get_username (s_gsm), ==, "ISP@CINGULARGPRS.COM"); g_assert_cmpstr (nm_setting_gsm_get_password (s_gsm), ==, "CINGULAR1"); - /* ===== SERIAL SETTING ===== */ s_serial = nm_connection_get_setting_serial (connection); g_assert (s_serial); g_assert (nm_setting_serial_get_parity (s_serial) == NM_SETTING_SERIAL_PARITY_ODD); @@ -1288,28 +1189,17 @@ test_read_gsm_connection (void) NMSettingConnection *s_con; NMSettingSerial *s_serial; NMSettingGsm *s_gsm; - gs_free_error GError *error = NULL; - gboolean success; - connection = nms_keyfile_reader_from_file (TEST_KEYFILES_DIR "/ATT_Data_Connect_Plain", &error); - g_assert_no_error (error); - g_assert (connection); + connection = keyfile_read_connection_from_file (TEST_KEYFILES_DIR "/ATT_Data_Connect_Plain"); - success = nm_connection_verify (connection, &error); - g_assert_no_error (error); - g_assert (success); - - /* ===== CONNECTION SETTING ===== */ s_con = nm_connection_get_setting_connection (connection); g_assert (s_con); g_assert_cmpstr (nm_setting_connection_get_id (s_con), ==, "AT&T Data Connect"); g_assert_cmpstr (nm_setting_connection_get_connection_type (s_con), ==, NM_SETTING_GSM_SETTING_NAME); - /* ===== BLUETOOTH SETTING ===== */ /* Plain GSM, so no BT setting expected */ g_assert (nm_connection_get_setting_bluetooth (connection) == NULL); - /* ===== GSM SETTING ===== */ s_gsm = nm_connection_get_setting_gsm (connection); g_assert (s_gsm); g_assert_cmpstr (nm_setting_gsm_get_apn (s_gsm), ==, "ISP.CINGULAR"); @@ -1321,7 +1211,6 @@ test_read_gsm_connection (void) g_assert_cmpstr (nm_setting_gsm_get_sim_id (s_gsm), ==, "89148000000060671234"); g_assert_cmpstr (nm_setting_gsm_get_sim_operator_id (s_gsm), ==, "310260"); - /* ===== SERIAL SETTING ===== */ s_serial = nm_connection_get_setting_serial (connection); g_assert (s_serial); g_assert_cmpint (nm_setting_serial_get_parity (s_serial), ==, NM_SETTING_SERIAL_PARITY_ODD); @@ -1387,25 +1276,16 @@ test_read_wired_8021x_tls_blob_connection (void) gs_unref_object NMConnection *connection = NULL; NMSettingWired *s_wired; NMSetting8021x *s_8021x; - gs_free_error GError *error = NULL; const char *tmp; - gboolean success; GBytes *blob; NMTST_EXPECT_NM_WARN ("keyfile: 802-1x.client-cert: certificate or key file '/CASA/dcbw/Desktop/certinfra/client.pem' does not exist*"); NMTST_EXPECT_NM_WARN ("keyfile: 802-1x.private-key: certificate or key file '/CASA/dcbw/Desktop/certinfra/client.pem' does not exist*"); - connection = nms_keyfile_reader_from_file (TEST_KEYFILES_DIR "/Test_Wired_TLS_Blob", &error); - g_assert_no_error (error); - g_assert (connection); - success = nm_connection_verify (connection, &error); - g_assert_no_error (error); - g_assert (success); + connection = keyfile_read_connection_from_file (TEST_KEYFILES_DIR "/Test_Wired_TLS_Blob"); - /* ===== Wired Setting ===== */ s_wired = nm_connection_get_setting_wired (connection); g_assert (s_wired != NULL); - /* ===== 802.1x Setting ===== */ s_8021x = nm_connection_get_setting_802_1x (connection); g_assert (s_8021x != NULL); @@ -1445,25 +1325,16 @@ test_read_wired_8021x_tls_bad_path_connection (void) gs_unref_object NMConnection *connection = NULL; NMSettingWired *s_wired; NMSetting8021x *s_8021x; - gs_free_error GError *error = NULL; const char *tmp; char *tmp2; - gboolean success; NMTST_EXPECT_NM_WARN ("*does not exist*"); - connection = nms_keyfile_reader_from_file (TEST_KEYFILES_DIR "/Test_Wired_TLS_Path_Missing", &error); + connection = keyfile_read_connection_from_file (TEST_KEYFILES_DIR "/Test_Wired_TLS_Path_Missing"); g_test_assert_expected_messages (); - g_assert_no_error (error); - g_assert (connection); - success = nm_connection_verify (connection, &error); - g_assert_no_error (error); - g_assert (success); - /* ===== Wired Setting ===== */ s_wired = nm_connection_get_setting_wired (connection); g_assert (s_wired != NULL); - /* ===== 802.1x Setting ===== */ s_8021x = nm_connection_get_setting_802_1x (connection); g_assert (s_8021x != NULL); @@ -1499,25 +1370,16 @@ test_read_wired_8021x_tls_old_connection (void) gs_unref_object NMConnection *connection = NULL; NMSettingWired *s_wired; NMSetting8021x *s_8021x; - gs_free_error GError *error = NULL; const char *tmp; - gboolean success; NMTST_EXPECT_NM_WARN ("keyfile: 802-1x.ca-cert: certificate or key file '/CASA/dcbw/Desktop/certinfra/CA/eaptest_ca_cert.pem' does not exist*"); NMTST_EXPECT_NM_WARN ("keyfile: 802-1x.client-cert: certificate or key file '/CASA/dcbw/Desktop/certinfra/client.pem' does not exist*"); NMTST_EXPECT_NM_WARN ("keyfile: 802-1x.private-key: certificate or key file '/CASA/dcbw/Desktop/certinfra/client.pem' does not exist*"); - connection = nms_keyfile_reader_from_file (TEST_KEYFILES_DIR "/Test_Wired_TLS_Old", &error); - g_assert_no_error (error); - g_assert (connection); - success = nm_connection_verify (connection, &error); - g_assert_no_error (error); - g_assert (success); + connection = keyfile_read_connection_from_file (TEST_KEYFILES_DIR "/Test_Wired_TLS_Old"); - /* ===== Wired Setting ===== */ s_wired = nm_connection_get_setting_wired (connection); g_assert (s_wired != NULL); - /* ===== 802.1x Setting ===== */ s_8021x = nm_connection_get_setting_802_1x (connection); g_assert (s_8021x != NULL); @@ -1547,23 +1409,14 @@ test_read_wired_8021x_tls_new_connection (void) gs_unref_object NMConnection *connection = NULL; NMSettingWired *s_wired; NMSetting8021x *s_8021x; - gs_free_error GError *error = NULL; const char *tmp; char *tmp2; - gboolean success; - connection = nms_keyfile_reader_from_file (TEST_KEYFILES_DIR "/Test_Wired_TLS_New", &error); - g_assert_no_error (error); - g_assert (connection); - success = nm_connection_verify (connection, &error); - g_assert_no_error (error); - g_assert (success); + connection = keyfile_read_connection_from_file (TEST_KEYFILES_DIR "/Test_Wired_TLS_New"); - /* ===== Wired Setting ===== */ s_wired = nm_connection_get_setting_wired (connection); g_assert (s_wired != NULL); - /* ===== 802.1x Setting ===== */ s_8021x = nm_connection_get_setting_802_1x (connection); g_assert (s_8021x != NULL); @@ -1701,12 +1554,7 @@ test_write_wired_8021x_tls_connection_path (void) g_clear_object (&reread); /* Read the connection back in and compare it to the one we just wrote out */ - reread = nms_keyfile_reader_from_file (testfile, &error); - if (!reread) { - g_assert (error); - g_warning ("Failed to re-read test connection: %s", error->message); - g_assert (reread); - } + reread = keyfile_read_connection_from_file (testfile); success = nm_connection_compare (connection, reread, NM_SETTING_COMPARE_FLAG_EXACT); if (!reread) { @@ -1817,12 +1665,7 @@ test_write_wired_8021x_tls_connection_blob (void) g_assert (g_file_test (new_priv_key, G_FILE_TEST_EXISTS)); /* Read the connection back in and compare it to the one we just wrote out */ - reread = nms_keyfile_reader_from_file (testfile, &error); - if (!reread) { - g_assert (error); - g_warning ("Failed to re-read test connection: %s", error->message); - g_assert (reread); - } + reread = keyfile_read_connection_from_file (testfile); /* Ensure the re-read connection's certificates use the path scheme */ s_8021x = nm_connection_get_setting_802_1x (reread); @@ -1862,29 +1705,20 @@ test_read_infiniband_connection (void) gs_unref_object NMConnection *connection = NULL; NMSettingConnection *s_con; NMSettingInfiniband *s_ib; - gs_free_error GError *error = NULL; const char *mac; guint8 expected_mac[INFINIBAND_ALEN] = { 0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, - 0x77, 0x88, 0x99, 0x01, 0x12, 0x23, 0x34, 0x45, 0x56, 0x67, 0x78, 0x89, - 0x90 }; + 0x77, 0x88, 0x99, 0x01, 0x12, 0x23, 0x34, 0x45, 0x56, 0x67, 0x78, 0x89, + 0x90 }; const char *expected_id = "Test InfiniBand Connection"; const char *expected_uuid = "4e80a56d-c99f-4aad-a6dd-b449bc398c57"; - gboolean success; - connection = nms_keyfile_reader_from_file (TEST_KEYFILES_DIR "/Test_InfiniBand_Connection", &error); - g_assert_no_error (error); - g_assert (connection); - success = nm_connection_verify (connection, &error); - g_assert_no_error (error); - g_assert (success); + connection = keyfile_read_connection_from_file (TEST_KEYFILES_DIR "/Test_InfiniBand_Connection"); - /* Connection setting */ s_con = nm_connection_get_setting_connection (connection); g_assert (s_con); g_assert_cmpstr (nm_setting_connection_get_id (s_con), ==, expected_id); g_assert_cmpstr (nm_setting_connection_get_uuid (s_con), ==, expected_uuid); - /* InfiniBand setting */ s_ib = nm_connection_get_setting_infiniband (connection); g_assert (s_ib); @@ -1952,31 +1786,21 @@ test_read_bridge_main (void) NMSettingConnection *s_con; NMSettingIPConfig *s_ip4; NMSettingBridge *s_bridge; - gs_free_error GError *error = NULL; const char *expected_id = "Test Bridge Main"; const char *expected_uuid = "8f061643-fe41-4d4c-a8d9-097d26e2ad3a"; - gboolean success; - connection = nms_keyfile_reader_from_file (TEST_KEYFILES_DIR "/Test_Bridge_Main", &error); - g_assert_no_error (error); - g_assert (connection); - success = nm_connection_verify (connection, &error); - g_assert_no_error (error); - g_assert (success); + connection = keyfile_read_connection_from_file (TEST_KEYFILES_DIR "/Test_Bridge_Main"); - /* Connection setting */ s_con = nm_connection_get_setting_connection (connection); g_assert (s_con); g_assert_cmpstr (nm_setting_connection_get_id (s_con), ==, expected_id); g_assert_cmpstr (nm_setting_connection_get_uuid (s_con), ==, expected_uuid); g_assert_cmpstr (nm_setting_connection_get_interface_name (s_con), ==, "br0"); - /* IPv4 setting */ s_ip4 = nm_connection_get_setting_ip4_config (connection); g_assert (s_ip4); g_assert_cmpstr (nm_setting_ip_config_get_method (s_ip4), ==, NM_SETTING_IP4_CONFIG_METHOD_AUTO); - /* Bridge setting */ s_bridge = nm_connection_get_setting_bridge (connection); g_assert (s_bridge); g_assert_cmpuint (nm_setting_bridge_get_forward_delay (s_bridge), ==, 2); @@ -2049,19 +1873,11 @@ test_read_bridge_component (void) NMSettingWired *s_wired; const char *mac; guint8 expected_mac[ETH_ALEN] = { 0x00, 0x22, 0x15, 0x59, 0x62, 0x97 }; - gs_free_error GError *error = NULL; const char *expected_id = "Test Bridge Component"; const char *expected_uuid = "d7b4f96c-c45e-4298-bef8-f48574f8c1c0"; - gboolean success; - connection = nms_keyfile_reader_from_file (TEST_KEYFILES_DIR "/Test_Bridge_Component", &error); - g_assert_no_error (error); - g_assert (connection); - success = nm_connection_verify (connection, &error); - g_assert_no_error (error); - g_assert (success); + connection = keyfile_read_connection_from_file (TEST_KEYFILES_DIR "/Test_Bridge_Component"); - /* Connection setting */ s_con = nm_connection_get_setting_connection (connection); g_assert (s_con); g_assert_cmpstr (nm_setting_connection_get_id (s_con), ==, expected_id); @@ -2069,14 +1885,12 @@ test_read_bridge_component (void) g_assert_cmpstr (nm_setting_connection_get_master (s_con), ==, "br0"); g_assert (nm_setting_connection_is_slave_type (s_con, NM_SETTING_BRIDGE_SETTING_NAME)); - /* Wired setting */ s_wired = nm_connection_get_setting_wired (connection); g_assert (s_wired); mac = nm_setting_wired_get_mac_address (s_wired); g_assert (mac); g_assert (nm_utils_hwaddr_matches (mac, -1, expected_mac, sizeof (expected_mac))); - /* BridgePort setting */ s_port = nm_connection_get_setting_bridge_port (connection); g_assert (s_port); g_assert (nm_setting_bridge_port_get_hairpin_mode (s_port)); @@ -2141,17 +1955,9 @@ test_read_new_wired_group_name (void) NMSettingWired *s_wired; const char *mac; guint8 expected_mac[ETH_ALEN] = { 0x00, 0x11, 0x22, 0x33, 0x44, 0x55 }; - gs_free_error GError *error = NULL; - gboolean success; - connection = nms_keyfile_reader_from_file (TEST_KEYFILES_DIR"/Test_New_Wired_Group_Name", &error); - g_assert_no_error (error); - g_assert (connection); - success = nm_connection_verify (connection, &error); - g_assert_no_error (error); - g_assert (success); + connection = keyfile_read_connection_from_file (TEST_KEYFILES_DIR"/Test_New_Wired_Group_Name"); - /* Wired setting */ s_wired = nm_connection_get_setting_wired (connection); g_assert (s_wired); g_assert_cmpint (nm_setting_wired_get_mtu (s_wired), ==, 1400); @@ -2221,14 +2027,8 @@ test_read_new_wireless_group_names (void) NMSettingWirelessSecurity *s_wsec; GBytes *ssid; const char *expected_ssid = "foobar"; - gs_free_error GError *error = NULL; - gboolean success; - connection = nms_keyfile_reader_from_file (TEST_KEYFILES_DIR"/Test_New_Wireless_Group_Names", &error); - nmtst_assert_success (connection, error); - - success = nm_connection_verify (connection, &error); - nmtst_assert_success (success, error); + connection = keyfile_read_connection_from_file (TEST_KEYFILES_DIR"/Test_New_Wireless_Group_Names"); s_wifi = nm_connection_get_setting_wireless (connection); g_assert (s_wifi); @@ -2323,17 +2123,9 @@ test_read_missing_vlan_setting (void) { gs_unref_object NMConnection *connection = NULL; NMSettingVlan *s_vlan; - gs_free_error GError *error = NULL; - gboolean success; - connection = nms_keyfile_reader_from_file (TEST_KEYFILES_DIR"/Test_Missing_Vlan_Setting", &error); - g_assert_no_error (error); - g_assert (connection); - success = nm_connection_verify (connection, &error); - g_assert_no_error (error); - g_assert (success); + connection = keyfile_read_connection_from_file (TEST_KEYFILES_DIR"/Test_Missing_Vlan_Setting"); - /* Ensure the VLAN setting exists */ s_vlan = nm_connection_get_setting_vlan (connection); g_assert (s_vlan); g_assert_cmpint (nm_setting_vlan_get_id (s_vlan), ==, 0); @@ -2345,17 +2137,9 @@ test_read_missing_vlan_flags (void) { gs_unref_object NMConnection *connection = NULL; NMSettingVlan *s_vlan; - gs_free_error GError *error = NULL; - gboolean success; - connection = nms_keyfile_reader_from_file (TEST_KEYFILES_DIR"/Test_Missing_Vlan_Flags", &error); - g_assert_no_error (error); - g_assert (connection); - success = nm_connection_verify (connection, &error); - g_assert_no_error (error); - g_assert (success); + connection = keyfile_read_connection_from_file (TEST_KEYFILES_DIR"/Test_Missing_Vlan_Flags"); - /* Ensure the VLAN setting exists */ s_vlan = nm_connection_get_setting_vlan (connection); g_assert (s_vlan); @@ -2368,15 +2152,8 @@ static void test_read_missing_id_uuid (void) { gs_unref_object NMConnection *connection = NULL; - gs_free_error GError *error = NULL; - gboolean success; - connection = nms_keyfile_reader_from_file (TEST_KEYFILES_DIR"/Test_Missing_ID_UUID", &error); - g_assert_no_error (error); - g_assert (connection); - success = nm_connection_verify (connection, &error); - g_assert_no_error (error); - g_assert (success); + connection = keyfile_read_connection_from_file (TEST_KEYFILES_DIR"/Test_Missing_ID_UUID"); /* Ensure the ID and UUID properties are there */ g_assert_cmpstr (nm_connection_get_id (connection), ==, "Test_Missing_ID_UUID"); @@ -2468,17 +2245,9 @@ test_read_enum_property (void) { gs_unref_object NMConnection *connection = NULL; NMSettingIPConfig *s_ip6; - gs_free_error GError *error = NULL; - gboolean success; - connection = nms_keyfile_reader_from_file (TEST_KEYFILES_DIR"/Test_Enum_Property", &error); - g_assert_no_error (error); - g_assert (connection); - success = nm_connection_verify (connection, &error); - g_assert_no_error (error); - g_assert (success); + connection = keyfile_read_connection_from_file (TEST_KEYFILES_DIR"/Test_Enum_Property"); - /* IPv6 setting */ s_ip6 = nm_connection_get_setting_ip6_config (connection); g_assert (s_ip6); g_assert_cmpint (nm_setting_ip6_config_get_ip6_privacy (NM_SETTING_IP6_CONFIG (s_ip6)), ==, NM_SETTING_IP6_CONFIG_PRIVACY_PREFER_TEMP_ADDR); @@ -2528,17 +2297,9 @@ test_read_flags_property (void) { gs_unref_object NMConnection *connection = NULL; NMSettingGsm *s_gsm; - gs_free_error GError *error = NULL; - gboolean success; - connection = nms_keyfile_reader_from_file (TEST_KEYFILES_DIR"/Test_Flags_Property", &error); - g_assert_no_error (error); - g_assert (connection); - success = nm_connection_verify (connection, &error); - g_assert_no_error (error); - g_assert (success); + connection = keyfile_read_connection_from_file (TEST_KEYFILES_DIR"/Test_Flags_Property"); - /* GSM setting */ s_gsm = nm_connection_get_setting_gsm (connection); g_assert (s_gsm); g_assert_cmpint (nm_setting_gsm_get_password_flags (s_gsm), ==, @@ -2591,14 +2352,8 @@ test_read_tc_config (void) NMTCQdisc *qdisc1, *qdisc2; NMTCAction *action1, *action2; NMTCTfilter *tfilter1, *tfilter2; - gs_free_error GError *error = NULL; - gboolean success; - connection = nms_keyfile_reader_from_file (TEST_KEYFILES_DIR "/Test_TC_Config", NULL); - g_assert (connection); - success = nm_connection_verify (connection, &error); - g_assert_no_error (error); - g_assert (success); + connection = keyfile_read_connection_from_file (TEST_KEYFILES_DIR "/Test_TC_Config"); s_tc = nm_connection_get_setting_tc_config (connection); g_assert (s_tc); From 2e0a95530f4150465ffd18f08bb0fe73891913f3 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 3 Oct 2018 00:02:30 +0200 Subject: [PATCH 06/10] keyfile/tests: assert against auto generated UUID for keyfile The algorithm for generating the UUID must be stable. Assert against that. --- src/settings/plugins/keyfile/tests/test-keyfile.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/settings/plugins/keyfile/tests/test-keyfile.c b/src/settings/plugins/keyfile/tests/test-keyfile.c index 5970a26b60..3cdbd17762 100644 --- a/src/settings/plugins/keyfile/tests/test-keyfile.c +++ b/src/settings/plugins/keyfile/tests/test-keyfile.c @@ -2152,12 +2152,15 @@ static void test_read_missing_id_uuid (void) { gs_unref_object NMConnection *connection = NULL; + gs_free char *expected_uuid = NULL; + const char *FILENAME = TEST_KEYFILES_DIR"/Test_Missing_ID_UUID"; - connection = keyfile_read_connection_from_file (TEST_KEYFILES_DIR"/Test_Missing_ID_UUID"); + expected_uuid = _nm_utils_uuid_generate_from_strings ("keyfile", FILENAME, NULL); + + connection = keyfile_read_connection_from_file (FILENAME); - /* Ensure the ID and UUID properties are there */ g_assert_cmpstr (nm_connection_get_id (connection), ==, "Test_Missing_ID_UUID"); - g_assert (nm_connection_get_uuid (connection)); + g_assert_cmpstr (nm_connection_get_uuid (connection), ==, expected_uuid); } static void From 345c91a0a4adf132285151e143e697a81acaee2a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 27 Sep 2018 12:58:58 +0200 Subject: [PATCH 07/10] keyfile: move file permission check of keyfile to helper function --- .../plugins/keyfile/nms-keyfile-reader.c | 25 ++------ .../plugins/keyfile/nms-keyfile-utils.c | 60 +++++++++++++++++++ .../plugins/keyfile/nms-keyfile-utils.h | 8 +++ 3 files changed, 72 insertions(+), 21 deletions(-) diff --git a/src/settings/plugins/keyfile/nms-keyfile-reader.c b/src/settings/plugins/keyfile/nms-keyfile-reader.c index 71578686cb..4d61ebb45d 100644 --- a/src/settings/plugins/keyfile/nms-keyfile-reader.c +++ b/src/settings/plugins/keyfile/nms-keyfile-reader.c @@ -28,6 +28,7 @@ #include "nm-keyfile-internal.h" #include "NetworkManagerUtils.h" +#include "nms-keyfile-utils.h" /*****************************************************************************/ @@ -116,31 +117,13 @@ NMConnection * nms_keyfile_reader_from_file (const char *filename, GError **error) { gs_unref_keyfile GKeyFile *key_file = NULL; - struct stat statbuf; NMConnection *connection = NULL; GError *verify_error = NULL; - if (stat (filename, &statbuf) != 0 || !S_ISREG (statbuf.st_mode)) { - g_set_error_literal (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION, - "File did not exist or was not a regular file"); + if (!nms_keyfile_utils_check_file_permissions (filename, + NULL, + error)) return NULL; - } - - if (!NM_FLAGS_HAS (nm_utils_get_testing (), NM_UTILS_TEST_NO_KEYFILE_OWNER_CHECK)) { - if (statbuf.st_mode & 0077) { - g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION, - "File permissions (%o) were insecure", - statbuf.st_mode); - return NULL; - } - - if (statbuf.st_uid != 0) { - g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION, - "File owner (%o) is insecure", - statbuf.st_mode); - return NULL; - } - } key_file = g_key_file_new (); if (!g_key_file_load_from_file (key_file, filename, G_KEY_FILE_NONE, error)) diff --git a/src/settings/plugins/keyfile/nms-keyfile-utils.c b/src/settings/plugins/keyfile/nms-keyfile-utils.c index 03f06670fc..2a183d2f1a 100644 --- a/src/settings/plugins/keyfile/nms-keyfile-utils.c +++ b/src/settings/plugins/keyfile/nms-keyfile-utils.c @@ -24,6 +24,7 @@ #include #include +#include #include "nm-setting-wired.h" #include "nm-setting-wireless.h" @@ -113,6 +114,65 @@ nms_keyfile_utils_should_ignore_file (const char *filename) return FALSE; } +/*****************************************************************************/ + +gboolean +nms_keyfile_utils_check_file_permissions_stat (const struct stat *st, + GError **error) +{ + g_return_val_if_fail (st, FALSE); + + if (!S_ISREG (st->st_mode)) { + g_set_error_literal (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION, + "file is not a regular file"); + return FALSE; + } + + if (!NM_FLAGS_HAS (nm_utils_get_testing (), NM_UTILS_TEST_NO_KEYFILE_OWNER_CHECK)) { + if (st->st_uid != 0) { + g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION, + "File owner (%lld) is insecure", + (long long) st->st_uid); + return FALSE; + } + + if (st->st_mode & 0077) { + g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION, + "File permissions (%03o) are insecure", + st->st_mode); + return FALSE; + } + } + + return TRUE; +} + +gboolean +nms_keyfile_utils_check_file_permissions (const char *filename, + struct stat *out_st, + GError **error) +{ + struct stat st; + int errsv; + + g_return_val_if_fail (filename && filename[0] == '/', FALSE); + + if (stat (filename, &st) != 0) { + errsv = errno; + g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION, + "cannot access file: %s", g_strerror (errsv)); + return FALSE; + } + + if (!nms_keyfile_utils_check_file_permissions_stat (&st, error)) + return FALSE; + + NM_SET_OUT (out_st, st); + return TRUE; +} + +/*****************************************************************************/ + char * nms_keyfile_utils_escape_filename (const char *filename) { diff --git a/src/settings/plugins/keyfile/nms-keyfile-utils.h b/src/settings/plugins/keyfile/nms-keyfile-utils.h index ec3bd4414a..13a3eb009b 100644 --- a/src/settings/plugins/keyfile/nms-keyfile-utils.h +++ b/src/settings/plugins/keyfile/nms-keyfile-utils.h @@ -37,4 +37,12 @@ char *nms_keyfile_utils_escape_filename (const char *filename); const char *nms_keyfile_utils_get_path (void); +struct stat; +gboolean nms_keyfile_utils_check_file_permissions_stat (const struct stat *st, + GError **error); + +gboolean nms_keyfile_utils_check_file_permissions (const char *filename, + struct stat *out_st, + GError **error); + #endif /* __NMS_KEYFILE_UTILS_H__ */ From 2e5985f2e93b4475b54b535d87bf2f7a466246b9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 27 Sep 2018 13:35:42 +0200 Subject: [PATCH 08/10] keyfile: refactor check whether filename starts with a dot check_prefix() was only ever called with "." as prefix. Simplify the implementation to explicitly check for a leading dot. --- .../plugins/keyfile/nms-keyfile-utils.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/settings/plugins/keyfile/nms-keyfile-utils.c b/src/settings/plugins/keyfile/nms-keyfile-utils.c index 2a183d2f1a..3912e5d76b 100644 --- a/src/settings/plugins/keyfile/nms-keyfile-utils.c +++ b/src/settings/plugins/keyfile/nms-keyfile-utils.c @@ -56,18 +56,11 @@ check_mkstemp_suffix (const char *path) } static gboolean -check_prefix (const char *base, const char *tag) +check_prefix_dot (const char *base) { - int len, tag_len; + nm_assert (base && base[0]); - g_return_val_if_fail (base != NULL, TRUE); - g_return_val_if_fail (tag != NULL, TRUE); - - len = strlen (base); - tag_len = strlen (tag); - if ((len > tag_len) && !g_ascii_strncasecmp (base, tag, tag_len)) - return TRUE; - return FALSE; + return base[0] == '.'; } static gboolean @@ -102,7 +95,7 @@ nms_keyfile_utils_should_ignore_file (const char *filename) /* Ignore hidden and backup files */ /* should_ignore_file() must mirror escape_filename() */ - if (check_prefix (base, ".") || check_suffix (base, "~")) + if (check_prefix_dot (base) || check_suffix (base, "~")) return TRUE; /* Ignore temporary files */ if (check_mkstemp_suffix (base)) @@ -198,7 +191,7 @@ nms_keyfile_utils_escape_filename (const char *filename) /* escape_filename() must avoid anything that should_ignore_file() would reject. * We can escape here more aggressivly then what we would read back. */ - if (check_prefix (str->str, ".")) + if (check_prefix_dot (str->str)) str->str[0] = ESCAPE_CHAR2; if (check_suffix (str->str, "~")) str->str[str->len - 1] = ESCAPE_CHAR2; From 02c8844178380c5f4cc1c6aa971f90cfea2be0dc Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 2 Oct 2018 19:44:31 +0200 Subject: [PATCH 09/10] keyfile: refactor setting default ID/UUID in nm_keyfile_read() Split out the functionality for auto-detecting the ID and UUID of a connection. First of all, nm_keyfile_read() is already overcomplicated. The next commit will require the caller to explicitly call these functions. --- libnm-core/nm-keyfile-internal.h | 6 ++++ libnm-core/nm-keyfile.c | 59 ++++++++++++++++++++++++-------- 2 files changed, 51 insertions(+), 14 deletions(-) diff --git a/libnm-core/nm-keyfile-internal.h b/libnm-core/nm-keyfile-internal.h index d6a1471162..a620d9aa1c 100644 --- a/libnm-core/nm-keyfile-internal.h +++ b/libnm-core/nm-keyfile-internal.h @@ -101,6 +101,12 @@ NMConnection *nm_keyfile_read (GKeyFile *keyfile, void *user_data, GError **error); +gboolean nm_keyfile_read_ensure_id (NMConnection *connection, + const char *fallback_id); + +gboolean nm_keyfile_read_ensure_uuid (NMConnection *connection, + const char *fallback_uuid_seed); + /*****************************************************************************/ typedef enum { diff --git a/libnm-core/nm-keyfile.c b/libnm-core/nm-keyfile.c index be37d2a8e3..4de0ca415f 100644 --- a/libnm-core/nm-keyfile.c +++ b/libnm-core/nm-keyfile.c @@ -2812,6 +2812,46 @@ read_vpn_secrets (KeyfileReaderInfo *info, NMSettingVpn *s_vpn) } } +gboolean +nm_keyfile_read_ensure_id (NMConnection *connection, + const char *fallback_id) +{ + NMSettingConnection *s_con; + + g_return_val_if_fail (NM_IS_CONNECTION (connection), FALSE); + g_return_val_if_fail (fallback_id, FALSE); + + s_con = nm_connection_get_setting_connection (connection); + g_return_val_if_fail (NM_IS_SETTING_CONNECTION (s_con), FALSE); + + if (nm_setting_connection_get_id (s_con)) + return FALSE; + + g_object_set (s_con, NM_SETTING_CONNECTION_ID, fallback_id, NULL); + return TRUE; +} + +gboolean +nm_keyfile_read_ensure_uuid (NMConnection *connection, + const char *fallback_uuid_seed) +{ + NMSettingConnection *s_con; + gs_free char *hashed_uuid = NULL; + + g_return_val_if_fail (NM_IS_CONNECTION (connection), FALSE); + g_return_val_if_fail (fallback_uuid_seed, FALSE); + + s_con = nm_connection_get_setting_connection (connection); + g_return_val_if_fail (NM_IS_SETTING_CONNECTION (s_con), FALSE); + + if (nm_setting_connection_get_uuid (s_con)) + return FALSE; + + hashed_uuid = _nm_utils_uuid_generate_from_strings ("keyfile", fallback_uuid_seed, NULL); + g_object_set (s_con, NM_SETTING_CONNECTION_UUID, hashed_uuid, NULL); + return TRUE; +} + /** * nm_keyfile_read: * @keyfile: the keyfile from which to create the connection @@ -2902,23 +2942,14 @@ nm_keyfile_read (GKeyFile *keyfile, nm_connection_add_setting (connection, NM_SETTING (s_con)); } - /* Make sure that we have 'id' even if not explicitly specified in the keyfile */ - if ( keyfile_name - && !nm_setting_connection_get_id (s_con)) { - gs_free char *base_name = NULL; + if (keyfile_name) { + gs_free char *basename = g_path_get_basename (keyfile_name); - base_name = g_path_get_basename (keyfile_name); - g_object_set (s_con, NM_SETTING_CONNECTION_ID, base_name, NULL); + nm_keyfile_read_ensure_id (connection, basename); } - /* Make sure that we have 'uuid' even if not explicitly specified in the keyfile */ - if ( keyfile_name - && !nm_setting_connection_get_uuid (s_con)) { - gs_free char *hashed_uuid = NULL; - - hashed_uuid = _nm_utils_uuid_generate_from_strings ("keyfile", keyfile_name, NULL); - g_object_set (s_con, NM_SETTING_CONNECTION_UUID, hashed_uuid, NULL); - } + if (keyfile_name) + nm_keyfile_read_ensure_uuid (connection, keyfile_name); /* Make sure that we have 'interface-name' even if it was specified in the * "wrong" (ie, deprecated) group. From 837d44ffa4bfb3ef1a1cd786336dcd2415e9259b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 2 Oct 2018 19:53:54 +0200 Subject: [PATCH 10/10] keyfile: split automatically setting ID/UUID for keyfile keyfile already supports omitting the "connection.id" and "connection.uuid". In that case, the ID would be taken from the keyfile's name, and the UUID was generated by md5 hashing the full filename. No longer do this during nm_keyfile_read(), instead let all callers call nm_keyfile_read_ensure_*() to their liking. This is done for two reasons: - a minor reason is, that one day we want to expose keyfile API as public API. That means, we also want to read keyfiles from stdin, where there is no filename available. The implementation which parses stdio needs to define their own way of auto-generating ID and UUID. Note how nm_keyfile_read()'s API no longer takes a filename as argument, which would be awkward for the stdin case. - Currently, we only support one keyfile directory, which (configurably) is "/etc/NetworkManager/system-connections". In the future, we want to support multiple keyfile dirctories, like "/var/run/NetworkManager/profiles" or "/usr/lib/NetworkManager/profiles". Here we want that a file "foo" (which does not specify a UUID) gets the same UUID regardless of the directory it is in. That seems better, because then the UUID won't change as you move the file between directories. Yes, that means, that the same UUID will be provided by multiple files, but NetworkManager must already cope with that situation anyway. Unfortunately, the UUID generation scheme hashes the full path. That means, we must hash the path name of the file "foo" inside the original "system-connections" directory. Refactor the code so that it accounds for a difference between the filename of the keyfile, and the profile_dir used for generating the UUID. --- libnm-core/nm-keyfile-internal.h | 1 - libnm-core/nm-keyfile.c | 38 ++----------- libnm-core/tests/test-keyfile.c | 46 +++++++++------- libnm-core/tests/test-setting.c | 6 ++- shared/nm-utils/nm-test-utils.h | 10 +++- .../plugins/ifcfg-rh/tests/test-ifcfg-rh.c | 2 +- .../plugins/keyfile/nms-keyfile-connection.c | 7 ++- .../plugins/keyfile/nms-keyfile-connection.h | 3 +- .../plugins/keyfile/nms-keyfile-plugin.c | 2 +- .../plugins/keyfile/nms-keyfile-reader.c | 54 +++++++++++++++++-- .../plugins/keyfile/nms-keyfile-reader.h | 6 ++- .../plugins/keyfile/nms-keyfile-writer.c | 5 +- .../plugins/keyfile/tests/test-keyfile.c | 1 + 13 files changed, 111 insertions(+), 70 deletions(-) diff --git a/libnm-core/nm-keyfile-internal.h b/libnm-core/nm-keyfile-internal.h index a620d9aa1c..94228d9512 100644 --- a/libnm-core/nm-keyfile-internal.h +++ b/libnm-core/nm-keyfile-internal.h @@ -95,7 +95,6 @@ typedef struct { } NMKeyfileReadTypeDataWarn; NMConnection *nm_keyfile_read (GKeyFile *keyfile, - const char *keyfile_name, const char *base_dir, NMKeyfileReadHandler handler, void *user_data, diff --git a/libnm-core/nm-keyfile.c b/libnm-core/nm-keyfile.c index 4de0ca415f..77c1534d53 100644 --- a/libnm-core/nm-keyfile.c +++ b/libnm-core/nm-keyfile.c @@ -2855,17 +2855,9 @@ nm_keyfile_read_ensure_uuid (NMConnection *connection, /** * nm_keyfile_read: * @keyfile: the keyfile from which to create the connection - * @keyfile_name: keyfile allows missing connection id and uuid - * and NetworkManager will create those when reading a connection - * from file. By providing a filename you can reproduce that behavior, - * but of course, it can only recreate the same UUID if you provide the - * same filename as NetworkManager core daemon would. - * @keyfile_name has only a relevance for setting the id or uuid if it - * is missing and as fallback for @base_dir. * @base_dir: when reading certificates from files with relative name, - * the relative path is made absolute using @base_dir. - * If @base_dir is missing, first try to get the pathname from @keyfile_name - * (if it is given as absolute path). As last, fallback to the current path. + * the relative path is made absolute using @base_dir. This must + * be an absolute path. * @handler: read handler * @user_data: user data for read handler * @error: error @@ -2877,7 +2869,6 @@ nm_keyfile_read_ensure_uuid (NMConnection *connection, */ NMConnection * nm_keyfile_read (GKeyFile *keyfile, - const char *keyfile_name, const char *base_dir, NMKeyfileReadHandler handler, void *user_data, @@ -2888,25 +2879,13 @@ nm_keyfile_read (GKeyFile *keyfile, NMSetting *setting; char **groups; gsize length; - int i; + gsize i; gboolean vpn_secrets = FALSE; KeyfileReaderInfo info = { 0 }; - gs_free char *base_dir_free = NULL; g_return_val_if_fail (keyfile, NULL); g_return_val_if_fail (!error || !*error, NULL); - - if (!base_dir) { - /* basedir is not given. Prefer it from the keyfile_name */ - if (keyfile_name && keyfile_name[0] == '/') { - base_dir = base_dir_free = g_path_get_dirname (keyfile_name); - } else { - /* if keyfile is not given or not an absolute path, fallback - * to current working directory. */ - base_dir = base_dir_free = g_get_current_dir (); - } - } else - g_return_val_if_fail (base_dir[0] == '/', NULL); + g_return_val_if_fail (base_dir && base_dir[0] == '/', NULL); connection = nm_simple_connection_new (); @@ -2942,15 +2921,6 @@ nm_keyfile_read (GKeyFile *keyfile, nm_connection_add_setting (connection, NM_SETTING (s_con)); } - if (keyfile_name) { - gs_free char *basename = g_path_get_basename (keyfile_name); - - nm_keyfile_read_ensure_id (connection, basename); - } - - if (keyfile_name) - nm_keyfile_read_ensure_uuid (connection, keyfile_name); - /* Make sure that we have 'interface-name' even if it was specified in the * "wrong" (ie, deprecated) group. */ diff --git a/libnm-core/tests/test-keyfile.c b/libnm-core/tests/test-keyfile.c index 865cb68ccd..9b9d1c0415 100644 --- a/libnm-core/tests/test-keyfile.c +++ b/libnm-core/tests/test-keyfile.c @@ -167,19 +167,28 @@ _nm_keyfile_write (NMConnection *connection, static NMConnection * _nm_keyfile_read (GKeyFile *keyfile, const char *keyfile_name, - const char *base_dir, NMKeyfileReadHandler read_handler, void *read_data, gboolean needs_normalization) { GError *error = NULL; NMConnection *con; + gs_free char *filename = NULL; + gs_free char *base_dir = NULL; g_assert (keyfile); + g_assert (!keyfile_name || (keyfile_name[0] == '/')); - con = nm_keyfile_read (keyfile, keyfile_name, base_dir, read_handler, read_data, &error); + base_dir = g_path_get_dirname (keyfile_name); + filename = g_path_get_basename (keyfile_name); + + con = nm_keyfile_read (keyfile, base_dir, read_handler, read_data, &error); g_assert_no_error (error); g_assert (NM_IS_CONNECTION (con)); + + nm_keyfile_read_ensure_id (con, filename); + nm_keyfile_read_ensure_uuid (con, keyfile_name); + if (needs_normalization) { nmtst_assert_connection_verifies_after_normalization (con, 0, 0); nmtst_connection_normalize (con); @@ -205,7 +214,6 @@ static void _keyfile_convert (NMConnection **con, GKeyFile **keyfile, const char *keyfile_name, - const char *base_dir, NMKeyfileReadHandler read_handler, void *read_data, NMKeyfileWriteHandler write_handler, @@ -229,7 +237,7 @@ _keyfile_convert (NMConnection **con, if (c0) { c0_k1 = _nm_keyfile_write (c0, write_handler, write_data); - c0_k1_c2 = _nm_keyfile_read (c0_k1, keyfile_name, base_dir, read_handler, read_data, FALSE); + c0_k1_c2 = _nm_keyfile_read (c0_k1, keyfile_name, read_handler, read_data, FALSE); c0_k1_c2_k3 = _nm_keyfile_write (c0_k1_c2, write_handler, write_data); g_assert (_nm_keyfile_equals (c0_k1, c0_k1_c2_k3, TRUE)); @@ -237,9 +245,9 @@ _keyfile_convert (NMConnection **con, if (k0) { NMSetting8021x *s1, *s2; - k0_c1 = _nm_keyfile_read (k0, keyfile_name, base_dir, read_handler, read_data, needs_normalization); + k0_c1 = _nm_keyfile_read (k0, keyfile_name, read_handler, read_data, needs_normalization); k0_c1_k2 = _nm_keyfile_write (k0_c1, write_handler, write_data); - k0_c1_k2_c3 = _nm_keyfile_read (k0_c1_k2, keyfile_name, base_dir, read_handler, read_data, FALSE); + k0_c1_k2_c3 = _nm_keyfile_read (k0_c1_k2, keyfile_name, read_handler, read_data, FALSE); /* It is a expected behavior, that if @k0 contains a relative path ca-cert, @k0_c1 will * contain that path as relative. But @k0_c1_k2 and @k0_c1_k2_c3 will have absolute paths. @@ -312,7 +320,7 @@ _test_8021x_cert_check (NMConnection *con, NMSetting8021x *s_8021x; gs_free char *kval = NULL; - _keyfile_convert (&con, &keyfile, NULL, NULL, NULL, NULL, NULL, NULL, FALSE); + _keyfile_convert (&con, &keyfile, "/_test_8021x_cert_check/foo", NULL, NULL, NULL, NULL, FALSE); s_8021x = nm_connection_get_setting_802_1x (con); @@ -449,14 +457,14 @@ test_8021x_cert_read (void) con = nmtst_create_connection_from_keyfile ( "[connection]\n" "type=ethernet", - "/test_8021x_cert_read/test0", NULL); + "/test_8021x_cert_read/test0"); CLEAR (&con, &keyfile); keyfile = _keyfile_load_from_data ( "[connection]\n" "type=ethernet" ); - _keyfile_convert (&con, &keyfile, "/test_8021x_cert_read/test1", NULL, NULL, NULL, NULL, NULL, TRUE); + _keyfile_convert (&con, &keyfile, "/test_8021x_cert_read/test1", NULL, NULL, NULL, NULL, TRUE); CLEAR (&con, &keyfile); keyfile = _keyfile_load_from_data ( @@ -471,7 +479,7 @@ test_8021x_cert_read (void) "private-key=102;105;108;101;58;47;47;47;104;111;109;101;47;100;99;98;119;47;68;101;115;107;116;111;112;47;99;101;114;116;105;110;102;114;97;47;99;108;105;101;110;116;46;112;101;109;0;\n" "private-key-password=12345testing\n" ); - _keyfile_convert (&con, &keyfile, "/test_8021x_cert_read/test2", NULL, NULL, NULL, NULL, NULL, TRUE); + _keyfile_convert (&con, &keyfile, "/test_8021x_cert_read/test2", NULL, NULL, NULL, NULL, TRUE); CLEAR (&con, &keyfile); keyfile = _keyfile_load_from_data ( @@ -500,7 +508,7 @@ test_8021x_cert_read (void) "/33333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333111111\n" "private-key-password=12345testing\n" ); - _keyfile_convert (&con, &keyfile, "/test_8021x_cert_read/test2", NULL, NULL, NULL, NULL, NULL, TRUE); + _keyfile_convert (&con, &keyfile, "/test_8021x_cert_read/test2", NULL, NULL, NULL, NULL, TRUE); s_8021x = nm_connection_get_setting_802_1x (con); g_assert (nm_setting_802_1x_get_ca_cert_scheme (s_8021x) == NM_SETTING_802_1X_CK_SCHEME_PATH); @@ -528,7 +536,7 @@ test_8021x_cert_read (void) "private-key=data:;base64,aGFsbG8=\n" // hallo "private-key-password=12345testing\n" ); - _keyfile_convert (&con, &keyfile, "/test_8021x_cert_read/test2", NULL, NULL, NULL, NULL, NULL, TRUE); + _keyfile_convert (&con, &keyfile, "/test_8021x_cert_read/test2", NULL, NULL, NULL, NULL, TRUE); s_8021x = nm_connection_get_setting_802_1x (con); g_assert (nm_setting_802_1x_get_ca_cert_scheme (s_8021x) == NM_SETTING_802_1X_CK_SCHEME_PATH); @@ -553,7 +561,7 @@ test_8021x_cert_read (void) "private-key=abc.deR\n" "private-key-password=12345testing\n" ); - _keyfile_convert (&con, &keyfile, "/test_8021x_cert_read/test2", NULL, NULL, NULL, NULL, NULL, TRUE); + _keyfile_convert (&con, &keyfile, "/test_8021x_cert_read/test2", NULL, NULL, NULL, NULL, TRUE); s_8021x = nm_connection_get_setting_802_1x (con); g_assert (nm_setting_802_1x_get_ca_cert_scheme (s_8021x) == NM_SETTING_802_1X_CK_SCHEME_PATH); @@ -578,7 +586,7 @@ test_8021x_cert_read (void) "private-key=hallo\n" "private-key-password=12345testing\n" ); - _keyfile_convert (&con, &keyfile, "/test_8021x_cert_read/test2", NULL, NULL, NULL, NULL, NULL, TRUE); + _keyfile_convert (&con, &keyfile, "/test_8021x_cert_read/test2", NULL, NULL, NULL, NULL, TRUE); s_8021x = nm_connection_get_setting_802_1x (con); g_assert (nm_setting_802_1x_get_ca_cert_scheme (s_8021x) == NM_SETTING_802_1X_CK_SCHEME_BLOB); @@ -605,7 +613,7 @@ test_team_conf_read_valid (void) "interface-name=nm-team1\n" "[team]\n" "config={\"foo\":\"bar\"}", - "/test_team_conf_read/valid", NULL); + "/test_team_conf_read/valid"); g_assert (con); s_team = nm_connection_get_setting_team (con); @@ -629,7 +637,7 @@ test_team_conf_read_invalid (void) "interface-name=nm-team1\n" "[team]\n" "config={foobar}", - "/test_team_conf_read/invalid", NULL); + "/test_team_conf_read/invalid"); g_assert (con); s_team = nm_connection_get_setting_team (con); @@ -657,7 +665,7 @@ test_user_1 (void) "[user]\n" "my-value.x=value1\n" "", - "/test_user_1/invalid", NULL); + "/test_user_1/invalid"); g_assert (con); s_user = NM_SETTING_USER (nm_connection_get_setting (con, NM_TYPE_SETTING_USER)); g_assert (s_user); @@ -704,7 +712,7 @@ test_user_1 (void) nm_connection_add_setting (con, NM_SETTING (s_user)); nmtst_connection_normalize (con); - _keyfile_convert (&con, &keyfile, NULL, NULL, NULL, NULL, NULL, NULL, FALSE); + _keyfile_convert (&con, &keyfile, "/test_user_1/foo", NULL, NULL, NULL, NULL, FALSE); } /*****************************************************************************/ @@ -725,7 +733,7 @@ test_vpn_1 (void) "service-type=a.b.c\n" "vpn-key-1=value1\n" "", - "/test_vpn_1/invalid", NULL); + "/test_vpn_1/invalid"); g_assert (con); s_vpn = NM_SETTING_VPN (nm_connection_get_setting (con, NM_TYPE_SETTING_VPN)); g_assert (s_vpn); diff --git a/libnm-core/tests/test-setting.c b/libnm-core/tests/test-setting.c index c8eda381ee..4e010feb6e 100644 --- a/libnm-core/tests/test-setting.c +++ b/libnm-core/tests/test-setting.c @@ -1319,13 +1319,15 @@ test_ethtool_1 (void) nmtst_assert_success (keyfile, error); con3 = nm_keyfile_read (keyfile, - "ethtool-keyfile-name", - NULL, + "/ignored/current/working/directory/for/loading/relative/paths", NULL, NULL, &error); nmtst_assert_success (con3, error); + nm_keyfile_read_ensure_id (con3, "unused-because-already-has-id"); + nm_keyfile_read_ensure_uuid (con3, "unused-because-already-has-uuid"); + nmtst_connection_normalize (con3); nmtst_assert_connection_equals (con, FALSE, con3, FALSE); diff --git a/shared/nm-utils/nm-test-utils.h b/shared/nm-utils/nm-test-utils.h index b575382e4c..c813986210 100644 --- a/shared/nm-utils/nm-test-utils.h +++ b/shared/nm-utils/nm-test-utils.h @@ -1857,26 +1857,32 @@ nmtst_assert_hwaddr_equals (gconstpointer hwaddr1, gssize hwaddr1_len, const cha #if defined(__NM_SIMPLE_CONNECTION_H__) && defined(__NM_SETTING_CONNECTION_H__) && defined(__NM_KEYFILE_INTERNAL_H__) static inline NMConnection * -nmtst_create_connection_from_keyfile (const char *keyfile_str, const char *keyfile_name, const char *base_dir) +nmtst_create_connection_from_keyfile (const char *keyfile_str, const char *full_filename) { GKeyFile *keyfile; GError *error = NULL; gboolean success; NMConnection *con; + gs_free char *filename = g_path_get_basename (full_filename); + gs_free char *base_dir = g_path_get_dirname (full_filename); g_assert (keyfile_str); + g_assert (full_filename && full_filename[0] == '/'); keyfile = g_key_file_new (); success = g_key_file_load_from_data (keyfile, keyfile_str, strlen (keyfile_str), G_KEY_FILE_NONE, &error); g_assert_no_error (error); g_assert (success); - con = nm_keyfile_read (keyfile, keyfile_name, base_dir, NULL, NULL, &error); + con = nm_keyfile_read (keyfile, base_dir, NULL, NULL, &error); g_assert_no_error (error); g_assert (NM_IS_CONNECTION (con)); g_key_file_unref (keyfile); + nm_keyfile_read_ensure_id (con, filename); + nm_keyfile_read_ensure_uuid (con, full_filename); + nmtst_connection_normalize (con); return con; diff --git a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c index 0c0dd64ee5..472bb8a665 100644 --- a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c +++ b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c @@ -9054,7 +9054,7 @@ test_team_reread_slave (void) "id=142\n" "ingress-priority-map=\n" "parent=enp31s0f1\n" - , "/test_team_reread_slave", NULL); + , "/test_team_reread_slave"); /* to double-check keyfile syntax, re-create the connection by hand. */ connection_2 = nmtst_create_minimal_connection ("team-slave-enp31s0f1-142", "74f435bb-ede4-415a-9d48-f580b60eba04", NM_SETTING_VLAN_SETTING_NAME, &s_con); diff --git a/src/settings/plugins/keyfile/nms-keyfile-connection.c b/src/settings/plugins/keyfile/nms-keyfile-connection.c index 12467b2e20..7511f206ad 100644 --- a/src/settings/plugins/keyfile/nms-keyfile-connection.c +++ b/src/settings/plugins/keyfile/nms-keyfile-connection.c @@ -124,6 +124,7 @@ nms_keyfile_connection_init (NMSKeyfileConnection *connection) NMSKeyfileConnection * nms_keyfile_connection_new (NMConnection *source, const char *full_path, + const char *profile_dir, GError **error) { GObject *object; @@ -131,13 +132,15 @@ nms_keyfile_connection_new (NMConnection *source, const char *uuid; gboolean update_unsaved = TRUE; - g_assert (source || full_path); + nm_assert (source || full_path); + nm_assert (!full_path || full_path[0] == '/'); + nm_assert (!profile_dir || profile_dir[0] == '/'); /* If we're given a connection already, prefer that instead of re-reading */ if (source) tmp = g_object_ref (source); else { - tmp = nms_keyfile_reader_from_file (full_path, error); + tmp = nms_keyfile_reader_from_file (full_path, profile_dir, error); if (!tmp) return NULL; diff --git a/src/settings/plugins/keyfile/nms-keyfile-connection.h b/src/settings/plugins/keyfile/nms-keyfile-connection.h index f96d7590f0..0773ced0c0 100644 --- a/src/settings/plugins/keyfile/nms-keyfile-connection.h +++ b/src/settings/plugins/keyfile/nms-keyfile-connection.h @@ -37,7 +37,8 @@ typedef struct _NMSKeyfileConnectionClass NMSKeyfileConnectionClass; GType nms_keyfile_connection_get_type (void); NMSKeyfileConnection *nms_keyfile_connection_new (NMConnection *source, - const char *filename, + const char *full_path, + const char *profile_dir, GError **error); #endif /* __NMS_KEYFILE_CONNECTION_H__ */ diff --git a/src/settings/plugins/keyfile/nms-keyfile-plugin.c b/src/settings/plugins/keyfile/nms-keyfile-plugin.c index 3bb872ca27..b3a15ebd5f 100644 --- a/src/settings/plugins/keyfile/nms-keyfile-plugin.c +++ b/src/settings/plugins/keyfile/nms-keyfile-plugin.c @@ -195,7 +195,7 @@ update_connection (NMSKeyfilePlugin *self, return FALSE; } - connection_new = nms_keyfile_connection_new (source, full_path, &local); + connection_new = nms_keyfile_connection_new (source, full_path, nms_keyfile_utils_get_path (), &local); if (!connection_new) { /* Error; remove the connection */ if (source) diff --git a/src/settings/plugins/keyfile/nms-keyfile-reader.c b/src/settings/plugins/keyfile/nms-keyfile-reader.c index 4d61ebb45d..f417e4325f 100644 --- a/src/settings/plugins/keyfile/nms-keyfile-reader.c +++ b/src/settings/plugins/keyfile/nms-keyfile-reader.c @@ -103,33 +103,77 @@ _handler_read (GKeyFile *keyfile, NMConnection * nms_keyfile_reader_from_keyfile (GKeyFile *key_file, const char *filename, + const char *base_dir, + const char *profile_dir, gboolean verbose, GError **error) { + NMConnection *connection; HandlerReadData data = { .verbose = verbose, }; + gs_free char *base_dir_free = NULL; + gs_free char *profile_filename_free = NULL; + const char *profile_filename = NULL; - return nm_keyfile_read (key_file, filename, NULL, _handler_read, &data, error); + nm_assert (filename && filename[0]); + nm_assert (!base_dir || base_dir[0] == '/'); + nm_assert (!profile_dir || profile_dir[0] == '/'); + + if (base_dir) + nm_assert (!strchr (filename, '/')); + else { + const char *s; + + nm_assert (filename[0] == '/'); + + /* @base_dir may be NULL, in which case @filename must be an absolute path, + * and the directory is taken as the @base_dir. */ + s = strrchr (filename, '/'); + base_dir = nm_strndup_a (255, filename, s - filename, &base_dir_free); + if ( !profile_dir + || nm_streq (base_dir, profile_dir)) + profile_filename = filename; + filename = &s[1]; + } + + connection = nm_keyfile_read (key_file, base_dir, _handler_read, &data, error); + if (!connection) + return NULL; + + nm_keyfile_read_ensure_id (connection, filename); + + if (!profile_filename) { + profile_filename_free = g_build_filename (profile_dir ?: base_dir, filename, NULL); + profile_filename = profile_filename_free; + } + nm_keyfile_read_ensure_uuid (connection, profile_filename); + + return connection; } NMConnection * -nms_keyfile_reader_from_file (const char *filename, GError **error) +nms_keyfile_reader_from_file (const char *full_filename, + const char *profile_dir, + GError **error) { gs_unref_keyfile GKeyFile *key_file = NULL; NMConnection *connection = NULL; GError *verify_error = NULL; - if (!nms_keyfile_utils_check_file_permissions (filename, + nm_assert (full_filename && full_filename[0] == '/'); + nm_assert (!profile_dir || profile_dir[0] == '/'); + + if (!nms_keyfile_utils_check_file_permissions (full_filename, NULL, error)) return NULL; key_file = g_key_file_new (); - if (!g_key_file_load_from_file (key_file, filename, G_KEY_FILE_NONE, error)) + if (!g_key_file_load_from_file (key_file, full_filename, G_KEY_FILE_NONE, error)) return NULL; - connection = nms_keyfile_reader_from_keyfile (key_file, filename, TRUE, error); + connection = nms_keyfile_reader_from_keyfile (key_file, full_filename, NULL, profile_dir, TRUE, error); if (!connection) return NULL; diff --git a/src/settings/plugins/keyfile/nms-keyfile-reader.h b/src/settings/plugins/keyfile/nms-keyfile-reader.h index b60c1e691d..c0fb06d19e 100644 --- a/src/settings/plugins/keyfile/nms-keyfile-reader.h +++ b/src/settings/plugins/keyfile/nms-keyfile-reader.h @@ -26,9 +26,13 @@ NMConnection *nms_keyfile_reader_from_keyfile (GKeyFile *key_file, const char *filename, + const char *base_dir, + const char *profile_dir, gboolean verbose, GError **error); -NMConnection *nms_keyfile_reader_from_file (const char *filename, GError **error); +NMConnection *nms_keyfile_reader_from_file (const char *full_filename, + const char *profile_dir, + GError **error); #endif /* __NMS_KEYFILE_READER_H__ */ diff --git a/src/settings/plugins/keyfile/nms-keyfile-writer.c b/src/settings/plugins/keyfile/nms-keyfile-writer.c index 569be76731..6b7c0019c5 100644 --- a/src/settings/plugins/keyfile/nms-keyfile-writer.c +++ b/src/settings/plugins/keyfile/nms-keyfile-writer.c @@ -172,6 +172,7 @@ _handler_write (NMConnection *connection, static gboolean _internal_write_connection (NMConnection *connection, const char *keyfile_dir, + const char *profile_dir, uid_t owner_uid, pid_t owner_grp, const char *existing_path, @@ -308,7 +309,7 @@ _internal_write_connection (NMConnection *connection, gs_unref_object NMConnection *reread = NULL; gboolean reread_same = FALSE; - reread = nms_keyfile_reader_from_keyfile (key_file, path, FALSE, NULL); + reread = nms_keyfile_reader_from_keyfile (key_file, path, NULL, profile_dir, FALSE, NULL); nm_assert (NM_IS_CONNECTION (reread)); @@ -358,6 +359,7 @@ nms_keyfile_writer_connection (NMConnection *connection, return _internal_write_connection (connection, keyfile_dir, + nms_keyfile_utils_get_path (), 0, 0, existing_path, force_rename, @@ -378,6 +380,7 @@ nms_keyfile_writer_test_connection (NMConnection *connection, GError **error) { return _internal_write_connection (connection, + keyfile_dir, keyfile_dir, owner_uid, owner_grp, NULL, diff --git a/src/settings/plugins/keyfile/tests/test-keyfile.c b/src/settings/plugins/keyfile/tests/test-keyfile.c index 3cdbd17762..6b5fe90f0b 100644 --- a/src/settings/plugins/keyfile/tests/test-keyfile.c +++ b/src/settings/plugins/keyfile/tests/test-keyfile.c @@ -73,6 +73,7 @@ check_ip_route (NMSettingIPConfig *config, int idx, const char *destination, int g_assert (full_filename && full_filename[0] == '/'); \ \ _connection = nms_keyfile_reader_from_file (full_filename, \ + NULL, \ (nmtst_get_rand_int () % 2) ? &_error : NULL); \ nmtst_assert_success (_connection, _error); \ nmtst_assert_connection_verifies_without_normalization (_connection); \