Commit 8ac1c13d authored by Thomas Markwalder's avatar Thomas Markwalder Committed by Tomek Mrugalski
Browse files

[5680] kea-dhcp4 supports client FQDN name sanitizing

src/lib/dhcpsrv/d2_client_mgr.h
    D2ClientMgr::adjustDomainName() - added logic to
    sanitize the inbound FQDN name when configured to do so

src/lib/dhcpsrv/tests/d2_client_unittest.cc
    TEST(D2ClientMgr, sanitizeFqdnV4)
    TEST(D2ClientMgr, sanitizeFqdnV6) - new tests

src/bin/dhcp4/tests/fqdn_unittest.cc
    TEST_F(NameDhcpv4SrvTest, sanitizeFqdn) - new test
parent 6402f68e
......@@ -1699,6 +1699,8 @@ TEST_F(NameDhcpv4SrvTest, replaceClientNameModeTest) {
CLIENT_NAME_PRESENT, NAME_NOT_REPLACED);
}
// Verifies that setting hostname-char-set sanitizes Hostname option
// values received from clients.
TEST_F(NameDhcpv4SrvTest, sanitizeHost) {
Dhcp4Client client(Dhcp4Client::SELECTING);
......@@ -1760,5 +1762,74 @@ TEST_F(NameDhcpv4SrvTest, sanitizeHost) {
}
}
// Verifies that setting hostname-char-set sanitizes FQDN option
// values received from clients.
TEST_F(NameDhcpv4SrvTest, sanitizeFqdn) {
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_;
Option4ClientFqdn::DomainNameType name_type_;
std::string sanitized_;
};
std::vector<Scenario> scenarios = {
{
"unqualified FQDN with invalid characters",
"one-&*_-host",
Option4ClientFqdn::PARTIAL,
"one-xxx-host.example.org."
},
{
"qualified FQDN with invalid characters",
"two-&*_-host.other.org",
Option4ClientFqdn::FULL,
"two-xxx-host.other.org."
},
{
"unqualified FQDN name with all valid characters",
"three-ok-host",
Option4ClientFqdn::PARTIAL,
"three-ok-host.example.org."
},
{
"qualified FQDN name with valid characters",
"four-ok-host.other.org",
Option4ClientFqdn::FULL,
"four-ok-host.other.org."
}
};
Pkt4Ptr resp;
Option4ClientFqdnPtr fqdn;
for (auto scenario = scenarios.begin(); scenario != scenarios.end(); ++scenario) {
SCOPED_TRACE((*scenario).description_);
{
// Set the hostname option.
ASSERT_NO_THROW(client.includeHostname((*scenario).original_));
ASSERT_NO_THROW(client.includeFQDN(0, (*scenario).original_, (*scenario).name_type_));
// 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 fqdn is what we expect.
fqdn = boost::dynamic_pointer_cast<Option4ClientFqdn>(resp->getOption(DHO_FQDN));
ASSERT_TRUE(fqdn);
EXPECT_EQ((*scenario).sanitized_, fqdn->getDomainName());
}
}
}
} // end of anonymous namespace
// Copyright (C) 2014-2017 Internet Systems Consortium, Inc. ("ISC")
// Copyright (C) 2014-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
......@@ -16,12 +16,14 @@
#include <dhcpsrv/d2_client_cfg.h>
#include <exceptions/exceptions.h>
#include <boost/algorithm/string.hpp>
#include <boost/shared_ptr.hpp>
#include <boost/noncopyable.hpp>
#include <stdint.h>
#include <string>
#include <vector>
#include <sstream>
namespace isc {
namespace dhcp {
......@@ -238,6 +240,12 @@ public:
/// If replace-client-name is false and the supplied name is a fully
/// qualified name, set the server FQDN to the supplied name.
///
/// If hostname-char-set is not empty, the inbound name will be
/// sanitized. This is done by iterating over the domain name labels,
/// sanitizing each individually, and then concatenating them into a
/// new sanitized name. It is done this way to guard against the case
/// where the hostname-char-set does not protect dots from replacement.
///
/// @param fqdn FQDN option from which to get client (inbound) name
/// @param fqdn_resp FQDN option to update with the adjusted name
/// @tparam T FQDN Option class containing the FQDN data such as
......@@ -462,9 +470,33 @@ D2ClientMgr::adjustDomainName(const T& fqdn, T& fqdn_resp) {
fqdn.getDomainName().empty()) {
fqdn_resp.setDomainName("", T::PARTIAL);
} else {
// Sanitize the name the client sent us, if we're configured to do so.
std::string client_name = fqdn.getDomainName();
if (d2_client_config_->getHostnameSanitizer()) {
// We do not know if the sanitizer's regexp preserves dots, so
// we'll scrub it label by label. Yeah, lucky us.
// Using boost::split is simpler than using dns::Name::split() as
// that returns Names which have trailing dots etc.
std::vector<std::string> labels;
boost::algorithm::split(labels, client_name, boost::is_any_of("."));
std::stringstream ss;
for (auto label = labels.begin(); label != labels.end(); ++label ) {
if (label != labels.begin()) {
ss << ".";
}
ss << d2_client_config_->getHostnameSanitizer()->scrub(*label);
}
client_name = ss.str();
}
// If the supplied name is partial, qualify it by adding the suffix.
if (fqdn.getDomainNameType() == T::PARTIAL) {
fqdn_resp.setDomainName(qualifyName(fqdn.getDomainName(),true), T::FULL);
fqdn_resp.setDomainName(qualifyName(client_name,true), T::FULL);
}
else {
fqdn_resp.setDomainName(client_name, T::FULL);
}
}
}
......
......@@ -11,6 +11,7 @@
#include <testutils/test_to_element.h>
#include <exceptions/exceptions.h>
#include <boost/algorithm/string.hpp>
#include <gtest/gtest.h>
using namespace std;
......@@ -1127,4 +1128,166 @@ TEST(D2ClientMgr, updateDirectionsV6) {
// Response S=1, N=1 isn't possible.
}
/// @brief Tests v4 FQDN name sanitizing
TEST(D2ClientMgr, sanitizeFqdnV4) {
D2ClientMgr mgr;
// Create enabled configuration.
// replace-client-name is false, client passes in empty fqdn
D2ClientConfigPtr cfg;
ASSERT_NO_THROW(cfg.reset(new D2ClientConfig(true,
isc::asiolink::IOAddress("127.0.0.1"), 477,
isc::asiolink::IOAddress("127.0.0.1"), 478,
1024,
dhcp_ddns::NCR_UDP, dhcp_ddns::FMT_JSON,
false, false, false, D2ClientConfig::RCM_NEVER,
"prefix", "suffix.com", "[^A-Za-z0-9-]", "x")));
ASSERT_NO_THROW(mgr.setD2ClientConfig(cfg));
ASSERT_EQ(D2ClientConfig::RCM_NEVER, cfg->getReplaceClientNameMode());
struct Scenario {
std::string description_;
std::string client_name_;
Option4ClientFqdn::DomainNameType name_type_;
std::string expected_name_;
};
std::vector<Scenario> scenarios = {
{
"full FQDN, name unchanged",
"One.123.example.com.",
Option4ClientFqdn::FULL,
"One.123.example.com."
},
{
"partial FQDN, name unchanged, but qualified",
"One.123",
Option4ClientFqdn::PARTIAL,
"One.123.suffix.com."
},
{
"full FQDN, scrubbed",
"O#n^e.123.ex&a*mple.com.",
Option4ClientFqdn::FULL,
"Oxnxe.123.exxaxmple.com."
},
{
"partial FQDN, scrubbed and qualified",
"One.1+2|3",
Option4ClientFqdn::PARTIAL,
"One.1x2x3.suffix.com."
},
{
// Some chars, like parens, get escaped by lib::dns::Name
// when output via Name::getDomainName(). This means they'll
// get replaced by TWO replacment chars, if the backslash "\"
// is an invalid character per hostname-char-set.
"full FQDN, scrubbed with escaped char",
"One.123.exa(mple.com.",
Option4ClientFqdn::FULL,
// expect the ( to be replaced by two x's
"One.123.exaxxmple.com."
}
};
Option4ClientFqdnPtr request;
Option4ClientFqdnPtr response;
for (auto scenario = scenarios.begin(); scenario != scenarios.end(); ++scenario) {
SCOPED_TRACE((*scenario).description_);
{
request.reset(new Option4ClientFqdn(0, Option4ClientFqdn::RCODE_CLIENT(),
(*scenario).client_name_,
(*scenario).name_type_));
response.reset(new Option4ClientFqdn(*request));
mgr.adjustDomainName<Option4ClientFqdn>(*request, *response);
EXPECT_EQ((*scenario).expected_name_, response->getDomainName());
EXPECT_EQ(Option4ClientFqdn::FULL, response->getDomainNameType());
}
}
}
/// @brief Tests v6 FQDN name sanitizing
/// @todo This test currently verifies that Option6ClientFqdn::DomainName
/// downcases strings used to construct it. For some reason, currently
/// uknown, Option4ClientFqdn preserves the case, while Option6ClientFqdn
/// downcases it (see setDomainName() in both classes. See Trac #5700.
TEST(D2ClientMgr, sanitizeFqdnV6) {
D2ClientMgr mgr;
// Create enabled configuration.
// replace-client-name is false, client passes in empty fqdn
D2ClientConfigPtr cfg;
ASSERT_NO_THROW(cfg.reset(new D2ClientConfig(true,
isc::asiolink::IOAddress("127.0.0.1"), 477,
isc::asiolink::IOAddress("127.0.0.1"), 478,
1024,
dhcp_ddns::NCR_UDP, dhcp_ddns::FMT_JSON,
false, false, false, D2ClientConfig::RCM_NEVER,
"prefix", "suffix.com", "[^A-Za-z0-9-]", "x")));
ASSERT_NO_THROW(mgr.setD2ClientConfig(cfg));
ASSERT_EQ(D2ClientConfig::RCM_NEVER, cfg->getReplaceClientNameMode());
struct Scenario {
std::string description_;
std::string client_name_;
Option6ClientFqdn::DomainNameType name_type_;
std::string expected_name_;
};
std::vector<Scenario> scenarios = {
{
"full FQDN, name unchanged",
"One.123.example.com.",
Option6ClientFqdn::FULL,
"one.123.example.com."
},
{
"partial FQDN, name unchanged, but qualified",
"One.123",
Option6ClientFqdn::PARTIAL,
"one.123.suffix.com."
},
{
"full FQDN, scrubbed",
"O#n^e.123.ex&a*mple.com.",
Option6ClientFqdn::FULL,
"oxnxe.123.exxaxmple.com."
},
{
"partial FQDN, scrubbed and qualified",
"One.1+2|3",
Option6ClientFqdn::PARTIAL,
"one.1x2x3.suffix.com."
},
{
// Some chars, like parens, get escaped by lib::dns::Name
// when output via Name::getDomainName(). This means they'll
// get replaced by TWO replacment chars, if the backslash "\"
// is an invalid character per hostname-char-set.
"full FQDN, scrubbed with escaped char",
"One.123.exa(mple.com.",
Option6ClientFqdn::FULL,
// expect the ( to be replaced by two x's
"one.123.exaxxmple.com."
}
};
Option6ClientFqdnPtr request;
Option6ClientFqdnPtr response;
for (auto scenario = scenarios.begin(); scenario != scenarios.end(); ++scenario) {
SCOPED_TRACE((*scenario).description_);
{
request.reset(new Option6ClientFqdn(0, (*scenario).client_name_,
(*scenario).name_type_));
response.reset(new Option6ClientFqdn(*request));
mgr.adjustDomainName<Option6ClientFqdn>(*request, *response);
EXPECT_EQ((*scenario).expected_name_, response->getDomainName());
EXPECT_EQ(Option6ClientFqdn::FULL, response->getDomainNameType());
}
}
}
} // end of anonymous namespace
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