mgmt: finalize migration to the new signed Interest format
Change-Id: I6d3fe1bd092fa80e185d36746a42c188ab00a60e
diff --git a/daemon/mgmt/command-authenticator.cpp b/daemon/mgmt/command-authenticator.cpp
index 9f24ba3..17651e9 100644
--- a/daemon/mgmt/command-authenticator.cpp
+++ b/daemon/mgmt/command-authenticator.cpp
@@ -44,11 +44,13 @@
// INFO: configuration change, etc
// DEBUG: per authentication request result
-/** \brief an Interest tag to indicate command signer
+/**
+ * \brief An Interest tag to store the command signer.
*/
using SignerTag = ndn::SimpleTag<Name, 20>;
-/** \brief obtain signer from SignerTag attached to Interest, if available
+/**
+ * \brief Obtain signer from a SignerTag attached to \p interest, if available.
*/
static std::optional<std::string>
getSignerFromTag(const Interest& interest)
@@ -62,7 +64,8 @@
}
}
-/** \brief a validation policy that only permits Interest signed by a trust anchor
+/**
+ * \brief A validation policy that only permits Interests signed by a trust anchor.
*/
class CommandAuthenticatorValidationPolicy final : public security::ValidationPolicy
{
@@ -71,7 +74,11 @@
checkPolicy(const Interest& interest, const shared_ptr<security::ValidationState>& state,
const ValidationContinuation& continueValidation) final
{
- 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;
}
@@ -205,11 +212,10 @@
{
m_validators[module]; // declares module, so that privilege is recognized
- auto self = this->shared_from_this();
- return [=] (const Name&, const Interest& interest,
- const ndn::mgmt::ControlParameters*,
- const ndn::mgmt::AcceptContinuation& accept,
- const ndn::mgmt::RejectContinuation& reject) {
+ return [module, self = shared_from_this()] (const Name&, const Interest& interest,
+ const ndn::mgmt::ControlParameters*,
+ const ndn::mgmt::AcceptContinuation& accept,
+ const ndn::mgmt::RejectContinuation& reject) {
auto validator = self->m_validators.at(module);
auto successCb = [accept, validator] (const Interest& interest1) {
@@ -221,19 +227,13 @@
accept(signer);
};
- auto failureCb = [reject] (const Interest& interest1, const security::ValidationError& err) {
- using ndn::mgmt::RejectReply;
- RejectReply reply = RejectReply::STATUS403;
- switch (err.getCode()) {
- case security::ValidationError::NO_SIGNATURE:
- case security::ValidationError::INVALID_KEY_LOCATOR:
- reply = RejectReply::SILENT;
- break;
- case security::ValidationError::POLICY_ERROR:
- if (interest1.getName().size() < ndn::command_interest::MIN_SIZE) { // "name too short"
- reply = RejectReply::SILENT;
- }
- break;
+ using ndn::security::ValidationError;
+ auto failureCb = [reject] (const Interest& interest1, const ValidationError& err) {
+ auto reply = ndn::mgmt::RejectReply::STATUS403;
+ if (err.getCode() == ValidationError::MALFORMED_SIGNATURE ||
+ err.getCode() == ValidationError::INVALID_KEY_LOCATOR) {
+ // do not waste cycles signing and sending a reply if the command is clearly malformed
+ reply = ndn::mgmt::RejectReply::SILENT;
}
NFD_LOG_DEBUG("reject " << interest1.getName() << " signer=" <<
getSignerFromTag(interest1).value_or("?") << " reason=" << err);
diff --git a/tests/daemon/mgmt/command-authenticator.t.cpp b/tests/daemon/mgmt/command-authenticator.t.cpp
index 635ee12..6ef3ce8 100644
--- a/tests/daemon/mgmt/command-authenticator.t.cpp
+++ b/tests/daemon/mgmt/command-authenticator.t.cpp
@@ -51,7 +51,7 @@
bool
authorize(const std::string& module, const Name& identity,
const std::function<void(Interest&)>& modifyInterest = nullptr,
- ndn::security::SignedInterestFormat format = ndn::security::SignedInterestFormat::V02)
+ ndn::security::SignedInterestFormat format = ndn::security::SignedInterestFormat::V03)
{
Interest interest = makeControlCommandRequest(Name("/prefix/" + module + "/verb"),
{}, format, identity);
@@ -252,9 +252,15 @@
}
bool
- authorize1(const std::function<void(Interest&)>& modifyInterest)
+ authorize1_V02(const std::function<void(Interest&)>& modifyInterest)
{
- return authorize("module1", id1, modifyInterest);
+ return authorize("module1", id1, modifyInterest, ndn::security::SignedInterestFormat::V02);
+ }
+
+ bool
+ authorize1_V03(const std::function<void(Interest&)>& modifyInterest)
+ {
+ return authorize("module1", id1, modifyInterest, ndn::security::SignedInterestFormat::V03);
}
protected:
@@ -263,9 +269,9 @@
BOOST_FIXTURE_TEST_SUITE(Reject, IdentityAuthorizedFixture)
-BOOST_AUTO_TEST_CASE(BadKeyLocator_NameTooShort)
+BOOST_AUTO_TEST_CASE(NameTooShort)
{
- BOOST_CHECK_EQUAL(authorize1(
+ BOOST_CHECK_EQUAL(authorize1_V02(
[] (Interest& interest) {
interest.setName("/prefix");
}
@@ -275,9 +281,18 @@
BOOST_AUTO_TEST_CASE(BadSigInfo)
{
- BOOST_CHECK_EQUAL(authorize1(
+ BOOST_CHECK_EQUAL(authorize1_V02(
[] (Interest& interest) {
- setNameComponent(interest, ndn::signed_interest::POS_SIG_INFO, "not-SignatureInfo");
+ setNameComponent(interest, ndn::command_interest::POS_SIG_INFO, "not-sig-info");
+ }
+ ), false);
+ BOOST_CHECK(lastRejectReply == ndn::mgmt::RejectReply::SILENT);
+
+ BOOST_CHECK_EQUAL(authorize1_V03(
+ [] (Interest& interest) {
+ auto sigInfo = interest.getSignatureInfo().value();
+ sigInfo.addCustomTlv("7F00"_block);
+ interest.setSignatureInfo(sigInfo);
}
), false);
BOOST_CHECK(lastRejectReply == ndn::mgmt::RejectReply::SILENT);
@@ -285,10 +300,20 @@
BOOST_AUTO_TEST_CASE(MissingKeyLocator)
{
- BOOST_CHECK_EQUAL(authorize1(
+ BOOST_CHECK_EQUAL(authorize1_V02(
[] (Interest& interest) {
- ndn::SignatureInfo sigInfo(tlv::SignatureSha256WithRsa);
- setNameComponent(interest, ndn::signed_interest::POS_SIG_INFO, ndn::make_span(sigInfo.wireEncode()));
+ ndn::SignatureInfo sigInfo(interest.getName().at(ndn::command_interest::POS_SIG_INFO).blockFromValue());
+ sigInfo.setKeyLocator(ndn::nullopt);
+ setNameComponent(interest, ndn::command_interest::POS_SIG_INFO, span(sigInfo.wireEncode()));
+ }
+ ), false);
+ BOOST_CHECK(lastRejectReply == ndn::mgmt::RejectReply::SILENT);
+
+ BOOST_CHECK_EQUAL(authorize1_V03(
+ [] (Interest& interest) {
+ auto sigInfo = interest.getSignatureInfo().value();
+ sigInfo.setKeyLocator(ndn::nullopt);
+ interest.setSignatureInfo(sigInfo);
}
), false);
BOOST_CHECK(lastRejectReply == ndn::mgmt::RejectReply::SILENT);
@@ -296,17 +321,92 @@
BOOST_AUTO_TEST_CASE(BadKeyLocatorType)
{
- BOOST_CHECK_EQUAL(authorize1(
- [] (Interest& interest) {
- ndn::KeyLocator kl;
- kl.setKeyDigest(ndn::makeBinaryBlock(tlv::KeyDigest,
- {0xDD, 0xDD, 0xDD, 0xDD, 0xDD, 0xDD, 0xDD, 0xDD}));
- ndn::SignatureInfo sigInfo(tlv::SignatureSha256WithRsa);
- sigInfo.setKeyLocator(kl);
- setNameComponent(interest, ndn::signed_interest::POS_SIG_INFO, ndn::make_span(sigInfo.wireEncode()));
+ ndn::KeyLocator kl;
+ kl.setKeyDigest(ndn::makeBinaryBlock(tlv::KeyDigest, {0xDD, 0xDD, 0xDD, 0xDD, 0xDD, 0xDD}));
+
+ BOOST_CHECK_EQUAL(authorize1_V02(
+ [&kl] (Interest& interest) {
+ ndn::SignatureInfo sigInfo(tlv::SignatureSha256WithEcdsa, kl);
+ setNameComponent(interest, ndn::command_interest::POS_SIG_INFO, span(sigInfo.wireEncode()));
}
), false);
BOOST_CHECK(lastRejectReply == ndn::mgmt::RejectReply::SILENT);
+
+ BOOST_CHECK_EQUAL(authorize1_V03(
+ [&kl] (Interest& interest) {
+ auto sigInfo = interest.getSignatureInfo().value();
+ sigInfo.setKeyLocator(kl);
+ interest.setSignatureInfo(sigInfo);
+ }
+ ), false);
+ BOOST_CHECK(lastRejectReply == ndn::mgmt::RejectReply::SILENT);
+}
+
+BOOST_AUTO_TEST_CASE(BadSigValue)
+{
+ BOOST_CHECK_EQUAL(authorize1_V02(
+ [] (Interest& interest) {
+ setNameComponent(interest, ndn::command_interest::POS_SIG_VALUE, "bad-signature");
+ }
+ ), false);
+ BOOST_CHECK(lastRejectReply == ndn::mgmt::RejectReply::STATUS403);
+
+ BOOST_CHECK_EQUAL(authorize1_V03(
+ [] (Interest& interest) {
+ interest.setSignatureValue({0xBA, 0xAD});
+ }
+ ), false);
+ BOOST_CHECK(lastRejectReply == ndn::mgmt::RejectReply::STATUS403);
+}
+
+BOOST_AUTO_TEST_CASE(MissingTimestamp)
+{
+ BOOST_CHECK_EQUAL(authorize1_V02(
+ [] (Interest& interest) {
+ setNameComponent(interest, ndn::command_interest::POS_TIMESTAMP, "not-timestamp");
+ }
+ ), false);
+ BOOST_CHECK(lastRejectReply == ndn::mgmt::RejectReply::STATUS403);
+
+ BOOST_CHECK_EQUAL(authorize1_V03(
+ [] (Interest& interest) {
+ auto sigInfo = interest.getSignatureInfo().value();
+ sigInfo.setTime(ndn::nullopt);
+ interest.setSignatureInfo(sigInfo);
+ }
+ ), false);
+ BOOST_CHECK(lastRejectReply == ndn::mgmt::RejectReply::STATUS403);
+}
+
+BOOST_AUTO_TEST_CASE(ReplayedTimestamp)
+{
+ name::Component timestampComp;
+ BOOST_CHECK_EQUAL(authorize1_V02(
+ [×tampComp] (const Interest& interest) {
+ timestampComp = interest.getName().at(ndn::command_interest::POS_TIMESTAMP);
+ }
+ ), true); // accept first command
+ BOOST_CHECK_EQUAL(authorize1_V02(
+ [×tampComp] (Interest& interest) {
+ setNameComponent(interest, ndn::command_interest::POS_TIMESTAMP, timestampComp);
+ }
+ ), false); // reject second command because timestamp equals first command
+ BOOST_CHECK(lastRejectReply == ndn::mgmt::RejectReply::STATUS403);
+
+ time::system_clock::time_point tp;
+ BOOST_CHECK_EQUAL(authorize1_V03(
+ [&tp] (const Interest& interest) {
+ tp = interest.getSignatureInfo().value().getTime().value();
+ }
+ ), true); // accept first command
+ BOOST_CHECK_EQUAL(authorize1_V03(
+ [&tp] (Interest& interest) {
+ auto sigInfo = interest.getSignatureInfo().value();
+ sigInfo.setTime(tp);
+ interest.setSignatureInfo(sigInfo);
+ }
+ ), false); // reject second command because timestamp equals first command
+ BOOST_CHECK(lastRejectReply == ndn::mgmt::RejectReply::STATUS403);
}
BOOST_AUTO_TEST_CASE(NotAuthorized)
@@ -318,32 +418,6 @@
BOOST_CHECK(lastRejectReply == ndn::mgmt::RejectReply::STATUS403);
}
-BOOST_AUTO_TEST_CASE(BadSig)
-{
- BOOST_CHECK_EQUAL(authorize1(
- [] (Interest& interest) {
- setNameComponent(interest, ndn::command_interest::POS_SIG_VALUE, "bad-signature-bits");
- }
- ), false);
- BOOST_CHECK(lastRejectReply == ndn::mgmt::RejectReply::STATUS403);
-}
-
-BOOST_AUTO_TEST_CASE(InvalidTimestamp)
-{
- name::Component timestampComp;
- BOOST_CHECK_EQUAL(authorize1(
- [×tampComp] (const Interest& interest) {
- timestampComp = interest.getName().at(ndn::command_interest::POS_TIMESTAMP);
- }
- ), true); // accept first command
- BOOST_CHECK_EQUAL(authorize1(
- [×tampComp] (Interest& interest) {
- setNameComponent(interest, ndn::command_interest::POS_TIMESTAMP, timestampComp);
- }
- ), false); // reject second command because timestamp equals first command
- BOOST_CHECK(lastRejectReply == ndn::mgmt::RejectReply::STATUS403);
-}
-
BOOST_FIXTURE_TEST_CASE(MissingAuthorizationsSection, CommandAuthenticatorFixture)
{
Name id0("/localhost/CommandAuthenticator/0");
diff --git a/tests/daemon/mgmt/rib-manager.t.cpp b/tests/daemon/mgmt/rib-manager.t.cpp
index ecdf8b2..8ae96ae 100644
--- a/tests/daemon/mgmt/rib-manager.t.cpp
+++ b/tests/daemon/mgmt/rib-manager.t.cpp
@@ -159,7 +159,7 @@
m_face.onSendInterest.connect([=] (const Interest& interest) {
if (interest.matchesData(derivedCert) &&
m_status.isLocalhopConfigured &&
- interest.template getTag<lp::NextHopFaceIdTag>() != nullptr) {
+ interest.getTag<lp::NextHopFaceIdTag>() != nullptr) {
m_face.put(derivedCert);
}
});
@@ -257,7 +257,7 @@
BOOST_FIXTURE_TEST_CASE(AddTopPrefix, AddTopPrefixFixture)
{
- BOOST_CHECK_EQUAL(m_rib.size(), 2);
+ BOOST_REQUIRE_EQUAL(m_rib.size(), 2);
std::vector<Name> ribEntryNames;
for (auto&& entry : m_rib) {
@@ -303,22 +303,32 @@
}
};
+template<typename Fixture, auto Format>
+struct FixtureWithFormat : public Fixture
+{
+ static constexpr ndn::security::SignedInterestFormat signedInterestFmt = Format;
+};
+
using AllFixtures = boost::mpl::vector<
- UnauthorizedRibManagerFixture,
- LocalhostAuthorizedRibManagerFixture,
- LocalhopAuthorizedRibManagerFixture,
- AuthorizedRibManagerFixture
+ FixtureWithFormat<UnauthorizedRibManagerFixture, ndn::security::SignedInterestFormat::V02>,
+ FixtureWithFormat<UnauthorizedRibManagerFixture, ndn::security::SignedInterestFormat::V03>,
+ FixtureWithFormat<LocalhostAuthorizedRibManagerFixture, ndn::security::SignedInterestFormat::V02>,
+ FixtureWithFormat<LocalhostAuthorizedRibManagerFixture, ndn::security::SignedInterestFormat::V03>,
+ FixtureWithFormat<LocalhopAuthorizedRibManagerFixture, ndn::security::SignedInterestFormat::V02>,
+ FixtureWithFormat<LocalhopAuthorizedRibManagerFixture, ndn::security::SignedInterestFormat::V03>,
+ FixtureWithFormat<AuthorizedRibManagerFixture, ndn::security::SignedInterestFormat::V02>,
+ FixtureWithFormat<AuthorizedRibManagerFixture, ndn::security::SignedInterestFormat::V03>
>;
BOOST_FIXTURE_TEST_CASE_TEMPLATE(CommandAuthorization, T, AllFixtures, T)
{
auto parameters = this->makeRegisterParameters("/test-authorization", 9527);
- auto commandHost = this->makeControlCommandRequest("/localhost/nfd/rib/register", parameters);
+ auto commandHost = this->makeControlCommandRequest("/localhost/nfd/rib/register", parameters,
+ T::signedInterestFmt);
auto commandHop = this->makeControlCommandRequest("/localhop/nfd/rib/register", parameters,
- ndn::security::SignedInterestFormat::V03,
- this->m_derivedId);
+ T::signedInterestFmt, this->m_derivedId);
if (this->m_status.isLocalhopConfigured) {
- commandHop.setTag(make_shared<lp::IncomingFaceIdTag>(123));
+ commandHop.setTag(std::make_shared<lp::IncomingFaceIdTag>(123));
}
auto successResp = this->makeResponse(200, "Success", parameters);
auto failureResp = ControlResponse(403, "authorization rejected");