security: include certificate name in KeyLocator

refs #5112

Change-Id: I0066b30ea48d8a51c66630aa186a372280eecd7f

ndn-cxx/security/key-chain.cpp

@@ -591,100 +591,136 @@ KeyChain::selfSign(Key& key)
 std::tuple<Name, SignatureInfo>
 KeyChain::prepareSignatureInfo(const SigningInfo& params)
 {
-  SignatureInfo sigInfo = params.getSignatureInfo();
-  pib::Identity identity;
-  pib::Key key;
-
   switch (params.getSignerType()) {
     case SigningInfo::SIGNER_TYPE_NULL: {
+      pib::Identity identity;
       try {
         identity = m_pib->getDefaultIdentity();
       }
       catch (const Pib::Error&) { // no default identity, use sha256 for signing.
-        sigInfo.setSignatureType(tlv::DigestSha256);
-        NDN_LOG_TRACE("Prepared signature info: " << sigInfo);
-        return std::make_tuple(SigningInfo::getDigestSha256Identity(), sigInfo);
+        return prepareSignatureInfoSha256(params);
       }
-      break;
+      return prepareSignatureInfoWithIdentity(params, identity);
     }
     case SigningInfo::SIGNER_TYPE_ID: {
-      identity = params.getPibIdentity();
+      auto identity = params.getPibIdentity();
       if (!identity) {
+        auto identityName = params.getSignerName();
         try {
-          identity = m_pib->getIdentity(params.getSignerName());
+          identity = m_pib->getIdentity(identityName);
         }
         catch (const Pib::Error&) {
           NDN_THROW_NESTED(InvalidSigningInfoError("Signing identity `" +
-                                                   params.getSignerName().toUri() + "` does not exist"));
+                                                   identityName.toUri() + "` does not exist"));
         }
       }
-      break;
+      if (!identity) {
+        NDN_THROW(InvalidSigningInfoError("Cannot determine signing parameters"));
+      }
+      return prepareSignatureInfoWithIdentity(params, identity);
     }
     case SigningInfo::SIGNER_TYPE_KEY: {
-      key = params.getPibKey();
+      auto key = params.getPibKey();
       if (!key) {
-        Name identityName = extractIdentityFromKeyName(params.getSignerName());
+        auto keyName = params.getSignerName();
+        auto identityName = extractIdentityFromKeyName(keyName);
         try {
-          key = m_pib->getIdentity(identityName).getKey(params.getSignerName());
+          key = m_pib->getIdentity(identityName).getKey(keyName);
         }
         catch (const Pib::Error&) {
           NDN_THROW_NESTED(InvalidSigningInfoError("Signing key `" +
-                                                   params.getSignerName().toUri() + "` does not exist"));
+                                                   keyName.toUri() + "` does not exist"));
         }
       }
-      break;
+      if (!key) {
+        NDN_THROW(InvalidSigningInfoError("Cannot determine signing parameters"));
+      }
+      return prepareSignatureInfoWithKey(params, key);
     }
     case SigningInfo::SIGNER_TYPE_CERT: {
-      Name identityName = extractIdentityFromCertName(params.getSignerName());
-      Name keyName = extractKeyNameFromCertName(params.getSignerName());
+      auto certName = params.getSignerName();
+      auto keyName = extractKeyNameFromCertName(certName);
+      auto identityName = extractIdentityFromCertName(certName);
+      pib::Key key;
       try {
-        identity = m_pib->getIdentity(identityName);
-        key = identity.getKey(keyName);
+        key = m_pib->getIdentity(identityName).getKey(keyName);
       }
       catch (const Pib::Error&) {
         NDN_THROW_NESTED(InvalidSigningInfoError("Signing certificate `" +
-                                                 params.getSignerName().toUri() + "` does not exist"));
+                                                 certName.toUri() + "` does not exist"));
       }
-      break;
+      return prepareSignatureInfoWithKey(params, key, certName);
     }
     case SigningInfo::SIGNER_TYPE_SHA256: {
-      sigInfo.setSignatureType(tlv::DigestSha256);
-      NDN_LOG_TRACE("Prepared signature info: " << sigInfo);
-      return std::make_tuple(SigningInfo::getDigestSha256Identity(), sigInfo);
+      return prepareSignatureInfoSha256(params);
     }
     case SigningInfo::SIGNER_TYPE_HMAC: {
-      const Name& keyName = params.getSignerName();
-      if (!m_tpm->hasKey(keyName)) {
-        m_tpm->importPrivateKey(keyName, params.getHmacKey());
-      }
-      sigInfo.setSignatureType(getSignatureType(KeyType::HMAC, params.getDigestAlgorithm()));
-      sigInfo.setKeyLocator(keyName);
-      NDN_LOG_TRACE("Prepared signature info: " << sigInfo);
-      return std::make_tuple(keyName, sigInfo);
-    }
-    default: {
-      NDN_THROW(InvalidSigningInfoError("Unrecognized signer type " +
-                                        boost::lexical_cast<std::string>(params.getSignerType())));
+      return prepareSignatureInfoHmac(params);
     }
   }
+  NDN_THROW(InvalidSigningInfoError("Unrecognized signer type " +
+                                    to_string(params.getSignerType())));
+}
 
-  if (!key) {
-    if (!identity) {
-      NDN_THROW(InvalidSigningInfoError("Cannot determine signing parameters"));
-    }
-    try {
-      key = identity.getDefaultKey();
-    }
-    catch (const Pib::Error&) {
-      NDN_THROW_NESTED(InvalidSigningInfoError("Signing identity `" + identity.getName().toUri() +
-                                               "` does not have a default certificate"));
-    }
+std::tuple<Name, SignatureInfo>
+KeyChain::prepareSignatureInfoSha256(const SigningInfo& params)
+{
+  auto sigInfo = params.getSignatureInfo();
+  sigInfo.setSignatureType(tlv::DigestSha256);
+
+  NDN_LOG_TRACE("Prepared signature info: " << sigInfo);
+  return std::make_tuple(SigningInfo::getDigestSha256Identity(), sigInfo);
+}
+
+std::tuple<Name, SignatureInfo>
+KeyChain::prepareSignatureInfoHmac(const SigningInfo& params)
+{
+  const Name& keyName = params.getSignerName();
+  if (!m_tpm->hasKey(keyName)) {
+    m_tpm->importPrivateKey(keyName, params.getHmacKey());
   }
 
-  BOOST_ASSERT(key);
+  auto sigInfo = params.getSignatureInfo();
+  sigInfo.setSignatureType(getSignatureType(KeyType::HMAC, params.getDigestAlgorithm()));
+  sigInfo.setKeyLocator(keyName);
 
+  NDN_LOG_TRACE("Prepared signature info: " << sigInfo);
+  return std::make_tuple(keyName, sigInfo);
+}
+
+std::tuple<Name, SignatureInfo>
+KeyChain::prepareSignatureInfoWithIdentity(const SigningInfo& params, const pib::Identity& identity)
+{
+  pib::Key key;
+  try {
+    key = identity.getDefaultKey();
+  }
+  catch (const Pib::Error&) {
+    NDN_THROW_NESTED(InvalidSigningInfoError("Signing identity `" + identity.getName().toUri() +
+                                              "` does not have a default key"));
+  }
+  return prepareSignatureInfoWithKey(params, key);
+}
+
+std::tuple<Name, SignatureInfo>
+KeyChain::prepareSignatureInfoWithKey(const SigningInfo& params, const pib::Key& key, optional<Name> certName)
+{
+  auto sigInfo = params.getSignatureInfo();
   sigInfo.setSignatureType(getSignatureType(key.getKeyType(), params.getDigestAlgorithm()));
-  sigInfo.setKeyLocator(key.getName());
+  if (!sigInfo.hasKeyLocator()) {
+    if (certName) {
+      sigInfo.setKeyLocator(certName);
+    }
+    else {
+      Name klName = key.getName();
+      try {
+        klName = key.getDefaultCertificate().getName();
+      }
+      catch (const Pib::Error&) {
+      }
+      sigInfo.setKeyLocator(klName);
+    }
+  }
 
   NDN_LOG_TRACE("Prepared signature info: " << sigInfo);
   return std::make_tuple(key.getName(), sigInfo);

ndn-cxx/security/key-chain.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).
  *
@@ -424,6 +424,19 @@ private: // signing
   std::tuple<Name, SignatureInfo>
   prepareSignatureInfo(const SigningInfo& params);
 
+  std::tuple<Name, SignatureInfo>
+  prepareSignatureInfoSha256(const SigningInfo& params);
+
+  std::tuple<Name, SignatureInfo>
+  prepareSignatureInfoHmac(const SigningInfo& params);
+
+  std::tuple<Name, SignatureInfo>
+  prepareSignatureInfoWithIdentity(const SigningInfo& params, const pib::Identity& identity);
+
+  std::tuple<Name, SignatureInfo>
+  prepareSignatureInfoWithKey(const SigningInfo& params, const pib::Key& key,
+                              optional<Name> certName = nullopt);
+
   /**
    * @brief Generate and return a raw signature for the byte ranges in @p bufs using
    *        the specified key and digest algorithm.

tests/key-chain-fixture.cpp

@@ -44,8 +44,11 @@ KeyChainFixture::~KeyChainFixture()
 }
 
 Certificate
-KeyChainFixture::makeCert(const Key& key, const std::string& issuer, const Key& signingKey)
+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)
@@ -62,8 +65,11 @@ KeyChainFixture::makeCert(const Key& key, const std::string& issuer, const Key&
   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(signingKey ? signingKey : key).setSignatureInfo(info));
+  m_keyChain.sign(cert, signingByKey(signer).setSignatureInfo(info));
   return cert;
 }
 

tests/key-chain-fixture.hpp

@@ -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).
  *
@@ -50,9 +50,12 @@ public:
    * @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());
+  makeCert(const Key& key, const std::string& issuer, const Key& signingKey = Key(),
+           optional<KeyLocator> keyLocator = nullopt);
 
   /**
    * @brief Saves an NDN certificate to a file

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

@@ -425,7 +425,7 @@ struct AsymmetricSigningBase : protected KeyChainFixture, protected PacketType
 
   const uint32_t expectedSigType = SignatureTypeTlvValue;
   const bool shouldHaveKeyLocator = true;
-  const optional<KeyLocator> expectedKeyLocator = cert.getName().getPrefix(-2);
+  const optional<KeyLocator> expectedKeyLocator = cert.getName();
 
   bool
   verify(const SigningInfo&) const

tests/unit/security/validator-fixture.cpp

@@ -36,8 +36,12 @@ ValidatorFixtureBase::ValidatorFixtureBase()
   processInterest = [this] (const Interest& interest) {
     auto cert = cache.find(interest);
     if (cert != nullptr) {
+      BOOST_TEST_MESSAGE("ValidatorFixture processInterest " << interest << " reply " << cert->getName());
       face.receive(*cert);
     }
+    else {
+      BOOST_TEST_MESSAGE("ValidatorFixture processInterest " << interest << " no reply");
+    }
   };
 }
 

tests/unit/security/validator.t.cpp

@@ -262,12 +262,13 @@ BOOST_AUTO_TEST_CASE(InfiniteCertChain)
     // 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);
+    auto cert = makeCert(requestedKey, "looper", parentKey, parentKey.getName());
     face.receive(cert);
   };
 
   Data data("/Security/ValidatorFixture/Sub1/Sub2/Data");
-  m_keyChain.sign(data, signingByIdentity(subIdentity));
+  m_keyChain.sign(data, signingByIdentity(subIdentity).setSignatureInfo(
+                        SignatureInfo().setKeyLocator(subIdentity.getDefaultKey().getName())));
 
   validator.setMaxDepth(40);
   BOOST_CHECK_EQUAL(validator.getMaxDepth(), 40);
@@ -290,27 +291,23 @@ BOOST_AUTO_TEST_CASE(LoopedCertChain)
   auto k2 = m_keyChain.createKey(s1, RsaKeyParams(name::Component("key2")));
   auto k3 = m_keyChain.createKey(s1, RsaKeyParams(name::Component("key3")));
 
-  auto makeCert = [this] (Key& key, const Key& signer) {
-    Certificate request = key.getDefaultCertificate();
-    request.setName(Name(key.getName()).append("looper").appendVersion());
-
-    SignatureInfo info;
-    info.setValidityPeriod(ValidityPeriod(time::system_clock::now() - 100_days,
-                                          time::system_clock::now() + 100_days));
-    m_keyChain.sign(request, signingByKey(signer).setSignatureInfo(info));
-    m_keyChain.addCertificate(key, request);
-
-    cache.insert(request);
+  auto makeLoopCert = [this] (Key& key, const Key& signer) {
+    auto cert = this->makeCert(key, "looper", signer, signer.getName());
+    m_keyChain.setDefaultCertificate(key, cert);
+    cache.insert(cert);
   };
 
-  makeCert(k1, k2);
-  makeCert(k2, k3);
-  makeCert(k3, k1);
+  makeLoopCert(k1, k2);
+  makeLoopCert(k2, k3);
+  makeLoopCert(k3, k1);
 
   Data data("/loop/Data");
   m_keyChain.sign(data, signingByKey(k1));
   VALIDATE_FAILURE(data, "Should fail, as certificate chain loops");
-  BOOST_CHECK_EQUAL(face.sentInterests.size(), 3);
+  BOOST_REQUIRE_EQUAL(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());
 }
 
 BOOST_AUTO_TEST_SUITE_END() // TestValidator