BIND 10 trac2522, updated. 7e0b47d099a071d438c45e7cde177d89a0f3151e [2522] TSIG documentation, TSIG unit test string constructors

BIND 10 source code commits bind10-changes at lists.isc.org
Wed May 22 03:02:42 UTC 2013


The branch, trac2522 has been updated
       via  7e0b47d099a071d438c45e7cde177d89a0f3151e (commit)
       via  87a710de67996ae01dcee1c2e05f09c9c392f82d (commit)
      from  7acdd8d445081e1a4c70ed58298367a96ccb55fd (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 7e0b47d099a071d438c45e7cde177d89a0f3151e
Author: Paul Selkirk <pselkirk at isc.org>
Date:   Tue May 21 23:02:46 2013 -0400

    [2522] TSIG documentation, TSIG unit test string constructors

commit 87a710de67996ae01dcee1c2e05f09c9c392f82d
Author: Paul Selkirk <pselkirk at isc.org>
Date:   Tue May 21 16:14:58 2013 -0400

    [2522] support multiline or space-separated fingerprint text in SSHFP

-----------------------------------------------------------------------

Summary of changes:
 src/lib/dns/rdata/any_255/tsig_250.cc     |   44 +++++++++++---------
 src/lib/dns/rdata/generic/sshfp_44.cc     |   25 +++++++++---
 src/lib/dns/tests/rdata_sshfp_unittest.cc |   14 ++++++-
 src/lib/dns/tests/rdata_tsig_unittest.cc  |   63 ++++++++++++++++++-----------
 4 files changed, 97 insertions(+), 49 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/lib/dns/rdata/any_255/tsig_250.cc b/src/lib/dns/rdata/any_255/tsig_250.cc
index aff8c62..4e86cf9 100644
--- a/src/lib/dns/rdata/any_255/tsig_250.cc
+++ b/src/lib/dns/rdata/any_255/tsig_250.cc
@@ -126,6 +126,8 @@ TSIG::constructFromLexer(MasterLexer& lexer, const Name* origin) {
     } else if (error_txt == "BADTIME") {
         error = TSIGError::BAD_TIME_CODE;
     } else {
+	/// we cast to uint32_t and range-check, because casting directly to
+	/// uint16_t will convert negative numbers to large positive numbers
         try {
             error = boost::lexical_cast<uint32_t>(error_txt);
         } catch (const boost::bad_lexical_cast&) {
@@ -158,27 +160,33 @@ TSIG::constructFromLexer(MasterLexer& lexer, const Name* origin) {
 
 /// \brief Constructor from string.
 ///
-/// The given string must represent a valid TSIG RDATA.  There can be
-/// extra space characters at the beginning or end of the text (which
-/// are simply ignored), but other extra text, including a new line,
-/// will make the construction fail with an exception.
+/// The given string must represent a valid TSIG RDATA.  There can be extra
+/// space characters at the beginning or end of the text (which are simply
+/// ignored), but other extra text, including a new line, will make the
+/// construction fail with an exception.
 ///
-/// Note that, since Algorithm Name is defined to be "in domain name syntax",
-/// but it is not actually a domain name, it does not have to be fully
-/// qualified.
+/// \c tsig_str must be formatted as follows:
+/// \code <Algorithm Name> <Time Signed> <Fudge> <MAC Size> [<MAC>]
+/// <Original ID> <Error> <Other Len> [<Other Data>]
+/// \endcode
 ///
-/// Error is an unsigned 16-bit decimal integer or a valid mnemonic for the
-/// Error field specified in RFC2845.  Currently, "NOERROR", "BADSIG",
-/// "BADKEY", and "BADTIME" are supported (case sensitive).  In future
-/// versions other representations that are compatible with the DNS RCODE
-/// will be supported.
+/// Note that, since the Algorithm Name field is defined to be "in domain name
+/// syntax", but it is not actually a domain name, it does not have to be
+/// fully qualified.
 ///
-/// MAC and Other Data are base-64 encoded strings that do not contain space
-/// characters.
-/// If MAC Size or Other Len is 0, MAC or Other Data must not appear in
-/// \c tsig_str, respectively.
-/// The decoded data of MAC is MAC Size bytes of binary stream.
-/// The decoded data of Other Data is Other Len bytes of binary stream.
+/// The Error field is an unsigned 16-bit decimal integer or a valid mnemonic
+/// as specified in RFC2845.  Currently, "NOERROR", "BADSIG", "BADKEY", and
+/// "BADTIME" are supported (case sensitive).  In future versions other
+/// representations that are compatible with the DNS RCODE may be supported.
+///
+/// The MAC and Other Data fields are base-64 encoded strings that do not
+/// contain space characters.
+/// If the MAC Size field is 0, the MAC field must not appear in \c tsig_str.
+/// If the Other Len field is 0, the Other Data field must not appear in
+/// \c tsig_str.
+/// The decoded data of the MAC field is MAC Size bytes of binary stream.
+/// The decoded data of the Other Data field is Other Len bytes of binary
+/// stream.
 ///
 /// An example of valid string is:
 /// \code "hmac-sha256. 853804800 300 3 AAAA 2845 0 0" \endcode
diff --git a/src/lib/dns/rdata/generic/sshfp_44.cc b/src/lib/dns/rdata/generic/sshfp_44.cc
index d7cf3ce..dc74050 100644
--- a/src/lib/dns/rdata/generic/sshfp_44.cc
+++ b/src/lib/dns/rdata/generic/sshfp_44.cc
@@ -64,11 +64,25 @@ SSHFP::constructFromLexer(MasterLexer& lexer) {
         isc_throw(InvalidRdataText, "SSHFP fingerprint type out of range");
     }
 
-    const MasterToken& token = lexer.getNextToken(MasterToken::STRING, true);
+    std::string fingerprint_str;
+    std::string fingerprint_substr;
+    while (true) {
+        const MasterToken& token =
+            lexer.getNextToken(MasterToken::STRING, true);
+        if ((token.getType() == MasterToken::END_OF_FILE) ||
+            (token.getType() == MasterToken::END_OF_LINE)) {
+            break;
+        }
+        token.getString(fingerprint_substr);
+        fingerprint_str.append(fingerprint_substr);
+    }
+    lexer.ungetToken();
+
     vector<uint8_t> fingerprint;
-    if ((token.getType() != MasterToken::END_OF_FILE) &&
-        (token.getType() != MasterToken::END_OF_LINE)) {
-        decodeHex(token.getString(), fingerprint);
+    // If fingerprint is missing, it's OK. See the API documentation of the
+    // constructor.
+    if (fingerprint_str.size() > 0) {
+        decodeHex(fingerprint_str, fingerprint);
     }
 
     return (new SSHFPImpl(algorithm, fingerprint_type, fingerprint));
@@ -85,7 +99,8 @@ SSHFP::constructFromLexer(MasterLexer& lexer) {
 /// ranges, but are not constrained to the values defined in RFC4255.
 ///
 /// The Fingerprint field may be absent, but if present it must contain a
-/// valid hex encoding of the fingerprint.
+/// valid hex encoding of the fingerprint. For compatibility with BIND 9,
+/// whitespace is allowed in the hex text (RFC4255 is silent on the matter).
 ///
 /// \throw InvalidRdataText if any fields are missing, out of their valid
 /// ranges, or incorrect.
diff --git a/src/lib/dns/tests/rdata_sshfp_unittest.cc b/src/lib/dns/tests/rdata_sshfp_unittest.cc
index 0117d7d..c900367 100644
--- a/src/lib/dns/tests/rdata_sshfp_unittest.cc
+++ b/src/lib/dns/tests/rdata_sshfp_unittest.cc
@@ -95,7 +95,17 @@ TEST_F(Rdata_SSHFP_Test, createFromText) {
     checkFromText_None("2 1   123456789abcdef67890123456789abcdef67890");
 
     // Combination of lowercase and uppercase
-    checkFromText_None("2 1   123456789ABCDEF67890123456789abcdef67890");
+    checkFromText_None("2 1 123456789ABCDEF67890123456789abcdef67890");
+
+    // spacing in the fingerprint field
+    checkFromText_None("2 1 123456789abcdef67890 123456789abcdef67890");
+
+    // multi-line fingerprint field
+    checkFromText_None("2 1 ( 123456789abcdef67890\n 123456789abcdef67890 )");
+
+    // string constructor throws if there's extra text,
+    // but lexer constructor doesn't
+    checkFromText_BadString(sshfp_txt + "\n" + sshfp_txt);
 }
 
 TEST_F(Rdata_SSHFP_Test, algorithmTypes) {
@@ -129,7 +139,7 @@ TEST_F(Rdata_SSHFP_Test, badText) {
     checkFromText_LexerError("ONE 2 123456789abcdef67890123456789abcdef67890");
     checkFromText_LexerError("1 TWO 123456789abcdef67890123456789abcdef67890");
     checkFromText_BadValue("1 2 BUCKLEMYSHOE");
-    checkFromText_BadString(sshfp_txt + " extra text");
+    checkFromText_BadValue(sshfp_txt + " extra text");
 
     // yes, these are redundant to the last test cases in algorithmTypes
     checkFromText_InvalidText(
diff --git a/src/lib/dns/tests/rdata_tsig_unittest.cc b/src/lib/dns/tests/rdata_tsig_unittest.cc
index 72529a7..8e91c14 100644
--- a/src/lib/dns/tests/rdata_tsig_unittest.cc
+++ b/src/lib/dns/tests/rdata_tsig_unittest.cc
@@ -38,20 +38,25 @@ using namespace isc::util;
 using namespace isc::dns::rdata;
 
 namespace {
-const string valid_text1 = "hmac-md5.sig-alg.reg.int. 1286779327 300 "
-    "0 16020 BADKEY 0";
-const string valid_text2 = "hmac-sha256. 1286779327 300 12 "
-    "FAKEFAKEFAKEFAKE 16020 BADSIG 0";
-const string valid_text3 = "hmac-sha1. 1286779327 300 12 "
-    "FAKEFAKEFAKEFAKE 16020 BADTIME 6 FAKEFAKE";
-const string valid_text4 = "hmac-sha1. 1286779327 300 12 "
-    "FAKEFAKEFAKEFAKE 16020 BADSIG 6 FAKEFAKE";
-const string valid_text5 = "hmac-sha256. 1286779327 300 12 "
-    "FAKEFAKEFAKEFAKE 16020 2845 0"; // using numeric error code
 
 class Rdata_TSIG_Test : public RdataTest {
 protected:
     Rdata_TSIG_Test() :
+	// no MAC or Other Data
+        valid_text1("hmac-md5.sig-alg.reg.int. 1286779327 300 "
+                    "0 16020 BADKEY 0"),
+	// MAC but no Other Data
+        valid_text2("hmac-sha256. 1286779327 300 12 "
+                    "FAKEFAKEFAKEFAKE 16020 BADSIG 0"),
+	// MAC and Other Data
+        valid_text3("hmac-sha1. 1286779327 300 12 "
+                    "FAKEFAKEFAKEFAKE 16020 BADTIME 6 FAKEFAKE"),
+	// MAC and Other Data (with Error that doesn't expect Other Data)
+        valid_text4("hmac-sha1. 1286779327 300 12 "
+                    "FAKEFAKEFAKEFAKE 16020 BADSIG 6 FAKEFAKE"),
+	// numeric error code
+        valid_text5("hmac-sha256. 1286779327 300 12 "
+                    "FAKEFAKEFAKEFAKE 16020 2845 0"),
         rdata_tsig(valid_text1)
     {}
 
@@ -90,6 +95,11 @@ protected:
     template <typename Output>
     void toWireCommonChecks(Output& output) const;
 
+    const string valid_text1;
+    const string valid_text2;
+    const string valid_text3;
+    const string valid_text4;
+    const string valid_text5;
     vector<uint8_t> expect_data;
     const any::TSIG rdata_tsig; // commonly used test RDATA
 };
@@ -106,19 +116,24 @@ TEST_F(Rdata_TSIG_Test, fromText) {
     EXPECT_EQ(0, rdata_tsig.getOtherLen());
     EXPECT_EQ(static_cast<void*>(NULL), rdata_tsig.getOtherData());
 
-    any::TSIG tsig2((string(valid_text2)));
+    any::TSIG tsig2(valid_text2);
     EXPECT_EQ(12, tsig2.getMACSize());
     EXPECT_EQ(TSIGError::BAD_SIG_CODE, tsig2.getError());
 
-    any::TSIG tsig3((string(valid_text3)));
+    any::TSIG tsig3(valid_text3);
     EXPECT_EQ(6, tsig3.getOtherLen());
 
     // The other data is unusual, but we don't reject it.
-    EXPECT_NO_THROW(any::TSIG(string(valid_text4)));
+    EXPECT_NO_THROW(any::TSIG tsig4(valid_text4));
 
     // numeric representation of TSIG error
-    any::TSIG tsig5((string(valid_text5)));
+    any::TSIG tsig5(valid_text5);
     EXPECT_EQ(2845, tsig5.getError());
+
+    // not fully qualified algorithm name
+    any::TSIG tsig1("hmac-md5.sig-alg.reg.int 1286779327 300 "
+                    "0 16020 BADKEY 0");
+    EXPECT_EQ(0, tsig1.compare(rdata_tsig));
 }
 
 TEST_F(Rdata_TSIG_Test, badText) {
@@ -269,12 +284,12 @@ TEST_F(Rdata_TSIG_Test, createFromParams) {
 
     const uint8_t fake_data[] = { 0x14, 0x02, 0x84, 0x14, 0x02, 0x84,
                                   0x14, 0x02, 0x84, 0x14, 0x02, 0x84 }; 
-    EXPECT_EQ(0, any::TSIG((string(valid_text2))).compare(
+    EXPECT_EQ(0, any::TSIG(valid_text2).compare(
                   any::TSIG(Name("hmac-sha256"), 1286779327, 300, 12,
                             fake_data, 16020, 16, 0, NULL)));
 
     const uint8_t fake_data2[] = { 0x14, 0x02, 0x84, 0x14, 0x02, 0x84 };
-    EXPECT_EQ(0, any::TSIG((string(valid_text3))).compare(
+    EXPECT_EQ(0, any::TSIG(valid_text3).compare(
                   any::TSIG(Name("hmac-sha1"), 1286779327, 300, 12,
                             fake_data, 16020, 18, 6, fake_data2)));
 
@@ -296,13 +311,13 @@ TEST_F(Rdata_TSIG_Test, createFromParams) {
 }
 
 TEST_F(Rdata_TSIG_Test, assignment) {
-    any::TSIG copy((string(valid_text2)));
+    any::TSIG copy(valid_text2);
     copy = rdata_tsig;
     EXPECT_EQ(0, copy.compare(rdata_tsig));
 
     // Check if the copied data is valid even after the original is deleted
     any::TSIG* copy2 = new any::TSIG(rdata_tsig);
-    any::TSIG copy3((string(valid_text2)));
+    any::TSIG copy3(valid_text2);
     copy3 = *copy2;
     delete copy2;
     EXPECT_EQ(0, copy3.compare(rdata_tsig));
@@ -329,7 +344,7 @@ Rdata_TSIG_Test::toWireCommonChecks(Output& output) const {
 
     expect_data.clear();
     output.clear();
-    any::TSIG(string(valid_text2)).toWire(output);
+    any::TSIG(valid_text2).toWire(output);
     UnitTestUtil::readWireData("rdata_tsig_toWire2.wire", expect_data);
     expect_data.erase(expect_data.begin(), expect_data.begin() + 2);
     EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
@@ -338,7 +353,7 @@ Rdata_TSIG_Test::toWireCommonChecks(Output& output) const {
 
     expect_data.clear();
     output.clear();
-    any::TSIG(string(valid_text3)).toWire(output);
+    any::TSIG(valid_text3).toWire(output);
     UnitTestUtil::readWireData("rdata_tsig_toWire3.wire", expect_data);
     expect_data.erase(expect_data.begin(), expect_data.begin() + 2);
     EXPECT_PRED_FORMAT4(UnitTestUtil::matchWireData,
@@ -377,10 +392,10 @@ TEST_F(Rdata_TSIG_Test, toWireRenderer) {
 }
 
 TEST_F(Rdata_TSIG_Test, toText) {
-    EXPECT_EQ(string(valid_text1), rdata_tsig.toText());
-    EXPECT_EQ(string(valid_text2), any::TSIG(string(valid_text2)).toText());
-    EXPECT_EQ(string(valid_text3), any::TSIG(string(valid_text3)).toText());
-    EXPECT_EQ(string(valid_text5), any::TSIG(string(valid_text5)).toText());
+    EXPECT_EQ(valid_text1, rdata_tsig.toText());
+    EXPECT_EQ(valid_text2, any::TSIG(valid_text2).toText());
+    EXPECT_EQ(valid_text3, any::TSIG(valid_text3).toText());
+    EXPECT_EQ(valid_text5, any::TSIG(valid_text5).toText());
 }
 
 TEST_F(Rdata_TSIG_Test, compare) {



More information about the bind10-changes mailing list