security: various enhancements to Interest and Data validation
* Avoid decoding SignatureInfo multiple times while validating an Interest
* Fix handling of signed Interests with malformed InterestSignatureInfo
* Report a NO_SIGNATURE error when SignatureInfo is missing or malformed
* Fail with POLICY_ERROR in ValidationPolicySimpleHierarchy when the
signing identity violates the policy
* Reduce code duplication
* Expand unit test coverage
Change-Id: I1c9d532b2307d5df8f4bd75152af57a4e10835aa
diff --git a/ndn-cxx/security/validation-policy-command-interest.cpp b/ndn-cxx/security/validation-policy-command-interest.cpp
index 3ee2cb8..171848e 100644
--- a/ndn-cxx/security/validation-policy-command-interest.cpp
+++ b/ndn-cxx/security/validation-policy-command-interest.cpp
@@ -61,7 +61,59 @@
if (!checkTimestamp(state, keyName, timestamp)) {
return;
}
- getInnerPolicy().checkPolicy(interest, state, std::bind(continueValidation, _1, _2));
+
+ getInnerPolicy().checkPolicy(interest, state, continueValidation);
+}
+
+std::tuple<bool, Name, time::system_clock::TimePoint>
+ValidationPolicyCommandInterest::parseCommandInterest(const Interest& interest,
+ const shared_ptr<ValidationState>& state)
+{
+ auto sigInfo = getSignatureInfo(interest, *state);
+ if (!state->getOutcome()) { // already failed
+ return std::make_tuple(false, Name(), time::system_clock::TimePoint{});
+ }
+
+ time::system_clock::TimePoint timestamp;
+
+ auto fmt = state->getTag<SignedInterestFormatTag>();
+ BOOST_ASSERT(fmt);
+ if (*fmt == SignedInterestFormat::V03) {
+ // SignatureTime is a hard requirement of this policy
+ // New apps should use the more flexible ValidationPolicySignedInterest (v0.3 only)
+ auto optionalTimestamp = sigInfo.getTime();
+ if (!optionalTimestamp) {
+ state->fail({ValidationError::POLICY_ERROR, "Signed Interest `" +
+ interest.getName().toUri() + "` lacks required SignatureTime element"});
+ return std::make_tuple(false, Name(), time::system_clock::TimePoint{});
+ }
+
+ timestamp = *optionalTimestamp;
+ }
+ else {
+ const Name& name = interest.getName();
+ if (name.size() < command_interest::MIN_SIZE) {
+ state->fail({ValidationError::POLICY_ERROR,
+ "Command Interest name too short `" + interest.getName().toUri() + "`"});
+ return std::make_tuple(false, Name(), time::system_clock::TimePoint{});
+ }
+
+ const auto& timestampComp = name.at(command_interest::POS_TIMESTAMP);
+ if (!timestampComp.isNumber()) {
+ state->fail({ValidationError::POLICY_ERROR, "Command Interest `" +
+ interest.getName().toUri() + "` lacks required timestamp component"});
+ return std::make_tuple(false, Name(), time::system_clock::TimePoint{});
+ }
+
+ timestamp = time::fromUnixTimestamp(time::milliseconds(timestampComp.toNumber()));
+ }
+
+ Name klName = getKeyLocatorName(sigInfo, *state);
+ if (!state->getOutcome()) { // already failed
+ return std::make_tuple(false, Name(), time::system_clock::TimePoint{});
+ }
+
+ return std::make_tuple(true, klName, timestamp);
}
void
@@ -76,53 +128,6 @@
}
}
-std::tuple<bool, Name, time::system_clock::TimePoint>
-ValidationPolicyCommandInterest::parseCommandInterest(const Interest& interest,
- const shared_ptr<ValidationState>& state) const
-{
- auto fmt = state->getTag<SignedInterestFormatTag>();
- BOOST_ASSERT(fmt);
-
- time::system_clock::TimePoint timestamp;
-
- if (*fmt == SignedInterestFormat::V03) {
- BOOST_ASSERT(interest.getSignatureInfo());
- auto optionalTimestamp = interest.getSignatureInfo()->getTime();
-
- // Note that timestamp is a hard requirement of this policy
- // TODO: Refactor to support other/combinations of the restrictions based on Nonce, Time, and/or SeqNum
- if (!optionalTimestamp) {
- state->fail({ValidationError::POLICY_ERROR, "Signed Interest `" +
- interest.getName().toUri() + "` does not include required SignatureTime element"});
- return std::make_tuple(false, Name(), time::system_clock::TimePoint{});
- }
- timestamp = *optionalTimestamp;
- }
- else {
- const Name& name = interest.getName();
- if (name.size() < command_interest::MIN_SIZE) {
- state->fail({ValidationError::POLICY_ERROR, "Command interest name `" +
- interest.getName().toUri() + "` is too short"});
- return std::make_tuple(false, Name(), time::system_clock::TimePoint{});
- }
-
- const auto& timestampComp = name.at(command_interest::POS_TIMESTAMP);
- if (!timestampComp.isNumber()) {
- state->fail({ValidationError::POLICY_ERROR, "Command interest `" +
- interest.getName().toUri() + "` does not include timestamp component"});
- return std::make_tuple(false, Name(), time::system_clock::TimePoint{});
- }
- timestamp = time::fromUnixTimestamp(time::milliseconds(timestampComp.toNumber()));
- }
-
- Name klName = getKeyLocatorName(interest, *state);
- if (!state->getOutcome()) { // already failed
- return std::make_tuple(false, Name(), time::system_clock::TimePoint{});
- }
-
- return std::make_tuple(true, klName, timestamp);
-}
-
bool
ValidationPolicyCommandInterest::checkTimestamp(const shared_ptr<ValidationState>& state,
const Name& keyName, time::system_clock::TimePoint timestamp)
diff --git a/ndn-cxx/security/validation-policy-command-interest.hpp b/ndn-cxx/security/validation-policy-command-interest.hpp
index 68a7719..d8fe1d0 100644
--- a/ndn-cxx/security/validation-policy-command-interest.hpp
+++ b/ndn-cxx/security/validation-policy-command-interest.hpp
@@ -33,11 +33,13 @@
namespace security {
inline namespace v2 {
-/** \brief Validation policy for stop-and-wait command Interests
- * \sa https://redmine.named-data.net/projects/ndn-cxx/wiki/CommandInterest
+/**
+ * @brief Validation policy for stop-and-wait command Interests.
*
- * This policy checks the timestamp field of a stop-and-wait command Interest.
- * Signed Interest validation and Data validation requests are delegated to an inner policy.
+ * This policy checks the timestamp field of a stop-and-wait command Interest.
+ * Signed Interest validation and Data validation requests are delegated to an inner policy.
+ *
+ * @sa https://redmine.named-data.net/projects/ndn-cxx/wiki/CommandInterest
*/
class ValidationPolicyCommandInterest : public ValidationPolicy
{
@@ -112,12 +114,12 @@
const ValidationContinuation& continueValidation) override;
private:
+ static std::tuple<bool, Name, time::system_clock::TimePoint>
+ parseCommandInterest(const Interest& interest, const shared_ptr<ValidationState>& state);
+
void
cleanup();
- std::tuple<bool, Name, time::system_clock::TimePoint>
- parseCommandInterest(const Interest& interest, const shared_ptr<ValidationState>& state) const;
-
bool
checkTimestamp(const shared_ptr<ValidationState>& state,
const Name& keyName, time::system_clock::TimePoint timestamp);
diff --git a/ndn-cxx/security/validation-policy-config.cpp b/ndn-cxx/security/validation-policy-config.cpp
index 18ed830..45bc867 100644
--- a/ndn-cxx/security/validation-policy-config.cpp
+++ b/ndn-cxx/security/validation-policy-config.cpp
@@ -241,15 +241,16 @@
return continueValidation(nullptr, state);
}
- Name klName = getKeyLocatorName(data, *state);
+ Name klName = getKeyLocatorName(data.getSignatureInfo(), *state);
if (!state->getOutcome()) { // already failed
return;
}
+ auto sigType = tlv::SignatureTypeValue(data.getSignatureType());
+
for (const auto& rule : m_dataRules) {
if (rule->match(tlv::Data, data.getName(), state)) {
- if (rule->check(tlv::Data, tlv::SignatureTypeValue(data.getSignatureType()),
- data.getName(), klName, state)) {
+ if (rule->check(tlv::Data, sigType, data.getName(), klName, state)) {
return continueValidation(make_shared<CertificateRequest>(klName), state);
}
// rule->check calls state->fail(...) if the check fails
@@ -271,39 +272,20 @@
return continueValidation(nullptr, state);
}
- Name klName = getKeyLocatorName(interest, *state);
+ auto sigInfo = getSignatureInfo(interest, *state);
if (!state->getOutcome()) { // already failed
return;
}
+ Name klName = getKeyLocatorName(sigInfo, *state);
+ if (!state->getOutcome()) { // already failed
+ return;
+ }
+
+ auto sigType = tlv::SignatureTypeValue(sigInfo.getSignatureType());
+
for (const auto& rule : m_interestRules) {
if (rule->match(tlv::Interest, interest.getName(), state)) {
-
- tlv::SignatureTypeValue sigType;
- auto fmt = state->getTag<SignedInterestFormatTag>();
- BOOST_ASSERT(fmt);
-
- if (*fmt == SignedInterestFormat::V03) {
- sigType = tlv::SignatureTypeValue(interest.getSignatureInfo()->getSignatureType());
- }
- else {
- if (interest.getName().size() < signed_interest::MIN_SIZE) {
- state->fail({ValidationError::INVALID_KEY_LOCATOR, "Invalid signed Interest: name too short"});
- return;
- }
-
- SignatureInfo si;
- try {
- si.wireDecode(interest.getName()[signed_interest::POS_SIG_INFO].blockFromValue());
- }
- catch (const tlv::Error& e) {
- state->fail({ValidationError::INVALID_KEY_LOCATOR, "Invalid signed Interest: "s + e.what()});
- return;
- }
-
- sigType = tlv::SignatureTypeValue(si.getSignatureType());
- }
-
if (rule->check(tlv::Interest, sigType, interest.getName(), klName, state)) {
return continueValidation(make_shared<CertificateRequest>(klName), state);
}
diff --git a/ndn-cxx/security/validation-policy-signed-interest.cpp b/ndn-cxx/security/validation-policy-signed-interest.cpp
index a439ea7..3ddab05 100644
--- a/ndn-cxx/security/validation-policy-signed-interest.cpp
+++ b/ndn-cxx/security/validation-policy-signed-interest.cpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2013-2020 Regents of the University of California.
+ * Copyright (c) 2013-2022 Regents of the University of California.
*
* This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
*
@@ -52,37 +52,40 @@
const shared_ptr<ValidationState>& state,
const ValidationContinuation& continueValidation)
{
- auto fmt = state->getTag<SignedInterestFormatTag>();
- BOOST_ASSERT(fmt);
-
if (!state->getOutcome()) {
return;
}
+ auto fmt = state->getTag<SignedInterestFormatTag>();
+ BOOST_ASSERT(fmt);
if (*fmt == SignedInterestFormat::V03 && !checkIncomingInterest(state, interest)) {
return;
}
- getInnerPolicy().checkPolicy(interest, state, std::bind(continueValidation, _1, _2));
+ getInnerPolicy().checkPolicy(interest, state, continueValidation);
}
bool
ValidationPolicySignedInterest::checkIncomingInterest(const shared_ptr<ValidationState>& state,
const Interest& interest)
{
- // Extract information from Interest
- BOOST_ASSERT(interest.getSignatureInfo());
- Name keyName = getKeyLocatorName(interest, *state);
- auto timestamp = interest.getSignatureInfo()->getTime();
- auto seqNum = interest.getSignatureInfo()->getSeqNum();
- auto nonce = interest.getSignatureInfo()->getNonce();
+ auto sigInfo = getSignatureInfo(interest, *state);
+ if (!state->getOutcome()) {
+ return false;
+ }
+ Name keyName = getKeyLocatorName(sigInfo, *state);
+ if (!state->getOutcome()) {
+ return false;
+ }
+ auto timestamp = sigInfo.getTime();
+ auto seqNum = sigInfo.getSeqNum();
+ auto nonce = sigInfo.getNonce();
auto record = m_byKeyName.find(keyName);
if (m_options.shouldValidateTimestamps) {
if (!timestamp.has_value()) {
- state->fail({ValidationError::POLICY_ERROR,
- "Timestamp is required by policy but is not present"});
+ state->fail({ValidationError::POLICY_ERROR, "Timestamp is required by policy but is not present"});
return false;
}
diff --git a/ndn-cxx/security/validation-policy-signed-interest.hpp b/ndn-cxx/security/validation-policy-signed-interest.hpp
index 3b8124a..e0caf85 100644
--- a/ndn-cxx/security/validation-policy-signed-interest.hpp
+++ b/ndn-cxx/security/validation-policy-signed-interest.hpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2013-2021 Regents of the University of California.
+ * Copyright (c) 2013-2022 Regents of the University of California.
*
* This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
*
@@ -34,9 +34,12 @@
namespace security {
inline namespace v2 {
-/** \brief Validation policy for signed Interests
+/**
+ * @brief Validation policy for signed Interests.
*
- * This policy checks the timestamp, sequence number, and nonce fields of signed Interests.
+ * This policy checks the timestamp, sequence number, and/or nonce fields of signed Interests.
+ *
+ * @sa https://named-data.net/doc/NDN-packet-spec/0.3/signed-interest.html
*/
class ValidationPolicySignedInterest : public ValidationPolicy
{
diff --git a/ndn-cxx/security/validation-policy-simple-hierarchy.cpp b/ndn-cxx/security/validation-policy-simple-hierarchy.cpp
index 40f554f..6b5937f 100644
--- a/ndn-cxx/security/validation-policy-simple-hierarchy.cpp
+++ b/ndn-cxx/security/validation-policy-simple-hierarchy.cpp
@@ -29,48 +29,58 @@
ValidationPolicySimpleHierarchy::checkPolicy(const Data& data, const shared_ptr<ValidationState>& state,
const ValidationContinuation& continueValidation)
{
- Name klName = getKeyLocatorName(data, *state);
+ Name klName = getKeyLocatorName(data.getSignatureInfo(), *state);
if (!state->getOutcome()) { // already failed
return;
}
+ Name identity;
try {
- if (extractIdentityNameFromKeyLocator(klName).isPrefixOf(data.getName())) {
- continueValidation(make_shared<CertificateRequest>(klName), state);
- return;
- }
+ identity = extractIdentityNameFromKeyLocator(klName);
}
catch (const KeyLocator::Error& e) {
state->fail({ValidationError::INVALID_KEY_LOCATOR, e.what()});
return;
}
- state->fail({ValidationError::INVALID_KEY_LOCATOR, "Data signing policy violation for " +
- data.getName().toUri() + " by " + klName.toUri()});
+ if (!identity.isPrefixOf(data.getName())) {
+ state->fail({ValidationError::POLICY_ERROR,
+ "Data " + data.getName().toUri() + " signed by " + klName.toUri()});
+ return;
+ }
+
+ continueValidation(make_shared<CertificateRequest>(klName), state);
}
void
ValidationPolicySimpleHierarchy::checkPolicy(const Interest& interest, const shared_ptr<ValidationState>& state,
const ValidationContinuation& continueValidation)
{
- Name klName = getKeyLocatorName(interest, *state);
+ auto sigInfo = getSignatureInfo(interest, *state);
+ if (!state->getOutcome()) { // already failed
+ return;
+ }
+ Name klName = getKeyLocatorName(sigInfo, *state);
if (!state->getOutcome()) { // already failed
return;
}
+ Name identity;
try {
- if (extractIdentityNameFromKeyLocator(klName).isPrefixOf(interest.getName())) {
- continueValidation(make_shared<CertificateRequest>(klName), state);
- return;
- }
+ identity = extractIdentityNameFromKeyLocator(klName);
}
catch (const KeyLocator::Error& e) {
state->fail({ValidationError::INVALID_KEY_LOCATOR, e.what()});
return;
}
- state->fail({ValidationError::INVALID_KEY_LOCATOR, "Interest signing policy violation for " +
- interest.getName().toUri() + " by " + klName.toUri()});
+ if (!identity.isPrefixOf(interest.getName())) {
+ state->fail({ValidationError::POLICY_ERROR,
+ "Interest " + interest.getName().toUri() + " signed by " + klName.toUri()});
+ return;
+ }
+
+ continueValidation(make_shared<CertificateRequest>(klName), state);
}
} // inline namespace v2
diff --git a/ndn-cxx/security/validation-policy.cpp b/ndn-cxx/security/validation-policy.cpp
index 8b8fabc..5dc199b 100644
--- a/ndn-cxx/security/validation-policy.cpp
+++ b/ndn-cxx/security/validation-policy.cpp
@@ -60,7 +60,7 @@
}
}
-static Name
+Name
getKeyLocatorName(const SignatureInfo& si, ValidationState& state)
{
if (si.getSignatureType() == tlv::DigestSha256) {
@@ -81,40 +81,32 @@
return kl.getName();
}
-Name
-getKeyLocatorName(const Data& data, ValidationState& state)
-{
- return getKeyLocatorName(data.getSignatureInfo(), state);
-}
-
-Name
-getKeyLocatorName(const Interest& interest, ValidationState& state)
+SignatureInfo
+getSignatureInfo(const Interest& interest, ValidationState& state)
{
auto fmt = state.getTag<SignedInterestFormatTag>();
BOOST_ASSERT(fmt);
if (*fmt == SignedInterestFormat::V03) {
- BOOST_ASSERT(interest.getSignatureInfo());
- return getKeyLocatorName(*interest.getSignatureInfo(), state);
+ BOOST_ASSERT(interest.getSignatureInfo().has_value());
+ return *interest.getSignatureInfo();
}
- // Try Signed Interest format from Packet Specification v0.2
+ // Try the old Signed Interest format from Packet Specification v0.2
const Name& name = interest.getName();
if (name.size() < signed_interest::MIN_SIZE) {
- state.fail({ValidationError::INVALID_KEY_LOCATOR, "Invalid signed Interest: name too short"});
+ state.fail({ValidationError::NO_SIGNATURE, "Interest name too short `" + name.toUri() + "`"});
return {};
}
- SignatureInfo si;
try {
- si.wireDecode(name[signed_interest::POS_SIG_INFO].blockFromValue());
+ return SignatureInfo(name[signed_interest::POS_SIG_INFO].blockFromValue());
}
catch (const tlv::Error& e) {
- state.fail({ValidationError::INVALID_KEY_LOCATOR, "Invalid signed Interest: "s + e.what()});
+ state.fail({ValidationError::NO_SIGNATURE,
+ "Malformed SignatureInfo in `" + name.toUri() + "`: " + e.what()});
return {};
}
-
- return getKeyLocatorName(si, state);
}
Name
@@ -126,13 +118,17 @@
return keyLocator;
}
- for (ssize_t i = 1; i <= std::min<ssize_t>(-Certificate::KEY_COMPONENT_OFFSET, keyLocator.size()); ++i) {
- if (keyLocator[-i] == Certificate::KEY_COMPONENT) {
- return keyLocator.getPrefix(-i);
+ auto len = static_cast<ssize_t>(keyLocator.size());
+ // note that KEY_COMPONENT_OFFSET is negative
+ auto lowerBound = std::max<ssize_t>(len + Certificate::KEY_COMPONENT_OFFSET, 0);
+ for (ssize_t i = len - 1; i >= lowerBound; --i) {
+ if (keyLocator[i] == Certificate::KEY_COMPONENT) {
+ return keyLocator.getPrefix(i);
}
}
- NDN_THROW(KeyLocator::Error("KeyLocator name `" + keyLocator.toUri() + "` does not respect the naming conventions"));
+ NDN_THROW(KeyLocator::Error("KeyLocator `" + keyLocator.toUri() +
+ "` does not respect the naming conventions"));
}
} // inline namespace v2
diff --git a/ndn-cxx/security/validation-policy.hpp b/ndn-cxx/security/validation-policy.hpp
index f8410d3..31ef78e 100644
--- a/ndn-cxx/security/validation-policy.hpp
+++ b/ndn-cxx/security/validation-policy.hpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2013-2021 Regents of the University of California.
+ * Copyright (c) 2013-2022 Regents of the University of California.
*
* This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
*
@@ -32,7 +32,7 @@
inline namespace v2 {
/**
- * @brief Abstraction that implements validation policy for Data and Interest packets
+ * @brief Abstraction that implements a validation policy for Interest and Data packets.
*/
class ValidationPolicy : noncopyable
{
@@ -147,26 +147,29 @@
unique_ptr<ValidationPolicy> m_innerPolicy;
};
-/** \brief extract KeyLocator.Name from a Data packet
+/**
+ * @brief Extract SignatureInfo from a signed Interest.
*
- * The Data packet must contain a KeyLocator of Name type.
- * Otherwise, state.fail is invoked with INVALID_KEY_LOCATOR error.
+ * Signed Interests according to Packet Specification v0.3+, as identified by the
+ * SignedInterestFormatTag inside @p state, must have an InterestSignatureInfo element.
+ * Legacy signed Interests must contain a (%Data)%SignatureInfo name component.
+ * In both cases, if any TLV parsing errors are encountered, ValidationState::fail()
+ * is invoked on @p state with a ValidationError::NO_SIGNATURE error code.
+ *
+ * @pre @p state must contain a SignedInterestFormatTag to indicate whether the %Interest is
+ * signed according to Packet Specification v0.3+ or a previous specification.
*/
-Name
-getKeyLocatorName(const Data& data, ValidationState& state);
+SignatureInfo
+getSignatureInfo(const Interest& interest, ValidationState& state);
-/** \brief extract KeyLocator.Name from signed Interest
+/**
+ * @brief Extract the KeyLocator name from a SignatureInfo element.
*
- * Signed Interests according to Packet Specification v0.3+, as identified inside the state, must
- * have an InterestSignatureInfo element. Legacy signed Interests must contain a
- * (Data)SignatureInfo name component. In both cases, the included KeyLocator must be of the Name
- * type. otherwise, state.fail will be invoked with an INVALID_KEY_LOCATOR error.
- *
- * Interests specified to this method must be tagged with a SignedInterestFormatTag to indicate
- * whether they are signed according to Packet Specification v0.3+ or a previous specification.
+ * @p sigInfo must contain a KeyLocator of %Name type. Otherwise, ValidationState::fail()
+ * is invoked on @p state with a ValidationError::INVALID_KEY_LOCATOR error code.
*/
Name
-getKeyLocatorName(const Interest& interest, ValidationState& state);
+getKeyLocatorName(const SignatureInfo& sigInfo, ValidationState& state);
/**
* @brief Extract identity name from key, version-less certificate, or certificate name
diff --git a/ndn-cxx/security/validation-state.cpp b/ndn-cxx/security/validation-state.cpp
index b926fd7..e917fbc 100644
--- a/ndn-cxx/security/validation-state.cpp
+++ b/ndn-cxx/security/validation-state.cpp
@@ -59,15 +59,13 @@
const auto& certToValidate = *it;
if (!verifySignature(certToValidate, *validatedCert)) {
- this->fail({ValidationError::INVALID_SIGNATURE, "Invalid signature of certificate `" +
- certToValidate.getName().toUri() + "`"});
+ this->fail({ValidationError::INVALID_SIGNATURE, "Certificate " + certToValidate.getName().toUri()});
m_certificateChain.erase(it, m_certificateChain.end());
return nullptr;
}
- else {
- NDN_LOG_TRACE_DEPTH("OK signature for certificate `" << certToValidate.getName() << "`");
- validatedCert = &certToValidate;
- }
+
+ NDN_LOG_TRACE_DEPTH("OK signature for certificate `" << certToValidate.getName() << "`");
+ validatedCert = &certToValidate;
}
return validatedCert;
}
@@ -103,8 +101,7 @@
m_outcome = true;
}
else {
- this->fail({ValidationError::INVALID_SIGNATURE, "Invalid signature of data `" +
- m_data.getName().toUri() + "`"});
+ this->fail({ValidationError::INVALID_SIGNATURE, "Data " + m_data.getName().toUri()});
}
}
@@ -157,8 +154,7 @@
m_outcome = true;
}
else {
- this->fail({ValidationError::INVALID_SIGNATURE, "Invalid signature of interest `" +
- m_interest.getName().toUri() + "`"});
+ this->fail({ValidationError::INVALID_SIGNATURE, "Interest " + m_interest.getName().toUri()});
}
}
diff --git a/ndn-cxx/security/validator.cpp b/ndn-cxx/security/validator.cpp
index ba00728..984508b 100644
--- a/ndn-cxx/security/validator.cpp
+++ b/ndn-cxx/security/validator.cpp
@@ -51,19 +51,11 @@
const DataValidationFailureCallback& failureCb)
{
auto state = make_shared<DataValidationState>(data, successCb, failureCb);
-
NDN_LOG_DEBUG_DEPTH("Start validating data " << data.getName());
- m_policy->checkPolicy(data, state,
- [this] (const shared_ptr<CertificateRequest>& certRequest, const shared_ptr<ValidationState>& state) {
- if (certRequest == nullptr) {
- state->bypassValidation();
- }
- else {
- // need to fetch key and validate it
- requestCertificate(certRequest, state);
- }
- });
+ m_policy->checkPolicy(data, state, [this] (auto&&... args) {
+ continueValidation(std::forward<decltype(args)>(args)...);
+ });
}
void
@@ -72,21 +64,20 @@
const InterestValidationFailureCallback& failureCb)
{
auto state = make_shared<InterestValidationState>(interest, successCb, failureCb);
- auto fmt = interest.getSignatureInfo() ? SignedInterestFormat::V03 : SignedInterestFormat::V02;
- state->setTag(make_shared<SignedInterestFormatTag>(fmt));
+ NDN_LOG_DEBUG_DEPTH("Start validating interest " << interest.getName());
- NDN_LOG_DEBUG_DEPTH("Start validating interest (" << fmt << ") " << interest.getName());
+ try {
+ auto fmt = interest.getSignatureInfo() ? SignedInterestFormat::V03 : SignedInterestFormat::V02;
+ state->setTag(make_shared<SignedInterestFormatTag>(fmt));
+ }
+ catch (const tlv::Error& e) {
+ return state->fail({ValidationError::NO_SIGNATURE, "Malformed InterestSignatureInfo in `" +
+ interest.getName().toUri() + "`: " + e.what()});
+ }
- m_policy->checkPolicy(interest, state,
- [this] (const shared_ptr<CertificateRequest>& certRequest, const shared_ptr<ValidationState>& state) {
- if (certRequest == nullptr) {
- state->bypassValidation();
- }
- else {
- // need to fetch key and validate it
- requestCertificate(certRequest, state);
- }
- });
+ m_policy->checkPolicy(interest, state, [this] (auto&&... args) {
+ continueValidation(std::forward<decltype(args)>(args)...);
+ });
}
void
@@ -114,6 +105,21 @@
}
void
+Validator::continueValidation(const shared_ptr<CertificateRequest>& certRequest,
+ const shared_ptr<ValidationState>& state)
+{
+ BOOST_ASSERT(state);
+
+ if (certRequest == nullptr) {
+ state->bypassValidation();
+ }
+ else {
+ // need to fetch key and validate it
+ requestCertificate(certRequest, state);
+ }
+}
+
+void
Validator::requestCertificate(const shared_ptr<CertificateRequest>& certRequest,
const shared_ptr<ValidationState>& state)
{
@@ -128,7 +134,7 @@
}
if (state->hasSeenCertificateName(certRequest->interest.getName())) {
- state->fail({ValidationError::LOOP_DETECTED, "`" + certRequest->interest.getName().toUri() + "`"});
+ state->fail({ValidationError::LOOP_DETECTED, certRequest->interest.getName().toUri()});
return;
}
diff --git a/ndn-cxx/security/validator.hpp b/ndn-cxx/security/validator.hpp
index 3fc011e..f08585d 100644
--- a/ndn-cxx/security/validator.hpp
+++ b/ndn-cxx/security/validator.hpp
@@ -179,9 +179,19 @@
validate(const Certificate& cert, const shared_ptr<ValidationState>& state);
/**
+ * @brief Validation callback invoked by the policy.
+ *
+ * @param certRequest Certificate request, may be nullptr.
+ * @param state The current validation state.
+ */
+ void
+ continueValidation(const shared_ptr<CertificateRequest>& certRequest,
+ const shared_ptr<ValidationState>& state);
+
+ /**
* @brief Request certificate for further validation.
*
- * @param certRequest Certificate request.
+ * @param certRequest Certificate request, must not be nullptr.
* @param state The current validation state.
*/
void
diff --git a/tests/unit/security/certificate-fetcher-direct-fetch.t.cpp b/tests/unit/security/certificate-fetcher-direct-fetch.t.cpp
index 183a947..7c9ba90 100644
--- a/tests/unit/security/certificate-fetcher-direct-fetch.t.cpp
+++ b/tests/unit/security/certificate-fetcher-direct-fetch.t.cpp
@@ -21,11 +21,10 @@
#include "ndn-cxx/security/certificate-fetcher-direct-fetch.hpp"
-#include "ndn-cxx/lp/nack.hpp"
#include "ndn-cxx/lp/tags.hpp"
#include "ndn-cxx/security/validation-policy-simple-hierarchy.hpp"
-#include "tests/boost-test.hpp"
+#include "tests/test-common.hpp"
#include "tests/unit/security/validator-fixture.hpp"
#include <boost/range/adaptor/sliced.hpp>
@@ -133,9 +132,7 @@
void
CertificateFetcherDirectFetchFixture<Nack>::makeResponse(const Interest& interest)
{
- lp::Nack nack(interest);
- nack.setHeader(lp::NackHeader().setReason(lp::NackReason::NO_ROUTE));
- face.receive(nack);
+ face.receive(makeNack(interest, lp::NackReason::NO_ROUTE));
}
using Failures = boost::mpl::vector<Timeout, Nack>;
@@ -174,9 +171,10 @@
BOOST_FIXTURE_TEST_CASE_TEMPLATE(ValidateFailureData, T, Failures, CertificateFetcherDirectFetchFixture<T>)
{
VALIDATE_FAILURE(this->data, "Should fail, as all interests either NACKed or timeout");
+ BOOST_TEST(this->lastError.getCode() == ValidationError::CANNOT_RETRIEVE_CERT);
// Direct fetcher sends two interests each time - to network and face
// 3 retries on nack or timeout (2 * (1 + 3) = 8)
- BOOST_CHECK_EQUAL(this->face.sentInterests.size(), 8);
+ BOOST_TEST(this->face.sentInterests.size() == 8);
// odd interests
for (const auto& sentInterest : this->face.sentInterests | boost::adaptors::strided(2)) {
@@ -197,9 +195,10 @@
static_cast<CertificateFetcherDirectFetch&>(this->validator.getFetcher()).setSendDirectInterestOnly(true);
VALIDATE_FAILURE(this->data, "Should fail, as all interests either NACKed or timeout");
+ BOOST_TEST(this->lastError.getCode() == ValidationError::CANNOT_RETRIEVE_CERT);
// Direct fetcher sends two interests each time - to network and face
// 3 retries on nack or timeout (1 + 3 = 4)
- BOOST_CHECK_EQUAL(this->face.sentInterests.size(), 4);
+ BOOST_TEST(this->face.sentInterests.size() == 4);
for (const auto& sentInterest : this->face.sentInterests) {
BOOST_CHECK(sentInterest.template getTag<lp::NextHopFaceIdTag>() != nullptr);
@@ -215,8 +214,8 @@
this->interest.template removeTag<lp::IncomingFaceIdTag>();
VALIDATE_FAILURE(this->data, "Should fail, as no interests are expected");
- BOOST_CHECK_EQUAL(this->face.sentInterests.size(), 0);
- BOOST_CHECK_NE(this->lastError.getCode(), ValidationError::IMPLEMENTATION_ERROR);
+ BOOST_TEST(this->lastError.getCode() == ValidationError::CANNOT_RETRIEVE_CERT);
+ BOOST_TEST(this->face.sentInterests.size() == 0);
}
BOOST_FIXTURE_TEST_CASE(ValidateSuccessInterest, CertificateFetcherDirectFetchFixture<Cert>)
@@ -240,9 +239,10 @@
BOOST_FIXTURE_TEST_CASE_TEMPLATE(ValidateFailureInterest, T, Failures, CertificateFetcherDirectFetchFixture<T>)
{
VALIDATE_FAILURE(this->interest, "Should fail, as all interests either NACKed or timeout");
+ BOOST_TEST(this->lastError.getCode() == ValidationError::CANNOT_RETRIEVE_CERT);
// Direct fetcher sends two interests each time - to network and face
// 3 retries on nack or timeout (2 * (1 + 3) = 4)
- BOOST_CHECK_EQUAL(this->face.sentInterests.size(), 8);
+ BOOST_TEST(this->face.sentInterests.size() == 8);
// odd interests
for (const auto& sentInterest : this->face.sentInterests | boost::adaptors::strided(2)) {
diff --git a/tests/unit/security/certificate-fetcher-from-network.t.cpp b/tests/unit/security/certificate-fetcher-from-network.t.cpp
index 29d8243..cb3e0b5 100644
--- a/tests/unit/security/certificate-fetcher-from-network.t.cpp
+++ b/tests/unit/security/certificate-fetcher-from-network.t.cpp
@@ -119,15 +119,17 @@
BOOST_FIXTURE_TEST_CASE_TEMPLATE(ValidateFailure, T, Failures, CertificateFetcherFromNetworkFixture<T>)
{
VALIDATE_FAILURE(this->data, "Should fail, as interests don't bring data");
+ BOOST_TEST(this->lastError.getCode() == ValidationError::CANNOT_RETRIEVE_CERT);
// first interest + 3 retries
- BOOST_CHECK_EQUAL(this->face.sentInterests.size(), 4);
+ BOOST_TEST(this->face.sentInterests.size() == 4);
this->face.sentInterests.clear();
this->advanceClocks(1_h, 2); // expire validator caches
VALIDATE_FAILURE(this->interest, "Should fail, as interests don't bring data");
- BOOST_CHECK_EQUAL(this->face.sentInterests.size(), 4);
+ BOOST_TEST(this->lastError.getCode() == ValidationError::CANNOT_RETRIEVE_CERT);
+ BOOST_TEST(this->face.sentInterests.size() == 4);
}
BOOST_AUTO_TEST_SUITE_END() // TestCertificateFetcherFromNetwork
diff --git a/tests/unit/security/certificate-fetcher-offline.t.cpp b/tests/unit/security/certificate-fetcher-offline.t.cpp
index f674993..f48971f 100644
--- a/tests/unit/security/certificate-fetcher-offline.t.cpp
+++ b/tests/unit/security/certificate-fetcher-offline.t.cpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2013-2020 Regents of the University of California.
+ * Copyright (c) 2013-2022 Regents of the University of California.
*
* This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
*
@@ -54,7 +54,8 @@
auto packet = Packet::makePacket(name);
m_keyChain.sign(packet, signingByIdentity(subIdentity));
VALIDATE_FAILURE(packet, "Should fail, as no cert should be requested");
- BOOST_CHECK_EQUAL(this->face.sentInterests.size(), 0);
+ BOOST_TEST(this->lastError.getCode() == ValidationError::CANNOT_RETRIEVE_CERT);
+ BOOST_TEST(this->face.sentInterests.size() == 0);
packet = Packet::makePacket(name);
m_keyChain.sign(packet, signingByIdentity(identity));
diff --git a/tests/unit/security/validation-policy-command-interest.t.cpp b/tests/unit/security/validation-policy-command-interest.t.cpp
index cf91ee3..d6fd74f 100644
--- a/tests/unit/security/validation-policy-command-interest.t.cpp
+++ b/tests/unit/security/validation-policy-command-interest.t.cpp
@@ -28,7 +28,6 @@
#include "tests/test-common.hpp"
#include "tests/unit/security/validator-fixture.hpp"
-#include <boost/lexical_cast.hpp>
#include <boost/mpl/vector.hpp>
namespace ndn {
@@ -40,9 +39,8 @@
BOOST_AUTO_TEST_SUITE(Security)
-class CommandInterestDefaultOptions
+struct CommandInterestDefaultOptions
{
-public:
static ValidationPolicyCommandInterest::Options
getOptions()
{
@@ -61,62 +59,82 @@
};
template<class T, class InnerPolicy = ValidationPolicySimpleHierarchy>
-class ValidationPolicyCommandInterestFixture : public HierarchicalValidatorFixture<CommandInterestPolicyWrapper<T, InnerPolicy>>
+class ValidationPolicyCommandInterestFixture
+ : public HierarchicalValidatorFixture<CommandInterestPolicyWrapper<T, InnerPolicy>>
{
-public:
+protected:
Interest
- makeCommandInterest(const Identity& identity, bool wantV3 = false)
+ makeCommandInterest(const Identity& identity, SignedInterestFormat format = SignedInterestFormat::V02)
{
- if (wantV3) {
- Interest i(Name(identity.getName()).append("CMD"));
- m_signer.makeSignedInterest(i, signingByIdentity(identity));
- return i;
+ Name name = identity.getName();
+ name.append("CMD");
+ switch (format) {
+ case SignedInterestFormat::V02:
+ return m_signer.makeCommandInterest(name, signingByIdentity(identity));
+ case SignedInterestFormat::V03: {
+ Interest interest(name);
+ m_signer.makeSignedInterest(interest, signingByIdentity(identity));
+ return interest;
+ }
}
- else {
- return m_signer.makeCommandInterest(Name(identity.getName()).append("CMD"),
- signingByIdentity(identity));
- }
+ NDN_CXX_UNREACHABLE;
}
-public:
+protected:
InterestSigner m_signer{this->m_keyChain};
};
BOOST_FIXTURE_TEST_SUITE(TestValidationPolicyCommandInterest,
ValidationPolicyCommandInterestFixture<CommandInterestDefaultOptions>)
+template<int secs>
+struct GracePeriodSeconds
+{
+ static ValidationPolicyCommandInterest::Options
+ getOptions()
+ {
+ ValidationPolicyCommandInterest::Options options;
+ options.gracePeriod = time::seconds(secs);
+ return options;
+ }
+};
+
BOOST_AUTO_TEST_SUITE(Accepts)
-BOOST_AUTO_TEST_CASE(Basic)
+BOOST_AUTO_TEST_CASE(BasicV02)
{
- auto i1 = makeCommandInterest(identity);
+ auto i1 = makeCommandInterest(identity, SignedInterestFormat::V02);
VALIDATE_SUCCESS(i1, "Should succeed (within grace period)");
VALIDATE_FAILURE(i1, "Should fail (replay attack)");
+ BOOST_TEST(lastError.getCode() == ValidationError::POLICY_ERROR);
advanceClocks(5_ms);
- auto i2 = makeCommandInterest(identity);
+ auto i2 = makeCommandInterest(identity, SignedInterestFormat::V02);
VALIDATE_SUCCESS(i2, "Should succeed (timestamp larger than previous)");
- auto i3 = m_signer.makeCommandInterest(Name(identity.getName()).append("CMD"), signingWithSha256());
+ auto i3 = m_signer.makeCommandInterest(Name(identity.getName()).append("CMD"), signingWithSha256());
VALIDATE_FAILURE(i3, "Should fail (Sha256 signature violates policy)");
+ BOOST_TEST(lastError.getCode() == ValidationError::POLICY_ERROR);
}
-BOOST_AUTO_TEST_CASE(BasicV3)
+BOOST_AUTO_TEST_CASE(BasicV03)
{
- auto i1 = makeCommandInterest(identity, true);
+ auto i1 = makeCommandInterest(identity, SignedInterestFormat::V03);
VALIDATE_SUCCESS(i1, "Should succeed (within grace period)");
VALIDATE_FAILURE(i1, "Should fail (replay attack)");
+ BOOST_TEST(lastError.getCode() == ValidationError::POLICY_ERROR);
advanceClocks(5_ms);
- auto i2 = makeCommandInterest(identity, true);
+ auto i2 = makeCommandInterest(identity, SignedInterestFormat::V03);
VALIDATE_SUCCESS(i2, "Should succeed (timestamp larger than previous)");
Interest i3(Name(identity.getName()).append("CMD"));
m_signer.makeSignedInterest(i3, signingWithSha256());
VALIDATE_FAILURE(i3, "Should fail (Sha256 signature violates policy)");
+ BOOST_TEST(lastError.getCode() == ValidationError::POLICY_ERROR);
}
-BOOST_AUTO_TEST_CASE(DataPassthru)
+BOOST_AUTO_TEST_CASE(DataPassthrough)
{
Data d1("/Security/ValidatorFixture/Sub1");
m_keyChain.sign(d1);
@@ -131,6 +149,7 @@
auto i1 = m_signer.makeCommandInterest("/hello/world/CMD", signingWithSha256());
VALIDATE_SUCCESS(i1, "Should succeed (within grace period)");
VALIDATE_FAILURE(i1, "Should fail (replay attack)");
+ BOOST_TEST(lastError.getCode() == ValidationError::POLICY_ERROR);
advanceClocks(5_ms);
auto i2 = m_signer.makeCommandInterest("/hello/world/CMD", signingWithSha256());
@@ -141,89 +160,118 @@
BOOST_AUTO_TEST_SUITE(Rejects)
-BOOST_AUTO_TEST_CASE(NameTooShort)
+BOOST_AUTO_TEST_CASE(NotSigned)
{
- auto i1 = makeInterest("/name/too/short");
- VALIDATE_FAILURE(*i1, "Should fail (name is too short)");
-}
-
-BOOST_AUTO_TEST_CASE(BadTimestamp)
-{
- auto i1 = makeCommandInterest(identity);
- setNameComponent(i1, command_interest::POS_TIMESTAMP, "not-timestamp");
- VALIDATE_FAILURE(i1, "Should fail (timestamp is missing)");
+ auto i1 = Interest("/short");
+ BOOST_TEST_REQUIRE(i1.getName().size() < signed_interest::MIN_SIZE);
+ VALIDATE_FAILURE(i1, "Should fail (not signed / name too short)");
+ BOOST_TEST(lastError.getCode() == ValidationError::NO_SIGNATURE);
}
BOOST_AUTO_TEST_CASE(BadSigInfo)
{
- auto i1 = makeCommandInterest(identity);
+ auto i1 = makeCommandInterest(identity, SignedInterestFormat::V02);
setNameComponent(i1, command_interest::POS_SIG_INFO, "not-SignatureInfo");
+ BOOST_TEST_REQUIRE(i1.getName().size() >= command_interest::MIN_SIZE);
VALIDATE_FAILURE(i1, "Should fail (signature info is missing)");
+ BOOST_TEST(lastError.getCode() == ValidationError::NO_SIGNATURE);
}
-BOOST_AUTO_TEST_CASE(MissingKeyLocator)
+BOOST_AUTO_TEST_CASE(BadTimestampV02)
{
- auto i1 = makeCommandInterest(identity);
+ // i1 is a valid signed interest (in v0.2 format) but is not a valid command interest
+ auto i1 = Interest("/short");
+ m_keyChain.sign(i1, signingByIdentity(identity).setSignedInterestFormat(SignedInterestFormat::V02));
+ BOOST_TEST_REQUIRE((i1.getName().size() >= signed_interest::MIN_SIZE &&
+ i1.getName().size() < command_interest::MIN_SIZE));
+ VALIDATE_FAILURE(i1, "Should fail (timestamp is missing)");
+ BOOST_TEST(lastError.getCode() == ValidationError::POLICY_ERROR);
+
+ auto i2 = makeCommandInterest(identity, SignedInterestFormat::V02);
+ setNameComponent(i2, command_interest::POS_TIMESTAMP, "not-timestamp");
+ BOOST_TEST_REQUIRE(i2.getName().size() >= command_interest::MIN_SIZE);
+ VALIDATE_FAILURE(i2, "Should fail (timestamp is malformed)");
+ BOOST_TEST(lastError.getCode() == ValidationError::POLICY_ERROR);
+}
+
+BOOST_AUTO_TEST_CASE(BadTimestampV03)
+{
+ auto i1 = makeCommandInterest(identity, SignedInterestFormat::V03);
+ auto si = i1.getSignatureInfo().value();
+ si.setTime(nullopt);
+ i1.setSignatureInfo(si);
+ VALIDATE_FAILURE(i1, "Should fail (timestamp is missing)");
+ BOOST_TEST(lastError.getCode() == ValidationError::POLICY_ERROR);
+}
+
+BOOST_AUTO_TEST_CASE(MissingKeyLocatorV02)
+{
+ auto i1 = makeCommandInterest(identity, SignedInterestFormat::V02);
SignatureInfo sigInfo(tlv::SignatureSha256WithRsa);
setNameComponent(i1, command_interest::POS_SIG_INFO,
sigInfo.wireEncode().begin(), sigInfo.wireEncode().end());
VALIDATE_FAILURE(i1, "Should fail (missing KeyLocator)");
+ BOOST_TEST(lastError.getCode() == ValidationError::INVALID_KEY_LOCATOR);
+}
+
+BOOST_AUTO_TEST_CASE(MissingKeyLocatorV03)
+{
+ auto i1 = makeCommandInterest(identity, SignedInterestFormat::V03);
+ auto si = i1.getSignatureInfo().value();
+ si.setKeyLocator(nullopt);
+ i1.setSignatureInfo(si);
+ VALIDATE_FAILURE(i1, "Should fail (missing KeyLocator)");
+ BOOST_TEST(lastError.getCode() == ValidationError::INVALID_KEY_LOCATOR);
}
BOOST_AUTO_TEST_CASE(BadKeyLocatorType)
{
- auto i1 = makeCommandInterest(identity);
+ auto i1 = makeCommandInterest(identity, SignedInterestFormat::V02);
KeyLocator kl;
kl.setKeyDigest(makeBinaryBlock(tlv::KeyDigest, {0xDD, 0xDD, 0xDD, 0xDD, 0xDD, 0xDD, 0xDD, 0xDD}));
- SignatureInfo sigInfo(tlv::SignatureSha256WithRsa);
- sigInfo.setKeyLocator(kl);
+ SignatureInfo sigInfo(tlv::SignatureSha256WithRsa, kl);
setNameComponent(i1, command_interest::POS_SIG_INFO,
sigInfo.wireEncode().begin(), sigInfo.wireEncode().end());
VALIDATE_FAILURE(i1, "Should fail (bad KeyLocator type)");
+ BOOST_TEST(lastError.getCode() == ValidationError::INVALID_KEY_LOCATOR);
}
BOOST_AUTO_TEST_CASE(BadCertName)
{
- auto i1 = makeCommandInterest(identity);
- KeyLocator kl;
- kl.setName("/bad/cert/name");
- SignatureInfo sigInfo(tlv::SignatureSha256WithRsa);
- sigInfo.setKeyLocator(kl);
+ auto i1 = makeCommandInterest(identity, SignedInterestFormat::V02);
+ SignatureInfo sigInfo(tlv::SignatureSha256WithEcdsa, KeyLocator("/bad/cert/name"));
setNameComponent(i1, command_interest::POS_SIG_INFO,
sigInfo.wireEncode().begin(), sigInfo.wireEncode().end());
VALIDATE_FAILURE(i1, "Should fail (bad certificate name)");
+ BOOST_TEST(lastError.getCode() == ValidationError::INVALID_KEY_LOCATOR);
}
BOOST_AUTO_TEST_CASE(InnerPolicyReject)
{
- auto i1 = makeCommandInterest(otherIdentity);
+ auto i1 = makeCommandInterest(otherIdentity, SignedInterestFormat::V02);
VALIDATE_FAILURE(i1, "Should fail (inner policy should reject)");
+ BOOST_TEST(lastError.getCode() == ValidationError::LOOP_DETECTED);
+
+ auto i2 = makeCommandInterest(otherIdentity, SignedInterestFormat::V03);
+ VALIDATE_FAILURE(i2, "Should fail (inner policy should reject)");
+ BOOST_TEST(lastError.getCode() == ValidationError::LOOP_DETECTED);
}
-class GracePeriod15Sec
-{
-public:
- static ValidationPolicyCommandInterest::Options
- getOptions()
- {
- ValidationPolicyCommandInterest::Options options;
- options.gracePeriod = 15_s;
- return options;
- }
-};
-
-BOOST_FIXTURE_TEST_CASE(TimestampOutOfGracePositive, ValidationPolicyCommandInterestFixture<GracePeriod15Sec>)
+BOOST_FIXTURE_TEST_CASE(TimestampOutOfGracePositive,
+ ValidationPolicyCommandInterestFixture<GracePeriodSeconds<15>>)
{
auto i1 = makeCommandInterest(identity); // signed at 0s
advanceClocks(16_s); // verifying at +16s
VALIDATE_FAILURE(i1, "Should fail (timestamp outside the grace period)");
+ BOOST_TEST(lastError.getCode() == ValidationError::POLICY_ERROR);
rewindClockAfterValidation();
auto i2 = makeCommandInterest(identity); // signed at +16s
VALIDATE_SUCCESS(i2, "Should succeed");
}
-BOOST_FIXTURE_TEST_CASE(TimestampOutOfGraceNegative, ValidationPolicyCommandInterestFixture<GracePeriod15Sec>)
+BOOST_FIXTURE_TEST_CASE(TimestampOutOfGraceNegative,
+ ValidationPolicyCommandInterestFixture<GracePeriodSeconds<15>>)
{
auto i1 = makeCommandInterest(identity); // signed at 0s
advanceClocks(1_s);
@@ -233,10 +281,12 @@
m_systemClock->advance(-18_s); // verifying at -16s
VALIDATE_FAILURE(i1, "Should fail (timestamp outside the grace period)");
+ BOOST_TEST(lastError.getCode() == ValidationError::POLICY_ERROR);
rewindClockAfterValidation();
// CommandInterestValidator should not remember i1's timestamp
VALIDATE_FAILURE(i2, "Should fail (timestamp outside the grace period)");
+ BOOST_TEST(lastError.getCode() == ValidationError::POLICY_ERROR);
rewindClockAfterValidation();
// CommandInterestValidator should not remember i2's timestamp, and should treat i3 as initial
@@ -250,9 +300,9 @@
VALIDATE_SUCCESS(i1, "Should succeed");
auto i2 = makeCommandInterest(identity); // signed at 0s
- setNameComponent(i2, command_interest::POS_TIMESTAMP,
- i1.getName()[command_interest::POS_TIMESTAMP]);
+ setNameComponent(i2, command_interest::POS_TIMESTAMP, i1.getName()[command_interest::POS_TIMESTAMP]);
VALIDATE_FAILURE(i2, "Should fail (timestamp reordered)");
+ BOOST_TEST(lastError.getCode() == ValidationError::POLICY_ERROR);
advanceClocks(2_s);
auto i3 = makeCommandInterest(identity); // signed at +2s
@@ -275,11 +325,13 @@
m_systemClock->advance(-1100_ms); // verifying at 0ms
VALIDATE_FAILURE(i2, "Should fail (timestamp reordered)");
+ BOOST_TEST(lastError.getCode() == ValidationError::POLICY_ERROR);
rewindClockAfterValidation();
// CommandInterestValidator should not remember i2's timestamp
advanceClocks(200_ms); // verifying at +200ms
VALIDATE_FAILURE(i3, "Should fail (timestamp reordered)");
+ BOOST_TEST(lastError.getCode() == ValidationError::POLICY_ERROR);
rewindClockAfterValidation();
advanceClocks(1200_ms); // verifying at 1400ms
@@ -290,25 +342,12 @@
BOOST_AUTO_TEST_SUITE(Options)
-template<class T>
-class GracePeriod
-{
-public:
- static ValidationPolicyCommandInterest::Options
- getOptions()
- {
- ValidationPolicyCommandInterest::Options options;
- options.gracePeriod = time::seconds(T::value);
- return options;
- }
-};
+using NonPositiveGracePeriods = boost::mpl::vector<
+ GracePeriodSeconds<0>,
+ GracePeriodSeconds<-1>
+>;
-typedef boost::mpl::vector<
- GracePeriod<boost::mpl::int_<0>>,
- GracePeriod<boost::mpl::int_<-1>>
-> GraceNonPositiveValues;
-
-BOOST_FIXTURE_TEST_CASE_TEMPLATE(GraceNonPositive, GracePeriod, GraceNonPositiveValues,
+BOOST_FIXTURE_TEST_CASE_TEMPLATE(GraceNonPositive, GracePeriod, NonPositiveGracePeriods,
ValidationPolicyCommandInterestFixture<GracePeriod>)
{
auto i1 = this->makeCommandInterest(this->identity); // signed at 0ms
@@ -323,6 +362,7 @@
this->advanceClocks(1_ms);
VALIDATE_FAILURE(i2, "Should fail when validating at 1ms");
+ BOOST_TEST(this->lastError.getCode() == ValidationError::POLICY_ERROR);
}
class LimitedRecordsOptions
@@ -402,11 +442,12 @@
advanceClocks(1_s);
for (size_t i = 0; i < 20; ++i) {
auto i2 = makeCommandInterest(identities.at(i)); // signed at +1s
-
VALIDATE_SUCCESS(i2, "Should succeed");
rewindClockAfterValidation();
}
+
VALIDATE_FAILURE(i1, "Should fail (timestamp reorder)");
+ BOOST_TEST(lastError.getCode() == ValidationError::POLICY_ERROR);
}
class ZeroRecordsOptions
diff --git a/tests/unit/security/validation-policy-signed-interest.t.cpp b/tests/unit/security/validation-policy-signed-interest.t.cpp
index c3d37e6..58f502e 100644
--- a/tests/unit/security/validation-policy-signed-interest.t.cpp
+++ b/tests/unit/security/validation-policy-signed-interest.t.cpp
@@ -28,7 +28,6 @@
#include "tests/test-common.hpp"
#include "tests/unit/security/validator-fixture.hpp"
-#include <boost/lexical_cast.hpp>
#include <boost/mpl/vector.hpp>
namespace ndn {
@@ -40,9 +39,8 @@
BOOST_AUTO_TEST_SUITE(Security)
-class SignedInterestDefaultOptions
+struct SignedInterestDefaultOptions
{
-public:
static ValidationPolicySignedInterest::Options
getOptions()
{
@@ -64,12 +62,7 @@
class ValidationPolicySignedInterestFixture
: public HierarchicalValidatorFixture<SignedInterestPolicyWrapper<T, InnerPolicy>>
{
-public:
- ValidationPolicySignedInterestFixture()
- : m_signer(this->m_keyChain)
- {
- }
-
+protected:
Interest
makeSignedInterest(const Identity& identity,
uint32_t signingFlags = InterestSigner::WantNonce | InterestSigner::WantTime)
@@ -79,8 +72,8 @@
return i;
}
-public:
- InterestSigner m_signer;
+protected:
+ InterestSigner m_signer{this->m_keyChain};
static constexpr uint32_t WantAll = InterestSigner::WantNonce |
InterestSigner::WantTime |
@@ -90,11 +83,12 @@
BOOST_FIXTURE_TEST_SUITE(TestValidationPolicySignedInterest,
ValidationPolicySignedInterestFixture<SignedInterestDefaultOptions>)
-BOOST_AUTO_TEST_CASE(BasicV3)
+BOOST_AUTO_TEST_CASE(Basic)
{
auto i1 = makeSignedInterest(identity, WantAll);
VALIDATE_SUCCESS(i1, "Should succeed (within grace period)");
VALIDATE_FAILURE(i1, "Should fail (replay attack)");
+ BOOST_TEST(lastError.getCode() == ValidationError::POLICY_ERROR);
advanceClocks(5_ms);
auto i2 = makeSignedInterest(identity, WantAll);
@@ -103,6 +97,7 @@
Interest i3(Name(identity.getName()).append("CMD"));
m_signer.makeSignedInterest(i3, signingWithSha256());
VALIDATE_FAILURE(i3, "Should fail (Sha256 signature violates policy)");
+ BOOST_TEST(lastError.getCode() == ValidationError::POLICY_ERROR);
}
BOOST_AUTO_TEST_CASE(DataPassthrough)
@@ -116,22 +111,23 @@
{
auto i1 = makeSignedInterest(otherIdentity);
VALIDATE_FAILURE(i1, "Should fail (inner policy should reject)");
+ BOOST_TEST(lastError.getCode() == ValidationError::LOOP_DETECTED);
}
-class LimitedRecordsOptions
+template<ssize_t count>
+struct MaxRecordCount
{
-public:
static ValidationPolicySignedInterest::Options
getOptions()
{
ValidationPolicySignedInterest::Options options;
options.timestampGracePeriod = 15_s;
- options.maxRecordCount = 3;
+ options.maxRecordCount = count;
return options;
}
};
-BOOST_FIXTURE_TEST_CASE(LimitedRecords, ValidationPolicySignedInterestFixture<LimitedRecordsOptions>)
+BOOST_FIXTURE_TEST_CASE(LimitedRecords, ValidationPolicySignedInterestFixture<MaxRecordCount<3>>)
{
Identity id1 = addSubCertificate("/Security/ValidatorFixture/Sub1", identity);
cache.insert(id1.getDefaultKey().getDefaultCertificate());
@@ -169,20 +165,7 @@
VALIDATE_SUCCESS(i01, "Should succeed despite timestamp is reordered, because record has been evicted");
}
-class UnlimitedRecordsOptions
-{
-public:
- static ValidationPolicySignedInterest::Options
- getOptions()
- {
- ValidationPolicySignedInterest::Options options;
- options.timestampGracePeriod = 15_s;
- options.maxRecordCount = -1;
- return options;
- }
-};
-
-BOOST_FIXTURE_TEST_CASE(UnlimitedRecords, ValidationPolicySignedInterestFixture<UnlimitedRecordsOptions>)
+BOOST_FIXTURE_TEST_CASE(UnlimitedRecords, ValidationPolicySignedInterestFixture<MaxRecordCount<-1>>)
{
std::vector<Identity> identities;
for (size_t i = 0; i < 20; ++i) {
@@ -195,27 +178,15 @@
advanceClocks(1_s);
for (size_t i = 0; i < 20; ++i) {
auto i2 = makeSignedInterest(identities.at(i)); // signed at +1s
-
VALIDATE_SUCCESS(i2, "Should succeed");
rewindClockAfterValidation();
}
+
VALIDATE_FAILURE(i1, "Should fail (timestamp reorder)");
+ BOOST_TEST(lastError.getCode() == ValidationError::POLICY_ERROR);
}
-class ZeroRecordsOptions
-{
-public:
- static ValidationPolicySignedInterest::Options
- getOptions()
- {
- ValidationPolicySignedInterest::Options options;
- options.timestampGracePeriod = 15_s;
- options.maxRecordCount = 0;
- return options;
- }
-};
-
-BOOST_FIXTURE_TEST_CASE(ZeroRecords, ValidationPolicySignedInterestFixture<ZeroRecordsOptions>)
+BOOST_FIXTURE_TEST_CASE(ZeroRecords, ValidationPolicySignedInterestFixture<MaxRecordCount<0>>)
{
auto i1 = makeSignedInterest(identity); // signed at 0s
advanceClocks(1_s);
@@ -232,11 +203,11 @@
{
auto i1 = makeSignedInterest(identity, InterestSigner::WantSeqNum);
VALIDATE_FAILURE(i1, "Should fail (timestamp missing)");
+ BOOST_TEST(lastError.getCode() == ValidationError::POLICY_ERROR);
}
-class DisabledTimestampValidationOptions
+struct DisabledTimestampValidationOptions
{
-public:
static ValidationPolicySignedInterest::Options
getOptions()
{
@@ -261,30 +232,31 @@
VALIDATE_SUCCESS(i2, "Should succeed");
}
-class GracePeriod15Sec
+template<int secs>
+struct GracePeriodSeconds
{
-public:
static ValidationPolicySignedInterest::Options
getOptions()
{
ValidationPolicySignedInterest::Options options;
- options.timestampGracePeriod = 15_s;
+ options.timestampGracePeriod = time::seconds(secs);
return options;
}
};
-BOOST_FIXTURE_TEST_CASE(TimestampTooOld, ValidationPolicySignedInterestFixture<GracePeriod15Sec>)
+BOOST_FIXTURE_TEST_CASE(TimestampTooOld, ValidationPolicySignedInterestFixture<GracePeriodSeconds<15>>)
{
auto i1 = makeSignedInterest(identity); // signed at 0s
advanceClocks(16_s); // verifying at +16s
VALIDATE_FAILURE(i1, "Should fail (timestamp outside the grace period)");
+ BOOST_TEST(lastError.getCode() == ValidationError::POLICY_ERROR);
rewindClockAfterValidation();
auto i2 = makeSignedInterest(identity); // signed at +16s
VALIDATE_SUCCESS(i2, "Should succeed");
}
-BOOST_FIXTURE_TEST_CASE(TimestampTooNew, ValidationPolicySignedInterestFixture<GracePeriod15Sec>)
+BOOST_FIXTURE_TEST_CASE(TimestampTooNew, ValidationPolicySignedInterestFixture<GracePeriodSeconds<15>>)
{
auto i1 = makeSignedInterest(identity); // signed at 0s
advanceClocks(1_s);
@@ -294,10 +266,12 @@
m_systemClock->advance(-18_s); // verifying at -16s
VALIDATE_FAILURE(i1, "Should fail (timestamp outside the grace period)");
+ BOOST_TEST(lastError.getCode() == ValidationError::POLICY_ERROR);
rewindClockAfterValidation();
// SignedInterestValidator should not remember i1's timestamp
VALIDATE_FAILURE(i2, "Should fail (timestamp outside the grace period)");
+ BOOST_TEST(lastError.getCode() == ValidationError::POLICY_ERROR);
rewindClockAfterValidation();
// SignedInterestValidator should not remember i2's timestamp, and should treat i3 as initial
@@ -315,6 +289,7 @@
si2->setTime(i1.getSignatureInfo()->getTime());
i2.setSignatureInfo(*si2);
VALIDATE_FAILURE(i2, "Should fail (timestamp reordered)");
+ BOOST_TEST(lastError.getCode() == ValidationError::POLICY_ERROR);
advanceClocks(2_s);
auto i3 = makeSignedInterest(identity); // signed at +2s
@@ -337,36 +312,25 @@
m_systemClock->advance(-1100_ms); // verifying at 0ms
VALIDATE_FAILURE(i2, "Should fail (timestamp reordered)");
+ BOOST_TEST(lastError.getCode() == ValidationError::POLICY_ERROR);
rewindClockAfterValidation();
// SignedInterestValidator should not remember i2's timestamp
advanceClocks(200_ms); // verifying at +200ms
VALIDATE_FAILURE(i3, "Should fail (timestamp reordered)");
+ BOOST_TEST(lastError.getCode() == ValidationError::POLICY_ERROR);
rewindClockAfterValidation();
advanceClocks(1200_ms); // verifying at 1400ms
VALIDATE_SUCCESS(i4, "Should succeed");
}
-template<class T>
-class GracePeriod
-{
-public:
- static ValidationPolicySignedInterest::Options
- getOptions()
- {
- ValidationPolicySignedInterest::Options options;
- options.timestampGracePeriod = time::seconds(T::value);
- return options;
- }
-};
+using NonPositiveGracePeriods = boost::mpl::vector<
+ GracePeriodSeconds<0>,
+ GracePeriodSeconds<-1>
+>;
-typedef boost::mpl::vector<
- GracePeriod<boost::mpl::int_<0>>,
- GracePeriod<boost::mpl::int_<-1>>
-> GraceNonPositiveValues;
-
-BOOST_FIXTURE_TEST_CASE_TEMPLATE(GraceNonPositive, GracePeriod, GraceNonPositiveValues,
+BOOST_FIXTURE_TEST_CASE_TEMPLATE(GraceNonPositive, GracePeriod, NonPositiveGracePeriods,
ValidationPolicySignedInterestFixture<GracePeriod>)
{
auto i1 = this->makeSignedInterest(this->identity); // signed at 0ms
@@ -383,6 +347,7 @@
this->advanceClocks(1_ms);
VALIDATE_FAILURE(i2, "Should fail when validating at 1ms");
+ BOOST_TEST(this->lastError.getCode() == ValidationError::POLICY_ERROR);
}
BOOST_AUTO_TEST_SUITE_END() // TimestampValidation
@@ -403,9 +368,8 @@
VALIDATE_SUCCESS(i2, "Should succeed");
}
-class SeqNumValidationOptions
+struct SeqNumValidationOptions
{
-public:
static ValidationPolicySignedInterest::Options
getOptions()
{
@@ -420,6 +384,7 @@
{
auto i1 = makeSignedInterest(identity, InterestSigner::WantTime);
VALIDATE_FAILURE(i1, "Should fail (sequence number missing");
+ BOOST_TEST(lastError.getCode() == ValidationError::POLICY_ERROR);
}
BOOST_FIXTURE_TEST_CASE(SeqNumReorder,
@@ -433,6 +398,7 @@
si2->setSeqNum(i1.getSignatureInfo()->getSeqNum());
i2.setSignatureInfo(*si2);
VALIDATE_FAILURE(i2, "Should fail (sequence number reordered)");
+ BOOST_TEST(lastError.getCode() == ValidationError::POLICY_ERROR);
auto i3 = makeSignedInterest(identity, WantAll); // seq num is i+2
VALIDATE_SUCCESS(i3, "Should succeed");
@@ -446,6 +412,7 @@
{
auto i1 = makeSignedInterest(identity, InterestSigner::WantTime); // Specifically exclude nonce
VALIDATE_FAILURE(i1, "Should fail (nonce missing)");
+ BOOST_TEST(lastError.getCode() == ValidationError::POLICY_ERROR);
}
BOOST_AUTO_TEST_CASE(DuplicateNonce)
@@ -458,6 +425,7 @@
si2->setNonce(i1.getSignatureInfo()->getNonce());
i2.setSignatureInfo(*si2);
VALIDATE_FAILURE(i2, "Should fail (duplicate nonce)");
+ BOOST_TEST(lastError.getCode() == ValidationError::POLICY_ERROR);
auto i3 = makeSignedInterest(identity, WantAll);
// On the off chance that the generated nonce is identical to i1
@@ -467,9 +435,8 @@
VALIDATE_SUCCESS(i3, "Should succeed");
}
-class DisabledNonceValidationOptions
+struct DisabledNonceValidationOptions
{
-public:
static ValidationPolicySignedInterest::Options
getOptions()
{
@@ -499,9 +466,8 @@
VALIDATE_SUCCESS(i3, "Should succeed");
}
-class NonceLimit2Options
+struct NonceLimit2Options
{
-public:
static ValidationPolicySignedInterest::Options
getOptions()
{
@@ -533,6 +499,7 @@
m_keyChain.sign(i3, signingByIdentity(identity).setSignedInterestFormat(SignedInterestFormat::V03)
.setSignatureInfo(*si3));
VALIDATE_FAILURE(i3, "Should fail (duplicate nonce)");
+ BOOST_TEST(lastError.getCode() == ValidationError::POLICY_ERROR);
// Pop i1's nonce off the list
auto i4 = makeSignedInterest(identity, WantAll);
diff --git a/tests/unit/security/validation-policy-simple-hierarchy.t.cpp b/tests/unit/security/validation-policy-simple-hierarchy.t.cpp
index 70fb49e..38ab30c 100644
--- a/tests/unit/security/validation-policy-simple-hierarchy.t.cpp
+++ b/tests/unit/security/validation-policy-simple-hierarchy.t.cpp
@@ -43,10 +43,13 @@
auto packet = Packet::makePacket(name);
VALIDATE_FAILURE(packet, "Unsigned");
+ BOOST_TEST((lastError.getCode() == ValidationError::NO_SIGNATURE || // Interest
+ lastError.getCode() == ValidationError::INVALID_KEY_LOCATOR)); // Data
packet = Packet::makePacket(name);
m_keyChain.sign(packet, signingWithSha256());
VALIDATE_FAILURE(packet, "Should not be accepted, name not prefix of /localhost/identity/digest-sha256");
+ BOOST_TEST(lastError.getCode() == ValidationError::POLICY_ERROR);
packet = Packet::makePacket("/localhost/identity/digest-sha256/foobar");
m_keyChain.sign(packet, signingWithSha256());
@@ -63,15 +66,17 @@
packet = Packet::makePacket(name);
m_keyChain.sign(packet, signingByIdentity(otherIdentity));
VALIDATE_FAILURE(packet, "Should fail, as signed by the policy-violating cert");
+ BOOST_TEST(lastError.getCode() == ValidationError::POLICY_ERROR);
packet = Packet::makePacket(name);
m_keyChain.sign(packet, signingByIdentity(subSelfSignedIdentity));
VALIDATE_FAILURE(packet, "Should fail, because subSelfSignedIdentity is not a trust anchor");
+ BOOST_TEST(lastError.getCode() == ValidationError::LOOP_DETECTED);
// TODO add checks with malformed packets
}
-BOOST_AUTO_TEST_CASE(NonKeyNameInsideLocator)
+BOOST_AUTO_TEST_CASE(CertNameInKeyLocator)
{
// auto cert = identity.getDefaultKey().getDefaultCertificate().wireEncode();
// std::cerr << "Certificate idCert{\"" << toHex(cert) << "\"_block};" << std::endl;
@@ -107,14 +112,13 @@
"483046022100BDD3E0EF2385658825EB73E87A02D1A16AA8ACE50840C1B91782836164AACA3B0221008007B3EBA9"
"B7638BD204766B08AF6E4221CDB88156CC7DA13CD916610D6D3AED"_block};
+ BOOST_REQUIRE_EQUAL(packet.getKeyLocator().value().getName(),
+ "/Security/ValidatorFixture/Sub1/KEY/%D7j1%B0%1E%14%09%2B/parent/%FD%00%00%01I%9DY%8C%A0");
+
this->cache.insert(idCert);
this->cache.insert(subIdCert);
this->validator.loadAnchor("", std::move(idCert));
- BOOST_REQUIRE(packet.getKeyLocator());
- BOOST_CHECK_EQUAL(packet.getKeyLocator()->getName(),
- "/Security/ValidatorFixture/Sub1/KEY/%D7j1%B0%1E%14%09%2B/parent/%FD%00%00%01I%9DY%8C%A0");
-
VALIDATE_SUCCESS(packet, "Should get accepted, as signed by the policy-compliant cert");
}
diff --git a/tests/unit/security/validation-policy.t.cpp b/tests/unit/security/validation-policy.t.cpp
index e982f72..3d76f62 100644
--- a/tests/unit/security/validation-policy.t.cpp
+++ b/tests/unit/security/validation-policy.t.cpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2013-2020 Regents of the University of California.
+ * Copyright (c) 2013-2022 Regents of the University of California.
*
* This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
*
@@ -35,17 +35,21 @@
BOOST_AUTO_TEST_CASE(ExtractIdentityNameFromKeyLocator)
{
auto id = m_keyChain.createIdentity("/random/identity");
-
auto keyName = id.getDefaultKey().getName();
auto certName = id.getDefaultKey().getDefaultCertificate().getName();
- auto partialCertName = id.getDefaultKey().getDefaultCertificate().getName().getPrefix(-1);
+ auto partialCertName = certName.getPrefix(-1);
BOOST_CHECK_EQUAL(extractIdentityNameFromKeyLocator(keyName), "/random/identity");
BOOST_CHECK_EQUAL(extractIdentityNameFromKeyLocator(certName), "/random/identity");
BOOST_CHECK_EQUAL(extractIdentityNameFromKeyLocator(partialCertName), "/random/identity");
+ BOOST_CHECK_EQUAL(extractIdentityNameFromKeyLocator("/KEY"), "/");
- BOOST_CHECK_THROW(extractIdentityNameFromKeyLocator(Name("/name/without/key/component")), std::runtime_error);
- BOOST_CHECK_THROW(extractIdentityNameFromKeyLocator(Name("/name/with/KEY/but/in/a/wrong/place")), std::runtime_error);
+ BOOST_CHECK_THROW(extractIdentityNameFromKeyLocator(Name("/short/name")),
+ KeyLocator::Error);
+ BOOST_CHECK_THROW(extractIdentityNameFromKeyLocator(Name("/name/without/key/component")),
+ KeyLocator::Error);
+ BOOST_CHECK_THROW(extractIdentityNameFromKeyLocator(Name("/name/with/KEY/but/in/a/wrong/place")),
+ KeyLocator::Error);
}
BOOST_AUTO_TEST_SUITE_END() // TestValidationPolicy
diff --git a/tests/unit/security/validator-fixture.cpp b/tests/unit/security/validator-fixture.cpp
index 8048fc4..8579fc8 100644
--- a/tests/unit/security/validator-fixture.cpp
+++ b/tests/unit/security/validator-fixture.cpp
@@ -56,6 +56,12 @@
advanceClocks(s_mockPeriod, s_mockTimes);
}
+void
+ValidatorFixtureBase::rewindClockAfterValidation()
+{
+ m_systemClock->advance(s_mockPeriod * s_mockTimes * -1);
+}
+
Identity
ValidatorFixtureBase::addSubCertificate(const Name& subIdentityName, const Identity& issuer)
{
diff --git a/tests/unit/security/validator-fixture.hpp b/tests/unit/security/validator-fixture.hpp
index b225c53..d8d357f 100644
--- a/tests/unit/security/validator-fixture.hpp
+++ b/tests/unit/security/validator-fixture.hpp
@@ -44,13 +44,11 @@
void
mockNetworkOperations();
- /** \brief undo clock advancement of mockNetworkOperations()
+ /**
+ * @brief Undo clock advancement of mockNetworkOperations()
*/
void
- rewindClockAfterValidation()
- {
- m_systemClock->advance(s_mockPeriod * s_mockTimes * -1);
- }
+ rewindClockAfterValidation();
/**
* @brief Issues a certificate for @p subIdentityName signed by @p issuer
@@ -71,8 +69,8 @@
ValidationError lastError{ValidationError::NO_ERROR};
private:
- const static time::milliseconds s_mockPeriod;
- const static int s_mockTimes;
+ static const time::milliseconds s_mockPeriod;
+ static const int s_mockTimes;
};
template<class ValidationPolicyT, class CertificateFetcherT = CertificateFetcherFromNetwork>
@@ -93,6 +91,7 @@
size_t nCallbacks = 0;
this->validator.validate(packet,
[&] (const Packet&) {
+ lastError = ValidationError::NO_ERROR;
++nCallbacks;
BOOST_CHECK_MESSAGE(expectSuccess,
(expectSuccess ? "OK: " : "FAILED: ") + detailedInfo);
diff --git a/tests/unit/security/validator.t.cpp b/tests/unit/security/validator.t.cpp
index 4acbe4d..842f2f5 100644
--- a/tests/unit/security/validator.t.cpp
+++ b/tests/unit/security/validator.t.cpp
@@ -22,7 +22,7 @@
#include "ndn-cxx/security/validator.hpp"
#include "ndn-cxx/security/validation-policy-simple-hierarchy.hpp"
-#include "tests/boost-test.hpp"
+#include "tests/test-common.hpp"
#include "tests/unit/security/validator-fixture.hpp"
namespace ndn {
@@ -46,33 +46,75 @@
BOOST_CHECK(validator.getPolicy().m_validator != nullptr);
BOOST_CHECK(validator.getPolicy().getInnerPolicy().m_validator != nullptr);
BOOST_CHECK(validator.getPolicy().getInnerPolicy().getInnerPolicy().m_validator != nullptr);
+
+ BOOST_CHECK_THROW(validator.getPolicy().setInnerPolicy(nullptr), std::invalid_argument);
}
-BOOST_AUTO_TEST_CASE(Timeouts)
+BOOST_AUTO_TEST_CASE(BadSignatureInfo)
{
- processInterest = nullptr; // no response for all interests
+ Interest interest("/Security/ValidatorFixture/Sub1/Sub2/Interest");
+ m_keyChain.sign(interest, signingByIdentity(subIdentity)
+ .setSignedInterestFormat(SignedInterestFormat::V03));
+
+ // add an unrecognized critical element inside InterestSignatureInfo
+ auto si = interest.getSignatureInfo().value();
+ si.addCustomTlv("7F00"_block);
+ interest.setSignatureInfo(si);
+ BOOST_REQUIRE_THROW(interest.getSignatureInfo(), tlv::Error);
+ BOOST_REQUIRE_NO_THROW(interest.getSignatureValue());
+
+ VALIDATE_FAILURE(interest, "InterestSignatureInfo decoding should fail");
+ BOOST_TEST(lastError.getCode() == ValidationError::NO_SIGNATURE);
+ BOOST_TEST(face.sentInterests.size() == 0);
+}
+
+BOOST_AUTO_TEST_CASE(BadSignatureValue)
+{
+ const uint8_t sv[] = {0x12, 0x34, 0x56, 0x78};
+
+ Interest interest("/Security/ValidatorFixture/Sub1/Sub2/Interest");
+ m_keyChain.sign(interest, signingByIdentity(subIdentity)
+ .setSignedInterestFormat(SignedInterestFormat::V03));
+ interest.setSignatureValue(sv);
+
+ VALIDATE_FAILURE(interest, "Signature check should fail");
+ BOOST_TEST(lastError.getCode() == ValidationError::INVALID_SIGNATURE);
+ BOOST_TEST(face.sentInterests.size() == 1);
+
+ Data data("/Security/ValidatorFixture/Sub1/Sub2/Data");
+ m_keyChain.sign(data, signingByIdentity(subIdentity));
+ data.setSignatureValue(sv);
+
+ VALIDATE_FAILURE(data, "Signature check should fail");
+ BOOST_TEST(lastError.getCode() == ValidationError::INVALID_SIGNATURE);
+ BOOST_TEST(face.sentInterests.size() == 1);
+}
+
+BOOST_AUTO_TEST_CASE(Timeout)
+{
+ processInterest = nullptr; // no response for any interest
Data data("/Security/ValidatorFixture/Sub1/Sub2/Data");
m_keyChain.sign(data, signingByIdentity(subIdentity));
VALIDATE_FAILURE(data, "Should fail to retrieve certificate");
- BOOST_CHECK_GT(face.sentInterests.size(), 1);
+ BOOST_TEST(lastError.getCode() == ValidationError::CANNOT_RETRIEVE_CERT);
+ BOOST_TEST(face.sentInterests.size() == 4);
}
-BOOST_AUTO_TEST_CASE(NackedInterests)
+BOOST_AUTO_TEST_CASE(Nack)
{
processInterest = [this] (const Interest& interest) {
- lp::Nack nack(interest);
- nack.setReason(lp::NackReason::NO_ROUTE);
- face.receive(nack);
+ face.receive(makeNack(interest, lp::NackReason::NO_ROUTE));
};
Data data("/Security/ValidatorFixture/Sub1/Sub2/Data");
m_keyChain.sign(data, signingByIdentity(subIdentity));
VALIDATE_FAILURE(data, "All interests should get NACKed");
+ BOOST_TEST(lastError.getCode() == ValidationError::CANNOT_RETRIEVE_CERT);
// 1 for the first interest, 3 for the retries on nack
- BOOST_CHECK_EQUAL(face.sentInterests.size(), 4);
+ BOOST_TEST(face.sentInterests.size() == 4);
}
BOOST_AUTO_TEST_CASE(MalformedCert)
@@ -84,7 +126,7 @@
BOOST_REQUIRE_THROW(Certificate(malformedCert.wireEncode()), tlv::Error);
auto originalProcessInterest = processInterest;
- processInterest = [this, &originalProcessInterest, &malformedCert] (const Interest& interest) {
+ processInterest = [&] (const Interest& interest) {
if (interest.getName().isPrefixOf(malformedCert.getName())) {
face.receive(malformedCert);
}
@@ -97,20 +139,20 @@
m_keyChain.sign(data, signingByIdentity(subIdentity));
VALIDATE_FAILURE(data, "Signed by a malformed certificate");
- BOOST_CHECK_EQUAL(face.sentInterests.size(), 1);
+ BOOST_TEST(lastError.getCode() == ValidationError::MALFORMED_CERT);
+ BOOST_TEST(face.sentInterests.size() == 1);
}
BOOST_AUTO_TEST_CASE(ExpiredCert)
{
Data expiredCert = subIdentity.getDefaultKey().getDefaultCertificate();
SignatureInfo info;
- info.setValidityPeriod(ValidityPeriod(time::system_clock::now() - 2_h,
- time::system_clock::now() - 1_h));
+ info.setValidityPeriod(ValidityPeriod::makeRelative(-2_h, -1_h));
m_keyChain.sign(expiredCert, signingByIdentity(identity).setSignatureInfo(info));
BOOST_REQUIRE_NO_THROW(Certificate(expiredCert.wireEncode()));
auto originalProcessInterest = processInterest;
- processInterest = [this, &originalProcessInterest, &expiredCert] (const Interest& interest) {
+ processInterest = [&] (const Interest& interest) {
if (interest.getName().isPrefixOf(expiredCert.getName())) {
face.receive(expiredCert);
}
@@ -123,7 +165,8 @@
m_keyChain.sign(data, signingByIdentity(subIdentity));
VALIDATE_FAILURE(data, "Signed by an expired certificate");
- BOOST_CHECK_EQUAL(face.sentInterests.size(), 1);
+ BOOST_TEST(lastError.getCode() == ValidationError::EXPIRED_CERT);
+ BOOST_TEST(face.sentInterests.size() == 1);
}
BOOST_AUTO_TEST_CASE(ResetAnchors)
@@ -133,6 +176,7 @@
Data data("/Security/ValidatorFixture/Sub1/Sub2/Data");
m_keyChain.sign(data, signingByIdentity(subIdentity));
VALIDATE_FAILURE(data, "Should fail, as no anchors configured");
+ BOOST_TEST(lastError.getCode() == ValidationError::LOOP_DETECTED);
}
BOOST_AUTO_TEST_CASE(TrustedCertCaching)
@@ -141,23 +185,23 @@
m_keyChain.sign(data, signingByIdentity(subIdentity));
VALIDATE_SUCCESS(data, "Should get accepted, as signed by the policy-compliant cert");
- BOOST_CHECK_EQUAL(face.sentInterests.size(), 1);
+ BOOST_TEST(face.sentInterests.size() == 1);
face.sentInterests.clear();
processInterest = nullptr; // disable data responses from mocked network
VALIDATE_SUCCESS(data, "Should get accepted, based on the cached trusted cert");
- BOOST_CHECK_EQUAL(face.sentInterests.size(), 0);
+ BOOST_TEST(face.sentInterests.size() == 0);
face.sentInterests.clear();
advanceClocks(1_h, 2); // expire trusted cache
VALIDATE_FAILURE(data, "Should try and fail to retrieve certs");
- BOOST_CHECK_GT(face.sentInterests.size(), 1);
- face.sentInterests.clear();
+ BOOST_TEST(lastError.getCode() == ValidationError::CANNOT_RETRIEVE_CERT);
+ BOOST_TEST(face.sentInterests.size() > 1);
}
-BOOST_AUTO_TEST_CASE(ResetVerifiedCertificates)
+BOOST_AUTO_TEST_CASE(ResetVerifiedCerts)
{
Data data("/Security/ValidatorFixture/Sub1/Sub2/Data");
m_keyChain.sign(data, signingByIdentity(subIdentity));
@@ -170,6 +214,7 @@
// reset trusted cache
validator.resetVerifiedCertificates();
VALIDATE_FAILURE(data, "Should fail, as no trusted cache or anchors");
+ BOOST_TEST(lastError.getCode() == ValidationError::LOOP_DETECTED);
}
BOOST_AUTO_TEST_CASE(UntrustedCertCaching)
@@ -178,20 +223,22 @@
m_keyChain.sign(data, signingByIdentity(subSelfSignedIdentity));
VALIDATE_FAILURE(data, "Should fail, as signed by the policy-violating cert");
- BOOST_CHECK_EQUAL(face.sentInterests.size(), 1);
+ BOOST_TEST(lastError.getCode() == ValidationError::LOOP_DETECTED);
+ BOOST_TEST(face.sentInterests.size() == 1);
face.sentInterests.clear();
processInterest = nullptr; // disable data responses from mocked network
VALIDATE_FAILURE(data, "Should fail again, but no network operations expected");
- BOOST_CHECK_EQUAL(face.sentInterests.size(), 0);
+ BOOST_TEST(lastError.getCode() == ValidationError::LOOP_DETECTED);
+ BOOST_TEST(face.sentInterests.size() == 0);
face.sentInterests.clear();
advanceClocks(10_min, 2); // expire untrusted cache
VALIDATE_FAILURE(data, "Should try and fail to retrieve certs");
- BOOST_CHECK_GT(face.sentInterests.size(), 1);
- face.sentInterests.clear();
+ BOOST_TEST(lastError.getCode() == ValidationError::CANNOT_RETRIEVE_CERT);
+ BOOST_TEST(face.sentInterests.size() > 1);
}
class ValidationPolicySimpleHierarchyForInterestOnly : public ValidationPolicySimpleHierarchy
@@ -209,41 +256,45 @@
HierarchicalValidatorFixture<ValidationPolicySimpleHierarchyForInterestOnly>)
{
Interest interest("/Security/ValidatorFixture/Sub1/Sub2/Interest");
- Data data("/Security/ValidatorFixture/Sub1/Sub2/Interest");
+ Data data("/Security/ValidatorFixture/Sub1/Sub2/Data");
VALIDATE_FAILURE(interest, "Unsigned");
- VALIDATE_SUCCESS(data, "Policy requests validation bypassing for all data");
- BOOST_CHECK_EQUAL(face.sentInterests.size(), 0);
+ BOOST_TEST(lastError.getCode() == ValidationError::NO_SIGNATURE);
+ VALIDATE_SUCCESS(data, "Policy bypasses validation for all data");
+ BOOST_TEST(face.sentInterests.size() == 0);
face.sentInterests.clear();
interest = Interest("/Security/ValidatorFixture/Sub1/Sub2/Interest");
m_keyChain.sign(interest, signingWithSha256());
m_keyChain.sign(data, signingWithSha256());
VALIDATE_FAILURE(interest, "Required KeyLocator/Name missing (not passed to policy)");
- VALIDATE_SUCCESS(data, "Policy requests validation bypassing for all data");
- BOOST_CHECK_EQUAL(face.sentInterests.size(), 0);
+ BOOST_TEST(lastError.getCode() == ValidationError::POLICY_ERROR);
+ VALIDATE_SUCCESS(data, "Policy bypasses validation for all data");
+ BOOST_TEST(face.sentInterests.size() == 0);
face.sentInterests.clear();
m_keyChain.sign(interest, signingByIdentity(identity));
m_keyChain.sign(data, signingByIdentity(identity));
VALIDATE_SUCCESS(interest, "Should get accepted, as signed by the anchor");
- VALIDATE_SUCCESS(data, "Policy requests validation bypassing for all data");
- BOOST_CHECK_EQUAL(face.sentInterests.size(), 0);
+ VALIDATE_SUCCESS(data, "Policy bypasses validation for all data");
+ BOOST_TEST(face.sentInterests.size() == 0);
face.sentInterests.clear();
m_keyChain.sign(interest, signingByIdentity(subIdentity));
m_keyChain.sign(data, signingByIdentity(subIdentity));
VALIDATE_FAILURE(interest, "Should fail, as policy is not allowed to create new trust anchors");
- VALIDATE_SUCCESS(data, "Policy requests validation bypassing for all data");
- BOOST_CHECK_EQUAL(face.sentInterests.size(), 1);
+ BOOST_TEST(lastError.getCode() == ValidationError::POLICY_ERROR);
+ VALIDATE_SUCCESS(data, "Policy bypasses validation for all data");
+ BOOST_TEST(face.sentInterests.size() == 1);
face.sentInterests.clear();
m_keyChain.sign(interest, signingByIdentity(otherIdentity));
m_keyChain.sign(data, signingByIdentity(otherIdentity));
VALIDATE_FAILURE(interest, "Should fail, as signed by the policy-violating cert");
- VALIDATE_SUCCESS(data, "Policy requests validation bypassing for all data");
+ BOOST_TEST(lastError.getCode() == ValidationError::POLICY_ERROR);
+ VALIDATE_SUCCESS(data, "Policy bypasses validation for all data");
// no network operations expected, as certificate is not validated by the policy
- BOOST_CHECK_EQUAL(face.sentInterests.size(), 0);
+ BOOST_TEST(face.sentInterests.size() == 0);
face.sentInterests.clear();
advanceClocks(1_h, 2); // expire trusted cache
@@ -251,8 +302,9 @@
m_keyChain.sign(interest, signingByIdentity(subSelfSignedIdentity));
m_keyChain.sign(data, signingByIdentity(subSelfSignedIdentity));
VALIDATE_FAILURE(interest, "Should fail, as policy is not allowed to create new trust anchors");
- VALIDATE_SUCCESS(data, "Policy requests validation bypassing for all data");
- BOOST_CHECK_EQUAL(face.sentInterests.size(), 1);
+ BOOST_TEST(lastError.getCode() == ValidationError::POLICY_ERROR);
+ VALIDATE_SUCCESS(data, "Policy bypasses validation for all data");
+ BOOST_TEST(face.sentInterests.size() == 1);
face.sentInterests.clear();
}
@@ -272,13 +324,15 @@
};
Data data("/Security/ValidatorFixture/Sub1/Sub2/Data");
- m_keyChain.sign(data, signingByIdentity(subIdentity).setSignatureInfo(
- SignatureInfo().setKeyLocator(subIdentity.getDefaultKey().getName())));
+ m_keyChain.sign(data, signingByIdentity(subIdentity)
+ .setSignatureInfo(SignatureInfo()
+ .setKeyLocator(subIdentity.getDefaultKey().getName())));
validator.setMaxDepth(40);
BOOST_CHECK_EQUAL(validator.getMaxDepth(), 40);
VALIDATE_FAILURE(data, "Should fail, as certificate should be looped");
- BOOST_CHECK_EQUAL(face.sentInterests.size(), 40);
+ BOOST_TEST(lastError.getCode() == ValidationError::EXCEEDED_DEPTH_LIMIT);
+ BOOST_TEST(face.sentInterests.size() == 40);
face.sentInterests.clear();
advanceClocks(1_h, 5); // expire caches
@@ -286,7 +340,8 @@
validator.setMaxDepth(30);
BOOST_CHECK_EQUAL(validator.getMaxDepth(), 30);
VALIDATE_FAILURE(data, "Should fail, as certificate chain is infinite");
- BOOST_CHECK_EQUAL(face.sentInterests.size(), 30);
+ BOOST_TEST(lastError.getCode() == ValidationError::EXCEEDED_DEPTH_LIMIT);
+ BOOST_TEST(face.sentInterests.size() == 30);
}
BOOST_AUTO_TEST_CASE(LoopedCertChain)
@@ -313,7 +368,8 @@
Data data("/loop/Data");
m_keyChain.sign(data, signingByKey(k1));
VALIDATE_FAILURE(data, "Should fail, as certificate chain loops");
- BOOST_REQUIRE_EQUAL(face.sentInterests.size(), 3);
+ BOOST_TEST(lastError.getCode() == ValidationError::LOOP_DETECTED);
+ BOOST_TEST_REQUIRE(face.sentInterests.size() == 3);
BOOST_CHECK_EQUAL(face.sentInterests[0].getName(), k1.getDefaultCertificate().getName());
BOOST_CHECK_EQUAL(face.sentInterests[1].getName(), k2.getName());
BOOST_CHECK_EQUAL(face.sentInterests[2].getName(), k3.getName());