Bug #16948

Deal with NetworkManager 1.20+ defaulting to its internal DHCP client

Added by intrigeri 2019-08-08 07:46:28 . Updated 2020-05-14 07:10:19 .

Status:
Confirmed
Priority:
Normal
Assignee:
Category:
Target version:
Start date:
Due date:
% Done:

0%

Feature Branch:
wip/feature/stretch-no-dhclient
Type of work:
Research
Blueprint:

Starter:
Affected tool:
Deliverable for:

Description

According to https://blogs.gnome.org/lkundrak/2019/08/06/networkmanager-1-20/, “The internal default DHCP client is used by default. It’s smaller and faster than dhclient, which was used previously. NetworkManager supports multiple different DHCP clients and the users and distributors have an option to modify the default”. We have a custom config for the traditional isc-dhcp-client that e.g. avoids leaking the Tails’ hostname on the network. Can we port that config to NM’s internal DHCP client? Or should we simply revert to using isc-dhcp-client?

Corresponding section of our design doc is “DHCP hostname leaks”.


Subtasks


Related issues

Related to Tails - Bug #11630: Check what to do wrt. NetworkManager's internal DHCP client vs. isc-dhcp-client Resolved 2016-08-11
Blocks Tails - Feature #16209: Core work: Foundations Team Confirmed

History

#1 Updated by intrigeri 2019-08-08 07:46:40

#2 Updated by intrigeri 2019-08-18 11:29:59

  • related to Bug #11630: Check what to do wrt. NetworkManager's internal DHCP client vs. isc-dhcp-client added

#3 Updated by intrigeri 2019-08-18 11:29:59

  • Feature Branch set to wip/feature/stretch-no-dhclient

#4 Updated by intrigeri 2020-05-04 14:17:36

  • Description updated

#5 Updated by xmunoz 2020-05-12 19:15:48

Here is a test that we ran using Network Manager on Debian sid:
1. Let network manager automatically connect to internet
2. Confirm that hostname is leaked by default using tcpdump and grep

$ tcpdump -n -U --immediate-mode -i ens3 -v udp port 67 or udp port 68 -v


3. Disable DHCP send hostname parameter

$ nmcli con modify Wired\ connection\ 1 ipv4.dhcp-send-hostname no


4. Confirm that hostname is no longer leaked using tcpdump command above

Now this must be configured by default for all interfaces. The pre-up dispatcher script described here might be a good option for ensuring that: https://developer.gnome.org/NetworkManager/stable/NetworkManager.html

#6 Updated by intrigeri 2020-05-13 09:14:09

Thanks for summing this up!

> Now this must be configured by default for all interfaces.

Indeed :)

> The pre-up dispatcher script described here might be a good option for ensuring that: https://developer.gnome.org/NetworkManager/stable/NetworkManager.html

There I read “The interface is connected to the network but is not yet fully activated”. Pessimistic-me suspects that “connected to the network” means “the DHCP negotiation was done already” so it may be too late for our needs.
I suppose real-world testing and/or code reading will answer this, and hopefully prove my pessimistic hunch wrong :)

#7 Updated by anonym 2020-05-13 12:31:27

intrigeri wrote:
> > The pre-up dispatcher script described here might be a good option for ensuring that: https://developer.gnome.org/NetworkManager/stable/NetworkManager.html
>
> There I read “The interface is connected to the network but is not yet fully activated”. Pessimistic-me suspects that “connected to the network” means “the DHCP negotiation was done already” so it may be too late for our needs.

Pessimistic you is correct. :/ I put a script in /etc/NetworkManager/dispatcher.d/pre-up.d and it ran after DHCP had leaked the hostname.

I also had a quick look at the NetworkManager sources, and skimming libnm-core/nm-setting-ip-config.c tells me that this option is hard-coded to default to TRUE, so there is no way to change this without changing the code.

However, since we already patch NetworkManager we could as well switch to this patch:

--- a/libnm-core/nm-setting-ip-config.c
+++ b/libnm-core/nm-setting-ip-config.c
@@ -5814,7 +5814,7 @@ nm_setting_ip_config_class_init (NMSettingIPConfigClass *klass)
     **/
    obj_properties[PROP_DHCP_SEND_HOSTNAME] =
        g_param_spec_boolean (NM_SETTING_IP_CONFIG_DHCP_SEND_HOSTNAME, "", "",
-                             TRUE,
+                             FALSE,
                              G_PARAM_READWRITE |
                              G_PARAM_STATIC_STRINGS);

But long-term it seems we either have to upstream a way to change this default, or add a pre-pre-up event that runs even earlier than pre-up, in particular before DHCP, that we can hook into and fix the setting with nmcli.

#8 Updated by intrigeri 2020-05-14 07:10:19

> However, since we already patch NetworkManager we could as well switch to this patch:

It would not change our maintenance running cost on the NetworkManager package front, but it would:

  • get rid of a dangerous piece of software talking to the network as root
  • simplify our code base by removing a bunch of hard to understand hacks in tails.git

So this seems a good incremental improvement ⇒ worth trying for 5.0 IMO :)

> But long-term it seems we either have to upstream a way to change this default,

It would be nice indeed.
Now, given the ongoing maintenance cost for us so far has been pretty low, and likely we would have to write the upstream code ourselves, I would not say “we have to”: I’m pretty sure we would not see a good ROI for this work in the next decades ;)

> or add a pre-pre-up event that runs even earlier than pre-up, in particular before DHCP, that we can hook into and fix the setting with nmcli.

FWIW, I’m not a fan, due to the combination of these 2 reasons:

  • In terms of Tails design: modifying a NM connection via nmcli while NM is activating it seems brittle design to me: it feels like modifying the branch you’re sitting on. I suspect this could work initially and then break with a future NM upgrade, due to locking, caching, etc.
  • Trying to put myself in upstream’s shoes: adding such a generic facility is a pretty big hammer, with significant long-term impact, that I doubt the case at hand can justify.