Blob Blame History Raw
From ec23a125054566b67b6db3a27c6d2127355b1470 Mon Sep 17 00:00:00 2001
From: Beniamino Galvani <bgalvani@redhat.com>
Date: Wed, 29 Apr 2020 18:18:28 +0200
Subject: [PATCH 1/1] n-dhcp4: don't fail dispatch in case of receive errors

Currently any error encountered in n_dhcp4_c_connection_dispatch_io()
causes a dispatch failure and interrupts the library state
machine. The recvmsg() on the socket can fail for different reasons;
one of these is for example that the UDP request previously sent got a
ICMP port-unreachable response. This can be reproduced in the
following way:

 ip netns add ns1
 ip link add veth0 type veth peer name veth1
 ip link set veth1 netns ns1
 ip link set veth0 up

 cat > dhcpd.conf <<EOF
 server-identifier 172.25.0.1;
 max-lease-time 120;
 default-lease-time 120;
 subnet 172.25.0.0 netmask 255.255.255.0 {
        range 172.25.0.100 172.25.0.200;
 }
 EOF

 ip -n ns1 link set veth1 up
 ip -n ns1 address add dev veth1 172.25.0.1/24
 ip netns exec ns1 iptables -A INPUT -p udp --dport 67 -j REJECT
 ip netns exec ns1 dhcpd -4 -cf dhcpd.conf -pf /tmp/dhcp-server.pid

If a client is started on veth0, it is able to obtain a lease despite
the firewall rule blocking DHCP, because dhcpd uses a packet
socket. Then it fails during the renewal because the recvmsg() fails:

 dhcp4 (veth0): send REQUEST of 172.25.0.178 to 172.25.0.1
 dhcp4 (veth0): error -111 dispatching events
 dhcp4 (veth0): state changed bound -> fail

The client should consider such errors non fatal and keep running.

https://bugzilla.redhat.com/show_bug.cgi?id=1829178
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/486
(cherry picked from commit c5d1d4c498c50dfc7d2d18b213a117dd1199f1de)
(cherry picked from commit bee01292f886dcff7114dc421983f1d50f1939b0)
(cherry picked from commit f2fdb6710f92d8621667e84485b82a9b712228c7)
---
 shared/n-dhcp4/src/n-dhcp4-c-connection.c | 29 ++++++++++++++++-------
 shared/n-dhcp4/src/n-dhcp4-c-probe.c      |  1 +
 2 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/shared/n-dhcp4/src/n-dhcp4-c-connection.c b/shared/n-dhcp4/src/n-dhcp4-c-connection.c
index a5c8ea66fe9d..30514e286d32 100644
--- a/shared/n-dhcp4/src/n-dhcp4-c-connection.c
+++ b/shared/n-dhcp4/src/n-dhcp4-c-connection.c
@@ -1136,6 +1136,13 @@ int n_dhcp4_c_connection_dispatch_timer(NDhcp4CConnection *connection,
         return 0;
 }
 
+/*
+ * Returns:
+ *  0                     on success
+ *  N_DHCP4_E_MALFORMED   if a malformed packet was received
+ *  N_DHCP4_E_UNEXPECTED  if the packet received contains unexpected data
+ *  N_DHCP4_E_AGAIN       if there was another error (non fatal for the client)
+ */
 int n_dhcp4_c_connection_dispatch_io(NDhcp4CConnection *connection,
                                      NDhcp4Incoming **messagep) {
         _c_cleanup_(n_dhcp4_incoming_freep) NDhcp4Incoming *message = NULL;
@@ -1150,10 +1157,11 @@ int n_dhcp4_c_connection_dispatch_io(NDhcp4CConnection *connection,
                                                  connection->scratch_buffer,
                                                  sizeof(connection->scratch_buffer),
                                                  &message);
-                if (r)
+                if (!r)
+                        break;
+                else if (r == N_DHCP4_E_MALFORMED)
                         return r;
-
-                break;
+                return N_DHCP4_E_AGAIN;
         case N_DHCP4_C_CONNECTION_STATE_DRAINING:
                 r = n_dhcp4_c_socket_packet_recv(connection->fd_packet,
                                                  connection->scratch_buffer,
@@ -1161,8 +1169,10 @@ int n_dhcp4_c_connection_dispatch_io(NDhcp4CConnection *connection,
                                                  &message);
                 if (!r)
                         break;
-                else if (r != N_DHCP4_E_AGAIN)
+                else if (r == N_DHCP4_E_MALFORMED)
                         return r;
+                else if (r != N_DHCP4_E_AGAIN)
+                        return N_DHCP4_E_AGAIN;
 
                 /*
                  * The UDP socket is open and the packet socket has been shut down
@@ -1180,18 +1190,21 @@ int n_dhcp4_c_connection_dispatch_io(NDhcp4CConnection *connection,
                                               connection->scratch_buffer,
                                               sizeof(connection->scratch_buffer),
                                               &message);
-                if (r)
+                if (!r)
+                        break;
+                else if (r == N_DHCP4_E_MALFORMED)
                         return r;
-
-                break;
+                return N_DHCP4_E_AGAIN;
         default:
                 abort();
                 return -ENOTRECOVERABLE;
         }
 
         r = n_dhcp4_c_connection_verify_incoming(connection, message, &type);
-        if (r)
+        if (r == N_DHCP4_E_MALFORMED || r == N_DHCP4_E_UNEXPECTED)
                 return r;
+        else if (r != 0)
+                return N_DHCP4_E_AGAIN;
 
         if (type == N_DHCP4_MESSAGE_OFFER || type == N_DHCP4_MESSAGE_ACK) {
                 n_dhcp4_c_log(connection->client_config, LOG_INFO,
diff --git a/shared/n-dhcp4/src/n-dhcp4-c-probe.c b/shared/n-dhcp4/src/n-dhcp4-c-probe.c
index e4477a7c7472..5e97129834d1 100644
--- a/shared/n-dhcp4/src/n-dhcp4-c-probe.c
+++ b/shared/n-dhcp4/src/n-dhcp4-c-probe.c
@@ -1242,6 +1242,7 @@ int n_dhcp4_client_probe_dispatch_io(NDhcp4ClientProbe *probe, uint32_t events)
                         return 0;
                 }
 
+                abort();
                 return r;
         }
 
-- 
2.26.2