Kea lease incorrectly classified as "static" when reservation exists on a different subnet

[dseven]

Important notices

Before you add a new report, we ask you kindly to acknowledge the following:

• I have read the contributing guidelines at https://github.com/opnsense/core/blob/master/CONTRIBUTING.md
• I am convinced that my issue is new after having checked both open and closed issues at https://github.com/opnsense/core/issues?q=is%3Aissue

Describe the bug

This applies to both IPv4 and IPv6.

When viewing Kea leases, the "Lease Type" column shows "static" if there exists a reservation matching the identity of the DHCP client (I suppose). It appears that this mapping does not take subnet into account, as in the case where a reservation for a given client exists on one subnet, any lease given to that client on a different subnet gets classified as "static", where it is actually "dynamic" (i.e. assigned from a pool, not a reservation).

To Reproduce

Steps to reproduce the behavior:

1. Create a reservation (v4 or v6) based on DUID or MAC.
2. Connect a client using that DUID or MAC to a different subnet that is also served by Kea DHCP and has a pool
3. View leases
4. Observe that a lease has been granted using a pool address, but is classified as "static"

Expected behavior

Leases should only be classified as "static" if a matching reservation exists on the same subnet

Screenshots

Environment

OPNsense 26.1.6-amd64

[Monviech]

I would not frame it as incorrect as the client identity is the MAC and DUID, not the subnet it belongs to.

core/src/opnsense/mvc/app/controllers/OPNsense/Kea/Api/LeasesController.php

Lines 64 to 79 in f648476

	// Mark records as reserved based on hwaddr (IPv4) or duid/hwaddr (IPv6) match
	$resv4 = [];
	$resv6 = [];
	foreach ((new KeaDhcpv4())->reservations->reservation->iterateItems() as $reservation) {
	$resv4[strtolower($reservation->hw_address->getValue())] = 'hwaddr';
	}
	foreach ((new KeaDhcpv6())->reservations->reservation->iterateItems() as $reservation) {
	// At least one of these is required in the model
	if (!$reservation->duid->isEmpty()) {
	$resv6[strtolower($reservation->duid->getValue())] = 'duid';
	} elseif (!$reservation->hw_address->isEmpty()) {
	$resv6[strtolower($reservation->hw_address->getValue())] = 'hwaddr';
	}
	}

There isn't much more that can be done here without drowning the already overloaded lease type into more ambiguity.

[dseven]

Have to disagree here. It's saying that the lease is static, when it clearly is not. That's just wrong. The lease exists within the context of the subnet. The fact that the identity of the client owning the lease is associated with a reservation in another subnet does not make this particular lease "static".

Maybe I'm missing something, but reservations and leases both have a subnet-id attribute. Can't we just check if the values are equal when deciding whether or not a match deems a lease "static"?

[Monviech]

I can see how a misconfiguration can happen because:

core/src/opnsense/mvc/app/models/OPNsense/Kea/KeaDhcpv6.xml

Lines 255 to 279 in f648476

	<duid type="TextField">
	<Mask>/^(?:[0-9A-Fa-f]{2}(?::[0-9A-Fa-f]{2})+)$/</Mask>
	<ValidationMessage>Value must be a colon-separated hexadecimal sequence (e.g., 01:02:f3).</ValidationMessage>
	<Constraints>
	<check001>
	<ValidationMessage>Duplicate entry exists.</ValidationMessage>
	<type>UniqueConstraint</type>
	<addFields>
	<field1>subnet</field1>
	</addFields>
	</check001>
	</Constraints>
	</duid>
	<hw_address type="MacAddressField">
	<Constraints>
	<check001>
	<ValidationMessage>Duplicate entry exists.</ValidationMessage>
	<type>UniqueConstraint</type>
	<caseInsensitive>Y</caseInsensitive>
	<addFields>
	<field1>subnet</field1>
	</addFields>
	</check001>
	</Constraints>
	</hw_address>

Both of these have a unique check, but there is no relation between them.

So a client identity can be created either via MAC and DUID in separate entries.
Essentially the root assumption of the issue seems that the input validation needs to be stricter here (to ensure only a single client identity is allowed to exist on all the subnets)

[dseven]

I think you're missing the point. There is no misconfiguration. It's perfectly fine for a reservation to exist in the context of a different subnet. This issue is that the lease is being incorrectly classified as "static" because the matching is too loose. Actually that's an assumption on my part - I should look for the part of the code that makes the static/dynamic determination - but it fits the symptom.

[Monviech]

No, I think something must be wrong with some input validation.

Doesnt this trigger if your IP address and your subnet are not matching?

core/src/opnsense/mvc/app/models/OPNsense/Kea/KeaDhcpv6.php

Lines 57 to 59 in f648476

	if (!Util::isIPInCIDR($reservation->ip_address->getValue(), $subnet) && !$reservation->ip_address->isEmpty()) {
	$messages->appendMessage(new Message(gettext("Address not in specified subnet"), $key . ".ip_address"));
	}

Edit:

Or is your main issue this:

Reservation exists on subnet A, client appears on subnet B, receives a dynamic lease from subnet B pool, lease table marks it static because MAC/DUID exists in reservation config.

If it is I misinterpreted your issue, sorry about that.

[dseven]

Or is your main issue this:

Reservation exists on subnet A, client appears on subnet B, receives a dynamic lease from subnet B pool, lease table marks it static because MAC/DUID exists in reservation config.

Yes! I thought I described that quite clearly?

I would say "exists in reservation config , even though it is in the context of a different subnet"

[Monviech]

I would rather move this to the KEA API if possible so I don't have to "invent" the staticness of a reservation if possible.

When using
https://kea.readthedocs.io/en/latest/api.html#lease6-get-all

does the "state" change if a lease is reserved?

I can see in the documentation it specifies:
https://kea.readthedocs.io/en/latest/api.html#lease6-get-by-state

Something about a "state": "registered", does that mean a lease has a reservation?

[dseven]

Funnily enough, I was just trying to find that out myself this morning. I was going to file another issue suggesting that the state attribute be displayed in the lease tables, but I'm having a hard time finding good documentation of the possible values. I can't find any explanation of the "registered" (4) value means. I can say empirically that it does not mean that there's a reservation, as all of my active reservation-based leases have state 0 (which seems to represent something like "active"). The API doc link you shared above shows an example value of 6, which I think it completely bogus - I haven't seen anything greater than 4: https://github.com/isc-projects/kea/blob/ac489f88002ca0c640b4fa60991d4950b3d8ce19/src/lib/dhcpsrv/lease.h#L65-L81

Unfortunately I don't see anything in the API response that tells us whether a lease was assigned an address from a reservation or a pool.

[Monviech]

That's quite unfortunate, as that means we must continue with assumptions. And however good I make them they will not reflect the truth.

It would be nice if KEA could return directly what is registered at an exact moment in time (when polling the API) or not.

Because we also never know if its internal decision engine leads towards a successful static lease, or not. (As seen in other tickets and in the forum etc...)

I can see if the assumptions can be improved a little, so I'm taking this, but the state they reflect will be always an assumption, even if it's a good one.

[Monviech]

I have a feeling we could be a little more creative inside the python script that yields all leases:

1. Get the current dhcpv4 and dhcpv6 configs from the socket
2. Build a map of all reservations and their parent subnet ids
3. Compare with the output of lease4-get-all and lease6-get-all (there are subnet-ids in there)
4. Add the key "mac" or "duid" keys directly inside there to the "lease-type" key

Then we can remove the guessing from the controller which does not know about the subnet-ids since they are generated during config generation in the model.

Such a concept should be doable, but it also shows that this is no little task either.

[dseven]

There is https://kea.readthedocs.io/en/stable/api.html#reservation-get-by-id too