diff --git a/src/settings/plugins/keyfile/nms-keyfile-connection.c b/src/settings/plugins/keyfile/nms-keyfile-connection.c index ff654acf8e..ca98b5677a 100644 --- a/src/settings/plugins/keyfile/nms-keyfile-connection.c +++ b/src/settings/plugins/keyfile/nms-keyfile-connection.c @@ -64,6 +64,8 @@ commit_changes (NMSettingsConnection *connection, NM_FLAGS_ALL (commit_reason, NM_SETTINGS_CONNECTION_COMMIT_REASON_USER_ACTION | NM_SETTINGS_CONNECTION_COMMIT_REASON_ID_CHANGED), &path, + NULL, + NULL, &error)) { callback (connection, error, user_data); g_clear_error (&error); diff --git a/src/settings/plugins/keyfile/nms-keyfile-plugin.c b/src/settings/plugins/keyfile/nms-keyfile-plugin.c index 97306d6656..0b084d6e55 100644 --- a/src/settings/plugins/keyfile/nms-keyfile-plugin.c +++ b/src/settings/plugins/keyfile/nms-keyfile-plugin.c @@ -532,7 +532,13 @@ add_connection (NMSettingsPlugin *config, gs_free char *path = NULL; if (save_to_disk) { - if (!nms_keyfile_writer_connection (connection, NULL, FALSE, &path, error)) + if (!nms_keyfile_writer_connection (connection, + NULL, + FALSE, + &path, + NULL, + NULL, + error)) return NULL; } return NM_SETTINGS_CONNECTION (update_connection (self, connection, path, NULL, FALSE, NULL, error)); diff --git a/src/settings/plugins/keyfile/nms-keyfile-reader.c b/src/settings/plugins/keyfile/nms-keyfile-reader.c index 39a0148021..05aae855a0 100644 --- a/src/settings/plugins/keyfile/nms-keyfile-reader.c +++ b/src/settings/plugins/keyfile/nms-keyfile-reader.c @@ -56,6 +56,10 @@ _fmt_warn (const char *group, NMSetting *setting, const char *property_name, con return message; } +typedef struct { + bool verbose; +} HandlerReadData; + static gboolean _handler_read (GKeyFile *keyfile, NMConnection *connection, @@ -64,11 +68,16 @@ _handler_read (GKeyFile *keyfile, void *user_data, GError **error) { + const HandlerReadData *handler_data = user_data; + if (type == NM_KEYFILE_READ_TYPE_WARN) { NMKeyfileReadTypeDataWarn *warn_data = type_data; NMLogLevel level; char *message_free = NULL; + if (!handler_data->verbose) + return TRUE; + if (warn_data->severity > NM_KEYFILE_WARN_SEVERITY_WARN) level = LOGL_ERR; else if (warn_data->severity >= NM_KEYFILE_WARN_SEVERITY_WARN) @@ -88,6 +97,19 @@ _handler_read (GKeyFile *keyfile, return FALSE; } +NMConnection * +nms_keyfile_reader_from_keyfile (GKeyFile *key_file, + const char *filename, + gboolean verbose, + GError **error) +{ + HandlerReadData data = { + .verbose = verbose, + }; + + return nm_keyfile_read (key_file, filename, NULL, _handler_read, &data, error); +} + NMConnection * nms_keyfile_reader_from_file (const char *filename, GError **error) { @@ -122,7 +144,7 @@ nms_keyfile_reader_from_file (const char *filename, GError **error) if (!g_key_file_load_from_file (key_file, filename, G_KEY_FILE_NONE, error)) goto out; - connection = nm_keyfile_read (key_file, filename, NULL, _handler_read, NULL, error); + connection = nms_keyfile_reader_from_keyfile (key_file, filename, TRUE, error); if (!connection) goto out; diff --git a/src/settings/plugins/keyfile/nms-keyfile-reader.h b/src/settings/plugins/keyfile/nms-keyfile-reader.h index c52fea31d2..f8b3fac8f4 100644 --- a/src/settings/plugins/keyfile/nms-keyfile-reader.h +++ b/src/settings/plugins/keyfile/nms-keyfile-reader.h @@ -24,6 +24,11 @@ #include +NMConnection *nms_keyfile_reader_from_keyfile (GKeyFile *key_file, + const char *filename, + gboolean verbose, + GError **error); + NMConnection *nms_keyfile_reader_from_file (const char *filename, 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 a673742050..92ed2849b5 100644 --- a/src/settings/plugins/keyfile/nms-keyfile-writer.c +++ b/src/settings/plugins/keyfile/nms-keyfile-writer.c @@ -32,6 +32,7 @@ #include "nm-keyfile-internal.h" #include "nms-keyfile-utils.h" +#include "nms-keyfile-reader.h" /*****************************************************************************/ @@ -174,9 +175,11 @@ _internal_write_connection (NMConnection *connection, const char *existing_path, gboolean force_rename, char **out_path, + NMConnection **out_reread, + gboolean *out_reread_same, GError **error) { - GKeyFile *key_file; + gs_unref_keyfile GKeyFile *key_file = NULL; gs_free char *data = NULL; gsize len; gs_free char *path = NULL; @@ -188,8 +191,15 @@ _internal_write_connection (NMConnection *connection, g_return_val_if_fail (!out_path || !*out_path, FALSE); g_return_val_if_fail (keyfile_dir && keyfile_dir[0] == '/', FALSE); - if (!nm_connection_verify (connection, error)) + switch (_nm_connection_verify (connection, error)) { + case NM_SETTING_VERIFY_NORMALIZABLE: + nm_assert_not_reached (); + /* fall-through */ + case NM_SETTING_VERIFY_SUCCESS: + break; + default: g_return_val_if_reached (FALSE); + } id = nm_connection_get_id (connection); g_assert (id && *id); @@ -200,7 +210,6 @@ _internal_write_connection (NMConnection *connection, if (!key_file) return FALSE; data = g_key_file_to_data (key_file, &len, error); - g_key_file_unref (key_file); if (!data) return FALSE; @@ -290,15 +299,48 @@ _internal_write_connection (NMConnection *connection, path = NULL; } + if (out_reread || out_reread_same) + { + gs_unref_object NMConnection *reread = NULL; + gboolean reread_same = FALSE; + + reread = nms_keyfile_reader_from_keyfile (key_file, path, FALSE, NULL); + + nm_assert (NM_IS_CONNECTION (reread)); + + if ( reread + && !nm_connection_normalize (reread, NULL, NULL, NULL)) { + nm_assert_not_reached (); + g_clear_object (&reread); + } + + if (reread && out_reread_same) { + reread_same = !!nm_connection_compare (reread, connection, NM_SETTING_COMPARE_FLAG_EXACT); + + nm_assert (reread_same == nm_connection_compare (connection, reread, NM_SETTING_COMPARE_FLAG_EXACT)); + nm_assert (reread_same == ({ + gs_unref_hashtable GHashTable *_settings = NULL; + + ( nm_connection_diff (reread, connection, NM_SETTING_COMPARE_FLAG_EXACT, &_settings) + && !_settings); + })); + } + + NM_SET_OUT (out_reread, g_steal_pointer (&reread)); + NM_SET_OUT (out_reread_same, reread_same); + } + return TRUE; } gboolean nms_keyfile_writer_connection (NMConnection *connection, - const char *existing_path, - gboolean force_rename, - char **out_path, - GError **error) + const char *existing_path, + gboolean force_rename, + char **out_path, + NMConnection **out_reread, + gboolean *out_reread_same, + GError **error) { return _internal_write_connection (connection, nms_keyfile_utils_get_path (), @@ -306,16 +348,20 @@ nms_keyfile_writer_connection (NMConnection *connection, existing_path, force_rename, out_path, + out_reread, + out_reread_same, error); } gboolean nms_keyfile_writer_test_connection (NMConnection *connection, - const char *keyfile_dir, - uid_t owner_uid, - pid_t owner_grp, - char **out_path, - GError **error) + const char *keyfile_dir, + uid_t owner_uid, + pid_t owner_grp, + char **out_path, + NMConnection **out_reread, + gboolean *out_reread_same, + GError **error) { return _internal_write_connection (connection, keyfile_dir, @@ -323,6 +369,8 @@ nms_keyfile_writer_test_connection (NMConnection *connection, NULL, FALSE, out_path, + out_reread, + out_reread_same, error); } diff --git a/src/settings/plugins/keyfile/nms-keyfile-writer.h b/src/settings/plugins/keyfile/nms-keyfile-writer.h index 4f43455d07..ca2e2a461f 100644 --- a/src/settings/plugins/keyfile/nms-keyfile-writer.h +++ b/src/settings/plugins/keyfile/nms-keyfile-writer.h @@ -28,6 +28,8 @@ gboolean nms_keyfile_writer_connection (NMConnection *connection, const char *existing_path, gboolean force_rename, char **out_path, + NMConnection **out_reread, + gboolean *out_reread_same, GError **error); gboolean nms_keyfile_writer_test_connection (NMConnection *connection, @@ -35,6 +37,8 @@ gboolean nms_keyfile_writer_test_connection (NMConnection *connection, uid_t owner_uid, pid_t owner_grp, char **out_path, + NMConnection **out_reread, + gboolean *out_reread_same, GError **error); #endif /* __NMS_KEYFILE_WRITER_H__ */ diff --git a/src/settings/plugins/keyfile/tests/test-keyfile.c b/src/settings/plugins/keyfile/tests/test-keyfile.c index f1102bd32b..1d1f5d8ccb 100644 --- a/src/settings/plugins/keyfile/tests/test-keyfile.c +++ b/src/settings/plugins/keyfile/tests/test-keyfile.c @@ -113,13 +113,25 @@ assert_reread_and_unlink (NMConnection *connection, gboolean normalize_connectio } static void -write_test_connection (NMConnection *connection, char **testfile) +assert_reread_same (NMConnection *connection, + NMConnection *reread) +{ + nmtst_assert_connection_verifies_without_normalization (reread); + nmtst_assert_connection_equals (connection, TRUE, reread, FALSE); +} + +static void +write_test_connection_reread (NMConnection *connection, + char **testfile, + NMConnection **out_reread, + gboolean *out_reread_same) { uid_t owner_uid; gid_t owner_grp; gboolean success; GError *error = NULL; GError **p_error = (nmtst_get_rand_int () % 2) ? &error : NULL; + gs_unref_object NMConnection *connection_normalized = NULL; g_assert (NM_IS_CONNECTION (connection)); g_assert (testfile && !*testfile); @@ -127,12 +139,32 @@ write_test_connection (NMConnection *connection, char **testfile) owner_uid = geteuid (); owner_grp = getegid (); - success = nms_keyfile_writer_test_connection (connection, TEST_SCRATCH_DIR, owner_uid, owner_grp, testfile, p_error); + connection_normalized = nmtst_connection_duplicate_and_normalize (connection); + + success = nms_keyfile_writer_test_connection (connection_normalized, + TEST_SCRATCH_DIR, + owner_uid, + owner_grp, + testfile, + out_reread, + out_reread_same, + p_error); g_assert_no_error (error); g_assert (success); g_assert (*testfile && (*testfile)[0]); } +static void +write_test_connection (NMConnection *connection, char **testfile) +{ + gs_unref_object NMConnection *reread = NULL; + gboolean reread_same = FALSE; + + write_test_connection_reread (connection, testfile, &reread, &reread_same); + assert_reread_same (connection, reread); + g_assert (reread_same); +} + static void write_test_connection_and_reread (NMConnection *connection, gboolean normalize_connection) { @@ -161,6 +193,27 @@ keyfile_load_from_file (const char *testfile) return keyfile; } +static void +_setting_copy_property_gbytes (NMConnection *src, NMConnection *dst, const char *setting_name, const char *property_name) +{ + gs_unref_bytes GBytes *blob = NULL; + NMSetting *s_src; + NMSetting *s_dst; + + g_assert (NM_IS_CONNECTION (src)); + g_assert (NM_IS_CONNECTION (dst)); + g_assert (setting_name); + g_assert (property_name); + + s_src = nm_connection_get_setting_by_name (src, setting_name); + g_assert (NM_IS_SETTING (s_src)); + s_dst = nm_connection_get_setting_by_name (dst, setting_name); + g_assert (NM_IS_SETTING (s_dst)); + + g_object_get (s_src, property_name, &blob, NULL); + g_object_set (s_dst, property_name, blob, NULL); +} + /*****************************************************************************/ static void @@ -1639,11 +1692,18 @@ test_write_wired_8021x_tls_connection_path (void) gs_free_error GError *error = NULL; gs_unref_keyfile GKeyFile *keyfile = NULL; gboolean relative = FALSE; + gboolean reread_same = FALSE; connection = create_wired_tls_connection (NM_SETTING_802_1X_CK_SCHEME_PATH); g_assert (connection != NULL); - write_test_connection (connection, &testfile); + write_test_connection_reread (connection, &testfile, &reread, &reread_same); + nmtst_assert_connection_verifies_without_normalization (reread); + _setting_copy_property_gbytes (connection, reread, NM_SETTING_802_1X_SETTING_NAME, NM_SETTING_802_1X_CA_CERT); + _setting_copy_property_gbytes (connection, reread, NM_SETTING_802_1X_SETTING_NAME, NM_SETTING_802_1X_CLIENT_CERT); + _setting_copy_property_gbytes (connection, reread, NM_SETTING_802_1X_SETTING_NAME, NM_SETTING_802_1X_PRIVATE_KEY); + assert_reread_same (connection, reread); + 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); @@ -1716,6 +1776,7 @@ test_write_wired_8021x_tls_connection_blob (void) char *new_client_cert; char *new_priv_key; const char *uuid; + gboolean reread_same = FALSE; gs_free_error GError *error = NULL; GBytes *password_raw = NULL; #define PASSWORD_RAW "password-raw\0test" @@ -1733,7 +1794,13 @@ test_write_wired_8021x_tls_connection_blob (void) NULL); g_bytes_unref (password_raw); - write_test_connection (connection, &testfile); + write_test_connection_reread (connection, &testfile, &reread, &reread_same); + nmtst_assert_connection_verifies_without_normalization (reread); + _setting_copy_property_gbytes (connection, reread, NM_SETTING_802_1X_SETTING_NAME, NM_SETTING_802_1X_CA_CERT); + _setting_copy_property_gbytes (connection, reread, NM_SETTING_802_1X_SETTING_NAME, NM_SETTING_802_1X_CLIENT_CERT); + _setting_copy_property_gbytes (connection, reread, NM_SETTING_802_1X_SETTING_NAME, NM_SETTING_802_1X_PRIVATE_KEY); + assert_reread_same (connection, reread); + g_clear_object (&reread); /* Check that the new certs got written out */ s_con = nm_connection_get_setting_connection (connection);