The BOOTIF MAC address can be prefixed with a hardware address
type. Typically it is 01 (for ethernet), but the legacy network module
accepts (and strips) any byte value.
It seems wrong to take any address type without validation. In
addition to "01", also accept a zero type which, according to the
bugzilla below, is used in some configurations to mean "undefined".
While at it, also accept ':' as separator for the first byte.
https://bugzilla.redhat.com/show_bug.cgi?id=1904099https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/713
This assert sometimes fails during copr builds. But the way
the assert was, it was hard to see what the actual problem
was.
Restructure the assert (again) to get the errno in the
test logs.
RFCs actually expect to honor the lifetime. See for example [1].
This is just not right, and totally arbitrary. It was added
when our libndp based implementation was added, but unclear
why this was done (beyond the code comment).
[1] page 204, v6LC.2.2.25: Processing Router Advertisement DNS (Host
only) at https://ipv6ready.org/docs/Core_Conformance_5_0_0.pdf
There is no actual change in behavior, because "struct nd_opt_hdr"
as two uint8_t, so in practice this struct was always packed already.
But make it explicit, because it's clear that we use these structs
to set the binary message and they need a well defined (packed) memory
layout.
The endpoints of WireGuard peers can be configured as DNS name, which
NetworkManager will resolve.
Since activating a profile might affect now names get resolved, we must
first resolve names before completing the activation of the WireGuard
device (and before reconfiguring DNS accordingly).
For example, if you configure exclusive DNS resolution via the WireGuard
device, and if the peer needs to be resolved via DNS, then resolving the
peer name must happen before the reconfiguration of DNS. Otherwise the
new DNS configuration will be broken due to being unable to reach the
WireGuard peer.
Fix that by waiting.
There is still an unfixed problem. If resolving any peers fails,
activation silently proceeds -- again possibly breaking the network
setup. Of course, NetworkManager will repeatedly try to re-resolve
the name, but that may never succeed if DNS would be resolved via
the VPN itself.
That is different from `wg set` which resolves hostnames and fails.
Consequently `wg-quick up` would also fail. But these are both one shot
applications, they are not around and basically let the user handle the
error (by reading the log and invoking the command again). NetworkManager
can do something different and proceed activation (as it will also
periodically re-resolve the hostnames again). Note that it's also valid
to activate a WireGuard device without any peers (and to modify the
activated device later with Reapply()). As such, having no peers (or
being unable to resolve a hostname) may be a valid configuration.
I think we should add an option/flag that when enabled will cause
the activation to fail of names cannot be resolved.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/535https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/616https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/721
With LTO and gcc-10.2.1-9.fc33.s390x we get:
src/platform/nm-platform.c:3325:1: error: link_duplex may be used uninitialized in this function [-Werror=maybe-uninitialized]
3325 | NM_UTILS_LOOKUP_STR_DEFINE(nm_platform_link_duplex_type_to_string,
| ^
src/devices/nm-device-ethernet.c:899: note: link_duplex was declared here
899 | NMPlatformLinkDuplexType link_duplex;
|
With gcc-10.2.1-9.fc33.s390x we get a (false positive) warning:
src/platform/tests/test-common.c: In function nmtstp_acd_defender_new:
src/platform/tests/test-common.c:2688:15: error: probe may be used uninitialized in this function [-Werror=maybe-uninitialized]
2688 | *defender = (NMTstpAcdDefender){
| ^
src/platform/tests/test-common.c:2656:56: note: probe was declared here
2656 | NAcdProbe * probe;
| ^
With LTO, the compiler can see that some code paths return without
initializing the variable. But it fails to see that those are code
paths after an assertion fail. Still that can lead to
"-Wmaybe-uninitialized" warnings in the caller.
Avoid that by not using g_return_if_fail() but nm_assert().
src/nm-ip6-config.c: In function '_nmtst_ip6_config_get_address':
./shared/nm-glib-aux/nm-dedup-multi.h:337:8: error: 'iter._next' may be used uninitialized in this function [-Werror=maybe-uninitialized]
337 | if (!iter->_next)
| ^
src/nm-ip6-config.c:1622:33: note: 'iter._next' was declared here
1622 | NMDedupMultiIter iter;
| ^
./shared/nm-glib-aux/nm-dedup-multi.h:343:8: error: 'iter._head' may be used uninitialized in this function [-Werror=maybe-uninitialized]
343 | if (iter->_next->next == iter->_head)
| ^
src/nm-ip6-config.c:1622:33: note: 'iter._head' was declared here
1622 | NMDedupMultiIter iter;
| ^
and more.
When compiling with LTO, the compiler can think that an
assertion failure (g_return*()) is regular code path, and
thus that the output variable is not set.
This can lead to "-Wmaybe-uninitialized" warnings in the
caller, despite this not happening in non-bug case.
Work aound that by setting the out argument.
Warning with LTO enabled and gcc-10.2.1-9.fc33.s390x:
src/nm-config-data.c: In function nm_config_data_get_device_config:
src/devices/nm-device.c:17454:9: error: is_fake may be used uninitialized in this function [-Werror=maybe-uninitialized]
17454 | m = nm_match_spec_device(specs,
| ^
src/devices/nm-device.c:17444:26: note: is_fake was declared here
17444 | gboolean is_fake;
| ^
src/nm-config-data.c: In function nm_config_data_get_connection_default:
src/devices/nm-device.c:17454:9: error: is_fake may be used uninitialized in this function [-Werror=maybe-uninitialized]
17454 | m = nm_match_spec_device(specs,
| ^
src/devices/nm-device.c:17444:26: note: is_fake was declared here
17444 | gboolean is_fake;
| ^
src/devices/nm-device.c: In function nm_device_check_unrealized_device_managed:
src/devices/nm-device.c:17454:9: error: is_fake may be used uninitialized in this function [-Werror=maybe-uninitialized]
17454 | m = nm_match_spec_device(specs,
| ^
src/devices/nm-lldp-listener.c: In function 'lldp_neighbor_to_variant':
./shared/nm-glib-aux/nm-shared-utils.h:1271:5: error: 'raw_len' may be used uninitialized in this function [-Werror=maybe-uninitialized]
1271 | g_variant_builder_add(builder,
| ^
src/devices/nm-lldp-listener.c:107:19: note: 'raw_len' was declared here
107 | gsize raw_len;
| ^
./shared/nm-glib-aux/nm-shared-utils.h:1271:5: error: 'raw_data' may be used uninitialized in this function [-Werror=maybe-uninitialized]
1271 | g_variant_builder_add(builder,
| ^
src/devices/nm-lldp-listener.c:106:19: note: 'raw_data' was declared here
106 | gconstpointer raw_data;
| ^
RFC4361 intends to set the same IAID/DUID for both DHCPv4 and DHCPv6.
Previously, we didn't have a mode for that.
Of course, you could always set "ipv4.dhcp-client-id" and
"ipv6.dhcp-duid" to (the same) hex string, but there was no
automatic mode. Instead we had:
- "ipv4.dhcp-client-id=duid" which sets the client ID to a stable,
generated DUID. However, there was no option so that the same
DUID/IAID would be automatically used for DHCPv6.
- there are various special values for "ipv6.dhcp-duid" which generate
a stable DUIDs. However, those values did not work for
"ipv4.dhcp-client-id".
Solve that by adding "ipv4.dhcp-client-id=ipv6-duid" which indicates to use
the DUID from DHCPv6's "ipv6.dhcp-duid" setting. As IAID it will prefer "ipv4.dhcp-iaid"
(if set), but fallback to "ipv6.dhcp-iaid".
https://tools.ietf.org/html/rfc4361https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/618https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/718
The default value is `optional' so the current check in place is
actually wrong and blocks NM from connecting to WPA3 networks.
Overall the check is not that useful as WPA3 Enterprise always
requires PMF to be 'required' so let's always force it instead.
Signed-off-by: Antonio Cardace <acardace@redhat.com>
Fixes: e874ccc917 ('wifi: add WPA-EAP-SUITE-B-192 support')
When building without "more-asserts" and LTO enabled, we can get
a warning about uninitalized "obj" variable:
src/platform/nm-linux-platform.c: In function 'ip_route_add':
src/platform/nm-platform.c:4761:24: warning: 'MEM[(struct NMPlatformIPRoute *)&obj + 24B].rt_source' may be used uninitialized in this function [-Wmaybe-uninitialized]
4761 | route->rt_source = nmp_utils_ip_config_source_round_trip_rtprot(route->rt_source);
| ^
src/platform/nm-platform.h:2139:25: warning: 'BIT_FIELD_REF <MEM[(const struct NMPlatformIPRoute *)&obj + 24B], 8, 72>' may be used uninitialized in this function [-Wmaybe-uninitialized]
2139 | return r->table_any ? 254u /* RT_TABLE_MAIN */
|
That is due to the "default" switch case which was unhandled
when building without more-asserts". Avoid that by reworking the
code.
When building without more-assertions and LTO, the compiler might think
that "wait" is uninitialized. Avoid the warning.
Initializing a variable is not a great solution either, because
potentially it could hide an actual bug. But it still seems to be
best.
src/nm-policy.c: In function update_system_hostname:
src/nm-policy.c:909: warning: wait may be used uninitialized in this function [-Wmaybe-uninitialized]
909 | if (wait) {
|
src/nm-policy.c:901: note: wait was declared here
901 | gboolean wait;
|
On copr builds, the unit tests sometimes fail to create a veth
interface. In those cases, kernel rejects the netlink request
with EPERM. copr uses mock on Fedora 33 hosts.
I think this is a kernel bug. Add a workaround by retrying a few times.
Linux headers and some libc headers have overlapping defines
for network types and functions.
In the past years, glibc and linux headers were improved to cooperate
so you could include either one, in any order.
With musl and possibly some older glibc versions that doesn't work so
well.
Reorder and change includes to make it work better. Yes, this looks
pretty random and unmotivated. The includes are changed in order to
successfully build on various libc/kernel versions, with the goal
of not using #if.
Due to mixing includes of userspace network headers (net/*) and
kernelspace onces (linux/if*) symbol redefinitions happen on musl.
[thaller@redhat.com: modified original patch]
We were asserting against error messages from strerror(), and on libmusl
these are different. Relax the checks.
We still assert against parts of the text, where possible. So a similar
problem could happen in the future or with another libc library.
Add a new key management option to support WPA3 Enteprise wifi
connection.
Only supported with wpa_supplicant for the time being.
Signed-off-by: Antonio Cardace <acardace@redhat.com>
Currently libnm headers include <linux/if_{ether,infiniband,vlan}.h>.
These are public headers, that means we drag in the linux header to all
users of <NetworkManager.h>.
Often the linux headers work badly together with certain headers from libc.
Depending on the libc version, you have to order linux headers in the right
order with respect to libc headers.
We should do better about libnm headers. As a first step, assume that
the linux headers don't get included by libnm, and explicitly include
them where they are needed.
These typedefs are defined by some libc headers, and we drag
them in by including some other standard headers.
It's not clear which headers we exactly need for them, and that
all libcs provide them the same.
Instead, just avoid them.
If the DNS configuration changes, the hostname previously determined
via reverse DNS lookup could be stale. Clear the resolver data of every
interface and try again.
Fixes: 09c8387114 ('policy: use the hostname setting')
Veth interfaces should be shown as Ethernet from
nm_device_get_type_description in order to provide backward
compatibility.
Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
<netinet/ether.h> with musl defines ethhdr struct, which conflicts
with <linux/if_ether.h>. The latter is included by "nm-utils.h",
so this is a problem.
Drop includes of "netinet/ether.h" that are not necessary.
wpa_supplicant has a property "scanning" and a "state=scanning".
Previously, NetworkManager considered both parts to indicate whether
supplicant is currently scanning (if either the property or the state
indicated scanning, it took that as indication for scanning).
If NetworkManager thinks that supplicant is scanning, it suppresses
explicit "Scan" requests. That alone is not severe, because the "Scan"
request is only to trigger a scan in supplicant (which supplicant
possibly is already doing in state "scanning").
However, what is severe is that NetworkManager will also block autoconnect
while supplicant is scanning. That is because NetworkManager wants to get
a complete scan result before deciding which network to connect to.
It seems that wpa_supplicant can get into "state=scanning" and stay
there indefinitely. This prevents NetworkManager from autoactivating
a profile.
Fix that, to only honor the "scanning" property.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/597
Fixes: b83f07916a ('supplicant: large rework of wpa_supplicant handling')
During shutdown, NM always tries to remove from ovsdb all bridges,
ports, interfaces that it previously added. Currently NM doesn't run
the main loop during shutdown and so it's not possible to perform
asynchronous operations. In particular, the NMOvsdb singleton is
disposed in a destructor where it's not possible to send out all the
queued deletions.
The result is that NM deletes only one OVS interface, keeping the
others. This needs to be fixed, but requires a rework of the shutdown
procedure that involves many parts of NM.
Even when a better shutdown procedure will be implemented, we should
support an unclean shutdown caused by e.g. a kernel panic or a NM
crash. In these cases, the interfaces added by NM would still linger
in the ovsdb.
Delete all those interface at NM startup. If there are connections
profiles for them, NM will create them again.
https://bugzilla.redhat.com/show_bug.cgi?id=1861296https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/700
By now, each NMDevice does the reverse lookup and caches the result
via nm_device_get_hostname_from_dns_lookup().
The code is no longer used in NMPolicy.
Fixes: 09c8387114 ('policy: use the hostname setting')
