This is loosely based on nms-ibft-reader, but with some significant
changes. Notably, it parses /sys/firmware/ibft directly instead of
iscsiadm output.
iscsiadm is not available on early boot (perhaps it's too large) and
turns out that parsing sysfs directly is easier and more
straightforwared anyways. A win-win situation.
It is not useful alone, it's in a separate commit just for the sake of
easier review.
We need a mode that:
* doesn't leave processes behind
* doesn't force an internal dhclient
* doesn't auto-generate default connections
* doesn't write out files into libdir, only /run
The original configure-and-quit mode doesn't really fit the initrd use. But
it's proobably not a good idea to just change its behavior.
This is useful for in-memory connections to persist NetworkManager
restarts (as opposed to machine restarts).
Perhaps most improtantly, this allows generating in-memory connections outside
NetworkManager, e.g. passing configuration from early boot firmware in initrd.
Note that this does *not* aspire to do more than it says on the tin:
Notably, it doesn't touch the problem of provisioning connections in multiple
persistent connection directories and thus doesn't have to deal with the
problem of deleting or overlaying the connections tha (rh #772414) deals
with.
Especially with configure-and-quit, it's easy to encounter a condition,
where the device reached a failed state, policy decides to quit, but the
active connection is not yet torn down from the device.
Upon the next start NetworkManager would think the connection succeeded
activating.
Make them just ask for connections from GDBus, as other D-Bus clients
do. GDBus anyway reuses the connection if it has one, but allows us to
deal with errors in a more civilized manner.
Using these unormalized was wrong all along, but by chance didn't hit
paths that needed normalized connections. This may change if we
actually write in memory connections to /run with the keyfile plugin,
because that one wants them normalized.
This also saves some work, because normalization does boring things for
us, such as adding default ipv4/ipv6/proxy settings everywhere.
On networked boot we need to somehow communicate this to the early boot
machinery. Sadly, no DBus there and we're running in configure-and-quit
mode.
Abusing the state file for this sounds almost reasonable and is
reasonably straightforward thing to do.
libcurl does not allow removing easy-handles from within a curl
callback.
That was already partly avoided for one handle alone. That is, when
a handle completed inside a libcurl callback, it would only invoke the
callback, but not yet delete it. However, that is not enough, because
from within a callback another handle can be cancelled, leading to
the removal of (the other) handle and a crash:
==24572== at 0x40319AB: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==24572== by 0x52DDAE5: Curl_close (url.c:392)
==24572== by 0x52EC02C: curl_easy_cleanup (easy.c:825)
==24572== by 0x5FDCD2: cb_data_free (nm-connectivity.c:215)
==24572== by 0x5FF6DE: nm_connectivity_check_cancel (nm-connectivity.c:585)
==24572== by 0x55F7F9: concheck_handle_complete (nm-device.c:2601)
==24572== by 0x574C12: concheck_cb (nm-device.c:2725)
==24572== by 0x5FD887: cb_data_invoke_callback (nm-connectivity.c:167)
==24572== by 0x5FD959: easy_header_cb (nm-connectivity.c:435)
==24572== by 0x52D73CB: chop_write (sendf.c:612)
==24572== by 0x52D73CB: Curl_client_write (sendf.c:668)
==24572== by 0x52D54ED: Curl_http_readwrite_headers (http.c:3904)
==24572== by 0x52E9EA7: readwrite_data (transfer.c:548)
==24572== by 0x52E9EA7: Curl_readwrite (transfer.c:1161)
==24572== by 0x52F4193: multi_runsingle (multi.c:1915)
==24572== by 0x52F5531: multi_socket (multi.c:2607)
==24572== by 0x52F5804: curl_multi_socket_action (multi.c:2771)
Fix that, by never invoking any callbacks when we are inside a libcurl
callback. Instead, the handle is marked for completion and queued. Later,
we complete all queue handles separately.
While at it, drop the @error argument from NMConnectivityCheckCallback.
It was only used to signal cancellation. Let's instead signal that via
status NM_CONNECTIVITY_CANCELLED.
https://bugzilla.gnome.org/show_bug.cgi?id=797136https://bugs.launchpad.net/ubuntu/+source/network-manager/+bug/1792745https://bugzilla.opensuse.org/show_bug.cgi?id=1107197https://github.com/NetworkManager/NetworkManager/pull/207
Fixes: d8a31794c8
As we accept addr_family %AF_UNSPEC to detect the address family,
we also need to return it. Just returning the binary address without
the address family makes no sense.
Note that NMSettingEthtool and NMSettingMatch don't have such
functions either.
We have API
nm_connection_get_setting (NMConnection *, GType)
nm_connection_get_setting_by_name (NMConnection *, const char *)
which can be used generically, meaning: the requested setting type
is an argument to the function. That is generally more useful and
flexible.
Don't add API which duplicates existing functionality and is (arguably)
inferiour. Drop it now. This is an ABI/API break for the current development
cycle where the 1.14.0 API is still unstable. Indeed it's already after
1.14-rc1, which is ugly. But it's also unlikely that somebody already uses
this API/ABI and is badly impacted by this change.
Note that nm_connection_get_setting() and nm_connection_get_setting_by_name()
are slightly inconvenient in C still, because they usually require a cast.
We should fix that by changing the return type to "void *". Such
a change may be possibly any time without breaking API/ABI (almost, it'd
be an API change when taking a function pointer without casting).
(cherry picked from commit a10156f516)
We cannot be sure who holds a reference to the proxy, and
who is gonna call us back after the VPN connection instance
is destroyed.
(cherry picked from commit 6ebb9091d2)
Got this assertion:
NetworkManager[12939]: <debug> [1536917977.4868] active-connection[0x563d8fd34540]: set state deactivated (was deactivating)
...
NetworkManager[12939]: nm-openvpn[1106] <info> openvpn[1132]: send SIGTERM
NetworkManager[12939]: nm-openvpn[1106] <info> wait for 1 openvpn processes to terminate...
NetworkManager[12939]: nm-openvpn[1106] <warn> openvpn[1132] exited with error code 1
NetworkManager[12939]: <info> [1536917977.5035] vpn-connection[0x563d8fd34540,2fdeaea3-975f-4325-8305-83ebca5eaa26,"my-openvpn-Red-Hat",0]: VPN plugin: requested secrets; state disconnected (9)
NetworkManager[12939]: plugin_interactive_secrets_required: assertion 'priv->vpn_state == STATE_CONNECT || priv->vpn_state == STATE_NEED_AUTH' failed
Meaning. We should either ensure that secrets_required_cb() signal callback
is disconnected from proxy's signal, or we gracefully handle callbacks at
unexpected moments. Do the latter.
(cherry picked from commit 92344dd084)
dhcpcd version 6, the first supporting IPv6, was released more than 5
years ago. Remove all checks on version number and IPv6 support.
(cherry picked from commit e0c49d7341)
Rename variables for the error number. Commonly the naming
is:
- errno: the error number from <errno.h> itself
- errsv: a copy of errno
- nlerr: a netlink error number
- err: an error code, but not a errno/errsv and not
a netlink error number.
(cherry picked from commit f4de941d98)
Internal DHCPv4 client requires a valid MAC address for functioning.
Just always require a MAC address to start DHCP, both v4 and v6.
We have no MAC address for example on Layer3 devices like tun or wireguard.
Also, before "0a797bdc2a systemd/dhcp: fix assertion starting DHCP
client without MAC address", if we tired to start sd_dhcp_client without
setting a MAC address, an assertion was triggered.
(cherry picked from commit e8fa75ce06)
An assertion in dhcp_network_bind_raw_socket() is triggered when
starting an sd_dhcp_client without setting setting a MAC address
first.
- sd_dhcp_client_start()
- client_start()
- client_start_delayed()
- dhcp_network_bind_raw_socket()
In that case, the arp-type and MAC address is still unset. Note that
dhcp_network_bind_raw_socket() already checks for a valid arp-type
and MAC address below, so we should just gracefully return -EINVAL.
Maybe sd_dhcp_client_start() should fail earlier when starting without
MAC address. But the failure here will be correctly propagated and
the start aborted.
See-also: https://github.com/systemd/systemd/pull/10054
(cherry picked from commit 34af574d58)
It's enough that all code paths in impl_ppp_manager_set_ifindex() log exactly
one message. Also, give all messages the same prefix, so that it's clear where
they come from.
(cherry picked from commit 2a45c32e8c)
In src/ppp/nm-pppd-plugin.c, it seems that pppd can invoke
phasechange(PHASE_RUNNING:) multiple times. Hence, the plugin
calls SetIfindex multiple times too. In nm-ppp-manager.c, we
want to make sure that the ifindex does not change after it
was set once. However, calling SetIfindex with the same ifindex
is not something worth warning. Just log a debug message and nothing.
Maybe the plugin should remember that it already set the ifindex,
and avoid multiple D-Bus calls. But it's unclear that that is desired.
For now, just downgrade the warning.
(cherry picked from commit 4a4439835d)
When unplugging an USB 3G modem device, pppd does not exit correctly and
we have the following traces:
Sep 10 07:58:24.616465 ModemManager[1158]: <info> (tty/ttyUSB0): released by device '/sys/devices/pci0000:00/0000:00:1c.0/0000:01:00.0/usb4/4-1'
Sep 10 07:58:24.620314 pppd[2292]: Modem hangup
Sep 10 07:58:24.621368 ModemManager[1158]: <info> (tty/ttyUSB1): released by device '/sys/devices/pci0000:00/0000:00:1c.0/0000:01:00.0/usb4/4-1'
Sep 10 07:58:24.621835 ModemManager[1158]: <warn> (ttyUSB1): could not re-acquire serial port lock: (5) Input/output error
Sep 10 07:58:24.621358 NetworkManager[1871]: <debug> ppp-manager: set-ifindex 4
Sep 10 07:58:24.621369 NetworkManager[1871]: <warn> ppp-manager: can't change the ifindex from 4 to 4
Sep 10 07:58:24.623982 NetworkManager[1871]: <info> device (ttyUSB0): state change: activated -> unmanaged (reason 'removed', sys-iface-state: 'removed')
Sep 10 07:58:24.624411 NetworkManager[1871]: <debug> kill child process 'pppd' (2292): wait for process to terminate after sending SIGTERM (15) (send SIGKILL in 1500 milliseconds)...
Sep 10 07:58:24.624440 NetworkManager[1871]: <debug> modem-broadband[ttyUSB0]: notifying ModemManager about the modem disconnection
Sep 10 07:58:24.626591 NetworkManager[1871]: <debug> modem-broadband[ttyUSB0]: notifying ModemManager about the modem disconnection
Sep 10 07:58:24.681016 NetworkManager[1871]: <warn> modem-broadband[ttyUSB0]: failed to disconnect modem: GDBus.Error:org.freedesktop.DBus.Error.UnknownMethod: No such interface 'org.freedesktop.ModemManager1.Modem.Simple' on object at path /org/freedesktop/ModemManager1/Modem/0
Sep 10 07:58:26.126817 NetworkManager[1871]: <debug> kill child process 'pppd' (2292): process not terminated after 1502368 usec. Sending SIGKILL signal
Sep 10 07:58:26.128121 NetworkManager[1871]: <info> device (ppp0): state change: disconnected -> unmanaged (reason 'unmanaged', sys-iface-state: 'removed')
Sep 10 07:58:26.135571 NetworkManager[1871]: <debug> kill child process 'pppd' (2292): terminated by signal 9 (1511158 usec elapsed)
This is due to nm-ppp-plugin waiting on SetIfIndex call until timeout,
which is longer than termination process timeout.
Calling g_dbus_method_invocation_return_value() on error fixes this.
Fixes: dd98ada33fhttps://mail.gnome.org/archives/networkmanager-list/2018-September/msg00010.html
(cherry picked from commit e66e4d0e71)
The Station.GetOrderedNetworks dbus method's return type has changed in
IWD commit 0a42f63d42be903a46c595693884772c1c84d39f as the last incompatible
API change before IWD 0.8 (docs change was made earlier in
0453308134a3aadb6a2ec6a78ea642e19427704c) so that network names and
types are no longer included in the reply. Expect this new reply
signature although still handle the old signature if we're using the
Device interface for IWD <= 0.7 compatibility.
It may be good idea to eventually pass the object manager instance from
nm-iwd-manager.c to nm-device-iwd.c to avoid using g_dbus_proxy_new_sync
and g_dbus_proxy_new_for_bus_sync in act_stage2_config, which possibly
generates a lot of DBus property queries.
https://github.com/NetworkManager/NetworkManager/pull/197
(cherry picked from commit 32506c8788)
Later we want to fully support wireguard devices. Also,
possibly activating a generic profile in a wireguard device
would make sense.
Anyway, for the moment, just prevent that from happening
by explicitly marking the device as unmanaged.
(cherry picked from commit e3bd482329)
Currently, NMDeviceWireguard does neither set connection_type_check_compatible
nor implement check_connection_compatible. That means, it appears to be compatible
with every connection profile, which is obviously wrong.
Allow devices not to implement check_connection_compatible() and avoid the issue
by rejecting profiles by default.
(cherry picked from commit baa0008313)
Ok, I changed my mind.
The new behavior seems to make more sense to me. Not that it matters,
because we always use nm_utils_strbuf*() API with buffers that we expect
to be large enough to contain the result. And when truncation occurs,
we usually don't care much about it. That is, there is no code that
uses nm_utils_strbuf*() API and handles string truncation in particular.
NM_IN_SET will only compare string pointers and isn't useful for
checking if nm_setting_wireless_get_mode (s_wifi) is infrastructure.
Fixes: 570e1fa75b
Make sure we free our IWD agent objects whenever we're freeing the
IWD Object Manager. We're registering those objects on the same DBus
connection as the Object Manager so that they're visible to IWD, and
our only reference to that connection is through priv->object_manager
so even though the connection isn't changing when we free the object
manager and create a new one, we still need to free the agent object.
We could maybe keep a reference to the connection, but I'm not sure
there's any warranty that it doesn't get closed. We could also use
nm_dbus_manager_get_connection (nm_dbus_manager_get ()) and only
register and free the agent once, since it happens to be the same
connection but it'd perhaps be a hack to rely on this.
- previously, parsing wireguard genl data resulted in memory corruption:
- _wireguard_update_from_allowedips_nla() takes pointers to
allowedip = &g_array_index (buf->allowedips, NMWireGuardAllowedIP, buf->allowedips->len - 1);
but resizing the GArray will invalidate this pointer. This happens
when there are multiple allowed-ips to parse.
- there was some confusion who owned the allowedips pointers.
_wireguard_peers_cpy() and _vt_cmd_obj_dispose_lnk_wireguard()
assumed each peer owned their own chunk, but _wireguard_get_link_properties()
would not duplicate the memory properly.
- rework memory handling for allowed_ips. Now, the NMPObjectLnkWireGuard
keeps a pointer _allowed_ips_buf. This buffer contains the instances for
all peers.
The parsing of the netlink message is the complicated part, because
we don't know upfront how many peers/allowed-ips we receive. During
construction, the tracking of peers/allowed-ips is complicated,
via a CList/GArray. At the end of that, we prettify the data
representation and put everything into two buffers. That is more
efficient and simpler for user afterwards. This moves complexity
to the way how the object is created, vs. how it is used later.
- ensure that we nm_explicit_bzero() private-key and preshared-key. However,
that only works to a certain point, because our netlink library does not
ensure that no data is leaked.
- don't use a "struct sockaddr" union for the peer's endpoint. Instead,
use a combintation of endpoint_family, endpoint_port, and
endpoint_addr.
- a lot of refactoring.
Move NMLinuxPlatformPrivate earlier.
In the past, I moved the declaration of NMLinuxPlatformPrivate
after utility functions which are independent from platform
instance.
However, parsing netlink messages actually requires
NMLinuxPlatformPrivate, because we want to access the "genl"
socket.
So, move the types to the beginning of the file, like we do
for most other source files.
