settings: no longer honor read-only flag to prevent modifying connection profiles

Note that we now support keyfiles from read-only storage in /usr/lib.
Note also that we do support modifying and deleting these profiles.
That works by placing a shadowing profile to /etc or /run.

Surely this is questionable. It means that once the user uses D-Bus
to modify/delete a profile in /usr/lib, that profile becomes forever
shadowed by a different file, and there is no D-Bus API to return
to the original file. The user would have to drop the shadowing storages
from the file system. That is a problem.

But on the other hand, disallowing changes to such read-only profiles
is not very useful either. If you no longer can use D-Bus to modify such
profiles, what's the value here? How are applications supposed to handle
such profiles if there is no D-Bus API to do something sensible to them?

So, whatever problems read-only profiles and this shadowing causes, I don't
think that the solution is to entirely disallow changes via D-Bus.

At that point, we can just as well allow changes to profiles from
ifupdown. Note that you still cannot modify the profile directly (as the
ifupdown plugin does not support that). But you can delete the profile
(either temporarily via a tombstone in /run or permanently via a
tombstone in /etc). You also can make the profile in-memory, and take
it from there. Note that if you try to later store the in-memory profile
to disk again, then it depends on the order of settings plugins whether
that succeeds. If you have "plugins=keyfile,ifupdown", then the profile
will be stored as keyfile to /etc. If you have "plugins=ifupdown,keyfile",
then the modification will only be done in /run and the "to-disk" command
silently ignored (there really is no better solution).

2 files changed, 0 insertions, 42 deletions

diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c
index 2c1105258f..25a27e68cc 100644
--- a/src/settings/nm-settings-connection.c
+++ b/src/settings/nm-settings-connection.c
@@ -1324,37 +1324,6 @@ auth_start (NMSettingsConnection *self,
 
 /**** DBus method handlers ************************************/
 
-static gboolean
-check_writable (NMConnection *self, GError **error)
-{
-	NMSettingConnection *s_con;
-
-	g_return_val_if_fail (NM_IS_CONNECTION (self), FALSE);
-
-	s_con = nm_connection_get_setting_connection (self);
-	if (!s_con) {
-		g_set_error_literal (error,
-		                     NM_SETTINGS_ERROR,
-		                     NM_SETTINGS_ERROR_INVALID_CONNECTION,
-		                     "Connection did not have required 'connection' setting");
-		return FALSE;
-	}
-
-	/* If the connection is read-only, that has to be changed at the source of
-	 * the problem (ex a system settings plugin that can't write connections out)
-	 * instead of over D-Bus.
-	 */
-	if (nm_setting_connection_get_read_only (s_con)) {
-		g_set_error_literal (error,
-		                     NM_SETTINGS_ERROR,
-		                     NM_SETTINGS_ERROR_READ_ONLY_CONNECTION,
-		                     "Connection is read-only");
-		return FALSE;
-	}
-
-	return TRUE;
-}
-
 static void
 get_settings_auth_cb (NMSettingsConnection *self,
                       GDBusMethodInvocation *context,
@@ -1603,13 +1572,6 @@ settings_connection_update (NMSettingsConnection *self,
 	UpdateInfo *info;
 	const char *permission;
 
-	/* If the connection is read-only, that has to be changed at the source of
-	 * the problem (ex a system settings plugin that can't write connections out)
-	 * instead of over D-Bus.
-	 */
-	if (!check_writable (nm_settings_connection_get_connection (self), &error))
-		goto error;
-
 	/* Check if the settings are valid first */
 	if (new_settings) {
 		if (!g_variant_is_of_type (new_settings, NM_VARIANT_TYPE_CONNECTION)) {
@@ -1837,9 +1799,6 @@ impl_settings_connection_delete (NMDBusObject *obj,
 
 	nm_assert (nm_settings_connection_still_valid (self));
 
-	if (!check_writable (nm_settings_connection_get_connection (self), &error))
-		goto err;
-
 	subject = _new_auth_subject (invocation, &error);
 	if (!subject)
 		goto err;
diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-parser.c b/src/settings/plugins/ifupdown/nms-ifupdown-parser.c
index b65013a431..41b20850ad 100644
--- a/src/settings/plugins/ifupdown/nms-ifupdown-parser.c
+++ b/src/settings/plugins/ifupdown/nms-ifupdown-parser.c
@@ -676,7 +676,6 @@ ifupdown_new_connection_from_if_block (if_block *block,
 	              NM_SETTING_CONNECTION_INTERFACE_NAME, block->name,
 	              NM_SETTING_CONNECTION_ID, idstr,
 	              NM_SETTING_CONNECTION_UUID, uuid,
-	              NM_SETTING_CONNECTION_READ_ONLY, TRUE,
 	              NM_SETTING_CONNECTION_AUTOCONNECT, (gboolean) (!!autoconnect),
 	              NULL);