security: in tpm::BackEndOsx, unwrap the key before importing it
We use our own openssl-based routines for the PKCS#8 decoding/decryption,
as they support a much wider range of algorithms than macOS's Keychain.
Change-Id: Ia841e7681a47563c696d054d46fe523f427c99cb
Refs: #4450
diff --git a/.travis.yml b/.travis.yml
index 1c238d3..6868f51 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -58,17 +58,16 @@
osx_image: xcode9.4
env: OSX_VERSION=10.13
- os: osx
- osx_image: xcode9.4
- env: OSX_VERSION=10.13 USE_OPENSSL_1_1=yes
- - os: osx
osx_image: xcode10
env: OSX_VERSION=10.13
+ - os: osx
+ osx_image: xcode10
+ env: OSX_VERSION=10.13 USE_OPENSSL_1_1=yes
allow_failures:
- env: COMPILER=clang++-3.6
- env: COMPILER=clang++-3.7 DISABLE_ASAN=yes
- env: COMPILER=clang++-8
- - env: OSX_VERSION=10.13 USE_OPENSSL_1_1=yes
fast_finish: true
diff --git a/src/security/tpm/back-end-osx.cpp b/src/security/tpm/back-end-osx.cpp
index 15bd41a..daf5281 100644
--- a/src/security/tpm/back-end-osx.cpp
+++ b/src/security/tpm/back-end-osx.cpp
@@ -23,6 +23,7 @@
#include "key-handle-osx.hpp"
#include "tpm.hpp"
#include "../transform/private-key.hpp"
+#include "../../encoding/buffer-stream.hpp"
#include "../../util/cf-string-osx.hpp"
#include <Security/Security.h>
@@ -150,6 +151,36 @@
}
}
+/**
+ * @brief Export a private key from the Keychain to @p outKey
+ */
+static void
+exportItem(const KeyRefOsx& keyRef, transform::PrivateKey& outKey)
+{
+ // use a temporary password for PKCS8 encoding
+ const char pw[] = "correct horse battery staple";
+ auto passphrase = cfstring::fromBuffer(reinterpret_cast<const uint8_t*>(pw), std::strlen(pw));
+
+ SecItemImportExportKeyParameters keyParams;
+ std::memset(&keyParams, 0, sizeof(keyParams));
+ keyParams.version = SEC_KEY_IMPORT_EXPORT_PARAMS_VERSION;
+ keyParams.passphrase = passphrase.get();
+
+ CFReleaser<CFDataRef> exportedKey;
+ OSStatus res = SecItemExport(keyRef.get(), // secItemOrArray
+ kSecFormatWrappedPKCS8, // outputFormat
+ 0, // flags
+ &keyParams, // keyParams
+ &exportedKey.get()); // exportedData
+
+ if (res != errSecSuccess) {
+ BOOST_THROW_EXCEPTION(BackEnd::Error("Failed to export private key: "s + getErrorMessage(res)));
+ }
+
+ outKey.loadPkcs8(CFDataGetBytePtr(exportedKey.get()), CFDataGetLength(exportedKey.get()),
+ pw, std::strlen(pw));
+}
+
BackEndOsx::BackEndOsx(const std::string&)
: m_impl(make_unique<Impl>())
{
@@ -297,19 +328,8 @@
ConstBufferPtr
BackEndOsx::derivePublicKey(const KeyRefOsx& key)
{
- CFReleaser<CFDataRef> exportedKey;
- OSStatus res = SecItemExport(key.get(), // secItemOrArray
- kSecFormatOpenSSL, // outputFormat
- 0, // flags
- nullptr, // keyParams
- &exportedKey.get()); // exportedData
-
- if (res != errSecSuccess) {
- BOOST_THROW_EXCEPTION(Error("Failed to export private key: "s + getErrorMessage(res)));
- }
-
transform::PrivateKey privateKey;
- privateKey.loadPkcs1(CFDataGetBytePtr(exportedKey.get()), CFDataGetLength(exportedKey.get()));
+ exportItem(key, privateKey);
return privateKey.derivePublicKey();
}
@@ -409,73 +429,78 @@
BOOST_THROW_EXCEPTION(Error("Failed to export private key: " + getErrorMessage(errSecItemNotFound)));
}
- auto passphrase = cfstring::fromBuffer(reinterpret_cast<const uint8_t*>(pw), pwLen);
- SecItemImportExportKeyParameters keyParams;
- std::memset(&keyParams, 0, sizeof(keyParams));
- keyParams.version = SEC_KEY_IMPORT_EXPORT_PARAMS_VERSION;
- keyParams.passphrase = passphrase.get();
-
- CFReleaser<CFDataRef> exportedKey;
- OSStatus res = SecItemExport(keychainItem.get(), // secItemOrArray
- kSecFormatWrappedPKCS8, // outputFormat
- 0, // flags
- &keyParams, // keyParams
- &exportedKey.get()); // exportedData
-
- if (res != errSecSuccess) {
- BOOST_THROW_EXCEPTION(Error("Failed to export private key: " + getErrorMessage(res)));
+ transform::PrivateKey exportedKey;
+ OBufferStream pkcs8;
+ try {
+ exportItem(keychainItem, exportedKey);
+ exportedKey.savePkcs8(pkcs8, pw, pwLen);
}
-
- return make_shared<Buffer>(CFDataGetBytePtr(exportedKey.get()), CFDataGetLength(exportedKey.get()));
+ catch (const transform::PrivateKey::Error& e) {
+ BOOST_THROW_EXCEPTION(Error("Failed to export private key: "s + e.what()));
+ }
+ return pkcs8.buf();
}
void
BackEndOsx::doImportKey(const Name& keyName, const uint8_t* buf, size_t size,
const char* pw, size_t pwLen)
{
- auto importedKey = makeCFDataNoCopy(buf, size);
+ transform::PrivateKey privKey;
+ OBufferStream pkcs1;
+ try {
+ // do the PKCS8 decoding ourselves, see bug #4450
+ privKey.loadPkcs8(buf, size, pw, pwLen);
+ privKey.savePkcs1(pkcs1);
+ }
+ catch (const transform::PrivateKey::Error& e) {
+ BOOST_THROW_EXCEPTION(Error("Failed to import private key: "s + e.what()));
+ }
+ auto keyToImport = makeCFDataNoCopy(pkcs1.buf()->data(), pkcs1.buf()->size());
- SecExternalFormat externalFormat = kSecFormatWrappedPKCS8;
+ SecExternalFormat externalFormat = kSecFormatOpenSSL;
SecExternalItemType externalType = kSecItemTypePrivateKey;
- auto passphrase = cfstring::fromBuffer(reinterpret_cast<const uint8_t*>(pw), pwLen);
- auto keyLabel = cfstring::fromStdString(keyName.toUri());
+ auto keyUri = keyName.toUri();
+ auto keyLabel = cfstring::fromStdString(keyUri);
CFReleaser<SecAccessRef> access;
- SecAccessCreate(keyLabel.get(), nullptr, &access.get());
+ OSStatus res = SecAccessCreate(keyLabel.get(), // descriptor
+ nullptr, // trustedlist (null == trust only the calling app)
+ &access.get()); // accessRef
+
+ if (res != errSecSuccess) {
+ BOOST_THROW_EXCEPTION(Error("Failed to import private key: " + getErrorMessage(res)));
+ }
SecItemImportExportKeyParameters keyParams;
std::memset(&keyParams, 0, sizeof(keyParams));
keyParams.version = SEC_KEY_IMPORT_EXPORT_PARAMS_VERSION;
- keyParams.passphrase = passphrase.get();
keyParams.accessRef = access.get();
CFReleaser<CFArrayRef> outItems;
- OSStatus res = SecItemImport(importedKey.get(), // importedData
- nullptr, // fileNameOrExtension
- &externalFormat, // inputFormat
- &externalType, // itemType
- 0, // flags
- &keyParams, // keyParams
- m_impl->keyChainRef, // importKeychain
- &outItems.get()); // outItems
+ res = SecItemImport(keyToImport.get(), // importedData
+ nullptr, // fileNameOrExtension
+ &externalFormat, // inputFormat
+ &externalType, // itemType
+ 0, // flags
+ &keyParams, // keyParams
+ m_impl->keyChainRef, // importKeychain
+ &outItems.get()); // outItems
if (res != errSecSuccess) {
BOOST_THROW_EXCEPTION(Error("Failed to import private key: " + getErrorMessage(res)));
}
// C-style cast is used as per Apple convention
- SecKeychainItemRef privateKey = (SecKeychainItemRef)CFArrayGetValueAtIndex(outItems.get(), 0);
+ SecKeychainItemRef keychainItem = (SecKeychainItemRef)CFArrayGetValueAtIndex(outItems.get(), 0);
SecKeychainAttribute attrs[1]; // maximum number of attributes
SecKeychainAttributeList attrList = {0, attrs};
- std::string keyUri = keyName.toUri();
{
attrs[attrList.count].tag = kSecKeyPrintName;
attrs[attrList.count].length = keyUri.size();
attrs[attrList.count].data = const_cast<char*>(keyUri.data());
attrList.count++;
}
-
- SecKeychainItemModifyAttributesAndData(privateKey, &attrList, 0, nullptr);
+ SecKeychainItemModifyAttributesAndData(keychainItem, &attrList, 0, nullptr);
}
} // namespace tpm
diff --git a/tests/unit-tests/security/tpm/back-end.t.cpp b/tests/unit-tests/security/tpm/back-end.t.cpp
index 6880a0b..5512ebd 100644
--- a/tests/unit-tests/security/tpm/back-end.t.cpp
+++ b/tests/unit-tests/security/tpm/back-end.t.cpp
@@ -20,14 +20,16 @@
*/
#include "security/tpm/back-end.hpp"
-#include "security/tpm/back-end-mem.hpp"
-#include "security/tpm/key-handle.hpp"
-#include "security/tpm/tpm.hpp"
-#include "security/transform.hpp"
-#include "security/transform/public-key.hpp"
-#include "security/transform/private-key.hpp"
+
#include "encoding/buffer-stream.hpp"
#include "security/pib/key.hpp"
+#include "security/tpm/key-handle.hpp"
+#include "security/tpm/tpm.hpp"
+#include "security/transform/bool-sink.hpp"
+#include "security/transform/buffer-source.hpp"
+#include "security/transform/private-key.hpp"
+#include "security/transform/public-key.hpp"
+#include "security/transform/verifier-filter.hpp"
#include "back-end-wrapper-file.hpp"
#include "back-end-wrapper-mem.hpp"
@@ -90,9 +92,8 @@
T wrapper;
BackEnd& tpm = wrapper.getTpm();
- // create an rsa key
+ // create an RSA key
Name identity("/Test/KeyName");
-
unique_ptr<KeyHandle> key = tpm.createKey(identity, RsaKeyParams());
Name keyName = key->getKeyName();
@@ -121,9 +122,8 @@
T wrapper;
BackEnd& tpm = wrapper.getTpm();
- // create an rsa key
+ // create an RSA key
Name identity("/Test/KeyName");
-
unique_ptr<KeyHandle> key = tpm.createKey(identity, RsaKeyParams());
Name keyName = key->getKeyName();
@@ -134,7 +134,6 @@
pubKey.loadPkcs8(pubKeyBits->data(), pubKeyBits->size());
ConstBufferPtr cipherText = pubKey.encrypt(content, sizeof(content));
-
ConstBufferPtr plainText = key->decrypt(cipherText->data(), cipherText->size());
BOOST_CHECK_EQUAL_COLLECTIONS(content, content + sizeof(content),
@@ -149,9 +148,8 @@
T wrapper;
BackEnd& tpm = wrapper.getTpm();
- // create an ec key
+ // create an EC key
Name identity("/Test/Ec/KeyName");
-
unique_ptr<KeyHandle> key = tpm.createKey(identity, EcKeyParams());
Name ecKeyName = key->getKeyName();
@@ -177,7 +175,7 @@
BOOST_AUTO_TEST_CASE_TEMPLATE(ImportExport, T, TestBackEnds)
{
- const std::string privateKeyPkcs1 =
+ const std::string privKeyPkcs1 =
"MIIEpAIBAAKCAQEAw0WM1/WhAxyLtEqsiAJgWDZWuzkYpeYVdeeZcqRZzzfRgBQT\n"
"sNozS5t4HnwTZhwwXbH7k3QN0kRTV826Xobws3iigohnM9yTK+KKiayPhIAm/+5H\n"
"GT6SgFJhYhqo1/upWdueojil6RP4/AgavHhopxlAVbk6G9VdVnlQcQ5Zv0OcGi73\n"
@@ -203,42 +201,58 @@
"cuHICmsCgYAtFJ1idqMoHxES3mlRpf2JxyQudP3SCm2WpGmqVzhRYInqeatY5sUd\n"
"lPLHm/p77RT7EyxQHTlwn8FJPuM/4ZH1rQd/vB+Y8qAtYJCexDMsbvLW+Js+VOvk\n"
"jweEC0nrcL31j9mF0vz5E6tfRu4hhJ6L4yfWs0gSejskeVB/w8QY4g==\n";
+ const std::string password("password");
+ const std::string wrongPassword("wrong");
T wrapper;
BackEnd& tpm = wrapper.getTpm();
Name keyName("/Test/KeyName/KEY/1");
tpm.deleteKey(keyName);
- BOOST_CHECK_EQUAL(tpm.hasKey(keyName), false);
+ BOOST_REQUIRE_EQUAL(tpm.hasKey(keyName), false);
transform::PrivateKey sKey;
- sKey.loadPkcs1Base64(reinterpret_cast<const uint8_t*>(privateKeyPkcs1.c_str()), privateKeyPkcs1.size());
-
- std::string password("password");
+ sKey.loadPkcs1Base64(reinterpret_cast<const uint8_t*>(privKeyPkcs1.data()), privKeyPkcs1.size());
OBufferStream os;
- sKey.savePkcs8(os, password.c_str(), password.size());
- ConstBufferPtr privateKeyBuffer = os.buf();
+ sKey.savePkcs8(os, password.data(), password.size());
+ auto pkcs8 = os.buf();
- tpm.importKey(keyName, privateKeyBuffer->data(), privateKeyBuffer->size(), password.c_str(), password.size());
+ // import with wrong password
+ BOOST_CHECK_THROW(tpm.importKey(keyName, pkcs8->data(), pkcs8->size(), wrongPassword.data(), wrongPassword.size()),
+ BackEnd::Error);
+ BOOST_CHECK_EQUAL(tpm.hasKey(keyName), false);
+
+ // import with correct password
+ tpm.importKey(keyName, pkcs8->data(), pkcs8->size(), password.data(), password.size());
BOOST_CHECK_EQUAL(tpm.hasKey(keyName), true);
- BOOST_CHECK_THROW(tpm.importKey(keyName, privateKeyBuffer->data(), privateKeyBuffer->size(), password.c_str(), password.size()),
+
+ // import already present key
+ BOOST_CHECK_THROW(tpm.importKey(keyName, pkcs8->data(), pkcs8->size(), password.data(), password.size()),
BackEnd::Error);
- ConstBufferPtr exportedKey = tpm.exportKey(keyName, password.c_str(), password.size());
+ // test derivePublicKey with the imported key
+ auto keyHdl = tpm.getKeyHandle(keyName);
+ auto pubKey = keyHdl->derivePublicKey();
+ BOOST_CHECK(pubKey != nullptr);
+
+ // export
+ auto exportedKey = tpm.exportKey(keyName, password.data(), password.size());
BOOST_CHECK_EQUAL(tpm.hasKey(keyName), true);
transform::PrivateKey sKey2;
- sKey2.loadPkcs8(exportedKey->data(), exportedKey->size(), password.c_str(), password.size());
+ sKey2.loadPkcs8(exportedKey->data(), exportedKey->size(), password.data(), password.size());
OBufferStream os2;
sKey.savePkcs1Base64(os2);
- ConstBufferPtr pkcs1Buffer = os2.buf();
+ auto pkcs1 = os2.buf();
- BOOST_CHECK_EQUAL_COLLECTIONS(privateKeyPkcs1.begin(), privateKeyPkcs1.end(),
- pkcs1Buffer->begin(), pkcs1Buffer->end());
+ // verify that the exported key is identical to the key that was imported
+ BOOST_CHECK_EQUAL_COLLECTIONS(privKeyPkcs1.begin(), privKeyPkcs1.end(),
+ pkcs1->begin(), pkcs1->end());
+ // export nonexistent key
tpm.deleteKey(keyName);
BOOST_CHECK_EQUAL(tpm.hasKey(keyName), false);
- BOOST_CHECK_THROW(tpm.exportKey(keyName, password.c_str(), password.size()), BackEnd::Error);
+ BOOST_CHECK_THROW(tpm.exportKey(keyName, password.data(), password.size()), BackEnd::Error);
}
BOOST_AUTO_TEST_CASE(RandomKeyId)