Oddly enough, valgrind was not complaining about this leak...
Fixes: 87b2d783b6 ('core: accept 'ssids':aay option in RequestScan() dictionary parameter')
(cherry picked from commit 5ed1edc02a)
(cherry picked from commit 568c19f07d)
Many device types take the MTU value from the wired setting; usually
they don't implement the can_reapply_change() method and so the MTU
can't be changed with the Reapply() API.
Instead of implementing the method for all such devices to support the
same property (adding a lot of duplicated code), add a check in
NMDevice to allow the reapply of MTU when we recognize that the device
uses the MTU from the wired setting.
Device types can still decide to implement can_reapply_change() and
support whatever properties they want, even from the wired setting.
(cherry picked from commit 9339d3310e)
(cherry picked from commit 1191eba15a)
If the activation of an assumed device fails, we first set the device
state to FAILED and then to ACTIVATED. In the FAILED state, the active
connection transitions to DEACTIVATED and clears its device pointer;
hence we end up with an inconsistent state which causes assertion
failures in other parts of the code (for example, get_best_ip_config()
assumes that the device of the best active connection is not NULL).
Don't first transition to FAILED and then to ACTIVATED, just set the
latter.
https://bugzilla.redhat.com/show_bug.cgi?id=1737774https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/351
(cherry picked from commit 93e9010b75)
(cherry picked from commit 366b90db87)
After we set link parameters (auto-negotiation, speed, duplex) in
stage1, the carrier can go down for several seconds because the
Ethernet PHY needs to renegotiate the link. Wait that carrier goes up
before starting the supplicant or the EAPoL start packet can be lost
causing an authentication failure.
https://bugzilla.redhat.com/show_bug.cgi?id=1759797
(cherry picked from commit 838e5b87c2)
With accept_ra set to 1, kernel sends its own router solicitation
messages and parses the advertisements. This duplicates what NM
already does in userspace and has unwanted consequences like [1] and
[2].
The only reason why accept_ra was re-enabled in the past was to apply
RA parameters like ReachableTime and RetransTimer [3]; but now NM
supports them and so accept_ra can be turned off again.
Also, note that previously the option was set in
addrconf6_start_with_link_ready(), and so this was done only when the
method was 'auto'. Instead, now we clear it for all methods except
'ignore'.
[1] https://mail.gnome.org/archives/networkmanager-list/2019-June/msg00027.html
[2] https://bugzilla.redhat.com/show_bug.cgi?id=1734470
[3] https://bugzilla.redhat.com/show_bug.cgi?id=1068673
(cherry picked from commit 5a534529e2)
IPv6 router advertisement messages contain the following parameters
(RFC 4861):
- Reachable time: 32-bit unsigned integer. The time, in
milliseconds, that a node assumes a neighbor is reachable after
having received a reachability confirmation. Used by the Neighbor
Unreachability Detection algorithm. A value of zero means
unspecified (by this router).
- Retrans Timer: 32-bit unsigned integer. The time, in milliseconds,
between retransmitted Neighbor Solicitation messages. Used by
address resolution and the Neighbor Unreachability Detection
algorithm. A value of zero means unspecified (by this router).
Currently NM ignores them; however, since it leaves accept_ra=1, the
kernel parses RAs and applies those parameters for us [1].
In the next commit kernel handling of RAs will be disabled, so let NM
set those neighbor-related parameters.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/ipv6/ndisc.c?h=v5.2#n1353
(cherry picked from commit 5f0c6f8d3b)
We try to set only one time the MTU from the connection to not
interfere with manual user changes.
If at some point the parent interface changes temporarily MTU to a
lower value (for example, because the connection was reactivated), the
kernel will also lower the MTU on child interface and we will not
update it ever again.
Add a workaround to this. If we detect that the MTU we want to set
from connection is higher that the allowed one, go into a state where
we follow the parent MTU until it is possible to set again the desired
MTU. This is a bit ugly, but I can't think of any nicer way to do it.
https://bugzilla.redhat.com/show_bug.cgi?id=1751079
(cherry picked from commit ec28f5b343)
A MACsec connection doesn't have an ordering dependency with its
parent connection and so it's possible that the parent gets activated
later and sets a greater MTU than the original one.
It is reasonable and useful to keep the MACsec MTU configured by
default as the maximum allowed by the parent interface, that is the
parent MTU minus the encapsulation overhead (32). The user can of
course override this by setting an explicit value in the
connection. We already do something similar for VLANs.
https://bugzilla.redhat.com/show_bug.cgi?id=1723690
(cherry picked from commit 438a0a9ad5)
Introduce a generic function to set a MTU based on parent's one. Also
define a device-specific @mtu_parent_delta value that specifies the
difference from parent MTU that should be set by default. For VLAN it
is zero but other interface types (for example MACsec) require a
positive value due to encapsulation overhead.
(cherry picked from commit 5cf57f4522)
In nm_acd_manager_announce_addresses() we should not only start the
probes but also add the acd file descriptor to the main loop.
Otherwise, a timer is armed to send the announcements but it never
fires and no announcements are sent.
Fixes: d9a4b59c18 ('acd: adapt NM code and build options')
https://bugzilla.redhat.com/show_bug.cgi?id=1767681
(cherry picked from commit 14992ab9cd)
If the 802.1X authentication fails and 802-1x.optional is set,
continue with activation. In this case, subscribe to the auth-state
supplicant property so that any dynamic IP method can be restarted
when the authentication succeeds. This is because upon authentication
the switch could have changed the VLAN we are connected to.
(cherry picked from commit 8afce75bf3)
The upstream apparently thought it's a great idea to change the agent
manager path. This fixes things for those unfortunate enough to run
IWD.
(cherry picked from commit 186d22a963)
The Bluetooth DUN device's NMModem would signal the reset of ifindex to zero
when it's disconnected and the NMDeviceBt would accordingly update the
bluetooth device's ip ifindex. This is not okay since commit ab4578302d
('device: refactor nm_device_set_ip_ifindex() and set_ip_iface()') which,
although claiming to be a refactoring, made such use of
nm_device_set_ip_ifindex() illegal. Resetting the ifindex is anyway not
necessary, since it's taken care of _cleanup_generic_post().
Let's leave the ifindex alone once the device is activated, in a manner
analogous to what NMDeviceModem.
Fixes: ab4578302d ('device: refactor nm_device_set_ip_ifindex() and set_ip_iface()')
Fixes: 78ca2a70c7 ('device: don't set invalid ip-iface'):
(cherry picked from commit a5ca504b5b)
Only reapply the IP configuration on link up if the IP state is CONF
or DONE. Previously we also reapplied it when the device was
disconnected (IP state NONE) and this could lead to a situation where
an incomplete config was applied; then we intersected the desired
configuration with the external - incomplete - one, causing the
removal of part of desired configuration (for example the default
route).
Fixes: d0b16b9283 ('device: unconditionally reapply IP configuration on link up')
https://bugzilla.redhat.com/show_bug.cgi?id=1754511https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/291
(cherry picked from commit 64a9dd3804)
In the accept() callback, the nettools client creates a UDP socket
with the received address as source, so the address must be already
configured on the interface.
Also, handle errors returned by nm_dhcp_client_accept().
Fixes: 401fee7c20 ('dhcp: support notifying the client of the result of DAD')
(cherry picked from commit 8b5bf6e4d1)
There's multiple things wrong there, but unnoticed because the error handling
was entirely missing or nobody is using thie anymore.
The Mesh ID needs to be set while the device is down. Also, the channel
needs to be set last, because that's what triggers the connection
attempt. For that the device needs to be up.
Also, fix the error handling.
(cherry picked from commit 13bd678dd5)
The comment is wrong. Since 6eaded9071 ('device: add
get_autoconnect_allowed() virtual function'), get_autoconnect_allowed()
is called before the device state is consulted.
(cherry picked from commit ad86ee4d48)
The nm-owned flag indicates whether the device was created by NM. If
the realization step fails, the device was not created and so nm-owned
should not be updated.
(cherry picked from commit 4bc4156424)
Only happens with debug logging enabled. So, not a large problem.
Found by Coverity.
Fixes: d9a4b59c18 ('acd: adapt NM code and build options')
(cherry picked from commit 0300c1823a)
CID 59391 (#1 of 1): Copy into fixed size buffer (STRING_OVERFLOW)
31. fixed_size_dest: You might overrun the 16-character fixed-size string be.ifspec.spec.ifname by copying priv->nas_ifname without checking the length.
(cherry picked from commit 23fa1b3272)
Warned by coverity: we assert above that brfd is -1, so we must always
restore it to -1 in the error case.
Technically, not a problem because socket() is documented to return
only -1 on error already. Apparently coverity does not believe that.
(cherry picked from commit 5b9a848a82)
- use nm_g_variant_unref_floating()
- rename _lldp_attr_take_str_ptr() to _lldp_attr_set_str_take().
The new name has the same "_lldp_attr_set_" prefix as other setters.
Also, with the previous name it is unclear why it takes a "str-ptr".
- setting the same attribute multiple times, ignores all but the first
value. Avoid cloning the string in that case, and explicitly choose
the set or take function.
(cherry picked from commit 0fbb54839e)
Valgrind complains:
==26355== 32 bytes in 2 blocks are definitely lost in loss record 2,829 of 6,716
==26355== at 0x4838748: malloc (vg_replace_malloc.c:308)
==26355== by 0x483AD63: realloc (vg_replace_malloc.c:836)
==26355== by 0x4F6AD4F: g_realloc (in /usr/lib64/libglib-2.0.so.0.6000.6)
==26355== by 0x4F87B33: ??? (in /usr/lib64/libglib-2.0.so.0.6000.6)
==26355== by 0x4F87B96: g_string_sized_new (in /usr/lib64/libglib-2.0.so.0.6000.6)
==26355== by 0x2D66E1: nm_utils_buf_utf8safe_escape (nm-shared-utils.c:1911)
==26355== by 0x4113B0: lldp_neighbor_new (nm-lldp-listener.c:676)
==26355== by 0x412788: process_lldp_neighbor (nm-lldp-listener.c:882)
==26355== by 0x4135CF: lldp_event_handler (nm-lldp-listener.c:931)
==26355== by 0x422CDB: lldp_callback (sd-lldp.c:50)
==26355== by 0x4235F9: lldp_add_neighbor (sd-lldp.c:166)
==26355== by 0x423679: lldp_handle_datagram (sd-lldp.c:189)
==26355== by 0x423C8B: lldp_receive_datagram (sd-lldp.c:235)
==26355== by 0x2F887A: source_dispatch (sd-event.c:2832)
==26355== by 0x2FAD43: sd_event_dispatch (sd-event.c:3245)
==26355== by 0x2D9237: event_dispatch (nm-sd.c:51)
==26355== by 0x4F64EDC: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.6000.6)
==26355== by 0x4F6526F: ??? (in /usr/lib64/libglib-2.0.so.0.6000.6)
==26355== by 0x4F655A2: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.6000.6)
==26355== by 0x140932: main (main.c:465)
==26355==
(cherry picked from commit ece270ea5f)
Not all masters type have a platform link and so it's wrong to check
for it to decide whether the slave should be really released. Move the
check to master devices that need it (bond, bridge and team).
OVS ports don't need the check because they don't call to platform to
remove a slave.
https://bugzilla.redhat.com/show_bug.cgi?id=1733709
(cherry picked from commit 57e3734b6c)
We set nm-owned to indicate whether a software device was created by
NM or it was pre-existing. When checking the existence, we must verify
also whether the link type is compatible with the device, otherwise it
is possible to match unrelated interfaces. For example, when checking
for the existence of an ovs-bridge (which is not compatible with any
platform link) we could match a unrelated platform link with the same
name.
https://bugzilla.redhat.com/show_bug.cgi?id=1733709
(cherry picked from commit 3cb4b36261)
With "wireguard.ip4-auto-default-route" and "wireguard.ip6-auto-default-route",
NetworkManager automatically adds policy routing rules for the default
route.
For that, it needs to pick (up to) two numbers:
- the fwmark. This is used both for WireGuard's fwmark setting and
is also the number of the routing table where the default-route is
added.
- the rule priority. NetworkManager adds two policy routing rules, and
we need to place them somewhere before the default rules (at 32766).
Previously, we looked at exiting platform configuration and picked
numbers that were not yet used. However, during restart of
NetworkManager, we leave the interface up and after restart we will
take over the previous configuration. At that point, we need to choose
the same fwmark/priority, otherwise the configuration changes.
But since we picked numbers that were not yet used, we would always choose
different numbers. For routing rules that meant that after restart a second
pair of rules was added.
We possibly could store this data in the device's state-file. But that
is complex. Instead, just pick numbers deterministically based on the
connection's UUID.
If the picked numbers are not suitable, then the user can still work
around that:
- for fwmark, the user can explicitly configure wireguard.fwmark
setting.
- for the policy routes, the user can explicitly add the rules with
the desired priorirites (arguably, currently the default-route cannot
be added like a regular route, so the table cannot be set. Possibly
the user would have to add two /1 routes instead with
suppress_prefixlength=1).
(cherry picked from commit cfb497e499)
We call _auto_default_route_init() at various places, for example during
coerce_route_table(). We cannot be sure that we don't call it before
activation starts (before stage1) or after device-cleanup.
That means, upon activation, we need to clear the state first. Do that in
act_stage1_prepare().
(cherry picked from commit dc219662fa)
#3 0x00007fb0aa9e7d3d in g_return_if_fail_warning
(log_domain=log_domain@entry=0x562295fd5ee3 "libnm", pretty_function=pretty_function@entry=0x562295fd71d0 <__func__.35180> "_connection_get_setting_check", expression=expression@entry=0x562295f8edf7 "NM_IS_CONNECTION (connection)") at ../glib/gmessages.c:2767
#4 0x0000562295df151a in _connection_get_setting_check (connection=0x0, setting_type=0x562297b17050 [NMSettingWireGuard/NMSetting]) at libnm-core/nm-connection.c:207
#5 0x0000562295df151a in _connection_get_setting_check (connection=0x0, setting_type=0x562297b17050 [NMSettingWireGuard/NMSetting]) at libnm-core/nm-connection.c:205
#6 0x0000562295ef132a in _nm_connection_get_setting (type=<optimized out>, connection=0x0) at ./libnm-core/nm-core-internal.h:483
#7 0x0000562295ef132a in _auto_default_route_init (self=self@entry=0x562297bf82b0 [NMDeviceWireGuard]) at src/devices/nm-device-wireguard.c:443
#8 0x0000562295ef1b98 in coerce_route_table (device=0x562297bf82b0 [NMDeviceWireGuard], addr_family=2, route_table=0, is_user_config=<optimized out>)
at src/devices/nm-device-wireguard.c:565
#9 0x0000562295ea42ae in _get_route_table (self=self@entry=0x562297bf82b0 [NMDeviceWireGuard], addr_family=addr_family@entry=2) at src/devices/nm-device.c:2311
#10 0x0000562295ea4593 in nm_device_get_route_table (self=0x562297bf82b0 [NMDeviceWireGuard], addr_family=2) at src/devices/nm-device.c:2338
#11 0x0000562295eabde0 in ip_config_merge_and_apply (self=0x562297bf82b0 [NMDeviceWireGuard], addr_family=2, commit=1) at src/devices/nm-device.c:7590
#12 0x0000562295ed2f0b in device_link_changed (self=self@entry=0x562297bf82b0 [NMDeviceWireGuard]) at src/devices/nm-device.c:3939
#13 0x00007fb0aa9dc7db in g_idle_dispatch (source=source@entry=0x562297bf0b30, callback=0x562295ed2880 <device_link_changed>, user_data=0x562297bf82b0) at ../glib/gmain.c:5627
#14 0x00007fb0aa9dfedd in g_main_dispatch (context=0x562297a28090) at ../glib/gmain.c:3189
#15 0x00007fb0aa9dfedd in g_main_context_dispatch (context=context@entry=0x562297a28090) at ../glib/gmain.c:3854
#16 0x00007fb0aa9e0270 in g_main_context_iterate (context=0x562297a28090, block=block@entry=1, dispatch=dispatch@entry=1, self=<optimized out>) at ../glib/gmain.c:3927
#17 0x00007fb0aa9e05a3 in g_main_loop_run (loop=0x562297a0b380) at ../glib/gmain.c:4123
#18 0x0000562295d0b147 in main (argc=<optimized out>, argv=<optimized out>) at src/main.c:465
https://bugzilla.redhat.com/show_bug.cgi?id=1734383
(cherry picked from commit 47fc1a4293)
For WireGuard (like for all IP-tunnels and IP-based VPNs), the IP addresses of
the peers must be reached outside the tunnel/VPN itself.
For VPN connections, NetworkManager usually adds a direct /32 route to
the external VPN gateway to the underlying device. For WireGuard that is
not done, because injecting a route to another device is ugly and error
prone. Worse: WireGuard with automatic roaming and multiple peers makes this
more complicated.
This is commonly a problem when setting the default-route via the VPN,
but there are also other subtle setups where special care must be taken
to prevent such routing loops.
WireGuard's wg-quick provides a simple, automatic solution by adding two policy
routing rules and relying on the WireGuard packets having a fwmark set (see [1]).
Let's also do that. Add new properties "wireguard.ip4-auto-default-route"
and "wireguard.ip6-auto-default-route" to enable/disable this. Note that
the default value lets NetworkManager automatically choose whether to
enable it (depending on whether there are any peers that have a default
route). This means, common scenarios should now work well without additional
configuration.
Note that this is also a change in behavior and upon package upgrade
NetworkManager may start adding policy routes (if there are peers that
have a default-route). This is a change in behavior, as the user already
clearly had this setup working and configured some working solution
already.
The new automatism picks the rule priority automatically and adds the
default-route to the routing table that has the same number as the fwmark.
If any of this is unsuitable, then the user is free to disable this
automatism. Note that since 1.18.0 NetworkManager supports policy routing (*).
That means, what this automatism does can be also achieved via explicit
configuration of the profile, which gives the user more flexibility to
adjust all parameters explicitly).
(*) but only since 1.20.0 NetworkManager supports the "suppress_prefixlength"
rule attribute, which makes it impossible to configure exactly this rule-based
solution with 1.18.0 NetworkManager.
[1] https://www.wireguard.com/netns/#improved-rule-based-routing
