device: restart DHCP when the MAC changes and the client-id is fixed

If the MAC changes there is the possibility that the DHCP client will
not be able to renew the address because it uses the old MAC as
CHADDR. Depending on the implementation, the DHCP server might use
CHADDR (so, the old address) as the destination MAC for DHCP replies,
and those packets will be lost.

To avoid this problem, we need to restart the DHCP client after a MAC
change; that has the disadvantage that the server will give out a
different address, unless the client is sending a explicit
client-id. So, limit the restart to situations where the client-id is
explicitly set to a fixed value.

https://bugzilla.redhat.com/show_bug.cgi?id=2110000
This commit is contained in:
Beniamino Galvani 2022-08-24 16:52:52 +02:00
parent 7f1b1f50fa
commit 959ef7b74f
5 changed files with 27 additions and 17 deletions

View file

@ -451,7 +451,7 @@ supplicant_auth_state_changed(NMSupplicantInterface *iface,
if (state == NM_SUPPLICANT_AUTH_STATE_SUCCESS) {
nm_clear_g_signal_handler(priv->supplicant.iface, &priv->supplicant.iface_state_id);
nm_device_update_dynamic_ip_setup(NM_DEVICE(self), "supplicant auth state changed");
nm_device_update_dynamic_ip_setup(NM_DEVICE(self), FALSE, "supplicant auth state changed");
}
}

View file

@ -6373,7 +6373,7 @@ _dev_unmanaged_check_external_down(NMDevice *self, gboolean only_if_unmanaged, g
}
void
nm_device_update_dynamic_ip_setup(NMDevice *self, const char *reason)
nm_device_update_dynamic_ip_setup(NMDevice *self, gboolean preserve_client_id, const char *reason)
{
NMDevicePrivate *priv;
@ -6388,10 +6388,14 @@ nm_device_update_dynamic_ip_setup(NMDevice *self, const char *reason)
g_hash_table_remove_all(priv->ip6_saved_properties);
if (priv->ipdhcp_data_4.state != NM_DEVICE_IP_STATE_NONE)
_dev_ipdhcpx_restart(self, AF_INET, FALSE);
if (priv->ipdhcp_data_6.state != NM_DEVICE_IP_STATE_NONE)
_dev_ipdhcpx_restart(self, AF_INET6, FALSE);
if (priv->ipdhcp_data_4.state != NM_DEVICE_IP_STATE_NONE) {
if (!preserve_client_id || priv->ipdhcp_data_4.client_id_fixed)
_dev_ipdhcpx_restart(self, AF_INET, FALSE);
}
if (priv->ipdhcp_data_6.state != NM_DEVICE_IP_STATE_NONE) {
if (!preserve_client_id || priv->ipdhcp_data_6.client_id_fixed)
_dev_ipdhcpx_restart(self, AF_INET6, FALSE);
}
if (priv->ipac6_data.ndisc) {
/* FIXME: todo */
@ -6696,6 +6700,7 @@ device_link_changed(gpointer user_data)
NMDeviceClass *klass = NM_DEVICE_GET_CLASS(self);
NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE(self);
gboolean ip_ifname_changed = FALSE;
gboolean hw_addr_changed;
nm_auto_nmpobj const NMPObject *pllink_keep_alive = NULL;
const NMPlatformLink *pllink;
const char *str;
@ -6742,9 +6747,9 @@ device_link_changed(gpointer user_data)
if (ifindex == nm_device_get_ip_ifindex(self))
_stats_update_counters_from_pllink(self, pllink);
had_hw_addr = (priv->hw_addr != NULL);
nm_device_update_hw_address(self);
got_hw_addr = (!had_hw_addr && priv->hw_addr);
had_hw_addr = (priv->hw_addr != NULL);
hw_addr_changed = nm_device_update_hw_address(self);
got_hw_addr = (!had_hw_addr && priv->hw_addr);
nm_device_update_permanent_hw_address(self, FALSE);
if (pllink->name[0] && !nm_streq(priv->iface, pllink->name)) {
@ -6794,7 +6799,9 @@ device_link_changed(gpointer user_data)
/* Update DHCP, etc, if needed */
if (ip_ifname_changed)
nm_device_update_dynamic_ip_setup(self, "IP interface changed");
nm_device_update_dynamic_ip_setup(self, FALSE, "IP interface changed");
else if (hw_addr_changed)
nm_device_update_dynamic_ip_setup(self, TRUE, "hw-address changed");
was_up = priv->up;
priv->up = NM_FLAGS_HAS(pllink->n_ifi_flags, IFF_UP);
@ -6854,7 +6861,7 @@ device_link_changed(gpointer user_data)
* renew DHCP leases and such.
*/
if (priv->state == NM_DEVICE_STATE_ACTIVATED) {
nm_device_update_dynamic_ip_setup(self, "interface got carrier");
nm_device_update_dynamic_ip_setup(self, FALSE, "interface got carrier");
}
}
@ -6916,7 +6923,7 @@ device_ip_link_changed(gpointer user_data)
priv->ip_iface_ = g_strdup(ip_iface);
update_prop_ip_iface(self);
nm_device_update_dynamic_ip_setup(self, "interface renamed");
nm_device_update_dynamic_ip_setup(self, FALSE, "interface renamed");
}
return G_SOURCE_REMOVE;

View file

@ -767,8 +767,9 @@ void nm_device_update_metered(NMDevice *self);
gboolean nm_device_update_hw_address(NMDevice *self);
void nm_device_update_initial_hw_address(NMDevice *self);
void nm_device_update_permanent_hw_address(NMDevice *self, gboolean force_freeze);
void nm_device_update_dynamic_ip_setup(NMDevice *self, const char *reason);
guint nm_device_get_supplicant_timeout(NMDevice *self);
void
nm_device_update_dynamic_ip_setup(NMDevice *self, gboolean preserve_client_id, const char *reason);
guint nm_device_get_supplicant_timeout(NMDevice *self);
gboolean nm_device_auth_retries_try_next(NMDevice *self);

View file

@ -2525,7 +2525,7 @@ supplicant_iface_state(NMDeviceWifi *self,
_LOGD(LOGD_WIFI,
"supplicant state settled after roaming, renew dynamic IP configuration");
nm_clear_g_source_inst(&priv->roam_supplicant_wait_source);
nm_device_update_dynamic_ip_setup(device, "roamed to a different AP");
nm_device_update_dynamic_ip_setup(device, FALSE, "roamed to a different AP");
}
}
break;
@ -2668,7 +2668,9 @@ supplicant_iface_notify_current_bss(NMSupplicantInterface *iface,
if (nm_supplicant_interface_get_state(priv->sup_iface)
== NM_SUPPLICANT_INTERFACE_STATE_COMPLETED) {
nm_device_update_dynamic_ip_setup(NM_DEVICE(self), "roamed to a different AP");
nm_device_update_dynamic_ip_setup(NM_DEVICE(self),
FALSE,
"roamed to a different AP");
} else {
/* Wait that the authentication to new the AP completes before
* trying to renew, otherwise the DHCP REQUEST could be lost

View file

@ -6578,7 +6578,7 @@ do_sleep_wake(NMManager *self, gboolean sleeping_changed)
&& !nm_device_get_unmanaged_flags(device, NM_UNMANAGED_SLEEPING)) {
/* DHCP leases of software devices could have gone stale
* so we need to renew them. */
nm_device_update_dynamic_ip_setup(device, "wake up");
nm_device_update_dynamic_ip_setup(device, FALSE, "wake up");
continue;
}