[2387] Move short keydata check for RSA/MD5 into getTag()

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

@@ -109,12 +109,6 @@ DNSKEY::DNSKEY(InputBuffer& buffer, size_t rdata_len) {
         buffer.readData(&keydata[0], rdata_len);
     }
 
-    // See RFC 4034 appendix B.1 for why the key data has to be at least
-    // 3 bytes long with RSA/MD5.
-    if (algorithm == 1 && keydata.size() < 3) {
-        isc_throw(InvalidRdataLength, "DNSKEY keydata too short");
-    }
-
     impl_ = new DNSKEYImpl(flags, protocol, algorithm, keydata);
 }
 
@@ -189,12 +183,6 @@ DNSKEY::constructFromLexer(MasterLexer& lexer) {
         decodeBase64(keydata_str, keydata);
     }
 
-    // See RFC 4034 appendix B.1 for why the key data has to be at least
-    // 3 bytes long with RSA/MD5.
-    if (algorithm == 1 && keydata.size() < 3) {
-        isc_throw(InvalidRdataLength, "DNSKEY keydata too short");
-    }
-
     impl_ = new DNSKEYImpl(flags, protocol, algorithm, keydata);
 }
 
@@ -273,6 +261,16 @@ uint16_t
 DNSKEY::getTag() const {
     if (impl_->algorithm_ == 1) {
         const int len = impl_->keydata_.size();
+
+        // See RFC 4034 appendix B.1 for why the key data must contain
+        // at least 4 bytes with RSA/MD5: 3 trailing bytes to extract
+        // the tag from, and 1 byte of exponent length subfield before
+        // modulus.
+        if (len < 4) {
+            isc_throw(isc::OutOfRange,
+                      "DNSKEY keydata too short for tag extraction");
+        }
+
         return ((impl_->keydata_[len - 3] << 8) + impl_->keydata_[len - 2]);
     }
 

src/lib/dns/rdata/generic/dnskey_48.h

@@ -43,7 +43,13 @@ public:
     ///
     /// Specialized methods
     ///
+
+    /// \brief Returns the key tag
+    ///
+    /// \throw isc::OutOfRange if the key data for RSA/MD5 is too short
+    /// to support tag extraction.
     uint16_t getTag() const;
+
     uint16_t getFlags() const;
     uint8_t getAlgorithm() const;
 

src/lib/dns/tests/rdata_dnskey_unittest.cc

@@ -102,6 +102,11 @@ TEST_F(Rdata_DNSKEY_Test, fromText) {
     // Missing keydata is OK
     EXPECT_NO_THROW(const generic::DNSKEY rdata_dnskey3("257 3 5"));
 
+    // Key data too short for RSA/MD5 algorithm is OK when
+    // constructing. But getTag() on this object would throw (see
+    // .getTag tests).
+    EXPECT_NO_THROW(const generic::DNSKEY rdata_dnskey4("1 1 1 YQ=="));
+
     // Flags field out of range
     checkFromText_InvalidText("65536 3 5 YmluZDEwLmlzYy5vcmc=");
 
@@ -117,9 +122,6 @@ TEST_F(Rdata_DNSKEY_Test, fromText) {
     // Invalid key data field (not Base64)
     checkFromText_BadValue("257 3 5 BAAAAAAAAAAAD");
 
-    // Key data too short for algorithm=1
-    checkFromText_InvalidLength("1 1 1 YQ==");
-
     // String instead of number
     checkFromText_LexerError("foo 3 5 YmluZDEwLmlzYy5vcmc=");
     checkFromText_LexerError("257 foo 5 YmluZDEwLmlzYy5vcmc=");
@@ -177,16 +179,18 @@ TEST_F(Rdata_DNSKEY_Test, createFromWire) {
     EXPECT_EQ(0, rdata_dnskey_missing_keydata.compare(
         *rdataFactoryFromFile(RRType("DNSKEY"), RRClass("IN"),
                               "rdata_dnskey_empty_keydata_fromWire.wire")));
-
-    // Short keydata for RSA/MD5 should throw
-    EXPECT_THROW(rdataFactoryFromFile
-                 (RRType("DNSKEY"), RRClass("IN"),
-                  "rdata_dnskey_short_keydata1_fromWire.wire"),
-                 InvalidRdataLength);
 }
 
 TEST_F(Rdata_DNSKEY_Test, getTag) {
     EXPECT_EQ(12892, rdata_dnskey.getTag());
+
+    // Short keydata with algorithm RSA/MD5 must throw.
+    const generic::DNSKEY rdata_dnskey_short_keydata1("1 1 1 YQ==");
+    EXPECT_THROW(rdata_dnskey_short_keydata1.getTag(), isc::OutOfRange);
+
+    // Short keydata with algorithm not RSA/MD5 must not throw.
+    const generic::DNSKEY rdata_dnskey_short_keydata2("257 3 5 YQ==");
+    EXPECT_NO_THROW(rdata_dnskey_short_keydata2.getTag());
 }
 
 TEST_F(Rdata_DNSKEY_Test, getAlgorithm) {

src/lib/dns/tests/testdata/.gitignore

@@ -43,7 +43,6 @@
 /rdata_minfo_toWireUncompressed2.wire
 /rdata_dnskey_fromWire.wire
 /rdata_dnskey_empty_keydata_fromWire.wire
-/rdata_dnskey_short_keydata1_fromWire.wire
 /rdata_nsec3_fromWire10.wire
 /rdata_nsec3_fromWire11.wire
 /rdata_nsec3_fromWire12.wire

src/lib/dns/tests/testdata/Makefile.am

@@ -17,7 +17,6 @@ BUILT_SOURCES += name_toWire5.wire name_toWire6.wire
 BUILT_SOURCES += rdatafields1.wire rdatafields2.wire rdatafields3.wire
 BUILT_SOURCES += rdatafields4.wire rdatafields5.wire rdatafields6.wire
 BUILT_SOURCES += rdata_dnskey_fromWire.wire rdata_dnskey_empty_keydata_fromWire.wire
-BUILT_SOURCES += rdata_dnskey_short_keydata1_fromWire.wire
 BUILT_SOURCES += rdata_nsec_fromWire4.wire rdata_nsec_fromWire5.wire
 BUILT_SOURCES += rdata_nsec_fromWire6.wire rdata_nsec_fromWire7.wire
 BUILT_SOURCES += rdata_nsec_fromWire8.wire rdata_nsec_fromWire9.wire
@@ -105,7 +104,6 @@ EXTRA_DIST += rdatafields1.spec rdatafields2.spec rdatafields3.spec
 EXTRA_DIST += rdatafields4.spec rdatafields5.spec rdatafields6.spec
 EXTRA_DIST += rdata_cname_fromWire rdata_dname_fromWire
 EXTRA_DIST += rdata_dnskey_fromWire.spec rdata_dnskey_empty_keydata_fromWire.spec
-EXTRA_DIST += rdata_dnskey_short_keydata1_fromWire.spec
 EXTRA_DIST += rdata_dhcid_fromWire rdata_dhcid_toWire
 EXTRA_DIST += rdata_ds_fromWire rdata_in_a_fromWire rdata_in_aaaa_fromWire
 EXTRA_DIST += rdata_mx_fromWire rdata_mx_toWire1 rdata_mx_toWire2

src/lib/dns/tests/testdata/rdata_dnskey_short_keydata1_fromWire.spec

@@ -1,11 +0,0 @@
-# DNSKEY test data with algorithm 1 (RSA/MD5) and digest less than 3
-# bytes long.
-
-[custom]
-sections: dnskey
-
-[dnskey]
-flags: 257
-protocol: 3
-algorithm: 1
-digest: BEA=