odhcpd icon indicating copy to clipboard operation
odhcpd copied to clipboard

dhcpv6: implement RFC9686

Open systemcrash opened this issue 2 months ago • 17 comments

Registering Self-Generated IPv6 Addresses Using DHCPv6

This functionality registers a client SLAAC or static address in the DHCPv6 pool, primarily for diagnostic purposes as outlined in §1.

Most of the machinery required for this functionality already exists, so we perform a few additional sets and checks in the IA code.

The address registration mechanism overview:

+--------+        +------------------+       +---------------+
| CLIENT |        | FIRST-HOP ROUTER |       | DHCPv6 SERVER |
+--------+        +---------+--------+       +-------+-------+
    |      SLAAC            |                        |
    |<--------------------> |                        |
    |                       |                        |
    |                                                |
    |  src: link-local address                       |
    | -------------------------------------------->  |
    |    INFORMATION-REQUEST or SOLICIT/...          |
    |       - OPTION REQUEST OPTION                  |
    |          -- OPTION_ADDR_REG_ENABLE             |
    |                                                |
    |    ...                                         |
    |                                                |
    |                                                |
    |<---------------------------------------------  |
    |     REPLY or ADVERTISE MESSAGE                 |
    |       - OPTION_ADDR_REG_ENABLE                 |
    |                                                |
    |                                                |
    |  src: address being registered                 |
    | -------------------------------------------->  |
    |    ADDR-REG-INFORM MESSAGE                     |Register/
    |                                                |log addresses
    |                                                |
    |                                                |
    | <--------------------------------------------  |
    |        ADDR-REG-REPLY MESSAGE                  |
    |                                                |

Closes #349

Might require a few tweaks. This code is untested, but simple enough to merge and see production when this option appears (or we add it in odhcp6c and test that way - ULA are supposed to be registered).

@Alphix and @Noltari at your leisure.

systemcrash avatar Dec 14 '25 01:12 systemcrash

Note to self: do we want a configure tuneable? Or do we just set client_addr_reg_enable to true only if M or O flags are set. The client SHOULD NOT send the ADDR-REG-INFORM message unless it has received a Router Advertisement (RA) message with either the M or O flags set to 1.

systemcrash avatar Dec 14 '25 01:12 systemcrash

And some more random points that don't fit some specific line(s) in the diff:

  • I think that there's no check that the to-be-registered address is of global scope? (GUA/ULA)
  • I think you should add a bool to struct dhcpv6_lease to indicate that this is a self-registered address and not a "normal" lease
  • That status should probably be included in the ubus flags for the DHCPv6 lease

(personal opinion) The RFC comments are slowly getting a bit out of hand, while a quick:

/* See RFC527, second paragraph */
if (something_unexpected)
        do_something_drastic();

Is helpful, citing ever longer passages from RFCs just blows up the size of the code, and anyone who is interested should really go read the referred-to paragraph/section of the relevant RFC anyway to make sure that they get the whole context.

Alphix avatar Dec 14 '25 10:12 Alphix

Note to self: do we want a configure tuneable?

I don't think it's necessary...if we're running a DHCPv6 server, it doesn't seem to serve any real purpose to not allow clients to inform us of their autoconfigured addresses...?

Or do we just set client_addr_reg_enable to true only if M or O flags are set.

If the M and O flags aren't set, we shouldn't expect to get any DHCPv6 requests in the first place? But another interesting twist would be if A isn't set and a client address registration msg comes in...we've told clients not to do autoconf...

Alphix avatar Dec 14 '25 10:12 Alphix

  • I think that there's no check that the to-be-registered address is of global scope? (GUA/ULA)

You're right. There is not (yet). The spec defines no server behaviour for this in §4.2.1.

It's mandated as part of §4.2 - what the client must do. ( It does no harm if the client erroneously lodges a LL with us. It's nothing that we would assign anyway from our available prefixes. IOW, it's also harmless for us to check this somewhere. And the implied behaviour is... that we discard it. )

static inline bool IN6_IS_ADDR_GLOBAL(const struct in6_addr *a)
{
	return !(
		IN6_IS_ADDR_UNSPECIFIED(a) ||
		IN6_IS_ADDR_LOOPBACK(a) ||
		IN6_IS_ADDR_MULTICAST(a) ||
		IN6_IS_ADDR_LINKLOCAL(a) ||
		IN6_IS_ADDR_ULA(a)
	);
}

?

https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/netinet_in.h.html defines others.

Edit: I think this is largely handled by if (ia_addr_present && !dhcpv6_ia_on_link(ia, a, iface)) ... else ...

  • I think you should add a bool to struct dhcpv6_lease to indicate that this is a self-registered address and not a "normal" lease

Not a bad idea.

(personal opinion) The RFC comments are slowly getting a bit out of hand, while a quick:

Hmm - understand. I think 2-3 lines for a code-block is fine, with exceptions for complex bits. Because this RFC basically uses existing functionality with some extra ifs, it's much clearer that we cover all of the MUST parts in the server behaviour (e.g. via a code search) since most machinery is already implicit.

In short, it'd be nice to get this tested before making more changes, or... you could write out your implementation and we compare notes :)

systemcrash avatar Dec 14 '25 15:12 systemcrash

OK @Alphix last commit addresses 'exactly one' and:

  • I think you should add a bool to struct dhcpv6_lease to indicate that this is a self-registered address and not a "normal" lease

  • That status should probably be included in the ubus flags for the DHCPv6 lease

systemcrash avatar Dec 14 '25 15:12 systemcrash

  • I think that there's no check that the to-be-registered address is of global scope? (GUA/ULA)

You're right. There is not (yet). The spec defines no server behaviour for this in §4.2.1.

It's mandated as part of §4.2 - what the client must do. ( It does no harm if the client erroneously lodges a LL with us. It's nothing that we would assign anyway from our available prefixes. IOW, it's also harmless for us to check this somewhere. And the implied behaviour is... that we discard it. ) ... Edit: I think this is largely handled by if (ia_addr_present && !dhcpv6_ia_on_link(ia, a, iface)) ... else ...

Hm, yes, dhcpv6_ia_on_link might actually be enough. The only thing it could get "wrong" is if the list of addresses that dhcpv6_ia_on_link checks against includes our own linklocal address (I haven't checked if it does, but the list comes from netlink, so my guess is that it does).

In short, it'd be nice to get this tested before making more changes, or... you could write out your implementation and we compare notes :)

Yeah, feel free to not do any changes until you've done more tests...odhcp6c support would probably be a good step. I always prefer developing bits and pieces like this with a suitable counterpart to test with...

...and no, I'm not going to write an implementation right now :)

Alphix avatar Dec 14 '25 16:12 Alphix

Thanks for the comprehensive review @Alphix. This already feels like a better result. If those last few changes look good - then I'll squash.

This is the on_link check https://github.com/openwrt/odhcpd/blob/03c1468355c0ffb8fa965cdf57f0d3fc8f01f006/src/dhcpv6-ia.c#L902-L934

where

valid_addr is https://github.com/openwrt/odhcpd/blob/03c1468355c0ffb8fa965cdf57f0d3fc8f01f006/src/dhcpv6-ia.h#L33-L36

That could probably be better named as something like addr_valid_lifetime but for the prefix bit.

systemcrash avatar Dec 14 '25 17:12 systemcrash

Thanks for the comprehensive review @Alphix. This already feels like a better result. If those last few changes look good - then I'll squash.

I think I'll just keep my mouth shut now until you have the chance to test this code. Feel free to squash/not squash as/when you prefer...

Alphix avatar Dec 14 '25 19:12 Alphix

The good news is that most of this is structurally correct, and almost worked on the first try (I got the replies but IA Addr was missing from response because I used type 3 and not 5) :)

Basically, assign_na() stuffs IA_NA(type 3)->IA Addr addresses in our linked list (we have T1, T2, constructed host_id), but RFC9686 sends one naked IA Addr (type 5), while odhcpd IA is set up to handle (non-naked) IA_NA->IA Addr. I think we need a new function specific for these IA Addr that come in (since we also receive ULA which have identical host_id portions).

I'll refresh with what I get working so we can chisel out a good way to handle these records.

DHCPv6
    Message type: Address Registration Inform (36)
    Transaction ID: 0x7ede1d
    Client Identifier
        Option: Client Identifier (1)
        Length: 18
        DUID: 0004xx
        DUID Type: Universally Unique IDentifier (UUID) (4)
        UUID: xx
    IA Address
        Option: IA Address (5)
        Length: 24
        IPv6 address: 2001:db8:10:1100:1200:27ff:fe59:96ef
        Preferred lifetime: 2147
        Valid lifetime: 2147

...
DHCPv6
    Message type: Address Registration Reply (37)
    Transaction ID: 0x7ede1d
    Server Identifier
        Option: Server Identifier (2)
        Length: 10
        DUID: 0003000xx
        DUID Type: link-layer address (3)
        Hardware type: Ethernet (1)
        Link-layer address: xx
        Link-layer address (Ethernet): xx
    Client Identifier
        Option: Client Identifier (1)
        Length: 18
        DUID: 0004xx
        DUID Type: Universally Unique IDentifier (UUID) (4)
        UUID: xx
    SOL_MAX_RT
        Option: SOL_MAX_RT (82)
        Length: 4
        Data: 0000003c
    DNS recursive name server
        Option: DNS recursive name server (23)
        Length: 16
         1 DNS server address: fd51:1c2a:8909::1
    Domain Search List
        Option: Domain Search List (24)
        Length: 5
        Domain name suffix search list
    IA Address
        Option: IA Address (5)
        Length: 24
        IPv6 address: 2001:db8:10:1100:1200:27ff:fe59:96ef
        Preferred lifetime: 2147
        Valid lifetime: 2147

systemcrash avatar Dec 15 '25 12:12 systemcrash

Basically, assign_na() stuffs IA_NA(type 3)->IA Addr addresses in our linked list (we have T1, T2, constructed host_id), but RFC9686 sends one naked IA Addr (type 5), while odhcpd IA is set up to handle (non-naked) IA_NA->IA Addr. I think we need a new function specific for these IA Addr that come in (since we also receive ULA which have identical host_id portions).

What a coincidence, I've been fiddling with the same stuff because of my work with creating better statefiles that can be used to persist leases across a restart/reboot. I think I might have something that can benefit both of us....basically I want to add an array of real IPv6 addresses to struct dhcpv6_lease, so that we can know exactly which addresses have been handed out (the assigned_host_id is not enough for that...consider a situation where an iface gets another prefix after it has already handed out a lease, for example)...

Alphix avatar Dec 15 '25 18:12 Alphix

Yeah, my solution at present is just to jam the peer IP in there (if checks pass), since this RFC mandates the IP Addr == sender addr. Now the ubus lease handling stuff needs tweaking 😅

systemcrash avatar Dec 15 '25 20:12 systemcrash

OK - the exchanges work with this, although the lease handling is still not yet ready.

systemcrash avatar Dec 17 '25 01:12 systemcrash

OK - the exchanges work with this, although the lease handling is still not yet ready.

Did you implement it in odhcp6c as well, or how are you testing the functionality?

Edit: and if you give me some time, I think I'll have something for you wrt. the lease handling...

Alphix avatar Dec 17 '25 07:12 Alphix

Implemented in 6c. Easiest way to test.

systemcrash avatar Dec 17 '25 13:12 systemcrash

OK - I've been testing this one out against 6c, and it works fine:

odhcp6c request

DHCPv6
    Message type: Address Registration Inform (36)
    Transaction ID: 0x793fe6
    Client Identifier
        Option: Client Identifier (1)
        Length: 18
        DUID: 0004xxx
        DUID Type: Universally Unique IDentifier (UUID) (4)
        UUID: xxx
    IA Address
        Option: IA Address (5)
        Length: 24
        IPv6 address: 2001:db8:20:1100:a00:27ff:fe59:96ef
        Preferred lifetime: 1917
        Valid lifetime: 1917

odhcpd reply:

DHCPv6
    Message type: Address Registration Reply (37)
    Transaction ID: 0x793fe6
    Server Identifier
        Option: Server Identifier (2)
        Length: 10
        DUID: 0003xx
        DUID Type: link-layer address (3)
        Hardware type: Ethernet (1)
        Link-layer address: xx
        Link-layer address (Ethernet): xx
    Client Identifier
        Option: Client Identifier (1)
        Length: 18
        DUID: 0004xxx
        DUID Type: Universally Unique IDentifier (UUID) (4)
        UUID: xxx
    IA Address
        Option: IA Address (5)
        Length: 24
        IPv6 address: 2001:db8:20:1100:a00:27ff:fe59:96ef
        Preferred lifetime: 1917
        Valid lifetime: 1917

and ubus call dhcp ipv6leases produces:

{
	"duid": "0004xx",
	"iaid": 0,
	"hostname": "",
	"accept-reconf": false,
	"flags": [
		"bound",
		"self-generated"
	],
	"ipv6-addr": [
		{
			"address": "fd51:1c2a:8909:0:a00:27ff:fe59:96ef",
			"preferred-lifetime": 1325,
			"valid-lifetime": 1325
		}
	],
	"valid": 4297
},
{
	"duid": "0004xx",
	"iaid": 0,
	"hostname": "",
	"accept-reconf": false,
	"flags": [
		"bound",
		"self-generated"
	],
	"ipv6-addr": [
		{
			"address": "2001:db8:20:1100:a00:27ff:fe59:96ef",
			"preferred-lifetime": 1325,
			"valid-lifetime": 1325
		}
	],
	"valid": 1325
}

systemcrash avatar Dec 18 '25 18:12 systemcrash

Awesome, I'll post a draft of the thing I'm working on....hopefully this evening (CET) or tomorrow...I hope you'll find that both of our projects will dovetail nicely...

Alphix avatar Dec 18 '25 18:12 Alphix

Let's see - I'm happy with where things are handling leases at present here - but you typically reveal something I hadn't thought of.

systemcrash avatar Dec 18 '25 18:12 systemcrash

Awesome, I'll post a draft of the thing I'm working on....hopefully this evening (CET) or tomorrow...I hope you'll find that both of our projects will dovetail nicely...

@systemcrash see #364

Alphix avatar Dec 19 '25 11:12 Alphix

I think this is ready for general consumption now. @Alphix you seem to dislike passing loads of parameters, but it feels like we can pass addrbuf once or twice to avoid doing inet_ntop at different steps.

The 6c half needs some more time - cleanup and hooking in to stats. Currently it triggers on RA, which doesn't seem optimal... but it works. So I can throw it out there so at least others can test this.

systemcrash avatar Dec 19 '25 18:12 systemcrash