fdo-mirrors/NetworkManager
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/NetworkManager/NetworkManager.git synced 2026-01-17 14:00:22 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

keyfile: merge branch 'th/keyfile-fix-empty-settings'

Browse source 
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/248
This commit is contained in:
Thomas Haller 2019-08-27 11:47:22 +02:00
commit 01ef7c4079
4 changed files with 190 additions and 71 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

98
libnm-core/nm-keyfile.c
View file

@ -1369,6 +1369,9 @@ nm_keyfile_detect_unqualified_path_scheme (const char *base_dir,
*/
path = get_cert_path (base_dir, (const guint8 *) data, data_len);
/* FIXME(keyfile-parse-in-memory): it is wrong that keyfile reader makes decisions based on
* the file systems content. The serialization/parsing should be entirely in-memory. */
if ( !memchr (data, '/', data_len)
&& !has_cert_ext (path)) {
if (!consider_exists)
@ -1449,6 +1452,10 @@ cert_parser (KeyfileReaderInfo *info, NMSetting *setting, const char *key)
path2 = path2_free;
}
/* FIXME(keyfile-parse-in-memory): keyfile reader must not access the file system and
* (in a first step) only operate in memory-only. If the presence of files should be checked,
* then by invoking a callback (and possibly keyfile settings plugin would
* collect the file names to be checked and check them later). */
if (!g_file_test (path2, G_FILE_TEST_EXISTS)) {
handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_INFO_MISSING_FILE,
_("certificate or key file '%s' does not exist"),
@ -2906,41 +2913,48 @@ static const ParseInfoSetting *const parse_infos[_NM_META_SETTING_TYPE_NUM] = {
),
};
static const ParseInfoProperty *
static void
_parse_info_find (NMSetting *setting,
const char *property_name,
const NMMetaSettingInfo **out_setting_info)
const NMMetaSettingInfo **out_setting_info,
const ParseInfoSetting **out_parse_info_setting,
const ParseInfoProperty **out_parse_info_property)
{
const NMMetaSettingInfo *setting_info;
const ParseInfoSetting *pis;
gssize idx;
const ParseInfoProperty *pip;
#if NM_MORE_ASSERTS > 10
{
guint i, j;
static int asserted = FALSE;
for (i = 0; i < G_N_ELEMENTS (parse_infos); i++) {
pis = parse_infos[i];
if (!asserted) {
for (i = 0; i < G_N_ELEMENTS (parse_infos); i++) {
pis = parse_infos[i];
if (!pis)
continue;
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;
const ParseInfoProperty *pip = pis->properties[j];
g_assert (pis->properties[0]);
for (j = 0; pis->properties[j]; j++) {
const ParseInfoProperty *pip0;
const ParseInfoProperty *pipj = pis->properties[j];
g_assert (pip->property_name);
if ( j > 0
&& (pip0 = pis->properties[j - 1])
&& strcmp (pip0->property_name, pip->property_name) >= 0) {
g_error ("Wrong order at index #%d.%d: \"%s.%s\" before \"%s.%s\"",
i, j - 1,
nm_meta_setting_infos[i].setting_name, pip0->property_name,
nm_meta_setting_infos[i].setting_name, pip->property_name);
g_assert (pipj->property_name);
if ( j > 0
&& (pip0 = pis->properties[j - 1])
&& strcmp (pip0->property_name, pipj->property_name) >= 0) {
g_error ("Wrong order at index #%d.%d: \"%s.%s\" before \"%s.%s\"",
i, j - 1,
nm_meta_setting_infos[i].setting_name, pip0->property_name,
nm_meta_setting_infos[i].setting_name, pipj->property_name);
}
}
}
asserted = TRUE;
}
}
#endif
@ -2948,16 +2962,25 @@ _parse_info_find (NMSetting *setting,
if ( !NM_IS_SETTING (setting)
|| !(setting_info = NM_SETTING_GET_CLASS (setting)->setting_info)) {
/* handle invalid setting objects gracefully. */
*out_setting_info = NULL;
return NULL;
NM_SET_OUT (out_setting_info, NULL);
NM_SET_OUT (out_parse_info_setting, NULL);
NM_SET_OUT (out_parse_info_property, NULL);
return;
}
nm_assert (setting_info->setting_name);
nm_assert (_NM_INT_NOT_NEGATIVE (setting_info->meta_type));
nm_assert (setting_info->meta_type < G_N_ELEMENTS (parse_infos));
*out_setting_info = setting_info;
pis = parse_infos[setting_info->meta_type];
pip = NULL;
if ( pis
&& property_name) {
gssize idx;
if ((pis = parse_infos[setting_info->meta_type])) {
G_STATIC_ASSERT_EXPR (G_STRUCT_OFFSET (ParseInfoProperty, property_name) == 0);
idx = nm_utils_ptrarray_find_binary_search ((gconstpointer *) pis->properties,
NM_PTRARRAY_LEN (pis->properties),
&property_name,
@ -2966,10 +2989,12 @@ _parse_info_find (NMSetting *setting,
NULL,
NULL);
if (idx >= 0)
return pis->properties[idx];
pip = pis->properties[idx];
}
return NULL;
NM_SET_OUT (out_setting_info, setting_info);
NM_SET_OUT (out_parse_info_setting, pis);
NM_SET_OUT (out_parse_info_property, pip);
}
/*****************************************************************************/
@ -2995,7 +3020,7 @@ read_one_setting_value (KeyfileReaderInfo *info,
key = property_info->name;
pip = _parse_info_find (setting, key, &setting_info);
_parse_info_find (setting, key, &setting_info, NULL, &pip);
nm_assert (setting_info);
@ -3613,7 +3638,7 @@ write_setting_value (KeyfileWriterInfo *info,
key = property_info->name;
pip = _parse_info_find (setting, key, &setting_info);
_parse_info_find (setting, key, &setting_info, NULL, &pip);
if (!pip) {
if (!setting_info) {
@ -3850,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;
@ -3862,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++) {
@ -3902,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);
}

69
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,
@ -3281,6 +3285,51 @@ test_parse_tc_handle (void)
/*****************************************************************************/
static void
test_empty_setting (void)
{
gs_unref_object NMConnection *con = NULL;
gs_unref_object NMConnection *con2 = NULL;
NMSettingBluetooth *s_bt;
NMSettingGsm *s_gsm;
gs_unref_keyfile GKeyFile *kf = NULL;
gs_free_error GError *error = NULL;
con = nmtst_create_minimal_connection ("bt-empty-gsm", "dca3192a-f2dc-48eb-b806-d0ff788f122c", NM_SETTING_BLUETOOTH_SETTING_NAME, NULL);
s_bt = _nm_connection_get_setting (con, NM_TYPE_SETTING_BLUETOOTH);
g_object_set (s_bt,
NM_SETTING_BLUETOOTH_TYPE, "dun",
NM_SETTING_BLUETOOTH_BDADDR, "aa:bb:cc:dd:ee:ff",
NULL);
s_gsm = NM_SETTING_GSM (nm_setting_gsm_new ());
nm_connection_add_setting (con, NM_SETTING (s_gsm));
nmtst_connection_normalize (con);
nmtst_assert_connection_verifies_without_normalization (con);
kf = nm_keyfile_write (con, NULL, NULL, &error);
nmtst_assert_success (kf, error);
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",
NULL,
NULL,
&error);
nmtst_assert_success (con2, error);
g_assert (nm_connection_get_setting (con2, NM_TYPE_SETTING_GSM));
nmtst_assert_connection_verifies_without_normalization (con2);
}
/*****************************************************************************/
NMTST_DEFINE ();
int
@ -3368,5 +3417,7 @@ main (int argc, char **argv)
g_test_add_func ("/libnm/test_team_setting", test_team_setting);
g_test_add_func ("/libnm/test_empty_setting", test_empty_setting);
return g_test_run ();
}

23
shared/nm-utils/nm-test-utils.h
View file

@ -2272,6 +2272,29 @@ nmtst_keyfile_assert_data (GKeyFile *kf, const char *data, gssize data_len)
g_assert_cmpmem (d2, d2_len, d1, d1_len);
}
static inline gssize
nmtst_keyfile_get_num_keys (GKeyFile *keyfile,
const char *group_name)
{
gs_strfreev char **keys = NULL;
gs_free_error GError *error = NULL;
gsize l;
g_assert (keyfile);
g_assert (group_name);
if (!g_key_file_has_group (keyfile, group_name))
return -1;
keys = g_key_file_get_keys (keyfile, group_name, &l, &error);
nmtst_assert_success (keys, error);
g_assert_cmpint (NM_PTRARRAY_LEN (keys), ==, l);
return l;
}
/*****************************************************************************/
#endif /* __NM_TEST_UTILS_H__ */

71
src/settings/plugins/keyfile/nms-keyfile-writer.c
View file

@ -126,6 +126,10 @@ cert_writer (NMConnection *connection,
new_path = g_strdup_printf ("%s/%s-%s.%s", info->keyfile_dir, nm_connection_get_uuid (connection),
cert_data->vtable->file_suffix, ext);
/* FIXME(keyfile-parse-in-memory): writer must not access/write to the file system before
* being sure that the entire profile can be written and all circumstances are good to
* proceed. That means, while writing we must only collect the blogs in-memory, and write
* them all in the end together (or not at all). */
success = nm_utils_file_set_contents (new_path,
(const char *) blob_data,
blob_len,
@ -197,10 +201,12 @@ _internal_write_connection (NMConnection *connection,
gs_free char *path = NULL;
const char *id;
WriteInfo info = { 0 };
GError *local_err = NULL;
gs_free_error GError *local_err = NULL;
int errsv;
gboolean rename;
int i_path;
gs_unref_object NMConnection *reread = NULL;
gboolean reread_same = FALSE;
g_return_val_if_fail (!out_path || !*out_path, FALSE);
g_return_val_if_fail (keyfile_dir && keyfile_dir[0] == '/', FALSE);
@ -313,6 +319,35 @@ _internal_write_connection (NMConnection *connection,
return FALSE;
}
if ( out_reread
|| out_reread_same) {
gs_free_error GError *reread_error = NULL;
reread = nms_keyfile_reader_from_keyfile (kf_file, path, NULL, profile_dir, FALSE, &reread_error);
if ( !reread
|| !nm_connection_normalize (reread, NULL, NULL, &reread_error)) {
nm_log_err (LOGD_SETTINGS, "BUG: the profile cannot be stored in keyfile format without becoming unusable: %s", reread_error->message);
g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
"keyfile writer produces an invalid connection: %s",
reread_error->message);
nm_assert_not_reached ();
return FALSE;
}
if (out_reread_same) {
reread_same = !!nm_connection_compare (reread, connection, NM_SETTING_COMPARE_FLAG_EXACT);
nm_assert (reread_same == nm_connection_compare (connection, reread, NM_SETTING_COMPARE_FLAG_EXACT));
nm_assert (reread_same == ({
gs_unref_hashtable GHashTable *_settings = NULL;
( nm_connection_diff (reread, connection, NM_SETTING_COMPARE_FLAG_EXACT, &_settings)
&& !_settings);
}));
}
}
nm_utils_file_set_contents (path,
kf_content_buf,
kf_content_len,
@ -323,7 +358,6 @@ _internal_write_connection (NMConnection *connection,
g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_FAILED,
"error writing to file '%s': %s",
path, local_err->message);
g_error_free (local_err);
return FALSE;
}
@ -344,36 +378,8 @@ _internal_write_connection (NMConnection *connection,
&& !nm_streq (path, existing_path))
unlink (existing_path);
if (out_reread || out_reread_same) {
gs_unref_object NMConnection *reread = NULL;
gboolean reread_same = FALSE;
reread = nms_keyfile_reader_from_keyfile (kf_file, path, NULL, profile_dir, FALSE, NULL);
nm_assert (NM_IS_CONNECTION (reread));
if ( reread
&& !nm_connection_normalize (reread, NULL, NULL, NULL)) {
nm_assert_not_reached ();
g_clear_object (&reread);
}
if (reread && out_reread_same) {
reread_same = !!nm_connection_compare (reread, connection, NM_SETTING_COMPARE_FLAG_EXACT);
nm_assert (reread_same == nm_connection_compare (connection, reread, NM_SETTING_COMPARE_FLAG_EXACT));
nm_assert (reread_same == ({
gs_unref_hashtable GHashTable *_settings = NULL;
( nm_connection_diff (reread, connection, NM_SETTING_COMPARE_FLAG_EXACT, &_settings)
&& !_settings);
}));
}
NM_SET_OUT (out_reread, g_steal_pointer (&reread));
NM_SET_OUT (out_reread_same, reread_same);
}
NM_SET_OUT (out_reread, g_steal_pointer (&reread));
NM_SET_OUT (out_reread_same, reread_same);
NM_SET_OUT (out_path, g_steal_pointer (&path));
return TRUE;
@ -448,4 +454,3 @@ nms_keyfile_writer_test_connection (NMConnection *connection,
out_reread_same,
error);
}