security: Make addKey/Cert behavior of PibImpl consistent
Change-Id: I30c6dd0007e9095ee07f7a8eda3b20db4c34c513
Refs: #3351
diff --git a/src/security/pib/pib-impl.hpp b/src/security/pib/pib-impl.hpp
index 9b343f5..ef859a6 100644
--- a/src/security/pib/pib-impl.hpp
+++ b/src/security/pib/pib-impl.hpp
@@ -86,8 +86,8 @@
/**
* @brief Add an identity.
*
- * If the identity already exists, do nothing.
- * If no default identity has been set, set the added one as default identity.
+ * If the identity already exists, do nothing. If no default identity has been set, set the
+ * added one as default identity.
*
* @param identity The name of the identity to add.
*/
@@ -95,10 +95,10 @@
addIdentity(const Name& identity) = 0;
/**
- * @brief Remove an identity
+ * @brief Remove an identity and related keys and certificates.
*
- * If the identity does not exist, do nothing.
- * Remove related keys and certificates as well.
+ * If the default identity is being removed, no default identity will be selected. If the
+ * identity does not exist, do nothing.
*
* @param identity The name of the identity to remove.
*/
@@ -120,8 +120,7 @@
/**
* @brief Set an identity with name @p identityName as the default identity.
*
- * Since adding an identity only requires the identity name, create the
- * identity if it does not exist.
+ * If @p identityName identity does not exist, it will be created.
*
* @param identityName The name for the default identity.
*/
@@ -149,10 +148,10 @@
/**
* @brief Add a key.
*
- * If the key already exists, do nothing.
- * If the identity does not exist, add the identity as well.
- * If no default key of the identity has been set, set the added one as default
- * key of the identity.
+ * If a key with the same name already exists, overwrite the key. If the identity does not
+ * exist, it will be created. If no default key of the identity has been set, set the added
+ * one as default key of the identity. If no default identity has been set, @p identity
+ * becomes the default.
*
* @param identity The name of the belonged identity.
* @param keyName The key name.
@@ -163,10 +162,9 @@
addKey(const Name& identity, const Name& keyName, const uint8_t* key, size_t keyLen) = 0;
/**
- * @brief Remove a key with @p keyName
+ * @brief Remove a key with @p keyName and related certificates
*
* If the key does not exist, do nothing.
- * Remove related certificates as well.
*/
virtual void
removeKey(const Name& keyName) = 0;
@@ -183,8 +181,8 @@
/**
* @brief Get all the key names of an identity with name @p identity
*
- * The returned key names can be used to create a KeyContainer.
- * With key name, identity name, backend implementation, one can create a Key frontend instance.
+ * The returned key names can be used to create a KeyContainer. With key name and backend
+ * implementation, one can create a Key frontend instance.
*
* @return the key name component set. If the identity does not exist, return an empty set.
*/
@@ -220,10 +218,12 @@
/**
* @brief Add a certificate.
*
- * If the certificate already exists, do nothing.
- * If the key or identity do not exist, add them as well.
- * If no default certificate of the key has been set, set the added one as
- * default certificate of the key.
+ * If a certificate with the same name (without implicit digest) already exists, overwrite
+ * the certificate. If the key or identity does not exist, they will be created. If no
+ * default certificate of the key has been set, set the added one as default certificate of
+ * the key. If no default key was set for the identity, it will be set as default key for
+ * the identity. If no default identity was selected, the certificate's identity becomes
+ * default.
*
* @param certificate The certificate to add.
*/
@@ -253,8 +253,8 @@
/**
* @brief Get a list of certificate names of a key with id @p keyName.
*
- * The returned certificate names can be used to create a CertificateContainer.
- * With certificate name and backend implementation, one can obtain the certificate directly.
+ * The returned certificate names can be used to create a CertificateContainer. With
+ * certificate name and backend implementation, one can obtain the certificate.
*
* @return The certificate name set. If the key does not exist, return an empty set.
*/
diff --git a/src/security/pib/pib-memory.cpp b/src/security/pib/pib-memory.cpp
index edf2ce9..aa93ba7 100644
--- a/src/security/pib/pib-memory.cpp
+++ b/src/security/pib/pib-memory.cpp
@@ -65,12 +65,14 @@
PibMemory::removeIdentity(const Name& identity)
{
m_identities.erase(identity);
- if (identity == m_defaultIdentity)
+ if (identity == m_defaultIdentity) {
m_hasDefaultIdentity = false;
+ m_defaultIdentity.clear();
+ }
- auto keyNames = this->getKeysOfIdentity(identity);
+ auto keyNames = getKeysOfIdentity(identity);
for (const Name& keyName : keyNames) {
- this->removeKey(keyName);
+ removeKey(keyName);
}
}
@@ -80,9 +82,9 @@
m_hasDefaultIdentity = false;
m_defaultIdentity.clear();
m_identities.clear();
- m_defaultKey.clear();
+ m_defaultKeys.clear();
m_keys.clear();
- m_defaultCert.clear();
+ m_defaultCerts.clear();
m_certs.clear();
}
@@ -103,8 +105,9 @@
Name
PibMemory::getDefaultIdentity() const
{
- if (m_hasDefaultIdentity)
+ if (m_hasDefaultIdentity) {
return m_defaultIdentity;
+ }
BOOST_THROW_EXCEPTION(Pib::Error("No default identity"));
}
@@ -119,12 +122,13 @@
PibMemory::addKey(const Name& identity, const Name& keyName,
const uint8_t* key, size_t keyLen)
{
- this->addIdentity(identity);
+ addIdentity(identity);
m_keys[keyName] = Buffer(key, keyLen);
- if (m_defaultKey.find(identity) == m_defaultKey.end())
- m_defaultKey[identity] = keyName;
+ if (m_defaultKeys.count(identity) == 0) {
+ m_defaultKeys[identity] = keyName;
+ }
}
void
@@ -133,31 +137,34 @@
Name identity = v2::extractIdentityFromKeyName(keyName);
m_keys.erase(keyName);
- m_defaultKey.erase(identity);
+ m_defaultKeys.erase(identity);
- auto certNames = this->getCertificatesOfKey(keyName);
+ auto certNames = getCertificatesOfKey(keyName);
for (const auto& certName : certNames) {
- this->removeCertificate(certName);
+ removeCertificate(certName);
}
}
Buffer
PibMemory::getKeyBits(const Name& keyName) const
{
- if (!hasKey(keyName))
+ if (!hasKey(keyName)) {
BOOST_THROW_EXCEPTION(Pib::Error("Key `" + keyName.toUri() + "` not found"));
+ }
- auto it = m_keys.find(keyName);
- return it->second;
+ auto key = m_keys.find(keyName);
+ BOOST_ASSERT(key != m_keys.end());
+ return key->second;
}
std::set<Name>
PibMemory::getKeysOfIdentity(const Name& identity) const
{
std::set<Name> ids;
- for (const auto& it : m_keys) {
- if (identity == v2::extractIdentityFromKeyName(it.first))
- ids.insert(it.first);
+ for (const auto& key : m_keys) {
+ if (identity == v2::extractIdentityFromKeyName(key.first)) {
+ ids.insert(key.first);
+ }
}
return ids;
}
@@ -165,20 +172,22 @@
void
PibMemory::setDefaultKeyOfIdentity(const Name& identity, const Name& keyName)
{
- if (!hasKey(keyName))
+ if (!hasKey(keyName)) {
BOOST_THROW_EXCEPTION(Pib::Error("Key `" + keyName.toUri() + "` not found"));
+ }
- m_defaultKey[identity] = keyName;
+ m_defaultKeys[identity] = keyName;
}
Name
PibMemory::getDefaultKeyOfIdentity(const Name& identity) const
{
- auto it = m_defaultKey.find(identity);
- if (it == m_defaultKey.end())
+ auto defaultKey = m_defaultKeys.find(identity);
+ if (defaultKey == m_defaultKeys.end()) {
BOOST_THROW_EXCEPTION(Pib::Error("No default key for identity `" + identity.toUri() + "`"));
+ }
- return it->second;
+ return defaultKey->second;
}
bool
@@ -194,25 +203,30 @@
Name keyName = certificate.getKeyName();
Name identity = certificate.getIdentity();
- this->addKey(identity, keyName, certificate.getContent().value(), certificate.getContent().value_size());
+ addKey(identity, keyName, certificate.getContent().value(), certificate.getContent().value_size());
m_certs[certName] = certificate;
- if (m_defaultCert.find(keyName) == m_defaultCert.end())
- m_defaultCert[keyName] = certName;
+ if (m_defaultCerts.count(keyName) == 0) {
+ m_defaultCerts[keyName] = certName;
+ }
}
void
PibMemory::removeCertificate(const Name& certName)
{
m_certs.erase(certName);
- m_defaultCert.erase(v2::extractKeyNameFromCertName(certName));
+ auto defaultCert = m_defaultCerts.find(v2::extractKeyNameFromCertName(certName));
+ if (defaultCert != m_defaultCerts.end() && defaultCert->second == certName) {
+ m_defaultCerts.erase(defaultCert);
+ }
}
v2::Certificate
PibMemory::getCertificate(const Name& certName) const
{
- if (!hasCertificate(certName))
+ if (!hasCertificate(certName)) {
BOOST_THROW_EXCEPTION(Pib::Error("Certificate `" + certName.toUri() + "` does not exist"));
+ }
auto it = m_certs.find(certName);
return it->second;
@@ -223,8 +237,9 @@
{
std::set<Name> certNames;
for (const auto& it : m_certs) {
- if (v2::extractKeyNameFromCertName(it.second.getName()) == keyName)
+ if (v2::extractKeyNameFromCertName(it.second.getName()) == keyName) {
certNames.insert(it.first);
+ }
}
return certNames;
}
@@ -232,24 +247,24 @@
void
PibMemory::setDefaultCertificateOfKey(const Name& keyName, const Name& certName)
{
- if (!hasCertificate(certName))
+ if (!hasCertificate(certName)) {
BOOST_THROW_EXCEPTION(Pib::Error("Certificate `" + certName.toUri() + "` does not exist"));
+ }
- m_defaultCert[keyName] = certName;
+ m_defaultCerts[keyName] = certName;
}
v2::Certificate
PibMemory::getDefaultCertificateOfKey(const Name& keyName) const
{
- auto it = m_defaultCert.find(keyName);
- if (it == m_defaultCert.end())
+ auto it = m_defaultCerts.find(keyName);
+ if (it == m_defaultCerts.end()) {
BOOST_THROW_EXCEPTION(Pib::Error("No default certificate for key `" + keyName.toUri() + "`"));
+ }
auto certIt = m_certs.find(it->second);
- if (certIt == m_certs.end())
- BOOST_THROW_EXCEPTION(Pib::Error("No default certificate for key `" + keyName.toUri() + "`"));
- else
- return certIt->second;
+ BOOST_ASSERT(certIt != m_certs.end());
+ return certIt->second;
}
} // namespace pib
diff --git a/src/security/pib/pib-memory.hpp b/src/security/pib/pib-memory.hpp
index c48f9fc..192f000 100644
--- a/src/security/pib/pib-memory.hpp
+++ b/src/security/pib/pib-memory.hpp
@@ -132,13 +132,13 @@
std::set<Name> m_identities;
/// @brief identity => default key Name
- std::map<Name, Name> m_defaultKey;
+ std::map<Name, Name> m_defaultKeys;
/// @brief keyName => keyBits
std::map<Name, Buffer> m_keys;
/// @brief keyName => default certificate Name
- std::map<Name, Name> m_defaultCert;
+ std::map<Name, Name> m_defaultCerts;
/// @brief certificate Name => certificate
std::map<Name, v2::Certificate> m_certs;
diff --git a/src/security/pib/pib-sqlite3.cpp b/src/security/pib/pib-sqlite3.cpp
index 8d8c437..b19f32c 100644
--- a/src/security/pib/pib-sqlite3.cpp
+++ b/src/security/pib/pib-sqlite3.cpp
@@ -279,9 +279,15 @@
void
PibSqlite3::addIdentity(const Name& identity)
{
- Sqlite3Statement statement(m_database, "INSERT INTO identities (identity) values (?)");
- statement.bind(1, identity.wireEncode(), SQLITE_TRANSIENT);
- statement.step();
+ if (!hasIdentity(identity)) {
+ Sqlite3Statement statement(m_database, "INSERT INTO identities (identity) values (?)");
+ statement.bind(1, identity.wireEncode(), SQLITE_TRANSIENT);
+ statement.step();
+ }
+
+ if (!hasDefaultIdentity()) {
+ setDefaultIdentity(identity);
+ }
}
void
@@ -331,6 +337,13 @@
}
bool
+PibSqlite3::hasDefaultIdentity() const
+{
+ Sqlite3Statement statement(m_database, "SELECT identity FROM identities WHERE is_default=1");
+ return (statement.step() == SQLITE_ROW);
+}
+
+bool
PibSqlite3::hasKey(const Name& keyName) const
{
Sqlite3Statement statement(m_database, "SELECT id FROM keys WHERE key_name=?");
@@ -343,20 +356,29 @@
PibSqlite3::addKey(const Name& identity, const Name& keyName,
const uint8_t* key, size_t keyLen)
{
- if (hasKey(keyName)) {
- return;
- }
-
// ensure identity exists
addIdentity(identity);
- Sqlite3Statement statement(m_database,
- "INSERT INTO keys (identity_id, key_name, key_bits) "
- "VALUES ((SELECT id FROM identities WHERE identity=?), ?, ?)");
- statement.bind(1, identity.wireEncode(), SQLITE_TRANSIENT);
- statement.bind(2, keyName.wireEncode(), SQLITE_TRANSIENT);
- statement.bind(3, key, keyLen, SQLITE_STATIC);
- statement.step();
+ if (!hasKey(keyName)) {
+ Sqlite3Statement statement(m_database,
+ "INSERT INTO keys (identity_id, key_name, key_bits) "
+ "VALUES ((SELECT id FROM identities WHERE identity=?), ?, ?)");
+ statement.bind(1, identity.wireEncode(), SQLITE_TRANSIENT);
+ statement.bind(2, keyName.wireEncode(), SQLITE_TRANSIENT);
+ statement.bind(3, key, keyLen, SQLITE_STATIC);
+ statement.step();
+ }
+ else {
+ Sqlite3Statement statement(m_database,
+ "UPDATE keys SET key_bits=? WHERE key_name=?");
+ statement.bind(1, key, keyLen, SQLITE_STATIC);
+ statement.bind(2, keyName.wireEncode(), SQLITE_TRANSIENT);
+ statement.step();
+ }
+
+ if (!hasDefaultKeyOfIdentity(identity)) {
+ setDefaultKeyOfIdentity(identity, keyName);
+ }
}
void
@@ -430,6 +452,18 @@
}
bool
+PibSqlite3::hasDefaultKeyOfIdentity(const Name& identity) const
+{
+ Sqlite3Statement statement(m_database,
+ "SELECT key_name "
+ "FROM keys JOIN identities ON keys.identity_id=identities.id "
+ "WHERE identities.identity=? AND keys.is_default=1");
+ statement.bind(1, identity.wireEncode(), SQLITE_TRANSIENT);
+
+ return (statement.step() == SQLITE_ROW);
+}
+
+bool
PibSqlite3::hasCertificate(const Name& certName) const
{
Sqlite3Statement statement(m_database, "SELECT id FROM certificates WHERE certificate_name=?");
@@ -444,14 +478,27 @@
const Block& content = certificate.getContent();
addKey(certificate.getIdentity(), certificate.getKeyName(), content.value(), content.value_size());
- Sqlite3Statement statement(m_database,
- "INSERT INTO certificates "
- "(key_id, certificate_name, certificate_data) "
- "VALUES ((SELECT id FROM keys WHERE key_name=?), ?, ?)");
- statement.bind(1, certificate.getKeyName().wireEncode(), SQLITE_TRANSIENT);
- statement.bind(2, certificate.getName().wireEncode(), SQLITE_TRANSIENT);
- statement.bind(3, certificate.wireEncode(), SQLITE_STATIC);
- statement.step();
+ if (!hasCertificate(certificate.getName())) {
+ Sqlite3Statement statement(m_database,
+ "INSERT INTO certificates "
+ "(key_id, certificate_name, certificate_data) "
+ "VALUES ((SELECT id FROM keys WHERE key_name=?), ?, ?)");
+ statement.bind(1, certificate.getKeyName().wireEncode(), SQLITE_TRANSIENT);
+ statement.bind(2, certificate.getName().wireEncode(), SQLITE_TRANSIENT);
+ statement.bind(3, certificate.wireEncode(), SQLITE_STATIC);
+ statement.step();
+ }
+ else {
+ Sqlite3Statement statement(m_database,
+ "UPDATE certificates SET certificate_data=? WHERE certificate_name=?");
+ statement.bind(1, certificate.wireEncode(), SQLITE_STATIC);
+ statement.bind(2, certificate.getName().wireEncode(), SQLITE_TRANSIENT);
+ statement.step();
+ }
+
+ if (!hasDefaultCertificateOfKey(certificate.getKeyName())) {
+ setDefaultCertificateOfKey(certificate.getKeyName(), certificate.getName());
+ }
}
void
@@ -520,6 +567,18 @@
BOOST_THROW_EXCEPTION(Pib::Error("No default certificate for key `" + keyName.toUri() + "`"));
}
+bool
+PibSqlite3::hasDefaultCertificateOfKey(const Name& keyName) const
+{
+ Sqlite3Statement statement(m_database,
+ "SELECT certificate_data "
+ "FROM certificates JOIN keys ON certificates.key_id=keys.id "
+ "WHERE certificates.is_default=1 AND keys.key_name=?");
+ statement.bind(1, keyName.wireEncode(), SQLITE_TRANSIENT);
+
+ return (statement.step() == SQLITE_ROW);
+}
+
} // namespace pib
} // namespace security
} // namespace ndn
diff --git a/src/security/pib/pib-sqlite3.hpp b/src/security/pib/pib-sqlite3.hpp
index 806a29c..8c47176 100644
--- a/src/security/pib/pib-sqlite3.hpp
+++ b/src/security/pib/pib-sqlite3.hpp
@@ -134,6 +134,16 @@
getDefaultCertificateOfKey(const Name& keyName) const final;
private:
+ bool
+ hasDefaultIdentity() const;
+
+ bool
+ hasDefaultKeyOfIdentity(const Name& identity) const;
+
+ bool
+ hasDefaultCertificateOfKey(const Name& keyName) const;
+
+private:
sqlite3* m_database;
};
diff --git a/src/security/pib/pib.hpp b/src/security/pib/pib.hpp
index 3d73098..06f100d 100644
--- a/src/security/pib/pib.hpp
+++ b/src/security/pib/pib.hpp
@@ -128,7 +128,6 @@
getDefaultIdentity() const;
NDN_CXX_PUBLIC_WITH_TESTS_ELSE_PRIVATE: // write operations should be private
-
/*
* @brief Create an identity with name @p identityName and return a reference to it.
*
@@ -143,6 +142,8 @@
/*
* @brief Remove an identity with name @p identityName.
*
+ * If the default identity is being removed, no default identity will be selected.
+ *
* @param identityName The name for the identity to be deleted
*/
void