security: modernize Identity and IdentityContainer; add logging
Change-Id: I1c8645082456de4cc8012ac97a1b95ab1ddbf835
diff --git a/docs/doxygen.conf.in b/docs/doxygen.conf.in
index 5c857c5..dab1abb 100644
--- a/docs/doxygen.conf.in
+++ b/docs/doxygen.conf.in
@@ -830,7 +830,7 @@
# Note that the wildcards are matched against the file with absolute path, so to
# exclude all test directories for example use the pattern */test/*
-EXCLUDE_PATTERNS = */ndn-cxx/impl/* */ndn-cxx/*/impl/* */ndn-cxx/util/nonstd/*
+EXCLUDE_PATTERNS = */ndn-cxx/impl/* */ndn-cxx/*/impl/* */ndn-cxx/detail/nonstd/*
# The EXCLUDE_SYMBOLS tag can be used to specify one or more symbol names
# (namespaces, classes, functions, etc.) that should be excluded from the
diff --git a/ndn-cxx/security/pib/identity-container.cpp b/ndn-cxx/security/pib/identity-container.cpp
index 3353ab9..c73af9d 100644
--- a/ndn-cxx/security/pib/identity-container.cpp
+++ b/ndn-cxx/security/pib/identity-container.cpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2013-2018 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).
*
@@ -20,23 +20,21 @@
*/
#include "ndn-cxx/security/pib/identity-container.hpp"
-#include "ndn-cxx/security/pib/pib-impl.hpp"
#include "ndn-cxx/security/pib/impl/identity-impl.hpp"
+#include "ndn-cxx/security/pib/pib-impl.hpp"
#include "ndn-cxx/util/concepts.hpp"
+#include "ndn-cxx/util/logger.hpp"
namespace ndn {
namespace security {
namespace pib {
+NDN_LOG_INIT(ndn.security.IdentityContainer);
+
NDN_CXX_ASSERT_FORWARD_ITERATOR(IdentityContainer::const_iterator);
-IdentityContainer::const_iterator::const_iterator()
- : m_container(nullptr)
-{
-}
-
-IdentityContainer::const_iterator::const_iterator(std::set<Name>::const_iterator it,
- const IdentityContainer& container)
+IdentityContainer::const_iterator::const_iterator(NameSet::const_iterator it,
+ const IdentityContainer& container) noexcept
: m_it(it)
, m_container(&container)
{
@@ -49,113 +47,86 @@
return m_container->get(*m_it);
}
-IdentityContainer::const_iterator&
-IdentityContainer::const_iterator::operator++()
-{
- ++m_it;
- return *this;
-}
-
-IdentityContainer::const_iterator
-IdentityContainer::const_iterator::operator++(int)
-{
- const_iterator it(*this);
- ++m_it;
- return it;
-}
-
bool
-IdentityContainer::const_iterator::operator==(const const_iterator& other)
+IdentityContainer::const_iterator::operator==(const const_iterator& other) const
{
bool isThisEnd = m_container == nullptr || m_it == m_container->m_identityNames.end();
bool isOtherEnd = other.m_container == nullptr || other.m_it == other.m_container->m_identityNames.end();
- return ((isThisEnd || isOtherEnd) ?
- (isThisEnd == isOtherEnd) :
- m_container->m_pibImpl == other.m_container->m_pibImpl && m_it == other.m_it);
-}
-
-bool
-IdentityContainer::const_iterator::operator!=(const const_iterator& other)
-{
- return !(*this == other);
+ if (isThisEnd)
+ return isOtherEnd;
+ return !isOtherEnd && m_container->m_pib == other.m_container->m_pib && m_it == other.m_it;
}
IdentityContainer::IdentityContainer(shared_ptr<PibImpl> pibImpl)
- : m_pibImpl(std::move(pibImpl))
+ : m_pib(std::move(pibImpl))
{
- BOOST_ASSERT(m_pibImpl != nullptr);
- m_identityNames = m_pibImpl->getIdentities();
-}
-
-IdentityContainer::const_iterator
-IdentityContainer::begin() const
-{
- return const_iterator(m_identityNames.begin(), *this);
-}
-
-IdentityContainer::const_iterator
-IdentityContainer::end() const
-{
- return const_iterator();
+ BOOST_ASSERT(m_pib != nullptr);
+ m_identityNames = m_pib->getIdentities();
}
IdentityContainer::const_iterator
IdentityContainer::find(const Name& identity) const
{
- return const_iterator(m_identityNames.find(identity), *this);
-}
-
-size_t
-IdentityContainer::size() const
-{
- return m_identityNames.size();
+ return {m_identityNames.find(identity), *this};
}
Identity
IdentityContainer::add(const Name& identityName)
{
- if (m_identityNames.count(identityName) == 0) {
- m_identityNames.insert(identityName);
- m_identities[identityName] = make_shared<detail::IdentityImpl>(identityName, m_pibImpl, true);
+ bool didInsert = m_identityNames.insert(identityName).second;
+ if (!didInsert) {
+ // identity already exists
+ return get(identityName);
}
- return get(identityName);
+
+ NDN_LOG_DEBUG("Adding " << identityName);
+ auto ret = m_identities.emplace(identityName,
+ std::make_shared<detail::IdentityImpl>(identityName, m_pib, true));
+ // consistency check
+ BOOST_ASSERT(ret.second);
+
+ return Identity(ret.first->second);
}
void
IdentityContainer::remove(const Name& identityName)
{
- m_identityNames.erase(identityName);
- m_identities.erase(identityName);
- m_pibImpl->removeIdentity(identityName);
+ if (m_identityNames.erase(identityName) > 0) {
+ NDN_LOG_DEBUG("Removing " << identityName);
+ m_identities.erase(identityName);
+ }
+ else {
+ // consistency check
+ BOOST_ASSERT(m_identities.find(identityName) == m_identities.end());
+ }
+ m_pib->removeIdentity(identityName);
}
Identity
IdentityContainer::get(const Name& identityName) const
{
- shared_ptr<detail::IdentityImpl> id;
auto it = m_identities.find(identityName);
-
if (it != m_identities.end()) {
- id = it->second;
+ return Identity(it->second);
}
- else {
- id = make_shared<detail::IdentityImpl>(identityName, m_pibImpl, false);
- m_identities[identityName] = id;
- }
+
+ auto id = std::make_shared<detail::IdentityImpl>(identityName, m_pib, false);
+ m_identities[identityName] = id;
return Identity(id);
}
void
IdentityContainer::reset()
{
+ NDN_LOG_DEBUG("Reloading");
m_identities.clear();
- m_identityNames = m_pibImpl->getIdentities();
+ m_identityNames = m_pib->getIdentities();
}
bool
IdentityContainer::isConsistent() const
{
- return m_identityNames == m_pibImpl->getIdentities();
+ return m_identityNames == m_pib->getIdentities();
}
} // namespace pib
diff --git a/ndn-cxx/security/pib/identity-container.hpp b/ndn-cxx/security/pib/identity-container.hpp
index 359e358..b354ade 100644
--- a/ndn-cxx/security/pib/identity-container.hpp
+++ b/ndn-cxx/security/pib/identity-container.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).
*
@@ -32,6 +32,7 @@
namespace security {
namespace pib {
+class Pib;
class PibImpl;
namespace detail {
@@ -39,13 +40,18 @@
} // namespace detail
/**
- * @brief Container of identities of a Pib
+ * @brief Container of identities of a PIB.
*
- * The container is used to search/enumerate identities of a Pib.
- * The container can be created only by Pib.
+ * The container is used to search/enumerate the identities in a PIB.
+ * It can be created only by the Pib class.
+ *
+ * @sa Pib::getIdentities()
*/
class IdentityContainer : noncopyable
{
+private:
+ using NameSet = std::set<Name>;
+
public:
class const_iterator
{
@@ -56,118 +62,135 @@
using pointer = value_type*;
using reference = value_type&;
- const_iterator();
+ const_iterator() = default;
Identity
operator*();
const_iterator&
- operator++();
+ operator++()
+ {
+ ++m_it;
+ return *this;
+ }
const_iterator
- operator++(int);
+ operator++(int)
+ {
+ const_iterator it(*this);
+ ++m_it;
+ return it;
+ }
bool
- operator==(const const_iterator& other);
+ operator==(const const_iterator& other) const;
bool
- operator!=(const const_iterator& other);
+ operator!=(const const_iterator& other) const
+ {
+ return !this->operator==(other);
+ }
private:
- const_iterator(std::set<Name>::const_iterator it, const IdentityContainer& container);
+ const_iterator(NameSet::const_iterator it, const IdentityContainer& container) noexcept;
private:
- std::set<Name>::const_iterator m_it;
- const IdentityContainer* m_container;
+ NameSet::const_iterator m_it;
+ const IdentityContainer* m_container = nullptr;
- friend class IdentityContainer;
+ friend IdentityContainer;
};
- typedef const_iterator iterator;
+ using iterator = const_iterator;
public:
const_iterator
- begin() const;
+ begin() const noexcept
+ {
+ return {m_identityNames.begin(), *this};
+ }
const_iterator
- end() const;
+ end() const noexcept
+ {
+ return {};
+ }
const_iterator
- find(const Name& keyId) const;
-
- size_t
- size() const;
+ find(const Name& identity) const;
/**
- * @brief Add @p identity into the container
+ * @brief Check whether the container is empty.
+ */
+ NDN_CXX_NODISCARD bool
+ empty() const noexcept
+ {
+ return m_identityNames.empty();
+ }
+
+ /**
+ * @brief Return the number of identities in the container.
+ */
+ size_t
+ size() const noexcept
+ {
+ return m_identityNames.size();
+ }
+
+ /**
+ * @brief Add @p identity into the container.
*/
Identity
- add(const Name& identityName);
+ add(const Name& identity);
/**
- * @brief Remove @p identity from the container
+ * @brief Remove @p identity from the container.
*/
void
remove(const Name& identity);
/**
- * @brief Get @p identity from the container
- * @throw Pib::Error @p identity does not exist
+ * @brief Return an identity by name.
+ * @throw Pib::Error The identity does not exist.
*/
Identity
get(const Name& identity) const;
/**
- * @brief Reset state of the container
+ * @brief Reset the state of the container.
*
- * This method removes all loaded identities and retrieves identity names from the PIB
- * implementation.
+ * This clears all cached information and reloads the identity names from the PIB backend.
*/
void
reset();
/**
- * @brief Check if the container is consistent with the backend storage
- *
- * @note this method is heavyweight and should be used in debugging mode only.
+ * @brief Check if the container is consistent with the backend storage.
+ * @note This method is heavyweight and should be used in debugging mode only.
*/
bool
isConsistent() const;
-NDN_CXX_PUBLIC_WITH_TESTS_ELSE_PRIVATE:
+NDN_CXX_PUBLIC_WITH_TESTS_ELSE_PRIVATE: // private interface for Pib
/**
- * @brief Create identity container
- * @param impl The PIB backend implementation.
+ * @brief Create identity container.
+ * @param pibImpl The PIB backend implementation.
*/
explicit
IdentityContainer(shared_ptr<PibImpl> pibImpl);
- const std::set<Name>&
- getIdentityNames() const
- {
- return m_identityNames;
- }
-
- const std::unordered_map<Name, shared_ptr<detail::IdentityImpl>>&
- getLoadedIdentities() const
- {
- return m_identities;
- }
-
-private:
- std::set<Name> m_identityNames;
- /// @brief Cache of loaded detail::IdentityImpl.
+NDN_CXX_PUBLIC_WITH_TESTS_ELSE_PRIVATE:
+ // cache of loaded IdentityImpl
mutable std::unordered_map<Name, shared_ptr<detail::IdentityImpl>> m_identities;
- shared_ptr<PibImpl> m_pibImpl;
+private:
+ NameSet m_identityNames;
+ const shared_ptr<PibImpl> m_pib;
- friend class Pib;
+ friend Pib;
};
} // namespace pib
-
-using pib::IdentityContainer;
-
} // namespace security
} // namespace ndn
diff --git a/ndn-cxx/security/pib/identity.cpp b/ndn-cxx/security/pib/identity.cpp
index 8776ad2..41e103b 100644
--- a/ndn-cxx/security/pib/identity.cpp
+++ b/ndn-cxx/security/pib/identity.cpp
@@ -26,9 +26,9 @@
namespace security {
namespace pib {
-Identity::Identity() = default;
+Identity::Identity() noexcept = default;
-Identity::Identity(weak_ptr<detail::IdentityImpl> impl)
+Identity::Identity(weak_ptr<detail::IdentityImpl> impl) noexcept
: m_impl(std::move(impl))
{
}
@@ -63,25 +63,25 @@
return lock()->getKeys();
}
-const Key&
+Key
Identity::setDefaultKey(const Name& keyName) const
{
return lock()->setDefaultKey(keyName);
}
-const Key&
+Key
Identity::setDefaultKey(span<const uint8_t> key, const Name& keyName) const
{
return lock()->setDefaultKey(key, keyName);
}
-const Key&
+Key
Identity::getDefaultKey() const
{
return lock()->getDefaultKey();
}
-Identity::operator bool() const
+Identity::operator bool() const noexcept
{
return !m_impl.expired();
}
@@ -90,29 +90,17 @@
Identity::lock() const
{
auto impl = m_impl.lock();
-
- if (impl == nullptr)
- NDN_THROW(std::domain_error("Invalid Identity instance"));
-
+ if (impl == nullptr) {
+ NDN_THROW(std::domain_error("Invalid PIB identity instance"));
+ }
return impl;
}
bool
-operator!=(const Identity& lhs, const Identity& rhs)
+Identity::equals(const Identity& other) const noexcept
{
- return lhs.m_impl.owner_before(rhs.m_impl) || rhs.m_impl.owner_before(lhs.m_impl);
-}
-
-std::ostream&
-operator<<(std::ostream& os, const Identity& id)
-{
- if (id) {
- os << id.getName();
- }
- else {
- os << "(empty)";
- }
- return os;
+ return !this->m_impl.owner_before(other.m_impl) &&
+ !other.m_impl.owner_before(this->m_impl);
}
} // namespace pib
diff --git a/ndn-cxx/security/pib/identity.hpp b/ndn-cxx/security/pib/identity.hpp
index d39d161..0bf3821 100644
--- a/ndn-cxx/security/pib/identity.hpp
+++ b/ndn-cxx/security/pib/identity.hpp
@@ -33,117 +33,118 @@
namespace pib {
+class IdentityContainer;
+
namespace detail {
class IdentityImpl;
} // namespace detail
/**
- * @brief A frontend handle of an Identity
+ * @brief Frontend handle for an identity in the PIB.
*
* Identity is at the top level in PIB's Identity-Key-Certificate hierarchy. An identity has a
- * Name, and contains zero or more keys, at most one of which is set as the default key of this
- * identity. Properties of a key can be accessed after obtaining a Key object.
+ * name, and contains zero or more keys, at most one of which is set as the default key of that
+ * identity. The properties of a key can be accessed after obtaining a Key object.
*/
class Identity
{
public:
/**
- * @brief Default Constructor
+ * @brief Default constructor.
*
- * Identity created using this default constructor is just a place holder.
- * It can obtain an actual instance from Pib::getIdentity(...). A typical
+ * An Identity created using this default constructor is just a placeholder.
+ * You can obtain an actual instance from Pib::getIdentity(). A typical
* usage would be for exception handling:
*
- * Identity id;
- * try {
- * id = pib.getIdentity(...);
- * }
- * catch (const Pib::Error&) {
- * ...
- * }
+ * @code
+ * Identity id;
+ * try {
+ * id = pib.getIdentity(...);
+ * }
+ * catch (const Pib::Error&) {
+ * ...
+ * }
+ * @endcode
*
- * An Identity instance created using this constructor is invalid. Calling a
- * member method on an invalid Identity instance may cause an std::domain_error.
+ * An instance created using this constructor is invalid. Calling a member
+ * function on an invalid Identity instance may throw an std::domain_error.
*/
- Identity();
+ Identity() noexcept;
/**
- * @brief Create an Identity with a backend implementation @p impl.
- *
- * This method should only be used by IdentityContainer.
- */
- explicit
- Identity(weak_ptr<detail::IdentityImpl> impl);
-
- /**
- * @brief Get the name of the identity.
+ * @brief Return the name of the identity.
*/
const Name&
getName() const;
/**
- * @brief Get a key with id @p keyName.
- * @throw std::invalid_argument @p keyName does not match identity
+ * @brief Obtain a handle to the key with the given name.
+ * @throw std::invalid_argument @p keyName does not match the identity.
* @throw Pib::Error the key does not exist.
*/
Key
getKey(const Name& keyName) const;
/**
- * @brief Get all keys for this identity.
+ * @brief Return all the keys of this identity.
*/
const KeyContainer&
getKeys() const;
/**
- * @brief Get the default key for this Identity.
+ * @brief Return the default key for this identity.
* @throw Pib::Error the default key does not exist.
*/
- const Key&
+ Key
getDefaultKey() const;
/**
- * @return True if the identity instance is valid
+ * @brief Returns true if the instance is valid.
*/
explicit
- operator bool() const;
+ operator bool() const noexcept;
-NDN_CXX_PUBLIC_WITH_TESTS_ELSE_PRIVATE: // write operations should be private
+NDN_CXX_PUBLIC_WITH_TESTS_ELSE_PRIVATE: // write operations are accessible only by KeyChain
/**
- * @brief Add @p key (in PKCS#8 format) with name @p keyName.
- * @return Handle of the added key.
- * @throw std::invalid_argument key name does not match identity
+ * @brief Add @p key (in PKCS #8 format) with name @p keyName.
*
- * If a key with the same name already exists, overwrite the key.
+ * If no default key is set before, the new key will be set as the default key of the identity.
+ * If a key with the same name already exists, it will be overwritten.
+ *
+ * @return Handle to the added key.
+ * @throw std::invalid_argument @p keyName does not match the identity.
*/
Key
addKey(span<const uint8_t> key, const Name& keyName) const;
/**
- * @brief Remove a key with @p keyName
- * @throw std::invalid_argument @p keyName does not match identity
+ * @brief Remove key with given name.
+ * @throw std::invalid_argument @p keyName does not match the identity.
*/
void
removeKey(const Name& keyName) const;
/**
* @brief Set an existing key with name @p keyName as the default key.
- * @return The default key.
- * @throw std::invalid_argument @p keyName does not match identity.
+ * @return Handle to the default key.
+ * @throw std::invalid_argument @p keyName does not match the identity.
* @throw Pib::Error the key does not exist.
*/
- const Key&
+ Key
setDefaultKey(const Name& keyName) const;
/**
- * @brief Add @p key with name @p keyName and set it as the default key.
- * @return The default key.
- * @throw std::invalid_argument @p keyName does not match identity.
- * @throw Pib::Error the key with the same name already exists.
+ * @brief Add/replace @p key with name @p keyName and set it as the default key.
+ * @return Handle to the default key.
+ * @throw std::invalid_argument @p keyName does not match the identity.
*/
- const Key&
+ Key
setDefaultKey(span<const uint8_t> key, const Name& keyName) const;
+NDN_CXX_PUBLIC_WITH_TESTS_ELSE_PRIVATE: // private interface for IdentityContainer
+ explicit
+ Identity(weak_ptr<detail::IdentityImpl> impl) noexcept;
+
private:
/**
* @brief Check the validity of the instance
@@ -153,25 +154,38 @@
shared_ptr<detail::IdentityImpl>
lock() const;
+ bool
+ equals(const Identity& other) const noexcept;
+
+ // NOTE
+ // The following "hidden friend" non-member operators are available
+ // via argument-dependent lookup only and must be defined inline.
+
+ friend bool
+ operator==(const Identity& lhs, const Identity& rhs)
+ {
+ return lhs.equals(rhs);
+ }
+
+ friend bool
+ operator!=(const Identity& lhs, const Identity& rhs)
+ {
+ return !lhs.equals(rhs);
+ }
+
+ friend std::ostream&
+ operator<<(std::ostream& os, const Identity& id)
+ {
+ return os << (id ? id.getName() : "(empty)");
+ }
+
private:
weak_ptr<detail::IdentityImpl> m_impl;
friend KeyChain;
- friend bool operator!=(const Identity&, const Identity&);
+ friend IdentityContainer;
};
-bool
-operator!=(const Identity& lhs, const Identity& rhs);
-
-inline bool
-operator==(const Identity& lhs, const Identity& rhs)
-{
- return !(lhs != rhs);
-}
-
-std::ostream&
-operator<<(std::ostream& os, const Identity& id);
-
} // namespace pib
using pib::Identity;
diff --git a/ndn-cxx/security/pib/impl/identity-impl.cpp b/ndn-cxx/security/pib/impl/identity-impl.cpp
index 4c6adb4..104f847 100644
--- a/ndn-cxx/security/pib/impl/identity-impl.cpp
+++ b/ndn-cxx/security/pib/impl/identity-impl.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).
*
@@ -21,18 +21,19 @@
#include "ndn-cxx/security/pib/impl/identity-impl.hpp"
#include "ndn-cxx/security/pib/pib-impl.hpp"
-#include "ndn-cxx/security/pib/pib.hpp"
+#include "ndn-cxx/util/logger.hpp"
namespace ndn {
namespace security {
namespace pib {
namespace detail {
+NDN_LOG_INIT(ndn.security.Identity);
+
IdentityImpl::IdentityImpl(const Name& identityName, shared_ptr<PibImpl> pibImpl, bool needInit)
: m_name(identityName)
, m_pib(std::move(pibImpl))
, m_keys(identityName, m_pib)
- , m_isDefaultKeyLoaded(false)
{
BOOST_ASSERT(m_pib != nullptr);
@@ -56,55 +57,38 @@
{
BOOST_ASSERT(m_keys.isConsistent());
- if (m_isDefaultKeyLoaded && m_defaultKey.getName() == keyName)
- m_isDefaultKeyLoaded = false;
-
+ if (m_defaultKey && m_defaultKey.getName() == keyName) {
+ NDN_LOG_DEBUG("Removing default key " << keyName);
+ m_defaultKey = {};
+ }
m_keys.remove(keyName);
}
Key
-IdentityImpl::getKey(const Name& keyName) const
+IdentityImpl::setDefaultKey(Key key)
{
BOOST_ASSERT(m_keys.isConsistent());
- return m_keys.get(keyName);
-}
+ BOOST_ASSERT(key);
-const KeyContainer&
-IdentityImpl::getKeys() const
-{
- BOOST_ASSERT(m_keys.isConsistent());
- return m_keys;
-}
+ m_defaultKey = std::move(key);
+ m_pib->setDefaultKeyOfIdentity(m_name, m_defaultKey.getName());
+ NDN_LOG_DEBUG("Default key set to " << m_defaultKey.getName());
-const Key&
-IdentityImpl::setDefaultKey(const Name& keyName)
-{
- BOOST_ASSERT(m_keys.isConsistent());
-
- m_defaultKey = m_keys.get(keyName);
- m_isDefaultKeyLoaded = true;
- m_pib->setDefaultKeyOfIdentity(m_name, keyName);
return m_defaultKey;
}
-const Key&
-IdentityImpl::setDefaultKey(span<const uint8_t> key, const Name& keyName)
-{
- addKey(key, keyName);
- return setDefaultKey(keyName);
-}
-
-const Key&
+Key
IdentityImpl::getDefaultKey() const
{
BOOST_ASSERT(m_keys.isConsistent());
- if (!m_isDefaultKeyLoaded) {
+ if (!m_defaultKey) {
m_defaultKey = m_keys.get(m_pib->getDefaultKeyOfIdentity(m_name));
- m_isDefaultKeyLoaded = true;
+ NDN_LOG_DEBUG("Caching default key " << m_defaultKey.getName());
}
- BOOST_ASSERT(m_pib->getDefaultKeyOfIdentity(m_name) == m_defaultKey.getName());
+ BOOST_ASSERT(m_defaultKey);
+ BOOST_ASSERT(m_defaultKey.getName() == m_pib->getDefaultKeyOfIdentity(m_name));
return m_defaultKey;
}
diff --git a/ndn-cxx/security/pib/impl/identity-impl.hpp b/ndn-cxx/security/pib/impl/identity-impl.hpp
index e71d170..eb14c89 100644
--- a/ndn-cxx/security/pib/impl/identity-impl.hpp
+++ b/ndn-cxx/security/pib/impl/identity-impl.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).
*
@@ -33,101 +33,80 @@
namespace detail {
/**
- * @brief Backend instance of Identity
+ * @brief Backend instance of Identity.
*
* An Identity has only one backend instance, but may have multiple frontend handles.
* Each frontend handle is associated with the only one backend IdentityImpl.
*
- * @throw PibImpl::Error when underlying implementation has non-semantic error.
+ * @throw PibImpl::Error When the underlying implementation has a non-semantic error.
+ * @sa Identity
*/
class IdentityImpl : noncopyable
{
public:
/**
- * @brief Create an Identity with @p identityName.
+ * @brief Create an identity with name @p identityName.
*
- * @param identityName The name of the Identity.
+ * @param identityName The name of the identity.
* @param pibImpl The PIB backend implementation.
* @param needInit If true, create the identity in backend when the identity does not exist.
* Otherwise, throw Pib::Error when the identity does not exist.
*/
IdentityImpl(const Name& identityName, shared_ptr<PibImpl> pibImpl, bool needInit = false);
- /**
- * @brief Get the name of the identity.
- */
+ // See security::pib::Identity for the documentation of the following methods
+
const Name&
getName() const
{
return m_name;
}
- /**
- * @brief Add @p key with name @p keyName (in PKCS #8 format).
- *
- * If no default key is set before, the new key will be set as the default key of the identity.
- * If a key with the same name already exists, it will be overwritten.
- *
- * @return the added key.
- * @throw std::invalid_argument key name does not match identity
- */
Key
addKey(span<const uint8_t> key, const Name& keyName);
- /**
- * @brief Remove a key with @p keyName
- * @throw std::invalid_argument @p keyName does not match identity
- */
void
removeKey(const Name& keyName);
- /**
- * @brief Get a key with id @p keyName.
- *
- * @throw std::invalid_argument @p keyName does not match identity
- * @throw Pib::Error the key does not exist.
- */
Key
- getKey(const Name& keyName) const;
+ getKey(const Name& keyName) const
+ {
+ BOOST_ASSERT(m_keys.isConsistent());
+ return m_keys.get(keyName);
+ }
- /**
- * @brief Get all keys for this Identity.
- */
const KeyContainer&
- getKeys() const;
+ getKeys() const
+ {
+ BOOST_ASSERT(m_keys.isConsistent());
+ return m_keys;
+ }
- /**
- * @brief Set the key with id @p keyName.
- * @throw std::invalid_argument @p keyName does not match identity
- * @throw Pib::Error the key does not exist.
- * @return The default key
- */
- const Key&
- setDefaultKey(const Name& keyName);
+ Key
+ setDefaultKey(const Name& keyName)
+ {
+ return setDefaultKey(m_keys.get(keyName));
+ }
- /**
- * @brief Add @p key with name @p keyName and set it as the default key.
- * @throw std::invalid_argument @p keyName does not match identity
- * @throw Pib::Error the key with the same name already exists
- * @return the default key
- */
- const Key&
- setDefaultKey(span<const uint8_t> key, const Name& keyName);
+ Key
+ setDefaultKey(span<const uint8_t> key, const Name& keyName)
+ {
+ return setDefaultKey(m_keys.add(key, keyName));
+ }
- /**
- * @brief Get the default key for this Identity.
- * @throw Pib::Error the default key does not exist.
- */
- const Key&
+ Key
getDefaultKey() const;
private:
- Name m_name;
+ Key
+ setDefaultKey(Key key);
- shared_ptr<PibImpl> m_pib;
+private:
+ const Name m_name;
+
+ const shared_ptr<PibImpl> m_pib;
KeyContainer m_keys;
- mutable bool m_isDefaultKeyLoaded;
mutable Key m_defaultKey;
};
diff --git a/tests/unit/security/pib/identity-container.t.cpp b/tests/unit/security/pib/identity-container.t.cpp
index fb5e9cc..77503a0 100644
--- a/tests/unit/security/pib/identity-container.t.cpp
+++ b/tests/unit/security/pib/identity-container.t.cpp
@@ -1,6 +1,6 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
/*
- * Copyright (c) 2013-2019 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).
*
@@ -21,7 +21,6 @@
#include "ndn-cxx/security/pib/identity-container.hpp"
#include "ndn-cxx/security/pib/impl/pib-memory.hpp"
-#include "ndn-cxx/security/pib/pib.hpp"
#include "tests/boost-test.hpp"
#include "tests/unit/security/pib/pib-data-fixture.hpp"
@@ -37,78 +36,75 @@
BOOST_AUTO_TEST_SUITE(Pib)
BOOST_FIXTURE_TEST_SUITE(TestIdentityContainer, PibDataFixture)
-using pib::Pib;
-
-BOOST_AUTO_TEST_CASE(Basic)
+BOOST_AUTO_TEST_CASE(AddGetRemove)
{
auto pibImpl = make_shared<PibMemory>();
- // start with an empty container
- IdentityContainer container(pibImpl);
- BOOST_CHECK_EQUAL(container.size(), 0);
- BOOST_CHECK_EQUAL(container.getLoadedIdentities().size(), 0);
+ {
+ // start with an empty container
+ IdentityContainer container(pibImpl);
+ BOOST_CHECK_EQUAL(container.size(), 0);
+ BOOST_CHECK_EQUAL(container.m_identities.size(), 0);
- // add the first identity
- Identity identity11 = container.add(id1);
- BOOST_CHECK_EQUAL(identity11.getName(), id1);
- BOOST_CHECK_EQUAL(container.size(), 1);
- BOOST_CHECK_EQUAL(container.getLoadedIdentities().size(), 1);
- BOOST_CHECK(container.find(id1) != container.end());
+ // add the first identity
+ Identity identity11 = container.add(id1);
+ BOOST_CHECK_EQUAL(identity11.getName(), id1);
+ BOOST_CHECK_EQUAL(container.size(), 1);
+ BOOST_CHECK_EQUAL(container.m_identities.size(), 1);
+ BOOST_CHECK(container.find(id1) != container.end());
- // add the same identity again
- Identity identity12 = container.add(id1);
- BOOST_CHECK_EQUAL(identity12.getName(), id1);
- BOOST_CHECK_EQUAL(container.size(), 1);
- BOOST_CHECK_EQUAL(container.getLoadedIdentities().size(), 1);
- BOOST_CHECK(container.find(id1) != container.end());
+ // add the same identity again
+ Identity identity12 = container.add(id1);
+ BOOST_CHECK_EQUAL(identity12.getName(), id1);
+ BOOST_CHECK_EQUAL(container.size(), 1);
+ BOOST_CHECK_EQUAL(container.m_identities.size(), 1);
+ BOOST_CHECK(container.find(id1) != container.end());
- // add the second identity
- Identity identity21 = container.add(id2);
- BOOST_CHECK_EQUAL(identity21.getName(), id2);
- BOOST_CHECK_EQUAL(container.size(), 2);
- BOOST_CHECK_EQUAL(container.getLoadedIdentities().size(), 2);
- BOOST_CHECK(container.find(id1) != container.end());
- BOOST_CHECK(container.find(id2) != container.end());
+ // add the second identity
+ Identity identity21 = container.add(id2);
+ BOOST_CHECK_EQUAL(identity21.getName(), id2);
+ BOOST_CHECK_EQUAL(container.size(), 2);
+ BOOST_CHECK_EQUAL(container.m_identities.size(), 2);
+ BOOST_CHECK(container.find(id1) != container.end());
+ BOOST_CHECK(container.find(id2) != container.end());
- // get identities
- BOOST_REQUIRE_NO_THROW(container.get(id1));
- BOOST_REQUIRE_NO_THROW(container.get(id2));
- BOOST_CHECK_THROW(container.get(Name("/non-existing")), Pib::Error);
+ // check identities
+ Identity identity1 = container.get(id1);
+ Identity identity2 = container.get(id2);
+ BOOST_CHECK_EQUAL(identity1.getName(), id1);
+ BOOST_CHECK_EQUAL(identity2.getName(), id2);
+ BOOST_CHECK_THROW(container.get(Name("/non-existing")), pib::Pib::Error);
+ }
- // check identity
- Identity identity1 = container.get(id1);
- Identity identity2 = container.get(id2);
- BOOST_CHECK_EQUAL(identity1.getName(), id1);
- BOOST_CHECK_EQUAL(identity2.getName(), id2);
+ {
+ // create a container from an existing (non-empty) PibImpl
+ // names are loaded immediately but identity cache should initially be empty
+ IdentityContainer container2(pibImpl);
+ BOOST_CHECK_EQUAL(container2.size(), 2);
+ BOOST_CHECK_EQUAL(container2.m_identities.size(), 0);
- // create another container from the same PibImpl
- // cache should be empty
- IdentityContainer container2(pibImpl);
- BOOST_CHECK_EQUAL(container2.size(), 2);
- BOOST_CHECK_EQUAL(container2.getLoadedIdentities().size(), 0);
+ // fetching the identities should populate the cache
+ BOOST_CHECK_EQUAL(container2.get(id1).getName(), id1);
+ BOOST_CHECK_EQUAL(container2.size(), 2);
+ BOOST_CHECK_EQUAL(container2.m_identities.size(), 1);
- // get key, cache should be filled
- BOOST_REQUIRE_NO_THROW(container2.get(id1));
- BOOST_CHECK_EQUAL(container2.size(), 2);
- BOOST_CHECK_EQUAL(container2.getLoadedIdentities().size(), 1);
+ BOOST_CHECK_EQUAL(container2.get(id2).getName(), id2);
+ BOOST_CHECK_EQUAL(container2.size(), 2);
+ BOOST_CHECK_EQUAL(container2.m_identities.size(), 2);
- BOOST_REQUIRE_NO_THROW(container2.get(id2));
- BOOST_CHECK_EQUAL(container2.size(), 2);
- BOOST_CHECK_EQUAL(container2.getLoadedIdentities().size(), 2);
+ // remove an identity
+ 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 a key
- container2.remove(id1);
- BOOST_CHECK_EQUAL(container2.size(), 1);
- BOOST_CHECK_EQUAL(container2.getLoadedIdentities().size(), 1);
- BOOST_CHECK(container2.find(id1) == container2.end());
- BOOST_CHECK(container2.find(id2) != container2.end());
-
- // remove another key
- container2.remove(id2);
- BOOST_CHECK_EQUAL(container2.size(), 0);
- BOOST_CHECK_EQUAL(container2.getLoadedIdentities().size(), 0);
- BOOST_CHECK(container2.find(id2) == container2.end());
-
+ // remove another identity
+ container2.remove(id2);
+ BOOST_CHECK_EQUAL(container2.size(), 0);
+ BOOST_CHECK_EQUAL(container2.m_identities.size(), 0);
+ BOOST_CHECK(container2.find(id2) == container2.end());
+ }
}
BOOST_AUTO_TEST_CASE(Iterator)
@@ -118,12 +114,10 @@
container.add(id1);
container.add(id2);
- std::set<Name> idNames;
- idNames.insert(id1);
- idNames.insert(id2);
+ const std::set<Name> idNames{id1, id2};
IdentityContainer::const_iterator it = container.begin();
- std::set<Name>::const_iterator testIt = idNames.begin();
+ auto testIt = idNames.begin();
BOOST_CHECK_EQUAL((*it).getName(), *testIt);
it++;
testIt++;
@@ -132,7 +126,8 @@
testIt++;
BOOST_CHECK(it == container.end());
- size_t count = 0;
+ // test range-based for
+ int count = 0;
testIt = idNames.begin();
for (const auto& identity : container) {
BOOST_CHECK_EQUAL(identity.getName(), *testIt);
diff --git a/tests/unit/security/pib/identity.t.cpp b/tests/unit/security/pib/identity.t.cpp
index 0198816..97f2369 100644
--- a/tests/unit/security/pib/identity.t.cpp
+++ b/tests/unit/security/pib/identity.t.cpp
@@ -41,34 +41,41 @@
BOOST_AUTO_TEST_CASE(ValidityChecking)
{
Identity id;
- BOOST_CHECK(!id);
- BOOST_CHECK_EQUAL(static_cast<bool>(id), false);
+ BOOST_TEST(!id);
+ BOOST_TEST(id == Identity());
- auto identityImpl = make_shared<detail::IdentityImpl>(id1, make_shared<PibMemory>(), true);
- id = Identity(identityImpl);
- BOOST_CHECK(id);
- BOOST_CHECK_EQUAL(!id, false);
+ auto impl = std::make_shared<detail::IdentityImpl>(id1, std::make_shared<PibMemory>(), true);
+ id = Identity(impl);
+ BOOST_TEST(id);
+ BOOST_TEST(id != Identity());
+
+ impl.reset();
+ BOOST_TEST(!id);
}
-// pib::Identity is a wrapper of pib::detail::IdentityImpl. Since the functionalities of
-// IdentityImpl have already been tested in detail/identity-impl.t.cpp, we only test the shared
+// pib::Identity is a wrapper of pib::detail::IdentityImpl. Since the functionality
+// of IdentityImpl is already tested in identity-impl.t.cpp, we only test the shared
// property of pib::Identity in this test case.
BOOST_AUTO_TEST_CASE(SharedImpl)
{
- auto identityImpl = make_shared<detail::IdentityImpl>(id1, make_shared<pib::PibMemory>(), true);
- Identity identity1(identityImpl);
- Identity identity2(identityImpl);
- BOOST_CHECK_EQUAL(identity1, identity2);
- BOOST_CHECK_NE(identity1, Identity());
- BOOST_CHECK_EQUAL(Identity(), Identity());
+ auto impl = std::make_shared<detail::IdentityImpl>(id1, std::make_shared<pib::PibMemory>(), true);
+ Identity identity1(impl);
+ Identity identity2(impl);
+ BOOST_TEST(identity1 == identity2);
+ BOOST_TEST(identity1 != Identity());
+ BOOST_TEST(Identity() != identity2);
+ BOOST_TEST(Identity() == Identity());
+
+ BOOST_CHECK_THROW(identity2.getKey(id1Key1Name), pib::Pib::Error);
identity1.addKey(id1Key1, id1Key1Name);
- BOOST_CHECK_NO_THROW(identity2.getKey(id1Key1Name));
+ BOOST_TEST(identity2.getKey(id1Key1Name));
+
identity2.removeKey(id1Key1Name);
BOOST_CHECK_THROW(identity1.getKey(id1Key1Name), pib::Pib::Error);
identity1.setDefaultKey(id1Key1, id1Key1Name);
- BOOST_CHECK_NO_THROW(identity2.getDefaultKey());
+ BOOST_TEST(identity2.getDefaultKey().getName() == id1Key1Name);
}
BOOST_AUTO_TEST_SUITE_END() // TestIdentity
diff --git a/tests/unit/security/pib/impl/identity-impl.t.cpp b/tests/unit/security/pib/impl/identity-impl.t.cpp
index 1b4527f..390cee4 100644
--- a/tests/unit/security/pib/impl/identity-impl.t.cpp
+++ b/tests/unit/security/pib/impl/identity-impl.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).
*
@@ -36,19 +36,17 @@
BOOST_AUTO_TEST_SUITE(Pib)
BOOST_FIXTURE_TEST_SUITE(TestIdentityImpl, ndn::security::tests::PibDataFixture)
-using security::Pib;
+using pib::Pib;
BOOST_AUTO_TEST_CASE(Basic)
{
- auto pibImpl = make_shared<pib::PibMemory>();
- IdentityImpl identity1(id1, pibImpl, true);
-
+ IdentityImpl identity1(id1, std::make_shared<pib::PibMemory>(), true);
BOOST_CHECK_EQUAL(identity1.getName(), id1);
}
-BOOST_AUTO_TEST_CASE(KeyOperation)
+BOOST_AUTO_TEST_CASE(KeyOperations)
{
- auto pibImpl = make_shared<pib::PibMemory>();
+ auto pibImpl = std::make_shared<pib::PibMemory>();
IdentityImpl identity1(id1, pibImpl, true);
BOOST_CHECK_NO_THROW(IdentityImpl(id1, pibImpl, false));
@@ -60,18 +58,18 @@
// get default key, throw Pib::Error
BOOST_CHECK_THROW(identity1.getDefaultKey(), Pib::Error);
// set non-existing key as default key, throw Pib::Error
- BOOST_REQUIRE_THROW(identity1.setDefaultKey(id1Key1Name), Pib::Error);
+ BOOST_CHECK_THROW(identity1.setDefaultKey(id1Key1Name), Pib::Error);
// add key
identity1.addKey(id1Key1, id1Key1Name);
- BOOST_CHECK_NO_THROW(identity1.getKey(id1Key1Name));
+ const auto& addedKey = identity1.getKey(id1Key1Name);
+ BOOST_CHECK_EQUAL(addedKey.getName(), id1Key1Name);
+ BOOST_TEST(addedKey.getPublicKey() == id1Key1, boost::test_tools::per_element());
// new key becomes default key when there is no default key
- BOOST_REQUIRE_NO_THROW(identity1.getDefaultKey());
- const Key& defaultKey0 = identity1.getDefaultKey();
+ const auto& defaultKey0 = identity1.getDefaultKey();
BOOST_CHECK_EQUAL(defaultKey0.getName(), id1Key1Name);
- BOOST_CHECK_EQUAL_COLLECTIONS(defaultKey0.getPublicKey().begin(), defaultKey0.getPublicKey().end(),
- id1Key1.begin(), id1Key1.end());
+ BOOST_TEST(defaultKey0.getPublicKey() == id1Key1, boost::test_tools::per_element());
// remove key
identity1.removeKey(id1Key1Name);
@@ -80,14 +78,9 @@
// set default key directly
BOOST_REQUIRE_NO_THROW(identity1.setDefaultKey(id1Key1, id1Key1Name));
- BOOST_REQUIRE_NO_THROW(identity1.getDefaultKey());
- BOOST_CHECK_NO_THROW(identity1.getKey(id1Key1Name));
-
- // check default key
- const Key& defaultKey1 = identity1.getDefaultKey();
+ const auto& defaultKey1 = identity1.getDefaultKey();
BOOST_CHECK_EQUAL(defaultKey1.getName(), id1Key1Name);
- BOOST_CHECK_EQUAL_COLLECTIONS(defaultKey1.getPublicKey().begin(), defaultKey1.getPublicKey().end(),
- id1Key1.begin(), id1Key1.end());
+ BOOST_TEST(defaultKey1.getPublicKey() == id1Key1, boost::test_tools::per_element());
// add another key
identity1.addKey(id1Key2, id1Key2Name);
@@ -95,11 +88,9 @@
// set default key through name
BOOST_REQUIRE_NO_THROW(identity1.setDefaultKey(id1Key2Name));
- BOOST_REQUIRE_NO_THROW(identity1.getDefaultKey());
- const Key& defaultKey2 = identity1.getDefaultKey();
+ const auto& defaultKey2 = identity1.getDefaultKey();
BOOST_CHECK_EQUAL(defaultKey2.getName(), id1Key2Name);
- BOOST_CHECK_EQUAL_COLLECTIONS(defaultKey2.getPublicKey().begin(), defaultKey2.getPublicKey().end(),
- id1Key2.begin(), id1Key2.end());
+ BOOST_TEST(defaultKey2.getPublicKey() == id1Key2, boost::test_tools::per_element());
// remove key
identity1.removeKey(id1Key1Name);
@@ -108,10 +99,9 @@
// set default key directly again, change the default setting
BOOST_REQUIRE_NO_THROW(identity1.setDefaultKey(id1Key1, id1Key1Name));
- const Key& defaultKey3 = identity1.getDefaultKey();
+ const auto& defaultKey3 = identity1.getDefaultKey();
BOOST_CHECK_EQUAL(defaultKey3.getName(), id1Key1Name);
- BOOST_CHECK_EQUAL_COLLECTIONS(defaultKey3.getPublicKey().begin(), defaultKey3.getPublicKey().end(),
- id1Key1.begin(), id1Key1.end());
+ BOOST_TEST(defaultKey3.getPublicKey() == id1Key1, boost::test_tools::per_element());
BOOST_CHECK_EQUAL(identity1.getKeys().size(), 2);
// remove all keys
@@ -126,23 +116,20 @@
BOOST_AUTO_TEST_CASE(Overwrite)
{
- auto pibImpl = make_shared<pib::PibMemory>();
- IdentityImpl identity1(id1, pibImpl, true);
+ IdentityImpl identity1(id1, std::make_shared<pib::PibMemory>(), true);
identity1.addKey(id1Key1, id1Key1Name);
auto k1 = identity1.getKey(id1Key1Name);
- BOOST_CHECK_EQUAL_COLLECTIONS(k1.getPublicKey().begin(), k1.getPublicKey().end(),
- id1Key1.begin(), id1Key1.end());
+ BOOST_TEST(k1.getPublicKey() == id1Key1, boost::test_tools::per_element());
identity1.addKey(id1Key2, id1Key1Name); // overwriting key should work
auto k2 = identity1.getKey(id1Key1Name);
- BOOST_CHECK_EQUAL_COLLECTIONS(k2.getPublicKey().begin(), k2.getPublicKey().end(),
- id1Key2.begin(), id1Key2.end());
+ BOOST_TEST(k2.getPublicKey() == id1Key2, boost::test_tools::per_element());
}
BOOST_AUTO_TEST_CASE(Errors)
{
- auto pibImpl = make_shared<pib::PibMemory>();
+ auto pibImpl = std::make_shared<pib::PibMemory>();
BOOST_CHECK_THROW(IdentityImpl(id1, pibImpl, false), Pib::Error);
IdentityImpl identity1(id1, pibImpl, true);