signature: disallow encoding invalid SignatureInfo

Also, check integer range when decoding SignatureInfo.

refs #3200

Change-Id: I1af4833211c9468ac7ecab48f7d0e88e9423b378

src/signature-info.cpp

@@ -60,6 +60,10 @@ template<encoding::Tag TAG>
 size_t
 SignatureInfo::wireEncode(EncodingImpl<TAG>& encoder) const
 {
+  if (m_type == -1) {
+    BOOST_THROW_EXCEPTION(Error("Cannot encode invalid SignatureInfo"));
+  }
+
   // SignatureInfo ::= SIGNATURE-INFO-TLV TLV-LENGTH
   //                     SignatureType
   //                     KeyLocator?
@@ -114,15 +118,15 @@ SignatureInfo::wireDecode(const Block& wire)
 
   Block::element_const_iterator it = m_wire.elements_begin();
 
-  // the first block must be SignatureType
+  // the first sub-element must be SignatureType
   if (it != m_wire.elements_end() && it->type() == tlv::SignatureType) {
-    m_type = readNonNegativeInteger(*it);
+    m_type = readNonNegativeIntegerAs<int32_t>(*it);
     ++it;
   }
   else
     BOOST_THROW_EXCEPTION(Error("Missing SignatureType in SignatureInfo"));
 
-  // the second block could be KeyLocator
+  // the second sub-element could be KeyLocator
   if (it != m_wire.elements_end() && it->type() == tlv::KeyLocator) {
     m_keyLocator.wireDecode(*it);
     m_hasKeyLocator = true;

src/signature.cpp

@@ -40,6 +40,15 @@ Signature::Signature(const SignatureInfo& info, const Block& value)
 {
 }
 
+tlv::SignatureTypeValue
+Signature::getType() const
+{
+  if (!*this) {
+    BOOST_THROW_EXCEPTION(Error("Signature is invalid"));
+  }
+  return static_cast<tlv::SignatureTypeValue>(m_info.getSignatureType());
+}
+
 void
 Signature::setInfo(const Block& info)
 {
@@ -55,4 +64,10 @@ Signature::setValue(const Block& value)
   m_value = value;
 }
 
+bool
+operator==(const Signature& lhs, const Signature& rhs)
+{
+  return lhs.getSignatureInfo() == rhs.getSignatureInfo() && lhs.getValue() == rhs.getValue();
+}
+
 } // namespace ndn

src/signature.hpp

@@ -109,12 +109,10 @@ public:
 
 public: // SignatureInfo fields
   /** @brief Get SignatureType
+   *  @throw Error signature is invalid
    */
-  uint32_t
-  getType() const
-  {
-    return m_info.getSignatureType();
-  }
+  tlv::SignatureTypeValue
+  getType() const;
 
   /** @brief Check if KeyLocator exists in SignatureInfo
    */
@@ -157,11 +155,8 @@ protected:
   mutable Block m_value;
 };
 
-inline bool
-operator==(const Signature& lhs, const Signature& rhs)
-{
-  return lhs.getInfo() == rhs.getInfo() && lhs.getValue() == rhs.getValue();
-}
+bool
+operator==(const Signature& lhs, const Signature& rhs);
 
 inline bool
 operator!=(const Signature& lhs, const Signature& rhs)

tests/unit-tests/data.t.cpp

@@ -310,7 +310,7 @@ BOOST_AUTO_TEST_CASE(Print)
                     "Name: /local/ndn/prefix\n"
                     "MetaInfo: ContentType: 0, FreshnessPeriod: 10000 milliseconds\n"
                     "Content: (size: 8)\n"
-                    "Signature: (type: 1, value_length: 128)\n");
+                    "Signature: (type: SignatureSha256WithRsa, value_length: 128)\n");
 }
 
 BOOST_AUTO_TEST_SUITE_END() // TestData

tests/unit-tests/security/v2/validation-policy-command-interest.t.cpp

@@ -130,7 +130,7 @@ BOOST_AUTO_TEST_CASE(BadSigInfo)
 BOOST_AUTO_TEST_CASE(MissingKeyLocator)
 {
   auto i1 = makeCommandInterest(identity);
-  SignatureInfo sigInfo;
+  SignatureInfo sigInfo(tlv::SignatureSha256WithRsa);
   setNameComponent(i1, command_interest::POS_SIG_INFO,
                    sigInfo.wireEncode().begin(), sigInfo.wireEncode().end());
   VALIDATE_FAILURE(i1, "Should fail (missing KeyLocator)");
@@ -141,7 +141,7 @@ BOOST_AUTO_TEST_CASE(BadKeyLocatorType)
   auto i1 = makeCommandInterest(identity);
   KeyLocator kl;
   kl.setKeyDigest(makeBinaryBlock(tlv::KeyDigest, "\xDD\xDD\xDD\xDD\xDD\xDD\xDD\xDD", 8));
-  SignatureInfo sigInfo;
+  SignatureInfo sigInfo(tlv::SignatureSha256WithRsa);
   sigInfo.setKeyLocator(kl);
   setNameComponent(i1, command_interest::POS_SIG_INFO,
                    sigInfo.wireEncode().begin(), sigInfo.wireEncode().end());
@@ -153,7 +153,7 @@ BOOST_AUTO_TEST_CASE(BadCertName)
   auto i1 = makeCommandInterest(identity);
   KeyLocator kl;
   kl.setName("/bad/cert/name");
-  SignatureInfo sigInfo;
+  SignatureInfo sigInfo(tlv::SignatureSha256WithRsa);
   sigInfo.setKeyLocator(kl);
   setNameComponent(i1, command_interest::POS_SIG_INFO,
                    sigInfo.wireEncode().begin(), sigInfo.wireEncode().end());

tests/unit-tests/signature-info.t.cpp

@@ -1,5 +1,5 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
-/**
+/*
  * Copyright (c) 2013-2017 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
@@ -231,26 +231,23 @@ BOOST_AUTO_TEST_CASE(OtherTlvs)
 
 BOOST_AUTO_TEST_CASE(OtherTlvsEncoding) // Bug #3914
 {
-  SignatureInfo info1;
+  SignatureInfo info1(tlv::SignatureSha256WithRsa);
   info1.appendTypeSpecificTlv(makeStringBlock(101, "First"));
   info1.appendTypeSpecificTlv(makeStringBlock(102, "Second"));
   info1.appendTypeSpecificTlv(makeStringBlock(103, "Third"));
 
-  BOOST_CHECK_EQUAL(boost::lexical_cast<std::string>(info1), "Unknown Signature Type { 101 102 103 }");
+  BOOST_CHECK_EQUAL(boost::lexical_cast<std::string>(info1), "SignatureSha256WithRsa { 101 102 103 }");
 
   SignatureInfo info2;
   info2.wireDecode(info1.wireEncode());
   BOOST_CHECK_EQUAL(info1, info2);
 
-  // // These octets are obtained by the snippet below.
-  // // This check is intended to detect unexpected encoding change in the future.
-  // for (uint8_t ch : info1.wireEncode()) {
-  //  printf("0x%02x, ", ch);
-  // }
   const uint8_t infoBytes[] = {
-    0x16, 0x20, 0x1b, 0x08, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x65, 0x05, 0x46, 0x69,
-    0x72, 0x73, 0x74, 0x66, 0x06, 0x53, 0x65, 0x63, 0x6f, 0x6e, 0x64, 0x67, 0x05, 0x54, 0x68, 0x69,
-    0x72, 0x64
+    0x16, 0x19, // SignatureInfo
+          0x1b, 0x01, 0x01, // SignatureType=1
+          0x65, 0x05, 0x46, 0x69, 0x72, 0x73, 0x74, // 101 "First"
+          0x66, 0x06, 0x53, 0x65, 0x63, 0x6f, 0x6e, 0x64, // 102 "Second"
+          0x67, 0x05, 0x54, 0x68, 0x69, 0x72, 0x64 // 103 "Third"
   };
 
   SignatureInfo info3(Block(infoBytes, sizeof(infoBytes)));