Skip to content

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

New issue

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

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

Already on GitHub? Sign in to your account

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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