**breaking change**: Switch Face and related classes to v2::KeyChain

security::v2::KeyChain is now exposed as ndn::KeyChain, which should
ensure that dependent code can be mostly compiled.  However, expect code
that explicitly uses the old KeyChain interface to be broken.

Change-Id: I7330d0250d92f3f0f2570ab6d0214ab3dfdd18cc
Refs: #3098

src/mgmt/dispatcher.cpp

@@ -42,7 +42,7 @@ makeAcceptAllAuthorization()
   };
 }
 
-Dispatcher::Dispatcher(Face& face, security::v1::KeyChain& keyChain,
+Dispatcher::Dispatcher(Face& face, KeyChain& keyChain,
                        const security::SigningInfo& signingInfo,
                        size_t imsCapacity)
   : m_face(face)

src/mgmt/dispatcher.hpp

@@ -145,7 +145,7 @@ class Dispatcher : noncopyable
    *  \param signingInfo signing parameters to sign Data with \p keyChain
    *  \param imsCapacity capacity of the internal InMemoryStorage used by dispatcher
    */
-  Dispatcher(Face& face, security::v1::KeyChain& keyChain,
+  Dispatcher(Face& face, KeyChain& keyChain,
              const security::SigningInfo& signingInfo = security::SigningInfo(),
              size_t imsCapacity = 256);
 
@@ -462,7 +462,7 @@ class Dispatcher : noncopyable
   std::unordered_map<Name, TopPrefixEntry> m_topLevelPrefixes;
 
   Face& m_face;
-  security::v1::KeyChain& m_keyChain;
+  KeyChain& m_keyChain;
   security::SigningInfo m_signingInfo;
 
   typedef std::unordered_map<PartialName, InterestHandler> HandlerMap;

src/mgmt/nfd/controller.cpp

@@ -36,9 +36,10 @@ const uint32_t Controller::ERROR_SERVER = 500;
 const uint32_t Controller::ERROR_LBOUND = 400;
 ValidatorNull Controller::s_validatorNull;
 
-Controller::Controller(Face& face, security::v1::KeyChain& keyChain, Validator& validator)
+Controller::Controller(Face& face, KeyChain& keyChain, Validator& validator)
   : m_face(face)
   , m_keyChain(keyChain)
+  , m_signer(keyChain)
   , m_validator(validator)
 {
 }
@@ -56,9 +57,8 @@ Controller::startCommand(const shared_ptr<ControlCommand>& command,
     onFailure1 : [] (const ControlResponse&) {};
 
   Name requestName = command->getRequestName(options.getPrefix(), parameters);
-  Interest interest(requestName);
+  Interest interest = m_signer.makeCommandInterest(requestName, options.getSigningInfo());
   interest.setInterestLifetime(options.getTimeout());
-  m_keyChain.sign(interest, options.getSigningInfo());
 
   m_face.expressInterest(interest,
     [=] (const Interest&, const Data& data) {

src/mgmt/nfd/controller.hpp

@@ -28,6 +28,7 @@
 #include "command-options.hpp"
 #include "../../security/validator-null.hpp"
 #include "../../security/key-chain.hpp"
+#include "../../security/command-interest-signer.hpp"
 
 namespace ndn {
 
@@ -66,7 +67,7 @@ class Controller : noncopyable
   /** \brief construct a Controller that uses face for transport,
    *         and uses the passed KeyChain to sign commands
    */
-  Controller(Face& face, security::v1::KeyChain& keyChain, security::Validator& validator = s_validatorNull);
+  Controller(Face& face, KeyChain& keyChain, security::Validator& validator = s_validatorNull);
 
   /** \brief start command execution
    */
@@ -170,7 +171,8 @@ class Controller : noncopyable
 
 protected:
   Face& m_face;
-  security::v1::KeyChain& m_keyChain;
+  KeyChain& m_keyChain;
+  security::CommandInterestSigner m_signer;
   security::Validator& m_validator;
 
 private:

src/security/command-interest-signer.cpp

@@ -47,7 +47,7 @@ CommandInterestPreparer::prepareCommandInterestName(Name name)
   return name;
 }
 
-CommandInterestSigner::CommandInterestSigner(v2::KeyChain& keyChain)
+CommandInterestSigner::CommandInterestSigner(KeyChain& keyChain)
   : m_keyChain(keyChain)
 {
 }

src/security/command-interest-signer.hpp

@@ -67,7 +67,7 @@ class CommandInterestSigner : private CommandInterestPreparer
 {
 public:
   explicit
-  CommandInterestSigner(v2::KeyChain& keyChain);
+  CommandInterestSigner(KeyChain& keyChain);
 
   /**
    * @brief Create CommandInterest
@@ -82,10 +82,10 @@ class CommandInterestSigner : private CommandInterestPreparer
    * @sa https://redmine.named-data.net/projects/ndn-cxx/wiki/CommandInterest
    */
   Interest
-  makeCommandInterest(const Name& name, const SigningInfo& params = v2::KeyChain::getDefaultSigningInfo());
+  makeCommandInterest(const Name& name, const SigningInfo& params = KeyChain::getDefaultSigningInfo());
 
 private:
-  v2::KeyChain& m_keyChain;
+  KeyChain& m_keyChain;
 };
 
 } // namespace security

src/security/command-interest-validator.cpp

@@ -20,7 +20,6 @@
  */
 
 #include "command-interest-validator.hpp"
-#include "v1/identity-certificate.hpp"
 #include <boost/lexical_cast.hpp>
 
 namespace ndn {
@@ -146,12 +145,14 @@ CommandInterestValidator::parseCommandInterest(const Interest& interest, Name& k
   }
 
   try {
-    keyName = v1::IdentityCertificate::certificateNameToPublicKeyName(keyLocator.getName());
+    v2::extractIdentityFromKeyName(keyLocator.getName());
   }
-  catch (const v1::IdentityCertificate::Error&) {
+  catch (const std::invalid_argument&) {
     return ErrorCode::BAD_CERT_NAME;
   }
 
+  keyName = keyLocator.getName();
+
   return ErrorCode::NONE;
 }
 

src/security/key-chain.hpp

@@ -19,21 +19,10 @@
  * See AUTHORS.md for complete list of ndn-cxx authors and contributors.
  */
 
-/**
- * @file security/key-chain.hpp
- */
+#ifndef NDN_CXX_SECURITY_KEY_CHAIN_HPP
+#define NDN_CXX_SECURITY_KEY_CHAIN_HPP
 
 #include "security-common.hpp"
-#include "v1/key-chain.hpp"
 #include "v2/key-chain.hpp"
 
-namespace ndn {
-namespace security {
-
-using security::v1::KeyChain;
-
-} // namespace security
-
-using ndn::security::KeyChain;
-
-} // namespace ndn
+#endif // NDN_CXX_SECURITY_KEY_CHAIN_HPP

src/security/v2/key-chain.hpp

@@ -503,6 +503,9 @@ public:                                                       \
 
 } // namespace v2
 } // namespace security
+
+using security::v2::KeyChain;
+
 } // namespace ndn
 
 #endif // NDN_SECURITY_V2_KEY_CHAIN_HPP

src/util/dummy-client-face.cpp

@@ -86,13 +86,13 @@ class DummyClientFace::Transport : public ndn::Transport
 
 DummyClientFace::DummyClientFace(const Options& options/* = DummyClientFace::DEFAULT_OPTIONS*/)
   : Face(make_shared<DummyClientFace::Transport>())
-  , m_internalKeyChain(new security::v1::KeyChain)
+  , m_internalKeyChain(new KeyChain)
   , m_keyChain(*m_internalKeyChain)
 {
   this->construct(options);
 }
 
-DummyClientFace::DummyClientFace(security::v1::KeyChain& keyChain,
+DummyClientFace::DummyClientFace(KeyChain& keyChain,
                                  const Options& options/* = DummyClientFace::DEFAULT_OPTIONS*/)
   : Face(make_shared<DummyClientFace::Transport>(), keyChain)
   , m_keyChain(keyChain)
@@ -103,13 +103,13 @@ DummyClientFace::DummyClientFace(security::v1::KeyChain& keyChain,
 DummyClientFace::DummyClientFace(boost::asio::io_service& ioService,
                                  const Options& options/* = DummyClientFace::DEFAULT_OPTIONS*/)
   : Face(make_shared<DummyClientFace::Transport>(), ioService)
-  , m_internalKeyChain(new security::v1::KeyChain)
+  , m_internalKeyChain(new KeyChain)
   , m_keyChain(*m_internalKeyChain)
 {
   this->construct(options);
 }
 
-DummyClientFace::DummyClientFace(boost::asio::io_service& ioService, security::v1::KeyChain& keyChain,
+DummyClientFace::DummyClientFace(boost::asio::io_service& ioService, KeyChain& keyChain,
                                  const Options& options/* = DummyClientFace::DEFAULT_OPTIONS*/)
   : Face(make_shared<DummyClientFace::Transport>(), ioService, keyChain)
   , m_keyChain(keyChain)

src/util/dummy-client-face.hpp

@@ -80,7 +80,7 @@ class DummyClientFace : public ndn::Face
   /** \brief Create a dummy face with internal IO service and the specified KeyChain
    */
   explicit
-  DummyClientFace(security::v1::KeyChain& keyChain, const Options& options = Options());
+  DummyClientFace(KeyChain& keyChain, const Options& options = Options());
 
   /** \brief Create a dummy face with the provided IO service
    */
@@ -89,7 +89,7 @@ class DummyClientFace : public ndn::Face
 
   /** \brief Create a dummy face with the provided IO service and the specified KeyChain
    */
-  DummyClientFace(boost::asio::io_service& ioService, security::v1::KeyChain& keyChain,
+  DummyClientFace(boost::asio::io_service& ioService, KeyChain& keyChain,
                   const Options& options = Options());
 
   /** \brief cause the Face to receive an interest
@@ -166,8 +166,8 @@ class DummyClientFace : public ndn::Face
   Signal<DummyClientFace, lp::Nack> onSendNack;
 
 private:
-  std::unique_ptr<security::v1::KeyChain> m_internalKeyChain;
-  security::v1::KeyChain& m_keyChain;
+  std::unique_ptr<KeyChain> m_internalKeyChain;
+  KeyChain& m_keyChain;
   std::function<void(time::milliseconds)> m_processEventsOverride;
 };
 

src/util/notification-stream.hpp

@@ -30,7 +30,7 @@
 
 #include "../name.hpp"
 #include "../face.hpp"
-#include "../security/v1/key-chain.hpp"
+#include "../security/v2/key-chain.hpp"
 
 #include "concepts.hpp"
 

tests/unit-tests/face.t.cpp

@@ -36,7 +36,7 @@ namespace tests {
 
 using ndn::util::DummyClientFace;
 
-class FaceFixture : public IdentityManagementV1TimeFixture
+class FaceFixture : public IdentityManagementTimeFixture
 {
 public:
   explicit
@@ -648,21 +648,18 @@ struct PibDirWithDefaultTpm
   const std::string PATH = "build/keys-with-default-tpm";
 };
 
-BOOST_FIXTURE_TEST_CASE(FaceTransport, PibDirFixture<PibDirWithDefaultTpm>)
+BOOST_FIXTURE_TEST_CASE(FaceTransport, IdentityManagementTimeFixture)
 {
-  KeyChain keyChain;
-  boost::asio::io_service io;
-
   BOOST_CHECK(Face().getTransport() != nullptr);
 
   BOOST_CHECK(Face(shared_ptr<Transport>()).getTransport() != nullptr);
   BOOST_CHECK(Face(shared_ptr<Transport>(), io).getTransport() != nullptr);
-  BOOST_CHECK(Face(shared_ptr<Transport>(), io, keyChain).getTransport() != nullptr);
+  BOOST_CHECK(Face(shared_ptr<Transport>(), io, m_keyChain).getTransport() != nullptr);
 
   auto transport = make_shared<TcpTransport>("localhost", "6363"); // no real io operations will be scheduled
   BOOST_CHECK(Face(transport).getTransport() == transport);
   BOOST_CHECK(Face(transport, io).getTransport() == transport);
-  BOOST_CHECK(Face(transport, io, keyChain).getTransport() == transport);
+  BOOST_CHECK(Face(transport, io, m_keyChain).getTransport() == transport);
 }
 
 class WithEnv : private IdentityManagementTimeFixture

tests/unit-tests/mgmt/dispatcher.t.cpp

@@ -34,7 +34,7 @@ namespace tests {
 
 using namespace ndn::tests;
 
-class DispatcherFixture : public IdentityManagementV1TimeFixture
+class DispatcherFixture : public IdentityManagementTimeFixture
 {
 public:
   DispatcherFixture()

tests/unit-tests/mgmt/nfd/controller-fixture.hpp

@@ -35,7 +35,7 @@ namespace tests {
 
 using namespace ndn::tests;
 
-class ControllerFixture : public IdentityManagementV1TimeFixture
+class ControllerFixture : public IdentityManagementTimeFixture
 {
 protected:
   ControllerFixture()
@@ -45,8 +45,7 @@ class ControllerFixture : public IdentityManagementV1TimeFixture
     , datasetFailCallback(bind(&ControllerFixture::recordDatasetFail, this, _1, _2))
   {
     Name identityName("/localhost/ControllerFixture");
-    this->addIdentity(identityName);
-    m_keyChain.setDefaultIdentity(identityName);
+    m_keyChain.setDefaultIdentity(this->addIdentity(identityName));
   }
 
   /** \brief controls whether Controller's validator should accept or reject validation requests

tests/unit-tests/security/command-interest-validator.t.cpp

@@ -20,6 +20,7 @@
  */
 
 #include "security/command-interest-validator.hpp"
+#include "security/command-interest-signer.hpp"
 #include "security/signing-helpers.hpp"
 
 #include "boost-test.hpp"
@@ -35,10 +36,11 @@ namespace tests {
 
 using namespace ndn::tests;
 
-class CommandInterestValidatorFixture : public IdentityManagementV1TimeFixture
+class CommandInterestValidatorFixture : public IdentityManagementTimeFixture
 {
 protected:
   CommandInterestValidatorFixture()
+    : signer(m_keyChain)
   {
     this->initialize(CommandInterestValidator::Options{});
   }
@@ -63,10 +65,8 @@ class CommandInterestValidatorFixture : public IdentityManagementV1TimeFixture
   shared_ptr<Interest>
   makeCommandInterest(uint64_t identity = 0)
   {
-    auto interest = makeInterest("/CommandInterestPrefix");
-    m_keyChain.sign(*interest, signingByIdentity(makeIdentity(identity)));
-    BOOST_TEST_MESSAGE("makeCommandInterest " << interest->getName());
-    return interest;
+    auto interest = signer.makeCommandInterest("/CommandInterestPrefix", signingByIdentity(makeIdentity(identity)));
+    return make_shared<Interest>(std::move(interest));
   }
 
   /** \brief check that validator accepts interest
@@ -75,7 +75,6 @@ class CommandInterestValidatorFixture : public IdentityManagementV1TimeFixture
   void
   assertAccept(const Interest& interest)
   {
-    BOOST_TEST_MESSAGE("assertAccept " << interest.getName());
     int nAccepts = 0;
     validator->validate(interest,
       [&nAccepts] (const shared_ptr<const Interest>&) { ++nAccepts; },
@@ -93,7 +92,6 @@ class CommandInterestValidatorFixture : public IdentityManagementV1TimeFixture
   void
   assertReject(const Interest& interest, CommandInterestValidator::ErrorCode error)
   {
-    BOOST_TEST_MESSAGE("assertReject " << interest.getName());
     int nRejects = 0;
     validator->validate(interest,
       [] (const shared_ptr<const Interest>&) {
@@ -109,6 +107,7 @@ class CommandInterestValidatorFixture : public IdentityManagementV1TimeFixture
   }
 
 protected:
+  CommandInterestSigner signer;
   DummyValidator* inner;
   unique_ptr<CommandInterestValidator> validator;
 };

tests/unit-tests/security/v2/key-chain.t.cpp

@@ -47,6 +47,12 @@ class TestHomeAndPibFixture : public TestHomeFixture<Path>
     unsetenv("NDN_CLIENT_PIB");
     unsetenv("NDN_CLIENT_TPM");
   }
+
+  ~TestHomeAndPibFixture()
+  {
+    const_cast<std::string&>(KeyChain::getDefaultPibLocator()).clear();
+    const_cast<std::string&>(KeyChain::getDefaultTpmLocator()).clear();
+  }
 };
 
 struct PibPathConfigFileHome