security: tolerate invalid or unsupported public keys in PIB

Change-Id: I3c1dd9d3fe81d0895e2d1706f0b29a8e61940beb
diff --git a/tests/unit/security/key-chain.t.cpp b/tests/unit/security/key-chain.t.cpp
index b7aef11..b3ea456 100644
--- a/tests/unit/security/key-chain.t.cpp
+++ b/tests/unit/security/key-chain.t.cpp
@@ -590,15 +590,14 @@
   }
 
   void
-  checkKeyLocatorName(const Certificate& cert, optional<Name> klName = nullopt) const
+  checkKeyLocatorName(const Certificate& cert, const 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()));
+    BOOST_CHECK_EQUAL(kl->getName(), klName.value_or(signerKey.getDefaultCertificate().getName()));
   }
 
   void
@@ -705,11 +704,17 @@
 BOOST_AUTO_TEST_CASE(ErrContent)
 {
   Certificate request(requester.getDefaultCertificate());
+
+  // malformed public key
   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);
+
+  // empty content
+  request.setContent(span<uint8_t>{});
+  BOOST_CHECK_THROW(signerKeyChain.makeCertificate(request, signerParams), std::invalid_argument);
 }
 
 BOOST_AUTO_TEST_SUITE_END() // MakeCertificate
diff --git a/tests/unit/security/pib/certificate-container.t.cpp b/tests/unit/security/pib/certificate-container.t.cpp
index 0ba9956..55b013e 100644
--- a/tests/unit/security/pib/certificate-container.t.cpp
+++ b/tests/unit/security/pib/certificate-container.t.cpp
@@ -95,6 +95,13 @@
     BOOST_CHECK(container2.find(id1Key1Cert1.getName()) == container2.end());
     BOOST_CHECK(container2.find(id1Key1Cert2.getName()) != container2.end());
 
+    // removing the same certificate again is a no-op
+    container2.remove(id1Key1Cert1.getName());
+    BOOST_CHECK_EQUAL(container2.size(), 1);
+    BOOST_CHECK_EQUAL(container2.m_certs.size(), 1);
+    BOOST_CHECK(container2.find(id1Key1Cert1.getName()) == container2.end());
+    BOOST_CHECK(container2.find(id1Key1Cert2.getName()) != container2.end());
+
     // remove another certificate
     container2.remove(id1Key1Cert2.getName());
     BOOST_CHECK_EQUAL(container2.size(), 0);
diff --git a/tests/unit/security/pib/identity-container.t.cpp b/tests/unit/security/pib/identity-container.t.cpp
index 893f912..79daace 100644
--- a/tests/unit/security/pib/identity-container.t.cpp
+++ b/tests/unit/security/pib/identity-container.t.cpp
@@ -97,6 +97,13 @@
     BOOST_CHECK(container2.find(id1) == container2.end());
     BOOST_CHECK(container2.find(id2) != container2.end());
 
+    // removing the same identity again is a no-op
+    container2.remove(id1);
+    BOOST_CHECK_EQUAL(container2.size(), 1);
+    BOOST_CHECK_EQUAL(container2.m_identities.size(), 1);
+    BOOST_CHECK(container2.find(id1) == container2.end());
+    BOOST_CHECK(container2.find(id2) != container2.end());
+
     // remove another identity
     container2.remove(id2);
     BOOST_CHECK_EQUAL(container2.size(), 0);
diff --git a/tests/unit/security/pib/identity.t.cpp b/tests/unit/security/pib/identity.t.cpp
index 5a1816e..be2d03b 100644
--- a/tests/unit/security/pib/identity.t.cpp
+++ b/tests/unit/security/pib/identity.t.cpp
@@ -39,14 +39,17 @@
   Identity id;
   BOOST_TEST(!id);
   BOOST_TEST(id == Identity());
+  BOOST_CHECK_THROW(id.getName(), std::domain_error);
 
   auto impl = std::make_shared<detail::IdentityImpl>(id1, makePibWithIdentity(id1));
   id = Identity(impl);
   BOOST_TEST(id);
   BOOST_TEST(id != Identity());
+  BOOST_TEST(id.getName() == id1);
 
   impl.reset();
   BOOST_TEST(!id);
+  BOOST_CHECK_THROW(id.getName(), std::domain_error);
 }
 
 // pib::Identity is a wrapper of pib::detail::IdentityImpl. Since the functionality
diff --git a/tests/unit/security/pib/impl/identity-impl.t.cpp b/tests/unit/security/pib/impl/identity-impl.t.cpp
index b7701aa..3f38e0e 100644
--- a/tests/unit/security/pib/impl/identity-impl.t.cpp
+++ b/tests/unit/security/pib/impl/identity-impl.t.cpp
@@ -46,7 +46,7 @@
 
 BOOST_AUTO_TEST_CASE(Properties)
 {
-  BOOST_CHECK_EQUAL(identity1.getName(), id1);
+  BOOST_TEST(identity1.getName() == id1);
 }
 
 BOOST_AUTO_TEST_CASE(KeyOperations)
@@ -128,7 +128,6 @@
 
 BOOST_AUTO_TEST_CASE(Errors)
 {
-  identity1.addKey(id1Key1, id1Key1Name);
   BOOST_CHECK_THROW(identity1.addKey(id2Key1, id2Key1Name), std::invalid_argument);
   BOOST_CHECK_THROW(identity1.removeKey(id2Key1Name), std::invalid_argument);
   BOOST_CHECK_THROW(identity1.getKey(id2Key1Name), std::invalid_argument);
diff --git a/tests/unit/security/pib/impl/key-impl.t.cpp b/tests/unit/security/pib/impl/key-impl.t.cpp
index 2f883a5..4cb960a 100644
--- a/tests/unit/security/pib/impl/key-impl.t.cpp
+++ b/tests/unit/security/pib/impl/key-impl.t.cpp
@@ -47,9 +47,9 @@
 
 BOOST_AUTO_TEST_CASE(Properties)
 {
-  BOOST_CHECK_EQUAL(key11.getIdentity(), id1);
-  BOOST_CHECK_EQUAL(key11.getName(), id1Key1Name);
-  BOOST_CHECK_EQUAL(key11.getKeyType(), KeyType::EC);
+  BOOST_TEST(key11.getIdentity() == id1);
+  BOOST_TEST(key11.getName() == id1Key1Name);
+  BOOST_TEST(key11.getKeyType() == KeyType::EC);
   BOOST_TEST(key11.getPublicKey() == id1Key1, boost::test_tools::per_element());
 }
 
@@ -138,11 +138,9 @@
 
 BOOST_AUTO_TEST_CASE(Errors)
 {
+  // illegal key name
   BOOST_CHECK_THROW(KeyImpl(Name("/wrong"), id1Key1, pibImpl), std::invalid_argument);
 
-  Buffer invalidKey;
-  BOOST_CHECK_THROW(KeyImpl(id1Key1Name, invalidKey, pibImpl), std::invalid_argument);
-
   BOOST_CHECK_THROW(key11.addCertificate(id1Key2Cert1), std::invalid_argument);
   BOOST_CHECK_THROW(key11.removeCertificate(id1Key2Cert1.getName()), std::invalid_argument);
   BOOST_CHECK_THROW(key11.getCertificate(id1Key2Cert1.getName()), std::invalid_argument);
@@ -150,6 +148,19 @@
   BOOST_CHECK_THROW(key11.setDefaultCertificate(id1Key2Cert1.getName()), std::invalid_argument);
 }
 
+BOOST_AUTO_TEST_CASE(UnknownKeyType)
+{
+  Name keyName = security::constructKeyName(id1, name::Component::fromEscapedString("foo"));
+  Buffer invalidKey{0x01, 0x02, 0x03, 0x04};
+  pibImpl->addKey(id1, keyName, invalidKey);
+
+  KeyImpl unknown(keyName, invalidKey, pibImpl);
+  BOOST_TEST(unknown.getIdentity() == id1);
+  BOOST_TEST(unknown.getName() == keyName);
+  BOOST_TEST(unknown.getKeyType() == KeyType::NONE);
+  BOOST_TEST(unknown.getPublicKey() == invalidKey, boost::test_tools::per_element());
+}
+
 BOOST_AUTO_TEST_SUITE_END() // TestKeyImpl
 BOOST_AUTO_TEST_SUITE_END() // Pib
 BOOST_AUTO_TEST_SUITE_END() // Security
diff --git a/tests/unit/security/pib/key-container.t.cpp b/tests/unit/security/pib/key-container.t.cpp
index fac7739..a7d29ef 100644
--- a/tests/unit/security/pib/key-container.t.cpp
+++ b/tests/unit/security/pib/key-container.t.cpp
@@ -103,6 +103,13 @@
     BOOST_CHECK(container2.find(id1Key1Name) == container2.end());
     BOOST_CHECK(container2.find(id1Key2Name) != container2.end());
 
+    // removing the same key again is a no-op
+    container2.remove(id1Key1Name);
+    BOOST_CHECK_EQUAL(container2.size(), 1);
+    BOOST_CHECK_EQUAL(container2.m_keys.size(), 1);
+    BOOST_CHECK(container2.find(id1Key1Name) == container2.end());
+    BOOST_CHECK(container2.find(id1Key2Name) != container2.end());
+
     // remove another key
     container2.remove(id1Key2Name);
     BOOST_CHECK_EQUAL(container2.size(), 0);
diff --git a/tests/unit/security/pib/key.t.cpp b/tests/unit/security/pib/key.t.cpp
index 8265f01..4d1da0c 100644
--- a/tests/unit/security/pib/key.t.cpp
+++ b/tests/unit/security/pib/key.t.cpp
@@ -39,15 +39,18 @@
   Key key;
   BOOST_TEST(!key);
   BOOST_TEST(key == Key());
+  BOOST_CHECK_THROW(key.getName(), std::domain_error);
 
   auto impl = std::make_shared<detail::KeyImpl>(id1Key1Name, id1Key1,
                                                 makePibWithKey(id1Key1Name, id1Key1));
   key = Key(impl);
   BOOST_TEST(key);
   BOOST_TEST(key != Key());
+  BOOST_TEST(key.getName() == id1Key1Name);
 
   impl.reset();
   BOOST_TEST(!key);
+  BOOST_CHECK_THROW(key.getName(), std::domain_error);
 }
 
 // pib::Key is a wrapper of pib::detail::KeyImpl. Since the functionality of KeyImpl is
diff --git a/tests/unit/security/transform/public-key.t.cpp b/tests/unit/security/transform/public-key.t.cpp
index e844293..e37cda5 100644
--- a/tests/unit/security/transform/public-key.t.cpp
+++ b/tests/unit/security/transform/public-key.t.cpp
@@ -113,23 +113,42 @@
   BOOST_TEST(*os6.buf() == *pKeyPkcs8, boost::test_tools::per_element());
 }
 
+BOOST_AUTO_TEST_CASE(LoadError)
+{
+  EcKeyTestData dataSet;
+  auto pkcs8Base64 = make_span(reinterpret_cast<const uint8_t*>(dataSet.pkcs8Base64.data()),
+                               dataSet.pkcs8Base64.size());
+  OBufferStream os;
+  bufferSource(pkcs8Base64) >> base64Decode() >> streamSink(os);
+  auto pkcs8 = os.buf();
+
+  PublicKey pKey;
+  // empty
+  BOOST_CHECK_THROW(pKey.loadPkcs8(span<uint8_t>{}), PublicKey::Error);
+  BOOST_CHECK_THROW(pKey.loadPkcs8Base64(span<uint8_t>{}), PublicKey::Error);
+  // truncated
+  BOOST_CHECK_THROW(pKey.loadPkcs8(make_span(*pkcs8).first(10)), PublicKey::Error);
+  BOOST_CHECK_THROW(pKey.loadPkcs8Base64(pkcs8Base64.first(10)), PublicKey::Error);
+}
+
 // NOTE: We cannot test RSA encryption by comparing the computed ciphertext to
 //       a known-good one, because OAEP padding is randomized and would produce
 //       different results every time. An encrypt/decrypt round-trip test is
 //       performed in private-key.t.cpp
 
-BOOST_AUTO_TEST_CASE(UnsupportedEcEncryption)
+BOOST_AUTO_TEST_CASE(UnsupportedEncryption)
 {
-  EcKeyTestData dataSet;
-
-  PublicKey pKey;
-  pKey.loadPkcs8Base64({reinterpret_cast<const uint8_t*>(dataSet.pkcs8Base64.data()),
-                        dataSet.pkcs8Base64.size()});
-
   OBufferStream os;
   bufferSource("Y2lhbyFob2xhIWhlbGxvIQ==") >> base64Decode() >> streamSink(os);
+  auto plain = os.buf();
 
-  BOOST_CHECK_THROW(pKey.encrypt(*os.buf()), PublicKey::Error);
+  PublicKey pKey;
+  BOOST_CHECK_THROW(pKey.encrypt(*plain), PublicKey::Error);
+
+  EcKeyTestData dataSet;
+  pKey.loadPkcs8Base64({reinterpret_cast<const uint8_t*>(dataSet.pkcs8Base64.data()),
+                        dataSet.pkcs8Base64.size()});
+  BOOST_CHECK_THROW(pKey.encrypt(*plain), PublicKey::Error);
 }
 
 BOOST_AUTO_TEST_SUITE_END() // TestPublicKey