From 165e5df6e070b9315c8062b94b0133e2e1534678 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Mon, 23 Jun 2025 10:49:21 +0200 Subject: [PATCH] nmcli: accept certain IP settings on port connections Commit bb850fda0ed9 ('nmcli: connection: process port-type, type and controller first') started correctly rejecting IP configuration on port connections. However, previously nmcli would accept IP parameters for ports when using a specific parameters order. To avoid breaking user scripts that may have relied on this behavior, introduce a backward compatibility quirk. Specifically, nmcli accepts a disabled/ignore IP method on a port connection. For any other IP setting on a port connection, a specific error message is now shown. https://issues.redhat.com/browse/RHEL-90756 https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/2227 --- src/nmcli/connections.c | 99 +++++++++++++--- .../test_004.expected | 40 +++++++ src/tests/client/test-client.py | 107 ++++++++++++++++++ tools/test-networkmanager-service.py | 1 + 4 files changed, 234 insertions(+), 13 deletions(-) diff --git a/src/nmcli/connections.c b/src/nmcli/connections.c index f53f2610d..409f80673 100644 --- a/src/nmcli/connections.c +++ b/src/nmcli/connections.c @@ -5266,6 +5266,45 @@ _copy_connection_properties(const char ***dst, return count; } +static gboolean +is_ip_setting_for_port_connection(NMConnection *connection, + const char *setting_name, + const char *property_name, + const char *value, + gboolean *out_is_method_disabled) +{ + NMSettingConnection *s_con; + + *out_is_method_disabled = FALSE; + + if (!NM_IN_STRSET(setting_name, + NM_SETTING_IP4_CONFIG_SETTING_NAME, + NM_SETTING_IP6_CONFIG_SETTING_NAME)) + return FALSE; + + s_con = nm_connection_get_setting_connection(connection); + if (!s_con) + return FALSE; + + if (!nm_setting_connection_get_controller(s_con)) + return FALSE; + + if (nm_streq(property_name, NM_SETTING_IP_CONFIG_METHOD)) { + if (nm_streq(setting_name, NM_SETTING_IP4_CONFIG_SETTING_NAME)) { + if (nm_streq(value, NM_SETTING_IP4_CONFIG_METHOD_DISABLED)) + *out_is_method_disabled = TRUE; + } else { + /* IPv6 */ + if (NM_IN_STRSET(value, + NM_SETTING_IP6_CONFIG_METHOD_DISABLED, + NM_SETTING_IP6_CONFIG_METHOD_IGNORE)) + *out_is_method_disabled = TRUE; + } + } + + return TRUE; +} + gboolean nmc_process_connection_properties(NmCli *nmc, NMConnection *connection, @@ -5428,24 +5467,58 @@ nmc_process_connection_properties(NmCli *nmc, if (argc == 1 && nmc->complete) complete_property_name(nmc, connection, modifier, option_sett, option_prop); - option_sett_expanded = - check_valid_name(option_sett, type_settings, port_settings, &local); - if (!option_sett_expanded) { - g_set_error(error, - NMCLI_ERROR, - NMC_RESULT_ERROR_USER_INPUT, - _("Error: invalid or not allowed setting '%s': %s."), - option_sett, - local->message); - g_clear_error(&local); - return FALSE; - } - argc--; argv++; if (!get_value(&value, &argc, &argv, option_orig, error)) return FALSE; + option_sett_expanded = + check_valid_name(option_sett, type_settings, port_settings, &local); + if (!option_sett_expanded) { + gboolean raise_error = TRUE; + gboolean is_method_disabled = FALSE; + + /* The setting does not exist or is now allowed for the given + * connection type or for the given port type. In the past nmcli + * accepted IP-config properties for port connections under some + * circumstances. For backward bug compatibility, still allow + * the user to set the IP method to disabled/ignore for ports, + * so that we don't break user scripts. + * */ + if (is_ip_setting_for_port_connection(connection, + option_sett, + option_prop, + value, + &is_method_disabled)) { + if (is_method_disabled) { + /* Allowed */ + option_sett_expanded = option_sett; + raise_error = FALSE; + g_clear_error(&local); + } else { + /* The property is not a disabled/ignore IP method. Raise a + * meaningful error, instead of the generic "setting X is not + * among LIST" */ + g_clear_error(&local); + g_set_error(&local, + NMCLI_ERROR, + NMC_RESULT_ERROR_USER_INPUT, + _("port connections cannot have IP configuration")); + raise_error = TRUE; + } + } + if (raise_error) { + g_set_error(error, + NMCLI_ERROR, + NMC_RESULT_ERROR_USER_INPUT, + _("Error: invalid or not allowed setting '%s': %s."), + option_sett, + local->message); + g_clear_error(&local); + return FALSE; + } + } + if (!argc && nmc->complete) { complete_property(nmc, option_sett, option_prop, value ?: "", connection); break; diff --git a/src/tests/client/test-client.check-on-disk/test_004.expected b/src/tests/client/test-client.check-on-disk/test_004.expected index 7c81ae3b8..09422f108 100644 --- a/src/tests/client/test-client.check-on-disk/test_004.expected +++ b/src/tests/client/test-client.check-on-disk/test_004.expected @@ -57981,3 +57981,43 @@ con-xx2 UUID-con-xx2-REPLACED-REPLACED-REPLA ethernet 0 nigdy con-1 5fcfd6d7-1e63-3332-8826-a7eda103792d ethernet 0 nigdy tak 0 nie /org/freedesktop/NetworkManager/Settings/Connection/1 nie -- -- -- -- /etc/NetworkManager/system-connections/con-1 <<< +size: 359 +location: src/tests/client/test-client.py:test_004()/934 +cmd: $NMCLI connection add type ethernet ifname foobar con-name con-port1 ipv4.method disabled ipv6.method ignore connection.port-type bridge connection.controller bridge1 +lang: C +returncode: 0 +stdout: 82 bytes +>>> +Connection 'con-port1' (UUID-con-port1-REPLACED-REPLACED-REP) successfully added. + +<<< +size: 353 +location: src/tests/client/test-client.py:test_004()/935 +cmd: $NMCLI connection add type ethernet ifname foobar con-name ethernet-foobar ipv6.method auto connection.port-type bridge connection.controller bridge1 +lang: C +returncode: 2 +stderr: 93 bytes +>>> +Error: invalid or not allowed setting 'ipv6': port connections cannot have IP configuration. + +<<< +size: 373 +location: src/tests/client/test-client.py:test_004()/936 +cmd: $NMCLI connection add type ovs-interface ifname ovs-int con-name con-ovs-int ipv4.method auto ipv6.method link-local connection.port-type ovs-port connection.controller ovs-port1 +lang: C +returncode: 0 +stdout: 84 bytes +>>> +Connection 'con-ovs-int' (UUID-con-ovs-int-REPLACED-REPLACED-R) successfully added. + +<<< +size: 377 +location: src/tests/client/test-client.py:test_004()/937 +cmd: $NMCLI connection add type ethernet ifname enp1s0 con-name con-port2 ipv4.method manual ipv4.addresses 192.0.2.1/24 ipv6.method dhcp connection.port-type vrf connection.controller vrf1 +lang: C +returncode: 0 +stdout: 82 bytes +>>> +Connection 'con-port2' (UUID-con-port2-REPLACED-REPLACED-REP) successfully added. + +<<< diff --git a/src/tests/client/test-client.py b/src/tests/client/test-client.py index 62b72b795..cc76d37b5 100755 --- a/src/tests/client/test-client.py +++ b/src/tests/client/test-client.py @@ -2125,6 +2125,113 @@ class TestNmcli(unittest.TestCase): replace_stdout=replace_uuids, ) + # It is allowed to set IP method disabled/ignore for ports + replace_uuids.append( + self.ctx.srv.ReplaceTextConUuid( + "con-port1", "UUID-con-port1-REPLACED-REPLACED-REP" + ) + ) + self.call_nmcli( + [ + "connection", + "add", + "type", + "ethernet", + "ifname", + "foobar", + "con-name", + "con-port1", + "ipv4.method", + "disabled", + "ipv6.method", + "ignore", + "connection.port-type", + "bridge", + "connection.controller", + "bridge1", + ], + replace_stdout=replace_uuids, + ) + + # It is NOT allowed to set IP method != disabled/ignore for ports + self.call_nmcli( + [ + "connection", + "add", + "type", + "ethernet", + "ifname", + "foobar", + "con-name", + "ethernet-foobar", + "ipv6.method", + "auto", + "connection.port-type", + "bridge", + "connection.controller", + "bridge1", + ], + replace_stdout=replace_uuids, + ) + + # ovs-interface connections support IP configuration + replace_uuids.append( + self.ctx.srv.ReplaceTextConUuid( + "con-ovs-int", "UUID-con-ovs-int-REPLACED-REPLACED-R" + ) + ) + self.call_nmcli( + [ + "connection", + "add", + "type", + "ovs-interface", + "ifname", + "ovs-int", + "con-name", + "con-ovs-int", + "ipv4.method", + "auto", + "ipv6.method", + "link-local", + "connection.port-type", + "ovs-port", + "connection.controller", + "ovs-port1", + ], + replace_stdout=replace_uuids, + ) + + # VRF ports support IP configuration + replace_uuids.append( + self.ctx.srv.ReplaceTextConUuid( + "con-port2", "UUID-con-port2-REPLACED-REPLACED-REP" + ) + ) + self.call_nmcli( + [ + "connection", + "add", + "type", + "ethernet", + "ifname", + "enp1s0", + "con-name", + "con-port2", + "ipv4.method", + "manual", + "ipv4.addresses", + "192.0.2.1/24", + "ipv6.method", + "dhcp", + "connection.port-type", + "vrf", + "connection.controller", + "vrf1", + ], + replace_stdout=replace_uuids, + ) + @nm_test_no_dbus def test_offline(self): # Make sure we're not using D-Bus diff --git a/tools/test-networkmanager-service.py b/tools/test-networkmanager-service.py index 8d39aef78..0a13f2bd0 100755 --- a/tools/test-networkmanager-service.py +++ b/tools/test-networkmanager-service.py @@ -613,6 +613,7 @@ class NmUtil: if t not in [ NM.SETTING_GSM_SETTING_NAME, NM.SETTING_MACVLAN_SETTING_NAME, + NM.SETTING_OVS_INTERFACE_SETTING_NAME, NM.SETTING_VLAN_SETTING_NAME, NM.SETTING_VPN_SETTING_NAME, NM.SETTING_WIMAX_SETTING_NAME,