[trac893] temporary change for short signatures: modify cryptolink to reject

the case of output_len != sig_len.  Until we implement RFC4635 truncation
consideration won't work correctly (and could dangerously accept very
short sigs).  Since the truncation support is not so urgent, it would be
better to defer to it a seprate later task.

src/lib/cryptolink/crypto_hmac.cc

@@ -144,8 +144,17 @@ public:
         // Botan's verify_mac checks if len matches the output_length,
         // which causes it to fail for truncated signatures, so we do
         // the check ourselves
+        // SEE BELOW FOR TEMPORARY CHANGE
         try {
             Botan::SecureVector<Botan::byte> our_mac = hmac_->final();
+            if (len < getOutputLength()) {
+                // Currently we don't support truncated signature.  To avoid
+                // validating too short signature accidently, we enforce the
+                // standard signature size for the moment.
+                // Once we support truncation correctly, this if-clause should
+                // (and the capitalized comment above) be removed.
+                len = getOutputLength();
+            }
             if (len == 0 || len > getOutputLength()) {
                 len = getOutputLength();
             }

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

@@ -43,6 +43,7 @@ BUILT_SOURCES += tsigrecord_toWire1.wire tsigrecord_toWire2.wire
 BUILT_SOURCES += tsig_verify1.wire tsig_verify2.wire tsig_verify3.wire
 BUILT_SOURCES += tsig_verify4.wire tsig_verify5.wire tsig_verify6.wire
 BUILT_SOURCES += tsig_verify7.wire tsig_verify8.wire tsig_verify9.wire
+BUILT_SOURCES += tsig_verify10.wire
 
 # NOTE: keep this in sync with real file listing
 # so is included in tarball
@@ -114,6 +115,7 @@ EXTRA_DIST += tsigrecord_toWire1.spec tsigrecord_toWire2.spec
 EXTRA_DIST += tsig_verify1.spec tsig_verify2.spec tsig_verify3.spec
 EXTRA_DIST += tsig_verify4.spec tsig_verify5.spec tsig_verify6.spec
 EXTRA_DIST += tsig_verify7.spec tsig_verify8.spec tsig_verify9.spec
+EXTRA_DIST += tsig_verify10.spec
 
 .spec.wire:
 	./gen-wiredata.py -o $@ $<

src/lib/dns/tests/tsig_unittest.cc

@@ -889,4 +889,19 @@ TEST_F(TSIGTest, signAterVerified) {
                  TSIGContextError);
 }
 
+TEST_F(TSIGTest, tooShortMAC) {
+    // Too short MAC should be rejected.
+    // Note: when we implement RFC4635-based checks, the error code will
+    // (probably) be FORMERR.
+
+    isc::util::detail::gettimeFunction = testGetTime<0x4da8877a>;
+    createMessageFromFile("tsig_verify10.wire");
+    {
+        SCOPED_TRACE("Verify test for request");
+        commonVerifyChecks(*tsig_verify_ctx, message.getTSIGRecord(),
+                           &received_data[0], received_data.size(),
+                           TSIGError::BAD_SIG(), TSIGContext::RECEIVED_REQUEST);
+    }
+}
+
 } // end namespace

src/lib/dns/tsig.cc

@@ -360,6 +360,7 @@ TSIGContext::verify(const TSIGRecord* const record, const void* const data,
     }
 
     // TODO: signature length check based on RFC4635
+    // (Right now we enforce the standard signature length in libcryptolink)
 
     // Handling empty MAC.  While RFC2845 doesn't explicitly prohibit other
     // cases, it can only reasonably happen in a response with BADSIG or