dhcpv6: implement RFC9686
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.
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.
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_leaseto indicate that this is a self-registered address and not a "normal" lease - That status should probably be included in the
ubusflags 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.
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_enableto 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...
- 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_leaseto 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 :)
OK @Alphix last commit addresses 'exactly one' and:
I think you should add a bool to
struct dhcpv6_leaseto indicate that this is a self-registered address and not a "normal" leaseThat status should probably be included in the
ubusflags for the DHCPv6 lease
- 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 :)
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.
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...
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
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)...
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 😅
OK - the exchanges work with this, although the lease handling is still not yet ready.
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...
Implemented in 6c. Easiest way to test.
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
}
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...
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.
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
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.