settings: add "version-id" argument to Update2() D-Bus call

We want to guard against concurrent modifications of profiles. We cannot
lock profiles, so what we instead do is expose (and bump) a version ID.
The user can check the version ID, plan ahead what to do, and tell
NetworkManager to only make the modification if no concurrent
modification was done. The conflict can be detected via the version ID.

The Update2() D-Bus call gets a parameter to only allow the request if
the version ID still matches.

nmcli should use this, but it is quite some effort to retry upon
concurrent modification. This is still to do.

Note that the user might make a decision that is based on multiple
profiles. As the new version-id is only per-profile, we cannot guard
against such inter-profile modifications. What would be needed, is a
UpdateMany() call, where we could modify multiple profiles at once, and
the action only takes effect if all version IDs show no concurrent
modification. That's not done yet, and maybe never will be.

3 files changed, 31 insertions, 1 deletions

diff --git a/introspection/org.freedesktop.NetworkManager.Settings.Connection.xml b/introspection/org.freedesktop.NetworkManager.Settings.Connection.xml
index a9240ec107..9b876e75d8 100644
--- a/introspection/org.freedesktop.NetworkManager.Settings.Connection.xml
+++ b/introspection/org.freedesktop.NetworkManager.Settings.Connection.xml
@@ -164,6 +164,10 @@
           <listitem><para>The settings plugin the connection will be migrated to
           such as "keyfile" or "ifcfg-rh".</para>
           <para role="since">Since 1.38</para></listitem>
+          <term><literal>version-id</literal>:</term>
+          <listitem><para>If specified, the update request is rejected if the
+          profile's version-id does not match. This can be used to catch concurrent
+          modifications. Zero means no version check.</para><para role="since">Since 1.44</para></listitem>
         </varlistentry>
         </variablelist>
 
diff --git a/src/core/settings/nm-settings-connection.c b/src/core/settings/nm-settings-connection.c
index d0725a7042..32911f16e4 100644
--- a/src/core/settings/nm-settings-connection.c
+++ b/src/core/settings/nm-settings-connection.c
@@ -1443,6 +1443,7 @@ typedef struct {
     NMSettingsUpdate2Flags flags;
     char                  *audit_args;
     char                  *plugin_name;
+    guint64                version_id;
     bool                   is_update2 : 1;
 } UpdateInfo;
 
@@ -1492,6 +1493,16 @@ update_auth_cb(NMSettingsConnection  *self,
 
     priv = NM_SETTINGS_CONNECTION_GET_PRIVATE(self);
 
+    if (info->version_id != 0 && info->version_id != priv->version_id) {
+        g_set_error_literal(&local,
+                            NM_SETTINGS_ERROR,
+                            NM_SETTINGS_ERROR_VERSION_ID_MISMATCH,
+                            "Update failed because profile changed in the meantime and the "
+                            "version-id mismatches");
+        error = local;
+        goto out;
+    }
+
     if (info->new_settings) {
         if (!_nm_connection_aggregate(info->new_settings,
                                       NM_CONNECTION_AGGREGATE_ANY_SECRETS,
@@ -1639,6 +1650,7 @@ settings_connection_update(NMSettingsConnection  *self,
                            GDBusMethodInvocation *context,
                            GVariant              *new_settings,
                            const char            *plugin_name,
+                           guint64                version_id,
                            NMSettingsUpdate2Flags flags)
 {
     NMSettingsConnectionPrivate *priv    = NM_SETTINGS_CONNECTION_GET_PRIVATE(self);
@@ -1695,6 +1707,7 @@ settings_connection_update(NMSettingsConnection  *self,
         .flags        = flags,
         .new_settings = tmp,
         .plugin_name  = g_strdup(plugin_name),
+        .version_id   = version_id,
     };
 
     permission = get_update_modify_permission(nm_settings_connection_get_connection(self),
@@ -1729,6 +1742,7 @@ impl_settings_connection_update(NMDBusObject                      *obj,
                                invocation,
                                settings,
                                NULL,
+                               0,
                                NM_SETTINGS_UPDATE2_FLAG_TO_DISK);
 }
 
@@ -1750,6 +1764,7 @@ impl_settings_connection_update_unsaved(NMDBusObject                      *obj,
                                invocation,
                                settings,
                                NULL,
+                               0,
                                NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY);
 }
 
@@ -1769,6 +1784,7 @@ impl_settings_connection_save(NMDBusObject                      *obj,
                                invocation,
                                NULL,
                                NULL,
+                               0,
                                NM_SETTINGS_UPDATE2_FLAG_TO_DISK);
 }
 
@@ -1785,6 +1801,7 @@ impl_settings_connection_update2(NMDBusObject                      *obj,
     gs_unref_variant GVariant *settings    = NULL;
     gs_unref_variant GVariant *args        = NULL;
     gs_free char              *plugin_name = NULL;
+    guint64                    version_id  = 0;
     guint32                    flags_u;
     GError                    *error = NULL;
     GVariantIter               iter;
@@ -1831,6 +1848,11 @@ impl_settings_connection_update2(NMDBusObject                      *obj,
             plugin_name = g_variant_dup_string(args_value, NULL);
             continue;
         }
+        if (nm_streq(args_name, "version-id")
+            && g_variant_is_of_type(args_value, G_VARIANT_TYPE_UINT64)) {
+            version_id = g_variant_get_uint64(args_value);
+            continue;
+        }
 
         error = g_error_new(NM_SETTINGS_ERROR,
                             NM_SETTINGS_ERROR_INVALID_ARGUMENTS,
@@ -1840,7 +1862,7 @@ impl_settings_connection_update2(NMDBusObject                      *obj,
         return;
     }
 
-    settings_connection_update(self, TRUE, invocation, settings, plugin_name, flags);
+    settings_connection_update(self, TRUE, invocation, settings, plugin_name, version_id, flags);
 }
 
 static void
diff --git a/src/libnm-core-public/nm-errors.h b/src/libnm-core-public/nm-errors.h
index 639c508e3e..a4d69c31ac 100644
--- a/src/libnm-core-public/nm-errors.h
+++ b/src/libnm-core-public/nm-errors.h
@@ -250,6 +250,9 @@ GQuark nm_secret_agent_error_quark(void);
  * @NM_SETTINGS_ERROR_UUID_EXISTS: a connection with that UUID already exists
  * @NM_SETTINGS_ERROR_INVALID_HOSTNAME: attempted to set an invalid hostname
  * @NM_SETTINGS_ERROR_INVALID_ARGUMENTS: invalid arguments
+ * @NM_SETTINGS_ERROR_VERSION_ID_MISMATCH: The profile's VersionId mismatched
+ *   and the update is rejected. See the "version-id" argument to Update2()
+ *   method. Since 1.44.
  *
  * Errors related to the settings/persistent configuration interface of
  * NetworkManager.
@@ -267,6 +270,7 @@ typedef enum {
     NM_SETTINGS_ERROR_UUID_EXISTS,          /*< nick=UuidExists >*/
     NM_SETTINGS_ERROR_INVALID_HOSTNAME,     /*< nick=InvalidHostname >*/
     NM_SETTINGS_ERROR_INVALID_ARGUMENTS,    /*< nick=InvalidArguments >*/
+    NM_SETTINGS_ERROR_VERSION_ID_MISMATCH,  /*< nick=VersionIdMismatch >*/
 } NMSettingsError;
 
 GQuark nm_settings_error_quark(void);