security: code cleanup and doxygen improvements in tpm::BackEndOsx

Change-Id: I6d6d77ab315a10c280e7643d4cec5269ed10fe5e

.jenkins.d/40-standalone-header-compilation.sh

@@ -7,7 +7,7 @@
 
 CXX=${CXX:-g++}
 STD=-std=c++14
-CXXFLAGS="-O2 -Wall -Wno-unused-local-typedef $(pkg-config --cflags libndn-cxx)"
+CXXFLAGS="-O2 -Wall -Wno-unused-const-variable -Wno-unused-local-typedef $(pkg-config --cflags libndn-cxx)"
 INCLUDEDIR="$(pkg-config --variable=includedir libndn-cxx)"/ndn-cxx
 
 echo "Using: $CXX $STD $CXXFLAGS"

src/net/detail/network-monitor-impl-osx.cpp

@@ -54,6 +54,7 @@
 
 #include "../network-address.hpp"
 #include "../../name.hpp"
+#include "../../util/cf-string-osx.hpp"
 #include "../../util/logger.hpp"
 
 #include <ifaddrs.h>      // for getifaddrs()
@@ -203,26 +204,6 @@ NetworkMonitorImplOsx::enumerateInterfaces()
   this->emitSignal(onEnumerationCompleted);
 }
 
-static std::string
-convertToStdString(CFStringRef cfStr)
-{
-  const char* cStr = CFStringGetCStringPtr(cfStr, kCFStringEncodingASCII);
-  if (cStr != nullptr) {
-    // fast path
-    return cStr;
-  }
-
-  // reserve space for the string + null terminator
-  std::string str(CFStringGetLength(cfStr) + 1, '\0');
-  if (!CFStringGetCString(cfStr, &str.front(), str.size(), kCFStringEncodingASCII)) {
-    BOOST_THROW_EXCEPTION(NetworkMonitorImplOsx::Error("CFString conversion failed"));
-  }
-
-  // drop the null terminator, std::string doesn't need it
-  str.pop_back();
-  return str;
-}
-
 std::set<std::string>
 NetworkMonitorImplOsx::getInterfaceNames() const
 {
@@ -241,7 +222,7 @@ NetworkMonitorImplOsx::getInterfaceNames() const
   size_t count = CFArrayGetCount(interfaces);
   for (size_t i = 0; i != count; ++i) {
     auto ifName = (CFStringRef)CFArrayGetValueAtIndex(interfaces, i);
-    ifNames.insert(convertToStdString(ifName));
+    ifNames.insert(util::cfstring::toStdString(ifName));
   }
   return ifNames;
 }
@@ -268,9 +249,7 @@ InterfaceState
 NetworkMonitorImplOsx::getInterfaceState(const NetworkInterface& netif) const
 {
   CFReleaser<CFStringRef> linkName =
-    CFStringCreateWithCString(kCFAllocatorDefault,
-                              ("State:/Network/Interface/" + netif.getName() + "/Link").data(),
-                              kCFStringEncodingASCII);
+    util::cfstring::fromStdString("State:/Network/Interface/" + netif.getName() + "/Link");
 
   CFReleaser<CFDictionaryRef> dict =
     (CFDictionaryRef)SCDynamicStoreCopyValue(m_scStore.get(), linkName.get());
@@ -426,7 +405,7 @@ NetworkMonitorImplOsx::onConfigChanged(CFArrayRef changedKeys)
 
   size_t count = CFArrayGetCount(changedKeys);
   for (size_t i = 0; i != count; ++i) {
-    Name key(convertToStdString((CFStringRef)CFArrayGetValueAtIndex(changedKeys, i)));
+    Name key(util::cfstring::toStdString((CFStringRef)CFArrayGetValueAtIndex(changedKeys, i)));
     std::string ifName = key.at(-2).toUri();
 
     auto ifIt = m_interfaces.find(ifName);

src/security/tpm/back-end-file.hpp

@@ -34,90 +34,55 @@ class PrivateKey;
 namespace tpm {
 
 /**
- * @brief The back-end implementation of file-based TPM.
+ * @brief The back-end implementation of a file-based TPM.
  *
  * In this TPM, each private key is stored in a separate file with permission 0400, i.e.,
  * owner read-only.  The key is stored in PKCS #1 format in base64 encoding.
  */
-class BackEndFile : public BackEnd
+class BackEndFile final : public BackEnd
 {
 public:
   /**
-   * @brief Create file-based TPM backend
-   * @param location Directory to store private keys
+   * @brief Create file-based TPM backend.
+   *
+   * @param location Directory to store private keys.
    */
   explicit
   BackEndFile(const std::string& location = "");
 
-  ~BackEndFile() override;
+  ~BackEndFile() final;
 
   static const std::string&
   getScheme();
 
 private: // inherited from tpm::BackEnd
-  /**
-   * @return True if a key with name @p keyName exists in TPM.
-   */
   bool
   doHasKey(const Name& keyName) const final;
 
-  /**
-   * @return The handle of a key with name @p keyName, or nullptr if the key does not exist
-   */
   unique_ptr<KeyHandle>
   doGetKeyHandle(const Name& keyName) const final;
 
-  /**
-   * @brief Create key for @p identityName according to @p params.
-   *
-   * The created key is named as: /<identityName>/[keyId]/KEY
-   * The key name is set in the returned KeyHandle.
-   *
-   * If the key with the same name exists, the old key will be overwritten.
-   * The behavior of using KeyHandler of removed key is undefined.
-   *
-   * @return The handle of the created key.
-   */
   unique_ptr<KeyHandle>
   doCreateKey(const Name& identityName, const KeyParams& params) final;
 
-  /**
-   * @brief Delete a key with name @p keyName.
-   *
-   * @throw Error the deletion failed
-   */
   void
   doDeleteKey(const Name& keyName) final;
 
-  /**
-   * @return A private key with name @p keyName in encrypted PKCS #8 format using password @p pw
-   * @throw Error the key cannot be exported, e.g., not enough privilege
-   */
   ConstBufferPtr
   doExportKey(const Name& keyName, const char* pw, size_t pwLen) final;
 
-  /**
-   * @brief Import a private key in encrypted PKCS #8 format
-   *
-   * @param keyName The name of imported private key
-   * @param buf Pointer to the key in encrypted PKCS #8 format
-   * @param size The size of the key in encrypted PKCS #8 format
-   * @param pw The password to decrypt the key
-   * @param pwLen The length of the password
-   * @throw Error import failed
-   */
   void
   doImportKey(const Name& keyName, const uint8_t* buf, size_t size, const char* pw, size_t pwLen) final;
 
 private:
   /**
-   * @brief Load a private key with name @p keyName from the key file directory
+   * @brief Load a private key with name @p keyName from the key directory.
    */
   unique_ptr<transform::PrivateKey>
   loadKey(const Name& keyName) const;
 
   /**
-   * @brief Save a private key with name @p keyName into the key file directory
+   * @brief Save a private key with name @p keyName into the key directory.
    */
   void
   saveKey(const Name& keyName, const transform::PrivateKey& key);

src/security/tpm/back-end-mem.hpp

@@ -29,75 +29,40 @@ namespace security {
 namespace tpm {
 
 /**
- * @brief The back-end implementation of in-memory TPM.
+ * @brief The back-end implementation of an in-memory TPM.
  */
-class BackEndMem : public BackEnd
+class BackEndMem final : public BackEnd
 {
 public:
   /**
-   * @brief Create memory-based TPM backend
-   * @param location Not used (required by the TPM-registration interface)
+   * @brief Create memory-based TPM backend.
+   *
+   * @param location Not used (required by the TPM registration interface).
    */
   explicit
   BackEndMem(const std::string& location = "");
 
-  ~BackEndMem() override;
+  ~BackEndMem() final;
 
   static const std::string&
   getScheme();
 
 private: // inherited from tpm::BackEnd
-
-  /**
-   * @return True if a key with name @p keyName exists in TPM.
-   */
   bool
   doHasKey(const Name& keyName) const final;
 
-  /**
-   * @return The handle of a key with name @p keyName, or nullptr if the key does not exist
-   */
   unique_ptr<KeyHandle>
   doGetKeyHandle(const Name& keyName) const final;
 
-  /**
-   * @brief Create key for @p identityName according to @p params.
-   *
-   * The created key is named as: /<identityName>/[keyId]/KEY
-   * The key name is set in the returned KeyHandle.
-   * If the key with the same name is created, the old one will be removed.
-   * The behavior of using KeyHandler of removed key is undefined.
-   *
-   * @return The handle of the created key.
-   */
   unique_ptr<KeyHandle>
   doCreateKey(const Name& identityName, const KeyParams& params) final;
 
-  /**
-   * @brief Delete a key with name @p keyName.
-   *
-   * @throw Error the deletion failed
-   */
   void
   doDeleteKey(const Name& keyName) final;
 
-  /**
-   * @return A private key with name @p keyName in encrypted PKCS #8 format using password @p pw
-   * @throw Error the key cannot be exported, e.g., not enough privilege
-   */
   ConstBufferPtr
   doExportKey(const Name& keyName, const char* pw, size_t pwLen) final;
 
-  /**
-   * @brief Import a private key in encrypted PKCS #8 format
-   *
-   * @param keyName The name of imported private key
-   * @param buf Pointer to the key in encrypted PKCS #8 format
-   * @param size The size of the key in encrypted PKCS #8 format
-   * @param pw The password to decrypt the key
-   * @param pwLen The length of password
-   * @throw Error import failed
-   */
   void
   doImportKey(const Name& keyName, const uint8_t* buf, size_t size, const char* pw, size_t pwLen) final;
 

src/security/tpm/back-end-osx.cpp

@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2013-2017 Regents of the University of California.
+ * Copyright (c) 2013-2018 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
  *
@@ -23,66 +23,37 @@
 #include "key-handle-osx.hpp"
 #include "tpm.hpp"
 #include "../transform/private-key.hpp"
+#include "../../util/cf-string-osx.hpp"
 
-#include <CoreServices/CoreServices.h>
-#include <Security/SecDigestTransform.h>
-#include <Security/SecRandom.h>
 #include <Security/Security.h>
 
 namespace ndn {
 namespace security {
 namespace tpm {
 
+namespace cfstring = util::cfstring;
 using util::CFReleaser;
 
 class BackEndOsx::Impl
 {
-public:
-  Impl()
-    : isTerminalMode(false)
-  {
-  }
-
-  /**
-   * @brief Get private key reference with name @p keyName.
-   *
-   * @param keyName
-   * @returns reference to the key
-   */
-  CFReleaser<SecKeychainItemRef>
-  getKey(const Name& keyName)
-  {
-    CFReleaser<CFStringRef> keyLabel = CFStringCreateWithCString(nullptr, keyName.toUri().c_str(),
-                                                                 kCFStringEncodingUTF8);
-
-    CFReleaser<CFMutableDictionaryRef> attrDict =
-      CFDictionaryCreateMutable(nullptr, 5, &kCFTypeDictionaryKeyCallBacks, nullptr);
-
-    CFDictionaryAddValue(attrDict.get(), kSecClass, kSecClassKey);
-    CFDictionaryAddValue(attrDict.get(), kSecAttrLabel, keyLabel.get());
-    CFDictionaryAddValue(attrDict.get(), kSecAttrKeyClass, kSecAttrKeyClassPrivate);
-    CFDictionaryAddValue(attrDict.get(), kSecReturnRef, kCFBooleanTrue);
-
-    CFReleaser<SecKeychainItemRef> keyItem;
-    // C-style cast is used as per Apple convention
-    OSStatus res = SecItemCopyMatching((CFDictionaryRef)attrDict.get(), (CFTypeRef*)&keyItem.get());
-    keyItem.retain();
-
-    if (res != errSecSuccess) {
-      if (res == errSecAuthFailed) {
-        BOOST_THROW_EXCEPTION(Error("Fail to unlock the keychain"));
-      }
-      BOOST_THROW_EXCEPTION(std::domain_error("Key does not exist"));
-    }
-
-    return keyItem;
-  }
-
 public:
   SecKeychainRef keyChainRef;
-  bool isTerminalMode;
+  bool isTerminalMode = false;
 };
 
+static CFReleaser<CFDataRef>
+makeCFDataNoCopy(const uint8_t* buf, size_t buflen)
+{
+  return CFDataCreateWithBytesNoCopy(kCFAllocatorDefault, buf, buflen, kCFAllocatorNull);
+}
+
+static CFReleaser<CFMutableDictionaryRef>
+makeCFMutableDictionary()
+{
+  return CFDictionaryCreateMutable(kCFAllocatorDefault, 0,
+                                   &kCFTypeDictionaryKeyCallBacks,
+                                   &kCFTypeDictionaryValueCallBacks);
+}
 
 static CFTypeRef
 getAsymKeyType(KeyType keyType)
@@ -107,7 +78,7 @@ getDigestAlgorithm(DigestAlgorithm digestAlgo)
   case DigestAlgorithm::SHA512:
     return kSecDigestSHA2;
   default:
-    return 0;
+    return nullptr;
   }
 }
 
@@ -128,14 +99,43 @@ getDigestSize(DigestAlgorithm digestAlgo)
   }
 }
 
+/**
+ * @brief Get reference to private key with name @p keyName.
+ * @param keyName
+ */
+static CFReleaser<SecKeychainItemRef>
+getKey(const Name& keyName)
+{
+  auto keyLabel = cfstring::fromStdString(keyName.toUri());
+
+  auto attrDict = makeCFMutableDictionary();
+  CFDictionaryAddValue(attrDict.get(), kSecClass, kSecClassKey);
+  CFDictionaryAddValue(attrDict.get(), kSecAttrLabel, keyLabel.get());
+  CFDictionaryAddValue(attrDict.get(), kSecAttrKeyClass, kSecAttrKeyClassPrivate);
+  CFDictionaryAddValue(attrDict.get(), kSecReturnRef, kCFBooleanTrue);
+
+  CFReleaser<SecKeychainItemRef> keyItem;
+  // C-style cast is used as per Apple convention
+  OSStatus res = SecItemCopyMatching((CFDictionaryRef)attrDict.get(), (CFTypeRef*)&keyItem.get());
+  keyItem.retain();
+
+  if (res != errSecSuccess) {
+    if (res == errSecAuthFailed) {
+      BOOST_THROW_EXCEPTION(BackEnd::Error("Fail to unlock the keychain"));
+    }
+    return nullptr;
+  }
+
+  return keyItem;
+}
+
 BackEndOsx::BackEndOsx(const std::string&)
   : m_impl(make_unique<Impl>())
 {
   SecKeychainSetUserInteractionAllowed(!m_impl->isTerminalMode);
 
   OSStatus res = SecKeychainCopyDefault(&m_impl->keyChainRef);
-
-  if (res == errSecNoDefaultKeychain) { //If no default key chain, create one.
+  if (res == errSecNoDefaultKeychain) {
     BOOST_THROW_EXCEPTION(Error("No default keychain, create one first"));
   }
 }
@@ -166,12 +166,11 @@ bool
 BackEndOsx::isTpmLocked() const
 {
   SecKeychainStatus keychainStatus;
-
   OSStatus res = SecKeychainGetStatus(m_impl->keyChainRef, &keychainStatus);
   if (res != errSecSuccess)
     return true;
   else
-    return ((kSecUnlockStateStatus & keychainStatus) == 0);
+    return (kSecUnlockStateStatus & keychainStatus) == 0;
 }
 
 bool
@@ -203,8 +202,8 @@ BackEndOsx::sign(const KeyRefOsx& key, DigestAlgorithm digestAlgo, const uint8_t
   }
 
   // Set input
-  CFReleaser<CFDataRef> dataRef = CFDataCreateWithBytesNoCopy(nullptr, buf, size, kCFAllocatorNull);
-  SecTransformSetAttribute(signer.get(), kSecTransformInputAttributeName, dataRef.get(), &error.get());
+  auto data = makeCFDataNoCopy(buf, size);
+  SecTransformSetAttribute(signer.get(), kSecTransformInputAttributeName, data.get(), &error.get());
   if (error != nullptr) {
     BOOST_THROW_EXCEPTION(Error("Fail to configure input of signer"));
   }
@@ -223,7 +222,7 @@ BackEndOsx::sign(const KeyRefOsx& key, DigestAlgorithm digestAlgo, const uint8_t
 
   // Set digest length
   long digestSize = getDigestSize(digestAlgo);
-  CFReleaser<CFNumberRef> cfDigestSize = CFNumberCreate(nullptr, kCFNumberLongType, &digestSize);
+  CFReleaser<CFNumberRef> cfDigestSize = CFNumberCreate(kCFAllocatorDefault, kCFNumberLongType, &digestSize);
   SecTransformSetAttribute(signer.get(), kSecDigestLengthAttribute, cfDigestSize.get(), &error.get());
   if (error != nullptr) {
     BOOST_THROW_EXCEPTION(Error("Fail to configure digest length of signer"));
@@ -253,8 +252,8 @@ BackEndOsx::decrypt(const KeyRefOsx& key, const uint8_t* cipherText, size_t ciph
     BOOST_THROW_EXCEPTION(Error("Fail to create decryptor"));
   }
 
-  CFReleaser<CFDataRef> dataRef = CFDataCreateWithBytesNoCopy(nullptr, cipherText, cipherSize, kCFAllocatorNull);
-  SecTransformSetAttribute(decryptor.get(), kSecTransformInputAttributeName, dataRef.get(), &error.get());
+  auto data = makeCFDataNoCopy(cipherText, cipherSize);
+  SecTransformSetAttribute(decryptor.get(), kSecTransformInputAttributeName, data.get(), &error.get());
   if (error != nullptr) {
     BOOST_THROW_EXCEPTION(Error("Fail to configure decryptor input"));
   }
@@ -304,12 +303,9 @@ BackEndOsx::derivePublicKey(const KeyRefOsx& key)
 bool
 BackEndOsx::doHasKey(const Name& keyName) const
 {
-  CFReleaser<CFStringRef> keyLabel = CFStringCreateWithCString(nullptr, keyName.toUri().c_str(),
-                                                               kCFStringEncodingUTF8);
-
-  CFReleaser<CFMutableDictionaryRef> attrDict =
-    CFDictionaryCreateMutable(nullptr, 4, &kCFTypeDictionaryKeyCallBacks, nullptr);
+  auto keyLabel = cfstring::fromStdString(keyName.toUri());
 
+  auto attrDict = makeCFMutableDictionary();
   CFDictionaryAddValue(attrDict.get(), kSecClass, kSecClassKey);
   CFDictionaryAddValue(attrDict.get(), kSecAttrLabel, keyLabel.get());
   CFDictionaryAddValue(attrDict.get(), kSecReturnRef, kCFBooleanTrue);
@@ -325,15 +321,12 @@ BackEndOsx::doHasKey(const Name& keyName) const
 unique_ptr<KeyHandle>
 BackEndOsx::doGetKeyHandle(const Name& keyName) const
 {
-  CFReleaser<SecKeychainItemRef> keyItem;
-  try {
-    keyItem = m_impl->getKey(keyName);
-  }
-  catch (const std::domain_error&) {
+  CFReleaser<SecKeychainItemRef> keychainItem = getKey(keyName);
+  if (keychainItem == nullptr) {
     return nullptr;
   }
 
-  return make_unique<KeyHandleOsx>((SecKeyRef)keyItem.get());
+  return make_unique<KeyHandleOsx>((SecKeyRef)keychainItem.get());
 }
 
 unique_ptr<KeyHandle>
@@ -356,10 +349,9 @@ BackEndOsx::doCreateKey(const Name& identityName, const KeyParams& params)
       BOOST_THROW_EXCEPTION(Tpm::Error("Fail to create a key pair: Unsupported key type"));
     }
   }
-  CFReleaser<CFNumberRef> cfKeySize = CFNumberCreate(nullptr, kCFNumberIntType, &keySize);
+  CFReleaser<CFNumberRef> cfKeySize = CFNumberCreate(kCFAllocatorDefault, kCFNumberIntType, &keySize);
 
-  CFReleaser<CFMutableDictionaryRef> attrDict =
-    CFDictionaryCreateMutable(nullptr, 2, &kCFTypeDictionaryKeyCallBacks, nullptr);
+  auto attrDict = makeCFMutableDictionary();
   CFDictionaryAddValue(attrDict.get(), kSecAttrKeyType, getAsymKeyType(keyType));
   CFDictionaryAddValue(attrDict.get(), kSecAttrKeySizeInBits, cfKeySize.get());
 
@@ -372,8 +364,6 @@ BackEndOsx::doCreateKey(const Name& identityName, const KeyParams& params)
   publicKey.retain();
   privateKey.retain();
 
-  BOOST_ASSERT(privateKey != nullptr);
-
   if (res != errSecSuccess) {
     if (res == errSecAuthFailed) {
       BOOST_THROW_EXCEPTION(Error("Fail to unlock the keychain"));
@@ -405,15 +395,13 @@ BackEndOsx::doCreateKey(const Name& identityName, const KeyParams& params)
 void
 BackEndOsx::doDeleteKey(const Name& keyName)
 {
-  CFReleaser<CFStringRef> keyLabel = CFStringCreateWithCString(nullptr, keyName.toUri().c_str(),
-                                                               kCFStringEncodingUTF8);
-
-  CFReleaser<CFMutableDictionaryRef> searchDict =
-    CFDictionaryCreateMutable(nullptr, 5, &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks);
+  auto keyLabel = cfstring::fromStdString(keyName.toUri());
 
+  auto searchDict = makeCFMutableDictionary();
   CFDictionaryAddValue(searchDict.get(), kSecClass, kSecClassKey);
   CFDictionaryAddValue(searchDict.get(), kSecAttrLabel, keyLabel.get());
   CFDictionaryAddValue(searchDict.get(), kSecMatchLimit, kSecMatchLimitAll);
+
   OSStatus res = SecItemDelete(searchDict.get());
 
   if (res != errSecSuccess) {
@@ -429,22 +417,18 @@ BackEndOsx::doDeleteKey(const Name& keyName)
 ConstBufferPtr
 BackEndOsx::doExportKey(const Name& keyName, const char* pw, size_t pwLen)
 {
-  CFReleaser<SecKeychainItemRef> privateKey;
-
-  try {
-    privateKey = m_impl->getKey(keyName);
-  }
-  catch (const std::domain_error&) {
-    BOOST_THROW_EXCEPTION(Tpm::Error("Private key does not exist in OSX Keychain"));
+  CFReleaser<SecKeychainItemRef> keychainItem = getKey(keyName);
+  if (keychainItem == nullptr) {
+    BOOST_THROW_EXCEPTION(Error("Private key does not exist in macOS Keychain"));
   }
 
   CFReleaser<CFDataRef> exportedKey;
   SecItemImportExportKeyParameters keyParams;
   memset(&keyParams, 0, sizeof(keyParams));
-  CFReleaser<CFStringRef> passphrase =
-    CFStringCreateWithBytes(0, reinterpret_cast<const uint8_t*>(pw), pwLen, kCFStringEncodingUTF8, false);
+  auto passphrase = cfstring::fromBuffer(reinterpret_cast<const uint8_t*>(pw), pwLen);
   keyParams.passphrase = passphrase.get();
-  OSStatus res = SecItemExport(privateKey.get(),       // secItemOrArray
+
+  OSStatus res = SecItemExport(keychainItem.get(),     // secItemOrArray
                                kSecFormatWrappedPKCS8, // outputFormat
                                0,                      // flags
                                &keyParams,             // keyParams
@@ -466,20 +450,17 @@ void
 BackEndOsx::doImportKey(const Name& keyName, const uint8_t* buf, size_t size,
                         const char* pw, size_t pwLen)
 {
-  CFReleaser<CFDataRef> importedKey = CFDataCreateWithBytesNoCopy(nullptr, buf, size, kCFAllocatorNull);
+  auto importedKey = makeCFDataNoCopy(buf, size);
 
   SecExternalFormat externalFormat = kSecFormatWrappedPKCS8;
   SecExternalItemType externalType = kSecItemTypePrivateKey;
 
-  CFReleaser<CFStringRef> keyLabel = CFStringCreateWithCString(nullptr, keyName.toUri().c_str(),
-                                                               kCFStringEncodingUTF8);
-  CFReleaser<CFStringRef> passphrase =
-    CFStringCreateWithBytes(nullptr, reinterpret_cast<const uint8_t*>(pw), pwLen, kCFStringEncodingUTF8, false);
+  auto passphrase = cfstring::fromBuffer(reinterpret_cast<const uint8_t*>(pw), pwLen);
+  auto keyLabel = cfstring::fromStdString(keyName.toUri());
   CFReleaser<SecAccessRef> access;
   SecAccessCreate(keyLabel.get(), nullptr, &access.get());
 
   CFArrayRef attributes = nullptr;
-
   const SecItemImportExportKeyParameters keyParams{
     SEC_KEY_IMPORT_EXPORT_PARAMS_VERSION, // version
     0, // flags
@@ -522,7 +503,7 @@ BackEndOsx::doImportKey(const Name& keyName, const uint8_t* buf, size_t size,
     attrList.count++;
   }
 
-  res = SecKeychainItemModifyAttributesAndData(privateKey, &attrList, 0, nullptr);
+  SecKeychainItemModifyAttributesAndData(privateKey, &attrList, 0, nullptr);
 }
 
 } // namespace tpm

src/security/tpm/back-end-osx.hpp

@@ -23,12 +23,13 @@
 #define NDN_SECURITY_TPM_BACK_END_OSX_HPP
 
 #include "back-end.hpp"
-#include "key-ref-osx.hpp"
 
 #ifndef NDN_CXX_HAVE_OSX_FRAMEWORKS
 #error "This file should not be compiled ..."
 #endif
 
+#include "key-ref-osx.hpp"
+
 namespace ndn {
 namespace security {
 namespace tpm {
@@ -36,17 +37,18 @@ namespace tpm {
 /**
  * @brief The back-end implementation of TPM based on macOS Keychain Services.
  */
-class BackEndOsx : public BackEnd
+class BackEndOsx final : public BackEnd
 {
 public:
   /**
-   * @brief Create TPM backed based on macOS KeyChain service
-   * @param location Not used (required by the TPM-registration interface)
+   * @brief Create TPM backed based on macOS Keychain Services.
+   *
+   * @param location Not used (required by the TPM registration interface).
    */
   explicit
   BackEndOsx(const std::string& location = "");
 
-  ~BackEndOsx() override;
+  ~BackEndOsx() final;
 
   static const std::string&
   getScheme();
@@ -78,54 +80,21 @@ public: // crypto transformation
   derivePublicKey(const KeyRefOsx& key);
 
 private: // inherited from tpm::BackEnd
-  /**
-   * @return True if a key with name @p keyName exists in TPM.
-   */
   bool
   doHasKey(const Name& keyName) const final;
 
-  /**
-   * @return The handle of a key with name @p keyName, or nullptr if the key does not exist
-   */
   unique_ptr<KeyHandle>
   doGetKeyHandle(const Name& keyName) const final;
 
-  /**
-   * @brief Create key for @p identityName according to @p params.
-   *
-   * The created key is named as: /<identityName>/[keyId]/KEY
-   * The key name is set in the returned KeyHandle.
-   *
-   * @return The handle of the created key.
-   */
   unique_ptr<KeyHandle>
   doCreateKey(const Name& identityName, const KeyParams& params) final;
 
-  /**
-   * @brief Delete a key with name @p keyName.
-   *
-   * @throw Error the deletion failed
-   */
   void
   doDeleteKey(const Name& keyName) final;
 
-  /**
-   * @return A private key with name @p keyName in encrypted PKCS #8 format using password @p pw
-   * @throw Error the key cannot be exported, e.g., not enough privilege
-   */
   ConstBufferPtr
   doExportKey(const Name& keyName, const char* pw, size_t pwLen) final;
 
-  /**
-   * @brief Import a private key in encrypted PKCS #8 format
-   *
-   * @param keyName The name of imported private key
-   * @param buf Pointer to the key in encrypted PKCS #8 format
-   * @param size The size of the key in encrypted PKCS #8 format
-   * @param pw The password to decrypt the private key
-   * @param pwLen The length of the password
-   * @throw Error import fails
-   */
   void
   doImportKey(const Name& keyName, const uint8_t* buf, size_t size, const char* pw, size_t pwLen) final;
 

src/security/tpm/back-end.hpp

@@ -22,10 +22,9 @@
 #ifndef NDN_SECURITY_TPM_BACK_END_HPP
 #define NDN_SECURITY_TPM_BACK_END_HPP
 
-#include "../../common.hpp"
-#include "../../name.hpp"
-#include "../../encoding/buffer.hpp"
 #include "../key-params.hpp"
+#include "../../encoding/buffer.hpp"
+#include "../../name.hpp"
 
 namespace ndn {
 namespace security {
@@ -34,7 +33,7 @@ namespace tpm {
 class KeyHandle;
 
 /**
- * @brief Abstraction of Tpm back-end.
+ * @brief Abstract interface for a TPM backend implementation.
  *
  * This class provides KeyHandle to the front-end and other TPM management operations.
  */
@@ -53,157 +52,137 @@ public:
 
 public: // key management
   /**
-   * @return True if a key with name @p keyName exists in TPM.
+   * @brief Check if the key with name @p keyName exists in the TPM.
+   *
+   * @return True if the key exists, false otherwise.
    */
   bool
   hasKey(const Name& keyName) const;
 
   /**
-   * @return The handle of a key with name @p keyName, or nullptr if the key does not exist.
+   * @brief Get the handle of the key with name @p keyName.
    *
-   * Calling getKeyHandle multiple times with the same keyName will return different KeyHandle
+   * Calling this function multiple times with the same @p keyName will return different KeyHandle
    * objects that all refer to the same key.
+   *
+   * @return The handle of the key, or nullptr if the key does not exist.
    */
   unique_ptr<KeyHandle>
   getKeyHandle(const Name& keyName) const;
 
   /**
-   * @brief Create key for @p identity according to @p params.
-   *
-   * The key name is set in the returned KeyHandle.
+   * @brief Create a key for @p identityName according to @p params.
    *
    * @return The handle of the created key.
-   * @throw Tpm::Error @p params are invalid
-   * @throw Error the key cannot be created
+   * @throw Tpm::Error @p params are invalid.
+   * @throw Error The key could not be created.
    */
   unique_ptr<KeyHandle>
-  createKey(const Name& identity, const KeyParams& params);
+  createKey(const Name& identityName, const KeyParams& params);
 
   /**
-   * @brief Delete a key with name @p keyName.
+   * @brief Delete the key with name @p keyName.
    *
-   * Continuing to use existing KeyHandles on a deleted key results in undefined behavior.
+   * @warning Continuing to use existing KeyHandle objects for a deleted key
+   *          results in undefined behavior.
    *
-   * @throw Error if the deletion fails.
+   * @throw Error The key could not be deleted.
    */
   void
   deleteKey(const Name& keyName);
 
   /**
-   * @return A private key with name @p keyName in encrypted PKCS #8 format using password @p pw
-   * @throw Error the key does not exist
-   * @throw Error the key cannot be exported, e.g., insufficient privilege
+   * @brief Get the private key with name @p keyName in encrypted PKCS #8 format.
+   *
+   * @param keyName The name of the key.
+   * @param pw The password to encrypt the private key.
+   * @param pwLen The length of the password.
+   *
+   * @return The encoded private key.
+   * @throw Error The key does not exist or cannot be exported.
    */
   ConstBufferPtr
   exportKey(const Name& keyName, const char* pw, size_t pwLen);
 
   /**
-   * @brief Import a private key in encrypted PKCS #8 format
+   * @brief Import a private key in encrypted PKCS #8 format.
    *
-   * @param keyName The name of imported private key
-   * @param pkcs8 Pointer to the key in encrypted PKCS #8 format
-   * @param pkcs8Len The size of the key in encrypted PKCS #8 format
-   * @param pw The password to decrypt the private key
-   * @param pwLen The length of the password
-   * @throw Error import failed
+   * @param keyName The name of the key to use in the TPM.
+   * @param pkcs8 Pointer to the key in encrypted PKCS #8 format.
+   * @param pkcs8Len The size of the key in encrypted PKCS #8 format.
+   * @param pw The password to decrypt the private key.
+   * @param pwLen The length of the password.
+   *
+   * @throw Error The key could not be imported.
    */
   void
   importKey(const Name& keyName, const uint8_t* pkcs8, size_t pkcs8Len, const char* pw, size_t pwLen);
 
   /**
-   * @brief Check if TPM is in terminal mode
+   * @brief Check if the TPM is in terminal mode.
    *
-   * Default implementation always returns true.
+   * The default implementation always returns true.
+   *
+   * @return True if in terminal mode, false otherwise.
    */
   virtual bool
   isTerminalMode() const;
 
   /**
-   * @brief Set the terminal mode of TPM.
+   * @brief Set the terminal mode of the TPM.
    *
-   * In terminal mode, TPM will not ask user permission from GUI.
-   *
-   * Default implementation does nothing.
+   * In terminal mode, the TPM will not ask for a password from the GUI.
+   * The default implementation does nothing.
    */
   virtual void
   setTerminalMode(bool isTerminal) const;
 
   /**
-   * @return True if TPM is locked, otherwise false
+   * @brief Check if the TPM is locked.
    *
-   * Default implementation always returns false.
+   * The default implementation always returns false.
+   *
+   * @return True if locked, false otherwise.
    */
   virtual bool
   isTpmLocked() const;
 
   /**
-   * @brief Unlock TPM
+   * @brief Unlock the TPM.
    *
-   * @param pw    The password to unlock TPM
-   * @param pwLen The password size.
+   * The default implementation does nothing and returns `!isTpmLocked()`.
    *
-   * Default implementation always returns !isTpmLocked()
+   * @param pw The password to unlock the TPM.
+   * @param pwLen The length of the password.
+   *
+   * @return True if the TPM was unlocked.
    */
   virtual bool
   unlockTpm(const char* pw, size_t pwLen) const;
 
-protected: // static helper method
+protected: // static helper methods
   /**
-   * @brief Set the key name in @p keyHandle according to @p identity and @p params
+   * @brief Set the key name in @p keyHandle according to @p identity and @p params.
    */
   static void
   setKeyName(KeyHandle& keyHandle, const Name& identity, const KeyParams& params);
 
 private: // pure virtual methods
-  /**
-   * @return True if a key with name @p keyName exists in TPM.
-   */
   virtual bool
   doHasKey(const Name& keyName) const = 0;
 
-  /**
-   * @return The handle of a key with name @p keyName, or nullptr if the key does not exist
-   */
   virtual unique_ptr<KeyHandle>
   doGetKeyHandle(const Name& keyName) const = 0;
 
-  /**
-   * @brief Create key for @p identityName according to @p params.
-   *
-   * The created key is named as: /<identityName>/[keyId]/KEY
-   * The key name is set in the returned KeyHandle.
-   *
-   * @return The handle of the created key.
-   * @throw Error key cannot be created
-   */
   virtual unique_ptr<KeyHandle>
   doCreateKey(const Name& identity, const KeyParams& params) = 0;
 
-  /**
-   * @brief Delete a key with name @p keyName.
-   *
-   * @throw Error the deletion failed
-   */
   virtual void
   doDeleteKey(const Name& keyName) = 0;
 
-  /**
-   * @return A private key with name @p keyName in encrypted PKCS #8 format using password @p pw
-   * @throw Error the key cannot be exported, e.g., insufficient privilege
-   */
   virtual ConstBufferPtr
   doExportKey(const Name& keyName, const char* pw, size_t pwLen) = 0;
 
-  /**
-   * @brief Import a private key in encrypted PKCS #8 format using @p password
-   *
-   * @param keyName The name of imported private key
-   * @param pkcs8 Pointer to the key in PKCS #8 format
-   * @param pkcs8Len The size of the key in PKCS #8 format
-   * @param pw The password to decrypt the private key
-   * @param pwLen The length of the password
-   * @throw Error import failed
-   */
   virtual void
   doImportKey(const Name& keyName, const uint8_t* pkcs8, size_t pkcs8Len, const char* pw, size_t pwLen) = 0;
 };

src/security/tpm/key-handle-osx.hpp

@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2013-2017 Regents of the University of California.
+ * Copyright (c) 2013-2018 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
  *
@@ -23,12 +23,13 @@
 #define NDN_SECURITY_TPM_KEY_HANDLE_OSX_HPP
 
 #include "key-handle.hpp"
-#include "key-ref-osx.hpp"
 
 #ifndef NDN_CXX_HAVE_OSX_FRAMEWORKS
 #error "This file should not be compiled ..."
 #endif
 
+#include "key-ref-osx.hpp"
+
 namespace ndn {
 namespace security {
 namespace tpm {

src/security/tpm/key-ref-osx.hpp

@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
-/**
- * Copyright (c) 2013-2017 Regents of the University of California.
+/*
+ * Copyright (c) 2013-2018 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
  *
@@ -22,6 +22,12 @@
 #ifndef NDN_SECURITY_TPM_KEY_REF_OSX_HPP
 #define NDN_SECURITY_TPM_KEY_REF_OSX_HPP
 
+#include "../../common.hpp"
+
+#ifndef NDN_CXX_HAVE_OSX_FRAMEWORKS
+#error "This file should not be compiled ..."
+#endif
+
 #include "../../util/cf-releaser-osx.hpp"
 #include <Security/Security.h>
 

src/security/v2/key-chain.cpp

@@ -89,14 +89,14 @@ KeyChain::getTpmFactories()
 const std::string&
 KeyChain::getDefaultPibScheme()
 {
-  return pib::PibSqlite3::getScheme();;
+  return pib::PibSqlite3::getScheme();
 }
 
 const std::string&
 KeyChain::getDefaultTpmScheme()
 {
 #if defined(NDN_CXX_HAVE_OSX_FRAMEWORKS) && defined(NDN_CXX_WITH_OSX_KEYCHAIN)
-  return tpm::BackEndOsx::getScheme();;
+  return tpm::BackEndOsx::getScheme();
 #else
   return tpm::BackEndFile::getScheme();
 #endif // defined(NDN_CXX_HAVE_OSX_FRAMEWORKS) && defined(NDN_CXX_WITH_OSX_KEYCHAIN)

src/util/cf-string-osx.cpp

@@ -0,0 +1,71 @@
+/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
+/*
+ * Copyright (c) 2013-2018 Regents of the University of California.
+ *
+ * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
+ *
+ * ndn-cxx library is free software: you can redistribute it and/or modify it under the
+ * terms of the GNU Lesser General Public License as published by the Free Software
+ * Foundation, either version 3 of the License, or (at your option) any later version.
+ *
+ * ndn-cxx library is distributed in the hope that it will be useful, but WITHOUT ANY
+ * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A
+ * PARTICULAR PURPOSE.  See the GNU Lesser General Public License for more details.
+ *
+ * You should have received copies of the GNU General Public License and GNU Lesser
+ * General Public License along with ndn-cxx, e.g., in COPYING.md file.  If not, see
+ * <http://www.gnu.org/licenses/>.
+ *
+ * See AUTHORS.md for complete list of ndn-cxx authors and contributors.
+ */
+
+#include "cf-string-osx.hpp"
+
+namespace ndn {
+namespace util {
+namespace cfstring {
+
+CFReleaser<CFStringRef>
+fromBuffer(const uint8_t* buf, size_t buflen)
+{
+  CFStringRef cfStr = CFStringCreateWithBytes(kCFAllocatorDefault, buf, buflen, kCFStringEncodingUTF8, false);
+  if (cfStr == nullptr) {
+    BOOST_THROW_EXCEPTION(std::runtime_error("Failed to create CFString from buffer"));
+  }
+  return cfStr;
+}
+
+CFReleaser<CFStringRef>
+fromStdString(const std::string& str)
+{
+  CFStringRef cfStr = CFStringCreateWithCString(kCFAllocatorDefault, str.data(), kCFStringEncodingUTF8);
+  if (cfStr == nullptr) {
+    BOOST_THROW_EXCEPTION(std::runtime_error("Failed to create CFString from std::string"));
+  }
+  return cfStr;
+}
+
+std::string
+toStdString(CFStringRef cfStr)
+{
+  const char* cStr = CFStringGetCStringPtr(cfStr, kCFStringEncodingUTF8);
+  if (cStr != nullptr) {
+    // fast path
+    return cStr;
+  }
+
+  // reserve space for the string + null terminator
+  std::string str(CFStringGetLength(cfStr) + 1, '\0');
+  // copy the CFString into the std::string buffer
+  if (!CFStringGetCString(cfStr, &str.front(), str.size(), kCFStringEncodingUTF8)) {
+    BOOST_THROW_EXCEPTION(std::runtime_error("CFString to std::string conversion failed"));
+  }
+  // drop the null terminator, std::string doesn't need it
+  str.pop_back();
+
+  return str;
+}
+
+} // namespace cfstring
+} // namespace util
+} // namespace ndn

src/util/cf-string-osx.hpp

@@ -0,0 +1,68 @@
+/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
+/*
+ * Copyright (c) 2013-2018 Regents of the University of California.
+ *
+ * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
+ *
+ * ndn-cxx library is free software: you can redistribute it and/or modify it under the
+ * terms of the GNU Lesser General Public License as published by the Free Software
+ * Foundation, either version 3 of the License, or (at your option) any later version.
+ *
+ * ndn-cxx library is distributed in the hope that it will be useful, but WITHOUT ANY
+ * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A
+ * PARTICULAR PURPOSE.  See the GNU Lesser General Public License for more details.
+ *
+ * You should have received copies of the GNU General Public License and GNU Lesser
+ * General Public License along with ndn-cxx, e.g., in COPYING.md file.  If not, see
+ * <http://www.gnu.org/licenses/>.
+ *
+ * See AUTHORS.md for complete list of ndn-cxx authors and contributors.
+ */
+
+#ifndef NDN_UTIL_CF_STRING_OSX_HPP
+#define NDN_UTIL_CF_STRING_OSX_HPP
+
+#include "../common.hpp"
+
+#ifndef NDN_CXX_HAVE_OSX_FRAMEWORKS
+#error "This file should not be included ..."
+#endif
+
+#include "cf-releaser-osx.hpp"
+
+/**
+ * @file
+ *
+ * This file contains utilities to deal with Apple Core Foundation's CFString and related types.
+ */
+
+namespace ndn {
+namespace util {
+namespace cfstring {
+
+/**
+ * @brief Create a CFString by copying bytes from a raw buffer
+ * @throw std::runtime_error creation failed
+ */
+CFReleaser<CFStringRef>
+fromBuffer(const uint8_t* buf, size_t buflen);
+
+/**
+ * @brief Create a CFString by copying characters from a std::string
+ * @throw std::runtime_error creation failed
+ */
+CFReleaser<CFStringRef>
+fromStdString(const std::string& str);
+
+/**
+ * @brief Convert a CFString to a std::string
+ * @throw std::runtime_error conversion failed
+ */
+std::string
+toStdString(CFStringRef cfStr);
+
+} // namespace cfstring
+} // namespace util
+} // namespace ndn
+
+#endif // NDN_UTIL_CF_STRING_OSX_HPP

tests/unit-tests/security/tpm/back-end-wrapper-file.hpp

@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
-/**
- * Copyright (c) 2013-2017 Regents of the University of California.
+/*
+ * Copyright (c) 2013-2018 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
  *
@@ -54,7 +54,7 @@ public:
   }
 
   std::string
-  getScheme()
+  getScheme() const
   {
     return "tpm-file";
   }

tests/unit-tests/security/tpm/back-end-wrapper-mem.hpp

@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
-/**
- * Copyright (c) 2013-2017 Regents of the University of California.
+/*
+ * Copyright (c) 2013-2018 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
  *
@@ -47,7 +47,7 @@ public:
   }
 
   std::string
-  getScheme()
+  getScheme() const
   {
     return "tpm-memory";
   }

tests/unit-tests/security/tpm/back-end-wrapper-osx.hpp

@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
-/**
- * Copyright (c) 2013-2016 Regents of the University of California.
+/*
+ * Copyright (c) 2013-2018 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
  *
@@ -24,7 +24,8 @@
 
 #include "security/tpm/back-end-osx.hpp"
 #include "security/tpm/key-handle-osx.hpp"
-#include <Availability.h>
+
+#include <cstdlib>
 
 namespace ndn {
 namespace security {
@@ -47,17 +48,17 @@ public:
       m_HOME = std::getenv("HOME");
 
     if (!oldHOME.empty())
-      setenv("HOME", oldHOME.c_str(), 1);
+      setenv("HOME", oldHOME.data(), 1);
     else
       unsetenv("HOME");
 
-    m_impl = unique_ptr<BackEnd>(new BackEndOsx);
+    m_impl = make_unique<BackEndOsx>();
   }
 
   ~BackEndWrapperOsx()
   {
     if (!m_HOME.empty())
-      setenv("HOME", m_HOME.c_str(), 1);
+      setenv("HOME", m_HOME.data(), 1);
     else
       unsetenv("HOME");
   }
@@ -69,7 +70,7 @@ public:
   }
 
   std::string
-  getScheme()
+  getScheme() const
   {
     return "tpm-osxkeychain";
   }