security: fix error handling in KeyChain::importSafeBag()
Tpm::importPrivateKey() now throws on error instead of
returning false, for consistency with similar functions.
Change-Id: Id07c2be3809e32d1779c0b5977232e4728528e3c
Refs: #4359
diff --git a/src/security/tpm/tpm.cpp b/src/security/tpm/tpm.cpp
index 0ed1062..d72440b 100644
--- a/src/security/tpm/tpm.cpp
+++ b/src/security/tpm/tpm.cpp
@@ -1,5 +1,5 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
-/**
+/*
* Copyright (c) 2013-2017 Regents of the University of California.
*
* This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
@@ -138,36 +138,27 @@
return m_backEnd->exportKey(keyName, pw, pwLen);
}
-bool
+void
Tpm::importPrivateKey(const Name& keyName, const uint8_t* pkcs8, size_t pkcs8Len,
const char* pw, size_t pwLen)
{
- try {
- m_backEnd->importKey(keyName, pkcs8, pkcs8Len, pw, pwLen);
- }
- catch (const BackEnd::Error&) {
- return false;
- }
- return true;
+ m_backEnd->importKey(keyName, pkcs8, pkcs8Len, pw, pwLen);
}
const KeyHandle*
Tpm::findKey(const Name& keyName) const
{
auto it = m_keys.find(keyName);
-
if (it != m_keys.end())
return it->second.get();
- unique_ptr<KeyHandle> handle = m_backEnd->getKeyHandle(keyName);
+ auto handle = m_backEnd->getKeyHandle(keyName);
+ if (handle == nullptr)
+ return nullptr;
- if (handle != nullptr) {
- KeyHandle* key = handle.get();
- m_keys[keyName] = std::move(handle);
- return key;
- }
-
- return nullptr;
+ const KeyHandle* key = handle.get();
+ m_keys[keyName] = std::move(handle);
+ return key;
}
} // namespace tpm
diff --git a/src/security/tpm/tpm.hpp b/src/security/tpm/tpm.hpp
index dde9d14..4ce57f2 100644
--- a/src/security/tpm/tpm.hpp
+++ b/src/security/tpm/tpm.hpp
@@ -1,5 +1,5 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
-/**
+/*
* Copyright (c) 2013-2017 Regents of the University of California.
*
* This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
@@ -22,10 +22,9 @@
#ifndef NDN_SECURITY_TPM_TPM_HPP
#define NDN_SECURITY_TPM_TPM_HPP
-#include "../security-common.hpp"
-#include "../../name.hpp"
-#include "../key-params.hpp"
#include "key-handle.hpp"
+#include "../key-params.hpp"
+#include "../../name.hpp"
#include <unordered_map>
@@ -180,7 +179,7 @@
* @param pw The password to encrypt the private key
* @param pwLen The length of the password
* @return The encoded private key wrapper.
- * @throw BackEnd::Error the key does not exist or it cannot be exported.
+ * @throw BackEnd::Error The key does not exist or it could not be exported.
*/
ConstBufferPtr
exportPrivateKey(const Name& keyName, const char* pw, size_t pwLen) const;
@@ -193,16 +192,16 @@
* @param pkcs8Len The length of the private key wrapper
* @param pw The password to encrypt the private key
* @param pwLen The length of the password
- * @return true if the operation is successful, false otherwise.
+ * @throw BackEnd::Error The key could not be imported.
*/
- bool
+ void
importPrivateKey(const Name& keyName, const uint8_t* pkcs8, size_t pkcs8Len,
const char* pw, size_t pwLen);
/**
* @brief Clear the key cache.
*
- * An empty cache can force Tpm to do key lookup in back-end.
+ * An empty cache can force Tpm to do key lookup in the back-end.
*/
void
clearKeyCache()
diff --git a/src/security/v2/key-chain.cpp b/src/security/v2/key-chain.cpp
index 2d0820d..9f496a1 100644
--- a/src/security/v2/key-chain.cpp
+++ b/src/security/v2/key-chain.cpp
@@ -51,17 +51,11 @@
// Therefore, the following standard PIB and TPMs need to be registered here.
// http://stackoverflow.com/q/9459980/2150331
-/////////
-// PIB //
-/////////
namespace pib {
NDN_CXX_V2_KEYCHAIN_REGISTER_PIB_BACKEND(PibSqlite3);
NDN_CXX_V2_KEYCHAIN_REGISTER_PIB_BACKEND(PibMemory);
} // namespace pib
-/////////
-// TPM //
-/////////
namespace tpm {
#if defined(NDN_CXX_HAVE_OSX_FRAMEWORKS) && defined(NDN_CXX_WITH_OSX_KEYCHAIN)
NDN_CXX_V2_KEYCHAIN_REGISTER_TPM_BACKEND(BackEndOsx);
@@ -355,8 +349,8 @@
try {
encryptedKey = m_tpm->exportPrivateKey(keyName, pw, pwLen);
}
- catch (const tpm::BackEnd::Error&) {
- BOOST_THROW_EXCEPTION(Error("Private `" + keyName.toUri() + "` key does not exist"));
+ catch (const tpm::BackEnd::Error& e) {
+ BOOST_THROW_EXCEPTION(Error("Failed to export private key `" + keyName.toUri() + "`: " + e.what()));
}
return make_shared<SafeBag>(certificate, *encryptedKey);
@@ -389,8 +383,8 @@
safeBag.getEncryptedKeyBag().data(), safeBag.getEncryptedKeyBag().size(),
pw, pwLen);
}
- catch (const std::runtime_error&) {
- BOOST_THROW_EXCEPTION(Error("Fail to import private key `" + keyName.toUri() + "`"));
+ catch (const tpm::BackEnd::Error& e) {
+ BOOST_THROW_EXCEPTION(Error("Failed to import private key `" + keyName.toUri() + "`: " + e.what()));
}
// check the consistency of private key and certificate
@@ -423,7 +417,6 @@
key.addCertificate(cert);
}
-
// public: signing
void
@@ -662,7 +655,7 @@
}
catch (const Pib::Error&) {
BOOST_THROW_EXCEPTION(InvalidSigningInfoError("Signing identity `" + identity.getName().toUri() +
- "` does not have default certificate"));
+ "` does not have a default certificate"));
}
}
diff --git a/tests/unit-tests/security/tpm/back-end.t.cpp b/tests/unit-tests/security/tpm/back-end.t.cpp
index ad8e39c..f118099 100644
--- a/tests/unit-tests/security/tpm/back-end.t.cpp
+++ b/tests/unit-tests/security/tpm/back-end.t.cpp
@@ -221,6 +221,8 @@
tpm.importKey(keyName, privateKeyBuffer->data(), privateKeyBuffer->size(), password.c_str(), password.size());
BOOST_CHECK_EQUAL(tpm.hasKey(keyName), true);
+ BOOST_CHECK_THROW(tpm.importKey(keyName, privateKeyBuffer->data(), privateKeyBuffer->size(), password.c_str(), password.size()),
+ BackEnd::Error);
ConstBufferPtr exportedKey = tpm.exportKey(keyName, password.c_str(), password.size());
BOOST_CHECK_EQUAL(tpm.hasKey(keyName), true);
@@ -236,6 +238,7 @@
tpm.deleteKey(keyName);
BOOST_CHECK_EQUAL(tpm.hasKey(keyName), false);
+ BOOST_CHECK_THROW(tpm.exportKey(keyName, password.c_str(), password.size()), BackEnd::Error);
}
BOOST_AUTO_TEST_CASE(RandomKeyId)
diff --git a/tests/unit-tests/security/v2/key-chain.t.cpp b/tests/unit-tests/security/v2/key-chain.t.cpp
index a874464..9575754 100644
--- a/tests/unit-tests/security/v2/key-chain.t.cpp
+++ b/tests/unit-tests/security/v2/key-chain.t.cpp
@@ -1,5 +1,5 @@
/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
-/**
+/*
* Copyright (c) 2013-2017 Regents of the University of California.
*
* This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
@@ -365,6 +365,7 @@
Block block = exported->wireEncode();
m_keyChain.deleteIdentity(id);
+ BOOST_CHECK_THROW(m_keyChain.exportSafeBag(cert, "1234", 4), KeyChain::Error);
BOOST_CHECK_EQUAL(m_keyChain.getTpm().hasKey(cert.getKeyName()), false);
BOOST_CHECK_EQUAL(m_keyChain.getPib().getIdentities().size(), 0);
@@ -372,6 +373,7 @@
SafeBag imported;
imported.wireDecode(block);
m_keyChain.importSafeBag(imported, "1234", 4);
+ BOOST_CHECK_THROW(m_keyChain.importSafeBag(imported, "1234", 4), KeyChain::Error);
BOOST_CHECK_EQUAL(m_keyChain.getTpm().hasKey(cert.getKeyName()), true);
BOOST_CHECK_EQUAL(m_keyChain.getPib().getIdentities().size(), 1);