The option is mandatory in the replies from server and so we don't
need to ask for it. dhclient doesn't do it either. But especially, it
seems that requesting the option causes some broken server
implementations to send duplicate instances of the option.
So, remove the option from the parameter request list of the internal
nettools and systemd DHCP implementation.
(cherry picked from commit 541db78259)
If the server sends a packet with multiple instances of the same
option, they are concatenated during n_dhcp4_incoming_linearize() and
evaluated as a single option as per section 7 of RFC 3396.
However, there are broken server implementations that send
self-contained options in multiple copies. They are reassembled to
form a single instance by the nettools client, which then fails to
parse them because they have a length greater than the expected one.
This problem can be reproduced by starting a server with:
dnsmasq --bind-interfaces --interface veth1 -d
--dhcp-range=172.25.1.100,172.25.1.200,1m
--dhcp-option=54,172.25.1.1
In this way dnsmasq sends a duplicate option 54 (server-id) when the
client requests it in the 'parameter request list' option, as
dhcp=systemd and dhcp=nettools currently do.
While this is a violation of the RFC by the server, both isc-dhcp and
systemd-networkd client implementations have mechanisms to deal with
this situation. dhclient simply takes the first bytes of the
aggregated option. systemd-networkd doesn't follow RFC 3396 and
doesn't aggregate multiple options; it considers only the last
occurrence of each option.
Change the parsing code to accept options that are longer than
necessary.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/324
(cherry picked from commit 1cbf9d22a5)
We now move the deletion of the context-busy-watcher to and idle handler
on the D-Bus GMainContext.
Note that the idle source does not take an additional reference on the
context. Hence, in certain cases it might happen that the context will
be completely unrefed before the idle handler runs. In that case, we
would leak the object.
Avoid that, by taking an additional reference to the GMainContext.
Note that the alternative would be to unref the context-busy-watcher
via the GSource's GDestroyNotify. That is not done, because then the
busy watcher might be unrefed in a different thread. Instead, we want
that to happen for the right context. The only minor downside of this
is that the user now always pays the price and must iterate the context
to fully clean up. But note that the user anyway must be prepared to
iterate the context after NMClient is gone. And that depends on some
unpredictable events that the user cannot control. That means, either
the user handles this correctly already, or the problem anyway exists
(randomly).
Of course all of the discussed "problems" are very specific. In practice, the
users uses the g_main_context_default() instance and anyway will either
keep iterating it or quit the process after the NMClient instance goes
away.
(cherry picked from commit c2f8400e66)
The context-busy-watch has two purposes:
1) it allows the user to watch whether the NMClient still has pending
GSource'es attached to the GMainContext.
2) thereby, it also keeps the inner GMainContext integrated into the
caller's (in case of synchronous initialization of NMClient).
Especially for 2), we must not get this wrong. Otherwise, we might
un-integrate the inner GMainContext too early and it will be leaked
indefinitely (because the user has no means to access or iterate it).
To be extra careful, extend the lifetime of the context-busy-watcher
for one more idle invocation. Theoretically, this should not be necessary,
but it's not clear whether something else is still pending.
The downside of that extra safety is that it is probably unnecessary in
practice. And in case where it is necessary, it hides an actual
issue, making it harder to notice and fix it.
(cherry picked from commit b572c0542a)
When passing a destroy notify to g_dbus_connection_signal_subscribe(),
that callback gets invoked as an idle handler of the associated
GMainContext. That caused to have yet another source attached to the
context after the NMClient gets destroyed.
Especially with synchronous initialization of NMClient that is bad,
because we may destroy the context-busy-watcher too early. That results
in removing the integration of the inner GMainContext into the caller's
context, and thus we leak the inner context indefinitely.
Avoid that leak by not passing a cleanup function to
g_dbus_connection_signal_subscribe().
Fixes: ce0e898fb4 ('libnm: refactor caching of D-Bus objects in NMClient')
(cherry picked from commit e280124757)
Groups currently are not exposed on D-Bus as separate objects.
Also, we might want to expose the property as "ao" instead of "as".
This API needs more thought.
There are likely no users that rely on this property. So, we will
drop it from server side, until it will be requested and newly designed.
Regardless, NMClient needs to gracefully ignore the property.
Despite we will remove it from 1.24 API, libnm should ignore the
property on previous versions. Mark it accordingly.
(cherry picked from commit 05f66697e4)
Do process the connections from the iBFT block if the rd.iscsi.ibft or
rd.iscsi.ibft=1 argument is present.
This is supposed to fix what was originally reported by Kairui Song
<kasong@redhat.com> here: https://github.com/dracutdevs/dracut/pull/697
(cherry picked from commit 39e1e723de)
If an argument in form ip=eth0:ibft is specified, we'd first create a
wired connection with con.interface-name and then proceed completing it
from the iBFT block. At that point we also add the MAC address, so the
interface-name is no longer necessary..
Worse even, for VLAN connections, it results in an attempt to create
a VLAN with the same name as the parent wired device. Ooops.
Let's just drop it. MAC address is guarranteed to be there and does the
right thing for both plain wired devices as well as VLANs.
(cherry picked from commit 59ead70952)
The latter requires __auto_type which is not available in GCC versions
older than 4.9. Fix the following compile error on RHEL 7.8:
CC src/src_libNetworkManagerBase_la-NetworkManagerUtils.lo
shared/n-dhcp4/src/n-dhcp4-c-probe.c: In function 'n_dhcp4_client_probe_transition_nak':
shared/n-dhcp4/src/n-dhcp4-c-probe.c:1008:17: error: unknown type name '__auto_type'
probe->ns_nak_restart_delay = c_clamp(probe->ns_nak_restart_delay * 2,
^
shared/n-dhcp4/src/n-dhcp4-c-probe.c:1008:17: error: unknown type name '__auto_type'
shared/n-dhcp4/src/n-dhcp4-c-probe.c:1008:17: error: unknown type name '__auto_type'
Fixes: 218782a9a3 ('n-dhcp4: restart the transaction after a NAK')
(cherry picked from commit 3a9b069c41)
The 'retracted' event is emitted when the client receives a NAK in the
rebooting, requesting, renewing or rebinding state, while 'expired'
means that the client wasn't able to renew the lease before expiry.
In both cases the old lease is no longer valid and n-dhcp4 keep trying
to get a lease, so the two events should be handlded in the same way.
Note that the systemd client doesn't have a 'retracted' event and
considers all NAKs as 'expired' events.
(cherry picked from commit 2523000b36)
It is not enough to set the INIT state after a NAK; a timeout
(ns_deferred) must be set so that it is added to the event fd. The
client retries immediately the first time, so that in the successful
case it gets an address quickly. To avoid flooding the network in case
of servers always replying with NAKs, next attempts are done with
intervals from 2 seconds to 5 minutes using exponential backoff. See
also systemd commit [1].
[1] 1d1a3e0afbhttps://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/325
(cherry picked from commit 218782a9a3)
When the client enters the INIT state, it calls listen() on the
connection connection to create the packet socket. However, if the
client is coming from the REBOOTING state after a NAK, the connection
is already in the listening state; do nothing in such case.
(cherry picked from commit 4bcdc3c1eb)
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)
Previously NetworkManager would wrongly add a broadcast address for the
network prefix that would collide with the IP address of the host on
the other end of the point-to-point link thus exhausting the IP address
space of the /31 network and preventing communication between the two
nodes.
Configuring a /31 address before this commit:
IP addr -> 10.0.0.0/31, broadcast addr -> 10.0.0.1
If 10.0.0.1 is configured as a broadcast address the communication
with host 10.0.0.1 will not be able to take place.
Configuring a /31 address after this commit:
IP addr -> 10.0.0.0/31, no broadcast address
Thus 10.0.0.0/31 and 10.0.0.1/31 are able to correctly communicate.
See RFC-3021. https://tools.ietf.org/html/rfc3021https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/295https://bugzilla.redhat.com/show_bug.cgi?id=1764986
(cherry picked from commit fa144b5ae9)
I hesitated to add this to libnm, because it's hardly used.
However, we already fetch the property during GetManagedObjects(),
we we should make it accessible, instead of requiring the user to
make another D-Bus call.
(cherry picked from commit 21b008d0ff)
If we know the address used previously, also tell the client to start
from the init-reboot phase, so that it will start with a DHCP request
instead of a discover.
(cherry picked from commit 6af6f70d81)
Instead of terminating the program when the dispatch function returns
an invalid return code, log an error message and convert the error
code to a valid, generic one.
https://bugs.archlinux.org/task/64880
(cherry picked from commit 36f8822c9b)
After t1, the client tries to renew the lease by contacting via the
udp socket the server specified in the server-id option. If this
fails, after t2 it tries to contact any server using broadcast. For
this to work, the packet socket must be used.
(cherry picked from commit f860e929c0)
Currently the client always starts from the INIT state (i.e. sending a
discover message). If a requested-ip was specified by the caller, it
is added as an option in the discover.
It was reported that some DHCP servers don't respond to discover
messages with the requested-ip option set [1][2].
The RFC allows to skip the discover by entering the INIT-REBOOT state
and starting directly with a broadcast request message containing the
requested IP address. Implement that.
[1] https://bugzilla.redhat.com/show_bug.cgi?id=1781856
[2] https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/310
(cherry picked from commit af03b77980)
Otherwise, this function cannot really be used via generated bindings.
Also, it's the only way to actually retrieve the set vlan-ids, without
it, you wouldn't know which ones are set.
Fixes: a9b4532fa7 ('libnm-core: add SR-IOV setting')
(cherry picked from commit c4a728217d)
Found by covscan:
NetworkManager-1.22.0/src/dhcp/nm-dhcp-nettools.c:945: check_return:
Calling "g_file_set_contents" without checking return value (as is
done elsewhere 16 out of 20 times).
Fixes: 9f89516928 ('dhcp: nettools: read/write lease files')
(cherry picked from commit cfc418f887)
