From 38dca2f044a5cf0f68752d3d73ceb5407f948238 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Tue, 12 Nov 2024 18:20:27 +0100 Subject: [PATCH] keyfile: write the gateway explicitly The keyfile format allows to specify the gateway in two ways: with a separate "gateway" key, or by appending the gateway address to one of the address$N lines: [ipv4] address1=192.0.2.1/24 gateway=192.0.2.254 [ipv4] address1=192.0.2.1/24,192.0.2.254 The former syntax is self-documenting and easier to understand for users, but NetworkManager defaults to the latter when writing connection files, for historical reasons. Change that and use the explicit form. Note that if a users has scripts manually parsing keyfiles, they could stop working and so this can be considered an API breakage. OTOH, those scripts are buggy if they don't support both forms, and they can already break with perfectly valid user-generated keyfiles. I think it's acceptable to change the default way to persist keyfiles; the only precaution would be that this patch should not be applied during a stable release cycle of a distro. --- .../keyfile/tests/keyfiles/Test_Write_Bridge_Main | 3 ++- .../keyfile/tests/keyfiles/Test_Write_Wired | 3 ++- .../keyfile/tests/keyfiles/Test_Write_Wired_IP6 | 3 ++- src/libnm-core-impl/nm-keyfile.c | 15 ++++----------- 4 files changed, 10 insertions(+), 14 deletions(-) diff --git a/src/core/settings/plugins/keyfile/tests/keyfiles/Test_Write_Bridge_Main b/src/core/settings/plugins/keyfile/tests/keyfiles/Test_Write_Bridge_Main index 49a7372384..03182b4379 100644 --- a/src/core/settings/plugins/keyfile/tests/keyfiles/Test_Write_Bridge_Main +++ b/src/core/settings/plugins/keyfile/tests/keyfiles/Test_Write_Bridge_Main @@ -9,7 +9,8 @@ interface-name=br0 [bridge] [ipv4] -address1=1.2.3.4/24,1.1.1.1 +address1=1.2.3.4/24 +gateway=1.1.1.1 method=manual [ipv6] diff --git a/src/core/settings/plugins/keyfile/tests/keyfiles/Test_Write_Wired b/src/core/settings/plugins/keyfile/tests/keyfiles/Test_Write_Wired index 0b377c826b..69ca9a6dc2 100644 --- a/src/core/settings/plugins/keyfile/tests/keyfiles/Test_Write_Wired +++ b/src/core/settings/plugins/keyfile/tests/keyfiles/Test_Write_Wired @@ -10,9 +10,10 @@ mac-address=99:88:77:66:55:44 mtu=900 [ipv4] -address1=192.168.0.5/24,192.168.0.1 +address1=192.168.0.5/24 address2=1.2.3.4/8 dns=4.2.2.1;4.2.2.2; +gateway=192.168.0.1 method=manual route1=10.10.10.2/24,10.10.10.1,3 route2=1.1.1.1/8,1.2.1.1,1 diff --git a/src/core/settings/plugins/keyfile/tests/keyfiles/Test_Write_Wired_IP6 b/src/core/settings/plugins/keyfile/tests/keyfiles/Test_Write_Wired_IP6 index 1efa7406c2..cc379ec66c 100644 --- a/src/core/settings/plugins/keyfile/tests/keyfiles/Test_Write_Wired_IP6 +++ b/src/core/settings/plugins/keyfile/tests/keyfiles/Test_Write_Wired_IP6 @@ -11,9 +11,10 @@ method=disabled [ipv6] addr-gen-mode=default -address1=abcd::beef/64,dcba::beef +address1=abcd::beef/64 dns=1::cafe; dns-search=wallaceandgromit.com; +gateway=dcba::beef method=manual [proxy] diff --git a/src/libnm-core-impl/nm-keyfile.c b/src/libnm-core-impl/nm-keyfile.c index 4e905f4d6e..93afaedfa0 100644 --- a/src/libnm-core-impl/nm-keyfile.c +++ b/src/libnm-core-impl/nm-keyfile.c @@ -2263,11 +2263,7 @@ ip6_addr_gen_mode_writer(KeyfileWriterInfo *info, } static void -write_ip_values(GKeyFile *file, - const char *setting_name, - GPtrArray *array, - const char *gateway, - gboolean is_route) +write_ip_values(GKeyFile *file, const char *setting_name, GPtrArray *array, gboolean is_route) { if (array->len > 0) { nm_auto_str_buf NMStrBuf output = NM_STR_BUF_INIT(2 * INET_ADDRSTRLEN + 10, FALSE); @@ -2300,7 +2296,7 @@ write_ip_values(GKeyFile *file, addr = nm_ip_address_get_address(address); plen = nm_ip_address_get_prefix(address); - gw = (i == 0) ? gateway : NULL; + gw = NULL; } nm_str_buf_set_size(&output, 0, FALSE, FALSE); @@ -2351,11 +2347,10 @@ addr_writer(KeyfileWriterInfo *info, NMSetting *setting, const char *key, const { GPtrArray *array; const char *setting_name = nm_setting_get_name(setting); - const char *gateway = nm_setting_ip_config_get_gateway(NM_SETTING_IP_CONFIG(setting)); array = (GPtrArray *) g_value_get_boxed(value); if (array && array->len) - write_ip_values(info->keyfile, setting_name, array, gateway, FALSE); + write_ip_values(info->keyfile, setting_name, array, FALSE); } static void @@ -2366,7 +2361,7 @@ route_writer(KeyfileWriterInfo *info, NMSetting *setting, const char *key, const array = (GPtrArray *) g_value_get_boxed(value); if (array && array->len) - write_ip_values(info->keyfile, setting_name, array, NULL, TRUE); + write_ip_values(info->keyfile, setting_name, array, TRUE); } static void @@ -3060,7 +3055,6 @@ static const ParseInfoSetting *const parse_infos[_NM_META_SETTING_TYPE_NUM] = { .parser = ip_dns_parser, .writer = dns_writer, ), PARSE_INFO_PROPERTY(NM_SETTING_IP_CONFIG_DNS_OPTIONS, .always_write = TRUE, ), - PARSE_INFO_PROPERTY(NM_SETTING_IP_CONFIG_GATEWAY, .writer_skip = TRUE, ), PARSE_INFO_PROPERTY(NM_SETTING_IP_CONFIG_ROUTES, .parser_no_check_key = TRUE, .parser = ip_address_or_route_parser, @@ -3088,7 +3082,6 @@ static const ParseInfoSetting *const parse_infos[_NM_META_SETTING_TYPE_NUM] = { .parser = ip_dns_parser, .writer = dns_writer, ), PARSE_INFO_PROPERTY(NM_SETTING_IP_CONFIG_DNS_OPTIONS, .always_write = TRUE, ), - PARSE_INFO_PROPERTY(NM_SETTING_IP_CONFIG_GATEWAY, .writer_skip = TRUE, ), PARSE_INFO_PROPERTY(NM_SETTING_IP_CONFIG_ROUTES, .parser_no_check_key = TRUE, .parser = ip_address_or_route_parser,