keyfile: let keyfile writer serialize setting with all default values

It's important whether a setting is present or not. Keyfile writer
omits properties that have a default value, that means, if the setting
has all-default values, it would be dropped. For [proxy] that doesn't
really matter, because we tend to normalize it back. For some settings
it matters:

  $ nmcli connection add type bluetooth con-name bt autoconnect no bluetooth.type dun bluetooth.bdaddr aa:bb:cc:dd:ee:ff gsm.apn a
  Connection 'bt' (652cabd8-d350-4246-a6f3-3dc17eeb028f) successfully added.

  $ nmcli connection modify bt gsm.apn ''

When storing this to keyfile, the [gsm] section was dropped
(server-side) and we fail an nm_assert() (omitted from the example
output below).

  <error> [1566732645.9845] BUG: failure to normalized profile that we just wrote to disk: bluetooth: 'dun' connection requires 'gsm' or 'cdma' setting
  <trace> [1566732645.9846] keyfile: commit: "/etc/NetworkManager/system-connections/bt.nmconnection": profile 652cabd8-d350-4246-a6f3-3dc17eeb028f (bt) written
  <trace> [1566732645.9846] settings: update[652cabd8-d350-4246-a6f3-3dc17eeb028f]: update-from-dbus: update profile "bt"
  <trace> [1566732645.9849] settings: storage[652cabd8-d350-4246-a6f3-3dc17eeb028f,3e504752a4a78fb3/keyfile]: change event with connection "bt" (file "/etc/NetworkManager/system-connections/>
  <trace> [1566732645.9849] settings: update[652cabd8-d350-4246-a6f3-3dc17eeb028f]: updating connection "bt" (3e504752a4a78fb3/keyfile)
  <debug> [1566732645.9857] ++ connection 'update connection' (0x7f7918003340/NMSimpleConnection/"bluetooth" < 0x55e1c52480e0/NMSimpleConnection/"bluetooth") [/org/freedesktop/NetworkManager>
  <debug> [1566732645.9857] ++ gsm                       [ 0x55e1c5276f80 < 0x55e1c53205f0 ]
  <debug> [1566732645.9858] ++ gsm.apn                   < 'a'

Of course, after reload the connection on disk is no loner valid.
Keyfile writer wrote an invalid setting.

  # nmcli connection reload

Logfile:

  <warn>  [1566732775.4920] keyfile: load: "/etc/NetworkManager/system-connections/bt.nmconnection": failed to load connection: invalid connection: bluetooth: 'dun' connection requires 'gsm' or 'cdma' setting
  ...
  <trace> [1566732775.5432] settings: update[652cabd8-d350-4246-a6f3-3dc17eeb028f]: delete connection "bt" (3e504752a4a78fb3/keyfile)
  <debug> [1566732775.5434] Deleting secrets for connection /org/freedesktop/NetworkManager/Settings (bt)
  <trace> [1566732775.5436] dbus-object[9a402fbe14c8d975]: unexport: "/org/freedesktop/NetworkManager/Settings/55"

2 files changed, 35 insertions, 17 deletions

diff --git a/libnm-core/nm-keyfile.c b/libnm-core/nm-keyfile.c
index a6499d70f1..d2360f8c2a 100644
--- a/libnm-core/nm-keyfile.c
+++ b/libnm-core/nm-keyfile.c
@@ -2935,8 +2935,9 @@ _parse_info_find (NMSetting *setting,
 
 				if (!pis)
 					continue;
+				if (!pis->properties)
+					continue;
 
-				g_assert (pis->properties);
 				g_assert (pis->properties[0]);
 				for (j = 0; pis->properties[j]; j++) {
 					const ParseInfoProperty *pip0;
@@ -3874,9 +3875,13 @@ nm_keyfile_write (NMConnection *connection,
 	for (i = 0; i < n_settings; i++) {
 		const NMSettInfoSetting *sett_info;
 		NMSetting *setting = settings[i];
+		const char *setting_name;
+		const char *setting_alias;
 
 		sett_info = _nm_setting_class_get_sett_info (NM_SETTING_GET_CLASS (setting));
 
+		setting_name = sett_info->setting_class->setting_info->setting_name;
+
 		if (sett_info->detail.gendata_info) {
 			guint k, n_keys;
 			const char *const*keys;
@@ -3886,7 +3891,6 @@ nm_keyfile_write (NMConnection *connection,
 			n_keys = _nm_setting_gendata_get_all (setting, &keys, NULL);
 
 			if (n_keys > 0) {
-				const char *setting_name = sett_info->setting_class->setting_info->setting_name;
 				GHashTable *h = _nm_setting_gendata_hash (setting, FALSE);
 
 				for (k = 0; k < n_keys; k++) {
@@ -3926,6 +3930,18 @@ nm_keyfile_write (NMConnection *connection,
 				goto out_with_info_error;
 		}
 
+		setting_alias = nm_keyfile_plugin_get_alias_for_setting_name (setting_name);
+		if (   (   setting_alias
+		        && g_key_file_has_group (info.keyfile, setting_alias))
+		    || g_key_file_has_group (info.keyfile, setting_name)) {
+			/* we have a section for the setting. Nothing to do. */
+		} else {
+			/* ensure the group is present. There is no API for that, so add and remove
+			 * a dummy key. */
+			g_key_file_set_value  (info.keyfile, setting_alias ?: setting_name, ".X", "1");
+			g_key_file_remove_key (info.keyfile, setting_alias ?: setting_name, ".X", NULL);
+		}
+
 		nm_assert (!info.error);
 	}
 
diff --git a/libnm-core/tests/test-setting.c b/libnm-core/tests/test-setting.c
index 0a314562c3..b24bb1e37f 100644
--- a/libnm-core/tests/test-setting.c
+++ b/libnm-core/tests/test-setting.c
@@ -2581,7 +2581,7 @@ static void
 test_roundtrip_conversion (gconstpointer test_data)
 {
 	const int MODE = GPOINTER_TO_INT (test_data);
-	const char *ID= nm_sprintf_bufa (100, "roundtip-conversion-%d", MODE);
+	const char *ID= nm_sprintf_bufa (100, "roundtrip-conversion-%d", MODE);
 	const char *UUID= "63376701-b61e-4318-bf7e-664a1c1eeaab";
 	const char *INTERFACE_NAME = nm_sprintf_bufa (100, "ifname%d", MODE);
 	guint32 ETH_MTU = nmtst_rand_select ((guint32) 0u,
@@ -2663,6 +2663,8 @@ test_roundtrip_conversion (gconstpointer test_data)
 		                     "addr-gen-mode=stable-privacy\n"
 		                     "dns-search=\n"
 		                     "method=auto\n"
+		                     "\n"
+		                     "[proxy]\n"
 		                     "",
 		                     ID,
 		                     UUID,
@@ -2722,6 +2724,8 @@ test_roundtrip_conversion (gconstpointer test_data)
 		                     "interface-name=%s\n"
 		                     "permissions=\n"
 		                     "\n"
+		                     "[wireguard]\n"
+		                     "\n"
 		                     "[ipv4]\n"
 		                     "dns-search=\n"
 		                     "method=disabled\n"
@@ -2730,6 +2734,8 @@ test_roundtrip_conversion (gconstpointer test_data)
 		                     "addr-gen-mode=stable-privacy\n"
 		                     "dns-search=\n"
 		                     "method=ignore\n"
+		                     "\n"
+		                     "[proxy]\n"
 		                     "",
 		                     ID,
 		                     UUID,
@@ -2770,7 +2776,8 @@ test_roundtrip_conversion (gconstpointer test_data)
 		                     "type=wireguard\n"
 		                     "interface-name=%s\n"
 		                     "permissions=\n"
-		                     "%s" /* [wireguard] */
+		                     "\n"
+		                     "[wireguard]\n"
 		                     "%s" /* fwmark */
 		                     "%s" /* listen-port */
 		                     "%s" /* private-key-flags */
@@ -2785,17 +2792,12 @@ test_roundtrip_conversion (gconstpointer test_data)
 		                     "addr-gen-mode=stable-privacy\n"
 		                     "dns-search=\n"
 		                     "method=ignore\n"
+		                     "\n"
+		                     "[proxy]\n"
 		                     "",
 		                     ID,
 		                     UUID,
 		                     INTERFACE_NAME,
-		                     (  (   (WG_FWMARK != 0)
-		                         || (WG_LISTEN_PORT != 0)
-		                         || (WG_PRIVATE_KEY_FLAGS != NM_SETTING_SECRET_FLAG_NONE)
-		                         || (   WG_PRIVATE_KEY
-		                             && WG_PRIVATE_KEY_FLAGS == NM_SETTING_SECRET_FLAG_NONE))
-		                      ? "\n[wireguard]\n"
-		                      : ""),
 		                     (  (WG_FWMARK != 0)
 		                      ? nm_sprintf_bufa (100, "fwmark=%u\n", WG_FWMARK)
 		                      : ""),
@@ -2887,6 +2889,8 @@ test_roundtrip_conversion (gconstpointer test_data)
 		                     "routing-rule1=priority 1 from ::/0 table 1000\n"
 		                     "routing-rule2=priority 2 from 1:2:3:b::/65 table 1001\n"
 		                     "routing-rule3=priority 3 from 1:2:3:c::/66 table 1002\n"
+		                     "\n"
+		                     "[proxy]\n"
 		                     "",
 		                     ID,
 		                     UUID,
@@ -3309,10 +3313,8 @@ test_empty_setting (void)
 	kf = nm_keyfile_write (con, NULL, NULL, &error);
 	nmtst_assert_success (kf, error);
 
-	/* the [gsm] setting was lost because it only has default values.
-	 * As a result, the connection also became invalid. */
-
-	g_assert (!g_key_file_has_group (kf, "gsm"));
+	g_assert (g_key_file_has_group (kf, "gsm"));
+	g_assert_cmpint (nmtst_keyfile_get_num_keys (kf, "gsm"), ==, 0);
 
 	con2 = nm_keyfile_read (kf,
 	                        "/ignored/current/working/directory/for/loading/relative/paths",
@@ -3321,9 +3323,9 @@ test_empty_setting (void)
 	                        &error);
 	nmtst_assert_success (con2, error);
 
-	g_assert (!nm_connection_get_setting (con2, NM_TYPE_SETTING_GSM));
+	g_assert (nm_connection_get_setting (con2, NM_TYPE_SETTING_GSM));
 
-	nmtst_assert_connection_unnormalizable (con2, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_INVALID_SETTING);
+	nmtst_assert_connection_verifies_without_normalization (con2);
 }
 
 /*****************************************************************************/