BIND 10 trac2522, updated. fcca4e8c09253602530fe66a04e8e9b71bbf4739 [2522] TSIG review updates
BIND 10 source code commits
bind10-changes at lists.isc.org
Fri May 17 19:40:31 UTC 2013
The branch, trac2522 has been updated
via fcca4e8c09253602530fe66a04e8e9b71bbf4739 (commit)
via 592db047a5a82c5af9e837dc2fc9a9df102af8ed (commit)
from c7e380374b96d23f682e81a8ff159c8e39538103 (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 fcca4e8c09253602530fe66a04e8e9b71bbf4739
Author: Paul Selkirk <pselkirk at isc.org>
Date: Fri May 17 15:40:23 2013 -0400
[2522] TSIG review updates
commit 592db047a5a82c5af9e837dc2fc9a9df102af8ed
Author: Paul Selkirk <pselkirk at isc.org>
Date: Fri May 17 14:05:22 2013 -0400
[2522] more and better SSHFP unit tests
-----------------------------------------------------------------------
Summary of changes:
src/lib/dns/rdata/any_255/tsig_250.cc | 40 +++++++++--------
src/lib/dns/rdata/any_255/tsig_250.h | 2 +-
src/lib/dns/rdata/generic/sshfp_44.cc | 6 +--
src/lib/dns/tests/rdata_sshfp_unittest.cc | 70 ++++++++++++++++++++---------
src/lib/dns/tests/rdata_tsig_unittest.cc | 43 +++++++++---------
5 files changed, 96 insertions(+), 65 deletions(-)
-----------------------------------------------------------------------
diff --git a/src/lib/dns/rdata/any_255/tsig_250.cc b/src/lib/dns/rdata/any_255/tsig_250.cc
index 5dead49..ddcdab9 100644
--- a/src/lib/dns/rdata/any_255/tsig_250.cc
+++ b/src/lib/dns/rdata/any_255/tsig_250.cc
@@ -19,7 +19,6 @@
#include <boost/lexical_cast.hpp>
#include <util/buffer.h>
-#include <util/strutil.h>
#include <util/encode/base64.h>
#include <dns/messagerenderer.h>
@@ -34,7 +33,6 @@ using namespace std;
using boost::lexical_cast;
using namespace isc::util;
using namespace isc::util::encode;
-using namespace isc::util::str;
using namespace isc::dns;
using isc::dns::rdata::generic::detail::createNameFromLexer;
@@ -74,18 +72,18 @@ struct TSIGImpl {
// helper function for string and lexer constructors
TSIGImpl*
-TSIG::constructFromLexer(MasterLexer& lexer) {
+TSIG::constructFromLexer(MasterLexer& lexer, const Name* origin) {
// RFC2845 defines Algorithm Name to be "in domain name syntax",
// but it's not actually a domain name, so we allow it to be not
// fully qualified.
- const Name root(".");
- const Name& algorithm = createNameFromLexer(lexer, &root);
+ const Name& algorithm =
+ createNameFromLexer(lexer, origin ? origin : &Name::ROOT_NAME());
- const string time_str =
+ const string& time_txt =
lexer.getNextToken(MasterToken::STRING).getString();
uint64_t time_signed;
try {
- time_signed = boost::lexical_cast<uint64_t>(time_str);
+ time_signed = boost::lexical_cast<uint64_t>(time_txt);
} catch (const boost::bad_lexical_cast&) {
isc_throw(InvalidRdataText, "Invalid TSIG Time");
}
@@ -103,7 +101,7 @@ TSIG::constructFromLexer(MasterLexer& lexer) {
isc_throw(InvalidRdataText, "TSIG MAC Size out of range");
}
- const string mac_txt = (macsize > 0) ?
+ const string& mac_txt = (macsize > 0) ?
lexer.getNextToken(MasterToken::STRING).getString() : "";
vector<uint8_t> mac;
decodeBase64(mac_txt, mac);
@@ -117,9 +115,9 @@ TSIG::constructFromLexer(MasterLexer& lexer) {
isc_throw(InvalidRdataText, "TSIG Original ID out of range");
}
- const string error_txt =
+ const string& error_txt =
lexer.getNextToken(MasterToken::STRING).getString();
- uint16_t error = 0;
+ uint32_t error = 0;
// XXX: In the initial implementation we hardcode the mnemonics.
// We'll soon generalize this.
if (error_txt == "NOERROR") {
@@ -132,13 +130,16 @@ TSIG::constructFromLexer(MasterLexer& lexer) {
error = TSIGError::BAD_TIME_CODE;
} else {
try {
- error = boost::lexical_cast<uint16_t>(error_txt);
+ error = boost::lexical_cast<uint32_t>(error_txt);
} catch (const boost::bad_lexical_cast&) {
isc_throw(InvalidRdataText, "Invalid TSIG Error");
}
+ if (error > 0xffff) {
+ isc_throw(InvalidRdataText, "TSIG Error out of range");
+ }
}
- const int32_t otherlen =
+ const uint32_t otherlen =
lexer.getNextToken(MasterToken::NUMBER).getNumber();
if (otherlen > 0xffff) {
isc_throw(InvalidRdataText, "TSIG Other Len out of range");
@@ -151,7 +152,8 @@ TSIG::constructFromLexer(MasterLexer& lexer) {
isc_throw(InvalidRdataText,
"TSIG Other Data length does not match Other Len");
}
- // also verify Error == BADTIME?
+ // RFC2845 says Other Data is "empty unless Error == BADTIME".
+ // However, we don't enforce that.
return (new TSIGImpl(algorithm, time_signed, fudge, mac, orig_id,
error, other_data));
@@ -212,7 +214,7 @@ TSIG::TSIG(const std::string& tsig_str) : impl_(NULL) {
MasterLexer lexer;
lexer.pushSource(ss);
- impl_ptr.reset(constructFromLexer(lexer));
+ impl_ptr.reset(constructFromLexer(lexer, NULL));
if (lexer.getNextToken().getType() != MasterToken::END_OF_FILE) {
isc_throw(InvalidRdataText,
@@ -242,9 +244,9 @@ TSIG::TSIG(const std::string& tsig_str) : impl_(NULL) {
///
/// \param lexer A \c MasterLexer object parsing a master file for the
/// RDATA to be created
-TSIG::TSIG(MasterLexer& lexer, const Name*,
- MasterLoader::Options, MasterLoaderCallbacks&) :
- impl_(constructFromLexer(lexer))
+TSIG::TSIG(MasterLexer& lexer, const Name* origin,
+ MasterLoader::Options, MasterLoaderCallbacks&) :
+ impl_(constructFromLexer(lexer, origin))
{
}
@@ -268,7 +270,9 @@ TSIG::TSIG(MasterLexer& lexer, const Name*,
/// But this constructor does not use this parameter; if necessary, the caller
/// must check consistency between the length parameter and the actual
/// RDATA length.
-TSIG::TSIG(InputBuffer& buffer, size_t) : impl_(NULL) {
+TSIG::TSIG(InputBuffer& buffer, size_t) :
+ impl_(NULL)
+{
Name algorithm(buffer);
uint8_t time_signed_buf[6];
diff --git a/src/lib/dns/rdata/any_255/tsig_250.h b/src/lib/dns/rdata/any_255/tsig_250.h
index f07295e..62f599a 100644
--- a/src/lib/dns/rdata/any_255/tsig_250.h
+++ b/src/lib/dns/rdata/any_255/tsig_250.h
@@ -142,7 +142,7 @@ public:
/// This method never throws an exception.
const void* getOtherData() const;
private:
- TSIGImpl* constructFromLexer(isc::dns::MasterLexer& lexer);
+ TSIGImpl* constructFromLexer(MasterLexer& lexer, const Name* origin);
TSIGImpl* impl_;
};
diff --git a/src/lib/dns/rdata/generic/sshfp_44.cc b/src/lib/dns/rdata/generic/sshfp_44.cc
index 2843f10..8e11249 100644
--- a/src/lib/dns/rdata/generic/sshfp_44.cc
+++ b/src/lib/dns/rdata/generic/sshfp_44.cc
@@ -135,9 +135,8 @@ SSHFP::SSHFP(const string& sshfp_str) :
/// RDATA to be created
SSHFP::SSHFP(MasterLexer& lexer, const Name*,
MasterLoader::Options, MasterLoaderCallbacks&) :
- impl_(NULL)
+ impl_(constructFromLexer(lexer))
{
- impl_ = constructFromLexer(lexer);
}
/// \brief Constructor from InputBuffer.
@@ -221,9 +220,6 @@ int
SSHFP::compare(const Rdata& other) const {
const SSHFP& other_sshfp = dynamic_cast<const SSHFP&>(other);
- /* This doesn't really make any sort of sense, but in the name of
- consistency... */
-
if (impl_->algorithm_ < other_sshfp.impl_->algorithm_) {
return (-1);
} else if (impl_->algorithm_ > other_sshfp.impl_->algorithm_) {
diff --git a/src/lib/dns/tests/rdata_sshfp_unittest.cc b/src/lib/dns/tests/rdata_sshfp_unittest.cc
index 0b32ded..cb1630c 100644
--- a/src/lib/dns/tests/rdata_sshfp_unittest.cc
+++ b/src/lib/dns/tests/rdata_sshfp_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (C) 2012 Internet Systems Consortium, Inc. ("ISC")
+// Copyright (C) 2012-2013 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
@@ -30,17 +30,45 @@
using isc::UnitTestUtil;
using namespace std;
+using namespace isc;
using namespace isc::dns;
using namespace isc::util;
using namespace isc::dns::rdata;
namespace {
class Rdata_SSHFP_Test : public RdataTest {
- // there's nothing to specialize
+protected:
+ Rdata_SSHFP_Test() :
+ sshfp_txt("2 1 123456789abcdef67890123456789abcdef67890"),
+ rdata_sshfp(sshfp_txt)
+ {}
+
+ void checkFromText_None(const string& rdata_str) {
+ checkFromText<generic::SSHFP, isc::Exception, isc::Exception>(
+ rdata_str, rdata_sshfp, false, false);
+ }
+
+ void checkFromText_LexerError(const string& rdata_str) {
+ checkFromText
+ <generic::SSHFP, InvalidRdataText, MasterLexer::LexerError>(
+ rdata_str, rdata_sshfp, true, true);
+ }
+
+ void checkFromText_BadValue(const string& rdata_str) {
+ checkFromText<generic::SSHFP, InvalidRdataText, BadValue>(
+ rdata_str, rdata_sshfp, true, true);
+ }
+
+ void checkFromText_BadString(const string& rdata_str) {
+ checkFromText
+ <generic::SSHFP, InvalidRdataText, isc::Exception>(
+ rdata_str, rdata_sshfp, true, false);
+ }
+
+ const string sshfp_txt;
+ const generic::SSHFP rdata_sshfp;
};
-const string sshfp_txt("2 1 123456789abcdef67890123456789abcdef67890");
-const generic::SSHFP rdata_sshfp(2, 1, "123456789abcdef67890123456789abcdef67890");
const uint8_t rdata_sshfp_wiredata[] = {
// algorithm
0x02,
@@ -56,22 +84,13 @@ const uint8_t rdata_sshfp_wiredata[] = {
TEST_F(Rdata_SSHFP_Test, createFromText) {
// Basic test
- const generic::SSHFP rdata_sshfp2(sshfp_txt);
- EXPECT_EQ(0, rdata_sshfp2.compare(rdata_sshfp));
+ checkFromText_None(sshfp_txt);
// With different spacing
- const generic::SSHFP rdata_sshfp3("2 1 123456789abcdef67890123456789abcdef67890");
- EXPECT_EQ(0, rdata_sshfp3.compare(rdata_sshfp));
+ checkFromText_None("2 1 123456789abcdef67890123456789abcdef67890");
// Combination of lowercase and uppercase
- const generic::SSHFP rdata_sshfp4("2 1 123456789ABCDEF67890123456789abcdef67890");
- EXPECT_EQ(0, rdata_sshfp4.compare(rdata_sshfp));
-}
-
-TEST_F(Rdata_SSHFP_Test, createFromLexer) {
- EXPECT_EQ(0, rdata_sshfp.compare(
- *test::createRdataUsingLexer(RRType::SSHFP(), RRClass::IN(),
- sshfp_txt)));
+ checkFromText_None("2 1 123456789ABCDEF67890123456789abcdef67890");
}
TEST_F(Rdata_SSHFP_Test, algorithmTypes) {
@@ -101,10 +120,11 @@ TEST_F(Rdata_SSHFP_Test, algorithmTypes) {
}
TEST_F(Rdata_SSHFP_Test, badText) {
- EXPECT_THROW(const generic::SSHFP rdata_sshfp("1"), InvalidRdataText);
- EXPECT_THROW(const generic::SSHFP rdata_sshfp("BUCKLE MY SHOES"), InvalidRdataText);
- EXPECT_THROW(const generic::SSHFP rdata_sshfp("1 2 foo"), InvalidRdataText);
- EXPECT_THROW(const generic::SSHFP rdata_sshfp("1 2 12ab bar"), InvalidRdataText);
+ checkFromText_LexerError("1");
+ checkFromText_LexerError("ONE 2 123456789abcdef67890123456789abcdef67890");
+ checkFromText_LexerError("1 TWO 123456789abcdef67890123456789abcdef67890");
+ checkFromText_BadValue("1 2 BUCKLEMYSHOES");
+ checkFromText_BadString(sshfp_txt + " extra text");
}
TEST_F(Rdata_SSHFP_Test, copy) {
@@ -161,6 +181,16 @@ TEST_F(Rdata_SSHFP_Test, createFromWire) {
InvalidBufferPosition);
}
+TEST_F(Rdata_SSHFP_Test, createFromComponents) {
+ const generic::SSHFP rdata_sshfp2(2, 1, "123456789abcdef67890123456789abcdef67890");
+ EXPECT_EQ(0, rdata_sshfp2.compare(rdata_sshfp));
+}
+
+TEST_F(Rdata_SSHFP_Test, createByCopy) {
+ const generic::SSHFP rdata_sshfp2(rdata_sshfp);
+ EXPECT_EQ(0, rdata_sshfp2.compare(rdata_sshfp));
+}
+
TEST_F(Rdata_SSHFP_Test, toText) {
EXPECT_TRUE(boost::iequals(sshfp_txt, rdata_sshfp.toText()));
diff --git a/src/lib/dns/tests/rdata_tsig_unittest.cc b/src/lib/dns/tests/rdata_tsig_unittest.cc
index 7b16730..72529a7 100644
--- a/src/lib/dns/tests/rdata_tsig_unittest.cc
+++ b/src/lib/dns/tests/rdata_tsig_unittest.cc
@@ -38,23 +38,22 @@ using namespace isc::util;
using namespace isc::dns::rdata;
namespace {
-const char* const valid_text1 = "hmac-md5.sig-alg.reg.int. 1286779327 300 "
+const string valid_text1 = "hmac-md5.sig-alg.reg.int. 1286779327 300 "
"0 16020 BADKEY 0";
-const char* const valid_text2 = "hmac-sha256. 1286779327 300 12 "
+const string valid_text2 = "hmac-sha256. 1286779327 300 12 "
"FAKEFAKEFAKEFAKE 16020 BADSIG 0";
-
-const char* const valid_text3 = "hmac-sha1. 1286779327 300 12 "
+const string valid_text3 = "hmac-sha1. 1286779327 300 12 "
"FAKEFAKEFAKEFAKE 16020 BADTIME 6 FAKEFAKE";
-const char* const valid_text4 = "hmac-sha1. 1286779327 300 12 "
+const string valid_text4 = "hmac-sha1. 1286779327 300 12 "
"FAKEFAKEFAKEFAKE 16020 BADSIG 6 FAKEFAKE";
-const char* const valid_text5 = "hmac-sha256. 1286779327 300 12 "
+const string valid_text5 = "hmac-sha256. 1286779327 300 12 "
"FAKEFAKEFAKEFAKE 16020 2845 0"; // using numeric error code
-const char* const too_long_label = "012345678901234567890123456789"
- "0123456789012345678901234567890123";
class Rdata_TSIG_Test : public RdataTest {
protected:
- Rdata_TSIG_Test() : rdata_tsig(valid_text1) {}
+ Rdata_TSIG_Test() :
+ rdata_tsig(valid_text1)
+ {}
void checkFromText_InvalidText(const string& rdata_str) {
checkFromText<any::TSIG, InvalidRdataText, InvalidRdataText>(
@@ -82,6 +81,12 @@ protected:
rdata_str, rdata_tsig, true, true);
}
+ void checkFromText_BadString(const string& rdata_str) {
+ checkFromText
+ <any::TSIG, InvalidRdataText, isc::Exception>(
+ rdata_str, rdata_tsig, true, false);
+ }
+
template <typename Output>
void toWireCommonChecks(Output& output) const;
@@ -118,11 +123,13 @@ TEST_F(Rdata_TSIG_Test, fromText) {
TEST_F(Rdata_TSIG_Test, badText) {
// too many fields
- EXPECT_THROW(any::TSIG("foo 0 0 0 0 BADKEY 0 0"), InvalidRdataText);
+ checkFromText_BadString(valid_text1 + " 0 0");
// not enough fields
checkFromText_LexerError("foo 0 0 0 0 BADKEY");
// bad domain name
- checkFromText_TooLongLabel(string(too_long_label) + "0 0 0 0 BADKEY 0");
+ checkFromText_TooLongLabel(
+ "0123456789012345678901234567890123456789012345678901234567890123"
+ " 0 0 0 0 BADKEY 0");
checkFromText_EmptyLabel("foo..bar 0 0 0 0 BADKEY");
// time is too large (2814...6 is 2^48)
checkFromText_InvalidText("foo 281474976710656 0 0 0 BADKEY 0");
@@ -150,8 +157,12 @@ TEST_F(Rdata_TSIG_Test, badText) {
checkFromText_InvalidText("foo 0 0 0 0 TEST 0");
// Numeric error code is too large
checkFromText_InvalidText("foo 0 0 0 0 65536 0");
+ // Numeric error code is negative
+ checkFromText_InvalidText("foo 0 0 0 0 -1 0");
// Other len is too large
checkFromText_InvalidText("foo 0 0 0 0 NOERROR 65536 FAKE");
+ // Other len is negative
+ checkFromText_LexerError("foo 0 0 0 0 NOERROR -1 FAKE");
// invalid Other len
checkFromText_LexerError("foo 0 0 0 0 NOERROR LEN FAKE");
// Other len and data mismatch
@@ -284,16 +295,6 @@ TEST_F(Rdata_TSIG_Test, createFromParams) {
isc::InvalidParameter);
}
-TEST_F(Rdata_TSIG_Test, createFromLexer) {
- EXPECT_EQ(0, rdata_tsig.compare(
- *test::createRdataUsingLexer(RRType::TSIG(), RRClass::ANY(),
- valid_text1)));
-
- // Exceptions cause NULL to be returned.
- EXPECT_FALSE(test::createRdataUsingLexer(RRType::TSIG(), RRClass::ANY(),
- "foo 0 0 0 0 BADKEY 0 0"));
-}
-
TEST_F(Rdata_TSIG_Test, assignment) {
any::TSIG copy((string(valid_text2)));
copy = rdata_tsig;
More information about the bind10-changes
mailing list