Commit 8d1f1d7f authored by Thomas Markwalder's avatar Thomas Markwalder Committed by Tomek Mrugalski

[5440] libdhcp++ now quietly drops empty host name options from inbound packets

src/lib/dhcp/libdhcp++.cc
    LibDHCP::unpackOptions4() - added logic to drop Host Name option
    if when empty

src/lib/dhcp/tests/libdhcp++_unittest.cc
    TEST_F(LibDhcpTest, emptyHostName)  - new unit test

src/bin/dhcp4/dhcp4_srv.cc
    Dhcpv4Srv::processHostnameOption() - removed prior 5440 logic
    to ignore blank hostname options

src/bin/dhcp4/dhcp4_messages.mes
    Removed prior 5440 message
parent 1f141f14
......@@ -98,11 +98,6 @@ client and transaction identification information. The second argument
specifies the hostname carried in the Hostname option sent by the
client.
% DHCP4_CLIENT_HOSTNAME_EMPTY %1: client sent bogus empty Hostname option
This informational message is issued when the server receives an
empty Hostname option. Such option is bogus (size is required to be
at least one) and is ignored.
% DHCP4_CLIENT_HOSTNAME_PROCESS %1: processing client's Hostname option
This debug message is issued when the server starts processing the Hostname
option sent in the client's query. The argument includes the client and
......
......@@ -1679,13 +1679,6 @@ Dhcpv4Srv::processHostnameOption(Dhcpv4Exchange& ex) {
OptionStringPtr opt_hostname = boost::dynamic_pointer_cast<OptionString>
(ex.getQuery()->getOption(DHO_HOST_NAME));
// Ignore empty/bogus Hostname option.
if (opt_hostname && opt_hostname->getValue().empty()) {
opt_hostname.reset();
LOG_INFO(ddns4_logger, DHCP4_CLIENT_HOSTNAME_EMPTY)
.arg(ex.getQuery()->getLabel());
}
if (opt_hostname) {
LOG_DEBUG(ddns4_logger, DBG_DHCP4_DETAIL_DATA, DHCP4_CLIENT_HOSTNAME_DATA)
.arg(ex.getQuery()->getLabel())
......
......@@ -836,7 +836,10 @@ TEST_F(NameDhcpv4SrvTest, serverUpdateWrongHostname) {
EXPECT_FALSE(hostname);
}
// Test that the server skips processing of an empty Hostname option.
// Test that the server does not see an empty Hostname option.
// Suppressing the empty Hostname is done in libdhcp++ during
// unpackcing, so technically we don't need this test but,
// hey it's already written.
TEST_F(NameDhcpv4SrvTest, serverUpdateEmptyHostname) {
Pkt4Ptr query;
ASSERT_NO_THROW(query = generatePktWithEmptyHostname(DHCPREQUEST));
......@@ -845,7 +848,6 @@ TEST_F(NameDhcpv4SrvTest, serverUpdateEmptyHostname) {
EXPECT_FALSE(hostname);
}
// Test that server generates the fully qualified domain name for the client
// if client supplies the partial name.
TEST_F(NameDhcpv4SrvTest, serverUpdateForwardPartialNameFqdn) {
......
......@@ -525,6 +525,15 @@ size_t LibDHCP::unpackOptions4(const OptionBuffer& buf,
return (last_offset);
}
// While an empty Host Name option is non-RFC compliant, some clients
// do send it. In the spirit of being liberal, we'll just drop it,
// rather than the dropping the whole packet. We do not have a
// way to log this from here but meh... a PCAP will show it arriving,
// and we know we drop it.
if (opt_len == 0 && opt_type == DHO_HOST_NAME) {
continue;
}
// Get all definitions with the particular option code. Note
// that option code is non-unique within this container
// however at this point we expect to get one option
......
......@@ -1016,6 +1016,37 @@ TEST_F(LibDhcpTest, unpackSubOptions4) {
EXPECT_EQ(0x0, option_bar->getValue());
}
// Verifies that an Host Name (option 12), will be dropped when empty,
// while subsequent options will still be unpacked.
TEST_F(LibDhcpTest, emptyHostName) {
uint8_t opts[] = {
12, 0, // Empty Hostname
60, 3, 10, 11, 12 // Class Id
};
vector<uint8_t> packed(opts, opts + sizeof(opts));
isc::dhcp::OptionCollection options; // list of options
list<uint16_t> deferred;
ASSERT_NO_THROW(
LibDHCP::unpackOptions4(packed, "dhcp4", options, deferred);
);
// Host Name should not exist, we quietly drop it when empty.
isc::dhcp::OptionCollection::const_iterator x = options.find(12);
ASSERT_TRUE(x == options.end());
// Verify Option 60 exists correctly
x = options.find(60);
ASSERT_FALSE(x == options.end());
EXPECT_EQ(60, x->second->getType());
ASSERT_EQ(3, x->second->getData().size());
EXPECT_EQ(5, x->second->len());
EXPECT_EQ(0, memcmp(&x->second->getData()[0], opts + 4, 3));
};
TEST_F(LibDhcpTest, stdOptionDefs4) {
// Create a buffer that holds dummy option data.
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment