Opened 5 years ago

Closed 4 years ago

#3887 closed defect (fixed)

option Name Server in DHCPv4 should be an array

Reported by: wlodekwencel Owned by: marcin
Priority: low Milestone: Kea0.9.2
Component: dhcp4 Version: git
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: Low
Sub-Project: DHCP Feature Depending on Ticket:
Estimated Difficulty: 0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

According to RFC 2132 option Name Server Option (code 5) should be an array of ipv4 addresses not a single one.

Simple patch:

diff --git a/src/lib/dhcp/std_option_defs.h b/src/lib/dhcp/std_option_defs.h
index 10835e2..7306c04 100644
--- a/src/lib/dhcp/std_option_defs.h
+++ b/src/lib/dhcp/std_option_defs.h
@@ -80,7 +80,7 @@ const OptionDefParams OPTION_DEF_PARAMS4[] = {
     { "routers", DHO_ROUTERS, OPT_IPV4_ADDRESS_TYPE, true, NO_RECORD_DEF, "" },
     { "time-servers", DHO_TIME_SERVERS, OPT_IPV4_ADDRESS_TYPE, true, NO_RECORD_DEF, "" },
     { "name-servers", DHO_NAME_SERVERS, OPT_IPV4_ADDRESS_TYPE,
-      false, NO_RECORD_DEF, "" },
+      true, NO_RECORD_DEF, "" },
     { "domain-name-servers", DHO_DOMAIN_NAME_SERVERS,
       OPT_IPV4_ADDRESS_TYPE, true, NO_RECORD_DEF, "" },
     { "log-servers", DHO_LOG_SERVERS, OPT_IPV4_ADDRESS_TYPE, true, NO_RECORD_DEF, "" },

Subtickets

Change History (8)

comment:1 Changed 5 years ago by marcin

It seems like a legitimate ticket and it should be corrected.

comment:2 Changed 5 years ago by hschempf

  • Milestone changed from Kea-proposed to Kea0.9.2

comment:3 Changed 4 years ago by marcin

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

comment:4 Changed 4 years ago by marcin

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

This was indeed the trivial change. Proposed ChangeLog entry:

XXX.	[bug]*		marcin
	Corrected invalid format of the DHCPv4 option 5 (name-servers).
	The corrected format comprises a list of IPv4 addresses,
	rather than a single IPv4 address.
	(Trac #3887, git abcd)

comment:6 Changed 4 years ago by tomek

  • Milestone changed from Kea0.9.2 to Kea0.9.2-final

comment:7 Changed 4 years ago by tmark

  • Owner changed from UnAssigned to tmark

comment:8 Changed 4 years ago by tmark

  • Owner changed from tmark to marcin

I am guessing that we are relying on unit tests of Option4AddrLst itself as proof that the option definition functions. I did not find DHO_NAME_SERVERS used in any of the tests other than TEST_F(LibDhcpTest?, stdOptionDefs4). Perhaps we should add explicit tests for each option as part of some future effort?

Changes are otherwise fine, please merge.

Remove the unrelated 3587 comment above prior to closing the ticket.

comment:9 Changed 4 years ago by marcin

  • Resolution set to fixed
  • Status changed from reviewing to closed

Merged with commit 54d1dbe6138e74c5efacfbaf85b77c87aea9ddf1

Note: See TracTickets for help on using tickets.