Merge 9bc9b2e7bc into 710653d3bc

With DHCP option concatenation being implemented using a new buffer,
code receives a temporary pointer whose lifetime is now much shorter.
Holding onto pointers to this buffer is invalid as it is freed as soon
as the options have finished parsing.

In the case of dhcp-relay related options, copy the memory in a
dedicated buffer to ensure proper lifetimes.

src/libsystemd-network/dhcp-lease-internal.h

@@ -23,7 +23,7 @@ struct sd_dhcp_raw_option {
         LIST_FIELDS(struct sd_dhcp_raw_option, options);
 
         uint8_t tag;
-        uint8_t length;
+        size_t length;
         void *data;
 };
 
@@ -89,9 +89,9 @@ struct sd_dhcp_lease {
 
 int dhcp_lease_new(sd_dhcp_lease **ret);
 
-int dhcp_lease_parse_options(uint8_t code, uint8_t len, const void *option, void *userdata);
+int dhcp_lease_parse_options(uint8_t code, size_t len, const void *option, void *userdata);
 int dhcp_lease_parse_search_domains(const uint8_t *option, size_t len, char ***domains);
-int dhcp_lease_insert_private_option(sd_dhcp_lease *lease, uint8_t tag, const void *data, uint8_t len);
+int dhcp_lease_insert_private_option(sd_dhcp_lease *lease, uint8_t tag, const void *data, size_t len);
 
 void dhcp_lease_set_timestamp(sd_dhcp_lease *lease, const triple_timestamp *timestamp);
 int dhcp_lease_set_default_subnet_mask(sd_dhcp_lease *lease);

src/libsystemd-network/dhcp-option.c

@@ -6,17 +6,28 @@
 #include <errno.h>
 #include <stdint.h>
 #include <stdio.h>
+#include <stdlib.h>
+#include <sys/param.h>
 
 #include "alloc-util.h"
 #include "dhcp-option.h"
+#include "dhcp-protocol.h"
 #include "dhcp-server-internal.h"
 #include "dns-domain.h"
+#include "hash-funcs.h"
+#include "hashmap.h"
 #include "hostname-util.h"
+#include "list.h"
+#include "macro.h"
 #include "memory-util.h"
 #include "ordered-set.h"
+#include "sd-dhcp-protocol.h"
 #include "strv.h"
+#include "unaligned.h"
 #include "utf8.h"
 
+static bool dhcp_option_can_merge(uint8_t code);
+
 /* Append type-length value structure to the options buffer */
 static int dhcp_option_append_tlv(uint8_t options[], size_t size, size_t *offset, uint8_t code, size_t optlen, const void *optval) {
         assert(options);
@@ -203,9 +214,9 @@ int dhcp_option_remove_option(uint8_t *options, size_t length, uint8_t option_co
         return length - r;
 }
 
-int dhcp_option_append(DHCPMessage *message, size_t size, size_t *offset,
-                       uint8_t overload,
-                       uint8_t code, size_t optlen, const void *optval) {
+static int dhcp_option_append_short(DHCPMessage *message, size_t size, size_t *offset,
+                                    uint8_t overload,
+                                    uint8_t code, size_t optlen, const void *optval) {
         const bool use_file = overload & DHCP_OVERLOAD_FILE;
         const bool use_sname = overload & DHCP_OVERLOAD_SNAME;
         int r;
@@ -276,10 +287,106 @@ int dhcp_option_append(DHCPMessage *message, size_t size, size_t *offset,
         return -ENOBUFS;
 }
 
-static int parse_options(const uint8_t options[], size_t buflen, uint8_t *overload,
-                         uint8_t *message_type, char **error_message, dhcp_option_callback_t cb,
-                         void *userdata) {
-        uint8_t code, len;
+static int dhcp_option_append_long(DHCPMessage *message, size_t size, size_t *offset,
+                                   uint8_t overload,
+                                   uint8_t code, size_t optlen, const uint8_t *optval) {
+        const bool use_file = overload & DHCP_OVERLOAD_FILE;
+        const bool use_sname = overload & DHCP_OVERLOAD_SNAME;
+        size_t available, to_write;
+        int r;
+
+        assert(message);
+        assert(offset);
+
+        /* If *offset is in range [0, size), we are writing to ->options,
+         * if *offset is in range [size, size + sizeof(message->file)) and use_file, we are writing to ->file,
+         * if *offset is in range [size + use_file*sizeof(message->file),
+         *                         size + use_file*sizeof(message->file) + sizeof(message->sname))
+         * and use_sname, we are writing to ->sname.
+         */
+
+        while (optlen) {
+                if (*offset + 3 < size) {
+                        /* still space in the options array for at least TL and end */
+                        available = size - 3 - *offset;
+                        to_write = MIN(available, MIN(optlen, (size_t)UINT8_MAX));
+
+                        r = option_append(message->options, size, offset, code, to_write, optval);
+                        if (r < 0)
+                                return r;
+
+                        optval += to_write;
+                        optlen -= to_write;
+                        if (to_write == available) {
+                                /* close the options array */
+                                r = option_append(message->options, size, offset,
+                                                  SD_DHCP_OPTION_END, 0, NULL);
+                                if (r < 0)
+                                        return r;
+                        }
+
+                        continue;
+                }
+
+                size_t file_offset = *offset - size;
+                if (use_file && file_offset + 3 < sizeof(message->file)) {
+                        /* still space in the file array for at least TL and end */
+                        available = sizeof(message->file) - 3 - file_offset;
+                        to_write = MIN(available, MIN(optlen, (size_t)UINT8_MAX));
+
+                        r = option_append(message->file, sizeof(message->file), &file_offset,
+                                          code, to_write, optval);
+                        if (r < 0)
+                                return r;
+
+                        optval += to_write;
+                        optlen -= to_write;
+                        *offset = size + file_offset;
+                        if (to_write == available) {
+                                /* close the file array */
+                                r = option_append(message->file, sizeof(message->file), &file_offset,
+                                                  SD_DHCP_OPTION_END, 0, NULL);
+                                if (r < 0)
+                                        return r;
+                        }
+
+                        continue;
+                }
+
+                size_t sname_offset = *offset - size - (use_file ? sizeof(message->file) : 0);
+                if (use_sname && sname_offset + 3 < sizeof(message->sname)) {
+                        /* still space in the sname array for at least TL and end */
+                        available = sizeof(message->sname) - 3 - sname_offset;
+                        to_write = MIN(available, MIN(optlen, (size_t)UINT8_MAX));
+
+                        r = option_append(message->sname, sizeof(message->sname), &sname_offset,
+                                          code, to_write, optval);
+                        if (r < 0)
+                                return r;
+                        optval += to_write;
+                        optlen -= to_write;
+                        *offset = size + (use_file ? sizeof(message->file) : 0) + sname_offset;
+                }
+
+                return -ENOBUFS;
+        }
+
+        return 0;
+}
+
+int dhcp_option_append(DHCPMessage *message, size_t size, size_t *offset,
+                       uint8_t overload,
+                       uint8_t code, size_t optlen, const void *optval) {
+        if (dhcp_option_can_merge(code))
+                return dhcp_option_append_long(message, size, offset, overload, code, optlen, optval);
+        else
+                return dhcp_option_append_short(message, size, offset, overload, code, optlen, optval);
+}
+
+static int parse_options(const uint8_t options[], size_t buflen, uint8_t *message_type,
+                         char **error_message, dhcp_option_callback_t cb, void *userdata) {
+        uint8_t code;
+        uint16_t len;
         const uint8_t *option;
         size_t offset = 0;
         int r;
@@ -287,18 +394,11 @@ static int parse_options(const uint8_t options[], size_t buflen, uint8_t *overlo
         while (offset < buflen) {
                 code = options[offset++];
 
-                switch (code) {
-                case SD_DHCP_OPTION_PAD:
-                        continue;
-
-                case SD_DHCP_OPTION_END:
-                        return 0;
-                }
-
-                if (buflen < offset + 1)
+                if (buflen < offset + 2)
                         return -ENOBUFS;
 
-                len = options[offset++];
+                len = unaligned_read_be16(&options[offset]);
+                offset += 2;
 
                 if (buflen < offset + len)
                         return -EINVAL;
@@ -333,14 +433,6 @@ static int parse_options(const uint8_t options[], size_t buflen, uint8_t *overlo
                         }
 
                         break;
-                case SD_DHCP_OPTION_OVERLOAD:
-                        if (len != 1)
-                                return -EINVAL;
-
-                        if (overload)
-                                *overload = *option;
-
-                        break;
 
                 default:
                         if (cb)
@@ -356,10 +448,387 @@ static int parse_options(const uint8_t options[], size_t buflen, uint8_t *overlo
         return 0;
 }
 
+/* draft-tojens-dhcp-option-concat-considerations
+ *
+ * Not all DHCP options are created equal, and some may not be concatenation-compatible. The draft
+ * defines a few option categories, and there we define our strategy wrt. concatenation.
+ *
+ * 1. Fixed length options: let's not concatenate those. Splitting most of those options is a waste
+ *    of space, thus it is unlikely that any agent split them.
+ *    Treat each occurence as a separate instance, and keep the "parse at least one" behavior.
+ *    Arguments can be made about "long" options (e.g. authentication options), as those can be
+ *    split without wasting space. However, be conservative and don't try to concatenate them
+ *    without further guidance.
+ * 2. Multiple-of-fixed-length options: let's eagerly concatenate those. If all instances have a
+ *    valid mod length, then so does the resulting instance. And if the resulting concatenated
+ *    option has an invalid length, so did (some of) the separate instances.
+ * 3. Arbitrary length options: let's be simple and also concatenate those.
+ *
+ * Note that this classification is *unopinionated* about the classification. Some options may be
+ * discussed as to whether they qualify as a category, or whether the category behavior makes
+ * sense.
+ * At the time of writing, SD_DHCP_OPTION_DIRECTORY_AGENT is suspect, as it's a multiple of
+ * fixed-length, but with a leading extra byte. In the light of both RFC3396 and the draft, the
+ * final behavior of concatenation is assumed to be best.
+ */
+static bool dhcp_option_can_merge(uint8_t code) {
+        switch (code) {
+        /* Fixed-length options */
+        case SD_DHCP_OPTION_SUBNET_MASK:
+        case SD_DHCP_OPTION_TIME_OFFSET:
+        case SD_DHCP_OPTION_BOOT_FILE_SIZE:
+        case SD_DHCP_OPTION_SWAP_SERVER:
+        case SD_DHCP_OPTION_FORWARD:
+        case SD_DHCP_OPTION_SOURCE_ROUTE:
+        case SD_DHCP_OPTION_DEFAULT_IP_TTL:
+        case SD_DHCP_OPTION_MTU_TIMEOUT:
+        case SD_DHCP_OPTION_MTU_INTERFACE:
+        case SD_DHCP_OPTION_MTU_SUBNET:
+        case SD_DHCP_OPTION_BROADCAST:
+        case SD_DHCP_OPTION_MASK_DISCOVERY:
+        case SD_DHCP_OPTION_MASK_SUPPLIER:
+        case SD_DHCP_OPTION_ROUTER_DISCOVERY:
+        case SD_DHCP_OPTION_ROUTER_REQUEST:
+        case SD_DHCP_OPTION_TRAILERS:
+        case SD_DHCP_OPTION_ARP_TIMEOUT:
+        case SD_DHCP_OPTION_ETHERNET:
+        case SD_DHCP_OPTION_DEFAULT_TCP_TTL:
+        case SD_DHCP_OPTION_KEEPALIVE_TIME:
+        case SD_DHCP_OPTION_KEEPALIVE_DATA:
+        case SD_DHCP_OPTION_NETBIOS_NODE_TYPE:
+        case SD_DHCP_OPTION_REQUESTED_IP_ADDRESS:
+        case SD_DHCP_OPTION_IP_ADDRESS_LEASE_TIME:
+        case SD_DHCP_OPTION_OVERLOAD:
+        case SD_DHCP_OPTION_MESSAGE_TYPE:
+        case SD_DHCP_OPTION_SERVER_IDENTIFIER:
+        case SD_DHCP_OPTION_MAXIMUM_MESSAGE_SIZE:
+        case SD_DHCP_OPTION_RENEWAL_TIME:
+        case SD_DHCP_OPTION_REBINDING_TIME:
+        case SD_DHCP_OPTION_NETWARE_IP_OPTION:
+        case SD_DHCP_OPTION_RAPID_COMMIT:
+        case SD_DHCP_OPTION_AUTHENTICATION:
+        case SD_DHCP_OPTION_CLIENT_LAST_TRANSACTION_TIME:
+        case SD_DHCP_OPTION_CLIENT_SYSTEM:
+        case SD_DHCP_OPTION_CLIENT_NDI:
+        case SD_DHCP_OPTION_UUID:
+        case SD_DHCP_OPTION_IPV6_ONLY_PREFERRED:
+        case SD_DHCP_OPTION_DHCP4O6_SOURCE_ADDRESS:
+        case SD_DHCP_OPTION_AUTO_CONFIG:
+        case SD_DHCP_OPTION_SUBNET_SELECTION:
+        case SD_DHCP_OPTION_GEOCONF:
+        case SD_DHCP_OPTION_GEOLOC:
+        case SD_DHCP_OPTION_BASE_TIME:
+        case SD_DHCP_OPTION_START_TIME_OF_STATE:
+        case SD_DHCP_OPTION_QUERY_START_TIME:
+        case SD_DHCP_OPTION_QUERY_END_TIME:
+        case SD_DHCP_OPTION_DHCP_STATE:
+        case SD_DHCP_OPTION_DATA_SOURCE:
+        case SD_DHCP_OPTION_PORT_PARAMS:
+        case SD_DHCP_OPTION_PXELINUX_MAGIC:
+        case SD_DHCP_OPTION_REBOOT_TIME:
+        case SD_DHCP_OPTION_6RD:
+        /* apple-specific, not documented, so be conservative */
+        case SD_DHCP_OPTION_LDAP:
+        case SD_DHCP_OPTION_NETINFO_ADDRESS:
+        case SD_DHCP_OPTION_NETINFO_TAG:
+        /* despite being a variable-length option, there are two sub-options defined in RFC3046, so
+         * it is expected to be so short that no splitting will happen. furthermore, it simplifies
+         * relay implementation.
+         */
+        case SD_DHCP_OPTION_RELAY_AGENT_INFORMATION:
+                return false;
+        /* Multiple of fixed-length options */
+        case SD_DHCP_OPTION_ROUTER:
+        case SD_DHCP_OPTION_TIME_SERVER:
+        case SD_DHCP_OPTION_NAME_SERVER:
+        case SD_DHCP_OPTION_DOMAIN_NAME_SERVER:
+        case SD_DHCP_OPTION_LOG_SERVER:
+        case SD_DHCP_OPTION_QUOTES_SERVER:
+        case SD_DHCP_OPTION_LPR_SERVER:
+        case SD_DHCP_OPTION_IMPRESS_SERVER:
+        case SD_DHCP_OPTION_RLP_SERVER:
+        case SD_DHCP_OPTION_POLICY_FILTER:
+        case SD_DHCP_OPTION_MTU_PLATEAU:
+        case SD_DHCP_OPTION_STATIC_ROUTE:
+        case SD_DHCP_OPTION_NIS_SERVER:
+        case SD_DHCP_OPTION_NTP_SERVER:
+        case SD_DHCP_OPTION_NETBIOS_NAME_SERVER:
+        case SD_DHCP_OPTION_NETBIOS_DIST_SERVER:
+        case SD_DHCP_OPTION_X_WINDOW_FONT:
+        case SD_DHCP_OPTION_X_WINDOW_MANAGER:
+        case SD_DHCP_OPTION_NIS_SERVER_ADDR:
+        case SD_DHCP_OPTION_HOME_AGENT_ADDRESSES:
+        case SD_DHCP_OPTION_SMTP_SERVER:
+        case SD_DHCP_OPTION_POP3_SERVER:
+        case SD_DHCP_OPTION_NNTP_SERVER:
+        case SD_DHCP_OPTION_WWW_SERVER:
+        case SD_DHCP_OPTION_FINGER_SERVER:
+        case SD_DHCP_OPTION_IRC_SERVER:
+        case SD_DHCP_OPTION_STREETTALK_SERVER:
+        case SD_DHCP_OPTION_STDA_SERVER:
+        case SD_DHCP_OPTION_DIRECTORY_AGENT: /* multiple of four plus one */
+        case SD_DHCP_OPTION_NDS_SERVER:
+        case SD_DHCP_OPTION_BCMCS_CONTROLLER_ADDRESS:
+        case SD_DHCP_OPTION_ASSOCIATED_IP:
+        case SD_DHCP_OPTION_NAME_SERVICE_SEARCH:
+        case SD_DHCP_OPTION_PANA_AGENT:
+        case SD_DHCP_OPTION_CAPWAP_AC_ADDRESS:
+        case SD_DHCP_OPTION_ANDSF_ADDRESS:
+        case SD_DHCP_OPTION_TFTP_SERVER_ADDRESS:
+        /* Arbitrary length options */
+        case SD_DHCP_OPTION_HOST_NAME:
+        case SD_DHCP_OPTION_MERIT_DUMP_FILE:
+        case SD_DHCP_OPTION_DOMAIN_NAME:
+        case SD_DHCP_OPTION_ROOT_PATH:
+        case SD_DHCP_OPTION_EXTENSION_FILE:
+        case SD_DHCP_OPTION_NIS_DOMAIN:
+        case SD_DHCP_OPTION_VENDOR_SPECIFIC:
+        case SD_DHCP_OPTION_NETBIOS_SCOPE:
+        case SD_DHCP_OPTION_PARAMETER_REQUEST_LIST:
+        case SD_DHCP_OPTION_ERROR_MESSAGE:
+        case SD_DHCP_OPTION_VENDOR_CLASS_IDENTIFIER:
+        case SD_DHCP_OPTION_CLIENT_IDENTIFIER:
+        case SD_DHCP_OPTION_NIS_DOMAIN_NAME:
+        case SD_DHCP_OPTION_BOOT_SERVER_NAME:
+        case SD_DHCP_OPTION_BOOT_FILENAME:
+        case SD_DHCP_OPTION_USER_CLASS:
+        case SD_DHCP_OPTION_SERVICE_SCOPE:
+        case SD_DHCP_OPTION_ISNS:
+        case SD_DHCP_OPTION_USER_AUTHENTICATION:
+        case SD_DHCP_OPTION_POSIX_TIMEZONE:
+        case SD_DHCP_OPTION_TZDB_TIMEZONE:
+        case SD_DHCP_OPTION_DHCP_CAPTIVE_PORTAL:
+        case SD_DHCP_OPTION_LOST_SERVER_FQDN:
+        case SD_DHCP_OPTION_FORCERENEW_NONCE_CAPABLE:
+        case SD_DHCP_OPTION_DOTS_RI:
+        case SD_DHCP_OPTION_STATUS_CODE:
+        case SD_DHCP_OPTION_CONFIGURATION_FILE:
+        case SD_DHCP_OPTION_PATH_PREFIX:
+        case SD_DHCP_OPTION_ACCESS_DOMAIN:
+        case SD_DHCP_OPTION_SUBNET_ALLOCATION:
+        case SD_DHCP_OPTION_VIRTUAL_SUBNET_SELECTION:
+        case SD_DHCP_OPTION_PRIVATE_PROXY_AUTODISCOVERY:
+        /* MUST implement DHCP option concatenation as per RFC3396 */
+        case SD_DHCP_OPTION_FQDN:
+        case SD_DHCP_OPTION_BCMCS_CONTROLLER_DOMAIN_NAME:
+        case SD_DHCP_OPTION_GEOCONF_CIVIC:
+        case SD_DHCP_OPTION_DOMAIN_SEARCH:
+        case SD_DHCP_OPTION_SIP_SERVER: /* RFC3396 is mentioned as "work in progress" */
+        case SD_DHCP_OPTION_CLASSLESS_STATIC_ROUTE:
+        case SD_DHCP_OPTION_CABLELABS_CLIENT_CONFIGURATION:
+        case SD_DHCP_OPTION_VENDOR_CLASS:
+        case SD_DHCP_OPTION_VENDOR_SPECIFIC_INFORMATION:
+        case SD_DHCP_OPTION_MOS_ADDRESS:
+        case SD_DHCP_OPTION_MOS_FQDN:
+        case SD_DHCP_OPTION_SIP_SERVICE_DOMAIN:
+        case SD_DHCP_OPTION_SZTP_REDIRECT:
+        case SD_DHCP_OPTION_RDNSS_SELECTION:
+        case SD_DHCP_OPTION_DOTS_ADDRESS:
+        case SD_DHCP_OPTION_PCP_SERVER:
+        case SD_DHCP_OPTION_PRIVATE_CLASSLESS_STATIC_ROUTE: /* Microsoft's version of 121 */
+        /* RFC2242: [...] and its maximum length is 255.
+         * While the length of the option is bounded, and fits in one option, the does not need
+         * splitting to fit in an option. However, due to the option being potentially be long,
+         * agents may decide to split the option e.g. across the option and sname fields.
+         * Let's support concatenation for this option.
+         */
+        case SD_DHCP_OPTION_NETWARE_IP_DOMAIN:
+        /* RFC2241: The maximum possible length for this option is 255 bytes.
+         * Same reasoning as the previous one.
+         */
+        case SD_DHCP_OPTION_NDS_TREE_NAME:
+        /* RFC2241: A single DHCP option can only contain 255 octets. Since an NDS context name can
+         *          be longer than that, this option can appear more than once in the DHCP packet.
+         *          The contents of all NDS Context options in the packet should be concatenated as
+         *          suggested in the DHCP specification [3, page 24] to get the complete NDS
+         *          context. A single encoded character could be split between two NDS Context
+         *          Options.
+         * This basically describes what RFC3396 later formalizes, but predates it.
+         */
+        case SD_DHCP_OPTION_NDS_CONTEXT:
+        /* RFC8520: The entire option MUST NOT exceed 255 octets.
+         * Same as with 2242: it may be split across multiple instances of the option.
+         */
+        case SD_DHCP_OPTION_MUD_URL:
+                return true;
+        default:
+                return false;
+        }
+}
+
+struct MergeOptionNode {
+        const uint8_t *data;
+        LIST_FIELDS(struct MergeOptionNode, options);
+};
+
+static struct MergeOptionNode* merge_option_free(struct MergeOptionNode *head) {
+        LIST_CLEAR(options, head, free);
+        return NULL;
+}
+
+DEFINE_PRIVATE_HASH_OPS_WITH_VALUE_DESTRUCTOR(merge_option_node_ops,
+                                              void,
+                                              trivial_hash_func,
+                                              trivial_compare_func,
+                                              struct MergeOptionNode,
+                                              merge_option_free);
+
+static int dhcp_option_merge_append(const uint8_t *buf, size_t buflen, OrderedHashmap *opts,
+                                    size_t *tot_len, uint8_t *overload) {
+        size_t offset = 0;
+        struct MergeOptionNode *head, *next;
+
+        while (offset < buflen) {
+                const uint8_t *data = &buf[offset];
+                uint8_t code = buf[offset++];
+
+                switch (code) {
+                case SD_DHCP_OPTION_PAD:
+                        continue;
+                case SD_DHCP_OPTION_END:
+                        return 0;
+                }
+
+                if (buflen < offset + 1)
+                        return -ENOBUFS;
+
+                uint8_t len = buf[offset++];
+
+                if (buflen < offset + len)
+                        return -EINVAL;
+
+                switch (code) {
+                case SD_DHCP_OPTION_OVERLOAD:
+                        if (len != 1)
+                                return -EINVAL;
+                        if (overload)
+                                *overload = buf[offset];
+                        offset++;
+                        continue;
+                }
+
+                next = new(struct MergeOptionNode, 1);
+                if (!next)
+                        return -ENOMEM;
+                LIST_INIT(options, next);
+                next->data = data;
+
+                head = ordered_hashmap_get(opts, UINT_TO_PTR(code));
+                if (head)
+                        LIST_APPEND(options, head, next);
+                else {
+                        int r = ordered_hashmap_put(opts, UINT_TO_PTR(code), next);
+                        if (r < 0) {
+                                free(next);
+                                return r;
+                        }
+                }
+
+                if (!head || !dhcp_option_can_merge(code))
+                        *tot_len += 3; /* option code + 16-bit length */
+                offset += len;
+                *tot_len += len;
+        }
+
+        return 0;
+}
+
+/* RFC3396 specifies how DHCP options longer than 255 bytes should be handled.
+ *
+ * Option is to be repeated multiple times and treated as one, long, option. However, the cut can
+ * happen at byte boundaries, and clients MUST assume no semantic meaning whatsoever from this cut.
+ * In practice, this means the cut can happen anywhere in the middle of a semantic unit of the
+ * option (e.g. DHCP option 121 (Classless Static Routes) can be split in the middle of a route).
+ *
+ * With the current architecture of stateless callbacks that take a whole chunk of memory, it
+ * becomes hard to retain data between two consecutive callbacks without major changes to the
+ * architecture.
+ *
+ * This function will merge DHCP options by concatenating options of the same type, producing a new
+ * TLV buffer, but using u16 len fields instead of u8. This works due to a DHCP message size having
+ * an inherent upper bound, lower than u16::MAX.
+ *
+ * Basically, this produces the aggregated option buffer the RFC talks about.
+ */
+static int dhcp_option_merge(const DHCPMessage *message, size_t buflen, uint8_t **ret_merged) {
+        _cleanup_ordered_hashmap_free_ OrderedHashmap *opts;
+        uint8_t *aggregate, overload = 0;
+        size_t tot_len = 0;
+        int r = 0;
+
+        opts = ordered_hashmap_new(&merge_option_node_ops);
+        if (!opts)
+                return -ENOMEM;
+
+        /* RFC3396 5. The Aggregate Option Buffer
+         *
+         *   [...], an option that doesn't fit into one field can't overlap the boundary into
+         *   another field - the encoding agent must instead break the option into two parts and
+         *   store one part in each buffer.
+         *
+         *   To simplify this discussion, we will talk about an aggregate option buffer, which will
+         *   be the aggregate of the three buffers. [...] The aggregate option buffer is made up of
+         *   the optional parameters field, the file field, and the sname field, in that order.
+         *
+         * Since options cannot overlap from one field to another, it is safe to read them in
+         * isolation.
+         */
+        r = dhcp_option_merge_append(message->options, buflen, opts, &tot_len, &overload);
+        if (r < 0)
+                return r;
+        if (overload & DHCP_OVERLOAD_FILE) {
+                r = dhcp_option_merge_append(message->file, sizeof(message->file), opts, &tot_len, NULL);
+                if (r < 0)
+                        return r;
+        }
+        if (overload & DHCP_OVERLOAD_SNAME) {
+                r = dhcp_option_merge_append(message->sname, sizeof(message->sname), opts, &tot_len, NULL);
+                if (r < 0)
+                        return r;
+        }
+
+        /* Consolidate in a new buffer */
+        aggregate = new(uint8_t, tot_len);
+        if (!aggregate)
+                return r;
+
+        size_t offset = 0;
+        struct MergeOptionNode *head;
+        ORDERED_HASHMAP_FOREACH(head, opts) {
+                uint8_t code = head->data[0];
+                if (dhcp_option_can_merge(code)) {
+                        uint8_t *opt_base = &aggregate[offset];
+                        offset += 3;
+
+                        size_t len = 0;
+                        LIST_FOREACH(options, opt, head) {
+                                size_t optlen = opt->data[1];
+                                memcpy_safe(&aggregate[offset], &opt->data[2], optlen);
+                                offset += optlen;
+                                len += optlen;
+                        }
+                        opt_base[0] = code;
+                        unaligned_write_be16(&opt_base[1], len);
+                } else
+                        LIST_FOREACH(options, opt, head) {
+                                size_t optlen = opt->data[1];
+
+                                aggregate[offset] = code;
+                                unaligned_write_be16(&aggregate[offset + 1], (uint16_t)optlen);
+                                memcpy_safe(&aggregate[offset + 3], &opt->data[2], optlen);
+
+                                offset += 3 + optlen;
+                        }
+        }
+
+        *ret_merged = aggregate;
+        return tot_len;
+}
+
 int dhcp_option_parse(DHCPMessage *message, size_t len, dhcp_option_callback_t cb, void *userdata, char **ret_error_message) {
         _cleanup_free_ char *error_message = NULL;
-        uint8_t overload = 0;
         uint8_t message_type = 0;
+        _cleanup_free_ uint8_t *options = NULL;
         int r;
 
         if (!message)
@@ -370,21 +839,14 @@ int dhcp_option_parse(DHCPMessage *message, size_t len, dhcp_option_callback_t c
 
         len -= sizeof(DHCPMessage);
 
-        r = parse_options(message->options, len, &overload, &message_type, &error_message, cb, userdata);
+        r = dhcp_option_merge(message, len, &options);
         if (r < 0)
                 return r;
+        len = r;
 
-        if (overload & DHCP_OVERLOAD_FILE) {
-                r = parse_options(message->file, sizeof(message->file), NULL, &message_type, &error_message, cb, userdata);
-                if (r < 0)
-                        return r;
-        }
-
-        if (overload & DHCP_OVERLOAD_SNAME) {
-                r = parse_options(message->sname, sizeof(message->sname), NULL, &message_type, &error_message, cb, userdata);
-                if (r < 0)
-                        return r;
-        }
+        r = parse_options(options, len, &message_type, &error_message, cb, userdata);
+        if (r < 0)
+                return r;
 
         if (message_type == 0)
                 return -ENOMSG;

src/libsystemd-network/dhcp-option.h

@@ -35,7 +35,7 @@ int dhcp_option_append(
 int dhcp_option_find_option(uint8_t *options, size_t length, uint8_t wanted_code, size_t *ret_offset);
 int dhcp_option_remove_option(uint8_t *options, size_t buflen, uint8_t option_code);
 
-typedef int (*dhcp_option_callback_t)(uint8_t code, uint8_t len, const void *option, void *userdata);
+typedef int (*dhcp_option_callback_t)(uint8_t code, size_t len, const void *option, void *userdata);
 
 int dhcp_option_parse(
                 DHCPMessage *message,

src/libsystemd-network/dhcp-server-internal.h

@@ -90,9 +90,10 @@ typedef struct DHCPRequest {
         be32_t server_id;
         be32_t requested_ip;
         usec_t lifetime;
-        const uint8_t *agent_info_option;
+        uint8_t *agent_info_option;
+        size_t agent_info_option_len;
         char *hostname;
-        const uint8_t *parameter_request_list;
+        uint8_t *parameter_request_list;
         size_t parameter_request_list_len;
         bool rapid_commit;
         triple_timestamp timestamp;

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

@@ -847,7 +847,7 @@ static int lease_parse_6rd(sd_dhcp_lease *lease, const uint8_t *option, size_t l
         return 0;
 }
 
-int dhcp_lease_parse_options(uint8_t code, uint8_t len, const void *option, void *userdata) {
+int dhcp_lease_parse_options(uint8_t code, size_t len, const void *option, void *userdata) {
         sd_dhcp_lease *lease = ASSERT_PTR(userdata);
         int r;
 
@@ -1173,7 +1173,7 @@ int dhcp_lease_parse_search_domains(const uint8_t *option, size_t len, char ***d
         return cnt;
 }
 
-int dhcp_lease_insert_private_option(sd_dhcp_lease *lease, uint8_t tag, const void *data, uint8_t len) {
+int dhcp_lease_insert_private_option(sd_dhcp_lease *lease, uint8_t tag, const void *data, size_t len) {
         struct sd_dhcp_raw_option *option, *before = NULL;
 
         assert(lease);

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

@@ -448,11 +448,17 @@ int dhcp_server_send_packet(sd_dhcp_server *server,
                 return r;
 
         if (req->agent_info_option) {
-                size_t opt_full_length = *(req->agent_info_option + 1) + 2;
+                /* should not happen as we don't merge the relay agent information option. see
+                   dhcp_option_can_merge for reference */
+                if (req->agent_info_option_len > UINT8_MAX)
+                        return -EINVAL;
                 /* there must be space left for SD_DHCP_OPTION_END */
-                if (optoffset + opt_full_length < req->max_optlen) {
-                        memcpy(packet->dhcp.options + optoffset, req->agent_info_option, opt_full_length);
-                        optoffset += opt_full_length;
+                if (optoffset + req->agent_info_option_len + 2 < req->max_optlen) {
+                        packet->dhcp.options[optoffset] = SD_DHCP_OPTION_RELAY_AGENT_INFORMATION;
+                        packet->dhcp.options[optoffset + 1] = req->agent_info_option_len;
+                        memcpy(&packet->dhcp.options[optoffset + 2], req->agent_info_option,
+                               req->agent_info_option_len);
+                        optoffset += req->agent_info_option_len + 2;
                 }
         }
 
@@ -729,7 +735,7 @@ static int server_send_forcerenew(
                                     sizeof(DHCPMessage) + optoffset);
 }
 
-static int parse_request(uint8_t code, uint8_t len, const void *option, void *userdata) {
+static int parse_request(uint8_t code, size_t len, const void *option, void *userdata) {
         DHCPRequest *req = ASSERT_PTR(userdata);
         int r;
 
@@ -761,9 +767,13 @@ static int parse_request(uint8_t code, uint8_t len, const void *option, void *us
 
                 break;
         case SD_DHCP_OPTION_RELAY_AGENT_INFORMATION:
-                req->agent_info_option = (uint8_t*)option - 2;
-
+                if (req->agent_info_option)
+                        mfree(req->agent_info_option);
+                req->agent_info_option = new(uint8_t, len);
+                memcpy(req->agent_info_option, option, len);
+                req->agent_info_option_len = len;
                 break;
+
         case SD_DHCP_OPTION_HOST_NAME: {
                 _cleanup_free_ char *p = NULL;
 
@@ -775,7 +785,10 @@ static int parse_request(uint8_t code, uint8_t len, const void *option, void *us
                 break;
         }
         case SD_DHCP_OPTION_PARAMETER_REQUEST_LIST:
-                req->parameter_request_list = option;
+                if (req->parameter_request_list)
+                        mfree(req->parameter_request_list);
+                req->parameter_request_list = new(uint8_t, len);
+                memcpy(req->parameter_request_list, option, len);
                 req->parameter_request_list_len = len;
                 break;
 
@@ -792,6 +805,8 @@ static DHCPRequest* dhcp_request_free(DHCPRequest *req) {
                 return NULL;
 
         free(req->hostname);
+        free(req->agent_info_option);
+        free(req->parameter_request_list);
         return mfree(req);
 }
 

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

@@ -161,7 +161,7 @@ static void test_dhcp_identifier_set_iaid(void) {
 #endif
 }
 
-static int check_options(uint8_t code, uint8_t len, const void *option, void *userdata) {
+static int check_options(uint8_t code, size_t len, const void *option, void *userdata) {
         switch (code) {
         case SD_DHCP_OPTION_CLIENT_IDENTIFIER: {
                 sd_dhcp_duid duid;

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

@@ -12,8 +12,15 @@
 #include "ether-addr-util.h"
 #include "macro.h"
 #include "memory-util.h"
+#include "sd-dhcp-protocol.h"
 #include "tests.h"
 
+struct opt_overrides {
+        uint8_t code;
+        uint8_t data[128];
+        size_t len;
+};
+
 struct option_desc {
         uint8_t sname[64];
         int snamelen;
@@ -22,6 +29,8 @@ struct option_desc {
         uint8_t options[128];
         int len;
         bool success;
+        struct opt_overrides overrides[16];
+        int overrideslen;
         int filepos;
         int snamepos;
         int pos;
@@ -52,6 +61,74 @@ static struct option_desc option_tests[] = {
           { 222, 3, 1, 2, 3 }, 5,
           { SD_DHCP_OPTION_OVERLOAD, 1,
             DHCP_OVERLOAD_FILE|DHCP_OVERLOAD_SNAME }, 3, true, },
+        /* test RFC3396: option split across multiple options, cut at an arbitrary byte boundary.
+         * Example here for a routing table, where the split is in the middle of the route.
+         * In practice, this will happen when the routes don't fit in a single option, but the
+         * behavior will likely be the same.
+         */
+        {
+                .options = {
+                        SD_DHCP_OPTION_MESSAGE_TYPE, 1, DHCP_ACK,
+                        SD_DHCP_OPTION_CLASSLESS_STATIC_ROUTE, 4, 22, 172, 16, 0,
+                        SD_DHCP_OPTION_CLASSLESS_STATIC_ROUTE, 4, 0, 0, 0, 0,
+                },
+                .len = 15,
+                .success = true,
+                /* the whole option must be reconstituted, so let's override the payload */
+                .overrides = {{
+                        .code = SD_DHCP_OPTION_CLASSLESS_STATIC_ROUTE,
+                        .data = {22, 172, 16, 0, 0, 0, 0, 0},
+                        .len = 8,
+                }},
+                .overrideslen = 1,
+        },
+        /* test draft-tojens-dhcp-option-concat-considerations: not all options should be
+         * concatenated. Concatenation-requiring is already tested by the previous test.
+         * test fixed-length options.
+         */
+        {
+                .options = {
+                        SD_DHCP_OPTION_MESSAGE_TYPE, 1, DHCP_ACK,
+                        SD_DHCP_OPTION_SUBNET_MASK, 4, 255, 255, 255, 0,
+                        SD_DHCP_OPTION_SUBNET_MASK, 4, 255, 255, 255, 0,
+                },
+                .len = 15,
+                .success = true,
+                /* no overrides: option should not be concatenated */
+        },
+        /* test multiple of fixed-length */
+        {
+                .options = {
+                        SD_DHCP_OPTION_MESSAGE_TYPE, 1, DHCP_ACK,
+                        SD_DHCP_OPTION_DOMAIN_NAME_SERVER, 3, 8, 8, 8,
+                        SD_DHCP_OPTION_DOMAIN_NAME_SERVER, 5, 8, 1, 1, 1, 1,
+                },
+                .len = 15,
+                .success = true,
+                .overrides = {{
+                        .code = SD_DHCP_OPTION_DOMAIN_NAME_SERVER,
+                        .data = {8, 8, 8, 8, 1, 1, 1, 1},
+                        .len = 8,
+                }},
+                .overrideslen = 1,
+        },
+        /* test arbitrary length */
+        {
+                .options = {
+                        SD_DHCP_OPTION_MESSAGE_TYPE, 1, DHCP_ACK,
+                        SD_DHCP_OPTION_HOST_NAME, 3, 'f', 'o', 'o',
+                        SD_DHCP_OPTION_HOST_NAME, 3, 'b', 'a', 'r',
+                        SD_DHCP_OPTION_HOST_NAME, 3, 'b', 'a', 'z',
+                },
+                .len = 18,
+                .success = true,
+                .overrides = {{
+                        .code = SD_DHCP_OPTION_HOST_NAME,
+                        .data = "foobarbaz",
+                        .len = 9,
+                }},
+                .overrideslen = 1,
+        },
 };
 
 static const char *dhcp_type(int type) {
@@ -143,12 +220,13 @@ static void test_ignore_opts(uint8_t *descoption, int *descpos, int *desclen) {
         }
 }
 
-static int test_options_cb(uint8_t code, uint8_t len, const void *option, void *userdata) {
+static int test_options_cb(uint8_t code, size_t len, const void *option, void *userdata) {
         struct option_desc *desc = userdata;
         uint8_t *descoption = NULL;
         int *desclen = NULL, *descpos = NULL;
         uint8_t optcode = 0;
         uint8_t optlen = 0;
+        size_t descoption_offset;
 
         assert_se((!desc && !code && !len) || desc);
 
@@ -193,9 +271,18 @@ static int test_options_cb(uint8_t code, uint8_t len, const void *option, void *
 
         optcode = descoption[*descpos];
         optlen = descoption[*descpos + 1];
+        descoption_offset = *descpos + 2;
+
+        for (int i = 0; i < desc->overrideslen; i++)
+                if (desc->overrides[i].code == optcode) {
+                        optlen = desc->overrides[i].len;
+                        descoption = desc->overrides[i].data;
+                        descoption_offset = 0;
+                        break;
+                }
 
         if (verbose)
-                printf("DHCP code %2d(%2d) len %2d(%2d) ", code, optcode,
+                printf("DHCP code %2d(%2d) len %4zu(%2d) ", code, optcode,
                                 len, optlen);
 
         assert_se(code == optcode);
@@ -205,9 +292,9 @@ static int test_options_cb(uint8_t code, uint8_t len, const void *option, void *
                 if (verbose)
                         printf("0x%02x(0x%02x) ",
                                ((uint8_t*) option)[i],
-                               descoption[*descpos + 2 + i]);
+                               descoption[descoption_offset + i]);
 
-                assert_se(((uint8_t*) option)[i] == descoption[*descpos + 2 + i]);
+                assert_se(((uint8_t*) option)[i] == descoption[descoption_offset + i]);
         }
 
         if (verbose)
@@ -362,6 +449,170 @@ static void test_option_set(void) {
                 printf ("\n");
 }
 
+struct long_option_test {
+        const char *name;
+        struct {
+                uint8_t code;
+                size_t len;
+                uint8_t data[1024];
+        } option;
+        size_t max_optlen;
+        bool use_file;
+        bool success;
+        uint8_t expected_option[500];
+        size_t expected_optlen;
+        uint8_t expected_file[128];
+        size_t expected_filelen;
+};
+
+static struct long_option_test long_option_tests[] = {
+        {
+                .name = "test that a regular option can be serialized",
+                .option.code = SD_DHCP_OPTION_SUBNET_MASK,
+                .option.len = 4,
+                .option.data = {255, 255, 255, 0},
+                .max_optlen = 128,
+                .success = true,
+                .expected_option = {SD_DHCP_OPTION_SUBNET_MASK, 4, 255, 255, 255, 0},
+                .expected_optlen = 6,
+        },
+        {
+                .name = "test that a regular option fails if there is not enough space",
+                .option.code = SD_DHCP_OPTION_SUBNET_MASK,
+                .option.len = 4,
+                .option.data = {255, 255, 255, 0},
+                .max_optlen = 2,
+                .success = false,
+        },
+        {
+                .name = "test that a regular option fallbacks to the file buffer when allowed",
+                .option.code = SD_DHCP_OPTION_SUBNET_MASK,
+                .option.len = 4,
+                .option.data = {255, 255, 255, 0},
+                .max_optlen = 2,
+                .use_file = true,
+                .success = true,
+                .expected_option = {SD_DHCP_OPTION_END, SD_DHCP_OPTION_PAD},
+                .expected_optlen = 2,
+                .expected_file = {SD_DHCP_OPTION_SUBNET_MASK, 4, 255, 255, 255, 0},
+                .expected_filelen = 6,
+        },
+        {
+                .name = "test that a mergeable long option is split across two options (RFC3396)",
+                .option.code = SD_DHCP_OPTION_CLASSLESS_STATIC_ROUTE,
+                .option.len = 256, /* one byte short to fit in one option! */
+                .option.data = {
+                        24, 10, 0, 0, 1, 2, 3, 4, 24, 10, 0, 1, 1, 2, 3, 4,
+                        24, 10, 0, 2, 1, 2, 3, 4, 24, 10, 0, 3, 1, 2, 3, 4,
+                        24, 10, 0, 4, 1, 2, 3, 4, 24, 10, 0, 5, 1, 2, 3, 4,
+                        24, 10, 0, 6, 1, 2, 3, 4, 24, 10, 0, 7, 1, 2, 3, 4,
+                        24, 10, 0, 8, 1, 2, 3, 4, 24, 10, 0, 9, 1, 2, 3, 4,
+                        24, 10, 0, 10, 1, 2, 3, 4, 24, 10, 0, 11, 1, 2, 3, 4,
+                        24, 10, 0, 12, 1, 2, 3, 4, 24, 10, 0, 13, 1, 2, 3, 4,
+                        24, 10, 0, 14, 1, 2, 3, 4, 24, 10, 0, 15, 1, 2, 3, 4,
+                        24, 10, 0, 16, 1, 2, 3, 4, 24, 10, 0, 17, 1, 2, 3, 4,
+                        24, 10, 0, 18, 1, 2, 3, 4, 24, 10, 0, 19, 1, 2, 3, 4,
+                        24, 10, 0, 20, 1, 2, 3, 4, 24, 10, 0, 21, 1, 2, 3, 4,
+                        24, 10, 0, 22, 1, 2, 3, 4, 24, 10, 0, 23, 1, 2, 3, 4,
+                        24, 10, 0, 24, 1, 2, 3, 4, 24, 10, 0, 25, 1, 2, 3, 4,
+                        24, 10, 0, 26, 1, 2, 3, 4, 24, 10, 0, 27, 1, 2, 3, 4,
+                        24, 10, 0, 28, 1, 2, 3, 4, 24, 10, 0, 29, 1, 2, 3, 4,
+                        24, 10, 0, 30, 1, 2, 3, 4, 24, 10, 0, 31, 1, 2, 3, 4,
+                },
+                .max_optlen = 500,
+                .success = true,
+                .expected_option = {
+                        SD_DHCP_OPTION_CLASSLESS_STATIC_ROUTE, 255,
+                        24, 10, 0, 0, 1, 2, 3, 4, 24, 10, 0, 1, 1, 2, 3, 4,
+                        24, 10, 0, 2, 1, 2, 3, 4, 24, 10, 0, 3, 1, 2, 3, 4,
+                        24, 10, 0, 4, 1, 2, 3, 4, 24, 10, 0, 5, 1, 2, 3, 4,
+                        24, 10, 0, 6, 1, 2, 3, 4, 24, 10, 0, 7, 1, 2, 3, 4,
+                        24, 10, 0, 8, 1, 2, 3, 4, 24, 10, 0, 9, 1, 2, 3, 4,
+                        24, 10, 0, 10, 1, 2, 3, 4, 24, 10, 0, 11, 1, 2, 3, 4,
+                        24, 10, 0, 12, 1, 2, 3, 4, 24, 10, 0, 13, 1, 2, 3, 4,
+                        24, 10, 0, 14, 1, 2, 3, 4, 24, 10, 0, 15, 1, 2, 3, 4,
+                        24, 10, 0, 16, 1, 2, 3, 4, 24, 10, 0, 17, 1, 2, 3, 4,
+                        24, 10, 0, 18, 1, 2, 3, 4, 24, 10, 0, 19, 1, 2, 3, 4,
+                        24, 10, 0, 20, 1, 2, 3, 4, 24, 10, 0, 21, 1, 2, 3, 4,
+                        24, 10, 0, 22, 1, 2, 3, 4, 24, 10, 0, 23, 1, 2, 3, 4,
+                        24, 10, 0, 24, 1, 2, 3, 4, 24, 10, 0, 25, 1, 2, 3, 4,
+                        24, 10, 0, 26, 1, 2, 3, 4, 24, 10, 0, 27, 1, 2, 3, 4,
+                        24, 10, 0, 28, 1, 2, 3, 4, 24, 10, 0, 29, 1, 2, 3, 4,
+                        24, 10, 0, 30, 1, 2, 3, 4, 24, 10, 0, 31, 1, 2, 3,
+                        SD_DHCP_OPTION_CLASSLESS_STATIC_ROUTE, 1,
+                        4,
+                },
+                .expected_optlen = 260,
+        },
+        {
+                .name = "test that a mergeable option is split across options and file if needed",
+                .option.code = SD_DHCP_OPTION_CLASSLESS_STATIC_ROUTE,
+                .option.len = 8,
+                .option.data = {24, 10, 0, 0, 1, 2, 3, 4},
+                .max_optlen = 8,
+                .use_file = true,
+                .success = true,
+                .expected_option = {
+                        SD_DHCP_OPTION_CLASSLESS_STATIC_ROUTE, 5, 24, 10, 0, 0, 1,
+                        SD_DHCP_OPTION_END,
+                },
+                .expected_optlen = 8,
+                .expected_file = {SD_DHCP_OPTION_CLASSLESS_STATIC_ROUTE, 3, 2, 3, 4},
+                .expected_filelen = 5,
+        },
+};
+
+static void test_option_append_long(struct long_option_test *desc) {
+        _cleanup_free_ DHCPMessage *result = NULL;
+        uint8_t overload = 0;
+        size_t offset = 0;
+        int r;
+
+        if (verbose)
+                printf(">>> %s\n", desc->name);
+
+        result = malloc0(sizeof(DHCPMessage) + desc->max_optlen);
+        assert_se(result);
+
+        if (desc->use_file)
+                overload |= DHCP_OVERLOAD_FILE;
+
+        r = dhcp_option_append(result, desc->max_optlen, &offset, overload,
+                               desc->option.code, desc->option.len, desc->option.data);
+        if (verbose)
+                printf("dhcp_option_append=%d, offset=%zu\n", r, offset);
+        if (!desc->success) {
+                assert_se(r < 0);
+                return;
+        }
+        assert_se(r == 0);
+        assert_se(offset == desc->expected_optlen + desc->expected_filelen);
+
+        if (verbose)
+                printf("opts: ");
+        for (unsigned i = 0; i < desc->expected_optlen; i++) {
+                if (verbose)
+                        printf("0x%02x(0x%02x) ", result->options[i], desc->expected_option[i]);
+                if (verbose && result->options[i] != desc->expected_option[i])
+                        printf("\n");
+                assert_se(result->options[i] == desc->expected_option[i]);
+        }
+        if (verbose)
+                printf("\n");
+
+        if (verbose)
+                printf("file: ");
+        for (unsigned i = 0; i < desc->expected_filelen; i++) {
+                if (verbose)
+                        printf("0x%02x(0x%02x) ", result->file[i], desc->expected_file[i]);
+                if (verbose && result->file[i] != desc->expected_file[i])
+                        printf("\n");
+                assert_se(result->file[i] == desc->expected_file[i]);
+        }
+        if (verbose)
+                printf("\n");
+}
+
 int main(int argc, char *argv[]) {
         test_setup_logging(LOG_DEBUG);
 
@@ -373,6 +624,9 @@ int main(int argc, char *argv[]) {
         FOREACH_ELEMENT(desc, option_tests)
                 test_options(desc);
 
+        FOREACH_ELEMENT(desc, long_option_tests)
+                test_option_append_long(desc);
+
         test_option_set();
 
         FOREACH_ELEMENT(desc, option_tests) {

src/systemd/sd-dhcp-protocol.h

@@ -21,7 +21,11 @@
 
 _SD_BEGIN_DECLARATIONS;
 
-/* https://www.iana.org/assignments/bootp-dhcp-parameters/bootp-dhcp-parameters.xhtml#options */
+/* https://www.iana.org/assignments/bootp-dhcp-parameters/bootp-dhcp-parameters.xhtml#options
+ *
+ * When adding a new option, please update the option classification in dhcp-option.c function
+ * `dhcp_option_can_merge` relative to their RFC3396 behavior.
+ */
 enum {
         SD_DHCP_OPTION_PAD                            = 0,   /* [RFC2132] */
         SD_DHCP_OPTION_SUBNET_MASK                    = 1,   /* [RFC2132] */