settings,libnm: merge branch 'th/settings-version-id'

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1667

9 files changed, 176 insertions, 39 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/introspection/org.freedesktop.NetworkManager.Settings.xml b/introspection/org.freedesktop.NetworkManager.Settings.xml
index ea271dfaa1..2932734f87 100644
--- a/introspection/org.freedesktop.NetworkManager.Settings.xml
+++ b/introspection/org.freedesktop.NetworkManager.Settings.xml
@@ -189,6 +189,16 @@
     <property name="CanModify" type="b" access="read"/>
 
     <!--
+        VersionId:
+
+        The version of the settings. This is incremented whenever the profile
+        changes and can be used to detect concurrent modifications.
+
+        Since: 1.44
+    -->
+    <property name="VersionId" type="t" access="read"/>
+
+    <!--
         NewConnection:
         @connection: Object path of the new connection.
 
diff --git a/src/core/settings/nm-settings-connection.c b/src/core/settings/nm-settings-connection.c
index 8ee0a29a3f..32911f16e4 100644
--- a/src/core/settings/nm-settings-connection.c
+++ b/src/core/settings/nm-settings-connection.c
@@ -109,7 +109,11 @@ _seen_bssids_hash_new(void)
 
 /*****************************************************************************/
 
-NM_GOBJECT_PROPERTIES_DEFINE(NMSettingsConnection, PROP_UNSAVED, PROP_FLAGS, PROP_FILENAME, );
+NM_GOBJECT_PROPERTIES_DEFINE(NMSettingsConnection,
+                             PROP_VERSION_ID,
+                             PROP_UNSAVED,
+                             PROP_FLAGS,
+                             PROP_FILENAME, );
 
 enum { UPDATED_INTERNAL, FLAGS_CHANGED, LAST_SIGNAL };
 
@@ -156,6 +160,8 @@ typedef struct _NMSettingsConnectionPrivate {
 
     guint64 last_secret_agent_version_id;
 
+    guint64 version_id;
+
     bool timestamp_set : 1;
 
     NMSettingsAutoconnectBlockedReason autoconnect_blocked_reason : 4;
@@ -1437,6 +1443,7 @@ typedef struct {
     NMSettingsUpdate2Flags flags;
     char                  *audit_args;
     char                  *plugin_name;
+    guint64                version_id;
     bool                   is_update2 : 1;
 } UpdateInfo;
 
@@ -1465,7 +1472,7 @@ update_complete(NMSettingsConnection *self, UpdateInfo *info, GError *error)
     g_clear_object(&info->new_settings);
     g_free(info->audit_args);
     g_free(info->plugin_name);
-    g_slice_free(UpdateInfo, info);
+    nm_g_slice_free(info);
 }
 
 static void
@@ -1479,14 +1486,23 @@ update_auth_cb(NMSettingsConnection  *self,
     UpdateInfo                     *info  = data;
     gs_free_error GError           *local = NULL;
     NMSettingsConnectionPersistMode persist_mode;
+    gs_unref_object NMConnection   *for_agent = NULL;
 
-    if (error) {
-        update_complete(self, info, error);
-        return;
-    }
+    if (error)
+        goto out;
 
     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,
@@ -1579,27 +1595,29 @@ update_auth_cb(NMSettingsConnection  *self,
         "update-from-dbus",
         &local);
 
-    if (!local) {
-        gs_unref_object NMConnection *for_agent = NULL;
-
-        /* Dupe the connection so we can clear out non-agent-owned secrets,
-         * as agent-owned secrets are the only ones we send back to be saved.
-         * Only send secrets to agents of the same UID that called update too.
-         */
-        for_agent = nm_simple_connection_new_clone(nm_settings_connection_get_connection(self));
-        _nm_connection_clear_secrets_by_secret_flags(for_agent, NM_SETTING_SECRET_FLAG_AGENT_OWNED);
-        nm_agent_manager_save_secrets(info->agent_mgr,
-                                      nm_dbus_object_get_path(NM_DBUS_OBJECT(self)),
-                                      for_agent,
-                                      info->subject);
+    if (local) {
+        error = local;
+        goto out;
     }
 
+    /* Dupe the connection so we can clear out non-agent-owned secrets,
+     * as agent-owned secrets are the only ones we send back to be saved.
+     * Only send secrets to agents of the same UID that called update too.
+     */
+    for_agent = nm_simple_connection_new_clone(nm_settings_connection_get_connection(self));
+    _nm_connection_clear_secrets_by_secret_flags(for_agent, NM_SETTING_SECRET_FLAG_AGENT_OWNED);
+    nm_agent_manager_save_secrets(info->agent_mgr,
+                                  nm_dbus_object_get_path(NM_DBUS_OBJECT(self)),
+                                  for_agent,
+                                  info->subject);
+
     /* Reset auto retries back to default since connection was updated */
     nm_manager_devcon_autoconnect_retries_reset(nm_settings_connection_get_manager(self),
                                                 NULL,
                                                 self);
 
-    update_complete(self, info, local);
+out:
+    update_complete(self, info, error);
 }
 
 static const char *
@@ -1632,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);
@@ -1679,14 +1698,17 @@ settings_connection_update(NMSettingsConnection  *self,
                                              &error))
         goto error;
 
-    info               = g_slice_new0(UpdateInfo);
-    info->is_update2   = is_update2;
-    info->context      = context;
-    info->agent_mgr    = g_object_ref(priv->agent_mgr);
-    info->subject      = subject;
-    info->flags        = flags;
-    info->new_settings = tmp;
-    info->plugin_name  = g_strdup(plugin_name);
+    info  = g_slice_new(UpdateInfo);
+    *info = (UpdateInfo){
+        .is_update2   = is_update2,
+        .context      = context,
+        .agent_mgr    = g_object_ref(priv->agent_mgr),
+        .subject      = subject,
+        .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),
                                               tmp ?: nm_settings_connection_get_connection(self));
@@ -1720,6 +1742,7 @@ impl_settings_connection_update(NMDBusObject                      *obj,
                                invocation,
                                settings,
                                NULL,
+                               0,
                                NM_SETTINGS_UPDATE2_FLAG_TO_DISK);
 }
 
@@ -1741,6 +1764,7 @@ impl_settings_connection_update_unsaved(NMDBusObject                      *obj,
                                invocation,
                                settings,
                                NULL,
+                               0,
                                NM_SETTINGS_UPDATE2_FLAG_IN_MEMORY);
 }
 
@@ -1760,6 +1784,7 @@ impl_settings_connection_save(NMDBusObject                      *obj,
                                invocation,
                                NULL,
                                NULL,
+                               0,
                                NM_SETTINGS_UPDATE2_FLAG_TO_DISK);
 }
 
@@ -1776,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;
@@ -1822,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,
@@ -1831,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
@@ -2639,6 +2670,23 @@ nm_settings_connection_get_uuid(NMSettingsConnection *self)
     return uuid;
 }
 
+guint64
+nm_settings_connection_get_version_id(NMSettingsConnection *self)
+{
+    g_return_val_if_fail(NM_IS_SETTINGS_CONNECTION(self), 0);
+
+    return NM_SETTINGS_CONNECTION_GET_PRIVATE(self)->version_id;
+}
+
+void
+nm_settings_connection_bump_version_id(NMSettingsConnection *self)
+{
+    g_return_if_fail(NM_IS_SETTINGS_CONNECTION(self));
+
+    NM_SETTINGS_CONNECTION_GET_PRIVATE(self)->version_id++;
+    _notify(self, PROP_VERSION_ID);
+}
+
 const char *
 nm_settings_connection_get_connection_type(NMSettingsConnection *self)
 {
@@ -2662,9 +2710,13 @@ _nm_settings_connection_cleanup_after_remove(NMSettingsConnection *self)
 static void
 get_property(GObject *object, guint prop_id, GValue *value, GParamSpec *pspec)
 {
-    NMSettingsConnection *self = NM_SETTINGS_CONNECTION(object);
+    NMSettingsConnection        *self = NM_SETTINGS_CONNECTION(object);
+    NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE(self);
 
     switch (prop_id) {
+    case PROP_VERSION_ID:
+        g_value_set_uint64(value, priv->version_id);
+        break;
     case PROP_UNSAVED:
         g_value_set_boolean(value, nm_settings_connection_get_unsaved(self));
         break;
@@ -2701,6 +2753,8 @@ nm_settings_connection_init(NMSettingsConnection *self)
 
     priv->agent_mgr = g_object_ref(nm_agent_manager_get());
     priv->settings  = g_object_ref(nm_settings_get());
+
+    priv->version_id = 1;
 }
 
 NMSettingsConnection *
@@ -2814,7 +2868,10 @@ static const NMDBusInterfaceInfoExtended interface_info_settings_connection = {
                                                            NM_SETTINGS_CONNECTION_FLAGS),
             NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE("Filename",
                                                            "s",
-                                                           NM_SETTINGS_CONNECTION_FILENAME), ), ),
+                                                           NM_SETTINGS_CONNECTION_FILENAME),
+            NM_DEFINE_DBUS_PROPERTY_INFO_EXTENDED_READABLE("VersionId",
+                                                           "t",
+                                                           NM_SETTINGS_CONNECTION_VERSION_ID), ), ),
 };
 
 static void
@@ -2832,6 +2889,15 @@ nm_settings_connection_class_init(NMSettingsConnectionClass *klass)
     object_class->dispose      = dispose;
     object_class->get_property = get_property;
 
+    obj_properties[PROP_VERSION_ID] =
+        g_param_spec_uint64(NM_SETTINGS_CONNECTION_VERSION_ID,
+                            "",
+                            "",
+                            0,
+                            G_MAXUINT64,
+                            0,
+                            G_PARAM_READABLE | G_PARAM_STATIC_STRINGS);
+
     obj_properties[PROP_UNSAVED] = g_param_spec_boolean(NM_SETTINGS_CONNECTION_UNSAVED,
                                                         "",
                                                         "",
diff --git a/src/core/settings/nm-settings-connection.h b/src/core/settings/nm-settings-connection.h
index eec4ae1856..835a978e40 100644
--- a/src/core/settings/nm-settings-connection.h
+++ b/src/core/settings/nm-settings-connection.h
@@ -141,9 +141,10 @@ typedef enum {
 #define NM_SETTINGS_CONNECTION_FLAGS_CHANGED    "flags-changed"
 
 /* Properties */
-#define NM_SETTINGS_CONNECTION_UNSAVED  "unsaved"
-#define NM_SETTINGS_CONNECTION_FLAGS    "flags"
-#define NM_SETTINGS_CONNECTION_FILENAME "filename"
+#define NM_SETTINGS_CONNECTION_UNSAVED    "unsaved"
+#define NM_SETTINGS_CONNECTION_VERSION_ID "version-id"
+#define NM_SETTINGS_CONNECTION_FLAGS      "flags"
+#define NM_SETTINGS_CONNECTION_FILENAME   "filename"
 
 /**
  * NMSettingsConnectionIntFlags:
@@ -231,6 +232,9 @@ const char *nm_settings_connection_get_filename(NMSettingsConnection *self);
 
 guint64 nm_settings_connection_get_last_secret_agent_version_id(NMSettingsConnection *self);
 
+guint64 nm_settings_connection_get_version_id(NMSettingsConnection *self);
+void    nm_settings_connection_bump_version_id(NMSettingsConnection *self);
+
 gboolean
 nm_settings_connection_has_unmodified_applied_connection(NMSettingsConnection *self,
                                                          NMConnection         *applied_connection,
diff --git a/src/core/settings/nm-settings.c b/src/core/settings/nm-settings.c
index c3d99cca3d..49a7c68753 100644
--- a/src/core/settings/nm-settings.c
+++ b/src/core/settings/nm-settings.c
@@ -1090,11 +1090,13 @@ _connection_changed_update(NMSettings                      *self,
 
     is_new = c_list_is_empty(&sett_conn->_connections_lst);
 
-    _LOGT("update[%s]: %s connection \"%s\" (" NM_SETTINGS_STORAGE_PRINT_FMT ")",
+    _LOGT("update[%s]: %s connection \"%s\" (" NM_SETTINGS_STORAGE_PRINT_FMT "), "
+          "new version-id %" G_GUINT64_FORMAT,
           nm_settings_storage_get_uuid(storage),
           is_new ? "adding" : "updating",
           nm_connection_get_id(connection),
-          NM_SETTINGS_STORAGE_PRINT_ARG(storage));
+          NM_SETTINGS_STORAGE_PRINT_ARG(storage),
+          (nm_settings_connection_get_version_id(sett_conn) + 1u));
 
     _nm_settings_connection_set_storage(sett_conn, storage);
 
@@ -1166,6 +1168,8 @@ _connection_changed_update(NMSettings                      *self,
                                      path);
     }
 
+    nm_settings_connection_bump_version_id(sett_conn);
+
     if (is_new) {
         nm_dbus_object_emit_signal(NM_DBUS_OBJECT(self),
                                    &interface_info_settings,
diff --git a/src/libnm-client-impl/libnm.ver b/src/libnm-client-impl/libnm.ver
index 7fd09072a2..919ffc74b8 100644
--- a/src/libnm-client-impl/libnm.ver
+++ b/src/libnm-client-impl/libnm.ver
@@ -1932,6 +1932,7 @@ global:
 
 libnm_1_44_0 {
 global:
+	nm_remote_connection_get_version_id;
 	nm_setting_gsm_get_initial_eps_apn;
 	nm_setting_gsm_get_initial_eps_config;
 	nm_setting_ip6_config_get_dhcp_pd_hint;
diff --git a/src/libnm-client-impl/nm-remote-connection.c b/src/libnm-client-impl/nm-remote-connection.c
index 38ea114b09..607c21a151 100644
--- a/src/libnm-client-impl/nm-remote-connection.c
+++ b/src/libnm-client-impl/nm-remote-connection.c
@@ -31,12 +31,14 @@ NM_GOBJECT_PROPERTIES_DEFINE(NMRemoteConnection,
                              PROP_UNSAVED,
                              PROP_FLAGS,
                              PROP_FILENAME,
+                             PROP_VERSION_ID,
                              PROP_VISIBLE, );
 
 typedef struct {
     GCancellable *get_settings_cancellable;
 
     char   *filename;
+    guint64 version_id;
     guint32 flags;
     bool    unsaved;
 
@@ -602,6 +604,23 @@ nm_remote_connection_get_filename(NMRemoteConnection *connection)
 }
 
 /**
+ * nm_remote_connection_get_version_id:
+ * @connection: the #NMRemoteConnection
+ *
+ * Returns: the version-id of the profile. This ID is incremented
+ *   whenever the profile is modified.
+ *
+ * Since: 1.44
+ */
+guint64
+nm_remote_connection_get_version_id(NMRemoteConnection *connection)
+{
+    g_return_val_if_fail(NM_IS_REMOTE_CONNECTION(connection), 0);
+
+    return NM_REMOTE_CONNECTION_GET_PRIVATE(connection)->version_id;
+}
+
+/**
  * nm_remote_connection_get_visible:
  * @connection: the #NMRemoteConnection
  *
@@ -724,6 +743,9 @@ get_property(GObject *object, guint prop_id, GValue *value, GParamSpec *pspec)
     case PROP_FILENAME:
         g_value_set_string(value, NM_REMOTE_CONNECTION_GET_PRIVATE(object)->filename);
         break;
+    case PROP_VERSION_ID:
+        g_value_set_uint64(value, NM_REMOTE_CONNECTION_GET_PRIVATE(object)->version_id);
+        break;
     case PROP_VISIBLE:
         g_value_set_boolean(value, NM_REMOTE_CONNECTION_GET_PRIVATE(object)->visible);
         break;
@@ -759,10 +781,11 @@ const NMLDBusMetaIface _nml_dbus_meta_iface_nm_settings_connection = NML_DBUS_ME
                                       NMRemoteConnection,
                                       _priv.filename),
         NML_DBUS_META_PROPERTY_INIT_U("Flags", PROP_FLAGS, NMRemoteConnection, _priv.flags),
-        NML_DBUS_META_PROPERTY_INIT_B("Unsaved",
-                                      PROP_UNSAVED,
+        NML_DBUS_META_PROPERTY_INIT_B("Unsaved", PROP_UNSAVED, NMRemoteConnection, _priv.unsaved),
+        NML_DBUS_META_PROPERTY_INIT_T("VersionId",
+                                      PROP_VERSION_ID,
                                       NMRemoteConnection,
-                                      _priv.unsaved), ), );
+                                      _priv.version_id), ), );
 
 static void
 nm_remote_connection_class_init(NMRemoteConnectionClass *klass)
@@ -820,6 +843,23 @@ nm_remote_connection_class_init(NMRemoteConnectionClass *klass)
                                                         G_PARAM_READABLE | G_PARAM_STATIC_STRINGS);
 
     /**
+     * NMRemoteConnection:version-id:
+     *
+     * The version ID of the profile that is incremented when the profile gets modified.
+     * This can be used to track concurrent modifications of the profile.
+     *
+     * Since: 1.44
+     **/
+    obj_properties[PROP_VERSION_ID] =
+        g_param_spec_uint64(NM_REMOTE_CONNECTION_VERSION_ID,
+                            "",
+                            "",
+                            0,
+                            G_MAXUINT64,
+                            0,
+                            G_PARAM_READABLE | G_PARAM_STATIC_STRINGS);
+
+    /**
      * NMRemoteConnection:visible:
      *
      * %TRUE if the remote connection is visible to the current user, %FALSE if
diff --git a/src/libnm-client-public/nm-remote-connection.h b/src/libnm-client-public/nm-remote-connection.h
index abfeecb367..1b4ea22cf1 100644
--- a/src/libnm-client-public/nm-remote-connection.h
+++ b/src/libnm-client-public/nm-remote-connection.h
@@ -32,6 +32,7 @@ G_BEGIN_DECLS
 #define NM_REMOTE_CONNECTION_UNSAVED         "unsaved"
 #define NM_REMOTE_CONNECTION_FLAGS           "flags"
 #define NM_REMOTE_CONNECTION_FILENAME        "filename"
+#define NM_REMOTE_CONNECTION_VERSION_ID      "version-id"
 #define NM_REMOTE_CONNECTION_VISIBLE         "visible"
 
 /**
@@ -120,6 +121,9 @@ const char *nm_remote_connection_get_filename(NMRemoteConnection *connection);
 
 gboolean nm_remote_connection_get_visible(NMRemoteConnection *connection);
 
+NM_AVAILABLE_IN_1_44
+guint64 nm_remote_connection_get_version_id(NMRemoteConnection *connection);
+
 G_END_DECLS
 
 #endif /* __NM_REMOTE_CONNECTION__ */
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);