dhcp/nettools: better track currently granted lease

When we accept/decline a lease, then that only works if we are in state
GRANTED. n-dhcp4 API also requires us, to provide the exact lease, that
we were announced earlier.

As such, we need to make sure that we don't accept/decline in the wrong
state. That means, to keep track of what we are doing more carefully.

The functions _dhcp_client_accept()/_dhcp_client_decline() now take
a l3cd argument, the one that we announced earlier. And we check that it
still matches.
This commit is contained in:
Thomas Haller
2022-05-19 18:59:45 +02:00
parent 4a256092ee
commit 52a0fe584c
3 changed files with 93 additions and 37 deletions

View File

@@ -73,11 +73,13 @@ G_DEFINE_ABSTRACT_TYPE(NMDhcpClient, nm_dhcp_client, G_TYPE_OBJECT)
/*****************************************************************************/
static gboolean _dhcp_client_accept(NMDhcpClient *self, GError **error);
static gboolean _dhcp_client_accept(NMDhcpClient *self, const NML3ConfigData *l3cd, GError **error);
static gboolean _dhcp_client_can_accept(NMDhcpClient *self);
_nm_unused static gboolean
_dhcp_client_decline(NMDhcpClient *self, const char *error_message, GError **error);
_nm_unused static gboolean _dhcp_client_decline(NMDhcpClient *self,
const NML3ConfigData *l3cd,
const char *error_message,
GError **error);
static void
l3_cfg_notify_cb(NML3Cfg *l3cfg, const NML3ConfigNotifyData *notify_data, NMDhcpClient *self);
@@ -482,18 +484,19 @@ nm_dhcp_client_stop_watch_child(NMDhcpClient *self, pid_t pid)
}
static gboolean
_dhcp_client_accept(NMDhcpClient *self, GError **error)
_dhcp_client_accept(NMDhcpClient *self, const NML3ConfigData *l3cd, GError **error)
{
NMDhcpClientPrivate *priv;
g_return_val_if_fail(NM_IS_DHCP_CLIENT(self), FALSE);
nm_assert(l3cd);
priv = NM_DHCP_CLIENT_GET_PRIVATE(self);
g_return_val_if_fail(priv->l3cd, FALSE);
if (NM_DHCP_CLIENT_GET_CLASS(self)->accept) {
return NM_DHCP_CLIENT_GET_CLASS(self)->accept(self, error);
return NM_DHCP_CLIENT_GET_CLASS(self)->accept(self, l3cd, error);
}
return TRUE;
@@ -514,18 +517,22 @@ _dhcp_client_can_accept(NMDhcpClient *self)
}
static gboolean
_dhcp_client_decline(NMDhcpClient *self, const char *error_message, GError **error)
_dhcp_client_decline(NMDhcpClient *self,
const NML3ConfigData *l3cd,
const char *error_message,
GError **error)
{
NMDhcpClientPrivate *priv;
g_return_val_if_fail(NM_IS_DHCP_CLIENT(self), FALSE);
nm_assert(l3cd);
priv = NM_DHCP_CLIENT_GET_PRIVATE(self);
g_return_val_if_fail(priv->l3cd, FALSE);
if (NM_DHCP_CLIENT_GET_CLASS(self)->decline) {
return NM_DHCP_CLIENT_GET_CLASS(self)->decline(self, error_message, error);
return NM_DHCP_CLIENT_GET_CLASS(self)->decline(self, l3cd, error_message, error);
}
return TRUE;
@@ -650,9 +657,9 @@ l3_cfg_notify_cb(NML3Cfg *l3cfg, const NML3ConfigNotifyData *notify_data, NMDhcp
priv->l3cfg_notify.wait_dhcp_commit = FALSE;
l3_cfg_notify_check_connected(self);
_LOGD("accept address");
_LOGD("accept lease");
if (!_dhcp_client_accept(self, &error)) {
if (!_dhcp_client_accept(self, priv->l3cd, &error)) {
gs_free char *reason = g_strdup_printf("error accepting lease: %s", error->message);
_emit_notify(self,

View File

@@ -200,9 +200,12 @@ typedef struct {
gboolean (*ip4_start)(NMDhcpClient *self, GError **error);
gboolean (*accept)(NMDhcpClient *self, GError **error);
gboolean (*accept)(NMDhcpClient *self, const NML3ConfigData *l3cd, GError **error);
gboolean (*decline)(NMDhcpClient *self, const char *error_message, GError **error);
gboolean (*decline)(NMDhcpClient *self,
const NML3ConfigData *l3cd,
const char *error_message,
GError **error);
gboolean (*ip6_start)(NMDhcpClient *self, const struct in6_addr *ll_addr, GError **error);

View File

@@ -52,7 +52,12 @@ typedef struct _NMDhcpNettoolsClass NMDhcpNettoolsClass;
typedef struct {
NDhcp4Client *client;
NDhcp4ClientProbe *probe;
struct {
NDhcp4ClientLease *lease;
const NML3ConfigData *lease_l3cd;
} granted;
GSource *event_source;
char *lease_file;
} NMDhcpNettoolsPrivate;
@@ -864,15 +869,19 @@ lease_save(NMDhcpNettools *self, NDhcp4ClientLease *lease, const char *lease_fil
}
static void
bound4_handle(NMDhcpNettools *self, NDhcp4ClientLease *lease, gboolean extended)
bound4_handle(NMDhcpNettools *self, guint event, NDhcp4ClientLease *lease)
{
NMDhcpNettoolsPrivate *priv = NM_DHCP_NETTOOLS_GET_PRIVATE(self);
NMDhcpClient *client = NM_DHCP_CLIENT(self);
const NMDhcpClientConfig *client_config;
nm_auto_unref_l3cd_init NML3ConfigData *l3cd = NULL;
GError *error = NULL;
gs_free_error GError *error = NULL;
nm_assert(NM_IN_SET(event, N_DHCP4_CLIENT_EVENT_GRANTED, N_DHCP4_CLIENT_EVENT_EXTENDED));
nm_assert(lease);
_LOGT("lease available (%s)", (event == N_DHCP4_CLIENT_EVENT_GRANTED) ? "granted" : "extended");
_LOGT("lease available (%s)", extended ? "extended" : "new");
client_config = nm_dhcp_client_get_config(client);
l3cd = lease_to_ip4_config(nm_dhcp_client_get_multi_idx(client),
client_config->iface,
@@ -881,16 +890,24 @@ bound4_handle(NMDhcpNettools *self, NDhcp4ClientLease *lease, gboolean extended)
&error);
if (!l3cd) {
_LOGW("failure to parse lease: %s", error->message);
g_clear_error(&error);
if (event == N_DHCP4_CLIENT_EVENT_GRANTED)
n_dhcp4_client_lease_decline(lease, "invalid lease");
_nm_dhcp_client_notify(NM_DHCP_CLIENT(self), NM_DHCP_CLIENT_EVENT_TYPE_FAIL, NULL);
return;
}
if (event == N_DHCP4_CLIENT_EVENT_GRANTED) {
priv->granted.lease = n_dhcp4_client_lease_ref(lease);
priv->granted.lease_l3cd = nm_l3_config_data_ref(l3cd);
} else
lease_save(self, lease, priv->lease_file);
_nm_dhcp_client_notify(NM_DHCP_CLIENT(self),
extended ? NM_DHCP_CLIENT_EVENT_TYPE_EXTENDED
: NM_DHCP_CLIENT_EVENT_TYPE_BOUND,
event == N_DHCP4_CLIENT_EVENT_GRANTED
? NM_DHCP_CLIENT_EVENT_TYPE_BOUND
: NM_DHCP_CLIENT_EVENT_TYPE_EXTENDED,
l3cd);
}
@@ -906,6 +923,16 @@ dhcp4_event_handle(NMDhcpNettools *self, NDhcp4ClientEvent *event)
_LOGT("client event %d", event->event);
client_config = nm_dhcp_client_get_config(NM_DHCP_CLIENT(self));
if (!NM_IN_SET(event->event, N_DHCP4_CLIENT_EVENT_LOG)) {
/* In almost all events (even those that we don't expect below), we clear
* the currently granted lease. That is, because in GRANTED state we
* expect to follow up with accept/decline, and that only works while
* we are still in the same state. Transitioning away to another state
* (on most events) will invalidate that. */
nm_clear_pointer(&priv->granted.lease, n_dhcp4_client_lease_unref);
nm_clear_l3cd(&priv->granted.lease_l3cd);
}
switch (event->event) {
case N_DHCP4_CLIENT_EVENT_OFFER:
r = n_dhcp4_client_lease_get_server_identifier(event->offer.lease, &server_id);
@@ -934,11 +961,10 @@ dhcp4_event_handle(NMDhcpNettools *self, NDhcp4ClientEvent *event)
_nm_dhcp_client_notify(NM_DHCP_CLIENT(self), NM_DHCP_CLIENT_EVENT_TYPE_FAIL, NULL);
break;
case N_DHCP4_CLIENT_EVENT_GRANTED:
priv->lease = n_dhcp4_client_lease_ref(event->granted.lease);
bound4_handle(self, event->granted.lease, FALSE);
bound4_handle(self, event->event, event->granted.lease);
break;
case N_DHCP4_CLIENT_EVENT_EXTENDED:
bound4_handle(self, event->extended.lease, TRUE);
bound4_handle(self, event->event, event->extended.lease);
break;
case N_DHCP4_CLIENT_EVENT_DOWN:
/* ignore down events, they are purely informational */
@@ -1098,46 +1124,65 @@ nettools_create(NMDhcpNettools *self, GError **error)
}
static gboolean
_accept(NMDhcpClient *client, GError **error)
_accept(NMDhcpClient *client, const NML3ConfigData *l3cd, GError **error)
{
NMDhcpNettools *self = NM_DHCP_NETTOOLS(client);
NMDhcpNettoolsPrivate *priv = NM_DHCP_NETTOOLS_GET_PRIVATE(self);
int r;
g_return_val_if_fail(priv->lease, FALSE);
_LOGT("accept");
r = n_dhcp4_client_lease_accept(priv->lease);
g_return_val_if_fail(l3cd, FALSE);
if (priv->granted.lease_l3cd != l3cd) {
nm_utils_error_set(error, NM_UTILS_ERROR_UNKNOWN, "calling accept in unexpected state");
return FALSE;
}
nm_assert(priv->granted.lease);
r = n_dhcp4_client_lease_accept(priv->granted.lease);
nm_clear_pointer(&priv->granted.lease, n_dhcp4_client_lease_unref);
nm_clear_l3cd(&priv->granted.lease_l3cd);
if (r) {
set_error_nettools(error, r, "failed to accept lease");
return FALSE;
}
priv->lease = n_dhcp4_client_lease_unref(priv->lease);
return TRUE;
}
static gboolean
decline(NMDhcpClient *client, const char *error_message, GError **error)
decline(NMDhcpClient *client, const NML3ConfigData *l3cd, const char *error_message, GError **error)
{
NMDhcpNettools *self = NM_DHCP_NETTOOLS(client);
NMDhcpNettoolsPrivate *priv = NM_DHCP_NETTOOLS_GET_PRIVATE(self);
int r;
nm_auto(n_dhcp4_client_lease_unrefp) NDhcp4ClientLease *lease = NULL;
g_return_val_if_fail(priv->lease, FALSE);
_LOGT("decline (%s)", error_message);
_LOGT("dhcp4-client: decline (%s)", error_message);
g_return_val_if_fail(l3cd, FALSE);
if (priv->granted.lease_l3cd != l3cd) {
nm_utils_error_set(error, NM_UTILS_ERROR_UNKNOWN, "calling decline in unexpected state");
return FALSE;
}
nm_assert(priv->granted.lease);
lease = g_steal_pointer(&priv->granted.lease);
nm_clear_l3cd(&priv->granted.lease_l3cd);
r = n_dhcp4_client_lease_decline(lease, error_message);
r = n_dhcp4_client_lease_decline(priv->lease, error_message);
if (r) {
set_error_nettools(error, r, "failed to decline lease");
return FALSE;
}
priv->lease = n_dhcp4_client_lease_unref(priv->lease);
return TRUE;
}
@@ -1348,7 +1393,8 @@ dispose(GObject *object)
nm_clear_g_free(&priv->lease_file);
nm_clear_g_source_inst(&priv->event_source);
nm_clear_pointer(&priv->lease, n_dhcp4_client_lease_unref);
nm_clear_pointer(&priv->granted.lease, n_dhcp4_client_lease_unref);
nm_clear_l3cd(&priv->granted.lease_l3cd);
nm_clear_pointer(&priv->probe, n_dhcp4_client_probe_free);
nm_clear_pointer(&priv->client, n_dhcp4_client_unref);