security: introduce KeyChain::makeCertificate

KeyChain::makeCertificate() captures a common routine of creating and
signing a certificate. Having it in the library allows deduplicating
similar code elsewhere.

Also add "find by certificate name" tests for CertificateCache and
TrustAnchorContainer.

refs #5112

Change-Id: I954587e1c03d6b372e3b4f04e702339d1ff1533e
diff --git a/ndn-cxx/security/certificate.cpp b/ndn-cxx/security/certificate.cpp
index 9b33cdd..3060fd3 100644
--- a/ndn-cxx/security/certificate.cpp
+++ b/ndn-cxx/security/certificate.cpp
@@ -42,6 +42,7 @@
 const size_t Certificate::MIN_CERT_NAME_LENGTH = 4;
 const size_t Certificate::MIN_KEY_NAME_LENGTH = 2;
 const name::Component Certificate::KEY_COMPONENT("KEY");
+const name::Component Certificate::DEFAULT_ISSUER_ID("NA");
 
 Certificate::Certificate()
 {
diff --git a/ndn-cxx/security/certificate.hpp b/ndn-cxx/security/certificate.hpp
index 698f64d..dbd3f75 100644
--- a/ndn-cxx/security/certificate.hpp
+++ b/ndn-cxx/security/certificate.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).
  *
@@ -149,6 +149,7 @@
   static const size_t MIN_CERT_NAME_LENGTH;
   static const size_t MIN_KEY_NAME_LENGTH;
   static const name::Component KEY_COMPONENT;
+  static const name::Component DEFAULT_ISSUER_ID;
 };
 
 std::ostream&
diff --git a/ndn-cxx/security/key-chain.cpp b/ndn-cxx/security/key-chain.cpp
index 8b07ab7..b412153 100644
--- a/ndn-cxx/security/key-chain.cpp
+++ b/ndn-cxx/security/key-chain.cpp
@@ -20,6 +20,7 @@
  */
 
 #include "ndn-cxx/security/key-chain.hpp"
+#include "ndn-cxx/security/signing-helpers.hpp"
 
 #include "ndn-cxx/encoding/buffer-stream.hpp"
 #include "ndn-cxx/util/config-file.hpp"
@@ -68,6 +69,7 @@
 
 NDN_LOG_INIT(ndn.security.KeyChain);
 
+const name::Component SELF("self");
 std::string KeyChain::s_defaultPibLocator;
 std::string KeyChain::s_defaultTpmLocator;
 
@@ -484,6 +486,29 @@
   }
 }
 
+Certificate
+KeyChain::makeCertificate(const pib::Key& publicKey, const SigningInfo& params,
+                          const MakeCertificateOptions& opts)
+{
+  return makeCertificate(publicKey.getName(), publicKey.getPublicKey(), params, opts);
+}
+
+Certificate
+KeyChain::makeCertificate(const Certificate& certRequest, const SigningInfo& params,
+                          const MakeCertificateOptions& opts)
+{
+  auto pkcs8 = certRequest.getContent().value_bytes();
+  try {
+    transform::PublicKey pub;
+    pub.loadPkcs8(pkcs8);
+  }
+  catch (const transform::PublicKey::Error& e) {
+    NDN_THROW_NESTED(std::invalid_argument("Certificate request contains invalid public key"));
+  }
+
+  return makeCertificate(extractKeyNameFromCertName(certRequest.getName()), pkcs8, params, opts);
+}
+
 // public: PIB/TPM creation helpers
 
 static inline std::tuple<std::string/*type*/, std::string/*location*/>
@@ -557,35 +582,52 @@
 // private: signing
 
 Certificate
+KeyChain::makeCertificate(const Name& keyName, span<const uint8_t> publicKey,
+                          SigningInfo params, const MakeCertificateOptions& opts)
+{
+  if (opts.freshnessPeriod <= 0_ms) {
+    // Certificate format requires FreshnessPeriod field to appear in the packet.
+    // We cannot rely on Certificate constructor to check this, because:
+    // (1) Metadata::wireEncode omits zero FreshnessPeriod in the wire encoding, but
+    //     Certificate constructor does not throw in this condition
+    // (2) Certificate constructor throws Data::Error, not a std::invalid_argument
+    NDN_THROW(std::invalid_argument("FreshnessPeriod is not positive"));
+  }
+
+  Name name(keyName);
+  name.append(opts.issuerId);
+  name.appendVersion(opts.version);
+
+  Data data;
+  data.setName(name);
+  data.setContentType(tlv::ContentType_Key);
+  data.setFreshnessPeriod(opts.freshnessPeriod);
+  data.setContent(publicKey);
+
+  auto sigInfo = params.getSignatureInfo();
+  // Call ValidityPeriod::makeRelative here instead of in MakeCertificateOptions struct
+  // because the caller may prepare MakeCertificateOptions first and call makeCertificate
+  // at a later time.
+  sigInfo.setValidityPeriod(opts.validity.value_or(ValidityPeriod::makeRelative(-1_s, 365_days)));
+  params.setSignatureInfo(sigInfo);
+
+  sign(data, params);
+  // let Certificate constructor double-check correctness of this function
+  return Certificate(std::move(data));
+}
+
+Certificate
 KeyChain::selfSign(Key& key)
 {
-  Certificate certificate;
-
-  // set name
-  Name certificateName = key.getName();
-  certificateName
-    .append("self")
-    .appendVersion();
-  certificate.setName(certificateName);
-
-  // set metainfo
-  certificate.setContentType(tlv::ContentType_Key);
-  certificate.setFreshnessPeriod(1_h);
-
-  // set content
-  certificate.setContent(key.getPublicKey());
-
-  // set signature-info
-  SignatureInfo signatureInfo;
+  MakeCertificateOptions opts;
+  opts.issuerId = SELF;
   // Note time::system_clock::max() or other NotAfter date results in incorrect encoded value
-  // because of overflow during conversion to boost::posix_time::ptime (bug #3915).
-  signatureInfo.setValidityPeriod(ValidityPeriod(time::system_clock::TimePoint(),
-                                                 time::system_clock::now() + 20 * 365_days));
+  // because of overflow during conversion to boost::posix_time::ptime (bug #3915, bug #5176).
+  opts.validity = ValidityPeriod::makeRelative(-1_s, 20 * 365_days);
+  auto cert = makeCertificate(key, signingByKey(key), opts);
 
-  sign(certificate, SigningInfo(key).setSignatureInfo(signatureInfo));
-
-  key.addCertificate(certificate);
-  return certificate;
+  key.addCertificate(cert);
+  return cert;
 }
 
 std::tuple<Name, SignatureInfo>
diff --git a/ndn-cxx/security/key-chain.hpp b/ndn-cxx/security/key-chain.hpp
index 3d96dbc..2fa84ca 100644
--- a/ndn-cxx/security/key-chain.hpp
+++ b/ndn-cxx/security/key-chain.hpp
@@ -32,6 +32,47 @@
 
 namespace ndn {
 namespace security {
+
+/**
+ * @brief Options to KeyChain::makeCertificate() .
+ */
+struct MakeCertificateOptions
+{
+  /**
+   * @brief Certificate name IssuerId component.
+   *
+   * Default is "NA".
+   */
+  name::Component issuerId = Certificate::DEFAULT_ISSUER_ID;
+
+  /**
+   * @brief Certificate name version component.
+   *
+   * Default is deriving from current timestamp using the logic of Name::appendVersion() .
+   */
+  optional<uint64_t> version;
+
+  /**
+   * @brief Certificate packet FreshnessPeriod.
+   *
+   * As required by the certificate format, this must be positive.
+   * Setting this to zero or negative causes @c std::invalid_argument exception.
+   *
+   * Default is 1 hour.
+   */
+  time::milliseconds freshnessPeriod = 1_h;
+
+  /**
+   * @brief Certificate ValidityPeriod.
+   *
+   * It isn't an error to specify a ValidityPeriod that does not include the current time
+   * or has zero duration, but the certificate won't be valid.
+   *
+   * Default is a ValidityPeriod from now until 365 days later.
+   */
+  optional<ValidityPeriod> validity;
+};
+
 inline namespace v2 {
 
 /**
@@ -289,6 +330,39 @@
   void
   sign(Interest& interest, const SigningInfo& params = SigningInfo());
 
+  /**
+   * @brief Create and sign a certificate packet.
+   * @param publicKey Public key being certified. It does not need to exist in this KeyChain.
+   * @param params Signing parameters. The referenced key must exist in this KeyChain.
+   *               It may contain SignatureInfo for customizing KeyLocator and CustomTlv (including
+   *               AdditionalDescription), but ValidityPeriod will be overwritten.
+   * @param opts Optional arguments.
+   * @return A certificate of @p publicKey signed by a key from this KeyChain found by @p params .
+   * @throw std::invalid_argument @p opts.freshnessPeriod is not positive.
+   * @throw Error Certificate signing failure.
+   */
+  Certificate
+  makeCertificate(const pib::Key& publicKey, const SigningInfo& params = SigningInfo(),
+                  const MakeCertificateOptions& opts = {});
+
+  /**
+   * @brief Create and sign a certificate packet.
+   * @param certRequest Certificate request enclosing the public key being certified.
+   *                    It does not need to exist in this KeyChain.
+   * @param params Signing parameters. The referenced key must exist in this KeyChain.
+   *               It may contain SignatureInfo for customizing KeyLocator and CustomTlv (including
+   *               AdditionalDescription), but ValidityPeriod will be overwritten.
+   * @param opts Optional arguments.
+   * @return A certificate of the public key enclosed in @p certRequest signed by a key from this
+   *         KeyChain found by @p params .
+   * @throw std::invalid_argument @p opts.freshnessPeriod is not positive.
+   * @throw std::invalid_argument @p certRequest contains invalid public key.
+   * @throw Error Certificate signing failure.
+   */
+  Certificate
+  makeCertificate(const Certificate& certRequest, const SigningInfo& params = SigningInfo(),
+                  const MakeCertificateOptions& opts = {});
+
 public: // export & import
   /**
    * @brief Export a certificate and its corresponding private key.
@@ -405,6 +479,10 @@
   getSignatureType(KeyType keyType, DigestAlgorithm digestAlgorithm);
 
 private: // signing
+  Certificate
+  makeCertificate(const Name& keyName, span<const uint8_t> publicKey, SigningInfo params,
+                  const MakeCertificateOptions& opts);
+
   /**
    * @brief Generate a self-signed certificate for a public key.
    *
diff --git a/ndn-cxx/security/validity-period.cpp b/ndn-cxx/security/validity-period.cpp
index fbe0430..385c895 100644
--- a/ndn-cxx/security/validity-period.cpp
+++ b/ndn-cxx/security/validity-period.cpp
@@ -39,6 +39,13 @@
 
 using boost::chrono::time_point_cast;
 
+ValidityPeriod
+ValidityPeriod::makeRelative(time::seconds validFrom, time::seconds validUntil,
+                             const time::system_clock::TimePoint& now)
+{
+  return ValidityPeriod(now + validFrom, now + validUntil);
+}
+
 ValidityPeriod::ValidityPeriod()
   : ValidityPeriod(time::system_clock::TimePoint() + 1_ns,
                    time::system_clock::TimePoint())
diff --git a/ndn-cxx/security/validity-period.hpp b/ndn-cxx/security/validity-period.hpp
index c36ceec..6d1941b 100644
--- a/ndn-cxx/security/validity-period.hpp
+++ b/ndn-cxx/security/validity-period.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).
  *
@@ -43,7 +43,18 @@
     using tlv::Error::Error;
   };
 
-public:
+  /**
+   * @brief Construct ValidityPeriod relative to a timepoint.
+   * @param validFrom NotBefore is computed as @c now+validFrom .
+   *                  This should be negative to construct a ValidityPeriod that includes @p now .
+   * @param validUntil NotAfter is computed as @c now+validTo .
+   *                   This should be positive to construct a ValidityPeriod that includes @p now .
+   * @param now Reference timepoint. Default is current system clock timestamp.
+   */
+  static ValidityPeriod
+  makeRelative(time::seconds validFrom, time::seconds validUntil,
+               const time::system_clock::TimePoint& now = time::system_clock::now());
+
   /** @brief Set validity period [UNIX epoch + 1 nanosecond, UNIX epoch] that is always invalid
    */
   ValidityPeriod();
diff --git a/tests/key-chain-fixture.cpp b/tests/key-chain-fixture.cpp
index af62de5..7bbcf6a 100644
--- a/tests/key-chain-fixture.cpp
+++ b/tests/key-chain-fixture.cpp
@@ -43,36 +43,6 @@
   }
 }
 
-Certificate
-KeyChainFixture::makeCert(const Key& key, const std::string& issuer, const Key& signingKey,
-                          optional<KeyLocator> keyLocator)
-{
-  const Key& signer = signingKey ? signingKey : key;
-
-  Certificate cert;
-  cert.setName(Name(key.getName())
-               .append(issuer)
-               .appendVersion());
-
-  // set metainfo
-  cert.setContentType(tlv::ContentType_Key);
-  cert.setFreshnessPeriod(1_h);
-
-  // set content
-  cert.setContent(key.getPublicKey());
-
-  // set signature info
-  ndn::SignatureInfo info;
-  auto now = time::system_clock::now();
-  info.setValidityPeriod(ValidityPeriod(now - 30_days, now + 30_days));
-  if (keyLocator) {
-    info.setKeyLocator(*keyLocator);
-  }
-
-  m_keyChain.sign(cert, signingByKey(signer).setSignatureInfo(info));
-  return cert;
-}
-
 bool
 KeyChainFixture::saveCert(const Data& cert, const std::string& filename)
 {
diff --git a/tests/key-chain-fixture.hpp b/tests/key-chain-fixture.hpp
index 9c4c699..ec1cac3 100644
--- a/tests/key-chain-fixture.hpp
+++ b/tests/key-chain-fixture.hpp
@@ -45,19 +45,6 @@
 
 public:
   /**
-   * @brief Creates and returns a certificate for a given key
-   * @param key The key for which to make a certificate
-   * @param issuer The IssuerId to include in the certificate name
-   * @param signingKey The key with which to sign the certificate; if not provided, the
-   *                   certificate will be self-signed
-   * @param keyLocator The KeyLocator name in the generated certificate; if nullopt,
-   *                   @p signingKey 's default certificate will be used
-   */
-  Certificate
-  makeCert(const Key& key, const std::string& issuer, const Key& signingKey = Key(),
-           optional<KeyLocator> keyLocator = nullopt);
-
-  /**
    * @brief Saves an NDN certificate to a file
    * @return true if successful, false otherwise
    */
diff --git a/tests/unit/security/certificate-cache.t.cpp b/tests/unit/security/certificate-cache.t.cpp
index f3b400e..8c715eb 100644
--- a/tests/unit/security/certificate-cache.t.cpp
+++ b/tests/unit/security/certificate-cache.t.cpp
@@ -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).
  *
@@ -42,6 +42,23 @@
     cert = identity.getDefaultKey().getDefaultCertificate();
   }
 
+  void
+  checkFindByInterest(const Name& name, bool canBePrefix, optional<Certificate> expected) const
+  {
+    Interest interest(name);
+    interest.setCanBePrefix(canBePrefix);
+    BOOST_TEST_CONTEXT(interest) {
+      auto found = certCache.find(interest);
+      if (expected) {
+        BOOST_REQUIRE(found != nullptr);
+        BOOST_CHECK_EQUAL(found->getName(), expected->getName());
+      }
+      else {
+        BOOST_CHECK(found == nullptr);
+      }
+    }
+  }
+
 public:
   CertificateCache certCache;
   Identity identity;
@@ -75,18 +92,13 @@
 {
   BOOST_CHECK_NO_THROW(certCache.insert(cert));
 
-  Interest i;
-  i.setCanBePrefix(true);
-  i.setName(cert.getIdentity());
-  BOOST_CHECK(certCache.find(i) != nullptr);
-  i.setName(cert.getKeyName());
-  BOOST_CHECK(certCache.find(i) != nullptr);
-  i.setName(Name(cert.getName()).appendVersion());
-  BOOST_CHECK(certCache.find(i) == nullptr);
+  checkFindByInterest(cert.getIdentity(), true, cert);
+  checkFindByInterest(cert.getKeyName(), true, cert);
+  checkFindByInterest(cert.getName(), false, cert);
+  checkFindByInterest(Name(cert.getName()).appendVersion(), true, nullopt);
 
   advanceClocks(12_s);
-  i.setName(cert.getIdentity());
-  BOOST_CHECK(certCache.find(i) == nullptr);
+  checkFindByInterest(cert.getIdentity(), true, nullopt);
 }
 
 BOOST_AUTO_TEST_SUITE_END() // TestCertificateCache
diff --git a/tests/unit/security/key-chain.t.cpp b/tests/unit/security/key-chain.t.cpp
index 94e75a0..b7aef11 100644
--- a/tests/unit/security/key-chain.t.cpp
+++ b/tests/unit/security/key-chain.t.cpp
@@ -26,6 +26,7 @@
 
 #include "tests/boost-test.hpp"
 #include "tests/key-chain-fixture.hpp"
+#include "tests/unit/clock-fixture.hpp"
 #include "tests/unit/test-home-env-saver.hpp"
 
 #include <boost/mpl/vector.hpp>
@@ -573,6 +574,146 @@
   }
 }
 
+class MakeCertificateFixture : public ClockFixture
+{
+public:
+  MakeCertificateFixture()
+    : requesterKeyChain("pib-memory:", "tpm-memory:")
+    , signerKeyChain("pib-memory:", "tpm-memory:")
+  {
+    m_systemClock->setNow(time::fromIsoString("20091117T203458,651387237").time_since_epoch());
+
+    requester = requesterKeyChain.createIdentity("/requester").getDefaultKey();
+    Name signerIdentityName("/signer");
+    signerKey = signerKeyChain.createIdentity(signerIdentityName).getDefaultKey();
+    signerParams = signingByIdentity(signerIdentityName);
+  }
+
+  void
+  checkKeyLocatorName(const Certificate& cert, optional<Name> klName = nullopt) const
+  {
+    auto kl = cert.getKeyLocator();
+    if (!kl.has_value()) {
+      BOOST_ERROR("KeyLocator is missing");
+      return;
+    }
+    BOOST_CHECK_EQUAL(kl->getName(),
+                      klName.value_or(signerKey.getDefaultCertificate().getName()));
+  }
+
+  void
+  checkCertFromDefaults(const Certificate& cert) const
+  {
+    BOOST_CHECK(Certificate::isValidName(cert.getName()));
+    BOOST_CHECK_EQUAL(cert.getKeyName(), requester.getName());
+    BOOST_CHECK_EQUAL(cert.getName()[-2], name::Component("NA"));
+    BOOST_CHECK(cert.getName()[-1].isVersion());
+
+    BOOST_CHECK_EQUAL(cert.getContentType(), tlv::ContentType_Key);
+    BOOST_CHECK_EQUAL(cert.getFreshnessPeriod(), 1_h);
+
+    BOOST_TEST(cert.getContent().value_bytes() == requester.getPublicKey(),
+               boost::test_tools::per_element());
+
+    checkKeyLocatorName(cert);
+
+    BOOST_CHECK(cert.isValid());
+    auto vp = cert.getValidityPeriod().getPeriod();
+    BOOST_CHECK_EQUAL(vp.first, time::fromIsoString("20091117T203458"));
+    BOOST_CHECK_EQUAL(vp.second, time::fromIsoString("20101117T203458"));
+
+    auto adBlock = cert.getSignatureInfo().getCustomTlv(tlv::AdditionalDescription);
+    BOOST_CHECK(!adBlock.has_value());
+  }
+
+public:
+  KeyChain requesterKeyChain;
+  pib::Key requester;
+
+  KeyChain signerKeyChain;
+  pib::Key signerKey;
+  Name signerCertificateName;
+  SigningInfo signerParams;
+};
+
+BOOST_FIXTURE_TEST_SUITE(MakeCertificate, MakeCertificateFixture)
+
+BOOST_AUTO_TEST_CASE(DefaultsFromKey)
+{
+  auto cert = signerKeyChain.makeCertificate(requester, signerParams);
+  checkCertFromDefaults(cert);
+}
+
+BOOST_AUTO_TEST_CASE(DefaultsFromCert)
+{
+  auto cert = signerKeyChain.makeCertificate(requester.getDefaultCertificate(), signerParams);
+  checkCertFromDefaults(cert);
+}
+
+BOOST_AUTO_TEST_CASE(Options)
+{
+  MakeCertificateOptions opts;
+  opts.issuerId = name::Component::fromEscapedString("ISSUER");
+  opts.version = 41218268;
+  opts.freshnessPeriod = 321_s;
+  opts.validity.emplace(time::fromIsoString("20060702T150405"),
+                        time::fromIsoString("20160702T150405"));
+
+  SignatureInfo sigInfo;
+  sigInfo.setKeyLocator(signerKey.getName());
+  sigInfo.setValidityPeriod(ValidityPeriod(time::fromIsoString("20060102T150405"),
+                                           time::fromIsoString("20160102T150405")));
+  sigInfo.addCustomTlv(Block(0xF0));
+  signerParams.setSignatureInfo(sigInfo);
+
+  auto cert = signerKeyChain.makeCertificate(requester, signerParams, opts);
+
+  BOOST_CHECK_EQUAL(cert.getName(),
+                    Name(requester.getName()).append(PartialName("ISSUER/v=41218268")));
+  BOOST_CHECK_EQUAL(cert.getFreshnessPeriod(), 321_s);
+  checkKeyLocatorName(cert, signerKey.getName());
+
+  auto vp = cert.getValidityPeriod().getPeriod();
+  BOOST_CHECK_EQUAL(vp.first, time::fromIsoString("20060702T150405"));
+  BOOST_CHECK_EQUAL(vp.second, time::fromIsoString("20160702T150405"));
+
+  BOOST_CHECK(cert.getSignatureInfo().getCustomTlv(0xF0).has_value());
+}
+
+BOOST_AUTO_TEST_CASE(ErrSigner)
+{
+  signerParams = signingByIdentity("/nonexistent");
+  BOOST_CHECK_THROW(signerKeyChain.makeCertificate(requester, signerParams), KeyChain::Error);
+}
+
+BOOST_AUTO_TEST_CASE(ErrZeroFreshness)
+{
+  MakeCertificateOptions opts;
+  opts.freshnessPeriod = 0_ms;
+  BOOST_CHECK_THROW(signerKeyChain.makeCertificate(requester, signerParams, opts),
+                    std::invalid_argument);
+}
+
+BOOST_AUTO_TEST_CASE(ErrNegativeFreshness)
+{
+  MakeCertificateOptions opts;
+  opts.freshnessPeriod = -1_ms;
+  BOOST_CHECK_THROW(signerKeyChain.makeCertificate(requester, signerParams, opts),
+                    std::invalid_argument);
+}
+
+BOOST_AUTO_TEST_CASE(ErrContent)
+{
+  Certificate request(requester.getDefaultCertificate());
+  const auto& oldContent = request.getContent();
+  std::vector<uint8_t> content(oldContent.value_begin(), oldContent.value_end());
+  content[0] ^= 0x80;
+  request.setContent(content);
+  BOOST_CHECK_THROW(signerKeyChain.makeCertificate(request, signerParams), std::invalid_argument);
+}
+
+BOOST_AUTO_TEST_SUITE_END() // MakeCertificate
+
 BOOST_FIXTURE_TEST_CASE(ImportPrivateKey, KeyChainFixture)
 {
   const Name keyName("/test/device2");
diff --git a/tests/unit/security/trust-anchor-container.t.cpp b/tests/unit/security/trust-anchor-container.t.cpp
index bb336b5..19d6867 100644
--- a/tests/unit/security/trust-anchor-container.t.cpp
+++ b/tests/unit/security/trust-anchor-container.t.cpp
@@ -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).
  *
@@ -60,6 +60,23 @@
     boost::filesystem::remove_all(certDirPath);
   }
 
+  void
+  checkFindByInterest(const Name& name, bool canBePrefix, optional<Certificate> expected) const
+  {
+    Interest interest(name);
+    interest.setCanBePrefix(canBePrefix);
+    BOOST_TEST_CONTEXT(interest) {
+      auto found = anchorContainer.find(interest);
+      if (expected) {
+        BOOST_REQUIRE(found != nullptr);
+        BOOST_CHECK_EQUAL(found->getName(), expected->getName());
+      }
+      else {
+        BOOST_CHECK(found == nullptr);
+      }
+    }
+  }
+
 public:
   const boost::filesystem::path certDirPath{boost::filesystem::path(UNIT_TESTS_TMPDIR) / "test-cert-dir"};
   const boost::filesystem::path certPath1{certDirPath / "trust-anchor-1.cert"};
@@ -150,29 +167,31 @@
 BOOST_AUTO_TEST_CASE(FindByInterest)
 {
   anchorContainer.insert("group1", certPath1.string(), 1_s);
-  Interest interest1;
-  interest1.setCanBePrefix(true);
-  interest1.setName(identity1.getName());
-  BOOST_CHECK(anchorContainer.find(interest1) != nullptr);
-  interest1.setName(identity1.getName().getPrefix(-1));
-  BOOST_CHECK(anchorContainer.find(interest1) != nullptr);
-  interest1.setName(Name(identity1.getName()).appendVersion());
-  BOOST_CHECK(anchorContainer.find(interest1) == nullptr);
 
-  auto cert3 = makeCert(identity1.getDefaultKey(), "3");
-  auto cert4 = makeCert(identity1.getDefaultKey(), "4");
-  auto cert5 = makeCert(identity1.getDefaultKey(), "5");
+  checkFindByInterest(identity1.getName(), true, cert1);
+  checkFindByInterest(identity1.getName().getPrefix(-1), true, cert1);
+  checkFindByInterest(cert1.getKeyName(), true, cert1);
+  checkFindByInterest(cert1.getName(), false, cert1);
+  checkFindByInterest(Name(identity1.getName()).appendVersion(), true, nullopt);
+
+  auto makeIdentity1Cert = [=] (const std::string& issuerId) {
+    auto key = identity1.getDefaultKey();
+    MakeCertificateOptions opts;
+    opts.issuerId = name::Component::fromEscapedString(issuerId);
+    return m_keyChain.makeCertificate(key, signingByKey(key), opts);
+  };
+
+  auto cert3 = makeIdentity1Cert("3");
+  auto cert4 = makeIdentity1Cert("4");
+  auto cert5 = makeIdentity1Cert("5");
 
   Certificate cert3Copy = cert3;
   anchorContainer.insert("group2", std::move(cert3Copy));
   anchorContainer.insert("group3", std::move(cert4));
   anchorContainer.insert("group4", std::move(cert5));
 
-  auto interest3 = Interest(cert3.getKeyName()).setCanBePrefix(true);
-  const Certificate* foundCert = anchorContainer.find(interest3);
-  BOOST_REQUIRE(foundCert != nullptr);
-  BOOST_CHECK(interest3.getName().isPrefixOf(foundCert->getName()));
-  BOOST_CHECK_EQUAL(foundCert->getName(), cert3.getName());
+  checkFindByInterest(cert3.getKeyName(), true, cert3);
+  checkFindByInterest(cert3.getName(), false, cert3);
 }
 
 BOOST_AUTO_TEST_SUITE_END() // TestTrustAnchorContainer
diff --git a/tests/unit/security/validator.t.cpp b/tests/unit/security/validator.t.cpp
index 0a45c72..4acbe4d 100644
--- a/tests/unit/security/validator.t.cpp
+++ b/tests/unit/security/validator.t.cpp
@@ -262,7 +262,12 @@
     // create another key for the same identity and sign it properly
     Key parentKey = m_keyChain.createKey(subIdentity);
     Key requestedKey = subIdentity.getKey(interest.getName());
-    auto cert = makeCert(requestedKey, "looper", parentKey, parentKey.getName());
+
+    SignatureInfo sigInfo;
+    sigInfo.setKeyLocator(parentKey.getName());
+    auto si = signingByKey(parentKey).setSignatureInfo(sigInfo);
+
+    auto cert = m_keyChain.makeCertificate(requestedKey, si);
     face.receive(cert);
   };
 
@@ -292,7 +297,11 @@
   auto k3 = m_keyChain.createKey(s1, RsaKeyParams(name::Component("key3")));
 
   auto makeLoopCert = [this] (Key& key, const Key& signer) {
-    auto cert = this->makeCert(key, "looper", signer, signer.getName());
+    SignatureInfo sigInfo;
+    sigInfo.setKeyLocator(signer.getName());
+    auto si = signingByKey(signer).setSignatureInfo(sigInfo);
+
+    auto cert = m_keyChain.makeCertificate(key, si);
     m_keyChain.setDefaultCertificate(key, cert);
     cache.insert(cert);
   };
diff --git a/tests/unit/security/validity-period.t.cpp b/tests/unit/security/validity-period.t.cpp
index 5dc8d9d..4a28df2 100644
--- a/tests/unit/security/validity-period.t.cpp
+++ b/tests/unit/security/validity-period.t.cpp
@@ -35,6 +35,34 @@
 BOOST_AUTO_TEST_SUITE(Security)
 BOOST_AUTO_TEST_SUITE(TestValidityPeriod)
 
+BOOST_AUTO_TEST_SUITE(MakeRelative)
+
+BOOST_AUTO_TEST_CASE(FromNow)
+{
+  auto vp = ValidityPeriod::makeRelative(-1_s, 365_days, time::fromIsoString("20091117T203458,651387237"));
+  auto period = vp.getPeriod();
+  BOOST_CHECK_EQUAL(period.first, time::fromIsoString("20091117T203458"));
+  BOOST_CHECK_EQUAL(period.second, time::fromIsoString("20101117T203458"));
+}
+
+BOOST_AUTO_TEST_CASE(Positive)
+{
+  auto vp = ValidityPeriod::makeRelative(10_s, 1_days, time::fromIsoString("20091117T203458,651387237"));
+  auto period = vp.getPeriod();
+  BOOST_CHECK_EQUAL(period.first, time::fromIsoString("20091117T203509"));
+  BOOST_CHECK_EQUAL(period.second, time::fromIsoString("20091118T203458"));
+}
+
+BOOST_AUTO_TEST_CASE(Negative)
+{
+  auto vp = ValidityPeriod::makeRelative(-1_days, -10_s, time::fromIsoString("20091117T203458,651387237"));
+  auto period = vp.getPeriod();
+  BOOST_CHECK_EQUAL(period.first, time::fromIsoString("20091116T203459"));
+  BOOST_CHECK_EQUAL(period.second, time::fromIsoString("20091117T203448"));
+}
+
+BOOST_AUTO_TEST_SUITE_END() // MakeRelative
+
 BOOST_FIXTURE_TEST_CASE(ConstructorSetter, ClockFixture)
 {
   auto now = m_systemClock->getNow();
diff --git a/tools/ndnsec/cert-gen.cpp b/tools/ndnsec/cert-gen.cpp
index e44d063..49b0362 100644
--- a/tools/ndnsec/cert-gen.cpp
+++ b/tools/ndnsec/cert-gen.cpp
@@ -63,8 +63,10 @@
                        "\"affiliation University of California, Los Angeles\"); "
                        "this option may be repeated multiple times")
     ("sign-id,s",      po::value<Name>(&signId), "signing identity")
-    ("issuer-id,i",    po::value<std::string>(&issuerId)->default_value("NA"),
-                       "issuer's ID to be included in the issued certificate name")
+    ("issuer-id,i",    po::value<std::string>(&issuerId),
+                       ("issuer's ID to be included in the issued certificate name, interpreted as "
+                        "name component in URI format (default: \"" +
+                        security::Certificate::DEFAULT_ISSUER_ID.toUri() + "\")").data())
     ;
 
   po::positional_options_description p;
@@ -124,39 +126,24 @@
 
   KeyChain keyChain;
 
-  auto certRequest = loadFromFile<security::Certificate>(requestFile);
+  auto request = loadFromFile<security::Certificate>(requestFile);
 
-  // validate that the content is a public key
-  auto keyContent = certRequest.getPublicKey();
-  security::transform::PublicKey pubKey;
-  pubKey.loadPkcs8(keyContent);
-
-  Name certName = certRequest.getKeyName();
-  certName
-    .append(issuerId)
-    .appendVersion();
-
-  security::Certificate cert;
-  cert.setName(certName);
-  cert.setContent(certRequest.getContent());
-  // TODO: add ability to customize
-  cert.setFreshnessPeriod(1_h);
-
-  SignatureInfo signatureInfo;
-  signatureInfo.setValidityPeriod(security::ValidityPeriod(notBefore, notAfter));
+  security::SigningInfo signer;
+  if (vm.count("sign-id") > 0) {
+    signer.setSigningIdentity(signId);
+  }
   if (!additionalDescription.empty()) {
-    signatureInfo.addCustomTlv(additionalDescription.wireEncode());
+    SignatureInfo sigInfo;
+    sigInfo.addCustomTlv(additionalDescription.wireEncode());
+    signer.setSignatureInfo(sigInfo);
   }
 
-  security::Identity identity;
-  if (vm.count("sign-id") == 0) {
-    identity = keyChain.getPib().getDefaultIdentity();
+  security::MakeCertificateOptions opts;
+  if (vm.count("issuer-id") > 0) {
+    opts.issuerId = name::Component::fromEscapedString(issuerId);
   }
-  else {
-    identity = keyChain.getPib().getIdentity(signId);
-  }
-
-  keyChain.sign(cert, security::SigningInfo(identity).setSignatureInfo(signatureInfo));
+  opts.validity.emplace(notBefore, notAfter);
+  auto cert = keyChain.makeCertificate(request, signer, opts);
 
   {
     using namespace security::transform;
diff --git a/tools/ndnsec/sign-req.cpp b/tools/ndnsec/sign-req.cpp
index f4655af..e202bc5 100644
--- a/tools/ndnsec/sign-req.cpp
+++ b/tools/ndnsec/sign-req.cpp
@@ -22,6 +22,8 @@
 #include "ndnsec.hpp"
 #include "util.hpp"
 
+#include "ndn-cxx/security/signing-helpers.hpp"
+
 namespace ndn {
 namespace ndnsec {
 
@@ -81,28 +83,10 @@
   }
 
   // Create signing request (similar to self-signed certificate)
-  security::Certificate certificate;
-
-  // set name
-  Name certificateName = key.getName();
-  certificateName
-    .append("cert-request")
-    .appendVersion();
-  certificate.setName(certificateName);
-
-  // set metainfo
-  certificate.setContentType(tlv::ContentType_Key);
-  certificate.setFreshnessPeriod(1_h);
-
-  // set content
-  certificate.setContent(key.getPublicKey());
-
-  // set signature-info
-  SignatureInfo signatureInfo;
-  auto now = time::system_clock::now();
-  signatureInfo.setValidityPeriod(security::ValidityPeriod(now, now + 10_days));
-
-  keyChain.sign(certificate, security::SigningInfo(key).setSignatureInfo(signatureInfo));
+  security::MakeCertificateOptions opts;
+  opts.issuerId = name::Component::fromEscapedString("cert-request");
+  opts.validity = security::ValidityPeriod::makeRelative(-1_s, 10_days);
+  auto certificate = keyChain.makeCertificate(key, security::signingByKey(key), opts);
 
   io::save(certificate, std::cout);