Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

sys/net/dhcpv6: include IA Prefix Option in SOLICIT #19225

Merged
merged 3 commits into from
Feb 2, 2023

Conversation

benpicco
Copy link
Contributor

Contribution description

I was always wondering why I would get a /62 from my DHCPv6 server when dhcpv6_client_req_ia_pd() is called with a prefix length of 64.
Turns out that prefix length was never included in the request.

This is a problem as e.g. Linux clients do not accept router advertisements for SLAAC if the prefix length != 64.

Add the IA Prefix Option when soliciting a prefix so we can tell the server what prefix length we want.

Testing procedure

Can be tested on native with your local DHCPv6 server:

  • create TAP bridge with LAN

    sudo dist/tools/tapsetup/tapsetup -u eno1
    
  • run gnrc_border_router

    make -C examples/gnrc_border_router REUSE_TAP=1 all term
    

The DHCPv6 server now returns a prefix with the requested length

DHCPv6 client: send SOLICIT
DHCPv6 client: resend SOLICIT
DHCPv6 client: received ADVERTISE
DHCPv6 client: scheduling REQUEST
DHCPv6 client: send REQUEST
DHCPv6 client: received REPLY
DHCPv6 client: scheduling RENEW in 1800 sec
DHCPv6 client: scheduling REBIND in 2880 sec

> nib prefix
2001:9e8:1425:e00::/64 dev #6  expires 7195 sec deprecates 3595 sec
2001:9e8:1425:efb::/64 dev #7  expires 7197 sec deprecates 3597 sec

Issues/PRs references

@benpicco benpicco added the Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) label Jan 31, 2023
@github-actions github-actions bot added Area: network Area: Networking Area: sys Area: System labels Jan 31, 2023
@benpicco benpicco changed the title sys/net/dhcpv6: add IA Prefix Option in SOLICIT sys/net/dhcpv6: include IA Prefix Option in SOLICIT Jan 31, 2023
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 31, 2023
@riot-ci
Copy link

riot-ci commented Jan 31, 2023

Murdock results

✔️ PASSED

a693ecc examples/gnrc_border_router: set number of leases to number of ZEPs

Success Failures Total Runtime
6807 0 6807 09m:21s

Artifacts

@github-actions github-actions bot added the Area: examples Area: Example Applications label Jan 31, 2023
@benpicco
Copy link
Contributor Author

I added two more small quality of life improvements.
I can move them to a separate PR if needed, but only minor cleanups, so might as well include them here.

@fabian18
Copy link
Contributor

fabian18 commented Feb 1, 2023

I have not read RFC 8415, but I seeked for "Prefix Option" within it.

For the record:

The client MAY include values in IA Prefix options (see
Section 21.22) encapsulated within IA_PD options as hints for the
delegated prefix and/or prefix length for which the client has a
preference. See Section 18.2.4 for more on prefix-length hints.

The client MAY include an IA option for each binding it desires but
has been unable to obtain. In this case, if the client includes the
IA_PD option to request prefix delegation, the client MAY include the
IA Prefix option encapsulated within the IA_PD option, with the
"IPv6-prefix" field set to 0 and the "prefix-length" field set to the
desired length of the prefix to be delegated.

The IA Prefix Option belongs in the IA Prefix Delegation Option. It is not a separate option apparently.

The client SHOULD NOT send an IA Prefix option with 0 in the
"prefix-length" field (and an unspecified value (::) in the
"IPv6-prefix" field). A client MAY send a non-zero value in the
"prefix-length" field and the unspecified value (::) in the
"IPv6-prefix" field to indicate a preference for the size of the
prefix to be delegated. See [RFC8168] for further details on prefix-
length hints.

Prefix length should be 0 and prefix should be ::.
Our client seems to put it into a Solicit and Request message.
In the Solicit message the prefix is correctly :: but in the Request message it seems to be uninitialized.
The server replies correctly with the Prefix Option inside the Prefix Delegation option.
See the two wireshark captures: dhcpv6_.zip

@benpicco
Copy link
Contributor Author

benpicco commented Feb 1, 2023

Good catch, we actually have to zero out the prefix.

@fabian18
Copy link
Contributor

fabian18 commented Feb 1, 2023

The IA Prefix Option belongs in the IA Prefix Delegation Option. It is not a separate option apparently.

So I think the IA PD option is reverse constructed like this, or similar:

patch
diff --git a/sys/net/application_layer/dhcpv6/client.c b/sys/net/application_layer/dhcpv6/client.c
index b33aeaa528..8d7471d7a8 100644
--- a/sys/net/application_layer/dhcpv6/client.c
+++ b/sys/net/application_layer/dhcpv6/client.c
@@ -15,6 +15,7 @@
 
 #include <assert.h>
 #include <stdbool.h>
+#include <string.h>
 
 #include "event.h"
 #include "event/timeout.h"
@@ -448,11 +449,11 @@ static inline size_t _compose_iapfx_opt(dhcpv6_opt_iapfx_t *iapfx,
     if (lease->pfx_len == 0) {
         return 0;
     }
+    /* set all unused/requested fields to 0 */
+    memset(iapfx, 0, sizeof(*iapfx));
 
     iapfx->type = byteorder_htons(DHCPV6_OPT_IAPFX);
     iapfx->len = byteorder_htons(len);
-    iapfx->pref = byteorder_htonl(0);
-    iapfx->valid = byteorder_htonl(0);
     iapfx->pfx_len = lease->pfx_len;
 
     return len + sizeof(dhcpv6_opt_t);
@@ -505,28 +506,33 @@ static inline size_t _add_ia_pd_from_config(uint8_t *buf, size_t len_max)
     if (!IS_USED(MODULE_DHCPV6_CLIENT_IA_PD)) {
         return 0;
     }
-
-    size_t msg_len = 0;
+    uint8_t *start = buf, *end = &buf[len_max];
 
     for (unsigned i = 0; i < CONFIG_DHCPV6_CLIENT_PFX_LEASE_MAX; i++) {
         uint32_t ia_id = pfx_leases[i].parent.ia_id.id;
         if (ia_id != 0) {
+            size_t len = 0;
+            /* add IA Prefix Option included in IA Prefix Delegation Option */
+            dhcpv6_opt_iapfx_t *pa_pfx = (dhcpv6_opt_iapfx_t *)(end - sizeof(dhcpv6_opt_iapfx_t));
+            if ((uint8_t *)pa_pfx < start) {
+                assert(0); /* not supposed to happen */
+                return 0;
+            }
+            len = _compose_iapfx_opt(pa_pfx, &pfx_leases[i], len);
+            end -= len;
             /* add Identity Association for Prefix Delegation Option */
-            dhcpv6_opt_ia_pd_t *ia_pd = (dhcpv6_opt_ia_pd_t *)(&buf[msg_len]);
-            msg_len += _compose_ia_pd_opt(ia_pd, ia_id, 0);
-
-            /* add IA Prefix Option */
-            dhcpv6_opt_iapfx_t *pa_pfx = (dhcpv6_opt_iapfx_t *)(&buf[msg_len]);
-            msg_len += _compose_iapfx_opt(pa_pfx, &pfx_leases[i], 0);
+            dhcpv6_opt_ia_pd_t *ia_pd = (dhcpv6_opt_ia_pd_t *)(end - sizeof(dhcpv6_opt_ia_pd_t));
+            if ((uint8_t *)ia_pd < start) {
+                assert(0); /* not supposed to happen */
+                return 0;
+            }
+            len = _compose_ia_pd_opt(ia_pd, ia_id, len);
+            /* move to buffer start */
+            memmove(start, ia_pd, len);
+            start += len;
         }
     }
-
-    if (msg_len > len_max) {
-        assert(0);
-        return 0;
-    }
-
-    return msg_len;
+    return start - buf;
 }
 
 static inline int32_t get_rand_ms_factor(void)

@benpicco
Copy link
Contributor Author

benpicco commented Feb 1, 2023

I think there is no need to move memory around, let's keep things simple and move the sub-option generation into the function the generates the option.

Copy link
Contributor

@fabian18 fabian18 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, it works fine. Please squash

@benpicco
Copy link
Contributor Author

benpicco commented Feb 2, 2023

bors merge

@bors
Copy link
Contributor

bors bot commented Feb 2, 2023

Build succeeded:

@bors bors bot merged commit a9dbf8b into RIOT-OS:master Feb 2, 2023
@benpicco benpicco deleted the dhcpv6_opt_iapfx_t branch February 2, 2023 02:24
@MrKevinWeiss MrKevinWeiss added this to the Release 2023.04 milestone Apr 25, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Area: examples Area: Example Applications Area: network Area: Networking Area: sys Area: System CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants