diff options
author | Thomas Haller <thaller@redhat.com> | 2018-10-02 19:53:54 +0200 |
---|---|---|
committer | Thomas Haller <thaller@redhat.com> | 2018-10-04 11:03:23 +0200 |
commit | 837d44ffa4bfb3ef1a1cd786336dcd2415e9259b (patch) | |
tree | 4c12eb78ac82500476b2c31673465065bfbe620e /libnm-core | |
parent | 02c8844178380c5f4cc1c6aa971f90cfea2be0dc (diff) |
keyfile: split automatically setting ID/UUID for keyfile
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.
Diffstat (limited to 'libnm-core')
-rw-r--r-- | libnm-core/nm-keyfile-internal.h | 1 | ||||
-rw-r--r-- | libnm-core/nm-keyfile.c | 38 | ||||
-rw-r--r-- | libnm-core/tests/test-keyfile.c | 46 | ||||
-rw-r--r-- | libnm-core/tests/test-setting.c | 6 |
4 files changed, 35 insertions, 56 deletions
diff --git a/libnm-core/nm-keyfile-internal.h b/libnm-core/nm-keyfile-internal.h index a620d9aa1..94228d951 100644 --- a/libnm-core/nm-keyfile-internal.h +++ b/libnm-core/nm-keyfile-internal.h @@ -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, diff --git a/libnm-core/nm-keyfile.c b/libnm-core/nm-keyfile.c index 4de0ca415..77c1534d5 100644 --- a/libnm-core/nm-keyfile.c +++ b/libnm-core/nm-keyfile.c @@ -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. */ diff --git a/libnm-core/tests/test-keyfile.c b/libnm-core/tests/test-keyfile.c index 865cb68cc..9b9d1c041 100644 --- a/libnm-core/tests/test-keyfile.c +++ b/libnm-core/tests/test-keyfile.c @@ -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 expected 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); diff --git a/libnm-core/tests/test-setting.c b/libnm-core/tests/test-setting.c index c8eda381e..4e010feb6 100644 --- a/libnm-core/tests/test-setting.c +++ b/libnm-core/tests/test-setting.c @@ -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); |