BIND 10 trac998, updated. f5edd310465966137f0cd4e2109d90f7e5d5965f [trac998] IPV4 checks working
BIND 10 source code commits
bind10-changes at lists.isc.org
Mon Jun 13 12:16:49 UTC 2011
The branch, trac998 has been updated
via f5edd310465966137f0cd4e2109d90f7e5d5965f (commit)
from 73ac6b09eeeebcdb03965076d4aa8a8a7a361ebe (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit f5edd310465966137f0cd4e2109d90f7e5d5965f
Author: Stephen Morris <stephen at isc.org>
Date: Mon Jun 13 13:16:08 2011 +0100
[trac998] IPV4 checks working
-----------------------------------------------------------------------
Summary of changes:
src/lib/acl/ip_check.h | 274 ++++++++++++++++++--------------
src/lib/acl/tests/ip_check_unittest.cc | 150 +++++++++++-------
2 files changed, 247 insertions(+), 177 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/acl/ip_check.h b/src/lib/acl/ip_check.h
index 911ee17..a598b83 100644
--- a/src/lib/acl/ip_check.h
+++ b/src/lib/acl/ip_check.h
@@ -16,6 +16,7 @@
#define __IP_CHECK_H
#include <boost/lexical_cast.hpp>
+#include <utility>
#include <vector>
#include <stdint.h>
@@ -29,16 +30,18 @@
namespace isc {
namespace acl {
+// Free functions
+
/// \brief Convert Mask Size to Mask
///
/// Given a mask size and a data type, return a value of that data type with the
-/// most significant maasksize bits set. For example, if the data type is an
-/// unsigned either-bit byte and the masksize is 3, the function would return
-/// an eight-bit byte with the binary value 11100000.
+/// most significant masksize bits set. For example, if the data type is an
+/// unsigned eight-bit byte and the masksize is 3, the function would return
+/// an unsigned eight-bit byte with the binary value 11100000.
///
-/// This is a templated function. The template parameter must be a signed type.
+/// The function is templated on the data type of the mask.
///
-/// \param masksize Size of the mask. This must be between 1 and sizeof(T).
+/// \param masksize Size of the mask. This must be between 1 and 8*sizeof(T).
/// An out of range exception is thrown if this is not the case.
///
/// \return Value with the most significant "masksize" bits set.
@@ -48,67 +51,99 @@ T createNetmask(size_t masksize) {
if ((masksize > 0) && (masksize <= 8 * sizeof(T))) {
- // To explain the logic, consider a single 4-bit word. The masksize in
- // this case can be between 1 and 4. The following table has the
- // following columns:
- //
- // Mask size (m): number of contiguous bits set
- //
- // Low value (lo): unsigned value of 4-bit word with the least-
- // significant m contiguous bits set.
+ // In the following discussion:
//
- // High value (hi): unsigned value of 4-bit word with the most-
- // significant m contiguous bits set.
+ // w is the width of the data type T in bits
+ // m is the value of masksize, the number of most signifcant bits we
+ // want to set.
//
- // m lo hi
- // 1 1 8
- // 2 3 12
- // 3 7 14
- // 4 15 15
+ // We note that the value of 2**m - 1 gives a result with the least
+ // significant m bits set and the most signficant (w-m) bits clear.
//
- // Clearly the low value is equal to (2**m - 1) (using ** to indicate
- // exponentiation). It takes a little thought to see that the high
- // value is equal to 2**4 - 2**(4-m). Unfortunately, this formula will
- // overflow as the intermediate value 2 << sizeof(T) will overflow in an
- // element of type T.
+ // Hence the value 2**(w-m) - 1 gives a result with the least signficant
+ // w-m bits set and the most significant m bits clear.
//
- // However, another way of looking it is that to set the most signifcant
- // m bits, we set all bits and clear the least-significant 4-m bits. If
- // T is a signed value, we can set all bits by setting it to -1. If m
- // is 4, we omit clearing any bits, otherwise we clear the bits
- // represented by the bit pattern 2**(3-m) - 1. (The value 2**(3-m)
- // will be greater than 0 and within the range of an unsigned data
- // type of the same size as T. So it should not overflow.)
+ // The 1's complement of this value gives is the result we want.
//
- // Therefore we proceed on the assumption that T is signed
- T mask = -1;
- if (masksize < 8 * sizeof(T)) {
- mask &= ~((2 << (8 * sizeof(T) - 1 - masksize)) - 1);
+ // Final note: masksize is non-zero, so we are assured that no term in
+ // the expression below will overflow.
+
+ return (~((1 << (8 * sizeof(T) - masksize)) - 1));
+ }
+
+ isc_throw(isc::OutOfRange, "mask size must be between 1 and " <<
+ 8 * sizeof(T));
+}
+
+/// \brief Split IP Address
+///
+/// Splits an IP address (given in the form of "xxxxxx/n" or "xxxxx" into a
+/// string representing the IP address and a number giving the size of the
+/// network mask in bits.
+///
+/// An exception will be thrown if the string format is invalid. N.B. This
+/// does NOT check that the address component is a valid IP address - only that
+/// some string is present.
+///
+/// \param addrmask Address and/or address/mask. The string should be passed
+/// without leading or trailing spaces.
+/// \param defmask Default value of the mask size, used if no mask is given.
+/// \param maxmask Maximum valid value of the mask size.
+///
+/// \return Pair of (string, uint32) holding the address string and the mask
+/// size value.
+
+std::pair<std::string, uint32_t>
+splitIpAddress(const std::string& addrmask, uint32_t defmask, uint32_t maxmask){
+
+ uint32_t masksize = defmask;
+
+ // See if a mask size was given
+ std::vector<std::string> components = isc::util::str::tokens(addrmask, "/");
+ if (components.size() == 2) {
+
+ // There appears to be, try converting it to a number.
+ try {
+ masksize = boost::lexical_cast<size_t>(components[1]);
+ } catch (boost::bad_lexical_cast&) {
+ isc_throw(isc::InvalidParameter,
+ "mask size specified in address/masksize " << addrmask <<
+ " is not valid");
}
- return (mask);
+ // Is it in the valid range?
+ if ((masksize == 0) || (masksize > maxmask)) {
+ isc_throw(isc::OutOfRange,
+ "mask size specified in address/masksize " << addrmask <<
+ " must be in range 1 <= masksize <= " << maxmask);
+ }
+
+ } else if (components.size() > 2) {
+ isc_throw(isc::InvalidParameter, "address/masksize of " <<
+ addrmask << " is not valid");
}
- // Invalid mask size
- isc_throw(isc::OutOfRange, "mask size of " << masksize << " is invalid " <<
- "for the data type which is " << sizeof(T) <<
- " bytes long");
+ return (std::make_pair(components[0], masksize));
}
-/// \brief IP V4 Check
+
+/// \brief IPV4 Check
///
/// This class performs a match between an IPv4 address specified in an ACL
/// (IP address, network mask and a flag indicating whether the check should
-/// be for a match or for no-match) and a given IPv4 address.
+/// be for a match or for a non-match) and a given IP address.
///
/// \param Context Structure holding address to be matched.
-template <typename Context> class Ipv4Check : public Check<Context> {
+template <typename Context>
+class Ipv4Check : public Check<Context> {
public:
-
- /// \brief Constructor
+ /// \brief IPV4 Constructor
+ ///
+ /// Constructs an IPv4 Check object from a network address given as a
+ /// 32-bit value in network byte order.
///
- /// \param address IP address to check for (as an address in host-byte
+ /// \param address IP address to check for (as an address in network-byte
/// order).
/// \param mask The network mask specified as an integer between 1 and
/// 32 This determines the number of bits in the mask to check.
@@ -117,84 +152,50 @@ public:
/// \param inverse If false (the default), matches() returns true if the
/// condition matches. If true, matches() returns true if the
/// condition does not match.
- Ipv4Check(uint32_t address, size_t masksize = 32, bool inverse = false) :
- address_(address), masksize_(masksize), netmask_(0), inverse_(inverse)
+ Ipv4Check(uint32_t address = 1, size_t masksize = 32, bool inverse = false):
+ address_(address), masksize_(masksize), inverse_(inverse), netmask_(0)
{
- init();
+ setNetmask();
}
- /// \brief Constructor
+
+
+ /// \brief String Constructor
+ ///
+ /// Constructs an IPv4 Check object from a network address and size of mask
+ /// given as a string of the form "a.b.c.d/n", where the "/n" part is
+ /// optional.
///
/// \param address IP address and netmask in the form "a.b.c.d/n" (where
- /// the "/n" part is optional.
+ /// the "/n" part is optional).
/// \param inverse If false (the default), matches() returns true if the
/// condition matches. If true, matches() returns true if the
/// condition does not match.
Ipv4Check(const std::string& address, bool inverse = false) :
- address_(0), masksize_(32), netmask_(0), inverse_(inverse)
+ address_(1), masksize_(32), inverse_(inverse), netmask_(0)
{
- // See if there is a netmask.
- std::vector<std::string> components =
- isc::util::str::tokens(address, "/");
- if (components.size() == 2) {
-
- // Yes there is, convert to a mask
- try {
- masksize_ = boost::lexical_cast<size_t>(components[1]);
- } catch (boost::bad_lexical_cast&) {
- isc_throw(isc::InvalidParameter,
- "mask specified in address/mask " << address <<
- " is not valid");
- }
- } else if (components.size() > 2) {
- isc_throw(isc::InvalidParameter, "address/mask of " <<
- address << " is not valid");
- }
+ // Split the address into address part and mask.
+ std::pair<std::string, uint32_t> result =
+ splitIpAddress(address, 8 * sizeof(uint32_t), 8 * sizeof(uint32_t));
// Try to convert the address.
- int result = inet_pton(AF_INET, components[0].c_str(), &address_);
- if (result == 0) {
- isc_throw(isc::InvalidParameter, "address/mask of " <<
- address << " is not valid");
+ int status = inet_pton(AF_INET, result.first.c_str(), &address_);
+ if (status == 0) {
+ isc_throw(isc::InvalidParameter, "address/masksize of " <<
+ address << " is not valid IPV4 address");
}
- address_ = ntohl(address_);
// All done, so finish initialization.
- init();
+ masksize_ = result.second;
+ setNetmask();
}
+
+
/// \brief Destructor
virtual ~Ipv4Check() {}
- /// \brief Comparison
- ///
- /// This is the actual comparison function that checks the IP address passed
- /// to this class with the matching information in the class itself.
- ///
- /// \param address Address to match against the check condition in the
- /// class.
- ///
- /// \return true if the address matches, false if it does not.
- virtual bool compare(uint32_t address) {
-
- // To check that the address given matches the stored network address
- // and netmask, we check the simple condition that:
- //
- // address_given & netmask_ == maskaddr_.
- //
- // However, we must return the negation of the result if inverse_ is
- // set. This leads to the truth table:
- //
- // Result inverse_ Return
- // false false false
- // false true true
- // true false true
- // true true false
- //
- // ... which is an XOR function.
- return (((address & netmask_) == maskaddr_) ^ inverse_);
- }
/// \brief The check itself
///
@@ -203,16 +204,20 @@ public:
/// link will fail if used for a type for which no match is provided.
///
/// \param context Information to be matched
- virtual bool matches(const Context& context) const {return false; }
+ virtual bool matches(const Context& context) const = 0;
+
+
/// \brief Estimated cost
///
/// Assume that the cost of the match is linear and depends on the number
- /// of compariosn operations.
+ /// of comparison operations.
virtual unsigned cost() const {
return (1); // Single check on a 32-bit word
}
+
+
///@{
/// Access methods - mainly for testing
@@ -235,24 +240,21 @@ public:
bool getInverse() {
return (inverse_);
}
-
///@}
-private:
- /// \brief Initialization
+ /// \brief Set Network Mask
///
- /// Common code shared by all constructors to set up the net mask and
- /// addresses.
- void init() {
+ /// Sets up the network mask from the mask size.
+ void setNetmask() {
// Validate that the mask is valid.
- if ((masksize_ >= 1) && (masksize_ <= 32)) {
+ if ((masksize_ >= 1) && (masksize_ <= 8 * sizeof(uint32_t))) {
// Calculate the bitmask given by the number of bits.
- netmask_ = isc::acl::createNetmask<int32_t>(masksize_);
+ netmask_ = isc::acl::createNetmask<uint32_t>(masksize_);
+
+ // ... and convert to network byte order.
+ netmask_ = htonl(netmask_);
- // For speed, store the masked off address. This saves a mask
- // operation every time the value is checked.
- maskaddr_ = address_ & netmask_;
} else {
isc_throw(isc::OutOfRange,
"mask size of " << masksize_ << " is invalid " <<
@@ -261,11 +263,45 @@ private:
}
}
+ /// \brief Comparison
+ ///
+ /// This is the actual comparison function that checks the IP address passed
+ /// to this class with the matching information in the class itself. It is
+ /// expected to be called from matches().
+ ///
+ /// \param address Address (in network byte order) to match against the
+ /// check condition in the class.
+ ///
+ /// \return true if the address matches, false if it does not.
+ virtual bool compare(uint32_t address) const {
+
+ // To check that the address given matches the stored network address
+ // and netmask, we check the simple condition that:
+ //
+ // address_given & netmask_ == stored_address & netmask_
+ //
+ // However, we must return the negation of the result if inverse_ is
+ // set. This leads to the truth table:
+ //
+ // Result inverse_ Return
+ // false false false
+ // false true true
+ // true false true
+ // true true false
+ //
+ // ... which is an XOR function. Although there is no explicit logical
+ /// XOR operator, with two bool arguments, "!=" serves that function.
+
+ return (((address & netmask_) == (address_ & netmask_)) != inverse_);
+ }
+
+ // Member variables
+
uint32_t address_; ///< IPv4 address
- uint32_t maskaddr_; ///< Masked IPV4 address
size_t masksize_; ///< Mask size passed to constructor
- int32_t netmask_; ///< Network mask applied to match
bool inverse_; ///< test for equality or inequality
+ uint32_t netmask_; ///< Network mask applied to match
+
};
} // namespace acl
diff --git a/src/lib/acl/tests/ip_check_unittest.cc b/src/lib/acl/tests/ip_check_unittest.cc
index 3560b29..9ff02e1 100644
--- a/src/lib/acl/tests/ip_check_unittest.cc
+++ b/src/lib/acl/tests/ip_check_unittest.cc
@@ -18,7 +18,34 @@
using namespace isc::acl;
-/// General tests
+// Declare a derived class to allow the abstract function to be declared
+// as a concrete one.
+
+class DerivedV4Check : public Ipv4Check<uint32_t> {
+public:
+ // Basic constructor
+ DerivedV4Check(uint32_t address = 1, size_t masksize = 32,
+ bool inverse = false) :
+ Ipv4Check<uint32_t>(address, masksize, inverse)
+ {}
+
+ // String constructor
+ DerivedV4Check(const std::string& address, bool inverse = false) :
+ Ipv4Check<uint32_t>(address, inverse)
+ {}
+
+ // Destructor
+ virtual ~DerivedV4Check()
+ {}
+
+ // Concrete implementation of abstract method
+ virtual bool matches(const uint32_t& context) const {
+ return (compare(context));
+ }
+};
+
+
+/// Tests of the free functions.
TEST(IpCheck, CreateNetmask) {
size_t i;
@@ -26,14 +53,16 @@ TEST(IpCheck, CreateNetmask) {
// 8-bit tests.
// Invalid arguments should throw.
- EXPECT_THROW(createNetmask<int8_t>(0), isc::OutOfRange);
- EXPECT_THROW(createNetmask<int8_t>(9), isc::OutOfRange);
+ EXPECT_THROW(createNetmask<uint8_t>(0), isc::OutOfRange);
+ EXPECT_THROW(createNetmask<uint8_t>(9), isc::OutOfRange);
- // Check on all possible 8-bit values
+ // Check on all possible 8-bit values. Use a signed type to generate a
+ // variable with the most significant bits set, as right-shifting it is
+ // guaranteed to introduce additional bits.
int8_t expected8;
for (i = 1, expected8 = 0x80; i <= 8; ++i, expected8 >>= 1) {
- EXPECT_EQ(static_cast<int32_t>(expected8),
- static_cast<int32_t>(createNetmask<int8_t>(i)));
+ EXPECT_EQ(static_cast<uint8_t>(expected8),
+ createNetmask<uint8_t>(i));
}
// Do the same for 32 bits.
@@ -43,99 +72,104 @@ TEST(IpCheck, CreateNetmask) {
// Check on all possible 8-bit values
int32_t expected32;
for (i = 1, expected32 = 0x80000000; i <= 32; ++i, expected32 >>= 1) {
- EXPECT_EQ(expected32, createNetmask<int32_t>(i));
+ EXPECT_EQ(static_cast<uint32_t>(expected32),
+ createNetmask<uint32_t>(i));
}
}
-
-// V4 tests
+// IPV4 tests
// Check that the constructor expands the network mask and stores the elements
// correctly. For these tests, we don't worry about the type of the context,
// so we declare it as an int.
TEST(IpCheck, V4ConstructorAddress) {
- // Alternating bits
- Ipv4Check<int> acl1(0x55555555);
- EXPECT_EQ(0x55555555, acl1.getAddress());
-
- Ipv4Check<int> acl2(0xcccccccc);
- EXPECT_EQ(0xcccccccc, acl2.getAddress());
+ DerivedV4Check acl1(0x12345678);
+ EXPECT_EQ(0x12345678, acl1.getAddress());
}
+// The mask is stored in network byte order, so the pattern expected must
+// also be converted to network byte order for the comparison to succeed.
TEST(IpCheck, V4ConstructorMask) {
// Valid values. Address of "1" is used as a placeholder
- Ipv4Check<int> acl1(1, 1);
- EXPECT_EQ(0x80000000, acl1.getNetmask());
+ DerivedV4Check acl1(1, 1);
+ uint32_t expected = htonl(0x80000000);
+ EXPECT_EQ(expected, acl1.getNetmask());
EXPECT_EQ(1, acl1.getMasksize());
- Ipv4Check<int> acl2(1, 24);
- EXPECT_EQ(0xffffff00, acl2.getNetmask());
+ DerivedV4Check acl2(1, 24);
+ expected = htonl(0xffffff00);
+ EXPECT_EQ(expected, acl2.getNetmask());
EXPECT_EQ(24, acl2.getMasksize());
// ... and some invalid network masks
- EXPECT_THROW(Ipv4Check<int>(1, 0), isc::OutOfRange);
+ EXPECT_THROW(DerivedV4Check(1, 0), isc::OutOfRange);
+ EXPECT_THROW(DerivedV4Check(1, 33), isc::OutOfRange);
}
TEST(IpCheck, V4ConstructorInverse) {
// Valid values. Address/mask of "1" is used as a placeholder
- Ipv4Check<int> acl1(1, 1);
+ DerivedV4Check acl1(1, 1);
EXPECT_FALSE(acl1.getInverse());
- Ipv4Check<int> acl2(1, 1, true);
+ DerivedV4Check acl2(1, 1, true);
EXPECT_TRUE(acl2.getInverse());
- Ipv4Check<int> acl3(1, 1, false);
+ DerivedV4Check acl3(1, 1, false);
EXPECT_FALSE(acl3.getInverse());
}
TEST(IpCheck, V4StringConstructor) {
- Ipv4Check<int> acl1("127.0.0.1");
- EXPECT_EQ(0x7f000001, acl1.getAddress());
+ DerivedV4Check acl1("127.0.0.1");
+ uint32_t expected = htonl(0x7f000001);
+ EXPECT_EQ(expected, acl1.getAddress());
EXPECT_EQ(32, acl1.getMasksize());
- Ipv4Check<int> acl2("255.255.255.0/24");
- EXPECT_EQ(0xffffff00, acl2.getAddress());
+ DerivedV4Check acl2("255.255.255.0/24");
+ expected = htonl(0xffffff00);
+ EXPECT_EQ(expected, acl2.getAddress());
EXPECT_EQ(24, acl2.getMasksize());
- EXPECT_THROW(Ipv4Check<int>("255.255.255.0/0"), isc::OutOfRange);
- EXPECT_THROW(Ipv4Check<int>("255.255.255.0/33"), isc::OutOfRange);
- EXPECT_THROW(Ipv4Check<int>("255.255.255.0/24/3"), isc::InvalidParameter);
- EXPECT_THROW(Ipv4Check<int>("255.255.255.0/ww"), isc::InvalidParameter);
- EXPECT_THROW(Ipv4Check<int>("aa.255.255.0/ww"), isc::InvalidParameter);
+ EXPECT_THROW(DerivedV4Check("255.255.255.0/0"), isc::OutOfRange);
+ EXPECT_THROW(DerivedV4Check("255.255.255.0/33"), isc::OutOfRange);
+ EXPECT_THROW(DerivedV4Check("255.255.255.0/24/3"), isc::InvalidParameter);
+ EXPECT_THROW(DerivedV4Check("255.255.255.0/ww"), isc::InvalidParameter);
+ EXPECT_THROW(DerivedV4Check("aa.255.255.0/ww"), isc::InvalidParameter);
}
-// Check that the comparison works - until we have a a message structure,
-// we can't check the matches function.
+// Check that the comparison works - note that "matches" just calls the
+// internal compare() code.
+//
+// Note that addresses passed to the class are expected to be in network-
+// byte order. Therefore for the comparisons to work as expected, we must
+// convert the values to network-byte order first.
TEST(IpCheck, V4Compare) {
- // Exact address - match if given address matches stored address
- Ipv4Check<int> acl1(0x23457f13, 32);
- EXPECT_TRUE(acl1.compare(0x23457f13));
- EXPECT_FALSE(acl1.compare(0x23457f12));
- EXPECT_FALSE(acl1.compare(0x13457f13));
+ // Exact address - match if given address matches stored address.
+ DerivedV4Check acl1(htonl(0x23457f13), 32);
+ EXPECT_TRUE(acl1.matches(htonl(0x23457f13)));
+ EXPECT_FALSE(acl1.matches(htonl(0x23457f12)));
+ EXPECT_FALSE(acl1.matches(htonl(0x13457f13)));
// Exact address - match if address does not match stored address
- Ipv4Check<int> acl2(0x23457f13, 32, true);
- EXPECT_FALSE(acl2.compare(0x23457f13));
- EXPECT_TRUE(acl2.compare(0x23457f12));
- EXPECT_TRUE(acl2.compare(0x13457f13));
+ DerivedV4Check acl2(htonl(0x23457f13), 32, true);
+ EXPECT_FALSE(acl2.matches(htonl(0x23457f13)));
+ EXPECT_TRUE(acl2.matches(htonl(0x23457f12)));
+ EXPECT_TRUE(acl2.matches(htonl(0x13457f13)));
// Match if the address matches a mask
- Ipv4Check<int> acl3(0x23450000, 16);
- EXPECT_TRUE(acl3.compare(0x23450000));
- EXPECT_TRUE(acl3.compare(0x23450001));
- EXPECT_TRUE(acl3.compare(0x2345ffff));
- EXPECT_FALSE(acl3.compare(0x23460000));
- EXPECT_FALSE(acl3.compare(0x2346ffff));
+ DerivedV4Check acl3(htonl(0x23450000), 16);
+ EXPECT_TRUE(acl3.matches(htonl(0x23450000)));
+ EXPECT_TRUE(acl3.matches(htonl(0x23450001)));
+ EXPECT_TRUE(acl3.matches(htonl(0x2345ffff)));
+ EXPECT_FALSE(acl3.matches(htonl(0x23460000)));
+ EXPECT_FALSE(acl3.matches(htonl(0x2346ffff)));
// Match if the address does not match a mask
- Ipv4Check<int> acl4(0x23450000, 16, true);
- EXPECT_FALSE(acl4.compare(0x23450000));
- EXPECT_FALSE(acl4.compare(0x23450001));
- EXPECT_FALSE(acl4.compare(0x2345ffff));
- EXPECT_TRUE(acl4.compare(0x23460000));
- EXPECT_TRUE(acl4.compare(0x2346ffff));
-
- //
-}
+ DerivedV4Check acl4(htonl(0x23450000), 16, true);
+ EXPECT_FALSE(acl4.matches(htonl(0x23450000)));
+ EXPECT_FALSE(acl4.matches(htonl(0x23450001)));
+ EXPECT_FALSE(acl4.matches(htonl(0x2345ffff)));
+ EXPECT_TRUE(acl4.matches(htonl(0x23460000)));
+ EXPECT_TRUE(acl4.matches(htonl(0x2346ffff)));
+}
More information about the bind10-changes
mailing list