From 1cc074167fd29c6c8c9dc5d7fd6c38e4dd91ca71 Mon Sep 17 00:00:00 2001
From: Yu Watanabe <watanabe.yu+github@gmail.com>
Date: Thu, 23 Sep 2021 14:58:28 +0900
Subject: [PATCH] sd-dhcp6-client: modernize dhcp6_option_parse()

- merge dhcp6_option_parse() with option_parse_hdr().
- do not assign/update any values on error.
- use assert() instead of assert_return(), as the assertions cannot
  be triggered by a library user.

(cherry picked from commit b89a3758e92894162e3c2dcb594a55acff3274d5)

Conflict:NA
Reference:https://github.com/systemd/systemd/commit/1cc074167fd29c6c8c9dc5d7fd6c38e4dd91ca71
---
 src/libsystemd-network/dhcp6-internal.h    | 10 +++-
 src/libsystemd-network/dhcp6-option.c      | 60 ++++++++++------------
 src/libsystemd-network/sd-dhcp6-lease.c    | 37 ++++++-------
 src/libsystemd-network/test-dhcp6-client.c | 50 +++++++++---------
 4 files changed, 76 insertions(+), 81 deletions(-)

diff --git a/src/libsystemd-network/dhcp6-internal.h b/src/libsystemd-network/dhcp6-internal.h
index 96d7de8cae..8d083d3858 100644
--- a/src/libsystemd-network/dhcp6-internal.h
+++ b/src/libsystemd-network/dhcp6-internal.h
@@ -101,8 +101,14 @@ int dhcp6_option_append_fqdn(uint8_t **buf, size_t *buflen, const char *fqdn);
 int dhcp6_option_append_user_class(uint8_t **buf, size_t *buflen, char * const *user_class);
 int dhcp6_option_append_vendor_class(uint8_t **buf, size_t *buflen, char * const *user_class);
 int dhcp6_option_append_vendor_option(uint8_t **buf, size_t *buflen, OrderedHashmap *vendor_options);
-int dhcp6_option_parse(uint8_t **buf, size_t *buflen, uint16_t *optcode,
-                       size_t *optlen, uint8_t **optvalue);
+
+int dhcp6_option_parse(
+                const uint8_t *buf,
+                size_t buflen,
+                size_t *offset,
+                uint16_t *ret_option_code,
+                size_t *ret_option_data_len,
+                const uint8_t **ret_option_data);
 int dhcp6_option_parse_status(DHCP6Option *option, size_t len);
 int dhcp6_option_parse_ia(sd_dhcp6_client *client, DHCP6Option *iaoption, be32_t iaid, DHCP6IA *ia, uint16_t *ret_status_code);
 int dhcp6_option_parse_ip6addrs(const uint8_t *optval, uint16_t optlen,
diff --git a/src/libsystemd-network/dhcp6-option.c b/src/libsystemd-network/dhcp6-option.c
index 0709cfd4fd..781d391c0c 100644
--- a/src/libsystemd-network/dhcp6-option.c
+++ b/src/libsystemd-network/dhcp6-option.c
@@ -370,47 +370,39 @@ int dhcp6_option_append_vendor_class(uint8_t **buf, size_t *buflen, char * const
         return dhcp6_option_append(buf, buflen, SD_DHCP6_OPTION_VENDOR_CLASS, total, p);
 }
 
-static int option_parse_hdr(uint8_t **buf, size_t *buflen, uint16_t *optcode, size_t *optlen) {
-        DHCP6Option *option = (DHCP6Option*) *buf;
-        uint16_t len;
-
-        assert_return(buf, -EINVAL);
-        assert_return(optcode, -EINVAL);
-        assert_return(optlen, -EINVAL);
-
-        if (*buflen < offsetof(DHCP6Option, data))
-                return -ENOMSG;
-
-        len = be16toh(option->len);
-
-        if (len > *buflen)
-                return -ENOMSG;
-
-        *optcode = be16toh(option->code);
-        *optlen = len;
+int dhcp6_option_parse(
+                const uint8_t *buf,
+                size_t buflen,
+                size_t *offset,
+                uint16_t *ret_option_code,
+                size_t *ret_option_data_len,
+                const uint8_t **ret_option_data) {
 
-        *buf += 4;
-        *buflen -= 4;
+        const DHCP6Option *option;
+        size_t len;
 
-        return 0;
-}
+        assert(buf);
+        assert(offset);
+        assert(ret_option_code);
+        assert(ret_option_data_len);
+        assert(ret_option_data);
 
-int dhcp6_option_parse(uint8_t **buf, size_t *buflen, uint16_t *optcode,
-                       size_t *optlen, uint8_t **optvalue) {
-        int r;
+        if (buflen < offsetof(DHCP6Option, data))
+                return -EBADMSG;
 
-        assert_return(buf && buflen && optcode && optlen && optvalue, -EINVAL);
+        if (*offset >= buflen - offsetof(DHCP6Option, data))
+                return -EBADMSG;
 
-        r = option_parse_hdr(buf, buflen, optcode, optlen);
-        if (r < 0)
-                return r;
+        option = (const DHCP6Option*) (buf + *offset);
+        len = be16toh(option->len);
 
-        if (*optlen > *buflen)
-                return -ENOBUFS;
+        if (len > buflen - offsetof(DHCP6Option, data) - *offset)
+                return -EBADMSG;
 
-        *optvalue = *buf;
-        *buflen -= *optlen;
-        *buf += *optlen;
+        *offset += offsetof(DHCP6Option, data) + len;
+        *ret_option_code = be16toh(option->code);
+        *ret_option_data_len = len;
+        *ret_option_data = option->data;
 
         return 0;
 }
diff --git a/src/libsystemd-network/sd-dhcp6-lease.c b/src/libsystemd-network/sd-dhcp6-lease.c
index 9082185bca..6375a22537 100644
--- a/src/libsystemd-network/sd-dhcp6-lease.c
+++ b/src/libsystemd-network/sd-dhcp6-lease.c
@@ -259,9 +259,6 @@ int sd_dhcp6_lease_get_domains(sd_dhcp6_lease *lease, char ***domains) {
 
 int dhcp6_lease_set_ntp(sd_dhcp6_lease *lease, uint8_t *optval, size_t optlen) {
         int r;
-        uint16_t subopt;
-        size_t sublen;
-        uint8_t *subval;
 
         assert_return(lease, -EINVAL);
         assert_return(optval, -EINVAL);
@@ -269,10 +266,14 @@ int dhcp6_lease_set_ntp(sd_dhcp6_lease *lease, uint8_t *optval, size_t optlen) {
         lease->ntp = mfree(lease->ntp);
         lease->ntp_count = 0;
 
-        while ((r = dhcp6_option_parse(&optval, &optlen, &subopt, &sublen,
-                                       &subval)) >= 0) {
-                int s;
-                char **servers;
+        for (size_t offset = 0; offset < optlen;) {
+                const uint8_t *subval;
+                size_t sublen;
+                uint16_t subopt;
+
+                r = dhcp6_option_parse(optval, optlen, &offset, &subopt, &sublen, &subval);
+                if (r < 0)
+                        return r;
 
                 switch(subopt) {
                 case DHCP6_NTP_SUBOPTION_SRV_ADDR:
@@ -280,19 +281,18 @@ int dhcp6_lease_set_ntp(sd_dhcp6_lease *lease, uint8_t *optval, size_t optlen) {
                         if (sublen != 16)
                                 return 0;
 
-                        s = dhcp6_option_parse_ip6addrs(subval, sublen,
-                                                        &lease->ntp,
-                                                        lease->ntp_count);
-                        if (s < 0)
-                                return s;
+                        r = dhcp6_option_parse_ip6addrs(subval, sublen, &lease->ntp, lease->ntp_count);
+                        if (r < 0)
+                                return r;
 
-                        lease->ntp_count = s;
+                        lease->ntp_count = r;
 
                         break;
 
-                case DHCP6_NTP_SUBOPTION_SRV_FQDN:
-                        r = dhcp6_option_parse_domainname_list(subval, sublen,
-                                                               &servers);
+                case DHCP6_NTP_SUBOPTION_SRV_FQDN: {
+                        char **servers;
+
+                        r = dhcp6_option_parse_domainname_list(subval, sublen, &servers);
                         if (r < 0)
                                 return 0;
 
@@ -300,12 +300,9 @@ int dhcp6_lease_set_ntp(sd_dhcp6_lease *lease, uint8_t *optval, size_t optlen) {
                         lease->ntp_fqdn_count = r;
 
                         break;
-                }
+                }}
         }
 
-        if (r != -ENOMSG)
-                return r;
-
         return 0;
 }
 
diff --git a/src/libsystemd-network/test-dhcp6-client.c b/src/libsystemd-network/test-dhcp6-client.c
index 5e3b191595..5d1f709f11 100644
--- a/src/libsystemd-network/test-dhcp6-client.c
+++ b/src/libsystemd-network/test-dhcp6-client.c
@@ -170,47 +170,47 @@ static int test_option(sd_event *e) {
                 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
                 'B', 'A', 'R',
         };
+        size_t offset, pos, optlen, outlen = sizeof(result);
+        const uint8_t *optval;
         uint16_t optcode;
-        size_t optlen;
-        uint8_t *optval, *buf, *out;
-        size_t zero = 0, pos = 3;
-        size_t buflen = sizeof(packet), outlen = sizeof(result);
+        uint8_t *out;
 
         log_debug("/* %s */", __func__);
 
-        assert_se(buflen == outlen);
+        assert_se(sizeof(packet) == sizeof(result));
 
-        assert_se(dhcp6_option_parse(&buf, &zero, &optcode, &optlen,
-                                     &optval) == -ENOMSG);
+        offset = 0;
+        assert_se(dhcp6_option_parse(packet, 0, &offset, &optcode, &optlen, &optval) == -EBADMSG);
 
-        buflen -= 3;
-        buf = &packet[3];
-        outlen -= 3;
-        out = &result[3];
+        offset = 3;
+        assert_se(dhcp6_option_parse(packet, 0, &offset, &optcode, &optlen, &optval) == -EBADMSG);
+
+        offset = 3;
+        assert_se(dhcp6_option_parse(packet, sizeof(packet), &offset, &optcode, &optlen, &optval) >= 0);
 
-        assert_se(dhcp6_option_parse(&buf, &buflen, &optcode, &optlen,
-                                     &optval) >= 0);
-        pos += 4 + optlen;
-        assert_se(buf == &packet[pos]);
         assert_se(optcode == SD_DHCP6_OPTION_ORO);
         assert_se(optlen == 7);
-        assert_se(buflen + pos == sizeof(packet));
+        assert_se(optval == packet + 7);
+
+        pos = 3;
+        outlen -= 3;
+        out = &result[3];
 
-        assert_se(dhcp6_option_append(&out, &outlen, optcode, optlen,
-                                      optval) >= 0);
+        assert_se(dhcp6_option_append(&out, &outlen, optcode, optlen, optval) >= 0);
+
+        pos += 4 + optlen;
         assert_se(out == &result[pos]);
         assert_se(*out == 0x00);
 
-        assert_se(dhcp6_option_parse(&buf, &buflen, &optcode, &optlen,
-                                     &optval) >= 0);
-        pos += 4 + optlen;
-        assert_se(buf == &packet[pos]);
+        assert_se(dhcp6_option_parse(packet, sizeof(packet), &offset, &optcode, &optlen, &optval) >= 0);
+
         assert_se(optcode == SD_DHCP6_OPTION_VENDOR_CLASS);
         assert_se(optlen == 9);
-        assert_se(buflen + pos == sizeof(packet));
+        assert_se(optval == packet + 18);
+
+        assert_se(dhcp6_option_append(&out, &outlen, optcode, optlen, optval) >= 0);
 
-        assert_se(dhcp6_option_append(&out, &outlen, optcode, optlen,
-                                      optval) >= 0);
+        pos += 4 + optlen;
         assert_se(out == &result[pos]);
         assert_se(*out == 'B');
 
-- 
2.33.0

