Commit 7b807b11 authored by Thomas Haller's avatar Thomas Haller

settings: avoid duplicate UUID in settings

When adding a connection to NMSettings we did not check for
duplicate connection UUIDs (which could for example happen
if two different plugins report a conflicting UUID).
Also, we would not check that an already added connection
changes it's UUID.

Both could lead to have duplicate connections (by UUID).

Avoid that two ways:
- when adding a connection to NMSettings, ensure that we don't add
  a conflicting UUID. Otherwise just bail out and do nothing.
- when modifying a connection that is already added to NMSettings,
  enforce that the UUID cannot change. Otherwise fail with error.

For ifcfg-rh plugin this situation still can happen during reload.
In this case error out and refuse to update the connection. After
all, the user configured invalid UUIDs.

https://bugzilla.redhat.com/show_bug.cgi?id=1171751
parent 8ba8a55c
......@@ -464,6 +464,15 @@ nm_settings_connection_replace_settings (NMSettingsConnection *self,
if (!nm_connection_normalize (new_connection, NULL, NULL, error))
return FALSE;
if ( nm_connection_get_path (NM_CONNECTION (self))
&& g_strcmp0 (nm_connection_get_uuid (NM_CONNECTION (self)), nm_connection_get_uuid (new_connection)) != 0) {
/* Updating the UUID is not allowed once the path is exported. */
g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
"connection %s cannot change the UUID from %s to %s", nm_connection_get_id (NM_CONNECTION (self)),
nm_connection_get_uuid (NM_CONNECTION (self)), nm_connection_get_uuid (new_connection));
return FALSE;
}
/* Do nothing if there's nothing to update */
if (nm_connection_compare (NM_CONNECTION (self),
new_connection,
......
......@@ -886,6 +886,7 @@ claim_connection (NMSettings *self,
GHashTableIter iter;
gpointer data;
char *path;
NMSettingsConnection *existing;
g_return_if_fail (NM_IS_SETTINGS_CONNECTION (connection));
g_return_if_fail (nm_connection_get_path (NM_CONNECTION (connection)) == NULL);
......@@ -904,6 +905,23 @@ claim_connection (NMSettings *self,
return;
}
existing = nm_settings_get_connection_by_uuid (self, nm_connection_get_uuid (NM_CONNECTION (connection)));
if (existing) {
/* Cannot add duplicate connections per UUID. Just return without action and
* log a warning.
*
* This means, that plugins must not provide duplicate connections (UUID).
* In fact, none of the plugins currently would do that.
*
* But globaly, over different setting plugins, there could be duplicates
* without the individual plugins being aware. Don't handle that at all, just
* error out. That should not happen unless the admin misconfigured the system
* to create conflicting connections. */
nm_log_warn (LOGD_SETTINGS, "plugin provided duplicate connection with UUID %s",
nm_connection_get_uuid (NM_CONNECTION (connection)));
return;
}
/* Read timestamp from look-aside file and put it into the connection's data */
nm_settings_connection_read_and_fill_timestamp (connection);
......
......@@ -129,6 +129,7 @@ _internal_new_connection (SCPluginIfcfg *self,
SCPluginIfcfgPrivate *priv = SC_PLUGIN_IFCFG_GET_PRIVATE (self);
NMIfcfgConnection *connection;
const char *cid;
const char *uuid;
if (!source)
nm_log_info (LOGD_SETTINGS, "parsing %s ... ", path);
......@@ -137,11 +138,20 @@ _internal_new_connection (SCPluginIfcfg *self,
if (!connection)
return NULL;
uuid = nm_connection_get_uuid (NM_CONNECTION (connection));
if (g_hash_table_contains (priv->connections, uuid)) {
g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
"Failed to add duplicate connection for UUID %s", uuid);
g_object_unref (connection);
return NULL;
}
cid = nm_connection_get_id (NM_CONNECTION (connection));
g_assert (cid);
g_hash_table_insert (priv->connections,
g_strdup (nm_connection_get_uuid (NM_CONNECTION (connection))),
g_strdup (uuid),
connection);
nm_log_info (LOGD_SETTINGS, " read connection '%s'", cid);
g_signal_connect (connection, NM_SETTINGS_CONNECTION_REMOVED,
......@@ -302,6 +312,15 @@ connection_new_or_changed (SCPluginIfcfg *self,
return;
}
if (g_strcmp0 (nm_connection_get_uuid (NM_CONNECTION (existing)), nm_connection_get_uuid (NM_CONNECTION (new))) != 0) {
/* FIXME: UUID changes are not supported by nm_settings_connection_replace_settings().
* This function should be merged with _internal_new_connection() to be like keyfiles
* update_connection(). */
nm_log_warn (LOGD_SETTINGS, "UUID changes are not supported. Cannot update connection %s (%s)", nm_settings_connection_get_filename (NM_SETTINGS_CONNECTION (new)), nm_connection_get_uuid (NM_CONNECTION (new)));
g_object_unref (new);
return;
}
nm_log_info (LOGD_SETTINGS, "updating %s", path);
g_object_set (existing,
NM_IFCFG_CONNECTION_UNMANAGED_SPEC, new_unmanaged,
......@@ -342,7 +361,8 @@ connection_new_or_changed (SCPluginIfcfg *self,
FALSE, /* don't set Unsaved */
"ifcfg-rh-update",
&error)) {
/* Shouldn't ever get here as 'new' was verified by the reader already */
/* Shouldn't ever get here as 'new' was verified by the reader already
* and the UUID did not change. */
g_assert_not_reached ();
}
g_assert_no_error (error);
......
......@@ -315,7 +315,8 @@ reload_connections (NMSystemConfigInterface *config)
FALSE, /* don't set Unsaved */
"ifnet-update",
&error)) {
/* Shouldn't ever get here as 'new' was verified by the reader already */
/* Shouldn't ever get here as 'new' was verified by the reader already
* and the UUID did not change. */
g_assert_not_reached ();
}
g_assert_no_error (error);
......
......@@ -251,7 +251,8 @@ update_connection (SCPluginKeyfile *self,
FALSE, /* don't set Unsaved */
"keyfile-update",
&local)) {
/* Shouldn't ever get here as 'connection_new' was verified by the reader already */
/* Shouldn't ever get here as 'connection_new' was verified by the reader already
* and the UUID did not change. */
g_assert_not_reached ();
}
g_assert_no_error (local);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment