security: fix extraction of signed portion during Data signature verification

refs #4583

Change-Id: Ia0fc93a6e2408bb5f0de16fd76fcb6d9f06a0398
diff --git a/ndn-cxx/data.cpp b/ndn-cxx/data.cpp
index 8106fd3..f5eb8b9 100644
--- a/ndn-cxx/data.cpp
+++ b/ndn-cxx/data.cpp
@@ -326,6 +326,19 @@
   return *this;
 }
 
+InputBuffers
+Data::extractSignedRanges() const
+{
+  InputBuffers bufs;
+  bufs.reserve(1); // One range containing data value up to, but not including, SignatureValue
+
+  wireEncode();
+  auto lastSignedIt = std::prev(m_wire.find(tlv::SignatureValue));
+  bufs.emplace_back(m_wire.value(),
+                    std::distance(m_wire.value_begin(), lastSignedIt->end()));
+  return bufs;
+}
+
 Data&
 Data::setContentType(uint32_t type)
 {
diff --git a/ndn-cxx/data.hpp b/ndn-cxx/data.hpp
index ac25c07..838df88 100644
--- a/ndn-cxx/data.hpp
+++ b/ndn-cxx/data.hpp
@@ -26,6 +26,7 @@
 #include "ndn-cxx/encoding/block.hpp"
 #include "ndn-cxx/meta-info.hpp"
 #include "ndn-cxx/name.hpp"
+#include "ndn-cxx/security/security-common.hpp"
 #include "ndn-cxx/signature-info.hpp"
 
 namespace ndn {
@@ -264,6 +265,13 @@
   Data&
   setSignatureValue(ConstBufferPtr value);
 
+  /** @brief Extract ranges of Data covered by the signature
+   *  @throw Error Data cannot be encoded or is missing ranges necessary for signing
+   *  @warning The returned pointers will be invalidated if wireDecode() or wireEncode() are called.
+   */
+  InputBuffers
+  extractSignedRanges() const;
+
 public: // MetaInfo fields
   uint32_t
   getContentType() const
diff --git a/ndn-cxx/security/verification-helpers.cpp b/ndn-cxx/security/verification-helpers.cpp
index d21fb2a..17df3f9 100644
--- a/ndn-cxx/security/verification-helpers.cpp
+++ b/ndn-cxx/security/verification-helpers.cpp
@@ -110,8 +110,7 @@
 parse(const Data& data)
 {
   try {
-    const Block& wire = data.wireEncode();
-    return ParseResult({{wire.value(), wire.value_size() - data.getSignatureValue().size()}},
+    return ParseResult(data.extractSignedRanges(),
                        data.getSignatureValue().value(),
                        data.getSignatureValue().value_size());
   }
diff --git a/tests/unit/data.t.cpp b/tests/unit/data.t.cpp
index 4bb0f64..69e0e69 100644
--- a/tests/unit/data.t.cpp
+++ b/tests/unit/data.t.cpp
@@ -574,6 +574,36 @@
   BOOST_CHECK_THROW(d.setSignatureValue(nullptr), std::invalid_argument);
 }
 
+BOOST_FIXTURE_TEST_CASE(ExtractSignedRanges, IdentityManagementFixture)
+{
+  Data d1("/test/prefix");
+  m_keyChain.sign(d1);
+  auto ranges1 = d1.extractSignedRanges();
+  BOOST_REQUIRE_EQUAL(ranges1.size(), 1);
+  const Block& wire1 = d1.wireEncode();
+  const auto& sigInfoWire1 = wire1.find(tlv::SignatureInfo);
+  BOOST_REQUIRE(sigInfoWire1 != wire1.elements_end());
+  BOOST_CHECK_EQUAL_COLLECTIONS(ranges1.front().first, ranges1.front().first + ranges1.front().second,
+                                wire1.value_begin(), sigInfoWire1->value_end());
+
+  // Test with decoded Data and ensure excludes elements after SignatureValue
+  const uint8_t WIRE[] = {
+    0x06, 0x0B, // Data
+          0x07, 0x00, // Name
+          0x16, 0x03, // SignatureInfo
+                0x1B, 0x01, // SignatureType
+                      0x00,
+          0x17, 0x00, // SignatureValue
+          0xAA, 0x00 // Unrecognized non-critical element
+  };
+  Block wire2(WIRE, sizeof(WIRE));
+  Data d2(wire2);
+  auto ranges2 = d2.extractSignedRanges();
+  BOOST_REQUIRE_EQUAL(ranges2.size(), 1);
+  BOOST_CHECK_EQUAL_COLLECTIONS(ranges2.front().first, ranges2.front().first + ranges2.front().second,
+                                &WIRE[2], &WIRE[9]);
+}
+
 BOOST_AUTO_TEST_CASE(Equality)
 {
   Data a;
diff --git a/tests/unit/security/verification-helpers.t.cpp b/tests/unit/security/verification-helpers.t.cpp
index e5e551c..f1e7185 100644
--- a/tests/unit/security/verification-helpers.t.cpp
+++ b/tests/unit/security/verification-helpers.t.cpp
@@ -662,6 +662,34 @@
   // - base version of verifyDigest is tested transitively
 }
 
+const uint8_t sha256DataUnrecognizedElements[] = {
+  0x06, 0x47, 0x07, 0x14, 0x08, 0x04, 0x74, 0x65, 0x73, 0x74, 0x08, 0x04, 0x64, 0x61, 0x74, 0x61,
+  0x08, 0x06, 0x53, 0x68, 0x61, 0x32, 0x35, 0x36, 0x14, 0x00, 0x15, 0x00, 0x16, 0x03, 0x1B, 0x01,
+  0x00, 0x17, 0x20, 0xE2, 0xE2, 0x2F, 0x02, 0x70, 0xA7, 0xF7, 0x48, 0x70, 0x45, 0x29, 0x46, 0xBD,
+  0xD2, 0x62, 0x24, 0xA6, 0x1E, 0x1D, 0x75, 0x2A, 0x26, 0x98, 0x04, 0xAD, 0x9C, 0x47, 0x63, 0xF8,
+  0x98, 0x5A, 0x49, 0xAA, 0x04, 0x01, 0x02, 0x03, 0x04
+};
+
+const uint8_t sha256InterestUnrecognizedElements[] = {
+  0x05, 0x71, 0x07, 0x3A, 0x08, 0x04, 0x74, 0x65, 0x73, 0x74, 0x08, 0x08, 0x69, 0x6E, 0x74, 0x65,
+  0x72, 0x65, 0x73, 0x74, 0x08, 0x06, 0x53, 0x68, 0x61, 0x32, 0x35, 0x36, 0x02, 0x20, 0x9A, 0x55,
+  0xC0, 0xB5, 0x36, 0xC6, 0x6C, 0x0E, 0x11, 0x41, 0xAF, 0xA5, 0x7F, 0x0B, 0x45, 0x53, 0x3D, 0xEE,
+  0x40, 0x1B, 0x9B, 0xAF, 0xEA, 0xBF, 0x3D, 0x56, 0x89, 0x12, 0xE0, 0x18, 0x98, 0x26, 0x0A, 0x04,
+  0xF7, 0x2C, 0x8A, 0x4B, 0x24, 0x00, 0x2C, 0x03, 0x1B, 0x01, 0x00, 0x2E, 0x20, 0xEF, 0x45, 0x55,
+  0x75, 0xC8, 0x18, 0x5E, 0xE9, 0x2A, 0xAE, 0x52, 0x61, 0x0A, 0x94, 0x41, 0x03, 0x36, 0x4C, 0x13,
+  0x59, 0xD4, 0xC7, 0xA4, 0x3A, 0xA0, 0x40, 0x61, 0x44, 0x33, 0x93, 0x5E, 0x99, 0xAA, 0x04, 0x01,
+  0x02, 0x03, 0x04
+};
+
+BOOST_AUTO_TEST_CASE(VerifyWithUnrecognizedElements) // Bug #4583
+{
+  Data data(Block(sha256DataUnrecognizedElements, sizeof(sha256DataUnrecognizedElements)));
+  Interest interest(Block(sha256InterestUnrecognizedElements, sizeof(sha256InterestUnrecognizedElements)));
+
+  BOOST_CHECK(verifyDigest(data, DigestAlgorithm::SHA256));
+  BOOST_CHECK(verifyDigest(interest, DigestAlgorithm::SHA256));
+}
+
 BOOST_AUTO_TEST_SUITE_END() // TestVerificationHelpers
 BOOST_AUTO_TEST_SUITE_END() // Security