Commit 60ee8464 authored by Marcin Siodelski's avatar Marcin Siodelski
Browse files

[3279] The server identifier should be encapsulated by the OptionCustom.

parent 6d552280
......@@ -1551,22 +1551,26 @@ Dhcpv4Srv::acceptServerId(const Pkt4Ptr& pkt) const {
}
// Server identifier is present. Let's convert it to 4-byte address
// and try to match with server identifiers used by the server.
Option4AddrLstPtr option_addrs =
boost::dynamic_pointer_cast<Option4AddrLst>(option);
OptionCustomPtr option_custom =
boost::dynamic_pointer_cast<OptionCustom>(option);
// Unable to convert the option to the option type which encapsulates it.
// We treat this as non-matching server id.
if (!option_addrs) {
if (!option_custom) {
return (false);
}
Option4AddrLst::AddressContainer addrs = option_addrs->getAddresses();
// The server identifier option should carry exactly one IPv4 address.
// This option is encapsulated by the class which accepts a list of
// IPv4 addresses. So, there is a potential risk that the client has sent
// a server identifier option with multiple addresses and it has been
// parsed by the server. Here we are filtering out such malformed
// messages here.
if (addrs.size() != 1) {
// If the option definition for the server identifier doesn't change,
// the OptionCustom object should have exactly one IPv4 address and
// this check is somewhat redundant. On the other hand, if someone
// breaks option it may be better to check that here.
if (option_custom->getDataFieldsNum() != 1) {
return (false);
}
// The server identifier MUST be an IPv4 address. If given address is
// v6, it is wrong.
IOAddress server_id = option_custom->readAddress();
if (!server_id.isV4()) {
return (false);
}
......@@ -1578,7 +1582,7 @@ Dhcpv4Srv::acceptServerId(const Pkt4Ptr& pkt) const {
// performance hit should be acceptable. If it turns out to
// be significant, we will have to cache server identifiers
// when sockets are opened.
return (IfaceMgr::instance().hasOpenSocket(addrs[0]));
return (IfaceMgr::instance().hasOpenSocket(server_id));
}
void
......
......@@ -1046,12 +1046,16 @@ TEST_F(Dhcpv4SrvFakeIfaceTest, acceptServerId) {
// accepted.
EXPECT_TRUE(srv.acceptServerId(pkt));
// Create definition of the server identifier option.
OptionDefinition def("server-identifier", DHO_DHCP_SERVER_IDENTIFIER,
"ipv4-address", false);
// Add a server identifier option which doesn't match server ids being
// used by the server. The accepted server ids are the IPv4 addresses
// configured on the interfaces. The 10.1.2.3 is not configured on
// any interface.
OptionPtr other_serverid(new Option4AddrLst(DHO_DHCP_SERVER_IDENTIFIER,
IOAddress("10.1.2.3")));
// any interfaces.
OptionCustomPtr other_serverid(new OptionCustom(def, Option::V6));
other_serverid->writeAddress(IOAddress("10.1.2.3"));
pkt->addOption(other_serverid);
EXPECT_FALSE(srv.acceptServerId(pkt));
......@@ -1060,8 +1064,8 @@ TEST_F(Dhcpv4SrvFakeIfaceTest, acceptServerId) {
// Add a server id being an IPv4 address configured on eth0 interface.
// A DHCPv4 message holding this server identifier should be accepted.
OptionPtr eth0_serverid(new Option4AddrLst(DHO_DHCP_SERVER_IDENTIFIER,
IOAddress("192.0.3.1")));
OptionCustomPtr eth0_serverid(new OptionCustom(def, Option::V6));
eth0_serverid->writeAddress(IOAddress("192.0.3.1"));
ASSERT_NO_THROW(pkt->addOption(eth0_serverid));
EXPECT_TRUE(srv.acceptServerId(pkt));
......@@ -1070,8 +1074,8 @@ TEST_F(Dhcpv4SrvFakeIfaceTest, acceptServerId) {
// Add a server id being an IPv4 address configured on eth1 interface.
// A DHCPv4 message holding this server identifier should be accepted.
OptionPtr eth1_serverid(new Option4AddrLst(DHO_DHCP_SERVER_IDENTIFIER,
IOAddress("10.0.0.1")));
OptionCustomPtr eth1_serverid(new OptionCustom(def, Option::V6));
eth1_serverid->writeAddress(IOAddress("10.0.0.1"));
ASSERT_NO_THROW(pkt->addOption(eth1_serverid));
EXPECT_TRUE(srv.acceptServerId(pkt));
......
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