remove_device() is also called when the device has no longer a valid
ifindex and so device_is_wake_on_lan() must do an extra check to avoid
the following assertion:
nmp_cache_lookup_entry_link: assertion 'ifindex > 0' failed
0 _g_log_abort () from target:/lib64/libglib-2.0.so.0
1 g_logv () from target:/lib64/libglib-2.0.so.0
2 g_log () from target:/lib64/libglib-2.0.so.0
3 nmp_cache_lookup_entry_link (cache=0xb858f0, ifindex=ifindex@entry=0) at ../src/platform/nmp-object.c:1713
4 nmp_cache_lookup_link (cache=<optimized out>, ifindex=ifindex@entry=0) at ../src/platform/nmp-object.c:1728
5 nm_platform_link_get_obj (self=self@entry=0xb85840, ifindex=ifindex@entry=0, visible_only=visible_only@entry=1) at ../src/platform/nm-platform.c:759
6 nm_platform_link_get (self=self@entry=0xb85840, ifindex=ifindex@entry=0) at ../src/platform/nm-platform.c:784
7 nm_platform_link_get_type (self=self@entry=0xb85840, ifindex=ifindex@entry=0) at ../src/platform/nm-platform.c:1065
8 link_get_wake_on_lan (platform=0xb85840, ifindex=0) at ../src/platform/nm-linux-platform.c:6963
9 nm_platform_link_get_wake_on_lan (self=self@entry=0xb85840, ifindex=0) at ../src/platform/nm-platform.c:1705
10 device_is_wake_on_lan (platform=0xb85840, device=<optimized out>) at ../src/nm-manager.c:1617
11 remove_device (self=0xbd0060, device=<optimized out>, device@entry=0xd298c0, quitting=quitting@entry=0, allow_unmanage=allow_unmanage@entry=1)
12 device_removed_cb (device=0xd298c0, user_data=0xbd0060) at ../src/nm-manager.c:1698
13 _g_closure_invoke_va () from target:/lib64/libgobject-2.0.so.0
14 g_signal_emit_valist () from target:/lib64/libgobject-2.0.so.0
15 g_signal_emit () from target:/lib64/libgobject-2.0.so.0
16 available_connections_check_delete_unrealized_on_idle (user_data=0xd298c0) at ../src/devices/nm-device.c:4446
Fixes: ca3bbede74
(cherry picked from commit 92e57ab292)
When nm_device_disconnect_async() returns, the device could be still
in DEACTIVATING state, and so we also register to device-state signal
notifications to know when the device state goes to DISCONNECTED.
Sometimes it happens that the device state goes to DISCONNECTED before
nm_device_disconnect_async() returns. In this case the signal handler
exits the main loop and then the callback for disconnect_async() is
executed anyway because it was already dispatched, leading to an
invalid memory access.
To avoid this we should cancel nm_device_disconnect_async() when we
are quitting the main loop.
Reproducer:
nmcli connection add type team ifname t1 con-name t1
nmcli connection up t1
nmcli device disconnect t1 & nmcli device delete t1
Crash example:
==14955==ERROR: AddressSanitizer: SEGV on unknown address 0xffffffff0000000b (pc 0x7f128c8ba3dd bp 0x0000004be080 sp 0x7ffcda7dc6e0 T0)
==14955==The signal is caused by a READ memory access.
0 0x7f128c8ba3dc in g_string_truncate (/lib64/libglib-2.0.so.0+0x713dc)
1 0x7f128c8bb4bb in g_string_printf (/lib64/libglib-2.0.so.0+0x724bb)
2 0x45bdfa in disconnect_device_cb clients/cli/devices.c:2321
3 0x7f128ca3d1a9 in g_simple_async_result_complete /usr/src/debug/glib2-2.58.1-1.fc29.x86_64/gio/gsimpleasyncresult.c:802
4 0x7f128cf85d0e in device_disconnect_cb libnm/nm-device.c:2354
5 0x7f128ca4ff73 in g_task_return_now /usr/src/debug/glib2-2.58.1-1.fc29.x86_64/gio/gtask.c:1148
6 0x7f128ca508d5 in g_task_return /usr/src/debug/glib2-2.58.1-1.fc29.x86_64/gio/gtask.c:1206
7 0x7f128ca8ecfc in reply_cb /usr/src/debug/glib2-2.58.1-1.fc29.x86_64/gio/gdbusproxy.c:2586
8 0x7f128ca4ff73 in g_task_return_now /usr/src/debug/glib2-2.58.1-1.fc29.x86_64/gio/gtask.c:1148
9 0x7f128ca508d5 in g_task_return /usr/src/debug/glib2-2.58.1-1.fc29.x86_64/gio/gtask.c:1206
10 0x7f128ca83440 in g_dbus_connection_call_done /usr/src/debug/glib2-2.58.1-1.fc29.x86_64/gio/gdbusconnection.c:5713
11 0x7f128ca4ff73 in g_task_return_now /usr/src/debug/glib2-2.58.1-1.fc29.x86_64/gio/gtask.c:1148
12 0x7f128ca4ffac in complete_in_idle_cb /usr/src/debug/glib2-2.58.1-1.fc29.x86_64/gio/gtask.c:1162
13 0x7f128c893b7a in g_idle_dispatch gmain.c:5620
14 0x7f128c89726c in g_main_dispatch gmain.c:3182
15 0x7f128c897637 in g_main_context_iterate gmain.c:3920
16 0x7f128c897961 in g_main_loop_run (/lib64/libglib-2.0.so.0+0x4e961)
17 0x473afb in main clients/cli/nmcli.c:1067
18 0x7f128c6a1412 in __libc_start_main (/lib64/libc.so.6+0x24412)
19 0x416c39 in _start (/usr/bin/nmcli+0x416c39)
https://github.com/NetworkManager/NetworkManager/pull/254https://bugzilla.redhat.com/show_bug.cgi?id=1546061
(cherry picked from commit cf1126f60b)
There's no reason the mesh shouldn't autoconnect. Almost.
The mesh and regular Wi-Fi shares the same radio. There, in the first
place, probably shouldn't have been separate NMDevices. Not sure whether
we can fix it at this point, but we can surely avoid unnecessary
competition between the two devices: give the regular Wi-Fi priority and
only connect mesh if the regular companion stays disconnected.
For the record; connections shipped on XO-1 laptops all have
autoconnect=off and thus are not affected by this.
(cherry picked from commit 3a999475ef)
A lot of drivers actually support changing the MAC address of a link
without taking it down.
Taking down a link is very bad, because kernel will remove routes
and IPv6 addresses.
For example, if the user used a dispatcher script to add routes,
these will be lost. Note that we may change the MAC address of a
device any time. For example, a VLAN device watches the parent's
MAC address and configures it (with a logging message "parent hardware
address changed to ...").
Try first whether we can change the MAC address without taking the
link down. Only if that fails, retry with taking it down first.
https://bugzilla.redhat.com/show_bug.cgi?id=1639274
(cherry picked from commit e206a34732)
Add helper nm_platform_link_get_ifi_flags() to access the
ifi-flags.
This replaces the internal API _link_get_flags() and makes it public.
However, the return value also allows to distinguish between errors
and valid flags.
Also, consider non-visible links. These are links that are in netlink,
but not visible in udev. The ifi-flags are inherrently netlink specific,
so it seems wrong to pretend that the link doesn't exist.
(cherry picked from commit b445b1f8fe)
Working on NetworkManager 1.12.4 and sometimes (rarely), when creating
a NM client object before NetworkManager service start, this object will
never be running.
In that case, we can see the following log:
"[GLIB-GLib-GIO WARN] Error calling GetManagedObjects() when name
owner :1.5 for name org.freedesktop.NetworkManager came back:
GDBus.Error:org.freedesktop.DBus.Error.UnknownMethod:
No such interface 'org.freedesktop.DBus.ObjectManager' on object
at path /org/freedesktop".
Object Manager object shall be registered before requesting dbus name
to be sure that 'org.freedesktop.Dbus.ObjectManager' interface is present
when name owner change is received by libnm.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/51
(cherry picked from commit dc0cdbb57e)
Sometimes the test fail:
$ make -j 10 src/platform/tests/test-address-linux
$ while true; do
NMTST_DEBUG=d ./tools/run-nm-test.sh src/platform/tests/test-address-linux 2>&1 > log.txt || break;
done
fails with:
ERROR: src/platform/tests/test-address-linux - Bail out! test:ERROR:src/platform/tests/test-common.c:790:nmtstp_ip_address_assert_lifetime: assertion failed (adr <= lft): (1001 <= 1000)
That is, because of a wrong check. Fix it.
(cherry picked from commit e180464bcc)
In editor_menu_main(), after saving a connection we wait that the
Update2() D-Bus call returns and then we copy the NMRemoteConnection
instance over to @connection.
This assumes that when Update2() returns the remote connection
instance is already updated with new settings. Indeed, on server side
the NMSettingsConnection first emits the "Updated" signal and then
returns to Update2(). However, the Updated signal doesn't include the
new setting values and so libnm has to fire an asynchronous
nmdbus_settings_connection_call_get_setting() to fetch the new
settings, which terminates after the Update2().
So, to be sure that the remote connection got updated we have also to
listen to the connection-changed signal, which is always emitted after
an update.
https://bugzilla.redhat.com/show_bug.cgi?id=1546805
(cherry picked from commit a370faeb59)
Connection secrets are lost after calling
nm_connection_replace_settings_from_connection() because @con_tmp
doesn't contain secrets; refetch them like we do when starting the
editor.
(cherry picked from commit 096eef61d4)
CAK is a connection secret and can be NULL for various reasons
(agent-owned, no permissions to get secrets, etc.). verify() must not
require it.
Fixes: 474a0dbfbe
(cherry picked from commit 8d5b01619b)
On RHEL, openvswitch package is not in the base set of packages. Hence,
we cannot depend NetworkManager-ovs package on openvswitch.
This isn't really a problem, because NetworkManager's OVS plugin must
anyway behave graceful when openvswich service is not running or not
available. It only means, that a user who wants to use the OVS plugin
needs to explicitly install the openvswitch package.
https://bugzilla.redhat.com/show_bug.cgi?id=1629178https://bugzilla.redhat.com/show_bug.cgi?id=1633190
(cherry picked from commit 669bd33022)
In certain cases the timeouts may not have been unref'ed before they
need to be re-added. Add the appropriate unref calls to ensure we don't
register the timeout multiple times.
This fixes possible cases where timeouts are triggered multiple times
and even on destroyed DHCPv6 clients.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/73
(cherry picked from commit e179202e47)
In the past, the headers "linux/if.h" and "net/if.h" were incompatible.
That means, we can either include one or the other, but not both.
This is fixed in the meantime, however the issue still exists when
building against older kernel/glibc.
That means, including one of these headers from a header file
is problematic. In particular if it's a header like "nm-platform.h",
which itself is dragged in by many other headers.
Avoid that by not including these headers from "platform.h", but instead
from the source files where needed (or possibly from less popular header
files).
Currently there is no problem. However, this allows an unknowing user to
include <net/if.h> at the same time with "nm-platform.h", which is easy
to get wrong.
(cherry picked from commit 37e47fbdab)
The hostname property is only initialized once, early on during
start. Move the initialization even earlier during object constructions.
This effectively makes the hostname an immutable property.
This also makes sense, because the hostname is used by IPv4 and
IPv6 DHCP instances alike.
(cherry picked from commit 787f4b57cd)
Add a new mode for the DHCPv4 client identifier.
"duid" is what the internal (systemd) DHCP client already does by
default. It is also the same as used by systemd-networkd's
"ClientIdentifier=duid" setting. What we still lack (compared to
networkd) are a way to overwrite IAID and the DUID.
Previously, this mode was used by the internal DHCP plugin
by default. However, it could not be explicitly configured.
In general, our default values should also be explicitly selectable.
Now the "duid" client identifier can also be used with the "dhclient"
plugin.
(cherry picked from commit 8861ac2976)
Note how client_start() in NMDhcpManager already asserts
that we have a MAC address. It's always present, like
for the IPv6 case.
(cherry picked from commit dfdd4e3bd3)
We already had "${DEVICE}" which uses the interface name.
In times of predictable interface naming, that works well.
It allows the user to generate IDs per device which don't
change when the hardware is replaced.
"${MAC}" is similar, except that is uses the permanent MAC
address of the device. The substitution results in the empty
word, if the device has no permanent MAC address (like software
devices).
The per-device substitutions "${DEVICE}" and "${MAC}" are especially
interesting with "connection.multi-connect=multiple".
(cherry picked from commit 7ffbf71276)
- use nm_auto to return early when something goes wrong
- don't modify NMDhcpClient's state until the end, when it looks
like we are (almost) started successfully.
- for IPv4, only attempt to load the lease if we actually are
interested in the address. Also, reduce the scope of the lease
variable, to the one place where we need it.
(cherry picked from commit ab314065b8)
The client-id is something that we want to determine top-down.
Meaning, if the user specifies it via ipv4.dhcp-client-id, then it
should be used. If the user leaves it unspecified, we choose a
default stable client-id. For the internal DHCP plugin, this is
a node specific client-id based on
- the predictable interface name
- and /etc/machine-id
It's not clear, why we should allow specifying the client-id in
the lease file as a third source of configuration. It really pushes
the configuration first down (when we do DHCP without lease file),
to store an additional bit of configuration for future DHCP attempts.
If the machine-id or the interface-name changes, then so does the
default client-id. In this case, also "ipv4.dhcp-client-id=stable"
changes. It's fair to require that the user keeps the machine-id
stable, if the machine identity doesn't change.
Also, the lease files are stored in /var/lib/NetworkManager, which
is more volatile than /etc/machine-id. So, if we think that machine-id
and interface-name is not stable, why would we assume that we have
a suitable lease file?
Also, if you do:
nmcli connection add con-name "$PROFILE" ... ipv4.dhcp-client-id ''
nmcli connection up $PROFILE
nmcli connection modify "$PROFILE" ipv4.dhcp-client-id mac
nmcli connection up $PROFILE
nmcli connection modify "$PROFILE" ipv4.dhcp-client-id ''
nmcli connection up $PROFILE
wouldn't you expect that the original (default) client-id is used again?
Also, this works badly with global connection defaults in
NetworkManager.conf. If you configure a connection default, previously
already this would always force the client-id and overrule the lease.
That is reasonable, but in which case would you ever want to use
the client-id from the lease?
(cherry picked from commit 5b9bc174d1)
- if we leave the client-id of sd_dhcp_client unset, it will
anyway generate a node-specific client-id (and may fail if
"/etc/machine-id" is invalid).
Anticipate that, and don't let the client-id unset. In case
we have no client-id from configuration or lease, just generate
the id ourself (using the same algorithm). The advantage is,
that we know it upfront and can store the client-id in the
NMDhcpClient instance. We no longer need to peel it out from
the lease later.
- to generate the IPv4 client-id, we need a valid MAC address. Also,
sd_dhcp_client needs a MAC address for dhcp_network_bind_raw_socket()
as well. Just require that a MAC address is always needed. Likewise,
we need a valid ifindex and ifname set.
- likewise for IPv6 and IPv4, cleanup detecting the arptype and
checking MAC address length. sd_dhcp_client_set_mac() is overly
strict at asserting input arguments, so we must validate them anyway.
- also, now that we always initialize the client-id when starting
the DHCP client, there is no need to retroactively extract it
again when we receive the first lease.
(cherry picked from commit c3e7e6170d)
A possible issue is that client_start() has about 136 arguments.
It doesn't get simpler by saving lines of code and writing them
all in the same line.
Wrap the lines.
While at it, use "FALSE" for "enforce_duid" argument, instead of "0".
It's a boolean.
(cherry picked from commit ce1cfd7232)
We don't do that for ip4_start() either. The duid/client-id
is stored inside the NMDhcpClient instance, and the function can
access it from there.
Maybe, it is often preferable to have stateless objects and not
relying on ip4_start() to obtain the client ID from the client's
state. However, the purpose of the NMDhcpClient object is to
hold state about DHCP. To simplify the complexity of objects that
inherrently have state, we should be careful about mutating the state.
It adds little additional complexity of only reading the state when
needed anyway. In fact, it adds complexity, because previously
it wasn't enough to check all callers of nm_dhcp_client_get_client_id()
to see where the client-id is used. Instead, one would also need to
follow the @duid argument several layers of the call stack.
(cherry picked from commit 7d55b1348b)
There should be lower layers that are concerned with writing
and reading dhclient configuration files. It's wrong to
have a nm_dhcp_dhclient_save_duid() function which requires
the caller to pre-escape the string to write. The caller shouldn't
be concerned with the file format, that's why the function
is used in the first place.
(cherry picked from commit cd9e418fbe)
We only used "client_id" for IPv4 and "duid" for IPv6. Merge them.
Another advantage is, that we can share the logging functionality
of _set_client_id().
(cherry picked from commit 7e341b73e0)
Drop unused function.
Aside from that, dhclient configuration files support a very complex
syntax. The parser was very naive and insufficient in parsing such
files. It's good we can just drop it.
(cherry picked from commit 025157d597)
Why would we do this? The configuration file we are reading back was
written by NetworkManager in the first place.
Maybe when assuming a connection after restart, this information could
be interesting. It however is not actually relevant.
Note how nm_dhcp_client_get_client_id() has only very few callers.
- nm_device_spawn_iface_helper() in 'nm-device.c'. In this case,
we either should use the client-id which we used when starting
DHCP, or none at all.
- ip4_start() in 'nm-dhcp-dhclient.c', but this is before starting
DHCP client and before it was re-read from configuration file.
- in "src/dhcp/nm-dhcp-systemd.c", but this has no effect for
the dhclient plugin.
(cherry picked from commit 5411fb0cc6)
Our internal DHCP client (from systemd) defaults to a particular client ID.
It is currently exposed as nm_sd_utils_generate_default_dhcp_client_id()
and is based on the systemd implementation.
One problem with that is, that it internally looks up the interface name
with if_indextoname() and reads /etc/machine-id. Both makes it harder
for testing.
Another problem is, that this way of generating the client-id is
currently limited to internal client. Why? If you use dhclient plugin,
you may still want to use the same algorithm. Also, there is no explict
"ipv4.dhcp-client-id" mode to select this client-id (so that it could
be used in combination with "dhclient" plugin).
As such, this code will be useful also aside systemd DHCP plugin.
Hence, the function should not be obviously tied to systemd code.
The implementation is simple enough, and since we already have a
unit-test, refactor the code to our own implementation.
(cherry picked from commit a55795772a)
Internal DHCP client generates a default client ID. For one,
we should ensure that this algorithm does not change without
us noticing, for example, when upgrading systemd code. Add
a test, that the generation algorithm works as we expect.
Also note, that the generation algorithm uses siphash24().
That means, siphash24() implementation also must not change
in the future, to ensure the client ID doesn't change. As we
patch systemd sources to use shared/c-siphash, this is not
obviously the case. Luckily c-siphash and systemd's siphash24 do
agree, so all is good. The test is here to ensure that.
Also, previously the generation algorithm is not exposed as a
function, sd_dhcp_client will just generate a client-id when
it needs it. However, later we want to know (and set) the client
id before starting DHCP and not leave it unspecified to an
implementation detail.
This patch only adds a unit-test for the existing DHCP client
ID generation to have something for comparison. In the next
commit this will change further.
(cherry picked from commit 187d356198)
- use NMUuid type where appropriate.
- no error handling for generate_duid_from_machine_id().
It cannot fail anymore.
- add thread-safety to generate_duid_from_machine_id() with
double-checked locking.
- use unions for converting the sha256 digest to the target
type.
(cherry picked from commit 50121ee028)
For testing purpose, it's bad to let nm_utils_stable_id_parse()
directly access nm_utils_get_boot_id_str(). Instead, the function
should have no side-effects.
Since the boot-id is anyway cached, accessing it is cheap. Even
if it likely won't be needed.
(cherry picked from commit c51e63feb6)
Tests might access the secret-key.
For CI builds we may very well build NM as root and also run
unit tests. In such a situation it's bad to persist the secret
key. For example, the SELinux label may be wrong, and subsequently
starting NetworkManager may cause errors. Avoid persisting the secret
key for tests.
(cherry picked from commit 581e1c3269)
