Commit 6fc6f278 authored by Marcin Siodelski's avatar Marcin Siodelski
Browse files

[master] Merge branch 'trac3749'

parents ed3ee8e8 3b9c8f99
......@@ -162,13 +162,6 @@ This debug message is issued when the client being in the INIT-REBOOT state
requested an address which is not assigned to him. The server will respond
to this client with DHCPNAK.
% DHCP4_INVALID_RELAY_INFO malformed packet received from client-id %1, hwaddr %2: %3
This message is logged when the client sends invalid combination of
values in the giaddr and hops fields. If the packet is relayed it should
contain non-zero values in both fields. If the packet is sent from the
directly connected client, both values should be set to zero. All other
combinations are invalid and trigger packet drop.
% DHCP4_LEASE_ADVERT lease %1 advertised (client client-id %2, hwaddr %3)
This debug message indicates that the server successfully advertised
a lease. It is up to the client to choose one server out of other advertised
......
......@@ -1770,23 +1770,9 @@ Dhcpv4Srv::accept(const Pkt4Ptr& query) const {
bool
Dhcpv4Srv::acceptDirectRequest(const Pkt4Ptr& pkt) const {
try {
// This is the first call to the isRelayed function for this packet,
// so we have to catch exceptions which will be emitted if the
// packet contains invalid combination of hops and giaddr. For all
// other invocations of isRelayed function we will not catch
// exceptions because we eliminate malformed packets here.
if (pkt->isRelayed()) {
return (true);
}
} catch (const Exception& ex) {
OptionPtr client_id = pkt->getOption(DHO_DHCP_CLIENT_IDENTIFIER);
HWAddrPtr hwaddr = pkt->getHWAddr();
LOG_INFO(dhcp4_logger, DHCP4_INVALID_RELAY_INFO)
.arg(client_id ? client_id->toText():"(no client-id)")
.arg(hwaddr ? hwaddr->toText():"(no hwaddr info)")
.arg(ex.what());
return (false);
// Accept all relayed messages.
if (pkt->isRelayed()) {
return (true);
}
// The source address must not be zero for the DHCPINFORM message from
// the directly connected client because the server will not know where
......
......@@ -342,45 +342,6 @@ TEST_F(Dhcpv4SrvTest, adjustIfaceDataBroadcast) {
}
// This test verifies that exception is thrown of the invalid combination
// of giaddr and hops is specified in a client's message.
TEST_F(Dhcpv4SrvTest, adjustIfaceDataInvalid) {
IfaceMgrTestConfig test_config(true);
IfaceMgr::instance().openSockets4();
boost::shared_ptr<Pkt4> req(new Pkt4(DHCPDISCOVER, 1234));
// The hops and giaddr values are used to determine if the client's
// message has been relayed or sent directly. The allowed combinations
// are (giaddr = 0 and hops = 0) or (giaddr != 0 and hops != 0). Any
// other combination is invalid and the adjustIfaceData should throw
// an exception. We will test that exception is indeed thrown.
req->setGiaddr(IOAddress("0.0.0.0"));
req->setHops(1);
// Clear client address as it hasn't got any address configured yet.
req->setCiaddr(IOAddress("0.0.0.0"));
// The query is sent to the broadcast address in the Select state.
req->setLocalAddr(IOAddress("255.255.255.255"));
// The query has been received on the DHCPv4 server port 67.
req->setLocalPort(DHCP4_SERVER_PORT);
// Set the interface. The response should be sent via the same interface.
req->setIface("eth0");
req->setIndex(1);
// Create the exchange using the req.
Dhcpv4Exchange ex = createExchange(req);
Pkt4Ptr resp = ex.getResponse();
// Assign some new address for this client.
resp->setYiaddr(IOAddress("192.0.2.13"));
// Clear the remote address.
resp->setRemoteAddr(IOAddress("0.0.0.0"));
EXPECT_THROW(NakedDhcpv4Srv::adjustIfaceData(ex), isc::BadValue);
}
// This test verifies that the server identifier option is appended to
// a specified DHCPv4 message and the server identifier is correct.
TEST_F(Dhcpv4SrvTest, appendServerID) {
......@@ -3479,16 +3440,15 @@ TEST_F(Dhcpv4SrvTest, acceptDirectRequest) {
Pkt4Ptr pkt(new Pkt4(DHCPDISCOVER, 1234));
// Set Giaddr and local server's unicast address, but don't set hops.
// Hops value must be greater than 0, when giaddr is set. Otherwise,
// message is considered malformed and the accept() function should
// return false.
// Hops value should not matter. The server will treat the message
// with the hops value of 0 and non-zero giaddr as relayed.
pkt->setGiaddr(IOAddress("192.0.10.1"));
pkt->setRemoteAddr(IOAddress("0.0.0.0"));
pkt->setLocalAddr(IOAddress("192.0.2.3"));
pkt->setIface("eth1");
EXPECT_FALSE(srv.accept(pkt));
EXPECT_TRUE(srv.accept(pkt));
// Let's set hops and check that the message is now accepted as
// Let's set hops and check that the message is still accepted as
// a relayed message.
pkt->setHops(1);
EXPECT_TRUE(srv.accept(pkt));
......
......@@ -436,21 +436,7 @@ Pkt4::addOption(const OptionPtr& opt) {
bool
Pkt4::isRelayed() const {
static const IOAddress zero_addr("0.0.0.0");
// For non-relayed message both Giaddr and Hops are zero.
if (getGiaddr() == zero_addr && getHops() == 0) {
return (false);
// For relayed message, both Giaddr and Hops are non-zero.
} else if (getGiaddr() != zero_addr && getHops() > 0) {
return (true);
}
// In any other case, the packet is considered malformed.
isc_throw(isc::BadValue, "invalid combination of giaddr = "
<< getGiaddr().toText() << " and hops = "
<< static_cast<int>(getHops()) << ". Valid values"
" are: (giaddr = 0 and hops = 0) or (giaddr != 0 and"
"hops != 0)");
return (!giaddr_.isV4Zero() && !giaddr_.isV4Bcast());
}
} // end of namespace isc::dhcp
......
......@@ -332,17 +332,11 @@ public:
/// This function returns a boolean value which indicates whether a DHCPv4
/// message has been relayed (if true is returned) or not (if false).
///
/// This function uses a combination of Giaddr and Hops. It is expected that
/// if Giaddr is not 0, the Hops is greater than 0. In this case the message
/// is considered relayed. If Giaddr is 0, the Hops value must also be 0. In
/// this case the message is considered non-relayed. For any other
/// combination of Giaddr and Hops, an exception is thrown to indicate that
/// the message is malformed.
/// The message is considered relayed if the giaddr field is non-zero and
/// non-broadcast.
///
/// @return Boolean value which indicates whether the message is relayed
/// (true) or non-relayed (false).
/// @throw isc::BadValue if invalid combination of Giaddr and Hops values is
/// found.
bool isRelayed() const;
/// @brief That's the data of input buffer used in RX packet.
......
// Copyright (C) 2011-2014 Internet Systems Consortium, Inc. ("ISC")
// Copyright (C) 2011-2015 Internet Systems Consortium, Inc. ("ISC")
//
// Permission to use, copy, modify, and/or distribute this software for any
// purpose with or without fee is hereby granted, provided that the above
......@@ -800,25 +800,26 @@ TEST_F(Pkt4Test, hwaddrSrcRemote) {
}
// This test verifies that the check for a message being relayed is correct.
// It also checks that the exception is thrown if the combination of hops and
// giaddr is invalid.
TEST_F(Pkt4Test, isRelayed) {
Pkt4 pkt(DHCPDISCOVER, 1234);
// By default, the hops and giaddr should be 0.
ASSERT_EQ("0.0.0.0", pkt.getGiaddr().toText());
ASSERT_TRUE(pkt.getGiaddr().isV4Zero());
ASSERT_EQ(0, pkt.getHops());
// For hops = 0 and giaddr = 0, the message is non-relayed.
// For zero giaddr the packet is non-relayed.
EXPECT_FALSE(pkt.isRelayed());
// Set giaddr but leave hops = 0. This should result in exception.
// Set giaddr but leave hops = 0.
pkt.setGiaddr(IOAddress("10.0.0.1"));
EXPECT_THROW(pkt.isRelayed(), isc::BadValue);
// Set hops. Now both hops and giaddr is set. The message is relayed.
EXPECT_TRUE(pkt.isRelayed());
// After setting hops the message should still be relayed.
pkt.setHops(10);
EXPECT_TRUE(pkt.isRelayed());
// Set giaddr to 0. For hops being set to non-zero value the function
// should throw an exception.
pkt.setGiaddr(IOAddress("0.0.0.0"));
EXPECT_THROW(pkt.isRelayed(), isc::BadValue);
// Set giaddr to 0. The message is now not-relayed.
pkt.setGiaddr(IOAddress(IOAddress::IPV4_ZERO_ADDRESS()));
EXPECT_FALSE(pkt.isRelayed());
// Setting the giaddr to 255.255.255.255 should not cause it to
// be relayed message.
pkt.setGiaddr(IOAddress(IOAddress::IPV4_BCAST_ADDRESS()));
EXPECT_FALSE(pkt.isRelayed());
}
// Tests whether a packet can be assigned to a class and later
......
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