Commit graph

21395 commits

Author SHA1 Message Date
Thomas Haller
429c311275 core: merge branch 'th/device-match-fix'
https://bugzilla.redhat.com/show_bug.cgi?id=1658057

(cherry picked from commit 066c464576)
2018-12-11 14:14:29 +01:00
Thomas Haller
0fb8a25a61 contrib/rpm: adjust match-device spec for 00-server-dhcp-client-id.conf
For older NetworkManager versions, a match spec that only contained except:
specifiers could never yield a positive match. That is not very useful and
got fixed by commit 242de347adbf653a709607979d36a0da1ca3ff0b (core: fix
device spec matching for a list of "except:").

Still, adjust the configuration snippet so that it also works with
configurations that predate the fix.

(cherry picked from commit 3627601bde)
2018-12-11 14:05:23 +01:00
Thomas Haller
8d34d23d52 core: fix match spec behavior for a list of all "except:"
If the spec specifies only negative matches (and none of them matches),
then the result shall be positive.

Meaning:

    [connection*] match-device=except:dhcp-plugin:dhclient
    [connection*] match-device=except:interface-name:eth0
    [.config] enabled=except:nm-version:1.14

should be the same as:

    [connection*] match-device=*,except:dhcp-plugin:dhclient
    [connection*] match-device=*,except:interface-name:eth0
    [.config] enabled=*,except:nm-version:1.14

and match by default. Previously, such specs would never yield a
positive match, which seems wrong.

Note that "except:" already has a special meaning. It is not merely
"not:". That is because we don't support "and:" nor grouping, but all
matches are combined by an implicit "or:". With such a meaning, having
a "not:" would be unclear to define. Instead it is defined that any
"except:" match always wins and makes the entire condition to explicitly
not match. As such, it makes sense to treat a match that only consists
of "except:" matches special.

This is a change in behavior, but the alternative meaning makes
little sense.

(cherry picked from commit a7ef23b326)
2018-12-11 14:05:21 +01:00
Thomas Haller
593af85d31 src/tests: add test for except match spec
(cherry picked from commit d48904c9a9)
2018-12-11 14:04:51 +01:00
Thomas Haller
51bd7dc4aa device: fix matching device-spec for DHCP plugin
https://bugzilla.redhat.com/show_bug.cgi?id=1658057

Fixes: b9eb264efe
(cherry picked from commit 1bff602c46)
2018-12-11 14:03:58 +01:00
Thomas Haller
d1e98e334d connectivity: fix determining the global connectivity state
Since we determine the connectivity state of each device individually,
the global connectivity state is an aggregate of all these states.

I am not sure about considering here devices that don't have the (best)
default route for their respective address family. But anyway.

When we aggregate the best connectivity, we chose the numerical largest
value. That is wrong, because PORTAL is numerically smaller than
LIMITED.

That means, if you have two devices, one with connectivity LIMITED and
one with connectivity PORTAL, then LIMITED wrongly wins.

Fixes: 6b7e9f9b22

https://bugzilla.redhat.com/show_bug.cgi?id=1619873
(cherry picked from commit ade753d06f)
2018-12-11 09:32:11 +01:00
Thomas Haller
51b7b10d3d libnm: add nm_connectivity_state_cmp() helper
(cherry picked from commit 487ee687d5)
2018-12-11 09:31:39 +01:00
Thomas Haller
7c07c59aa3 shared: add NM_MAX_WITH_CMP() macro
(cherry picked from commit e0191d4201)
2018-12-11 09:31:38 +01:00
Thomas Haller
35eedebfcf libnm/docs: fix gtk-doc for NMSettingIP4Config:dhcp-client-id
Fixes: 5ef93c3323
(cherry picked from commit dc34de2734)
2018-12-10 16:32:20 +01:00
Thomas Haller
6fc5dd43fd examples: fix wrong example configuration in "31-mac-addr-change.conf"
Fixes: b869d9cc0d
(cherry picked from commit 6cf183a0e1)
2018-12-10 16:32:20 +01:00
Beniamino Galvani
b2686110ef device: always take device down when changing MAC for wifi devices
If the device is not taken down wpa_supplicant does not detect that
the MAC changed and the authentication can fail due to wrong key
derivation.

Fixes: e206a34732

https://bugzilla.redhat.com/show_bug.cgi?id=1656157
(cherry picked from commit 29e8f6d5a1)
2018-12-10 13:34:27 +01:00
Beniamino Galvani
02eb4bfa9d core: avoid assertion when removing devices
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)
2018-12-05 10:31:42 +01:00
Beniamino Galvani
9135603c93 man: add missing connection defaults
(cherry picked from commit a482b27593)
2018-12-01 15:28:32 +01:00
Beniamino Galvani
70cbda40cb device: fix wrong connection default property
Fixes: 96cabbcbb8
(cherry picked from commit 218d7687a0)
2018-12-01 15:28:29 +01:00
Beniamino Galvani
b64abe8572 cli: avoid crash on device disconnect
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/254
https://bugzilla.redhat.com/show_bug.cgi?id=1546061
(cherry picked from commit cf1126f60b)
2018-11-30 14:25:37 +01:00
Lubomir Rintel
77e8a8bb8d wifi/olpc-mesh: allow autoconnect
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)
2018-11-29 17:55:35 +01:00
Soapux
f8244a72ed meson/libnm.pc: set vpnservicedir path relative to ${prefix}
Make it possible to relocate it under a different prefix:

  $ pkg-config --define-variable=prefix=/whatever
               --variable=vpnservicedir libnm

https://github.com/NetworkManager/NetworkManager/pull/202
(cherry picked from commit a31271d154)
2018-11-29 16:12:03 +01:00
Thomas Haller
047747f9e3 dnsmasq/shared: fix setting DNS nameserver and search for shared dnsmasq
Fixes: c8fa7b6f57

(cherry picked from commit 803514df27)
2018-11-29 14:36:25 +01:00
Thomas Haller
dfd376963e device: merge branch 'th/device-set-mac-addr-no-down'
https://bugzilla.redhat.com/show_bug.cgi?id=1639274

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/54

(cherry picked from commit a8f0da9f91)
2018-11-29 14:12:52 +01:00
Thomas Haller
ca4e28ea2b device: avoid taking down link to change MAC address
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)
2018-11-29 14:07:10 +01:00
Thomas Haller
f8a13422f1 platform: add nm_platform_link_get_ifi_flags() helper
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)
2018-11-29 14:06:32 +01:00
Sebastien Fabre
b2eb897bc0 dbus: register object manager object before requesting dbus name
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)
2018-11-29 07:58:05 +01:00
Thomas Haller
66cbca1f21 platform/tests: fix assertion for unit test for address lifetime
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)
2018-11-28 16:17:33 +01:00
Patrick Talbert
e28bfa41a0 clients: Fix typo in _dump_team_link_watcher output
This makes the output of a 'con show' for a team device look suspect.

Signed-off-by: Patrick Talbert <ptalbert@redhat.com>

Fixes: 4657390d45

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/merge_requests/53
(cherry picked from commit 25a0739658)
2018-11-23 17:41:42 +01:00
Beniamino Galvani
05c27c3c50 lldp: fix parsing of vlan-name attribute
We used to read 3 bytes after the TLV, fix this.

Also, check that string length is at most 32 bytes as specified in
figure E.3 of IEEE 802.1AB-2009.

Fixes: 18133ea142

https://bugzilla.redhat.com/show_bug.cgi?id=1652210
(cherry picked from commit e909778710)
2018-11-22 09:43:18 +01:00
Lubomir Rintel
379eff21ed dhcp: drop unused variable
src/dhcp/nm-dhcp-client.c:552:16: error: unused variable 'str' [-Werror,-Wunused-variable]
        gs_free char *str = NULL;
                      ^

Fixes: 787f4b57cd
(cherry picked from commit 6f111b3d2e)
2018-11-22 07:37:32 +01:00
Beniamino Galvani
dca89e15b1 build: dist initrd meson.build files
Fixes: b544f7243d

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/77
(cherry picked from commit ca0c025486)
2018-11-22 07:37:14 +01:00
Beniamino Galvani
738962dbfe build: meson: fix wrong man page link
Fixes: 98b4a19a53

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/issues/78
(cherry picked from commit 38299a1d78)
2018-11-22 07:37:00 +01:00
Lubomir Rintel
f7e6f0044c libnm/remote-connection: fix "flags" property getter
Fixes: acc8244ca2

https://github.com/NetworkManager/NetworkManager/pull/250
(cherry picked from commit 42e4d09844)
2018-11-21 11:50:36 +01:00
Beniamino Galvani
6af71ce4f4 cli: merge branch 'bg/cli-editor-save-rh1546805'
https://github.com/NetworkManager/NetworkManager/pull/244
https://bugzilla.redhat.com/show_bug.cgi?id=1546805
(cherry picked from commit 8af4f76638)
2018-11-20 15:28:00 +01:00
Beniamino Galvani
3af068a5de cli: wait for changed signal after updating a connection
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)
2018-11-20 15:27:36 +01:00
Beniamino Galvani
d09e33f38c cli: editor: reload secrets after updating connection
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)
2018-11-20 15:27:34 +01:00
Beniamino Galvani
c3233a6c17 libnm-core: macsec: don't require a cak in verify()
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)
2018-11-20 15:27:30 +01:00
Thomas Haller
d5cf3a9776 libnm/macsec: tighten up verification and normalize mka_cak/mka_ckn properties
(cherry picked from commit 474a0dbfbe)
2018-11-20 15:27:22 +01:00
Lubomir Rintel
8ff9a51ca1 rpm: add a bundled(systemd) provide
Required by Fedora Packaging Standards [1] and also really useful.

[1] https://fedoraproject.org/wiki/Bundled_Libraries?rd=Packaging:Bundled_Libraries#Requirement_if_you_bundle

https://github.com/NetworkManager/NetworkManager/pull/247
https://bugzilla.redhat.com/show_bug.cgi?id=1645462

(cherry picked from commit adc0150e7f)
2018-11-16 09:55:47 +01:00
Thomas Haller
f92d2d7766 contrib/rpm: don't depend NetworkManager-ovs package on openvswitch on RHEL
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=1629178
https://bugzilla.redhat.com/show_bug.cgi?id=1633190
(cherry picked from commit 669bd33022)
2018-11-16 09:52:30 +01:00
Benjamin Berg
bbef6cf44a systemd: fix crash by unrefing event sources before re-adding them
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)
2018-11-15 14:51:05 +01:00
Thomas Haller
aa7acb0ae3 build: avoid header conflict for <linux/if.h> and <net/if.h> with "nm-platform.h"
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)
2018-11-14 14:39:10 +01:00
Thomas Haller
8f4e4cc54f dhcp: merge branch 'th/dhcp-client-id-mac-rh1640494'
https://github.com/NetworkManager/NetworkManager/pull/243

(cherry picked from commit 2799634082)
2018-11-14 14:18:54 +01:00
Thomas Haller
62a5ff605b dhcp: initialize hostname as construct-property
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)
2018-11-14 14:18:06 +01:00
Thomas Haller
22ffd02f87 core/trivial: add code comment about DHCP client-id/DUID
(cherry picked from commit e9630e7d57)
2018-11-14 14:18:06 +01:00
Thomas Haller
525830a4a0 dhcp: add "ipv4.dhcp-client-id=duid" setting
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)
2018-11-14 14:18:06 +01:00
Thomas Haller
cb50647144 dhcp: always require hwaddr in internal DHCP clint ip6_start()
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)
2018-11-14 14:18:06 +01:00
Thomas Haller
0983f14be6 doc: add hint about ipv4.dhcp-client-id=stable
(cherry picked from commit 5ef93c3323)
2018-11-14 14:18:06 +01:00
Thomas Haller
f70e762a4f all: add "${MAC}" substituion for "connection.stable-id"
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)
2018-11-14 14:18:06 +01:00
Thomas Haller
a31edf0b3e dhcp: cleanup error handling in internal DHCP client's start
- 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)
2018-11-14 14:18:06 +01:00
Thomas Haller
b21652b8d0 dhcp: don't load IPv4 client-id from lease file
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)
2018-11-14 14:18:06 +01:00
Thomas Haller
d54b444cb4 dhcp: cleanup initializing IPv4 client-id for internal DHCP
- 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)
2018-11-14 14:18:06 +01:00
Thomas Haller
71e3db4f56 dhcp/trivial: wrap lines in calling client_start()
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)
2018-11-14 14:18:06 +01:00
Thomas Haller
1fecb5ec9d dhcp: minor refactoring return paths in NMDhcpDhclient.get_duid()
(cherry picked from commit d6d2b7296f)
2018-11-14 14:18:06 +01:00