Commit graph

17329 commits

Author SHA1 Message Date
Jan Vaclav
8d8eaca0d0 nmtui: correctly check that connection list is non-empty
Previously, the "edit" or "delete" buttons were clickable even
if there were no available connections, which was not expected
and caused an assertion to fail when clicked. This is because
the connections list could contain connections that were later
filtered out and not displayed in the final list, but the check
did not take this into account.

Make it so that the buttons are clickable only if we *actually*
have any available connections to edit or delete.

Fixes: 3bda3fb60c ('nmtui: initial import of nmtui')
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1991
(cherry picked from commit c9fefcd095)
(cherry picked from commit f6e4d1b2e0)
(cherry picked from commit de444a4562)
2024-12-23 13:56:52 +01:00
Martin von Gagern
a009cabcb4 core: fix dhcp leases in l3cd cloning.
This will copy DHCP lease from src (not self), fixing an assertion in
nm_l3_config_data_new_clone.

Resolves: https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/1620
Fixes: 55b722820d ('l3cfg: fix nm_l3_config_data_new_clone() to make exact copy')
(cherry picked from commit a25d593922)
(cherry picked from commit 61e781c4f5)
(cherry picked from commit fdc909741d)
2024-12-23 13:51:48 +01:00
Beniamino Galvani
14d6b7be3e cloud-setup: fix placement of directives in the systemd unit
The "StartLimitIntervalSec" and "StartLimitBurst" directives should be
in the [Unit] section instead of the [Service] one.

Fixes: 927cff9f17 ('cloud-setup: allow bigger restart bursts')
(cherry picked from commit a531458456)
(cherry picked from commit e34c7cd5a2)
(cherry picked from commit 97ee4c688f)
2024-12-23 13:51:23 +01:00
Beniamino Galvani
317b9a3a9c ovs: fix assertion failure in netdev datapath mode
When using the netdev datapath, we wait for the link to appear in
different steps:

 1. initially, in act_stage3_ip_config() connects to platform's
   "link-changed" signal to detect when the TUN interface appears;

 2. when the interface appears, _netdev_tun_link_cb() schedules
   _set_ip_ifindex_tun() in a idle handler;

 3. _set_ip_ifindex_tun() checks if the link is ready (e.g. if the MAC
   address is correct) and in that case it reschedules stage3, which
   will move forward with the activation;

 4. if the link is not ready in _set_ip_ifindex_tun(), the function
   connects again to platform's "link-changed" signal to react to link
   changes;

 5. after the link changes and it is ready, _netdev_tun_link_cb()
   reschedules stage3, which moves forward with the activation;

With the current implementation it is possible that after step 2, if
act_stage3_ip_config() runs because it was already scheduled, it
registers again to the "link-changed" event; then when
_set_ip_ifindex_tun() is invoked it will hit assertion:

  nm_assert(!priv->wait_link.tun_link_signal_id);

Fix this by preventing that the signal gets registered again after
step 2.

Fixes-test: @ovs_datapath_type_netdev_with_cloned_mac

Fixes: acf485196c ('ovs-interface: wait that the cloned MAC changes instead of setting it')

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/2024
(cherry picked from commit b6e69f3467)
(cherry picked from commit 50da988182)
(cherry picked from commit d82d8a027c)
2024-12-23 13:50:50 +01:00
Jan Vaclav
d13f0c04ed nmcli/edit: fix memory leak in extract_setting_and_property
In case the user selects a setting/property with "goto" command, and
then attempts to tab-complete a setting/property pair, the original sett
and prop strings are overriden without freeing:

  nmcli > goto 802-1x.pac-file
  nmcli 802-1x.pac-file> set 802-1.lal<TAB>

Fixes: 79bc271685 ('cli: TAB-completion for enum-style property values (rh #1034126)')
(cherry picked from commit ca47fd882e)
(cherry picked from commit 796844dc09)
2024-12-23 13:49:51 +01:00
Beniamino Galvani
06032aa758 ndisc: preserve router preferences
If we add multiple default routes with the same metric and different
preferences, kernel merges them into a single ECMP route, with overall
preference equal to the preference of the first route
added. Therefore, the preference of individual routes is not
respected.

To avoid that, add routes with different metrics if they have
different preferences, so that they are not merged together.

We could configure only the route(s) with highest preference ignoring
the others, and the effect would be the same. However, it is better to
add all routes so that users can easily see from "ip route" that there
are multiple routers available.

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/1468
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1983

Fixes: 032b4e4371 ('core: use router preference for IPv6 routes')
(cherry picked from commit c437625a76)
(cherry picked from commit 8445076d55)
2024-12-23 13:46:47 +01:00
Beniamino Galvani
e4d7cb7453 policy: assert that the auto-activate list is empty on dispose
We are currently asserting that the list of devices waiting for
auto-activation in NMPolicy is not empty. This condition is always
false because:

 - NMDevice holds a reference to NMManager
 - NMManager holds a reference to NMPolicy
 - on dispose, NMDevice asserts that it's not in NMPolicy's
   auto-activate list

Therefore if there is any NMDevice alive, NMPolicy must be alive as
well. Instead, if there is no NMDevice alive the list must be empty.

The assertion could fail only when the NMPolicy instance gets
disposed, which usually doesn't happen because it's still referenced
at shutdown.

Fixes: aede228974 ('core: assert that devices are not registered when disposing NMPolicy')
(cherry picked from commit 27b646cfa1)
(cherry picked from commit 1b51404703)
2024-12-23 13:46:32 +01:00
Jan Vaclav
19a8feaad6 agent: print error message character in hex form if it's unprintable
Currently, when the agent manager is sent a registration request
containing UTF-8 characters, it will form an invalid error message
using only one of the bytes from the UTF-8 sequence, which causes
an assertion in glib to fail, which replaces the returned error message
with "[Invalid UTF-8]". It will also print an assertion failure to the
console, or crash NetworkManager on non-release builds.

This commit makes it so that it instead prints out the character in
hexadecimal form if it isn't normally printable, so that it is once
again a valid UTF-8 string.

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1965

Fixes: a30cf19858 ('agent: add agent manager and minimal agent class')
(cherry picked from commit c9327b2e8b)
(cherry picked from commit f6f466ccf8)
2024-12-23 13:45:54 +01:00
Jan Vaclav
277863ef8f nmtui: handle write() errors correctly in nmt_newt_edit_string
It might happen that write() returns -1, but the errno is not EINTR.
In that case, the length would be incremented by 1, and the data pointer
to the data being written would be moved back by 1 byte on every error.

Make it so that the function exits with an error if it indicates an error.

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1971

Fixes: 3bda3fb60c ('nmtui: initial import of nmtui')
(cherry picked from commit 13317bd536)
(cherry picked from commit f1888900bd)
2024-12-23 13:45:04 +01:00
Beniamino Galvani
1006b39ef1 wifi: fix list corruption when scanning with explicit SSID
Calling c_list_link_tail() on a list entry that already belongs to
another list corrupts the other list, in this case 'old_lst_head';
this is explained in the documentation of c_list_link_before():

 * @what is not inspected prior to being linked. Hence, it better not
 * be linked into another list, or the other list will be corrupted.

This can be reproduced by invoking "nmcli device wifi rescan ssid x"
multiple times; in this way, _scan_request_ssids_track() reuses the
previous SSID data, the list gets corrupted and this causes a crash.

Fixes: 7500e90b53 ('wifi: rework scanning of Wi-Fi device')

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/2076
(cherry picked from commit 3b75577871)
(cherry picked from commit 3917235a2d)
(cherry picked from commit 409acc6185)
2024-11-26 16:21:43 +01:00
Gris Ge
296fc53ea8 sriov: only valid sriov capacity when enabled
NetworkManager current code will refuse to activate a connection if its
interface has no SRIOV capacity but holding a empty SRIOV settings.

This patch only valid SRIOV capacity when it is enabled(total_vfs > 0).

Resolves: https://issues.redhat.com/browse/RHEL-58397

Signed-off-by: Gris Ge <fge@redhat.com>
(cherry picked from commit 421ccf8b4c)
(cherry picked from commit c9e31e70cb)
(cherry picked from commit 90a3b01468)
2024-11-13 11:48:02 +01:00
Beniamino Galvani
83c32e9f17 device: fix bug when deactivating port connections asynchronously
When the attach_port()/detach_port() methods do not return immediately
(currently, only for OVS ports), the following situation can arise:

 - nm_device_controller_attach_port() starts the attachment by sending
   the command to ovsdb. Note that here we don't set
   `PortInfo->port_is_attached` to TRUE yet; that happens only after
   the asynchronous command returns;

 - the activation of the port gets interrupted because the connection
   is deleted;

 - the port device enters the deactivating state, triggering function
   port_state_changed()

 - the function calls nm_device_controller_release_port() which checks
   whether the port is already attached; since
   `PortInfo->port_is_attached` is not set yet, it assumes the port
   doesn't need to be detached;

 - in the meantime, the ovsdb operation succeeds. As a consequence,
   the kernel link is created even if the connection no longer exists.

Fix this by turning `port_is_attached` into a tri-state variable that
also tracks when the port is attaching. When it is, we need to perform
an explicit detach during deactivation.

Fixes: 9fcbc6b37d ('device: make attach_port() asynchronous')

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/2043

Resolves: https://issues.redhat.com/browse/RHEL-58026
(cherry picked from commit a8329587c8)
(cherry picked from commit d809ca6db2)
(cherry picked from commit ca6ca684b2)
2024-11-06 11:36:01 +01:00
Beniamino Galvani
73f10372f9 libnm-core: fix validation of ovs-dpdk interface name
An ovs-dpdk interface doesn't have a kernel link and doesn't have the
15-character limit on the name.

Fixes: 3efe070dfc ('libnm: validate "connection.interface-name" at one place only')

Resolves: https://issues.redhat.com/browse/RHEL-60233

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/2044
(cherry picked from commit fda05b0af0)
(cherry picked from commit f6e4e53775)
(cherry picked from commit c7035db5b4)
2024-09-26 09:54:46 +02:00
Beniamino Galvani
a0825a8fd5 dhcp: remove the address synchronously on lease expiry
When the lease expires, the DHCP client emits a LEASE_UPDATE event
with a NULL l3cd. After returning from the handler, it sends
immediately a DHCP DISCOVER message to try to get a new lease.

It is important that when the DISCOVER gets sent the address is no
longer configured on the interface. Otherwise, the server could see
that it is already in use and assign a different one. Therefore,
remove the address synchronously when handling the event.

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/1532
(cherry picked from commit 514a3cb610)
(cherry picked from commit f2e61addc4)
2024-09-13 10:17:41 +02:00
Íñigo Huguet
55812963fd cloud-setup: azure: ensure that primary address is placed first
The primary address is that placed at position 0 of all the IP Addresses
of the interface. Sometimes we put it in a different position in the
ipv4s array because we insert them in the order we receive, but it might
happen that the HTTP responses comes back in wrong order.

In order to solve this, we pass the index of the IPv4 address to the
callback and the address is added in the right position directly.

Co-authored-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
(cherry picked from commit 72014db629)
(cherry picked from commit c976e21237)
2024-08-30 13:10:01 +02:00
Lubomir Rintel
097dfdf711 cloud-setup: allow bigger restart bursts
On daemon startup, we may end up enqueueing many nm-cloud-setup.service
restarts in very a short time. That is perfectly fine, just bump the
thresholds so that systemd doesn't get in the way too quickly.

100 requests in 1 seconds seem like a fair choice -- little bit on the
conservative side, yet still giving the service manager some room to
interfere on a chance things really go awry.

https://issues.redhat.com/browse/RHEL-49694
(cherry picked from commit 927cff9f17)
(cherry picked from commit 4dc35c7274)
2024-08-30 13:09:53 +02:00
Beniamino Galvani
7ae0f3edf0 policy: retry hostname resolution when it fails
Currently if the system hostname can't be determined, NetworkManager
only retries when something changes: a new address is added, the DHCP
lease changes, etc.

However, it might happen that the current failure in looking up the
hostname is caused by an external factor, like a temporary outage of
the DNS server.

Add a mechanism to retry the resolution with an increasing timeout.

https://issues.redhat.com/browse/RHEL-17972
(cherry picked from commit 04ad4c86d0)
(cherry picked from commit 3555dbd2f2)
2024-08-30 13:02:51 +02:00
Beniamino Galvani
e3861be845 core: also use /etc/hosts for hostname resolution
Before introducing the hostname lookup via nm-daemon-helper and
systemd-resolved, we used GLib's GResolver which internally relies on
the libc resolver and generally also returns results from /etc/hosts.

With the new mechanism we only ask to systemd-resolved (with
NO_SYNTHESIZE) or perform the lookup via the "dns" NSS module. In both
ways, /etc/hosts is not evaluated.

Since users relied on having the hostname resolved via /etc/hosts,
restore that behavior. Now, after trying the resolution via
systemd-resolved and the "dns" NSS module, we also try via the "files"
NSS module which reads /etc/hosts.

Fixes: 27eae4043b ('device: add a nm_device_resolve_address()')
(cherry picked from commit 410afccb32)
(cherry picked from commit cb54fe7ce9)
2024-08-19 17:04:49 +02:00
Beniamino Galvani
e86ddd9fc5 nm-daemon-helper: add "service" argument
Introduce a new argument to specify a comma-separated list of NSS
services to use for the "resolve-address" command. For now only accept
"dns" and "files"; the latter can be used to do a lookup into
/etc/hosts.

Note that previously the command failed in presence of extra
arguments. Therefore, when downgrading NetworkManager without
restarting the service, the previously-installed version of the daemon
(newer) would spawn the helper with the extra argument, and the
newly-installed version of the helper (older) would fail. This issue
only impacts hostname resolution and can be fixed by just restarting
the daemon.

In the upgrade path everything works as before, with the only
difference that the helper will use by default both "dns" and "files"
services.

Don't strictly check for the absence of extra arguments, so that in
the future we can introduce more arguments without necessarily break
the downgrade path.

(cherry picked from commit 229bebfae9)
(cherry picked from commit c36a74f698)
2024-08-19 16:56:00 +02:00
Thomas Haller
9905bcdcb7 lldp: fix multiple access to argument in logging macro
Fixes: 630de288d2 ('lldp: add libnm-lldp as fork of systemd's sd_lldp_rx')
(cherry picked from commit 4365de5226)
(cherry picked from commit a1c18ce20d)
2024-08-19 15:24:05 +02:00
Thomas Haller
6da9b98975 lldp: fix crash dereferencing NULL pointer during debug logging
During nm_lldp_neighbor_parse(), the NMLldpNeighbor is not yet added to
the NMLldpRX instance. Consequently, n->lldp_rx is NULL.

Note how we use lldp_x for logging, because we need it for the context
for which interface the logging statement is.

Thus, those debug logging statements will follow a NULL pointer and lead
to a crash.

Fixes: 630de288d2 ('lldp: add libnm-lldp as fork of systemd's sd_lldp_rx')

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/1550
(cherry picked from commit c2cddd3241)
(cherry picked from commit 8a2f7bd6e0)
2024-08-19 15:24:05 +02:00
Wen Liang
02db74ed0b policy: unblock the autoconnect for children when parent is available
When parent is available and in the process of activation, we should
unblock the autoconnect and schedule an auto activate for the children.
Notice that when the parent is the ovs-interface, the kernel link is
only created in stage3, if we only unblock the children in the stage1,
then the children device and connection will be blocked again due
to the fact the kernel link for the parent ovs-interface is not
existed yet, thus, we have to separately unblock the children
when the parent ovs-interface is in the activated state.

https://issues.redhat.com/browse/RHEL-46904
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/2003
https://gitlab.freedesktop.org/NetworkManager/NetworkManager-ci/-/merge_requests/1735
(cherry picked from commit 5f64f292e6)
(cherry picked from commit 8243425c6d)
2024-08-13 08:37:10 -04:00
Fernando Fernandez Mancera
f8f5626f72 ovs: fix triggering stage3 activation without DHCP client initialized
It is possible that we learn the link is ready on stage3_ip_config
rather than in link_changed event due to a stage3_ip_config scheduled by
another component. In such cases, we proceed with IP configuration
without allocating the resources needed like initializing DHCP client.

In order to avoid that, if we learn during stage3_ip_config that the
link is now ready, we need to schedule another stage3_ip_config to
allocate the resources we might need.

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/2004

Fixes: 83bf7a8cdb ('ovs: wait for the link to be ready before activating')
(cherry picked from commit 40d51b9104)
(cherry picked from commit 63dfd3b60b)
2024-07-31 11:09:31 +02:00
Fernando Fernandez Mancera
6328a1a0d1 ovs: wait for the link to be ready before activating
When activating an ovs-interface we already wait for the cloned MAC
address to be set, ifindex is present and platform link also present but
in some cases this is not enough.

If an udev rule is in place it might modify the interface when it is in
a later stage of the activation causing some race conditions or
problems. In order to solve that, we must wait until the link is fully
initialized.

(cherry picked from commit 83bf7a8cdb)
(cherry picked from commit 00e178351b)
2024-07-25 10:43:09 +02:00
Gris Ge
79c81fc06a activation: Allow changing controller of exposed active connection
When activating a port with its controller deactivating by new
activation, NM will register `state-change` signal waiting controller to
have new active connections. Once controller got new active connection,
the port will invoke `nm_active_connection_set_controller()` which lead
to assert error on
    g_return_if_fail(!nm_dbus_object_is_exported(NM_DBUS_OBJECT(self)))

because this active connection is already exposed as DBUS object.

To fix the problem, we remove the restriction on controller been
write-only and notify DBUS object changes for controller property.

Signed-off-by: Gris Ge <fge@redhat.com>
(cherry picked from commit 83a2595970)
(cherry picked from commit 3f3d1a4f54)
2024-07-19 13:16:47 +02:00
Beniamino Galvani
c11e1738ab crypto: regenerate PKCS#12 certificates
The PKCS#12 certificates are using SHA1 and RC2-40, which are deprecated;
regenerate them with:

 $ openssl pkcs12 -export -in test_key_and_cert.pem -inkey test_key_and_cert.pem -certfile test_ca_cert.pem -name "test-pkcs12" -out test-cert.p12
 (key: test)

 $ openssl pkcs12 -export -in test2_key_and_cert.pem -inkey test2_key_and_cert.pem -certfile test2_ca_cert.pem -name "test2-pkcs12" -out test2-cert.p12
 (key: 12345testing)

Before:
 $ openssl pkcs12 -info -in test-cert.p12
 MAC: sha1, Iteration 2048
 MAC length: 20, salt length: 8
 PKCS7 Encrypted data: pbeWithSHA1And40BitRC2-CBC, Iteration 2048

After:
 $ openssl pkcs12 -info -in test-cert.p12
 MAC: sha256, Iteration 2048
 MAC length: 32, salt length: 8
 PKCS7 Encrypted data: PBES2, PBKDF2, AES-256-CBC, Iteration 2048, PRF hmacWithSHA256
(cherry picked from commit 0d657af256)
2024-06-28 13:20:11 +02:00
Íñigo Huguet
91efd19bfc code format: run code format script 2024-06-28 11:14:22 +00:00
Beniamino Galvani
ecb26f5ad9 ovs: allow specifying a system OVS interface by MAC address
The OVS interface can be matched via MAC address; in that case, the
"connection.interface-name" property of the connection is empty.

When populating the ovsdb, we need to pass the actual interface name
from the device, not the one from the connection.

Fixes: 830a5a14cb ('device: add support for OpenVSwitch devices')

https://issues.redhat.com/browse/RHEL-34617
(cherry picked from commit be28a11735)
(cherry picked from commit 129f9e5be6)
2024-06-21 20:42:20 +02:00
Jan Vaclav
fff23d22ba gen-metadata-nm-settings-nmcli: add missing va_end in _prop_log() after using vaargs
It was missing before.

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1970

Fixes: af5c1c8019 ('man nm-settings-nmcli: autogenerate the "Format" field')
(cherry picked from commit 7d0115ea98)
(cherry picked from commit 7f8bbd6fcb)
2024-06-21 20:40:33 +02:00
Thomas Haller
697ee0388b config: fix crash in assertion during NetworkManager --print-config
Fixes: f6345180b1 ('core/config: fix duplicate entires in `NetworkManager --print-config` output')
(cherry picked from commit 5472f28a40)
(cherry picked from commit 41a5cb5308)
2024-06-21 20:40:32 +02:00
Jan Vaclav
581e0b0f2a libnm/wired: fix mac comparison in _remove_mac_blacklist_item_by_value
The comparison checking for MAC address equality had previously been flipped around.

Fixes: b084ad7f2b ('libnm-core: canonicalize hardware addresses in settings')
(cherry picked from commit a9c4c1d84e)
2024-06-21 20:40:32 +02:00
Beniamino Galvani
e17e207b3c libnm-core: fix memory leak in nm_setting_to_string()
Fixes: f957ea2b34 ('core/setting: rework nm_connection_dump()')

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1934
(cherry picked from commit ee5f4402bf)
(cherry picked from commit a9d7554a02)
2024-06-19 14:43:31 +02:00
Íñigo Huguet
a8a59e3e0a libnmc: don't strip prefix tags from secret names
The daemon is now capable of understanding and removing these prefix
tags by itself. It is better than this is not a responsibility of the
secret agent because it requires changes in all secret agents to work
properly (see https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/1536).

If the secret agent knows what these prefix tags are, it can remove them
only in the text that is displayed in the UI, but maintaining the
original string as the secret name that is returned to the daemon.

Secret agents that doesn't know what these prefix tags are won't do
anything with them, and they will also return the same string as secret
name, as expected. The only drawback is that they might display the full
string to the user, which is not a nice UX but it will at least work.

Also, allow to translate the secret name for the UI in libnmc.

(cherry picked from commit 18240bb72d)
(cherry picked from commit e217ec040d)
2024-06-18 16:06:13 +02:00
Íñigo Huguet
bdbcda1e22 vpn: handle hint tags in the daemon
Commit 345bd1b187 ('libnmc: fix secrets request on 2nd stage of 2FA
authentication') and commit 27c701ebfb ('libnmc: allow user input in
ECHO mode for 2FA challenges') introduced 2 new tags that hints for the
secret agents can have as prefix.

These tags were processed (and removed) in the secret agents, not in the
daemon. This is wrong because a system with an updated VPN plugin but a
not yet updated secret agent (like nm-plasma) will fail: it won't remove
the prefix and the daemon will save the secret with the prefix, i.e.
"x-dynamic-challenge:challenge-response" instead of just
"challenge-response". Then, VPN plugins doesn't recognize it, failing the
profile's activation. This is, in fact, an API break.

Also, if the VPN connection already existed before updating NM and the
VPN plugin, the secret flags are not added to the profile (they are only
added when the profile is created or modified). This causes the user's
first time response is saved to the profile, so the activation fails the
second and next times.

See:
- https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/1536
- https://gitlab.gnome.org/GNOME/NetworkManager-openvpn/-/issues/142

Anyway, in a good design the daemon should contain almost all the logic
and the clients should keep as simple as possible. Fix above's problems
by letting the daemon to receive the secret names with the prefix
already included. The daemon will strip it and will know what it means.

Note that this is done only in the functions that saves the secrets from
the data received via D-Bus. For example, nm_setting_vpn_add_secret
doesn't need to do it because this value shouldn't come from VPN
plugin's hints.

(cherry picked from commit 0583e1f843)
(cherry picked from commit 574741783c)
2024-06-18 16:06:13 +02:00
Beniamino Galvani
83847cc621 vpn: allow IP configurations with routes and without addresses
Usually, when the method is "auto" we want to avoid configuring routes
until the automatic method completes. To achieve that, we clear the
"allow_routes_without_address" flag of l3cds when the method is "auto".

For VPNs, IP configurations with only routes are perfectly valid,
therefore set the flag.

(cherry picked from commit d1ffdb28eb)
(cherry picked from commit 5b4ed809cc)
2024-06-17 15:37:08 +02:00
Beniamino Galvani
e008ec7345 core: add nm_l3_config_data_set_allow_routes_without_address()
Add a function to set the allow-routes-without-address flag for
l3cds. It will be used in the next commit.

(cherry picked from commit a3ce13c947)
(cherry picked from commit 5fa063f90d)
2024-06-17 15:37:08 +02:00
Beniamino Galvani
ea731bba9b core: rename l3cd's "dhcp_enabled" to "allow_routes_without_address"
The name "dhcp_enabled" is misleading because the flag is set for
method=auto, which doesn't necessarily imply DHCP. Also, it doesn't
convey what the flag is used for. Rename it to
"allow_routes_without_address".

(cherry picked from commit b31febea22)
(cherry picked from commit 6897b6ecfd)
2024-06-17 15:37:08 +02:00
Beniamino Galvani
476a9553f6 vpn: allow IP configurations without addresses
An IPv4-over-IPv6 (or vice-versa) IPsec VPN can return IP
configurations with routes and without addresses. For example, in this
scenario:

         +---------------+         +---------------+
         |  fd01::10/64  <-- VPN -->  fd02::20/64  |
         |     host1     |         |     host2     |
         +-------^-------+         +-------^-------+
                 |                         |
         +-------v-------+         +-------v-------+
         |    subnet1    |         |    subnet2    |
         | 172.16.1.0/24 |         | 172.16.2.0/24 |
         +---------------+         +---------------+

host1 and host2 establish a IPv6 tunnel which encapsulates packets
between the two IPv4 subnets. Therefore, in routed mode, host1 will
need to configure a route like "172.16.2.0/24 via ipsec1" even if the
host doesn't have any IPv4 address on the VPN interface.

Accept IP configurations without address from the VPN; only check that
the address and prefix are sane if they are provided.

(cherry picked from commit 97f185e1f8)
(cherry picked from commit 518b7c5bd5)
2024-06-17 15:37:08 +02:00
Gris Ge
91f63030b1 checkpoint: fix port reactivation when controller is deactivating
Problem:

    Given a OVS port with `autoconnect-ports` set to default or false,
    when reactivation required for checkpoint rollback,
    previous activated OVS interface will be in deactivate state after
    checkpoint rollback.

The root cause:

    The `activate_stage1_device_prepare()` will mark the device as
    failed when controller is deactivating or deactivated.
    In `activate_stage1_device_prepare()`, the controller device is
    retrieved from NMActiveConnection, it will be NULL when NMActiveConnection
    is in deactivated state. This will cause device been set to
    `NM_DEVICE_STATE_REASON_DEPENDENCY_FAILED` which prevent all follow
    up `autoconnect` actions.

Fix:
    When noticing controller is deactivating or deactivated with reason
    `NM_DEVICE_STATE_REASON_NEW_ACTIVATION`, use new function
    `nm_active_connection_set_controller_dev()` to wait on controller
    device state between NM_DEVICE_STATE_PREPARE and
    NM_DEVICE_STATE_ACTIVATED. After that, use existing
    `nm_active_connection_set_controller()` to use new
    NMActiveConnection of controller to move on.

Resolves: https://issues.redhat.com/browse/RHEL-31972

Signed-off-by: Gris Ge <fge@redhat.com>
(cherry picked from commit a68d2fd780)
(cherry picked from commit 4726822fb0)
2024-06-01 17:18:58 +02:00
Thomas Haller
3f3f446f6e core: workaround "-Wnonnull-compare" warning in nm_lldp_neighbor_tlv_get_oui()
../src/libnm-lldp/nm-lldp-neighbor.c: In function ‘nm_lldp_neighbor_tlv_get_oui’:
  ../src/libnm-std-aux/nm-std-aux.h:191:12: error: ‘nonnull’ argument ‘oui’ compared to NULL [-Werror=nonnull-compare]
    191 |         if (expr)                      \
        |            ^
  ../src/libnm-std-aux/nm-std-aux.h:202:27: note: in expansion of macro ‘_NM_BOOLEAN_EXPR_IMPL’
    202 |                           _NM_BOOLEAN_EXPR_IMPL(NM_UNIQ, expr))
        |                           ^~~~~~~~~~~~~~~~~~~~~
  ../src/libnm-glib-aux/nm-macros-internal.h:1693:31: note: in expansion of macro ‘NM_BOOLEAN_EXPR’
   1693 | #define _G_BOOLEAN_EXPR(expr) NM_BOOLEAN_EXPR(expr)
        |                               ^~~~~~~~~~~~~~~
  /usr/include/glib-2.0/glib/gmacros.h:1244:43: note: in expansion of macro ‘_G_BOOLEAN_EXPR’
   1244 | #define G_LIKELY(expr) (__builtin_expect (_G_BOOLEAN_EXPR(expr), 1))
        |                                           ^~~~~~~~~~~~~~~
  /usr/include/glib-2.0/glib/gmessages.h:661:9: note: in expansion of macro ‘G_LIKELY’
    661 |     if (G_LIKELY (expr)) \
        |         ^~~~~~~~
  ../src/libnm-lldp/nm-lldp-neighbor.c:651:5: note: in expansion of macro ‘g_return_val_if_fail’
    651 |     g_return_val_if_fail(oui, -EINVAL);
        |     ^~~~~~~~~~~~~~~~~~~~

(cherry picked from commit a500538fb2)
2024-06-01 17:18:58 +02:00
Beniamino Galvani
ebf25794d9 checkpoint: preserve in-memory state of connections
If a connection is in-memory (i.e. has flag "unsaved"), after a
checkpoint and rollback it can be wrongly persisted to disk:

 - if the connection was modified and written to disk after the
   rollback, during the rollback we update it again with persist mode
   "keep", which keeps it on disk;

 - if the connection was deleted after the rollback, during the
   rollback we add it again with persist mode "to-disk".

Instead, remember whether the connection had the "unsaved" flag set
and try to restore the previous state.

However, this is not straightforward as there are 4 different possible
states for the settings connection: persistent; in-memory only;
in-memory shadowing a persistent file; in-memory shadowing a detached
persistent file (i.e. the deletion of the connection doesn't delete
the persistent file). Handle all those cases.

Fixes: 3e09aed2a0 ('checkpoint: add create, rollback and destroy D-Bus API')
(cherry picked from commit c979bfeb8b)
2024-05-23 13:46:24 +02:00
Beniamino Galvani
e5837aa1d3 settings: add nm_settings_connection_persist_mode_to_string()
(cherry picked from commit a48b7fe7b9)
2024-05-23 13:44:11 +02:00
Beniamino Galvani
9f992dd85e dcb: fix test compilation
GCC 14 with LTO generates the following warning:

  src/core/tests/test-dcb.c: In function 'test_dcb_cleanup':
  src/core/tests/test-dcb.c:283:5: error: array subscript _3 is outside array bounds of 'const char *[0:]' [-Werror=array-bounds=]
    283 |     g_assert_cmpstr(expected.cmds[expected.num], ==, NULL);
        |     ^
  src/core/tests/test-dcb.c:14:17: note: while referencing 'cmds'
     14 |     const char *cmds[];
        |                 ^
  src/core/tests/test-dcb.c:261:24: note: defined here 'expected'
    261 |     static DcbExpected expected = {
        |                        ^

Define the commands as a fixed array instead of flexible array member.

(cherry picked from commit 9ff7ff28fc)
2024-05-15 12:11:57 +02:00
Beniamino Galvani
efa1d0e173 libnm-core: avoid compiler warnings in team settings
GCC 14 with LTO complains with:

  In function 'nm_team_link_watcher_new_ethtool',
      inlined from 'nm_team_link_watcher_new_ethtool' at src/libnm-core-impl/nm-setting-team.c:106:1:
  src/libnm-core-impl/nm-setting-team.c:130:33: error: array subscript 'struct NMTeamLinkWatcher[0]' is partly outside array bounds of 'unsigned char[16]' [-Werror=array-bounds=]
    130 |     watcher->ref_count          = 1;
        |                                 ^
  src/libnm-core-impl/nm-setting-team.c:128:15: note: object of size 16 allocated by 'g_malloc'
    128 |     watcher = g_malloc(nm_offsetofend(NMTeamLinkWatcher, ethtool));
        |               ^

even if the warning is disabled via pragma directives in that
code. This looks like the following GCC bug:

  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80922

saying

  We do not track warning options (and thus optimize pragmas /
  attributes) across LTO because they are not saved in the function
  specific optimization flag section.

We use a (NMTeamLinkWatcher *) to point to a memory area that is
shorter than the struct, because depending on the watcher type we need
to store different parameters; in this way we can save few bytes of
memory for some watcher types. However, this often breaks when
upgrading the compiler; instead just allocate the full struct.

(cherry picked from commit d369f55192)
2024-05-15 12:11:52 +02:00
Beniamino Galvani
78d5ba4d4f libnm-glib-aux: fix comments about UUID generation
Whether the length is supplied explicitly or implicitly (via -1), the
result is the same. Update the comment.

(cherry picked from commit fcea2f174e)
2024-05-15 12:11:42 +02:00
Beniamino Galvani
cbf20a2317 libnm-glib-aux: fix "maybe-uninitialized" error when generating UUID
GCC 14 complans with:

  src/libnm-glib-aux/nm-uuid.c: In function 'nm_uuid_generate_from_strings_strv':
  src/libnm-glib-aux/nm-uuid.c:492:12: error: '_1' may be used uninitialized [-Werror=maybe-uninitialized]
    492 |     return nm_uuid_generate_from_string_str(s, slen, uuid_type, type_args);
        |            ^
  src/libnm-glib-aux/nm-uuid.c:392:1: note: by argument 1 of type 'const char *' to 'nm_uuid_generate_from_string_str' declared here
    392 | nm_uuid_generate_from_string_str(const char   *s,
        | ^

"-Wmaybe-uninitialized" diagnoses passing pointers or references to
uninitialized memory to functions taking const-qualified arguments.

In this case, nm_uuid_generate_from_string_str()'s first argument is a
"const char *" and so the compiler expects that the string is always
initialized. However, it is not initialized when len is zero.

A non-null zero-length array can be specified in two ways: by setting
len to zero, or by setting len to -1 and having NULL as first
element. Handle both cases in the same way.

(cherry picked from commit 2386c0f52d)
2024-05-15 12:11:35 +02:00
Íñigo Huguet
15ffa8ec6f platform: avoid routes resync for routes that we don't track
When we recibe a Netlink message with a "route change" event, normally
we just ignore it if it's a route that we don't track (i.e. because of
the route protocol).

However, it's not that easy if it has the NLM_F_REPLACE flag because
that means that it might be replacing another route. If the kernel has
similar routes which are candidates for the replacement, it's hard for
NM to guess which one of those is being replaced (as the kernel doesn't
have a "route ID" or similar field to indicate it). Moreover, the kernel
might choose to replace a route that we don't have on cache, so we know
nothing about it.

It is important to note that we cannot just discard Netlink messages of
routes that we don't track if they has the NLM_F_REPLACE. For example,
if we are tracking a route with proto=static, we might receive a replace
message, changing that route to proto=other_proto_that_we_dont_track. We
need to process that message and remove the route from our cache.

As NM doesn't know what route is being replaced, trying to guess will
lead to errors that will leave the cache in an inconsistent state.
Because of that, it just do a cache resync for the routes.

For IPv4 there was an optimization to this: if we don't have in the
cache any route candidate for the replacement there are only 2 possible
options: either add the new route to the cache or discard it if we are
not interested on it. We don't need a resync for that.

This commit is extending that optimization to IPv6 routes. There is no
reason why it shouldn't work in the same way than with IPv4. This
optimization will only work well as long as we find potential candidate
routes in the same way than the kernel (comparing the same fields). NM
calls to this "comparing by WEAK_ID". But this can also happen with IPv4
routes.

It is worth it to enable this optimization because there are routing
daemons using custom routing protocols that makes tens or hundreds of
updates per second. If they use NLM_F_REPLACE, this caused NM to do a
resync hundreds of times per second leading to a 100% CPU usage:
https://issues.redhat.com/browse/RHEL-26195

An additional but smaller optimization is done in this commit: if we
receive a route message for routes that we don't track AND doesn't have
the NLM_F_REPLACE flag, we can ignore the entire message, thus avoiding
the memory allocation of the nmp_object. That nmp_object was going to be
ignored later, anyway, so better to avoid these allocations that, with
the routing daemon of the above's example, can happen hundreds of times
per second.

With this changes, the CPU usage doing `ip route replace` 300 times/s
drops from 100% to 1%. Doing `ip route replace` as fast as possible,
without any rate limitting, still keeps NM with a 3% CPU usage in the
system that I have used to test.

(cherry picked from commit 4d426f581d)
2024-05-13 13:33:06 +02:00
Wen Liang
86fbfb3806 device: use subnet when the applied connection has IPv6 method shared
We should use the IPv6 subnet when we have an applied connection
stored on the downlink device, and the IPv6 method should be "shared"
for that applied connection. It does not make sense to register l3cd
and set router config for ndisc instance when the downlink device is
already deactivated.

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1880
Resolves: https://issues.redhat.com/browse/RHEL-17350

Signed-off-by: Wen Liang <liangwen12year@gmail.com>
(cherry picked from commit 15901a7489)
2024-05-02 14:35:22 +02:00
Íñigo Huguet
2636797b0b man: fix missing deprecation message
In the gtkdoc comments, the text below tags like `Since: 1.2` is
discarded. In the property `autoconnect-slaves` a line indicating its
deprecation was below one of these tags. As a result, it was missing in
the man page. Fix it.

Fixes: 194455660d ('connection: deprecate NMSettingConnection autoconnect-slaves property')
(cherry picked from commit 7427e9d320)
2024-04-18 15:31:54 +02:00
Fernando Fernandez Mancera
2fac176986 libnm-lldp: use ETH_P_ALL instead of NM_ETHERTYPE_LLDP for the socket
When creating the socket for listening to LLDP frames we are setting
NM_ETHERTYPE_LLDP (0x88cc) as protocol. In most of the cases, that is
correct but when the interface is attached as a port to a OVS bridge,
kernel is not matching the protocol correctly. The reason might be that
some metadata is added to the packet, but we are not completely sure
about it.

Instead, we should use ETH_P_ALL to match all the protocols. Later, we
have a eBPF filter to drop the packet by multicast MAC address or
protocol. This is how lldpd is doing it for example.

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1903
(cherry picked from commit 9ac1d6e22b)
2024-04-05 11:52:18 +02:00