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

keyfile: let keyfile writer serialize setting with all default values

Browse Source 
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"
This commit is contained in:
Thomas Haller
2019-08-25 13:29:41 +02:00
parent e6eb01c18f
commit ddd148e02b
2 changed files with 35 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
libnm-core/nm-keyfile.c
View File

@@ -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);
}

32
libnm-core/tests/test-setting.c
View File

@@ -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);
}
/*****************************************************************************/