Opened 3 years ago

Closed 2 years ago

#3652 closed defect (fixed)

Host Reservation configuration allows for multiple reservations for the same address

Reported by: marcin Owned by: fdupont
Priority: low Milestone: Kea0.9.2-beta
Component: ~dhcpconf(obsolete) Version: git
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DHCP Feature Depending on Ticket:
Estimated Difficulty: 0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

The CfgHosts class should verify that the reservation being added uses an address reserved already and reject it.

Subtickets

Change History (12)

comment:1 Changed 3 years ago by tomek

  • Milestone changed from Kea-proposed to Kea0.9.2

comment:2 Changed 3 years ago by hschempf

  • Priority changed from medium to low

comment:3 Changed 3 years ago by fdupont

According to the comment in lib/dhcpsrv/cfg_hosts.h this is not feasible when per hardware address and per DUID reservations are mixed:

The server is unable to verify that the specific DUID and HW address belong to the same client, until the client sends a DHCP message.

so we can raise a false positive when 2 equivalent hw-address/DUIDreservations are entered for the same host. As hw-address and DUID are mutually exclusive this case is a real world one...

comment:4 Changed 3 years ago by fdupont

The same IPv6 address case is already rejected and there is an unit test (add6Invalid2Hosts) to check it.
IPv4 is different, not yet handled and IMHO requiring a new exception (DuplicateAddress??).

comment:5 Changed 3 years ago by fdupont

  • Owner set to fdupont
  • Status changed from new to accepted

comment:6 Changed 3 years ago by fdupont

Added for the IPv4 already reserved case:

  • new exception ReservedAddress
  • new check in add4()
  • fixed unit tests on multiple host entries
  • a specific unit test add4AlreadyReserved.

Ready for review. For the ChangeLog something like Check if a new reservation tries to use an already reserved IPv4 address.

comment:7 Changed 3 years ago by fdupont

  • Owner changed from fdupont to UnAssigned
  • Status changed from accepted to reviewing

comment:8 Changed 2 years ago by marcin

  • Owner changed from UnAssigned to marcin

comment:9 follow-up: Changed 2 years ago by marcin

  • Owner changed from marcin to fdupont

Reviewed commit 1fdd6924a600a5b8b9113189fbd33fcf604bec60

The changes look good. The only concern I have is that we throw different exception type for v4 case and different for the v6 case when trying to reserve already reserved address. I actually think that in the v6 case we should maybe also throw ReservedAddress exception, rather than DuplicateHost?

As for the ChangeLog entry, I propose: "libdhcpsrv: check if new host reservation tries to use an already reserved address".

comment:10 in reply to: ↑ 9 Changed 2 years ago by fdupont

The errors are very different in DHCPv4 and DHCPv6:

  • in v4 it is an address which is assigned by accident to 2 different clients (so ReversedAddress).
  • in v6 it is a client which is defined with both its identifiers (so DuplicateHost).

Of course things are not so clear but there is no easy way to really distinguish between so IMHO the best is to keep IPv4 addresses are rare vs IPv6 offers 2 identifiers.

comment:11 Changed 2 years ago by fdupont

Merged. Note there are some related tickets: #3746, #3704, etc, so the subject itself is not closed.

comment:12 Changed 2 years ago by fdupont

  • Resolution set to fixed
  • Status changed from reviewing to closed
Note: See TracTickets for help on using tickets.