[trac812] reject NULL or len=0 data

3 files changed, 19 insertions, 0 deletions

diff --git a/src/lib/dns/tests/tsig_unittest.cc b/src/lib/dns/tests/tsig_unittest.cc
index 5b75f6b0c6..669e38f425 100644
--- a/src/lib/dns/tests/tsig_unittest.cc
+++ b/src/lib/dns/tests/tsig_unittest.cc
@@ -38,6 +38,7 @@
 #include <dns/tests/unittest_util.h>
 
 using namespace std;
+using namespace isc;
 using namespace isc::dns;
 using namespace isc::util;
 using namespace isc::util::encode;
@@ -271,6 +272,13 @@ TEST_F(TSIGTest, signAtActualTime) {
     }
 }
 
+TEST_F(TSIGTest, signBadData) {
+    // some specific bad data should be rejected proactively.
+    const unsigned char dummy_data = 0;
+    EXPECT_THROW(tsig_ctx->sign(0, NULL, 10), InvalidParameter);
+    EXPECT_THROW(tsig_ctx->sign(0, &dummy_data, 0), InvalidParameter);
+}
+
 // Same test as "sign" but use a different algorithm just to confirm we don't
 // naively hardcode constants specific to a particular algorithm.
 // Test data generated by
diff --git a/src/lib/dns/tsig.cc b/src/lib/dns/tsig.cc
index 1ba1e80544..3a51f03a7c 100644
--- a/src/lib/dns/tsig.cc
+++ b/src/lib/dns/tsig.cc
@@ -108,6 +108,10 @@ ConstTSIGRecordPtr
 TSIGContext::sign(const uint16_t qid, const void* const data,
                   const size_t data_len)
 {
+    if (data == NULL || data_len == 0) {
+        isc_throw(InvalidParameter, "TSIG sign error: empty data is given");
+    }
+
     TSIGError error(TSIGError::NOERROR());
     const uint64_t now = (gettimeofdayWrapper() & 0x0000ffffffffffffULL);
 
diff --git a/src/lib/dns/tsig.h b/src/lib/dns/tsig.h
index 7bc6d7ea42..f743d472bc 100644
--- a/src/lib/dns/tsig.h
+++ b/src/lib/dns/tsig.h
@@ -224,6 +224,12 @@ public:
     /// (RR), even though this value should be stored in the first two octets
     /// (in wire format) of the given data.
     ///
+    /// \note This method still checks and rejects empty data (\c NULL pointer
+    /// data or the specified data length is 0) in order to avoid catastrophic
+    /// effect such as program crash.  Empty data is not necessarily invalid
+    /// for HMAC computation, but obviously it doesn't make sense for a DNS
+    /// message.
+    ///
     /// This method can throw exceptions (see the list), but does not provide
     /// the strong exception guarantee.  That is, if an exception is thrown,
     /// the internal state of the \c TSIGContext object can be changed, in
@@ -232,6 +238,7 @@ public:
     /// to catch the exception and try to recover from it, it must drop the
     /// TSIG session and start a new session with a new context.
     ///
+    /// \exception InvalidParameter \c data is NULL or \c data_len is 0
     /// \exception cryptolink::LibraryError Some unexpected error in the
     /// underlying crypto operation
     /// \exception std::bad_alloc Temporary resource allocation failure