security: provide getKeyLocatorName helper function

refs #3920

Change-Id: Iba8fa7776ca63445608f4eb6fa2e4c54307cc99f
diff --git a/src/security/v2/validation-policy-command-interest.cpp b/src/security/v2/validation-policy-command-interest.cpp
index 5753ef3..d765e44 100644
--- a/src/security/v2/validation-policy-command-interest.cpp
+++ b/src/security/v2/validation-policy-command-interest.cpp
@@ -95,30 +95,12 @@
     return std::make_tuple(false, Name(), 0);
   }
 
-  SignatureInfo sig;
-  try {
-    sig.wireDecode(name[signed_interest::POS_SIG_INFO].blockFromValue());
-  }
-  catch (const tlv::Error&) {
-    state->fail({ValidationError::POLICY_ERROR, "Command interest `" +
-                 interest.getName().toUri() + "` does not include SignatureInfo component"});
+  Name klName = getKeyLocatorName(interest, *state);
+  if (!state->getOutcome()) { // already failed
     return std::make_tuple(false, Name(), 0);
   }
 
-  if (!sig.hasKeyLocator()) {
-    state->fail({ValidationError::INVALID_KEY_LOCATOR, "Command interest `" +
-                 interest.getName().toUri() + "` does not include KeyLocator"});
-    return std::make_tuple(false, Name(), 0);
-  }
-
-  const KeyLocator& keyLocator = sig.getKeyLocator();
-  if (keyLocator.getType() != KeyLocator::KeyLocator_Name) {
-    state->fail({ValidationError::INVALID_KEY_LOCATOR, "Command interest `" +
-                 interest.getName().toUri() + "` KeyLocator type is not Name"});
-    return std::make_tuple(false, Name(), 0);
-  }
-
-  return std::make_tuple(true, keyLocator.getName(), timestampComp.toNumber());
+  return std::make_tuple(true, klName, timestampComp.toNumber());
 }
 
 bool
diff --git a/src/security/v2/validation-policy-simple-hierarchy.cpp b/src/security/v2/validation-policy-simple-hierarchy.cpp
index ee1ad94..0ad6128 100644
--- a/src/security/v2/validation-policy-simple-hierarchy.cpp
+++ b/src/security/v2/validation-policy-simple-hierarchy.cpp
@@ -29,19 +29,17 @@
 ValidationPolicySimpleHierarchy::checkPolicy(const Data& data, const shared_ptr<ValidationState>& state,
                                              const ValidationContinuation& continueValidation)
 {
-  if (!data.getSignature().hasKeyLocator()) {
-    return state->fail({ValidationError::Code::INVALID_KEY_LOCATOR, "Required key locator is missing"});
+  Name klName = getKeyLocatorName(data, *state);
+  if (!state->getOutcome()) { // already failed
+    return;
   }
-  const KeyLocator& locator = data.getSignature().getKeyLocator();
-  if (locator.getType() != KeyLocator::KeyLocator_Name) {
-    return state->fail({ValidationError::Code::INVALID_KEY_LOCATOR, "Key locator not Name"});
-  }
-  if (locator.getName().getPrefix(-2).isPrefixOf(data.getName())) {
-    continueValidation(make_shared<CertificateRequest>(Interest(locator.getName())), state);
+
+  if (klName.getPrefix(-2).isPrefixOf(data.getName())) {
+    continueValidation(make_shared<CertificateRequest>(Interest(klName)), state);
   }
   else {
     state->fail({ValidationError::Code::INVALID_KEY_LOCATOR, "Data signing policy violation for " +
-                 data.getName().toUri() + " by " + locator.getName().toUri()});
+                 data.getName().toUri() + " by " + klName.toUri()});
   }
 }
 
@@ -49,27 +47,17 @@
 ValidationPolicySimpleHierarchy::checkPolicy(const Interest& interest, const shared_ptr<ValidationState>& state,
                                              const ValidationContinuation& continueValidation)
 {
-  SignatureInfo info;
-  try {
-    info.wireDecode(interest.getName().at(signed_interest::POS_SIG_INFO).blockFromValue());
+  Name klName = getKeyLocatorName(interest, *state);
+  if (!state->getOutcome()) { // already failed
+    return;
   }
-  catch (const tlv::Error& e) {
-    return state->fail({ValidationError::Code::INVALID_KEY_LOCATOR, "Invalid signed interest (" +
-                        std::string(e.what()) + ")"});
-  }
-  if (!info.hasKeyLocator()) {
-    return state->fail({ValidationError::Code::INVALID_KEY_LOCATOR, "Required key locator is missing"});
-  }
-  const KeyLocator& locator = info.getKeyLocator();
-  if (locator.getType() != KeyLocator::KeyLocator_Name) {
-    return state->fail({ValidationError::Code::INVALID_KEY_LOCATOR, "Key locator not Name"});
-  }
-  if (locator.getName().getPrefix(-2).isPrefixOf(interest.getName())) {
-    continueValidation(make_shared<CertificateRequest>(Interest(locator.getName())), state);
+
+  if (klName.getPrefix(-2).isPrefixOf(interest.getName())) {
+    continueValidation(make_shared<CertificateRequest>(Interest(klName)), state);
   }
   else {
     state->fail({ValidationError::Code::INVALID_KEY_LOCATOR, "Interest signing policy violation for " +
-                 interest.getName().toUri() + " by " + locator.getName().toUri()});
+                 interest.getName().toUri() + " by " + klName.toUri()});
   }
 }
 
diff --git a/src/security/v2/validation-policy.cpp b/src/security/v2/validation-policy.cpp
index 6783f80..25fb1d2 100644
--- a/src/security/v2/validation-policy.cpp
+++ b/src/security/v2/validation-policy.cpp
@@ -59,6 +59,52 @@
   }
 }
 
+static Name
+getKeyLocatorName(const SignatureInfo& si, ValidationState& state)
+{
+  if (!si.hasKeyLocator()) {
+    state.fail({ValidationError::Code::INVALID_KEY_LOCATOR, "KeyLocator is missing"});
+    return Name();
+  }
+
+  const KeyLocator& kl = si.getKeyLocator();
+  if (kl.getType() != KeyLocator::KeyLocator_Name) {
+    state.fail({ValidationError::Code::INVALID_KEY_LOCATOR, "KeyLocator type is not Name"});
+    return Name();
+  }
+
+  return kl.getName();
+}
+
+Name
+getKeyLocatorName(const Data& data, ValidationState& state)
+{
+  return getKeyLocatorName(data.getSignature().getSignatureInfo(), state);
+}
+
+Name
+getKeyLocatorName(const Interest& interest, ValidationState& state)
+{
+  const Name& name = interest.getName();
+  if (name.size() < signed_interest::MIN_SIZE) {
+    state.fail({ValidationError::INVALID_KEY_LOCATOR,
+                "Invalid signed Interest: name too short"});
+    return Name();
+  }
+
+  SignatureInfo si;
+  try {
+    si.wireDecode(name.at(signed_interest::POS_SIG_INFO).blockFromValue());
+  }
+  catch (const tlv::Error& e) {
+    state.fail({ValidationError::Code::INVALID_KEY_LOCATOR,
+                "Invalid signed Interest: " + std::string(e.what())});
+    return Name();
+  }
+
+  return getKeyLocatorName(si, state);
+}
+
 } // namespace v2
 } // namespace security
 } // namespace ndn
diff --git a/src/security/v2/validation-policy.hpp b/src/security/v2/validation-policy.hpp
index 66216c8..351ffed 100644
--- a/src/security/v2/validation-policy.hpp
+++ b/src/security/v2/validation-policy.hpp
@@ -138,6 +138,22 @@
   unique_ptr<ValidationPolicy> m_innerPolicy;
 };
 
+/** \brief extract KeyLocator.Name from Data
+ *
+ *  Data must contain a KeyLocator of Name type.
+ *  Otherwise, state.fail is invoked with INVALID_KEY_LOCATOR error.
+ */
+Name
+getKeyLocatorName(const Data& data, ValidationState& state);
+
+/** \brief extract KeyLocator.Name from signed Interest
+ *
+ *  Interest must have SignatureInfo and contain a KeyLocator of Name type.
+ *  Otherwise, state.fail is invoked with INVALID_KEY_LOCATOR error.
+ */
+Name
+getKeyLocatorName(const Interest& interest, ValidationState& state);
+
 } // namespace v2
 } // namespace security
 } // namespace ndn
diff --git a/src/security/v2/validation-state.cpp b/src/security/v2/validation-state.cpp
index 4267c7f..76d0b97 100644
--- a/src/security/v2/validation-state.cpp
+++ b/src/security/v2/validation-state.cpp
@@ -34,14 +34,14 @@
 #define NDN_LOG_TRACE_DEPTH(x) NDN_LOG_TRACE(std::string(this->getDepth() + 1, '>') << " " << x)
 
 ValidationState::ValidationState()
-  : m_hasOutcome(false)
+  : m_outcome(boost::logic::indeterminate)
 {
 }
 
 ValidationState::~ValidationState()
 {
   NDN_LOG_TRACE(__func__);
-  BOOST_ASSERT(m_hasOutcome);
+  BOOST_ASSERT(!boost::logic::indeterminate(m_outcome));
 }
 
 size_t
@@ -98,7 +98,7 @@
 
 DataValidationState::~DataValidationState()
 {
-  if (!m_hasOutcome) {
+  if (boost::logic::indeterminate(m_outcome)) {
     this->fail({ValidationError::Code::IMPLEMENTATION_ERROR,
                 "Validator/policy did not invoke success or failure callback"});
   }
@@ -110,8 +110,8 @@
   if (verifySignature(m_data, trustedCert)) {
     NDN_LOG_TRACE_DEPTH("OK signature for data `" << m_data.getName() << "`");
     m_successCb(m_data);
-    BOOST_ASSERT(!m_hasOutcome);
-    m_hasOutcome = true;
+    BOOST_ASSERT(boost::logic::indeterminate(m_outcome));
+    m_outcome = true;
   }
   else {
     this->fail({ValidationError::Code::INVALID_SIGNATURE, "Invalid signature of data `" +
@@ -124,8 +124,8 @@
 {
   NDN_LOG_TRACE_DEPTH("Signature verification bypassed for data `" << m_data.getName() << "`");
   m_successCb(m_data);
-  BOOST_ASSERT(!m_hasOutcome);
-  m_hasOutcome = true;
+  BOOST_ASSERT(boost::logic::indeterminate(m_outcome));
+  m_outcome = true;
 }
 
 void
@@ -133,8 +133,8 @@
 {
   NDN_LOG_DEBUG_DEPTH(error);
   m_failureCb(m_data, error);
-  BOOST_ASSERT(!m_hasOutcome);
-  m_hasOutcome = true;
+  BOOST_ASSERT(boost::logic::indeterminate(m_outcome));
+  m_outcome = false;
 }
 
 const Data&
@@ -158,7 +158,7 @@
 
 InterestValidationState::~InterestValidationState()
 {
-  if (!m_hasOutcome) {
+  if (boost::logic::indeterminate(m_outcome)) {
     this->fail({ValidationError::Code::IMPLEMENTATION_ERROR,
                 "Validator/policy did not invoke success or failure callback"});
   }
@@ -170,8 +170,8 @@
   if (verifySignature(m_interest, trustedCert)) {
     NDN_LOG_TRACE_DEPTH("OK signature for interest `" << m_interest.getName() << "`");
     this->afterSuccess(m_interest);
-    BOOST_ASSERT(!m_hasOutcome);
-    m_hasOutcome = true;
+    BOOST_ASSERT(boost::logic::indeterminate(m_outcome));
+    m_outcome = true;
   }
   else {
     this->fail({ValidationError::Code::INVALID_SIGNATURE, "Invalid signature of interest `" +
@@ -184,8 +184,8 @@
 {
   NDN_LOG_TRACE_DEPTH("Signature verification bypassed for interest `" << m_interest.getName() << "`");
   this->afterSuccess(m_interest);
-  BOOST_ASSERT(!m_hasOutcome);
-  m_hasOutcome = true;
+  BOOST_ASSERT(boost::logic::indeterminate(m_outcome));
+  m_outcome = true;
 }
 
 void
@@ -193,8 +193,8 @@
 {
   NDN_LOG_DEBUG_DEPTH(error);
   m_failureCb(m_interest, error);
-  BOOST_ASSERT(!m_hasOutcome);
-  m_hasOutcome = true;
+  BOOST_ASSERT(boost::logic::indeterminate(m_outcome));
+  m_outcome = false;
 }
 
 const Interest&
diff --git a/src/security/v2/validation-state.hpp b/src/security/v2/validation-state.hpp
index 821b85c..8d0e5ba 100644
--- a/src/security/v2/validation-state.hpp
+++ b/src/security/v2/validation-state.hpp
@@ -27,8 +27,9 @@
 #include "certificate.hpp"
 #include "../../util/signal.hpp"
 
-#include <unordered_set>
 #include <list>
+#include <unordered_set>
+#include <boost/logic/tribool.hpp>
 
 namespace ndn {
 namespace security {
@@ -65,6 +66,12 @@
   virtual
   ~ValidationState();
 
+  boost::logic::tribool
+  getOutcome() const
+  {
+    return m_outcome;
+  }
+
   /**
    * @brief Call the failure callback
    */
@@ -129,7 +136,7 @@
   verifyCertificateChain(const Certificate& trustedCert);
 
 protected:
-  bool m_hasOutcome;
+  boost::logic::tribool m_outcome;
 
 private:
   std::unordered_set<Name> m_seenCertificateNames;