Commit 1825ead4 authored by Thomas Markwalder's avatar Thomas Markwalder Committed by Tomek Mrugalski
Browse files

[5680] kea-dhcp4 now uses hostname sanititzer when configured for it

src/bin/dhcp4/dhcp4_srv.cc
    Dhcpv4Srv::processHostnameOption() - sanitizes client hostname
    if configured to do so

src/bin/dhcp4/tests/fqdn_unittest.cc
    TEST_F(NameDhcpv4SrvTest, sanitizeHostname) - new test to
    verify hostname sanitizing works as expected

src/lib/dhcpsrv/d2_client_cfg.h
    D2ClientConfig::getHostnameSanitizer() added missing getter

src/lib/dhcpsrv/tests/d2_client_unittest.cc
    TEST(D2ClientConfigTest, constructorsAndAccessors) - updated to
    verify hostname sanitizing stuff

src/lib/util/strutil.cc
    fixed regex compilation issue
parent eb264aec
......@@ -1779,11 +1779,9 @@ Dhcpv4Srv::processHostnameOption(Dhcpv4Exchange& ex) {
.arg(ex.getQuery()->getLabel());
return;
}
// Copy construct the hostname provided by the client. It is entirely
// possible that we will use the hostname option provided by the client
// to perform the DNS update and we will send the same option to him to
// indicate that we accepted this hostname.
OptionStringPtr opt_hostname_resp(new OptionString(*opt_hostname));
// Stores the value we eventually use, so we can send it back.
OptionStringPtr opt_hostname_resp;
// The hostname option may be unqualified or fully qualified. The lab_count
// holds the number of labels for the name. The number of 1 means that
......@@ -1792,7 +1790,6 @@ Dhcpv4Srv::processHostnameOption(Dhcpv4Exchange& ex) {
// By checking the number of labels present in the hostname we may infer
// whether client has sent the fully qualified or unqualified hostname.
if ((replace_name_mode == D2ClientConfig::RCM_ALWAYS ||
replace_name_mode == D2ClientConfig::RCM_WHEN_PRESENT)
|| label_count < 2) {
......@@ -1805,17 +1802,28 @@ Dhcpv4Srv::processHostnameOption(Dhcpv4Exchange& ex) {
/// point.
/// For now, we just remain liberal and expect that the DNS will handle
/// conversion if needed and possible.
opt_hostname_resp->setValue(".");
} else if (label_count == 2) {
// If there are two labels, it means that the client has specified
// the unqualified name. We have to concatenate the unqualified name
// with the domain name. The false value passed as a second argument
// indicates that the trailing dot should not be appended to the
// hostname. We don't want to append the trailing dot because
// we don't know whether the hostname is partial or not and some
// clients do not handle the hostnames with the trailing dot.
opt_hostname_resp->setValue(d2_mgr.qualifyName(hostname, false));
opt_hostname_resp.reset(new OptionString(Option::V4, DHO_HOST_NAME, "."));
} else {
// Sanitize the name the client sent us, if we're configured to do so.
isc::util::str::StringSanitizerPtr sanitizer = d2_mgr.getD2ClientConfig()
->getHostnameSanitizer();
if (sanitizer) {
hostname = sanitizer->scrub(hostname);
}
if (label_count == 2) {
// If there are two labels, it means that the client has specified
// the unqualified name. We have to concatenate the unqualified name
// with the domain name. The false value passed as a second argument
// indicates that the trailing dot should not be appended to the
// hostname. We don't want to append the trailing dot because
// we don't know whether the hostname is partial or not and some
// clients do not handle the hostnames with the trailing dot.
opt_hostname_resp.reset(new OptionString(Option::V4, DHO_HOST_NAME,
d2_mgr.qualifyName(hostname, false)));
} else {
opt_hostname_resp.reset(new OptionString(Option::V4, DHO_HOST_NAME, hostname));
}
}
LOG_DEBUG(ddns4_logger, DBG_DHCP4_DETAIL_DATA, DHCP4_RESPONSE_HOSTNAME_DATA)
......
// Copyright (C) 2013-2017 Internet Systems Consortium, Inc. ("ISC")
// Copyright (C) 2013-2018 Internet Systems Consortium, Inc. ("ISC")
//
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
......@@ -29,6 +29,7 @@ namespace {
/// @brief Set of JSON configurations used by the FQDN tests.
const char* CONFIGS[] = {
// 0
"{ \"interfaces-config\": {"
" \"interfaces\": [ \"*\" ]"
"},"
......@@ -53,6 +54,7 @@ const char* CONFIGS[] = {
"\"qualifying-suffix\": \"\""
"}"
"}",
// 1
"{ \"interfaces-config\": {"
" \"interfaces\": [ \"*\" ]"
"},"
......@@ -77,6 +79,7 @@ const char* CONFIGS[] = {
"\"qualifying-suffix\": \"fake-suffix.isc.org.\""
"}"
"}",
// 2
// Simple config with DDNS updates disabled. Note pool is one address
// large to ensure we get a specific address back.
"{ \"interfaces-config\": {"
......@@ -93,6 +96,7 @@ const char* CONFIGS[] = {
"\"qualifying-suffix\": \"fake-suffix.isc.org.\""
"}"
"}",
// 3
// Simple config with DDNS updates enabled. Note pool is one address
// large to ensure we get a specific address back.
"{ \"interfaces-config\": {"
......@@ -109,6 +113,7 @@ const char* CONFIGS[] = {
"\"qualifying-suffix\": \"fake-suffix.isc.org.\""
"}"
"}",
// 4
// Configuration which disables DNS updates but contains a reservation
// for a hostname. Reserved hostname should be assigned to a client if
// the client includes it in the Parameter Request List option.
......@@ -136,6 +141,7 @@ const char* CONFIGS[] = {
"\"qualifying-suffix\": \"\""
"}"
"}",
// 5
// Configuration which disables DNS updates but contains a reservation
// for a hostname and the qualifying-suffix which should be appended to
// the reserved hostname in the Hostname option returned to a client.
......@@ -162,7 +168,35 @@ const char* CONFIGS[] = {
"\"enable-updates\": false,"
"\"qualifying-suffix\": \"example.isc.org\""
"}"
"}"
"}",
// 6
// Configuration which enables DNS updates and hostname sanitization
"{ \"interfaces-config\": {"
" \"interfaces\": [ \"*\" ]"
"},"
"\"valid-lifetime\": 3000,"
"\"subnet4\": [ { "
" \"subnet\": \"10.0.0.0/24\", "
" \"id\": 1,"
" \"pools\": [ { \"pool\": \"10.0.0.10-10.0.0.100\" } ],"
" \"option-data\": [ {"
" \"name\": \"routers\","
" \"data\": \"10.0.0.200,10.0.0.201\""
" } ],"
" \"reservations\": ["
" {"
" \"hw-address\": \"aa:bb:cc:dd:ee:ff\","
" \"hostname\": \"unique-xxx-host.example.org\""
" }"
" ]"
" }],"
"\"dhcp-ddns\": {"
"\"enable-updates\": true,"
"\"hostname-char-set\" : \"[^A-Za-z0-9.-]\","
"\"hostname-char-replacement\" : \"x\","
"\"qualifying-suffix\": \"example.org\""
"}"
"}",
};
class NameDhcpv4SrvTest : public Dhcpv4SrvTest {
......@@ -425,7 +459,8 @@ public:
// Test that the server's processes the hostname (or lack thereof)
// in a client request correctly, according to the replace-client-name
// mode configuration parameter.
// mode configuration parameter. We include hostname sanitizer to ensure
// it does not interfere with name replacement.
//
// @param mode - value to use for replace-client-name
// @param client_name_flag - specifies whether or not the client request
......@@ -449,6 +484,8 @@ public:
"\"dhcp-ddns\": {"
"\"enable-updates\": true,"
"\"qualifying-suffix\": \"fake-suffix.isc.org.\","
"\"hostname-char-set\": \"[^A-Za-z0-9.-]\","
"\"hostname-char-replacement\": \"x\","
"\"replace-client-name\": \"%s\""
"}}";
......@@ -1662,4 +1699,66 @@ TEST_F(NameDhcpv4SrvTest, replaceClientNameModeTest) {
CLIENT_NAME_PRESENT, NAME_NOT_REPLACED);
}
TEST_F(NameDhcpv4SrvTest, sanitizeHost) {
Dhcp4Client client(Dhcp4Client::SELECTING);
// Configure DHCP server.
configure(CONFIGS[6], *client.getServer());
// Make sure that DDNS is enabled.
ASSERT_TRUE(CfgMgr::instance().ddnsEnabled());
ASSERT_NO_THROW(client.getServer()->startD2());
struct Scenario {
std::string description_;
std::string original_;
std::string sanitized_;
};
std::vector<Scenario> scenarios = {
{
"unqualified host name with invalid characters",
"one-&$_-host",
"one-xxx-host.example.org"
},
{
"qualified host name with invalid characters",
"two-&$_-host.other.org",
"two-xxx-host.other.org"
},
{
"unqualified host name with all valid characters",
"three-ok-host",
"three-ok-host.example.org"
},
{
"qualified host name with valid characters",
"four-ok-host.other.org",
"four-ok-host.other.org"
}
};
Pkt4Ptr resp;
OptionStringPtr hostname;
for (auto scenario = scenarios.begin(); scenario != scenarios.end(); ++scenario) {
SCOPED_TRACE((*scenario).description_);
{
// Set the hostname option.
ASSERT_NO_THROW(client.includeHostname((*scenario).original_));
// Send the DHCPDISCOVER and make sure that the server responded.
ASSERT_NO_THROW(client.doDiscover());
resp = client.getContext().response_;
ASSERT_TRUE(resp);
ASSERT_EQ(DHCPOFFER, static_cast<int>(resp->getType()));
// Make sure the response hostname is what we expect.
hostname = boost::dynamic_pointer_cast<OptionString>(resp->getOption(DHO_HOST_NAME));
ASSERT_TRUE(hostname);
EXPECT_EQ((*scenario).sanitized_, hostname->getValue());
}
}
}
} // end of anonymous namespace
......@@ -209,6 +209,12 @@ public:
return(hostname_char_replacement_);
}
/// @brief Return pointer to compiled regular expression string sanitizer
/// Will be empty if hostname-char-set is empty.
util::str::StringSanitizerPtr getHostnameSanitizer() const {
return(hostname_sanitizer_);
}
/// @brief Compares two D2ClientConfigs for equality
bool operator == (const D2ClientConfig& other) const;
......
......@@ -128,6 +128,10 @@ TEST(D2ClientConfigTest, constructorsAndAccessors) {
EXPECT_EQ(d2_client_config->getGeneratedPrefix(), generated_prefix);
EXPECT_EQ(d2_client_config->getQualifyingSuffix(), qualifying_suffix);
EXPECT_EQ(d2_client_config->getHostnameCharSet(), hostname_char_set);
EXPECT_EQ(d2_client_config->getHostnameCharReplacement(), hostname_char_replacement);
EXPECT_TRUE(d2_client_config->getHostnameSanitizer());
ASSERT_TRUE(d2_client_config->getContext());
EXPECT_EQ(d2_client_config->getContext()->str(), user_context);
......
......@@ -334,7 +334,7 @@ public:
try {
std::regex_replace(std::ostream_iterator<char>(result),
original.begin(), original.end(),
scrub_expr, char_replacement_);
scrub_exp_, char_replacement_);
} catch (const std::exception& ex) {
isc_throw(isc::BadValue, "replacing '" << char_set_ << "' with '"
<< char_replacement_ << "' in '" << original << "' failed: ,"
......
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