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

keyfile: split automatically setting ID/UUID for keyfile

Browse source 
keyfile already supports omitting the "connection.id" and
"connection.uuid". In that case, the ID would be taken from the
keyfile's name, and the UUID was generated by md5 hashing the
full filename.

No longer do this during nm_keyfile_read(), instead let all
callers call nm_keyfile_read_ensure_*() to their liking. This is done
for two reasons:

 - a minor reason is, that one day we want to expose keyfile API
   as public API. That means, we also want to read keyfiles from
   stdin, where there is no filename available. The implementation
   which parses stdio needs to define their own way of auto-generating
   ID and UUID. Note how nm_keyfile_read()'s API no longer takes a
   filename as argument, which would be awkward for the stdin case.

 - Currently, we only support one keyfile directory, which (configurably)
   is "/etc/NetworkManager/system-connections".
   In the future, we want to support multiple keyfile dirctories, like
   "/var/run/NetworkManager/profiles" or "/usr/lib/NetworkManager/profiles".
   Here we want that a file "foo" (which does not specify a UUID) gets the
   same UUID regardless of the directory it is in. That seems better, because
   then the UUID won't change as you move the file between directories.
   Yes, that means, that the same UUID will be provided by multiple
   files, but NetworkManager must already cope with that situation anyway.
   Unfortunately, the UUID generation scheme hashes the full path. That
   means, we must hash the path name of the file "foo" inside the
   original "system-connections" directory.
   Refactor the code so that it accounds for a difference between the
   filename of the keyfile, and the profile_dir used for generating
   the UUID.

(cherry picked from commit 837d44ffa4)
This commit is contained in:
Thomas Haller 2018-10-02 19:53:54 +02:00
parent 0642fc2d35
commit ae5a09d720
13 changed files with 111 additions and 70 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
libnm-core/nm-keyfile-internal.h
View file

@ -95,7 +95,6 @@ typedef struct {
} NMKeyfileReadTypeDataWarn;
NMConnection *nm_keyfile_read (GKeyFile *keyfile,
const char *keyfile_name,
const char *base_dir,
NMKeyfileReadHandler handler,
void *user_data,

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

@ -2855,17 +2855,9 @@ nm_keyfile_read_ensure_uuid (NMConnection *connection,
/**
* nm_keyfile_read:
* @keyfile: the keyfile from which to create the connection
* @keyfile_name: keyfile allows missing connection id and uuid
* and NetworkManager will create those when reading a connection
* from file. By providing a filename you can reproduce that behavior,
* but of course, it can only recreate the same UUID if you provide the
* same filename as NetworkManager core daemon would.
* @keyfile_name has only a relevance for setting the id or uuid if it
* is missing and as fallback for @base_dir.
* @base_dir: when reading certificates from files with relative name,
* the relative path is made absolute using @base_dir.
* If @base_dir is missing, first try to get the pathname from @keyfile_name
* (if it is given as absolute path). As last, fallback to the current path.
* the relative path is made absolute using @base_dir. This must
* be an absolute path.
* @handler: read handler
* @user_data: user data for read handler
* @error: error
@ -2877,7 +2869,6 @@ nm_keyfile_read_ensure_uuid (NMConnection *connection,
*/
NMConnection *
nm_keyfile_read (GKeyFile *keyfile,
const char *keyfile_name,
const char *base_dir,
NMKeyfileReadHandler handler,
void *user_data,
@ -2888,25 +2879,13 @@ nm_keyfile_read (GKeyFile *keyfile,
NMSetting *setting;
char **groups;
gsize length;
int i;
gsize i;
gboolean vpn_secrets = FALSE;
KeyfileReaderInfo info = { 0 };
gs_free char *base_dir_free = NULL;
g_return_val_if_fail (keyfile, NULL);
g_return_val_if_fail (!error || !*error, NULL);
if (!base_dir) {
/* basedir is not given. Prefer it from the keyfile_name */
if (keyfile_name && keyfile_name[0] == '/') {
base_dir = base_dir_free = g_path_get_dirname (keyfile_name);
} else {
/* if keyfile is not given or not an absolute path, fallback
* to current working directory. */
base_dir = base_dir_free = g_get_current_dir ();
}
} else
g_return_val_if_fail (base_dir[0] == '/', NULL);
g_return_val_if_fail (base_dir && base_dir[0] == '/', NULL);
connection = nm_simple_connection_new ();
@ -2942,15 +2921,6 @@ nm_keyfile_read (GKeyFile *keyfile,
nm_connection_add_setting (connection, NM_SETTING (s_con));
}
if (keyfile_name) {
gs_free char *basename = g_path_get_basename (keyfile_name);
nm_keyfile_read_ensure_id (connection, basename);
}
if (keyfile_name)
nm_keyfile_read_ensure_uuid (connection, keyfile_name);
/* Make sure that we have 'interface-name' even if it was specified in the
* "wrong" (ie, deprecated) group.
*/

46
libnm-core/tests/test-keyfile.c
View file

@ -167,19 +167,28 @@ _nm_keyfile_write (NMConnection *connection,
static NMConnection *
_nm_keyfile_read (GKeyFile *keyfile,
const char *keyfile_name,
const char *base_dir,
NMKeyfileReadHandler read_handler,
void *read_data,
gboolean needs_normalization)
{
GError *error = NULL;
NMConnection *con;
gs_free char *filename = NULL;
gs_free char *base_dir = NULL;
g_assert (keyfile);
g_assert (!keyfile_name || (keyfile_name[0] == '/'));
con = nm_keyfile_read (keyfile, keyfile_name, base_dir, read_handler, read_data, &error);
base_dir = g_path_get_dirname (keyfile_name);
filename = g_path_get_basename (keyfile_name);
con = nm_keyfile_read (keyfile, base_dir, read_handler, read_data, &error);
g_assert_no_error (error);
g_assert (NM_IS_CONNECTION (con));
nm_keyfile_read_ensure_id (con, filename);
nm_keyfile_read_ensure_uuid (con, keyfile_name);
if (needs_normalization) {
nmtst_assert_connection_verifies_after_normalization (con, 0, 0);
nmtst_connection_normalize (con);
@ -205,7 +214,6 @@ static void
_keyfile_convert (NMConnection **con,
GKeyFile **keyfile,
const char *keyfile_name,
const char *base_dir,
NMKeyfileReadHandler read_handler,
void *read_data,
NMKeyfileWriteHandler write_handler,
@ -229,7 +237,7 @@ _keyfile_convert (NMConnection **con,
if (c0) {
c0_k1 = _nm_keyfile_write (c0, write_handler, write_data);
c0_k1_c2 = _nm_keyfile_read (c0_k1, keyfile_name, base_dir, read_handler, read_data, FALSE);
c0_k1_c2 = _nm_keyfile_read (c0_k1, keyfile_name, read_handler, read_data, FALSE);
c0_k1_c2_k3 = _nm_keyfile_write (c0_k1_c2, write_handler, write_data);
g_assert (_nm_keyfile_equals (c0_k1, c0_k1_c2_k3, TRUE));
@ -237,9 +245,9 @@ _keyfile_convert (NMConnection **con,
if (k0) {
NMSetting8021x *s1, *s2;
k0_c1 = _nm_keyfile_read (k0, keyfile_name, base_dir, read_handler, read_data, needs_normalization);
k0_c1 = _nm_keyfile_read (k0, keyfile_name, read_handler, read_data, needs_normalization);
k0_c1_k2 = _nm_keyfile_write (k0_c1, write_handler, write_data);
k0_c1_k2_c3 = _nm_keyfile_read (k0_c1_k2, keyfile_name, base_dir, read_handler, read_data, FALSE);
k0_c1_k2_c3 = _nm_keyfile_read (k0_c1_k2, keyfile_name, read_handler, read_data, FALSE);
/* It is a expeced behavior, that if @k0 contains a relative path ca-cert, @k0_c1 will
* contain that path as relative. But @k0_c1_k2 and @k0_c1_k2_c3 will have absolute paths.
@ -312,7 +320,7 @@ _test_8021x_cert_check (NMConnection *con,
NMSetting8021x *s_8021x;
gs_free char *kval = NULL;
_keyfile_convert (&con, &keyfile, NULL, NULL, NULL, NULL, NULL, NULL, FALSE);
_keyfile_convert (&con, &keyfile, "/_test_8021x_cert_check/foo", NULL, NULL, NULL, NULL, FALSE);
s_8021x = nm_connection_get_setting_802_1x (con);
@ -449,14 +457,14 @@ test_8021x_cert_read (void)
con = nmtst_create_connection_from_keyfile (
"[connection]\n"
"type=ethernet",
"/test_8021x_cert_read/test0", NULL);
"/test_8021x_cert_read/test0");
CLEAR (&con, &keyfile);
keyfile = _keyfile_load_from_data (
"[connection]\n"
"type=ethernet"
);
_keyfile_convert (&con, &keyfile, "/test_8021x_cert_read/test1", NULL, NULL, NULL, NULL, NULL, TRUE);
_keyfile_convert (&con, &keyfile, "/test_8021x_cert_read/test1", NULL, NULL, NULL, NULL, TRUE);
CLEAR (&con, &keyfile);
keyfile = _keyfile_load_from_data (
@ -471,7 +479,7 @@ test_8021x_cert_read (void)
"private-key=102;105;108;101;58;47;47;47;104;111;109;101;47;100;99;98;119;47;68;101;115;107;116;111;112;47;99;101;114;116;105;110;102;114;97;47;99;108;105;101;110;116;46;112;101;109;0;\n"
"private-key-password=12345testing\n"
);
_keyfile_convert (&con, &keyfile, "/test_8021x_cert_read/test2", NULL, NULL, NULL, NULL, NULL, TRUE);
_keyfile_convert (&con, &keyfile, "/test_8021x_cert_read/test2", NULL, NULL, NULL, NULL, TRUE);
CLEAR (&con, &keyfile);
keyfile = _keyfile_load_from_data (
@ -500,7 +508,7 @@ test_8021x_cert_read (void)
"/33333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333111111\n"
"private-key-password=12345testing\n"
);
_keyfile_convert (&con, &keyfile, "/test_8021x_cert_read/test2", NULL, NULL, NULL, NULL, NULL, TRUE);
_keyfile_convert (&con, &keyfile, "/test_8021x_cert_read/test2", NULL, NULL, NULL, NULL, TRUE);
s_8021x = nm_connection_get_setting_802_1x (con);
g_assert (nm_setting_802_1x_get_ca_cert_scheme (s_8021x) == NM_SETTING_802_1X_CK_SCHEME_PATH);
@ -528,7 +536,7 @@ test_8021x_cert_read (void)
"private-key=data:;base64,aGFsbG8=\n" // hallo
"private-key-password=12345testing\n"
);
_keyfile_convert (&con, &keyfile, "/test_8021x_cert_read/test2", NULL, NULL, NULL, NULL, NULL, TRUE);
_keyfile_convert (&con, &keyfile, "/test_8021x_cert_read/test2", NULL, NULL, NULL, NULL, TRUE);
s_8021x = nm_connection_get_setting_802_1x (con);
g_assert (nm_setting_802_1x_get_ca_cert_scheme (s_8021x) == NM_SETTING_802_1X_CK_SCHEME_PATH);
@ -553,7 +561,7 @@ test_8021x_cert_read (void)
"private-key=abc.deR\n"
"private-key-password=12345testing\n"
);
_keyfile_convert (&con, &keyfile, "/test_8021x_cert_read/test2", NULL, NULL, NULL, NULL, NULL, TRUE);
_keyfile_convert (&con, &keyfile, "/test_8021x_cert_read/test2", NULL, NULL, NULL, NULL, TRUE);
s_8021x = nm_connection_get_setting_802_1x (con);
g_assert (nm_setting_802_1x_get_ca_cert_scheme (s_8021x) == NM_SETTING_802_1X_CK_SCHEME_PATH);
@ -578,7 +586,7 @@ test_8021x_cert_read (void)
"private-key=hallo\n"
"private-key-password=12345testing\n"
);
_keyfile_convert (&con, &keyfile, "/test_8021x_cert_read/test2", NULL, NULL, NULL, NULL, NULL, TRUE);
_keyfile_convert (&con, &keyfile, "/test_8021x_cert_read/test2", NULL, NULL, NULL, NULL, TRUE);
s_8021x = nm_connection_get_setting_802_1x (con);
g_assert (nm_setting_802_1x_get_ca_cert_scheme (s_8021x) == NM_SETTING_802_1X_CK_SCHEME_BLOB);
@ -605,7 +613,7 @@ test_team_conf_read_valid (void)
"interface-name=nm-team1\n"
"[team]\n"
"config={\"foo\":\"bar\"}",
"/test_team_conf_read/valid", NULL);
"/test_team_conf_read/valid");
g_assert (con);
s_team = nm_connection_get_setting_team (con);
@ -629,7 +637,7 @@ test_team_conf_read_invalid (void)
"interface-name=nm-team1\n"
"[team]\n"
"config={foobar}",
"/test_team_conf_read/invalid", NULL);
"/test_team_conf_read/invalid");
g_assert (con);
s_team = nm_connection_get_setting_team (con);
@ -657,7 +665,7 @@ test_user_1 (void)
"[user]\n"
"my-value.x=value1\n"
"",
"/test_user_1/invalid", NULL);
"/test_user_1/invalid");
g_assert (con);
s_user = NM_SETTING_USER (nm_connection_get_setting (con, NM_TYPE_SETTING_USER));
g_assert (s_user);
@ -704,7 +712,7 @@ test_user_1 (void)
nm_connection_add_setting (con, NM_SETTING (s_user));
nmtst_connection_normalize (con);
_keyfile_convert (&con, &keyfile, NULL, NULL, NULL, NULL, NULL, NULL, FALSE);
_keyfile_convert (&con, &keyfile, "/test_user_1/foo", NULL, NULL, NULL, NULL, FALSE);
}
/*****************************************************************************/
@ -725,7 +733,7 @@ test_vpn_1 (void)
"service-type=a.b.c\n"
"vpn-key-1=value1\n"
"",
"/test_vpn_1/invalid", NULL);
"/test_vpn_1/invalid");
g_assert (con);
s_vpn = NM_SETTING_VPN (nm_connection_get_setting (con, NM_TYPE_SETTING_VPN));
g_assert (s_vpn);

6
libnm-core/tests/test-setting.c
View file

@ -1319,13 +1319,15 @@ test_ethtool_1 (void)
nmtst_assert_success (keyfile, error);
con3 = nm_keyfile_read (keyfile,
"ethtool-keyfile-name",
NULL,
"/ignored/current/working/directory/for/loading/relative/paths",
NULL,
NULL,
&error);
nmtst_assert_success (con3, error);
nm_keyfile_read_ensure_id (con3, "unused-because-already-has-id");
nm_keyfile_read_ensure_uuid (con3, "unused-because-already-has-uuid");
nmtst_connection_normalize (con3);
nmtst_assert_connection_equals (con, FALSE, con3, FALSE);

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

@ -1857,26 +1857,32 @@ nmtst_assert_hwaddr_equals (gconstpointer hwaddr1, gssize hwaddr1_len, const cha
#if defined(__NM_SIMPLE_CONNECTION_H__) && defined(__NM_SETTING_CONNECTION_H__) && defined(__NM_KEYFILE_INTERNAL_H__)
static inline NMConnection *
nmtst_create_connection_from_keyfile (const char *keyfile_str, const char *keyfile_name, const char *base_dir)
nmtst_create_connection_from_keyfile (const char *keyfile_str, const char *full_filename)
{
GKeyFile *keyfile;
GError *error = NULL;
gboolean success;
NMConnection *con;
gs_free char *filename = g_path_get_basename (full_filename);
gs_free char *base_dir = g_path_get_dirname (full_filename);
g_assert (keyfile_str);
g_assert (full_filename && full_filename[0] == '/');
keyfile = g_key_file_new ();
success = g_key_file_load_from_data (keyfile, keyfile_str, strlen (keyfile_str), G_KEY_FILE_NONE, &error);
g_assert_no_error (error);
g_assert (success);
con = nm_keyfile_read (keyfile, keyfile_name, base_dir, NULL, NULL, &error);
con = nm_keyfile_read (keyfile, base_dir, NULL, NULL, &error);
g_assert_no_error (error);
g_assert (NM_IS_CONNECTION (con));
g_key_file_unref (keyfile);
nm_keyfile_read_ensure_id (con, filename);
nm_keyfile_read_ensure_uuid (con, full_filename);
nmtst_connection_normalize (con);
return con;

2
src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c
View file

@ -9054,7 +9054,7 @@ test_team_reread_slave (void)
"id=142\n"
"ingress-priority-map=\n"
"parent=enp31s0f1\n"
, "/test_team_reread_slave", NULL);
, "/test_team_reread_slave");
/* to double-check keyfile syntax, re-create the connection by hand. */
connection_2 = nmtst_create_minimal_connection ("team-slave-enp31s0f1-142", "74f435bb-ede4-415a-9d48-f580b60eba04", NM_SETTING_VLAN_SETTING_NAME, &s_con);

7
src/settings/plugins/keyfile/nms-keyfile-connection.c
View file

@ -124,6 +124,7 @@ nms_keyfile_connection_init (NMSKeyfileConnection *connection)
NMSKeyfileConnection *
nms_keyfile_connection_new (NMConnection *source,
const char *full_path,
const char *profile_dir,
GError **error)
{
GObject *object;
@ -131,13 +132,15 @@ nms_keyfile_connection_new (NMConnection *source,
const char *uuid;
gboolean update_unsaved = TRUE;
g_assert (source || full_path);
nm_assert (source || full_path);
nm_assert (!full_path || full_path[0] == '/');
nm_assert (!profile_dir || profile_dir[0] == '/');
/* If we're given a connection already, prefer that instead of re-reading */
if (source)
tmp = g_object_ref (source);
else {
tmp = nms_keyfile_reader_from_file (full_path, error);
tmp = nms_keyfile_reader_from_file (full_path, profile_dir, error);
if (!tmp)
return NULL;

3
src/settings/plugins/keyfile/nms-keyfile-connection.h
View file

@ -37,7 +37,8 @@ typedef struct _NMSKeyfileConnectionClass NMSKeyfileConnectionClass;
GType nms_keyfile_connection_get_type (void);
NMSKeyfileConnection *nms_keyfile_connection_new (NMConnection *source,
const char *filename,
const char *full_path,
const char *profile_dir,
GError **error);
#endif /* __NMS_KEYFILE_CONNECTION_H__ */

2
src/settings/plugins/keyfile/nms-keyfile-plugin.c
View file

@ -195,7 +195,7 @@ update_connection (NMSKeyfilePlugin *self,
return FALSE;
}
connection_new = nms_keyfile_connection_new (source, full_path, &local);
connection_new = nms_keyfile_connection_new (source, full_path, nms_keyfile_utils_get_path (), &local);
if (!connection_new) {
/* Error; remove the connection */
if (source)

54
src/settings/plugins/keyfile/nms-keyfile-reader.c
View file

@ -103,33 +103,77 @@ _handler_read (GKeyFile *keyfile,
NMConnection *
nms_keyfile_reader_from_keyfile (GKeyFile *key_file,
const char *filename,
const char *base_dir,
const char *profile_dir,
gboolean verbose,
GError **error)
{
NMConnection *connection;
HandlerReadData data = {
.verbose = verbose,
};
gs_free char *base_dir_free = NULL;
gs_free char *profile_filename_free = NULL;
const char *profile_filename = NULL;
return nm_keyfile_read (key_file, filename, NULL, _handler_read, &data, error);
nm_assert (filename && filename[0]);
nm_assert (!base_dir || base_dir[0] == '/');
nm_assert (!profile_dir || profile_dir[0] == '/');
if (base_dir)
nm_assert (!strchr (filename, '/'));
else {
const char *s;
nm_assert (filename[0] == '/');
/* @base_dir may be NULL, in which case @filename must be an absolute path,
* and the directory is taken as the @base_dir. */
s = strrchr (filename, '/');
base_dir = nm_strndup_a (255, filename, s - filename, &base_dir_free);
if ( !profile_dir
|| nm_streq (base_dir, profile_dir))
profile_filename = filename;
filename = &s[1];
}
connection = nm_keyfile_read (key_file, base_dir, _handler_read, &data, error);
if (!connection)
return NULL;
nm_keyfile_read_ensure_id (connection, filename);
if (!profile_filename) {
profile_filename_free = g_build_filename (profile_dir ?: base_dir, filename, NULL);
profile_filename = profile_filename_free;
}
nm_keyfile_read_ensure_uuid (connection, profile_filename);
return connection;
}
NMConnection *
nms_keyfile_reader_from_file (const char *filename, GError **error)
nms_keyfile_reader_from_file (const char *full_filename,
const char *profile_dir,
GError **error)
{
gs_unref_keyfile GKeyFile *key_file = NULL;
NMConnection *connection = NULL;
GError *verify_error = NULL;
if (!nms_keyfile_utils_check_file_permissions (filename,
nm_assert (full_filename && full_filename[0] == '/');
nm_assert (!profile_dir || profile_dir[0] == '/');
if (!nms_keyfile_utils_check_file_permissions (full_filename,
NULL,
error))
return NULL;
key_file = g_key_file_new ();
if (!g_key_file_load_from_file (key_file, filename, G_KEY_FILE_NONE, error))
if (!g_key_file_load_from_file (key_file, full_filename, G_KEY_FILE_NONE, error))
return NULL;
connection = nms_keyfile_reader_from_keyfile (key_file, filename, TRUE, error);
connection = nms_keyfile_reader_from_keyfile (key_file, full_filename, NULL, profile_dir, TRUE, error);
if (!connection)
return NULL;

6
src/settings/plugins/keyfile/nms-keyfile-reader.h
View file

@ -26,9 +26,13 @@
NMConnection *nms_keyfile_reader_from_keyfile (GKeyFile *key_file,
const char *filename,
const char *base_dir,
const char *profile_dir,
gboolean verbose,
GError **error);
NMConnection *nms_keyfile_reader_from_file (const char *filename, GError **error);
NMConnection *nms_keyfile_reader_from_file (const char *full_filename,
const char *profile_dir,
GError **error);
#endif /* __NMS_KEYFILE_READER_H__ */

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

@ -172,6 +172,7 @@ _handler_write (NMConnection *connection,
static gboolean
_internal_write_connection (NMConnection *connection,
const char *keyfile_dir,
const char *profile_dir,
uid_t owner_uid,
pid_t owner_grp,
const char *existing_path,
@ -308,7 +309,7 @@ _internal_write_connection (NMConnection *connection,
gs_unref_object NMConnection *reread = NULL;
gboolean reread_same = FALSE;
reread = nms_keyfile_reader_from_keyfile (key_file, path, FALSE, NULL);
reread = nms_keyfile_reader_from_keyfile (key_file, path, NULL, profile_dir, FALSE, NULL);
nm_assert (NM_IS_CONNECTION (reread));
@ -358,6 +359,7 @@ nms_keyfile_writer_connection (NMConnection *connection,
return _internal_write_connection (connection,
keyfile_dir,
nms_keyfile_utils_get_path (),
0, 0,
existing_path,
force_rename,
@ -378,6 +380,7 @@ nms_keyfile_writer_test_connection (NMConnection *connection,
GError **error)
{
return _internal_write_connection (connection,
keyfile_dir,
keyfile_dir,
owner_uid, owner_grp,
NULL,

1
src/settings/plugins/keyfile/tests/test-keyfile.c
View file

@ -73,6 +73,7 @@ check_ip_route (NMSettingIPConfig *config, int idx, const char *destination, int
g_assert (full_filename && full_filename[0] == '/'); \
\
_connection = nms_keyfile_reader_from_file (full_filename, \
NULL, \
(nmtst_get_rand_int () % 2) ? &_error : NULL); \
nmtst_assert_success (_connection, _error); \
nmtst_assert_connection_verifies_without_normalization (_connection); \