Merge b5171f0f48 into b7eefa1996

CID#1565824

Follow-up for f6793bbcf0

man/systemd.network.xml

@@ -2546,6 +2546,17 @@ NFTSet=prefix:netdev:filter:eth_ipv4_prefix</programlisting>
         </listitem>
       </varlistentry>
 
+      <varlistentry>
+        <term><varname>BOOTP=</varname></term>
+        <listitem>
+          <para>Takes a boolean. The DHCPv4 client can be configured to communicate with BOOP servers that
+          don't accept Option 53, DHCP Message Type. In this configuration, a BOOTP Request is sent without
+          any options by default. A BOOTP reply that contains Option 1: Subnet Mask is expected.</para>
+
+          <xi:include href="version-info.xml" xpointer="v257"/>
+        </listitem>
+      </varlistentry>
+
       <!-- How to use the DHCP lease -->
 
       <varlistentry>

src/basic/cgroup-util.c

@@ -799,7 +799,7 @@ int cg_pid_get_path(const char *controller, pid_t pid, char **ret_path) {
                                 continue;
                 }
 
-                char *path = strdup(e + 1);
+                _cleanup_free_ char *path = strdup(e + 1);
                 if (!path)
                         return -ENOMEM;
 
@@ -812,7 +812,7 @@ int cg_pid_get_path(const char *controller, pid_t pid, char **ret_path) {
                 if (e)
                         *e = 0;
 
-                *ret_path = path;
+                *ret_path = TAKE_PTR(path);
                 return 0;
         }
 }

src/libsystemd-network/dhcp-packet.c

@@ -14,19 +14,13 @@
 
 #define DHCP_CLIENT_MIN_OPTIONS_SIZE            312
 
-int dhcp_message_init(
+int bootp_message_init(
                 DHCPMessage *message,
                 uint8_t op,
                 uint32_t xid,
-                uint8_t type,
                 uint16_t arp_type,
                 uint8_t hlen,
-                const uint8_t *chaddr,
-                size_t optlen,
-                size_t *optoffset) {
-
-        size_t offset = 0;
-        int r;
+                const uint8_t *chaddr) {
 
         assert(IN_SET(op, BOOTREQUEST, BOOTREPLY));
         assert(chaddr || hlen == 0);
@@ -51,6 +45,27 @@ int dhcp_message_init(
         message->xid = htobe32(xid);
         message->magic = htobe32(DHCP_MAGIC_COOKIE);
 
+        return 0;
+}
+
+int dhcp_message_init(
+                DHCPMessage *message,
+                uint8_t op,
+                uint32_t xid,
+                uint8_t type,
+                uint16_t arp_type,
+                uint8_t hlen,
+                const uint8_t *chaddr,
+                size_t optlen,
+                size_t *optoffset) {
+
+        size_t offset = 0;
+        int r;
+
+        r = bootp_message_init(message, op, xid, arp_type, hlen, chaddr);
+        if (r < 0)
+                return r;
+
         r = dhcp_option_append(message, optlen, &offset, 0,
                                SD_DHCP_OPTION_MESSAGE_TYPE, 1, &type);
         if (r < 0)

src/libsystemd-network/dhcp-packet.h

@@ -6,6 +6,14 @@
 
 #include "dhcp-protocol.h"
 
+int bootp_message_init(
+                DHCPMessage *message,
+                uint8_t op,
+                uint32_t xid,
+                uint16_t arp_type,
+                uint8_t hlen,
+                const uint8_t *chaddr);
+
 int dhcp_message_init(
                 DHCPMessage *message,
                 uint8_t op,

src/libsystemd-network/sd-dhcp-client.c

@@ -105,6 +105,7 @@ struct sd_dhcp_client {
         int socket_priority;
         bool socket_priority_set;
         bool ipv6_acquired;
+        bool bootp;
 };
 
 static const uint8_t default_req_opts[] = {
@@ -656,6 +657,15 @@ int sd_dhcp_client_set_fallback_lease_lifetime(sd_dhcp_client *client, uint64_t
         return 0;
 }
 
+int sd_dhcp_client_set_bootp(sd_dhcp_client *client, int bootp) {
+        assert_return(client, -EINVAL);
+        assert_return(!sd_dhcp_client_is_running(client), -EBUSY);
+
+        client->bootp = bootp;
+
+        return 0;
+}
+
 static void client_set_state(sd_dhcp_client *client, DHCPState state) {
         assert(client);
 
@@ -792,7 +802,11 @@ static int client_message_init(
         packet = malloc0(size);
         if (!packet)
                 return -ENOMEM;
-
+        if (client->bootp) {
+                optoffset = 0;
+                r = bootp_message_init(&packet->dhcp, BOOTREQUEST, client->xid, client->arp_type,
+                                       client->hw_addr.length, client->hw_addr.bytes);
+        } else
                 r = dhcp_message_init(&packet->dhcp, BOOTREQUEST, client->xid, type,
                                       client->arp_type, client->hw_addr.length, client->hw_addr.bytes,
                                       optlen, &optoffset);
@@ -825,6 +839,7 @@ static int client_message_init(
         if (client->request_broadcast || client->arp_type != ARPHRD_ETHER)
                 packet->dhcp.flags = htobe16(0x8000);
 
+        if (!client->bootp) {
                 /* Some DHCP servers will refuse to issue an DHCP lease if the Client
                    Identifier option is not set */
                 r = dhcp_option_append(&packet->dhcp, optlen, &optoffset, 0,
@@ -833,6 +848,7 @@ static int client_message_init(
                                        client->client_id.raw);
                 if (r < 0)
                         return r;
+        }
 
         /* RFC2131 section 3.5:
            in its initial DHCPDISCOVER or DHCPREQUEST message, a
@@ -1061,6 +1077,22 @@ static int client_send_discover(sd_dhcp_client *client) {
         if (r < 0)
                 return r;
 
+        /* RFC1542 section 3.5:
+           if the client has no information to communicate to the server,
+           the octet immediately following the magic cookie SHOULD be set
+           to the "End" tag (255) and the remaining octets of the 'vend'
+           field SHOULD be set to zero.
+         */
+        /* Use this RFC, along with the fact that some BOOTP servers require
+           a 64-byte vend field, to suggest that we always zero and send 64
+           bytes in the options field.
+         */
+        if (client->bootp)
+                if (optoffset < 64 && optlen >= 64) {
+                        memset(&discover->dhcp.options[optoffset], 0, optlen - optoffset);
+                        optoffset = 64;
+                }
+
         /* We currently ignore:
            The client SHOULD wait a random time between one and ten seconds to
            desynchronize the use of DHCP at startup.
@@ -1509,16 +1541,18 @@ static int client_parse_message(
         }
 
         r = dhcp_option_parse(message, len, dhcp_lease_parse_options, lease, &error_message);
-        if (r < 0)
+        if (r == -ENOMSG && client->bootp)
+                r = DHCP_ACK; /* BOOTP messages don't have a DHCP message type option */
+        else if (r < 0)
                 return log_dhcp_client_errno(client, r, "Failed to parse DHCP options, ignoring: %m");
 
         switch (client->state) {
         case DHCP_STATE_SELECTING:
                 if (r == DHCP_ACK) {
-                        if (!client->rapid_commit)
+                        if (!client->rapid_commit && !client->bootp)
                                 return log_dhcp_client_errno(client, SYNTHETIC_ERRNO(ENOMSG),
                                                              "received unexpected ACK, ignoring.");
-                        if (!lease->rapid_commit)
+                        if (!lease->rapid_commit && !client->bootp)
                                 return log_dhcp_client_errno(client, SYNTHETIC_ERRNO(ENOMSG),
                                                              "received rapid ACK without Rapid Commit option, ignoring.");
                 } else if (r == DHCP_OFFER) {
@@ -1561,11 +1595,17 @@ static int client_parse_message(
         lease->next_server = message->siaddr;
         lease->address = message->yiaddr;
 
+        if (client->bootp)
+                lease->lifetime = USEC_INFINITY;
+
+        if (lease->server_address == 0 && !client->bootp)
+                return log_dhcp_client_errno(client, SYNTHETIC_ERRNO(ENOMSG),
+                                             "received lease lacks server address, ignoring.");
+
         if (lease->address == 0 ||
-            lease->server_address == 0 ||
             lease->lifetime == 0)
                 return log_dhcp_client_errno(client, SYNTHETIC_ERRNO(ENOMSG),
-                                             "received lease lacks address, server address or lease lifetime, ignoring.");
+                                             "received lease lacks address or lease lifetime, ignoring.");
 
         r = dhcp_lease_set_default_subnet_mask(lease);
         if (r < 0)
@@ -1601,7 +1641,7 @@ static int client_handle_offer_or_rapid_ack(sd_dhcp_client *client, DHCPMessage
 
         dhcp_lease_unref_and_replace(client->lease, lease);
 
-        if (client->lease->rapid_commit) {
+        if (client->lease->rapid_commit || client->bootp) {
                 log_dhcp_client(client, "ACK");
                 return SD_DHCP_CLIENT_EVENT_IP_ACQUIRE;
         }
@@ -2007,8 +2047,8 @@ static int client_handle_message(sd_dhcp_client *client, DHCPMessage *message, s
                 if (r < 0)
                         return 0; /* invalid message, let's ignore it */
 
-                if (client->lease->rapid_commit)
-                        /* got a successful rapid commit */
+                if (client->lease->rapid_commit || client->bootp)
+                        /* got a successful rapid commit or bootp reply */
                         return client_enter_bound(client, r);
 
                 return client_enter_requesting(client);
@@ -2225,7 +2265,7 @@ int sd_dhcp_client_send_release(sd_dhcp_client *client) {
         size_t optoffset, optlen;
         int r;
 
-        if (!sd_dhcp_client_is_running(client) || !client->lease)
+        if (!sd_dhcp_client_is_running(client) || !client->lease || client->bootp)
                 return 0; /* do nothing */
 
         r = client_message_init(client, &release, DHCP_RELEASE, &optlen, &optoffset);

src/libsystemd-network/test-dhcp-client.c

@@ -532,6 +532,145 @@ static void test_addr_acq(sd_event *e) {
         xid = 0;
 }
 
+static uint8_t test_addr_bootp_reply[] = {
+        0x45, 0x00, 0x01, 0x48, 0x00, 0x00, 0x40, 0x00,
+        0xff, 0x11, 0x70, 0xa3, 0x0a, 0x00, 0x00, 0x02,
+        0xff, 0xff, 0xff, 0xff, 0x00, 0x43, 0x00, 0x44,
+        0x01, 0x2c, 0x2b, 0x91, 0x02, 0x01, 0x06, 0x00,
+        0x69, 0xd3, 0x79, 0x11, 0x17, 0x00, 0x80, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x0a, 0x46, 0x00, 0x02,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x50, 0x2d, 0xf4, 0x1f, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x63, 0x82, 0x53, 0x63, 0x01, 0x04, 0xff, 0x00,
+        0x00, 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+        0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+        0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+        0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+        0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+        0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+};
+
+static int test_bootp_acquired(sd_dhcp_client *client, int event,
+                               void *userdata) {
+        sd_event *e = userdata;
+        sd_dhcp_lease *lease;
+        struct in_addr addr;
+
+        assert_se(client);
+        assert_se(IN_SET(event, SD_DHCP_CLIENT_EVENT_IP_ACQUIRE, SD_DHCP_CLIENT_EVENT_SELECTING));
+
+        assert_se(sd_dhcp_client_get_lease(client, &lease) >= 0);
+        assert_se(lease);
+
+        assert_se(sd_dhcp_lease_get_address(lease, &addr) >= 0);
+        assert_se(memcmp(&addr.s_addr, &test_addr_bootp_reply[44],
+                      sizeof(addr.s_addr)) == 0);
+
+        assert_se(sd_dhcp_lease_get_netmask(lease, &addr) >= 0);
+        assert_se(memcmp(&addr.s_addr, &test_addr_bootp_reply[270],
+                      sizeof(addr.s_addr)) == 0);
+
+        if (verbose)
+                log_info("  BOOTP address acquired");
+
+        sd_event_exit(e, 0);
+
+        return 0;
+}
+
+static int test_bootp_recv_request(size_t size, DHCPMessage *request) {
+        uint16_t udp_check = 0;
+        int res;
+
+        xid = request->xid;
+
+        if (verbose)
+                log_info("  recv BOOTP Request  0x%08x", be32toh(xid));
+
+        callback_recv = NULL;
+
+        memcpy(&test_addr_bootp_reply[26], &udp_check, sizeof(udp_check));
+        memcpy(&test_addr_bootp_reply[32], &xid, sizeof(xid));
+        memcpy(&test_addr_bootp_reply[56], hw_addr.bytes, hw_addr.length);
+
+        res = write(test_fd[1], test_addr_bootp_reply,
+                    sizeof(test_addr_bootp_reply));
+        assert_se(res == sizeof(test_addr_bootp_reply));
+
+        if (verbose)
+                log_info("  sent BOOTP Reply");
+
+        return 0;
+};
+
+static void test_acquire_bootp(sd_event *e) {
+        sd_dhcp_client *client;
+        int res, r;
+
+        if (verbose)
+                log_info("* %s", __func__);
+
+        r = sd_dhcp_client_new(&client, false);
+        assert_se(r >= 0);
+        assert_se(client);
+
+        r = sd_dhcp_client_attach_event(client, e, 0);
+        assert_se(r >= 0);
+
+        r = sd_dhcp_client_set_bootp(client, true);
+        assert_se(r >= 0);
+
+        assert_se(sd_dhcp_client_set_ifindex(client, 42) >= 0);
+        assert_se(sd_dhcp_client_set_mac(client, hw_addr.bytes, bcast_addr.bytes, hw_addr.length, ARPHRD_ETHER) >= 0);
+
+        assert_se(sd_dhcp_client_set_callback(client, test_bootp_acquired, e) >= 0);
+
+        callback_recv = test_bootp_recv_request;
+
+        assert_se(sd_event_add_time_relative(e, NULL, CLOCK_BOOTTIME,
+                                             30 * USEC_PER_SEC, 0,
+                                             NULL, INT_TO_PTR(-ETIMEDOUT)) >= 0);
+
+        res = sd_dhcp_client_start(client);
+        assert_se(IN_SET(res, 0, -EINPROGRESS));
+
+        assert_se(sd_event_loop(e) >= 0);
+
+        assert_se(sd_dhcp_client_set_callback(client, NULL, NULL) >= 0);
+        assert_se(sd_dhcp_client_stop(client) >= 0);
+        sd_dhcp_client_unref(client);
+
+        test_fd[1] = safe_close(test_fd[1]);
+
+        callback_recv = NULL;
+        xid = 0;
+}
+
 int main(int argc, char *argv[]) {
         _cleanup_(sd_event_unrefp) sd_event *e;
 
@@ -549,6 +688,10 @@ int main(int argc, char *argv[]) {
         test_discover_message(e);
         test_addr_acq(e);
 
+        sd_event_unref(e);
+        assert_se(sd_event_new(&e) >= 0);
+        test_acquire_bootp(e);
+
 #if HAVE_VALGRIND_VALGRIND_H
         /* Make sure the async_close thread has finished.
          * valgrind would report some of the phread_* structures

src/network/networkd-dhcp4.c

@@ -1477,6 +1477,10 @@ static int dhcp4_configure(Link *link) {
         if (r < 0)
                 return log_link_debug_errno(link, r, "DHCPv4 CLIENT: Failed to allocate DHCPv4 client: %m");
 
+        r = sd_dhcp_client_set_bootp(link->dhcp_client, link->network->dhcp_send_bootp);
+        if (r < 0)
+                return log_link_debug_errno(link, r, "DHCPv4 CLIENT: Failed to set BOOTP flag: %m");
+
         r = sd_dhcp_client_attach_event(link->dhcp_client, link->manager->event, 0);
         if (r < 0)
                 return log_link_debug_errno(link, r, "DHCPv4 CLIENT: Failed to attach event to DHCPv4 client: %m");

src/network/networkd-link.c

@@ -1443,6 +1443,7 @@ int link_reconfigure_impl(Link *link, LinkReconfigurationFlag flags) {
 }
 
 typedef struct LinkReconfigurationData {
+        Manager *manager;
         Link *link;
         LinkReconfigurationFlag flags;
         sd_bus_message *message;
@@ -1473,6 +1474,12 @@ static void link_reconfiguration_data_destroy_callback(LinkReconfigurationData *
                 }
 
                 if (!data->counter || *data->counter <= 0) {
+                        /* Update the state files before replying the bus method. Otherwise,
+                         * systemd-networkd-wait-online following networkctl reload/reconfigure may read an
+                         * outdated state file and wrongly handle an interface is already in the configured
+                         * state. */
+                        (void) manager_clean_all(data->manager);
+
                         r = sd_bus_reply_method_return(data->message, NULL);
                         if (r < 0)
                                 log_warning_errno(r, "Failed to reply for DBus method, ignoring: %m");
@@ -1521,6 +1528,7 @@ int link_reconfigure_full(Link *link, LinkReconfigurationFlag flags, sd_bus_mess
         }
 
         *data = (LinkReconfigurationData) {
+                .manager = link->manager,
                 .link = link_ref(link),
                 .flags = flags,
                 .message = sd_bus_message_ref(message), /* message may be NULL, but _ref() works fine. */

src/network/networkd-network-gperf.gperf

@@ -244,6 +244,7 @@ DHCPv4.QuickAck,                             config_parse_bool,
 DHCPv4.RequestOptions,                       config_parse_dhcp_request_options,                        AF_INET,                       0
 DHCPv4.Anonymize,                            config_parse_bool,                                        0,                             offsetof(Network, dhcp_anonymize)
 DHCPv4.SendHostname,                         config_parse_dhcp_send_hostname,                          AF_INET,                       0
+DHCPv4.BOOTP,                                config_parse_bool,                                        0,                             offsetof(Network, dhcp_send_bootp)
 DHCPv4.Hostname,                             config_parse_hostname,                                    0,                             offsetof(Network, dhcp_hostname)
 DHCPv4.Label,                                config_parse_dhcp_label,                                  0,                             offsetof(Network, dhcp_label)
 DHCPv4.RequestBroadcast,                     config_parse_tristate,                                    0,                             offsetof(Network, dhcp_broadcast)

src/network/networkd-network.h

@@ -179,6 +179,7 @@ struct Network {
         OrderedHashmap *dhcp_client_send_vendor_options;
         char *dhcp_netlabel;
         NFTSetContext dhcp_nft_set_context;
+        bool dhcp_send_bootp;
 
         /* DHCPv6 Client support */
         bool dhcp6_use_address;

src/systemd/sd-dhcp-client.h

@@ -147,6 +147,9 @@ int sd_dhcp_client_set_socket_priority(
 int sd_dhcp_client_set_fallback_lease_lifetime(
                 sd_dhcp_client *client,
                 uint64_t fallback_lease_lifetime);
+int sd_dhcp_client_set_bootp(
+                sd_dhcp_client *client,
+                int bootp);
 
 int sd_dhcp_client_add_option(sd_dhcp_client *client, sd_dhcp_option *v);
 int sd_dhcp_client_add_vendor_option(sd_dhcp_client *client, sd_dhcp_option *v);

test/test-network/systemd-networkd-tests.py

@@ -6406,11 +6406,11 @@ class NetworkdRATests(unittest.TestCase, Utilities):
 
         for i in [100, 200, 300, 512, 1024, 2048]:
             if i not in [metric_1, metric_2]:
-                self.assertNotIn(f'{i}', output)
+                self.assertNotIn(f'metric {i} ', output)
 
         for i in ['low', 'medium', 'high']:
             if i not in [preference_1, preference_2]:
-                self.assertNotIn(f'{i}', output)
+                self.assertNotIn(f'pref {i}', output)
 
     def test_router_preference(self):
         copy_network_unit('25-veth-client.netdev',