security: modernize Pib class and PIB backends

Change-Id: I02361ac4f628e2ca8b6f83fd0dfad4cb460cf028
diff --git a/ndn-cxx/security/pib/impl/pib-memory.cpp b/ndn-cxx/security/pib/impl/pib-memory.cpp
index 3abade7..4ccdb1f 100644
--- a/ndn-cxx/security/pib/impl/pib-memory.cpp
+++ b/ndn-cxx/security/pib/impl/pib-memory.cpp
@@ -20,24 +20,23 @@
  */
 
 #include "ndn-cxx/security/pib/impl/pib-memory.hpp"
-#include "ndn-cxx/security/pib/pib.hpp"
-#include "ndn-cxx/security/security-common.hpp"
 
+#include <boost/range/adaptor/filtered.hpp>
 #include <boost/range/adaptor/map.hpp>
+#include <boost/range/algorithm_ext/insert.hpp>
 
 namespace ndn {
 namespace security {
 namespace pib {
 
 PibMemory::PibMemory(const std::string&)
-  : m_hasDefaultIdentity(false)
 {
 }
 
 const std::string&
 PibMemory::getScheme()
 {
-  static std::string scheme = "pib-memory";
+  static const std::string scheme("pib-memory");
   return scheme;
 }
 
@@ -47,16 +46,10 @@
   m_tpmLocator = tpmLocator;
 }
 
-std::string
-PibMemory::getTpmLocator() const
-{
-  return m_tpmLocator;
-}
-
 bool
 PibMemory::hasIdentity(const Name& identity) const
 {
-  return (m_identities.count(identity) > 0);
+  return m_identities.count(identity) > 0;
 }
 
 void
@@ -64,9 +57,8 @@
 {
   m_identities.insert(identity);
 
-  if (!m_hasDefaultIdentity) {
+  if (!m_defaultIdentity) {
     m_defaultIdentity = identity;
-    m_hasDefaultIdentity = true;
   }
 }
 
@@ -74,13 +66,12 @@
 PibMemory::removeIdentity(const Name& identity)
 {
   m_identities.erase(identity);
+
   if (identity == m_defaultIdentity) {
-    m_hasDefaultIdentity = false;
-    m_defaultIdentity.clear();
+    m_defaultIdentity.reset();
   }
 
-  auto keyNames = getKeysOfIdentity(identity);
-  for (const Name& keyName : keyNames) {
+  for (const Name& keyName : getKeysOfIdentity(identity)) {
     removeKey(keyName);
   }
 }
@@ -88,8 +79,7 @@
 void
 PibMemory::clearIdentities()
 {
-  m_hasDefaultIdentity = false;
-  m_defaultIdentity.clear();
+  m_defaultIdentity.reset();
   m_identities.clear();
   m_defaultKeys.clear();
   m_keys.clear();
@@ -109,18 +99,18 @@
   if (!hasIdentity(identityName)) {
     NDN_THROW(Pib::Error("Cannot set non-existing identity `" + identityName.toUri() + "` as default"));
   }
+
   m_defaultIdentity = identityName;
-  m_hasDefaultIdentity = true;
 }
 
 Name
 PibMemory::getDefaultIdentity() const
 {
-  if (m_hasDefaultIdentity) {
-    return m_defaultIdentity;
+  if (!m_defaultIdentity) {
+    NDN_THROW(Pib::Error("No default identity"));
   }
 
-  NDN_THROW(Pib::Error("No default identity"));
+  return *m_defaultIdentity;
 }
 
 bool
@@ -132,9 +122,10 @@
 void
 PibMemory::addKey(const Name& identity, const Name& keyName, span<const uint8_t> key)
 {
+  // ensure identity exists
   addIdentity(identity);
 
-  m_keys[keyName] = Buffer(key.begin(), key.end());
+  m_keys[keyName] = Buffer(key.begin(), key.end()); // use insert_or_assign in C++17
 
   if (m_defaultKeys.count(identity) == 0) {
     m_defaultKeys[identity] = keyName;
@@ -149,8 +140,7 @@
   m_keys.erase(keyName);
   m_defaultKeys.erase(identity);
 
-  auto certNames = getCertificatesOfKey(keyName);
-  for (const auto& certName : certNames) {
+  for (const auto& certName : getCertificatesOfKey(keyName)) {
     removeCertificate(certName);
   }
 }
@@ -159,7 +149,7 @@
 PibMemory::getKeyBits(const Name& keyName) const
 {
   if (!hasKey(keyName)) {
-    NDN_THROW(Pib::Error("Key `" + keyName.toUri() + "` not found"));
+    NDN_THROW(Pib::Error("Key `" + keyName.toUri() + "` not found in PIB"));
   }
 
   auto key = m_keys.find(keyName);
@@ -170,20 +160,18 @@
 std::set<Name>
 PibMemory::getKeysOfIdentity(const Name& identity) const
 {
-  std::set<Name> ids;
-  for (const auto& keyName : m_keys | boost::adaptors::map_keys) {
-    if (identity == extractIdentityFromKeyName(keyName)) {
-      ids.insert(keyName);
-    }
-  }
-  return ids;
+  std::set<Name> keyNames;
+  boost::insert(keyNames,
+                m_keys | boost::adaptors::map_keys | boost::adaptors::filtered(
+                  [&] (const auto& kn) { return extractIdentityFromKeyName(kn) == identity; }));
+  return keyNames;
 }
 
 void
 PibMemory::setDefaultKeyOfIdentity(const Name& identity, const Name& keyName)
 {
   if (!hasKey(keyName)) {
-    NDN_THROW(Pib::Error("Key `" + keyName.toUri() + "` not found"));
+    NDN_THROW(Pib::Error("Cannot set non-existing key `" + keyName.toUri() + "` as default"));
   }
 
   m_defaultKeys[identity] = keyName;
@@ -203,7 +191,7 @@
 bool
 PibMemory::hasCertificate(const Name& certName) const
 {
-  return (m_certs.count(certName) > 0);
+  return m_certs.count(certName) > 0;
 }
 
 void
@@ -212,6 +200,7 @@
   const Name& certName = certificate.getName();
   const Name& keyName = certificate.getKeyName();
 
+  // ensure key exists
   addKey(certificate.getIdentity(), keyName, certificate.getContent().value_bytes());
 
   m_certs[certName] = certificate;
@@ -234,7 +223,7 @@
 PibMemory::getCertificate(const Name& certName) const
 {
   if (!hasCertificate(certName)) {
-    NDN_THROW(Pib::Error("Certificate `" + certName.toUri() +  "` does not exist"));
+    NDN_THROW(Pib::Error("Certificate `" + certName.toUri() + "` not found in PIB"));
   }
 
   auto it = m_certs.find(certName);
@@ -245,11 +234,9 @@
 PibMemory::getCertificatesOfKey(const Name& keyName) const
 {
   std::set<Name> certNames;
-  for (const auto& it : m_certs) {
-    if (extractKeyNameFromCertName(it.second.getName()) == keyName) {
-      certNames.insert(it.first);
-    }
-  }
+  boost::insert(certNames,
+                m_certs | boost::adaptors::map_keys | boost::adaptors::filtered(
+                  [&] (const auto& cn) { return extractKeyNameFromCertName(cn) == keyName; }));
   return certNames;
 }
 
@@ -257,7 +244,7 @@
 PibMemory::setDefaultCertificateOfKey(const Name& keyName, const Name& certName)
 {
   if (!hasCertificate(certName)) {
-    NDN_THROW(Pib::Error("Certificate `" + certName.toUri() +  "` does not exist"));
+    NDN_THROW(Pib::Error("Cannot set non-existing certificate `" + certName.toUri() + "` as default"));
   }
 
   m_defaultCerts[keyName] = certName;
diff --git a/ndn-cxx/security/pib/impl/pib-memory.hpp b/ndn-cxx/security/pib/impl/pib-memory.hpp
index fb1bf13..74abea3 100644
--- a/ndn-cxx/security/pib/impl/pib-memory.hpp
+++ b/ndn-cxx/security/pib/impl/pib-memory.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).
  *
@@ -29,17 +29,17 @@
 namespace pib {
 
 /**
- * @brief An in-memory implementation of Pib
+ * @brief An in-memory PIB implementation.
  *
- * All the contents in Pib are stored in memory
- * and have the same lifetime as the implementation instance.
+ * All the contents of an instance of this PIB are stored in memory only
+ * and have the same lifetime as the class instance itself.
  */
 class PibMemory : public PibImpl
 {
 public:
   /**
-   * @brief Create memory based PIB backend
-   * @param location Not used (required by the PIB-registration interface)
+   * @brief Create memory-based PIB backend.
+   * @param location Ignored (required by the PIB registration interface)
    */
   explicit
   PibMemory(const std::string& location = "");
@@ -48,12 +48,15 @@
   getScheme();
 
 public: // TpmLocator management
+  std::string
+  getTpmLocator() const override
+  {
+    return m_tpmLocator;
+  }
+
   void
   setTpmLocator(const std::string& tpmLocator) override;
 
-  std::string
-  getTpmLocator() const override;
-
 public: // Identity management
   bool
   hasIdentity(const Name& identity) const override;
@@ -123,21 +126,19 @@
 private:
   std::string m_tpmLocator;
 
-  bool m_hasDefaultIdentity;
-  Name m_defaultIdentity;
-
   std::set<Name> m_identities;
+  optional<Name> m_defaultIdentity;
 
-  /// @brief identity => default key Name
+  /// identity name => default key name
   std::map<Name, Name> m_defaultKeys;
 
-  /// @brief keyName => keyBits
+  /// key name => key bits
   std::map<Name, Buffer> m_keys;
 
-  /// @brief keyName => default certificate Name
+  /// key name => default certificate name
   std::map<Name, Name> m_defaultCerts;
 
-  /// @brief certificate Name => certificate
+  /// certificate name => certificate object
   std::map<Name, Certificate> m_certs;
 };
 
diff --git a/ndn-cxx/security/pib/impl/pib-sqlite3.cpp b/ndn-cxx/security/pib/impl/pib-sqlite3.cpp
index c994eae..06e14cc 100644
--- a/ndn-cxx/security/pib/impl/pib-sqlite3.cpp
+++ b/ndn-cxx/security/pib/impl/pib-sqlite3.cpp
@@ -20,8 +20,6 @@
  */
 
 #include "ndn-cxx/security/pib/impl/pib-sqlite3.hpp"
-#include "ndn-cxx/security/pib/pib.hpp"
-#include "ndn-cxx/security/security-common.hpp"
 #include "ndn-cxx/util/sqlite3-statement.hpp"
 
 #include <sqlite3.h>
@@ -35,7 +33,7 @@
 
 using util::Sqlite3Statement;
 
-static const std::string INITIALIZATION = R"SQL(
+static const char DB_INIT[] = R"SQL(
 CREATE TABLE IF NOT EXISTS
   tpmInfo(
     tpm_locator           BLOB
@@ -136,7 +134,6 @@
       WHERE identity_id=NEW.identity_id;
   END;
 
-
 CREATE TABLE IF NOT EXISTS
   certificates(
     id                    INTEGER PRIMARY KEY,
@@ -230,7 +227,7 @@
 
   // initialize PIB tables
   char* errmsg = nullptr;
-  result = sqlite3_exec(m_database, INITIALIZATION.c_str(), nullptr, nullptr, &errmsg);
+  result = sqlite3_exec(m_database, DB_INIT, nullptr, nullptr, &errmsg);
   if (result != SQLITE_OK && errmsg != nullptr) {
     std::string what = "PIB database cannot be initialized: "s + errmsg;
     sqlite3_free(errmsg);
@@ -246,7 +243,7 @@
 const std::string&
 PibSqlite3::getScheme()
 {
-  static std::string scheme = "pib-sqlite3";
+  static const std::string scheme("pib-sqlite3");
   return scheme;
 }
 
@@ -269,11 +266,11 @@
 PibSqlite3::getTpmLocator() const
 {
   Sqlite3Statement statement(m_database, "SELECT tpm_locator FROM tpmInfo");
-  int res = statement.step();
-  if (res == SQLITE_ROW)
+
+  if (statement.step() == SQLITE_ROW)
     return statement.getString(0);
-  else
-    return "";
+
+  return {};
 }
 
 bool
@@ -319,9 +316,9 @@
   std::set<Name> identities;
   Sqlite3Statement statement(m_database, "SELECT identity FROM identities");
 
-  while (statement.step() == SQLITE_ROW)
+  while (statement.step() == SQLITE_ROW) {
     identities.insert(Name(statement.getBlock(0)));
-
+  }
   return identities;
 }
 
@@ -331,6 +328,7 @@
   if (!hasIdentity(identityName)) {
     NDN_THROW(Pib::Error("Cannot set non-existing identity `" + identityName.toUri() + "` as default"));
   }
+
   Sqlite3Statement statement(m_database, "UPDATE identities SET is_default=1 WHERE identity=?");
   statement.bind(1, identityName.wireEncode(), SQLITE_TRANSIENT);
   statement.step();
@@ -343,15 +341,15 @@
 
   if (statement.step() == SQLITE_ROW)
     return Name(statement.getBlock(0));
-  else
-    NDN_THROW(Pib::Error("No default identity"));
+
+  NDN_THROW(Pib::Error("No default identity"));
 }
 
 bool
 PibSqlite3::hasDefaultIdentity() const
 {
   Sqlite3Statement statement(m_database, "SELECT identity FROM identities WHERE is_default=1");
-  return (statement.step() == SQLITE_ROW);
+  return statement.step() == SQLITE_ROW;
 }
 
 bool
@@ -359,8 +357,7 @@
 {
   Sqlite3Statement statement(m_database, "SELECT id FROM keys WHERE key_name=?");
   statement.bind(1, keyName.wireEncode(), SQLITE_TRANSIENT);
-
-  return (statement.step() == SQLITE_ROW);
+  return statement.step() == SQLITE_ROW;
 }
 
 void
@@ -407,15 +404,14 @@
 
   if (statement.step() == SQLITE_ROW)
     return Buffer(statement.getBlob(0), statement.getSize(0));
-  else
-    NDN_THROW(Pib::Error("Key `" + keyName.toUri() + "` does not exist"));
+
+  NDN_THROW(Pib::Error("Key `" + keyName.toUri() + "` not found in PIB"));
 }
 
 std::set<Name>
 PibSqlite3::getKeysOfIdentity(const Name& identity) const
 {
   std::set<Name> keyNames;
-
   Sqlite3Statement statement(m_database,
                              "SELECT key_name "
                              "FROM keys JOIN identities ON keys.identity_id=identities.id "
@@ -425,7 +421,6 @@
   while (statement.step() == SQLITE_ROW) {
     keyNames.insert(Name(statement.getBlock(0)));
   }
-
   return keyNames;
 }
 
@@ -433,7 +428,7 @@
 PibSqlite3::setDefaultKeyOfIdentity(const Name& identity, const Name& keyName)
 {
   if (!hasKey(keyName)) {
-    NDN_THROW(Pib::Error("Key `" + keyName.toUri() + "` does not exist"));
+    NDN_THROW(Pib::Error("Cannot set non-existing key `" + keyName.toUri() + "` as default"));
   }
 
   Sqlite3Statement statement(m_database, "UPDATE keys SET is_default=1 WHERE key_name=?");
@@ -454,11 +449,10 @@
                              "WHERE identities.identity=? AND keys.is_default=1");
   statement.bind(1, identity.wireEncode(), SQLITE_TRANSIENT);
 
-  if (statement.step() == SQLITE_ROW) {
+  if (statement.step() == SQLITE_ROW)
     return Name(statement.getBlock(0));
-  }
-  else
-    NDN_THROW(Pib::Error("No default key for identity `" + identity.toUri() + "`"));
+
+  NDN_THROW(Pib::Error("No default key for identity `" + identity.toUri() + "`"));
 }
 
 bool
@@ -469,8 +463,7 @@
                              "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);
+  return statement.step() == SQLITE_ROW;
 }
 
 bool
@@ -478,7 +471,7 @@
 {
   Sqlite3Statement statement(m_database, "SELECT id FROM certificates WHERE certificate_name=?");
   statement.bind(1, certName.wireEncode(), SQLITE_TRANSIENT);
-  return (statement.step() == SQLITE_ROW);
+  return statement.step() == SQLITE_ROW;
 }
 
 void
@@ -527,24 +520,23 @@
 
   if (statement.step() == SQLITE_ROW)
     return Certificate(statement.getBlock(0));
-  else
-    NDN_THROW(Pib::Error("Certificate `" + certName.toUri() + "` does not exit"));
+
+  NDN_THROW(Pib::Error("Certificate `" + certName.toUri() + "` not found in PIB"));
 }
 
 std::set<Name>
 PibSqlite3::getCertificatesOfKey(const Name& keyName) const
 {
   std::set<Name> certNames;
-
   Sqlite3Statement statement(m_database,
                              "SELECT certificate_name "
                              "FROM certificates JOIN keys ON certificates.key_id=keys.id "
                              "WHERE keys.key_name=?");
   statement.bind(1, keyName.wireEncode(), SQLITE_TRANSIENT);
 
-  while (statement.step() == SQLITE_ROW)
+  while (statement.step() == SQLITE_ROW) {
     certNames.insert(Name(statement.getBlock(0)));
-
+  }
   return certNames;
 }
 
@@ -552,7 +544,7 @@
 PibSqlite3::setDefaultCertificateOfKey(const Name& keyName, const Name& certName)
 {
   if (!hasCertificate(certName)) {
-    NDN_THROW(Pib::Error("Certificate `" + certName.toUri() + "` does not exist"));
+    NDN_THROW(Pib::Error("Cannot set non-existing certificate `" + certName.toUri() + "` as default"));
   }
 
   Sqlite3Statement statement(m_database,
@@ -572,8 +564,8 @@
 
   if (statement.step() == SQLITE_ROW)
     return Certificate(statement.getBlock(0));
-  else
-    NDN_THROW(Pib::Error("No default certificate for key `" + keyName.toUri() + "`"));
+
+  NDN_THROW(Pib::Error("No default certificate for key `" + keyName.toUri() + "`"));
 }
 
 bool
diff --git a/ndn-cxx/security/pib/pib-impl.hpp b/ndn-cxx/security/pib/pib-impl.hpp
index 5b61c86..bdf742b 100644
--- a/ndn-cxx/security/pib/pib-impl.hpp
+++ b/ndn-cxx/security/pib/pib-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).
  *
@@ -32,27 +32,28 @@
 namespace pib {
 
 /**
- * @brief Abstract class of PIB implementation
+ * @brief PIB backend interface.
  *
- * This class defines the interface that an actual PIB (e.g., one based on sqlite3)
- * implementation should provide.
+ * This abstract class defines the interface that an actual PIB implementation
+ * (e.g., one based on sqlite3) must provide.
+ *
+ * @sa Pib
  */
 class PibImpl : noncopyable
 {
 public:
   /**
-   * @brief represents a non-semantic error
+   * @brief Represents a non-semantic error.
    *
    * A subclass of PibImpl may throw a subclass of this type when
    * there's a non-semantic error, such as a storage problem.
-  */
+   */
   class Error : public std::runtime_error
   {
   public:
     using std::runtime_error::runtime_error;
   };
 
-public:
   virtual
   ~PibImpl() = default;
 
diff --git a/ndn-cxx/security/pib/pib.cpp b/ndn-cxx/security/pib/pib.cpp
index a67bda5..936a5ec 100644
--- a/ndn-cxx/security/pib/pib.cpp
+++ b/ndn-cxx/security/pib/pib.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).
  *
@@ -27,14 +27,13 @@
 namespace security {
 namespace pib {
 
-NDN_LOG_INIT(ndn.security.pib.Pib);
+NDN_LOG_INIT(ndn.security.Pib);
 
 Pib::Pib(const std::string& scheme, const std::string& location, shared_ptr<PibImpl> impl)
   : m_scheme(scheme)
   , m_location(location)
-  , m_isDefaultIdentityLoaded(false)
-  , m_identities(impl)
   , m_impl(std::move(impl))
+  , m_identities(m_impl)
 {
   BOOST_ASSERT(m_impl != nullptr);
 }
@@ -53,6 +52,8 @@
   if (tpmLocator == m_impl->getTpmLocator()) {
     return;
   }
+
+  NDN_LOG_DEBUG("Resetting TPM locator to " << tpmLocator);
   reset();
   m_impl->setTpmLocator(tpmLocator);
 }
@@ -60,7 +61,7 @@
 std::string
 Pib::getTpmLocator() const
 {
-  std::string tpmLocator = m_impl->getTpmLocator();
+  auto tpmLocator = m_impl->getTpmLocator();
   if (tpmLocator.empty()) {
     NDN_THROW(Pib::Error("TPM info does not exist"));
   }
@@ -70,37 +71,35 @@
 void
 Pib::reset()
 {
-  m_impl->clearIdentities();
   m_impl->setTpmLocator("");
-  m_isDefaultIdentityLoaded = false;
+  m_impl->clearIdentities();
+  m_defaultIdentity = {};
   m_identities.reset();
 }
 
 Identity
-Pib::addIdentity(const Name& identity)
+Pib::addIdentity(const Name& identityName)
 {
   BOOST_ASSERT(m_identities.isConsistent());
-
-  return m_identities.add(identity);
+  return m_identities.add(identityName);
 }
 
 void
-Pib::removeIdentity(const Name& identity)
+Pib::removeIdentity(const Name& identityName)
 {
   BOOST_ASSERT(m_identities.isConsistent());
 
-  if (m_isDefaultIdentityLoaded && m_defaultIdentity.getName() == identity) {
-    m_isDefaultIdentityLoaded = false;
+  if (m_defaultIdentity && m_defaultIdentity.getName() == identityName) {
+    NDN_LOG_DEBUG("Removing default identity " << identityName);
+    m_defaultIdentity = {};
   }
-
-  m_identities.remove(identity);
+  m_identities.remove(identityName);
 }
 
 Identity
 Pib::getIdentity(const Name& identity) const
 {
   BOOST_ASSERT(m_identities.isConsistent());
-
   return m_identities.get(identity);
 }
 
@@ -108,36 +107,34 @@
 Pib::getIdentities() const
 {
   BOOST_ASSERT(m_identities.isConsistent());
-
   return m_identities;
 }
 
-const Identity&
+Identity
 Pib::setDefaultIdentity(const Name& identityName)
 {
   BOOST_ASSERT(m_identities.isConsistent());
 
   m_defaultIdentity = m_identities.add(identityName);
-  m_isDefaultIdentityLoaded = true;
-  NDN_LOG_DEBUG("Default identity is set to " << identityName);
-
   m_impl->setDefaultIdentity(identityName);
+  NDN_LOG_DEBUG("Default identity set to " << identityName);
+
+  BOOST_ASSERT(m_defaultIdentity);
   return m_defaultIdentity;
 }
 
-const Identity&
+Identity
 Pib::getDefaultIdentity() const
 {
   BOOST_ASSERT(m_identities.isConsistent());
 
-  if (!m_isDefaultIdentityLoaded) {
+  if (!m_defaultIdentity) {
     m_defaultIdentity = m_identities.get(m_impl->getDefaultIdentity());
-    m_isDefaultIdentityLoaded = true;
-    NDN_LOG_DEBUG("Default identity is " << m_defaultIdentity.getName());
+    NDN_LOG_DEBUG("Caching default identity " << m_defaultIdentity.getName());
   }
 
-  BOOST_ASSERT(m_impl->getDefaultIdentity() == m_defaultIdentity.getName());
-
+  BOOST_ASSERT(m_defaultIdentity);
+  BOOST_ASSERT(m_defaultIdentity.getName() == m_impl->getDefaultIdentity());
   return m_defaultIdentity;
 }
 
diff --git a/ndn-cxx/security/pib/pib.hpp b/ndn-cxx/security/pib/pib.hpp
index 5fb38a3..837e029 100644
--- a/ndn-cxx/security/pib/pib.hpp
+++ b/ndn-cxx/security/pib/pib.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).
  *
@@ -31,7 +31,7 @@
 class PibImpl;
 
 /**
- * @brief represents the PIB
+ * @brief Frontend to the Public Information Base.
  *
  * The PIB (Public Information Base) stores the public portion of a user's cryptography keys.
  * The format and location of stored information is indicated by the PibLocator.
@@ -40,88 +40,85 @@
  * by the PIB to enforce this association and prevent one from operating on mismatched PIB and TPM.
  *
  * Information in the PIB is organized in a hierarchy of Identity-Key-Certificate. At the top level,
- * the Pib class provides access to identities, and allows setting a default identity. Properties of
- * an identity can be accessed after obtaining an Identity object.
+ * the Pib class provides access to identities and allows setting a default identity. The properties
+ * of an identity can be accessed after obtaining an Identity object.
  *
  * @note Pib instance is created and managed only by KeyChain. KeyChain::getPib() returns
- *       a const reference to the managed Pib instance, through which it is possible to
+ *       a reference to the managed Pib instance, through which it is possible to
  *       retrieve information about identities, keys, and certificates.
  *
- * @throw PibImpl::Error when underlying implementation has non-semantic error.
+ * @throw PibImpl::Error When the underlying implementation has a non-semantic error.
  */
 class Pib : noncopyable
 {
 public:
-  /// @brief represents a semantic error
+  /// @brief Represents a semantic error.
   class Error : public std::runtime_error
   {
   public:
     using std::runtime_error::runtime_error;
   };
 
-public:
   ~Pib();
 
   /**
-   * @brief return the scheme of the PIB Locator
+   * @brief Return the scheme of the PIB Locator.
    */
-  std::string
+  const std::string&
   getScheme() const
   {
     return m_scheme;
   }
 
   /**
-   * @brief Get PIB Locator
+   * @brief Return the PIB Locator.
    */
   std::string
   getPibLocator() const;
 
   /**
-   * @brief Set the corresponding TPM information to @p tpmLocator.
-   *
-   * If the provided @p tpmLocator is different from the existing one, PIB will be reset.
-   * Otherwise, nothing will be changed.
+   * @brief Set the associated TPM information to @p tpmLocator.
+   * @note If the provided @p tpmLocator differs from the current one, reset() is called.
    */
   void
   setTpmLocator(const std::string& tpmLocator);
 
   /**
-   * @brief Get TPM Locator
-   * @throws Error if TPM locator is empty
+   * @brief Return the associated TPM Locator.
+   * @throws Error TPM locator is not set
    */
   std::string
   getTpmLocator() const;
 
   /**
-   * @brief Reset content in PIB, including reset of the TPM locator
+   * @brief Reset the contents of the PIB, including reset of the TPM Locator.
    */
   void
   reset();
 
   /**
-   * @brief Get an identity with name @p identityName.
-   * @throw Pib::Error if the identity does not exist.
+   * @brief Return an identity with name @p identityName.
+   * @throw Pib::Error The desired identity does not exist.
    */
   Identity
   getIdentity(const Name& identityName) const;
 
   /**
-   * @brief Get all the identities
+   * @brief Return all the identities.
    */
   const IdentityContainer&
   getIdentities() const;
 
   /**
-   * @brief Get the default identity.
-   * @throw Pib::Error if no default identity exists.
+   * @brief Return the default identity.
+   * @throw Pib::Error No default identity exists.
    */
-  const Identity&
+  Identity
   getDefaultIdentity() const;
 
-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 Create a Pib instance
+   * @brief Create a Pib instance.
    *
    * @param scheme The scheme for the Pib
    * @param location The location for the Pib
@@ -132,7 +129,7 @@
   /**
    * @brief Add an identity.
    *
-   * If no default identity is set before, the new identity will be set as the default identity
+   * If no default identity is currently set, the new identity will become the default identity.
    *
    * @return handle of the added identity.
    */
@@ -142,7 +139,7 @@
   /**
    * @brief Remove an identity.
    *
-   * If the default identity is being removed, no default identity will be selected.
+   * If the default identity is being removed, the PIB will no longer have a default identity.
    */
   void
   removeIdentity(const Name& identity);
@@ -150,29 +147,20 @@
   /**
    * @brief Set an identity as the default identity.
    *
-   * Create the identity if it does not exist.
+   * The identity will be created if it does not exist.
    *
    * @return handle of the default identity
    */
-  const Identity&
+  Identity
   setDefaultIdentity(const Name& identity);
 
-  shared_ptr<PibImpl>
-  getImpl()
-  {
-    return m_impl;
-  }
-
-protected:
-  std::string m_scheme;
-  std::string m_location;
-
-  mutable bool m_isDefaultIdentityLoaded;
-  mutable Identity m_defaultIdentity;
+private:
+  const std::string m_scheme;
+  const std::string m_location;
+  const shared_ptr<PibImpl> m_impl;
 
   IdentityContainer m_identities;
-
-  shared_ptr<PibImpl> m_impl;
+  mutable Identity m_defaultIdentity;
 
   friend KeyChain;
 };
diff --git a/tests/unit/security/pib/pib.t.cpp b/tests/unit/security/pib/pib.t.cpp
index bcd90f9..c0d4f28 100644
--- a/tests/unit/security/pib/pib.t.cpp
+++ b/tests/unit/security/pib/pib.t.cpp
@@ -38,19 +38,6 @@
 
 using pib::Pib;
 
-BOOST_AUTO_TEST_CASE(ValidityChecking)
-{
-  Pib pib("pib-memory", "", make_shared<PibMemory>());
-
-  Identity id = pib.addIdentity(id1);
-  BOOST_CHECK(id);
-  BOOST_CHECK_EQUAL(!id, false);
-
-  Key key = id.addKey(id1Key1, id1Key1Name);
-  BOOST_CHECK(key);
-  BOOST_CHECK_EQUAL(!key, false);
-}
-
 BOOST_AUTO_TEST_CASE(TpmLocator)
 {
   Pib pib("pib-memory", "", make_shared<PibMemory>());
@@ -78,46 +65,55 @@
 BOOST_AUTO_TEST_CASE(IdentityOperations)
 {
   Pib pib("pib-memory", "", make_shared<PibMemory>());
-  BOOST_CHECK_EQUAL(pib.getIdentities().size(), 0);
 
-  // get non-existing identity, throw Pib::Error
+  // PIB starts with no identities
+  BOOST_CHECK_EQUAL(pib.getIdentities().size(), 0);
   BOOST_CHECK_THROW(pib.getIdentity(id1), Pib::Error);
   // get default identity when it is not set yet, throw Pib::Error
   BOOST_CHECK_THROW(pib.getDefaultIdentity(), Pib::Error);
 
   // add identity
   pib.addIdentity(id1);
-  BOOST_CHECK_NO_THROW(pib.getIdentity(id1));
   BOOST_CHECK_EQUAL(pib.getIdentities().size(), 1);
+  BOOST_CHECK_EQUAL(pib.getIdentity(id1).getName(), id1);
+  // add another
+  pib.addIdentity(id2);
+  BOOST_CHECK_EQUAL(pib.getIdentities().size(), 2);
+  BOOST_CHECK_EQUAL(pib.getIdentity(id2).getName(), id2);
 
-  // new key becomes default key when there was no default key
-  BOOST_REQUIRE_NO_THROW(pib.getDefaultIdentity());
+  // first identity implicitly becomes the default when there was no default identity
   BOOST_CHECK_EQUAL(pib.getDefaultIdentity().getName(), id1);
 
-  // remove identity
+  // remove both identities
+  pib.removeIdentity(id2);
+  BOOST_CHECK_EQUAL(pib.getIdentities().size(), 1);
+  BOOST_CHECK_EQUAL(pib.getIdentity(id1).getName(), id1);
+  BOOST_CHECK_THROW(pib.getIdentity(id2), Pib::Error);
+  BOOST_CHECK_EQUAL(pib.getDefaultIdentity().getName(), id1);
   pib.removeIdentity(id1);
+  BOOST_CHECK_EQUAL(pib.getIdentities().size(), 0);
   BOOST_CHECK_THROW(pib.getIdentity(id1), Pib::Error);
   BOOST_CHECK_THROW(pib.getDefaultIdentity(), Pib::Error);
-  BOOST_CHECK_EQUAL(pib.getIdentities().size(), 0);
 
-  // set default identity
-  BOOST_REQUIRE_NO_THROW(pib.setDefaultIdentity(id1));
-  BOOST_REQUIRE_NO_THROW(pib.getDefaultIdentity());
+  // set default identity (and implicitly create it)
+  pib.setDefaultIdentity(id1);
   BOOST_CHECK_EQUAL(pib.getDefaultIdentity().getName(), id1);
   BOOST_CHECK_EQUAL(pib.getIdentities().size(), 1);
-  BOOST_REQUIRE_NO_THROW(pib.setDefaultIdentity(id2));
-  BOOST_REQUIRE_NO_THROW(pib.getDefaultIdentity());
+  pib.setDefaultIdentity(id2);
   BOOST_CHECK_EQUAL(pib.getDefaultIdentity().getName(), id2);
   BOOST_CHECK_EQUAL(pib.getIdentities().size(), 2);
 
   // remove default identity
   pib.removeIdentity(id2);
+  BOOST_CHECK_EQUAL(pib.getIdentities().size(), 1);
+  BOOST_CHECK_EQUAL(pib.getIdentity(id1).getName(), id1);
   BOOST_CHECK_THROW(pib.getIdentity(id2), Pib::Error);
   BOOST_CHECK_THROW(pib.getDefaultIdentity(), Pib::Error);
-  BOOST_CHECK_EQUAL(pib.getIdentities().size(), 1);
-  pib.removeIdentity(id1);
-  BOOST_CHECK_THROW(pib.getIdentity(id1), Pib::Error);
-  BOOST_CHECK_EQUAL(pib.getIdentities().size(), 0);
+
+  // adding an identity now makes it the default
+  pib.addIdentity(id2);
+  BOOST_CHECK_EQUAL(pib.getIdentities().size(), 2);
+  BOOST_CHECK_EQUAL(pib.getDefaultIdentity().getName(), id2);
 }
 
 BOOST_AUTO_TEST_SUITE_END() // TestPib