fdo-mirrors/NetworkManager
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/NetworkManager/NetworkManager.git synced 2026-01-05 04:30:16 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

ifcfg-rh: add internal API to re-read connection after write

Browse source 
Our reader/writer has flaws. We easily write out something that
is re-read differently. That is a problem and should be fixed.

Add API to re-read the connection after writing.

Extend the tests to check that the re-read value is identical
to what we wrote. In some cases, this does not hold. That is
usually a bug which needs fixing. Note that for certificate
blobs and paths we may intentionally mutate the connection
during writing, so there are valid cases where a connection
is re-read differently.
This commit is contained in:
Thomas Haller 2017-02-28 12:59:56 +01:00
parent 5ef4db18ce
commit 13e9967a3a
5 changed files with 174 additions and 31 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-connection.c
View file

@ -345,11 +345,15 @@ commit_changes (NMSettingsConnection *connection,
success = writer_update_connection (NM_CONNECTION (connection),
IFCFG_DIR,
filename,
NULL,
NULL,
&error);
} else {
success = writer_new_connection (NM_CONNECTION (connection),
IFCFG_DIR,
&ifcfg_path,
NULL,
NULL,
&error);
if (success) {
nm_settings_connection_set_filename (connection, ifcfg_path);

2
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-plugin.c
View file

@ -687,7 +687,7 @@ add_connection (NMSettingsPlugin *config,
return NULL;
if (save_to_disk) {
if (!writer_new_connection (connection, IFCFG_DIR, &path, error))
if (!writer_new_connection (connection, IFCFG_DIR, &path, NULL, NULL, error))
return NULL;
}
return NM_SETTINGS_CONNECTION (update_connection (self, connection, path, NULL, FALSE, NULL, error));

45
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c
View file

@ -2714,6 +2714,8 @@ write_connection (NMConnection *connection,
const char *ifcfg_dir,
const char *filename,
char **out_filename,
NMConnection **out_reread,
gboolean *out_reread_same,
GError **error)
{
NMSettingConnection *s_con;
@ -2725,6 +2727,7 @@ write_connection (NMConnection *connection,
nm_assert (NM_IS_CONNECTION (connection));
nm_assert (nm_connection_verify (connection, NULL));
nm_assert (!out_reread || !*out_reread);
if (!writer_can_write_connection (connection, error))
return FALSE;
@ -2850,6 +2853,40 @@ write_connection (NMConnection *connection,
if (!svWriteFile (ifcfg, 0644, error))
return FALSE;
if (out_reread || out_reread_same) {
gs_unref_object NMConnection *reread = NULL;
gs_free_error GError *local = NULL;
gs_free char *unhandled = NULL;
gboolean reread_same = FALSE;
reread = connection_from_file (ifcfg_name, &unhandled, &local, NULL);
if (unhandled) {
_LOGW ("failure to re-read the new connection from file \"%s\": %s",
ifcfg_name, "connection is unhandled");
g_clear_object (&reread);
} else if (!reread) {
_LOGW ("failure to re-read the new connection from file \"%s\": %s",
ifcfg_name, local ? local->message : "<unknown>");
} else {
if (out_reread_same) {
if (nm_connection_compare (reread, connection, NM_SETTING_COMPARE_FLAG_EXACT))
reread_same = TRUE;
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);
}
/* Only return the filename if this was a newly written ifcfg */
if (out_filename && !filename)
*out_filename = g_steal_pointer (&ifcfg_name);
@ -2886,15 +2923,19 @@ gboolean
writer_new_connection (NMConnection *connection,
const char *ifcfg_dir,
char **out_filename,
NMConnection **out_reread,
gboolean *out_reread_same,
GError **error)
{
return write_connection (connection, ifcfg_dir, NULL, out_filename, error);
return write_connection (connection, ifcfg_dir, NULL, out_filename, out_reread, out_reread_same, error);
}
gboolean
writer_update_connection (NMConnection *connection,
const char *ifcfg_dir,
const char *filename,
NMConnection **out_reread,
gboolean *out_reread_same,
GError **error)
{
if (utils_has_complex_routes (filename)) {
@ -2903,6 +2944,6 @@ writer_update_connection (NMConnection *connection,
return FALSE;
}
return write_connection (connection, ifcfg_dir, filename, NULL, error);
return write_connection (connection, ifcfg_dir, filename, NULL, out_reread, out_reread_same, error);
}

4
src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.h
View file

@ -29,11 +29,15 @@ gboolean writer_can_write_connection (NMConnection *connection,
gboolean writer_new_connection (NMConnection *connection,
const char *ifcfg_dir,
char **out_filename,
NMConnection **out_reread,
gboolean *out_reread_same,
GError **error);
gboolean writer_update_connection (NMConnection *connection,
const char *ifcfg_dir,
const char *filename,
NMConnection **out_reread,
gboolean *out_reread_same,
GError **error);
#endif /* _WRITER_H_ */

150
src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
View file

@ -88,22 +88,72 @@
|| ( _val_string && nm_streq0 (_val, _val_string))); \
} G_STMT_END
#define _writer_update_connection(connection, ifcfg_dir, filename) \
static void
_assert_reread_same (NMConnection *connection, NMConnection *reread)
{
nmtst_assert_connection_verifies_without_normalization (reread);
nmtst_assert_connection_equals (connection, TRUE, reread, FALSE);
}
static void
_assert_reread_same_FIXME (NMConnection *connection, NMConnection *reread)
{
gs_unref_object NMConnection *connection_normalized = NULL;
gs_unref_hashtable GHashTable *settings = NULL;
/* FIXME: these assertion failures should not happen as we expect
* that re-reading a connection after write yields the same result.
*
* Needs investation and fixing. */
nmtst_assert_connection_verifies_without_normalization (reread);
connection_normalized = nmtst_connection_duplicate_and_normalize (connection);
g_assert (!nm_connection_compare (connection_normalized, reread, NM_SETTING_COMPARE_FLAG_EXACT));
g_assert (!nm_connection_diff (connection_normalized, reread, NM_SETTING_COMPARE_FLAG_EXACT, &settings));
}
#define _writer_update_connection_reread(connection, ifcfg_dir, filename, out_reread, out_reread_same) \
G_STMT_START { \
NMConnection *_connection = (connection); \
NMConnection **_out_reread = (out_reread); \
gboolean *_out_reread_same = (out_reread_same); \
const char *_ifcfg_dir = (ifcfg_dir); \
const char *_filename = (filename); \
GError *_error = NULL; \
gboolean _success; \
\
g_assert (NM_IS_CONNECTION (connection)); \
g_assert (_ifcfg_dir && _ifcfg_dir[0]); \
g_assert (_filename && _filename[0]); \
\
_success = writer_update_connection (_connection, _ifcfg_dir, _filename, &_error); \
_success = writer_update_connection (_connection, _ifcfg_dir, _filename, _out_reread, _out_reread_same, &_error); \
nmtst_assert_success (_success, _error); \
} G_STMT_END
#define _writer_update_connection(connection, ifcfg_dir, filename) \
G_STMT_START { \
gs_unref_object NMConnection *_reread = NULL; \
NMConnection *_c = (connection); \
gboolean _reread_same = FALSE; \
\
_writer_update_connection_reread (_c, ifcfg_dir, filename, &_reread, &_reread_same); \
_assert_reread_same (_c, _reread); \
g_assert (_reread_same); \
} G_STMT_END
#define _writer_update_connection_FIXME(connection, ifcfg_dir, filename) \
G_STMT_START { \
gs_unref_object NMConnection *_reread = NULL; \
NMConnection *_c = (connection); \
gboolean _reread_same = FALSE; \
\
/* FIXME: this should not happen. Fix to use _writer_update_connection() */ \
\
_writer_update_connection_reread (_c, ifcfg_dir, filename, &_reread, &_reread_same); \
_assert_reread_same_FIXME (_c, _reread); \
g_assert (!_reread_same); \
} G_STMT_END
static NMConnection *
_connection_from_file (const char *filename,
const char *network_file,
@ -147,14 +197,18 @@ _connection_from_file_fail (const char *filename,
}
static void
_writer_new_connection (NMConnection *connection,
const char *ifcfg_dir,
char **out_filename)
_writer_new_connection_reread (NMConnection *connection,
const char *ifcfg_dir,
char **out_filename,
NMConnection **out_reread,
gboolean *out_reread_same)
{
gboolean success;
GError *error = NULL;
char *filename = NULL;
gs_unref_object NMConnection *con_verified = NULL;
gs_unref_object NMConnection *reread_copy = NULL;
NMConnection **reread = out_reread ?: ((nmtst_get_rand_int () % 2) ? &reread_copy : NULL);
g_assert (NM_IS_CONNECTION (connection));
g_assert (ifcfg_dir);
@ -164,21 +218,55 @@ _writer_new_connection (NMConnection *connection,
success = writer_new_connection (con_verified,
ifcfg_dir,
&filename,
reread,
out_reread_same,
&error);
nmtst_assert_success (success, error);
g_assert (filename && filename[0]);
if (reread)
nmtst_assert_connection_verifies_without_normalization (*reread);
if (out_filename)
*out_filename = filename;
else
g_free (filename);
}
static void
_writer_new_connection (NMConnection *connection,
const char *ifcfg_dir,
char **out_filename)
{
gs_unref_object NMConnection *reread = NULL;
gboolean reread_same = FALSE;
_writer_new_connection_reread (connection, ifcfg_dir, out_filename, &reread, &reread_same);
_assert_reread_same (connection, reread);
g_assert (reread_same);
}
static void
_writer_new_connection_FIXME (NMConnection *connection,
const char *ifcfg_dir,
char **out_filename)
{
gs_unref_object NMConnection *reread = NULL;
gboolean reread_same = FALSE;
/* FIXME: this should not happen. Fix it to use _writer_new_connection() instead. */
_writer_new_connection_reread (connection, ifcfg_dir, out_filename, &reread, &reread_same);
_assert_reread_same_FIXME (connection, reread);
g_assert (!reread_same);
}
static void
_writer_new_connection_fail (NMConnection *connection,
const char *ifcfg_dir,
GError **error)
{
gs_unref_object NMConnection *reread = NULL;
gboolean success;
GError *local = NULL;
char *filename = NULL;
@ -189,9 +277,12 @@ _writer_new_connection_fail (NMConnection *connection,
success = writer_new_connection (connection,
ifcfg_dir,
&filename,
&reread,
NULL,
&local);
nmtst_assert_no_success (success, local);
g_assert (!filename);
g_assert (!reread);
g_propagate_error (error, local);
}
@ -1567,12 +1658,15 @@ test_read_write_802_1X_subj_matches (void)
g_assert_cmpstr (nm_setting_802_1x_get_phase2_altsubject_match (s_8021x, 0), ==, "x.yourdomain.tld");
g_assert_cmpstr (nm_setting_802_1x_get_phase2_altsubject_match (s_8021x, 1), ==, "y.yourdomain.tld");
g_test_expect_message ("NetworkManager", G_LOG_LEVEL_MESSAGE,
"*missing IEEE_8021X_CA_CERT for EAP method 'peap'; this is insecure!");
_writer_new_connection (connection,
TEST_SCRATCH_DIR "/network-scripts/",
&testfile);
g_test_assert_expected_messages ();
g_test_expect_message ("NetworkManager", G_LOG_LEVEL_MESSAGE,
"*missing IEEE_8021X_CA_CERT*peap*");
"*missing IEEE_8021X_CA_CERT for EAP method 'peap'; this is insecure!");
reread = _connection_from_file (testfile, NULL, TYPE_ETHERNET, NULL);
g_test_assert_expected_messages ();
@ -1832,9 +1926,9 @@ test_clear_master (void)
g_assert_cmpstr (nm_setting_connection_get_slave_type (s_con), ==, NULL);
/* 4. update the connection on disk */
_writer_update_connection (connection,
TEST_SCRATCH_DIR "/network-scripts/",
testfile);
_writer_update_connection_FIXME (connection,
TEST_SCRATCH_DIR "/network-scripts/",
testfile);
keyfile = utils_get_keys_path (testfile);
g_assert (!g_file_test (keyfile, G_FILE_TEST_EXISTS));
@ -1917,9 +2011,9 @@ test_write_dns_options (void)
nmtst_assert_connection_verifies (connection);
_writer_new_connection (connection,
TEST_SCRATCH_DIR "/network-scripts/",
&testfile);
_writer_new_connection_FIXME (connection,
TEST_SCRATCH_DIR "/network-scripts/",
&testfile);
reread = _connection_from_file (testfile, NULL, TYPE_ETHERNET, NULL);
@ -3781,9 +3875,9 @@ test_write_wired_static (void)
nmtst_assert_connection_verifies (connection);
_writer_new_connection (connection,
TEST_SCRATCH_DIR "/network-scripts/",
&testfile);
_writer_new_connection_FIXME (connection,
TEST_SCRATCH_DIR "/network-scripts/",
&testfile);
route6file = utils_get_route6_path (testfile);
reread = _connection_from_file (testfile, NULL, TYPE_ETHERNET, NULL);
@ -4451,9 +4545,9 @@ test_write_wired_8021x_tls (gconstpointer test_data)
nmtst_assert_connection_verifies (connection);
_writer_new_connection (connection,
TEST_SCRATCH_DIR "/network-scripts/",
&testfile);
_writer_new_connection_FIXME (connection,
TEST_SCRATCH_DIR "/network-scripts/",
&testfile);
reread = _connection_from_file (testfile, NULL, TYPE_WIRELESS, NULL);
@ -4590,9 +4684,9 @@ test_write_wired_aliases (void)
svCloseFile (ifcfg);
g_assert (g_file_test (TEST_SCRATCH_ALIAS_BASE ":5", G_FILE_TEST_EXISTS));
_writer_new_connection (connection,
TEST_SCRATCH_DIR "/network-scripts/",
&testfile);
_writer_new_connection_FIXME (connection,
TEST_SCRATCH_DIR "/network-scripts/",
&testfile);
/* Re-check the alias files */
g_assert (g_file_test (TEST_SCRATCH_ALIAS_BASE ":2", G_FILE_TEST_EXISTS));
@ -5444,9 +5538,9 @@ test_write_wifi_leap_secret_flags (gconstpointer data)
nmtst_assert_connection_verifies (connection);
_writer_new_connection (connection,
TEST_SCRATCH_DIR "/network-scripts/",
&testfile);
_writer_new_connection_FIXME (connection,
TEST_SCRATCH_DIR "/network-scripts/",
&testfile);
reread = _connection_from_file (testfile, NULL, TYPE_WIRELESS, NULL);
@ -6592,9 +6686,9 @@ test_write_wifi_wep_agent_keys (void)
nmtst_assert_connection_verifies (connection);
_writer_new_connection (connection,
TEST_SCRATCH_DIR "/network-scripts/",
&testfile);
_writer_new_connection_FIXME (connection,
TEST_SCRATCH_DIR "/network-scripts/",
&testfile);
reread = _connection_from_file (testfile, NULL, TYPE_WIRELESS, NULL);