colin/NetworkManager
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

checkpoint: preserve in-memory state of connections

Browse Source 
If a connection is in-memory (i.e. has flag "unsaved"), after a
checkpoint and rollback it can be wrongly persisted to disk:

 - if the connection was modified and written to disk after the
   rollback, during the rollback we update it again with persist mode
   "keep", which keeps it on disk;

 - if the connection was deleted after the rollback, during the
   rollback we add it again with persist mode "to-disk".

Instead, remember whether the connection had the "unsaved" flag set
and try to restore the previous state.

However, this is not straightforward as there are 4 different possible
states for the settings connection: persistent; in-memory only;
in-memory shadowing a persistent file; in-memory shadowing a detached
persistent file (i.e. the deletion of the connection doesn't delete
the persistent file). Handle all those cases.

Fixes: 3e09aed2a0 ('checkpoint: add create, rollback and destroy D-Bus API')
This commit is contained in:
Beniamino Galvani
2024-04-15 10:51:24 +02:00
parent a48b7fe7b9
commit c979bfeb8b
2 changed files with 203 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
NEWS
View File

@@ -15,6 +15,8 @@ USE AT YOUR OWN RISK. NOT RECOMMENDED FOR PRODUCTION USE!
visible in nmcli via "nmcli -f all device show $DEV". visible in nmcli via "nmcli -f all device show $DEV".
* Deprecated 802-11-wireless and 802-11-wired property 'mac-address-blacklist' * Deprecated 802-11-wireless and 802-11-wired property 'mac-address-blacklist'
and introduced the 'mac-address-denylist' property. and introduced the 'mac-address-denylist' property.
* Properly restore in-memory connection profiles during the rollback
of a checkpoint.
* Fix detection of 6 GHz band capability for WiFi devices * Fix detection of 6 GHz band capability for WiFi devices
* Allow IPv6 SLAAC and static IPv6 DNS server assignment for modem broadband * Allow IPv6 SLAAC and static IPv6 DNS server assignment for modem broadband
when IPv6 device address was not explicitly passed on by ModemManager when IPv6 device address was not explicitly passed on by ModemManager

238
src/core/nm-checkpoint.c
View File

@@ -10,6 +10,7 @@
#include "nm-active-connection.h" #include "nm-active-connection.h"
#include "nm-act-request.h" #include "nm-act-request.h"
#include "libnm-core-aux-intern/nm-auth-subject.h" #include "libnm-core-aux-intern/nm-auth-subject.h"
#include "libnm-core-intern/nm-keyfile-internal.h"
#include "nm-core-utils.h" #include "nm-core-utils.h"
#include "nm-dbus-interface.h" #include "nm-dbus-interface.h"
#include "devices/nm-device.h" #include "devices/nm-device.h"
@@ -17,6 +18,7 @@
#include "nm-manager.h" #include "nm-manager.h"
#include "settings/nm-settings.h" #include "settings/nm-settings.h"
#include "settings/nm-settings-connection.h" #include "settings/nm-settings-connection.h"
#include "settings/plugins/keyfile/nms-keyfile-storage.h"
#include "nm-simple-connection.h" #include "nm-simple-connection.h"
#include "nm-utils.h" #include "nm-utils.h"
@@ -29,11 +31,14 @@ typedef struct {
NMDevice *device; NMDevice *device;
NMConnection *applied_connection; NMConnection *applied_connection;
NMConnection *settings_connection; NMConnection *settings_connection;
NMConnection *settings_connection_shadowed;
guint64 ac_version_id; guint64 ac_version_id;
NMDeviceState state; NMDeviceState state;
bool is_software : 1; bool is_software : 1;
bool realized : 1; bool realized : 1;
bool activation_lifetime_bound_to_profile_visibility : 1; bool activation_lifetime_bound_to_profile_visibility : 1;
bool settings_connection_is_unsaved : 1;
bool settings_connection_is_shadowed_owned : 1;
NMUnmanFlagOp unmanaged_explicit; NMUnmanFlagOp unmanaged_explicit;
NMActivationReason activation_reason; NMActivationReason activation_reason;
gulong dev_exported_change_id; gulong dev_exported_change_id;
@@ -150,37 +155,111 @@ nm_checkpoint_includes_devices_of(NMCheckpoint *self, NMCheckpoint *cp_for_devic
return NULL; return NULL;
} }
static NMConnection *
parse_connection_from_shadowed_file(const char *path, GError **error)
{
nm_auto_unref_keyfile GKeyFile *keyfile = NULL;
gs_free char *base_dir = NULL;
char *sep;
keyfile = g_key_file_new();
if (!g_key_file_load_from_file(keyfile, path, G_KEY_FILE_NONE, error))
return NULL;
sep = strrchr(path, '/');
base_dir = g_strndup(path, sep - path);
return nm_keyfile_read(keyfile, base_dir, NM_KEYFILE_HANDLER_FLAGS_NONE, NULL, NULL, error);
}
static NMSettingsConnection * static NMSettingsConnection *
find_settings_connection(NMCheckpoint *self, find_settings_connection(NMCheckpoint *self,
DeviceCheckpoint *dev_checkpoint, DeviceCheckpoint *dev_checkpoint,
gboolean *need_update, gboolean *need_update,
gboolean *need_activation) gboolean *need_update_shadowed,
gboolean *need_activation,
NMSettingsConnectionPersistMode *persist_mode)
{ {
NMCheckpointPrivate *priv = NM_CHECKPOINT_GET_PRIVATE(self); NMCheckpointPrivate *priv = NM_CHECKPOINT_GET_PRIVATE(self);
NMActiveConnection *active; NMActiveConnection *active;
NMSettingsConnection *sett_conn; NMSettingsConnection *sett_conn;
const char *shadowed_file;
NMConnection *shadowed_connection = NULL;
const char *uuid, *ac_uuid; const char *uuid, *ac_uuid;
const CList *tmp_clist; const CList *tmp_clist;
gboolean sett_conn_unsaved;
NMSettingsStorage *storage;
*need_activation = FALSE; *need_activation = FALSE;
*need_update = FALSE; *need_update = FALSE;
*need_update_shadowed = FALSE;
/* With regard to storage, there are 4 different possible states for the settings
* connection: 1) persistent; 2) in-memory only; 3) in-memory shadowing a persistent
* file; 4) in-memory shadowing a detached persistent file (i.e. the deletion of
* the connection doesn't delete the persistent file).
*/
if (dev_checkpoint->settings_connection_is_unsaved) {
if (dev_checkpoint->settings_connection_shadowed) {
if (dev_checkpoint->settings_connection_is_shadowed_owned)
*persist_mode = NM_SETTINGS_CONNECTION_PERSIST_MODE_IN_MEMORY;
else
*persist_mode = NM_SETTINGS_CONNECTION_PERSIST_MODE_IN_MEMORY_DETACHED;
} else
*persist_mode = NM_SETTINGS_CONNECTION_PERSIST_MODE_IN_MEMORY_ONLY;
} else {
*persist_mode = NM_SETTINGS_CONNECTION_PERSIST_MODE_TO_DISK;
}
uuid = nm_connection_get_uuid(dev_checkpoint->settings_connection); uuid = nm_connection_get_uuid(dev_checkpoint->settings_connection);
sett_conn = nm_settings_get_connection_by_uuid(NM_SETTINGS_GET, uuid); sett_conn = nm_settings_get_connection_by_uuid(NM_SETTINGS_GET, uuid);
if (!sett_conn) /* Check if the connection changed */
return NULL; if (sett_conn
&& !nm_connection_compare(dev_checkpoint->settings_connection,
/* Now check if the connection changed, ... */ nm_settings_connection_get_connection(sett_conn),
if (!nm_connection_compare(dev_checkpoint->settings_connection, NM_SETTING_COMPARE_FLAG_IGNORE_TIMESTAMP)) {
nm_settings_connection_get_connection(sett_conn),
NM_SETTING_COMPARE_FLAG_EXACT)) {
_LOGT("rollback: settings connection %s changed", uuid); _LOGT("rollback: settings connection %s changed", uuid);
*need_update = TRUE; *need_update = TRUE;
*need_activation = TRUE; *need_activation = TRUE;
} }
/* ... is active, ... */ storage = sett_conn ? nm_settings_connection_get_storage(sett_conn) : NULL;
shadowed_file = storage ? nm_settings_storage_get_shadowed_storage(storage, NULL) : NULL;
shadowed_connection =
shadowed_file ? parse_connection_from_shadowed_file(shadowed_file, NULL) : NULL;
if (dev_checkpoint->settings_connection_shadowed) {
if (!shadowed_connection
|| !nm_connection_compare(dev_checkpoint->settings_connection_shadowed,
shadowed_connection,
NM_SETTING_COMPARE_FLAG_IGNORE_TIMESTAMP)) {
_LOGT("rollback: shadowed connection changed for %s", uuid);
*need_update_shadowed = TRUE;
*need_update = TRUE;
}
} else {
if (shadowed_connection) {
_LOGT("rollback: shadowed connection changed for %s", uuid);
*need_update = TRUE;
}
}
if (!sett_conn)
return NULL;
/* Check if the connection unsaved flag changed */
sett_conn_unsaved = NM_FLAGS_HAS(nm_settings_connection_get_flags(sett_conn),
NM_SETTINGS_CONNECTION_INT_FLAGS_UNSAVED);
if (sett_conn_unsaved != dev_checkpoint->settings_connection_is_unsaved) {
_LOGT("rollback: storage changed for settings connection %s: unsaved (%d -> %d)",
uuid,
dev_checkpoint->settings_connection_is_unsaved,
sett_conn_unsaved);
*need_update = TRUE;
}
/* Check if the active state changed */
nm_manager_for_each_active_connection (priv->manager, active, tmp_clist) { nm_manager_for_each_active_connection (priv->manager, active, tmp_clist) {
ac_uuid = ac_uuid =
nm_settings_connection_get_uuid(nm_active_connection_get_settings_connection(active)); nm_settings_connection_get_uuid(nm_active_connection_get_settings_connection(active));
@@ -196,7 +275,7 @@ find_settings_connection(NMCheckpoint *self,
return sett_conn; return sett_conn;
} }
/* ... or if the connection was reactivated/reapplied */ /* Check if the connection was reactivated/reapplied */
if (nm_active_connection_version_id_get(active) != dev_checkpoint->ac_version_id) { if (nm_active_connection_version_id_get(active) != dev_checkpoint->ac_version_id) {
_LOGT("rollback: active connection version id of %s changed", uuid); _LOGT("rollback: active connection version id of %s changed", uuid);
*need_activation = TRUE; *need_activation = TRUE;
@@ -212,12 +291,19 @@ restore_and_activate_connection(NMCheckpoint *self, DeviceCheckpoint *dev_checkp
NMSettingsConnection *connection; NMSettingsConnection *connection;
gs_unref_object NMAuthSubject *subject = NULL; gs_unref_object NMAuthSubject *subject = NULL;
GError *local_error = NULL; GError *local_error = NULL;
gboolean need_update, need_activation; gboolean need_update;
gboolean need_update_shadowed;
gboolean need_activation;
NMSettingsConnectionPersistMode persist_mode; NMSettingsConnectionPersistMode persist_mode;
NMSettingsConnectionIntFlags sett_flags; NMSettingsConnectionIntFlags sett_flags;
NMSettingsConnectionIntFlags sett_mask; NMSettingsConnectionIntFlags sett_mask;
connection = find_settings_connection(self, dev_checkpoint, &need_update, &need_activation); connection = find_settings_connection(self,
dev_checkpoint,
&need_update,
&need_update_shadowed,
&need_activation,
&persist_mode);
/* FIXME: we need to ensure to re-create/update the profile for the /* FIXME: we need to ensure to re-create/update the profile for the
* same settings plugin. E.g. if it was a keyfile in /run or /etc, * same settings plugin. E.g. if it was a keyfile in /run or /etc,
@@ -229,9 +315,26 @@ restore_and_activate_connection(NMCheckpoint *self, DeviceCheckpoint *dev_checkp
sett_mask = NM_SETTINGS_CONNECTION_INT_FLAGS_NONE; sett_mask = NM_SETTINGS_CONNECTION_INT_FLAGS_NONE;
if (connection) { if (connection) {
if (need_update_shadowed) {
_LOGD("rollback: updating shadowed file for connection %s",
nm_connection_get_uuid(dev_checkpoint->settings_connection));
nm_settings_connection_update(
connection,
NULL,
dev_checkpoint->settings_connection_shadowed,
NM_SETTINGS_CONNECTION_PERSIST_MODE_TO_DISK,
sett_flags,
sett_mask,
NM_SETTINGS_CONNECTION_UPDATE_REASON_RESET_SYSTEM_SECRETS
| NM_SETTINGS_CONNECTION_UPDATE_REASON_UPDATE_NON_SECRET,
"checkpoint-rollback",
NULL);
}
if (need_update) { if (need_update) {
_LOGD("rollback: updating connection %s", nm_settings_connection_get_uuid(connection)); _LOGD("rollback: updating connection %s with persist mode \"%s\"",
persist_mode = NM_SETTINGS_CONNECTION_PERSIST_MODE_KEEP; nm_connection_get_uuid(dev_checkpoint->settings_connection),
nm_settings_connection_persist_mode_to_string(persist_mode));
nm_settings_connection_update( nm_settings_connection_update(
connection, connection,
NULL, NULL,
@@ -246,21 +349,54 @@ restore_and_activate_connection(NMCheckpoint *self, DeviceCheckpoint *dev_checkp
} }
} else { } else {
/* The connection was deleted, recreate it */ /* The connection was deleted, recreate it */
_LOGD("rollback: adding connection %s again", if (need_update_shadowed) {
nm_connection_get_uuid(dev_checkpoint->settings_connection)); _LOGD("rollback: adding back shadowed file for connection %s",
nm_connection_get_uuid(dev_checkpoint->settings_connection));
persist_mode = NM_SETTINGS_CONNECTION_PERSIST_MODE_TO_DISK; if (!nm_settings_add_connection(NM_SETTINGS_GET,
if (!nm_settings_add_connection(NM_SETTINGS_GET, NULL,
NULL, dev_checkpoint->settings_connection_shadowed,
dev_checkpoint->settings_connection, NM_SETTINGS_CONNECTION_PERSIST_MODE_TO_DISK,
persist_mode, NM_SETTINGS_CONNECTION_ADD_REASON_NONE,
NM_SETTINGS_CONNECTION_ADD_REASON_NONE, sett_flags,
sett_flags, &connection,
&connection, &local_error)) {
&local_error)) { _LOGD("rollback: connection add failure: %s", local_error->message);
_LOGD("rollback: connection add failure: %s", local_error->message); g_clear_error(&local_error);
g_clear_error(&local_error); return FALSE;
return FALSE; }
_LOGD("rollback: updating connection %s with persist mode \"%s\"",
nm_connection_get_uuid(dev_checkpoint->settings_connection),
nm_settings_connection_persist_mode_to_string(persist_mode));
nm_settings_connection_update(
connection,
NULL,
dev_checkpoint->settings_connection,
persist_mode,
sett_flags,
sett_mask,
NM_SETTINGS_CONNECTION_UPDATE_REASON_RESET_SYSTEM_SECRETS
| NM_SETTINGS_CONNECTION_UPDATE_REASON_UPDATE_NON_SECRET,
"checkpoint-rollback",
NULL);
} else {
_LOGD("rollback: adding back connection %s with persist mode \"%s\"",
nm_connection_get_uuid(dev_checkpoint->settings_connection),
nm_settings_connection_persist_mode_to_string(persist_mode));
if (!nm_settings_add_connection(NM_SETTINGS_GET,
NULL,
dev_checkpoint->settings_connection,
persist_mode,
NM_SETTINGS_CONNECTION_ADD_REASON_NONE,
sett_flags,
&connection,
&local_error)) {
_LOGD("rollback: connection add failure: %s", local_error->message);
g_clear_error(&local_error);
return FALSE;
}
} }
need_activation = TRUE; need_activation = TRUE;
} }
@@ -362,11 +498,15 @@ nm_checkpoint_rollback(NMCheckpoint *self)
while (g_hash_table_iter_next(&iter, (gpointer *) &device, (gpointer *) &dev_checkpoint)) { while (g_hash_table_iter_next(&iter, (gpointer *) &device, (gpointer *) &dev_checkpoint)) {
guint32 result = NM_ROLLBACK_RESULT_OK; guint32 result = NM_ROLLBACK_RESULT_OK;
_LOGD("rollback: restoring device %s (state %d, realized %d, explicitly unmanaged %d)", _LOGD("rollback: restoring device %s (state %d, realized %d, explicitly unmanaged %d, "
"connection-unsaved %d, connection-shadowed %d, connection-shadowed-owned %d)",
dev_checkpoint->original_dev_name, dev_checkpoint->original_dev_name,
(int) dev_checkpoint->state, (int) dev_checkpoint->state,
dev_checkpoint->realized, dev_checkpoint->realized,
dev_checkpoint->unmanaged_explicit); dev_checkpoint->unmanaged_explicit,
dev_checkpoint->settings_connection_is_unsaved,
!!dev_checkpoint->settings_connection_shadowed,
dev_checkpoint->settings_connection_is_shadowed_owned);
if (nm_device_is_real(device)) { if (nm_device_is_real(device)) {
if (!dev_checkpoint->realized) { if (!dev_checkpoint->realized) {
@@ -518,6 +658,7 @@ device_checkpoint_destroy(gpointer data)
g_clear_object(&dev_checkpoint->applied_connection); g_clear_object(&dev_checkpoint->applied_connection);
g_clear_object(&dev_checkpoint->settings_connection); g_clear_object(&dev_checkpoint->settings_connection);
g_clear_object(&dev_checkpoint->device); g_clear_object(&dev_checkpoint->device);
g_clear_object(&dev_checkpoint->settings_connection_shadowed);
g_free(dev_checkpoint->original_dev_path); g_free(dev_checkpoint->original_dev_path);
g_free(dev_checkpoint->original_dev_name); g_free(dev_checkpoint->original_dev_name);
@@ -555,7 +696,7 @@ _dev_exported_changed(NMDBusObject *obj, NMCheckpoint *checkpoint)
} }
static DeviceCheckpoint * static DeviceCheckpoint *
device_checkpoint_create(NMCheckpoint *checkpoint, NMDevice *device) device_checkpoint_create(NMCheckpoint *self, NMDevice *device)
{ {
DeviceCheckpoint *dev_checkpoint; DeviceCheckpoint *dev_checkpoint;
NMConnection *applied_connection; NMConnection *applied_connection;
@@ -579,7 +720,7 @@ device_checkpoint_create(NMCheckpoint *checkpoint, NMDevice *device)
dev_checkpoint->dev_exported_change_id = g_signal_connect(device, dev_checkpoint->dev_exported_change_id = g_signal_connect(device,
NM_DBUS_OBJECT_EXPORTED_CHANGED, NM_DBUS_OBJECT_EXPORTED_CHANGED,
G_CALLBACK(_dev_exported_changed), G_CALLBACK(_dev_exported_changed),
checkpoint); self);
if (nm_device_get_unmanaged_mask(device, NM_UNMANAGED_USER_EXPLICIT)) { if (nm_device_get_unmanaged_mask(device, NM_UNMANAGED_USER_EXPLICIT)) {
dev_checkpoint->unmanaged_explicit = dev_checkpoint->unmanaged_explicit =
@@ -589,6 +730,11 @@ device_checkpoint_create(NMCheckpoint *checkpoint, NMDevice *device)
act_request = nm_device_get_act_request(device); act_request = nm_device_get_act_request(device);
if (act_request) { if (act_request) {
NMSettingsStorage *storage;
gboolean shadowed_owned = FALSE;
const char *shadowed_file;
gs_free_error GError *error = NULL;
settings_connection = nm_act_request_get_settings_connection(act_request); settings_connection = nm_act_request_get_settings_connection(act_request);
applied_connection = nm_act_request_get_applied_connection(act_request); applied_connection = nm_act_request_get_applied_connection(act_request);
@@ -602,6 +748,24 @@ device_checkpoint_create(NMCheckpoint *checkpoint, NMDevice *device)
dev_checkpoint->activation_lifetime_bound_to_profile_visibility = dev_checkpoint->activation_lifetime_bound_to_profile_visibility =
NM_FLAGS_HAS(nm_active_connection_get_state_flags(NM_ACTIVE_CONNECTION(act_request)), NM_FLAGS_HAS(nm_active_connection_get_state_flags(NM_ACTIVE_CONNECTION(act_request)),
NM_ACTIVATION_STATE_FLAG_LIFETIME_BOUND_TO_PROFILE_VISIBILITY); NM_ACTIVATION_STATE_FLAG_LIFETIME_BOUND_TO_PROFILE_VISIBILITY);
dev_checkpoint->settings_connection_is_unsaved =
NM_FLAGS_HAS(nm_settings_connection_get_flags(settings_connection),
NM_SETTINGS_CONNECTION_INT_FLAGS_UNSAVED);
storage = nm_settings_connection_get_storage(settings_connection);
shadowed_file =
storage ? nm_settings_storage_get_shadowed_storage(storage, &shadowed_owned) : NULL;
if (shadowed_file) {
dev_checkpoint->settings_connection_is_shadowed_owned = shadowed_owned;
dev_checkpoint->settings_connection_shadowed =
parse_connection_from_shadowed_file(shadowed_file, &error);
if (!dev_checkpoint->settings_connection_shadowed) {
_LOGW("error reading shadowed connection file for %s: %s",
nm_device_get_iface(device),
error->message);
}
}
} }
return dev_checkpoint; return dev_checkpoint;