security: Correct KeyChain::addCertificate semantics
The documentation of KeyChain::addCertificate was ambiguous. On the one
hand, it stated that the certificate with the same name will be
overwritten. On the other hand, it stated that it will thrown an
exception if certificate with the same name exists. This commit ensures
the former behavior, which is consistent with the old KeyChain.
Change-Id: I911f7c86c634caf260ecb9a5dbdf13b066f8711f
diff --git a/tests/unit-tests/security/pib/detail/identity-impl.t.cpp b/tests/unit-tests/security/pib/detail/identity-impl.t.cpp
index 10c7bb8..2f35d2f 100644
--- a/tests/unit-tests/security/pib/detail/identity-impl.t.cpp
+++ b/tests/unit-tests/security/pib/detail/identity-impl.t.cpp
@@ -121,6 +121,18 @@
BOOST_CHECK_THROW(identity1.getDefaultKey(), Pib::Error);
}
+BOOST_AUTO_TEST_CASE(Overwrite)
+{
+ auto pibImpl = make_shared<pib::PibMemory>();
+ IdentityImpl identity1(id1, pibImpl, true);
+
+ identity1.addKey(id1Key1.buf(), id1Key1.size(), id1Key1Name);
+ BOOST_CHECK(identity1.getKey(id1Key1Name).getPublicKey() == id1Key1);
+
+ identity1.addKey(id1Key2.buf(), id1Key2.size(), id1Key1Name); // overwriting key should work
+ BOOST_CHECK(identity1.getKey(id1Key1Name).getPublicKey() == id1Key2);
+}
+
BOOST_AUTO_TEST_CASE(Errors)
{
auto pibImpl = make_shared<pib::PibMemory>();
@@ -129,7 +141,6 @@
IdentityImpl identity1(id1, pibImpl, true);
identity1.addKey(id1Key1.buf(), id1Key1.size(), id1Key1Name);
- BOOST_CHECK_THROW(identity1.addKey(id1Key1.buf(), id1Key1.size(), id1Key1Name), Pib::Error);
BOOST_CHECK_THROW(identity1.addKey(id2Key1.buf(), id2Key1.size(), 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-tests/security/pib/detail/key-impl.t.cpp b/tests/unit-tests/security/pib/detail/key-impl.t.cpp
index b25a870..5257113 100644
--- a/tests/unit-tests/security/pib/detail/key-impl.t.cpp
+++ b/tests/unit-tests/security/pib/detail/key-impl.t.cpp
@@ -25,6 +25,7 @@
#include "../pib-data-fixture.hpp"
#include "boost-test.hpp"
+#include "identity-management-fixture.hpp"
namespace ndn {
namespace security {
@@ -130,13 +131,49 @@
BOOST_CHECK_EQUAL(key11.getCertificates().size(), 0);
}
+class OverwriteFixture : public ndn::security::tests::PibDataFixture,
+ public ndn::tests::IdentityManagementFixture
+{
+};
+
+BOOST_FIXTURE_TEST_CASE(Overwrite, OverwriteFixture)
+{
+ auto pibImpl = make_shared<pib::PibMemory>();
+
+ BOOST_CHECK_THROW(KeyImpl(id1Key1Name, pibImpl), Pib::Error);
+ KeyImpl(id1Key1Name, id1Key1.buf(), id1Key1.size(), pibImpl);
+ KeyImpl key1(id1Key1Name, pibImpl);
+
+ KeyImpl(id1Key1Name, id1Key2.buf(), id1Key2.size(), pibImpl); // overwriting of the key should work
+ KeyImpl key2(id1Key1Name, pibImpl);
+
+ BOOST_CHECK(key1.getPublicKey() != key2.getPublicKey()); // key1 cached the original public key
+ BOOST_CHECK(key2.getPublicKey() == id1Key2);
+
+ key1.addCertificate(id1Key1Cert1);
+ BOOST_CHECK_EQUAL(key1.getCertificate(id1Key1Cert1.getName()), id1Key1Cert1);
+
+ auto otherCert = id1Key1Cert1;
+ SignatureInfo info;
+ info.setValidityPeriod(ValidityPeriod(time::system_clock::now(),
+ time::system_clock::now() + time::seconds(1)));
+ m_keyChain.sign(otherCert, SigningInfo().setSignatureInfo(info));
+
+ BOOST_CHECK_EQUAL(otherCert.getName(), id1Key1Cert1.getName());
+ BOOST_CHECK(otherCert.getContent() == id1Key1Cert1.getContent());
+ BOOST_CHECK_NE(otherCert, id1Key1Cert1);
+
+ key1.addCertificate(otherCert);
+
+ BOOST_CHECK_EQUAL(key1.getCertificate(id1Key1Cert1.getName()), otherCert);
+}
+
BOOST_AUTO_TEST_CASE(Errors)
{
auto pibImpl = make_shared<pib::PibMemory>();
BOOST_CHECK_THROW(KeyImpl(id1Key1Name, pibImpl), Pib::Error);
KeyImpl key11(id1Key1Name, id1Key1.buf(), id1Key1.size(), pibImpl);
- BOOST_CHECK_THROW(KeyImpl(id1Key1Name, id1Key1.buf(), id1Key1.size(), pibImpl), Pib::Error);
BOOST_CHECK_THROW(KeyImpl(Name("/wrong"), pibImpl), std::invalid_argument);
BOOST_CHECK_THROW(KeyImpl(Name("/wrong"), id1Key1.buf(), id1Key1.size(), pibImpl), std::invalid_argument);
@@ -144,7 +181,6 @@
BOOST_CHECK_THROW(KeyImpl(id1Key2Name, wrongKey.buf(), wrongKey.size(), pibImpl), std::invalid_argument);
key11.addCertificate(id1Key1Cert1);
- BOOST_CHECK_THROW(key11.addCertificate(id1Key1Cert1), Pib::Error);
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);
diff --git a/tests/unit-tests/security/v2/key-chain.t.cpp b/tests/unit-tests/security/v2/key-chain.t.cpp
index 950b97e..4eaf75e 100644
--- a/tests/unit-tests/security/v2/key-chain.t.cpp
+++ b/tests/unit-tests/security/v2/key-chain.t.cpp
@@ -228,8 +228,7 @@
m_keyChain.addCertificate(key3, key3Cert1);
BOOST_CHECK_EQUAL(key3.getCertificates().size(), 1);
BOOST_REQUIRE_NO_THROW(key3.getDefaultCertificate());
- // Overwrite the same cert again, should throw Pib::Error.
- BOOST_REQUIRE_THROW(m_keyChain.addCertificate(key3, key3Cert1), Pib::Error);
+ m_keyChain.addCertificate(key3, key3Cert1); // overwriting the cert should work
BOOST_CHECK_EQUAL(key3.getCertificates().size(), 1);
// Add another cert
Certificate key3Cert2 = key3Cert1;