[master] Merge branch 'trac1641'

src/lib/dns/rdata/generic/detail/nsec_bitmap.cc

@@ -12,11 +12,17 @@
 // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
 // PERFORMANCE OF THIS SOFTWARE.
 
-#include <stdint.h>
-
-#include <vector>
+#include <exceptions/exceptions.h>
 
 #include <dns/exceptions.h>
+#include <dns/rdata.h>
+#include <dns/rrtype.h>
+
+#include <cassert>
+#include <sstream>
+#include <vector>
+#include <cstring>
+#include <stdint.h>
 
 using namespace std;
 
@@ -70,6 +76,78 @@ checkRRTypeBitmaps(const char* const rrtype_name,
         first = false;
     }
 }
+
+void
+buildBitmapsFromText(const char* const rrtype_name,
+                     istringstream& iss, vector<uint8_t>& typebits)
+{
+    uint8_t bitmap[8 * 1024];       // 64k bits
+    memset(bitmap, 0, sizeof(bitmap));
+
+    do {
+        string type;
+        iss >> type;
+        if (iss.bad() || iss.fail()) {
+            isc_throw(InvalidRdataText, "Unexpected input for "
+                      << rrtype_name << " bitmap");
+        }
+        try {
+            const int code = RRType(type).getCode();
+            bitmap[code / 8] |= (0x80 >> (code % 8));
+        } catch (const InvalidRRType&) {
+            isc_throw(InvalidRdataText, "Invalid RRtype in "
+                      << rrtype_name << " bitmap: " << type);
+        }
+    } while (!iss.eof());
+
+    for (int window = 0; window < 256; ++window) {
+        int octet;
+        for (octet = 31; octet >= 0; octet--) {
+            if (bitmap[window * 32 + octet] != 0) {
+                break;
+            }
+        }
+        if (octet < 0) {
+            continue;
+        }
+        typebits.push_back(window);
+        typebits.push_back(octet + 1);
+        for (int i = 0; i <= octet; ++i) {
+            typebits.push_back(bitmap[window * 32 + i]);
+        }
+    }
+}
+
+void
+bitmapsToText(const vector<uint8_t>& typebits, ostringstream& oss) {
+    // In the following loop we use string::at() rather than operator[].
+    // Since the index calculation is a bit complicated, it will be safer
+    // and easier to find a bug (if any).  Note that this conversion method
+    // is generally not expected to be very efficient, so the slight overhead
+    // of at() should be acceptable.
+    const size_t typebits_len = typebits.size();
+    size_t len = 0;
+    for (size_t i = 0; i < typebits_len; i += len) {
+        assert(i + 2 <= typebits.size());
+        const unsigned int block = typebits.at(i);
+        len = typebits.at(i + 1);
+        assert(len > 0 && len <= 32);
+        i += 2;
+        for (size_t j = 0; j < len; ++j) {
+            if (typebits.at(i + j) == 0) {
+                continue;
+            }
+            for (size_t k = 0; k < 8; ++k) {
+                if ((typebits.at(i + j) & (0x80 >> k)) == 0) {
+                    continue;
+                }
+                const unsigned int t = block * 256 + j * 8 + k;
+                assert(t < 65536);
+                oss << " " << RRType(t);
+            }
+        }
+    }
+}
 }
 }
 }

src/lib/dns/rdata/generic/detail/nsec_bitmap.h

@@ -14,6 +14,7 @@
 
 #include <stdint.h>
 
+#include <sstream>
 #include <vector>
 
 namespace isc {
@@ -22,14 +23,22 @@ namespace rdata {
 namespace generic {
 namespace detail {
 namespace nsec {
-/// Check if a given "type bitmap" for NSEC/NSEC3 is valid.
+
+/// \file
+///
+/// This helper module provides some utilities that handle NSEC and NSEC3
+/// type bitmaps.  The format and processing of the type bitmaps are generally
+/// the same for these two RRs, so it would make sense to consolidate
+/// the processing logic into a single implementation module.
+///
+/// The functions defined here are essentially private and are only expected
+/// to be called from the \c NSEC and \c NSEC3 class implementations.
+
+/// \brief Check if a given "type bitmap" for NSEC/NSEC3 is valid.
 ///
-/// This helper function checks given wire format data (stored in a
+/// This function checks given wire format data (stored in a
 /// \c std::vector) is a valid type bitmaps used for the NSEC and NSEC3 RRs
-/// according to RFC4034 and RFC5155.  The validation logic is the same
-/// for these two RRs, so a unified check function is provided.
-/// This function is essentially private and is only expected to be called
-/// from the \c NSEC and \c NSEC3 class implementations.
+/// according to RFC4034 and RFC5155.
 ///
 /// \exception DNSMessageFORMERR The bitmap is not valid.
 ///
@@ -39,6 +48,48 @@ namespace nsec {
 /// is the total length of the bitmaps.
 void checkRRTypeBitmaps(const char* const rrtype_name,
                         const std::vector<uint8_t>& typebits);
+
+/// \brief Convert textual sequence of RR types into type bitmaps.
+///
+/// This function extracts a sequence of strings, converts each sequence
+/// into an RR type, and builds NSEC/NSEC3 type bitmaps with the corresponding
+/// bits for the extracted types being on.  The resulting bitmaps (which are
+/// in the wire format according to RFC4034 and RFC5155) are stored in the
+/// given vector.  This function expects the given string stream ends with
+/// the sequence.
+///
+/// \exception InvalidRdataText The given input stream does not meet the
+/// assumption (e.g. including invalid form of RR type, not ending with
+/// an RR type string).
+///
+/// \param rrtype_name Either "NSEC" or "NSEC3"; used as part of exception
+/// messages.
+/// \param iss Input stream that consists of a complete sequence of textual
+/// RR types for which the corresponding bits are set.
+/// \param typebits A placeholder for the resulting bitmaps.  Expected to be
+/// empty, but it's not checked.
+void buildBitmapsFromText(const char* const rrtype_name,
+                          std::istringstream& iss,
+                          std::vector<uint8_t>& typebits);
+
+/// \brief Convert type bitmaps to textual sequence of RR types.
+///
+/// This function converts wire-format data of type bitmaps for NSEC/NSEC3
+/// into a sequence of corresponding RR type strings, and inserts them
+/// into the given output stream with separating them by a single space
+/// character.
+///
+/// This function assumes the given bitmaps are valid in terms of RFC4034
+/// and RFC5155 (in practice, it assumes it's from a validly constructed
+/// NSEC or NSEC3 object); if it detects a format error, it aborts the
+/// program with assert().
+///
+/// \param typebits The type bitmaps in wire format.  The size of vector
+/// is the total length of the bitmaps.
+/// \param oss The output stream to which the converted RR type sequence
+/// are to be inserted.
+void bitmapsToText(const std::vector<uint8_t>& typebits,
+                   std::ostringstream& oss);
 }
 }
 }

src/lib/dns/rdata/generic/nsec3_50.cc

@@ -53,12 +53,12 @@ struct NSEC3Impl {
         salt_(salt), next_(next), typebits_(typebits)
     {}
 
-    uint8_t hashalg_;
-    uint8_t flags_;
-    uint16_t iterations_;
-    vector<uint8_t> salt_;
-    vector<uint8_t> next_;
-    vector<uint8_t> typebits_;
+    const uint8_t hashalg_;
+    const uint8_t flags_;
+    const uint16_t iterations_;
+    const vector<uint8_t> salt_;
+    const vector<uint8_t> next_;
+    const vector<uint8_t> typebits_;
 };
 
 NSEC3::NSEC3(const string& nsec3_str) :
@@ -116,36 +116,7 @@ NSEC3::NSEC3(const string& nsec3_str) :
     }
 
     vector<uint8_t> typebits;
-    uint8_t bitmap[8 * 1024];       // 64k bits
-    memset(bitmap, 0, sizeof(bitmap));
-    do { 
-        string type;
-        iss >> type;
-        if (type.length() != 0) {
-            try {
-                const int code = RRType(type).getCode();
-                bitmap[code / 8] |= (0x80 >> (code % 8));
-            } catch (...) {
-                isc_throw(InvalidRdataText, "Invalid RRtype in NSEC3");
-            }
-        }
-    } while (!iss.eof());
-
-    for (int window = 0; window < 256; window++) {
-        int octet;
-        for (octet = 31; octet >= 0; octet--) {
-            if (bitmap[window * 32 + octet] != 0) {
-                break;
-            }
-        }
-        if (octet < 0)
-            continue;
-        typebits.push_back(window);
-        typebits.push_back(octet + 1);
-        for (int i = 0; i <= octet; i++) {
-            typebits.push_back(bitmap[window * 32 + i]);
-        }
-    }
+    buildBitmapsFromText("NSEC3", iss, typebits);
 
     impl_ = new NSEC3Impl(hashalg, flags, iterations, salt, next, typebits);
 }
@@ -174,6 +145,10 @@ NSEC3::NSEC3(InputBuffer& buffer, size_t rdata_len) {
         rdata_len -= saltlen;
     }
 
+    if (rdata_len < 1) {
+        isc_throw(DNSMessageFORMERR, "NSEC3 too short to contain hash length, "
+                  "length: " << rdata_len + saltlen + 5);
+    }
     const uint8_t nextlen = buffer.readUint8();
     --rdata_len;
     if (nextlen == 0 || rdata_len < nextlen) {
@@ -220,57 +195,78 @@ NSEC3::~NSEC3() {
 string
 NSEC3::toText() const {
     ostringstream s;
-    int len = 0;
-    for (size_t i = 0; i < impl_->typebits_.size(); i += len) {
-        assert(i + 2 <= impl_->typebits_.size());
-        int window = impl_->typebits_[i];
-        len = impl_->typebits_[i + 1];
-        assert(len >= 0 && len < 32);
-        i += 2;
-        for (int j = 0; j < len; j++) {
-            if (impl_->typebits_[i + j] == 0) {
-                continue;
-            }
-            for (int k = 0; k < 8; k++) {
-                if ((impl_->typebits_[i + j] & (0x80 >> k)) == 0) {
-                    continue;
-                }
-                int t = window * 256 + j * 8 + k;
-                s << " " << RRType(t).toText();
-            }
-        }
-    }
+    bitmapsToText(impl_->typebits_, s);
 
     using namespace boost;
     return (lexical_cast<string>(static_cast<int>(impl_->hashalg_)) +
-        " " + lexical_cast<string>(static_cast<int>(impl_->flags_)) +
-        " " + lexical_cast<string>(static_cast<int>(impl_->iterations_)) +
-        " " + encodeHex(impl_->salt_) +
-        " " + encodeBase32Hex(impl_->next_) + s.str());
+            " " + lexical_cast<string>(static_cast<int>(impl_->flags_)) +
+            " " + lexical_cast<string>(static_cast<int>(impl_->iterations_)) +
+            " " + (impl_->salt_.empty() ? "-" : encodeHex(impl_->salt_)) +
+            " " + encodeBase32Hex(impl_->next_) + s.str());
+}
+
+template <typename OUTPUT_TYPE>
+void
+toWireHelper(const NSEC3Impl& impl, OUTPUT_TYPE& output) {
+    output.writeUint8(impl.hashalg_);
+    output.writeUint8(impl.flags_);
+    output.writeUint16(impl.iterations_);
+    output.writeUint8(impl.salt_.size());
+    if (!impl.salt_.empty()) {
+        output.writeData(&impl.salt_[0], impl.salt_.size());
+    }
+    assert(!impl.next_.empty());
+    output.writeUint8(impl.next_.size());
+    output.writeData(&impl.next_[0], impl.next_.size());
+    if (!impl.typebits_.empty()) {
+        output.writeData(&impl.typebits_[0], impl.typebits_.size());
+    }
 }
 
 void
 NSEC3::toWire(OutputBuffer& buffer) const {
-    buffer.writeUint8(impl_->hashalg_);
-    buffer.writeUint8(impl_->flags_);
-    buffer.writeUint16(impl_->iterations_);
-    buffer.writeUint8(impl_->salt_.size());
-    buffer.writeData(&impl_->salt_[0], impl_->salt_.size());
-    buffer.writeUint8(impl_->next_.size());
-    buffer.writeData(&impl_->next_[0], impl_->next_.size());
-    buffer.writeData(&impl_->typebits_[0], impl_->typebits_.size());
+    toWireHelper(*impl_, buffer);
 }
 
 void
 NSEC3::toWire(AbstractMessageRenderer& renderer) const {
-    renderer.writeUint8(impl_->hashalg_);
-    renderer.writeUint8(impl_->flags_);
-    renderer.writeUint16(impl_->iterations_);
-    renderer.writeUint8(impl_->salt_.size());
-    renderer.writeData(&impl_->salt_[0], impl_->salt_.size());
-    renderer.writeUint8(impl_->next_.size());
-    renderer.writeData(&impl_->next_[0], impl_->next_.size());
-    renderer.writeData(&impl_->typebits_[0], impl_->typebits_.size());
+    toWireHelper(*impl_, renderer);
+}
+
+namespace {
+// This is a helper subroutine for compare().  It compares two binary
+// data stored in vector<uint8_t> objects based on the "Canonical RR Ordering"
+// as defined in Section 6.3 of RFC4034, that is, the data are treated
+// "as a left-justified unsigned octet sequence in which the absence of an
+// octet sorts before a zero octet."
+//
+// If check_length_first is true, it treats the compared data as if they
+// began with a single-octet "length" field whose value is the size of the
+// corresponding vector.  In this case, if the sizes of the two vectors are
+// different the shorter one is always considered the "smaller"; the contents
+// of the vector don't matter.
+//
+// This function returns:
+// -1 if v1 is considered smaller than v2
+// 1 if v1 is considered larger than v2
+// 0 otherwise
+int
+compareVectors(const vector<uint8_t>& v1, const vector<uint8_t>& v2,
+               bool check_length_first = true)
+{
+    const size_t len1 = v1.size();
+    const size_t len2 = v2.size();
+    const size_t cmplen = min(len1, len2);
+    if (check_length_first && len1 != len2) {
+        return (len1 - len2);
+    }
+    const int cmp = cmplen == 0 ? 0 : memcmp(&v1.at(0), &v2.at(0), cmplen);
+    if (cmp != 0) {
+        return (cmp);
+    } else {
+        return (len1 - len2);
+    }
+}
 }
 
 int
@@ -287,44 +283,18 @@ NSEC3::compare(const Rdata& other) const {
         return (impl_->iterations_ < other_nsec3.impl_->iterations_ ? -1 : 1);
     }
 
-    size_t this_len = impl_->salt_.size();
-    size_t other_len = other_nsec3.impl_->salt_.size();
-    size_t cmplen = min(this_len, other_len);
-    int cmp = memcmp(&impl_->salt_[0], &other_nsec3.impl_->salt_[0], cmplen);
+    int cmp = compareVectors(impl_->salt_, other_nsec3.impl_->salt_);
     if (cmp != 0) {
         return (cmp);
-    } else if (this_len < other_len) {
-        return (-1);
-    } else if (this_len > other_len) {
-        return (1);
     }
-
-    this_len = impl_->salt_.size();
-    other_len = other_nsec3.impl_->salt_.size();
-    cmplen = min(this_len, other_len);
-    cmp = memcmp(&impl_->next_[0], &other_nsec3.impl_->next_[0], cmplen);
-    if (cmp != 0) {
-        return (cmp);
-    } else if (this_len < other_len) {
-        return (-1);
-    } else if (this_len > other_len) {
-        return (1);
-    }
-
-    this_len = impl_->typebits_.size();
-    other_len = other_nsec3.impl_->typebits_.size();
-    cmplen = min(this_len, other_len);
-    cmp = memcmp(&impl_->typebits_[0], &other_nsec3.impl_->typebits_[0],
-                 cmplen);
+    cmp = compareVectors(impl_->next_, other_nsec3.impl_->next_);
     if (cmp != 0) {
         return (cmp);
-    } else if (this_len < other_len) {
-        return (-1);
-    } else if (this_len > other_len) {
-        return (1);
-    } else {
-        return (0);
     }
+    // Note that bitmap doesn't have a dedicated length field, so we shouldn't
+    // terminate the comparison just because the lengths are different.
+    return (compareVectors(impl_->typebits_, other_nsec3.impl_->typebits_,
+                           false));
 }
 
 uint8_t

src/lib/dns/rdata/generic/nsec_47.cc

@@ -54,42 +54,18 @@ NSEC::NSEC(const string& nsec_str) :
 {
     istringstream iss(nsec_str);
     string nextname;
-    uint8_t bitmap[8 * 1024];       // 64k bits
-    vector<uint8_t> typebits;
 
     iss >> nextname;
     if (iss.bad() || iss.fail()) {
         isc_throw(InvalidRdataText, "Invalid NSEC name");
     }
-
-    memset(bitmap, 0, sizeof(bitmap));
-    do { 
-        string type;
-        iss >> type;
-        try {
-            const int code = RRType(type).getCode();
-            bitmap[code / 8] |= (0x80 >> (code % 8));
-        } catch (...) {
-            isc_throw(InvalidRdataText, "Invalid RRtype in NSEC");
-        }
-    } while (!iss.eof());
-
-    for (int window = 0; window < 256; window++) {
-        int octet;
-        for (octet = 31; octet >= 0; octet--) {
-            if (bitmap[window * 32 + octet] != 0) {
-                break;
-            }
-        }
-        if (octet < 0)
-            continue;
-        typebits.push_back(window);
-        typebits.push_back(octet + 1);
-        for (int i = 0; i <= octet; i++) {
-            typebits.push_back(bitmap[window * 32 + i]);
-        }
+    if (iss.eof()) {
+        isc_throw(InvalidRdataText, "NSEC bitmap is missing");
     }
 
+    vector<uint8_t> typebits;
+    buildBitmapsFromText("NSEC", iss, typebits);
+
     impl_ = new NSECImpl(Name(nextname), typebits);
 }
 
@@ -135,34 +111,8 @@ NSEC::~NSEC() {
 string
 NSEC::toText() const {
     ostringstream s;
-    int len = 0;
     s << impl_->nextname_;
-
-    // In the following loop we use string::at() rather than operator[].
-    // Since the index calculation is a bit complicated, it will be safer
-    // and easier to find a bug (if any).  Note that this conversion method
-    // is generally not expected to be very efficient, so the slight overhead
-    // of at() should be acceptable.
-    for (size_t i = 0; i < impl_->typebits_.size(); i += len) {
-        assert(i + 2 <= impl_->typebits_.size());
-        const int block = impl_->typebits_.at(i);
-        len = impl_->typebits_.at(i + 1);
-        assert(len > 0 && len <= 32);
-        i += 2;
-        for (int j = 0; j < len; j++) {
-            if (impl_->typebits_.at(i + j) == 0) {
-                continue;
-            }
-            for (int k = 0; k < 8; k++) {
-                if ((impl_->typebits_.at(i + j) & (0x80 >> k)) == 0) {
-                    continue;
-                }
-                const int t = block * 256 + j * 8 + k;
-                s << " " << RRType(t);
-            }
-        }
-    }
-
+    bitmapsToText(impl_->typebits_, s);
     return (s.str());
 }
 

src/lib/dns/tests/rdata_nsec3_unittest.cc

@@ -44,8 +44,14 @@ class Rdata_NSEC3_Test : public RdataTest {
 public:
     Rdata_NSEC3_Test() :
         nsec3_txt("1 1 1 D399EAAB H9RSFB7FPF2L8HG35CMPC765TDK23RP6 "
-                  "NS SOA RRSIG DNSKEY NSEC3PARAM") {}
-    string nsec3_txt;
+                  "NS SOA RRSIG DNSKEY NSEC3PARAM"),
+        nsec3_nosalt_txt("1 1 1 - H9RSFB7FPF2L8HG35CMPC765TDK23RP6 A" ),
+        obuffer(0), renderer(obuffer)
+    {}
+    const string nsec3_txt;
+    const string nsec3_nosalt_txt;
+    OutputBuffer obuffer;
+    MessageRenderer renderer;
 };
 
 TEST_F(Rdata_NSEC3_Test, fromText) {
@@ -60,8 +66,7 @@ TEST_F(Rdata_NSEC3_Test, fromText) {
                                    "NS SOA RRSIG DNSKEY NSEC3PARAM"));
 
     // 0-length salt
-    EXPECT_EQ(0, generic::NSEC3("1 1 1 - H9RSFB7FPF2L8HG35CMPC765TDK23RP6 "
-                                "A").getSalt().size());
+    EXPECT_EQ(0, generic::NSEC3(nsec3_nosalt_txt).getSalt().size());
 
     // salt that has the possible max length
     EXPECT_EQ(255, generic::NSEC3("1 1 1 " + string(255 * 2, '0') +
@@ -80,8 +85,12 @@ TEST_F(Rdata_NSEC3_Test, fromText) {
 }
 
 TEST_F(Rdata_NSEC3_Test, toText) {
+    // normal case
     const generic::NSEC3 rdata_nsec3(nsec3_txt);
     EXPECT_EQ(nsec3_txt, rdata_nsec3.toText());
+
+    // empty salt case
+    EXPECT_EQ(nsec3_nosalt_txt, generic::NSEC3(nsec3_nosalt_txt).toText());
 }
 
 TEST_F(Rdata_NSEC3_Test, badText) {
@@ -165,7 +174,11 @@ TEST_F(Rdata_NSEC3_Test, createFromWire) {
                                       "rdata_nsec3_fromWire14.wire"),
                  DNSMessageFORMERR);
 
-    //
+    // RDLEN is too short to hold the hash length field
+    EXPECT_THROW(rdataFactoryFromFile(RRType::NSEC3(), RRClass::IN(),
+                                      "rdata_nsec3_fromWire17.wire"),
+                 DNSMessageFORMERR);
+
     // Short buffer cases.  The data is valid NSEC3 RDATA, but the buffer
     // is trimmed at the end.  All cases should result in an exception from
     // the buffer class.
@@ -180,21 +193,35 @@ TEST_F(Rdata_NSEC3_Test, createFromWire) {
     }
 }
 
-TEST_F(Rdata_NSEC3_Test, toWireRenderer) {
-    renderer.skip(2);
-    const generic::NSEC3 rdata_nsec3(nsec3_txt);
-    rdata_nsec3.toWire(renderer);
-
-    vector<unsigned char> data;
-    UnitTestUtil::readWireData("rdata_nsec3_fromWire1", data);
-    EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
-                        static_cast<const uint8_t *>(obuffer.getData()) + 2,
-                        obuffer.getLength() - 2, &data[2], data.size() - 2);
+template <typename OUTPUT_TYPE>
+void
+toWireCheck(OUTPUT_TYPE& output, const char* const data_file) {
+    vector<uint8_t> data;
+    UnitTestUtil::readWireData(data_file, data);
+    InputBuffer buffer(&data[0], data.size());
+    const uint16_t rdlen = buffer.readUint16();
+    const generic::NSEC3 nsec3 =
+        dynamic_cast<const generic::NSEC3&>(*createRdata(
+                                                RRType::NSEC3(), RRClass::IN(),
+                                                buffer, rdlen));
+
+    output.clear();
+    output.writeUint16(rdlen);
+    nsec3.toWire(output);
+    EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData, output.getData(),
+                        output.getLength(), &data[0], data.size());
 }
 
-TEST_F(Rdata_NSEC3_Test, toWireBuffer) {
-    const generic::NSEC3 rdata_nsec3(nsec3_txt);
-    rdata_nsec3.toWire(obuffer);
+TEST_F(Rdata_NSEC3_Test, toWire) {
+    // normal case
+    toWireCheck(renderer, "rdata_nsec3_fromWire1");
+    toWireCheck(obuffer, "rdata_nsec3_fromWire1");