security: simplify/cleanup PublicKey implementation
Change-Id: I9ead8916d0c6d9264e087f594b8d3aaf07366c29
diff --git a/src/security/transform/public-key.cpp b/src/security/transform/public-key.cpp
index c17fdc0..3240cee 100644
--- a/src/security/transform/public-key.cpp
+++ b/src/security/transform/public-key.cpp
@@ -20,20 +20,26 @@
*/
#include "public-key.hpp"
-#include "buffer-source.hpp"
-#include "stream-source.hpp"
-#include "base64-encode.hpp"
#include "base64-decode.hpp"
+#include "base64-encode.hpp"
+#include "buffer-source.hpp"
#include "stream-sink.hpp"
-#include "../../encoding/buffer-stream.hpp"
+#include "stream-source.hpp"
#include "../detail/openssl-helper.hpp"
+#include "../../encoding/buffer-stream.hpp"
#define ENSURE_PUBLIC_KEY_LOADED(key) \
do { \
- if (key == nullptr) \
+ if ((key) == nullptr) \
BOOST_THROW_EXCEPTION(Error("Public key has not been loaded yet")); \
} while (false)
+#define ENSURE_PUBLIC_KEY_NOT_LOADED(key) \
+ do { \
+ if ((key) != nullptr) \
+ BOOST_THROW_EXCEPTION(Error("Public key has already been loaded")); \
+ } while (false)
+
namespace ndn {
namespace security {
namespace transform {
@@ -41,7 +47,7 @@
class PublicKey::Impl
{
public:
- Impl()
+ Impl() noexcept
: key(nullptr)
{
}
@@ -56,7 +62,7 @@
};
PublicKey::PublicKey()
- : m_impl(new Impl)
+ : m_impl(make_unique<Impl>())
{
}
@@ -67,36 +73,37 @@
{
ENSURE_PUBLIC_KEY_LOADED(m_impl->key);
+ int keyType =
#if OPENSSL_VERSION_NUMBER < 0x1010000fL
- switch (EVP_PKEY_type(m_impl->key->type)) {
+ EVP_PKEY_type(m_impl->key->type);
#else
- switch (EVP_PKEY_base_id(m_impl->key)) {
+ EVP_PKEY_base_id(m_impl->key);
#endif // OPENSSL_VERSION_NUMBER < 0x1010000fL
+
+ switch (keyType) {
case EVP_PKEY_RSA:
return KeyType::RSA;
case EVP_PKEY_EC:
return KeyType::EC;
default:
- BOOST_THROW_EXCEPTION(Error("Public key type is not recognized"));
+ BOOST_THROW_EXCEPTION(Error("Unrecognized public key type"));
}
}
void
PublicKey::loadPkcs8(const uint8_t* buf, size_t size)
{
- m_impl->key = d2i_PUBKEY(nullptr, &buf, size);
+ ENSURE_PUBLIC_KEY_NOT_LOADED(m_impl->key);
- ENSURE_PUBLIC_KEY_LOADED(m_impl->key);
+ if (d2i_PUBKEY(&m_impl->key, &buf, static_cast<long>(size)) == nullptr)
+ BOOST_THROW_EXCEPTION(Error("Failed to load public key"));
}
void
PublicKey::loadPkcs8(std::istream& is)
{
OBufferStream os;
- {
- using namespace transform;
- streamSource(is) >> streamSink(os);
- }
+ streamSource(is) >> streamSink(os);
this->loadPkcs8(os.buf()->buf(), os.buf()->size());
}
@@ -104,10 +111,7 @@
PublicKey::loadPkcs8Base64(const uint8_t* buf, size_t size)
{
OBufferStream os;
- {
- using namespace transform;
- bufferSource(buf, size) >> base64Decode() >> streamSink(os);
- }
+ bufferSource(buf, size) >> base64Decode() >> streamSink(os);
this->loadPkcs8(os.buf()->buf(), os.buf()->size());
}
@@ -115,24 +119,19 @@
PublicKey::loadPkcs8Base64(std::istream& is)
{
OBufferStream os;
- {
- using namespace transform;
- streamSource(is) >> base64Decode() >> streamSink(os);
- }
+ streamSource(is) >> base64Decode() >> streamSink(os);
this->loadPkcs8(os.buf()->buf(), os.buf()->size());
}
void
PublicKey::savePkcs8(std::ostream& os) const
{
- using namespace transform;
bufferSource(*this->toPkcs8()) >> streamSink(os);
}
void
PublicKey::savePkcs8Base64(std::ostream& os) const
{
- using namespace transform;
bufferSource(*this->toPkcs8()) >> base64Encode() >> streamSink(os);
}
@@ -171,9 +170,8 @@
uint8_t* pkcs8 = nullptr;
int len = i2d_PUBKEY(m_impl->key, &pkcs8);
-
- if (pkcs8 == nullptr)
- BOOST_THROW_EXCEPTION(Error("Failed to convert to pkcs8 format"));
+ if (len < 0)
+ BOOST_THROW_EXCEPTION(Error("Cannot convert key to PKCS #8 format"));
auto buffer = make_shared<Buffer>(pkcs8, len);
OPENSSL_free(pkcs8);
@@ -198,7 +196,6 @@
BOOST_THROW_EXCEPTION(Error("Failed to estimate output length"));
auto out = make_shared<Buffer>(outlen);
-
if (EVP_PKEY_encrypt(ctx, out->buf(), &outlen, plainText, plainLen) <= 0)
BOOST_THROW_EXCEPTION(Error("Failed to encrypt plaintext"));
diff --git a/src/security/transform/public-key.hpp b/src/security/transform/public-key.hpp
index db08cad..a94814d 100644
--- a/src/security/transform/public-key.hpp
+++ b/src/security/transform/public-key.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).
@@ -29,8 +29,6 @@
namespace security {
namespace transform {
-class VerifierFilter;
-
/**
* @brief Abstraction of public key in crypto transformation
*/
@@ -47,13 +45,11 @@
}
};
- friend class VerifierFilter;
-
public:
/**
- * @brief Create a public key instance
+ * @brief Create an empty public key instance
*
- * One must call loadXXXX(...) to load public key.
+ * One must call loadXXXX(...) to load a public key.
*/
PublicKey();
@@ -102,7 +98,7 @@
savePkcs8Base64(std::ostream& os) const;
/**
- * @return Cipher text of @p plainText encrypted using the public key.
+ * @return Cipher text of @p plainText encrypted using this public key.
*
* Only RSA encryption is supported for now.
*/
@@ -110,10 +106,12 @@
encrypt(const uint8_t* plainText, size_t plainLen) const;
private:
+ friend class VerifierFilter;
+
/**
- * @return A pointer to an EVP_PKEY instance.
+ * @return A pointer to an OpenSSL EVP_PKEY instance.
*
- * One need to explicitly cast the return value to EVP_PKEY*.
+ * The caller needs to explicitly cast the return value to `EVP_PKEY*`.
*/
void*
getEvpPkey() const;
diff --git a/tests/unit-tests/security/transform/public-key.t.cpp b/tests/unit-tests/security/transform/public-key.t.cpp
index 26903e7..41b0582 100644
--- a/tests/unit-tests/security/transform/public-key.t.cpp
+++ b/tests/unit-tests/security/transform/public-key.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).
@@ -20,13 +20,14 @@
*/
#include "security/transform/public-key.hpp"
-#include "security/transform/buffer-source.hpp"
-#include "security/transform/base64-decode.hpp"
-#include "security/transform/stream-sink.hpp"
-#include "encoding/buffer-stream.hpp"
-#include <boost/mpl/list.hpp>
+#include "encoding/buffer-stream.hpp"
+#include "security/transform.hpp"
+
#include "boost-test.hpp"
+#include <boost/mpl/vector.hpp>
+
+#include <sstream>
namespace ndn {
namespace security {
@@ -37,12 +38,9 @@
BOOST_AUTO_TEST_SUITE(Transform)
BOOST_AUTO_TEST_SUITE(TestPublicKey)
-class RsaPublicKeyTestData
+struct RsaKeyTestData
{
-public:
- RsaPublicKeyTestData()
- {
- publicKeyPkcs8 =
+ const std::string publicKeyPkcs8 =
"MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAw0WM1/WhAxyLtEqsiAJg\n"
"WDZWuzkYpeYVdeeZcqRZzzfRgBQTsNozS5t4HnwTZhwwXbH7k3QN0kRTV826Xobw\n"
"s3iigohnM9yTK+KKiayPhIAm/+5HGT6SgFJhYhqo1/upWdueojil6RP4/AgavHho\n"
@@ -50,18 +48,11 @@
"ZwIL5PuE9BiO6I39cL9z7EK1SfZhOWvDe/qH7YhD/BHwcWit8FjRww1glwRVTJsA\n"
"9rH58ynaAix0tcR/nBMRLUX+e3rURHg6UbSjJbdb9qmKM1fTGHKUzL/5pMG6uBU0\n"
"ywIDAQAB\n";
- }
-
-public:
- std::string publicKeyPkcs8;
};
-class EcPublicKeyTestData
+struct EcKeyTestData
{
-public:
- EcPublicKeyTestData()
- {
- publicKeyPkcs8 =
+ const std::string publicKeyPkcs8 =
"MIIBSzCCAQMGByqGSM49AgEwgfcCAQEwLAYHKoZIzj0BAQIhAP////8AAAABAAAA\n"
"AAAAAAAAAAAA////////////////MFsEIP////8AAAABAAAAAAAAAAAAAAAA////\n"
"///////////8BCBaxjXYqjqT57PrvVV2mIa8ZR0GsMxTsPY7zjw+J9JgSwMVAMSd\n"
@@ -69,22 +60,16 @@
"RdiYwpZP40Li/hp/m47n60p8D54WK84zV2sxXs7LtkBoN79R9QIhAP////8AAAAA\n"
"//////////+85vqtpxeehPO5ysL8YyVRAgEBA0IABGhuFibgwLdEJBDOLdvSg1Hc\n"
"5EJTDxq6ls5FoYLfThp8HOjuwGSz0qw8ocMqyku1y0V5peQ4rEPd0bwcpZd9svA=\n";
- }
-
-public:
- std::string publicKeyPkcs8;
};
-typedef boost::mpl::list<RsaPublicKeyTestData,
- EcPublicKeyTestData> PublicKeyTestDataSets;
+using KeyTestDataSets = boost::mpl::vector<RsaKeyTestData, EcKeyTestData>;
-BOOST_AUTO_TEST_CASE_TEMPLATE(SaveLoad, T, PublicKeyTestDataSets)
+BOOST_AUTO_TEST_CASE_TEMPLATE(SaveLoad, T, KeyTestDataSets)
{
T dataSet;
const uint8_t* pKeyPkcs8Base64 = reinterpret_cast<const uint8_t*>(dataSet.publicKeyPkcs8.c_str());
size_t pKeyPkcs8Base64Len = dataSet.publicKeyPkcs8.size();
-
OBufferStream os;
bufferSource(pKeyPkcs8Base64, pKeyPkcs8Base64Len) >> base64Decode() >> streamSink(os);
ConstBufferPtr pKeyPkcs8Buf = os.buf();
@@ -92,19 +77,19 @@
size_t pKeyPkcs8Len = pKeyPkcs8Buf->size();
PublicKey pKey1;
- BOOST_REQUIRE_NO_THROW(pKey1.loadPkcs8Base64(pKeyPkcs8Base64, pKeyPkcs8Base64Len));
+ BOOST_CHECK_NO_THROW(pKey1.loadPkcs8Base64(pKeyPkcs8Base64, pKeyPkcs8Base64Len));
std::stringstream ss2(dataSet.publicKeyPkcs8);
PublicKey pKey2;
- BOOST_REQUIRE_NO_THROW(pKey2.loadPkcs8Base64(ss2));
+ BOOST_CHECK_NO_THROW(pKey2.loadPkcs8Base64(ss2));
PublicKey pKey3;
- BOOST_REQUIRE_NO_THROW(pKey3.loadPkcs8(pKeyPkcs8, pKeyPkcs8Len));
+ BOOST_CHECK_NO_THROW(pKey3.loadPkcs8(pKeyPkcs8, pKeyPkcs8Len));
std::stringstream ss4;
ss4.write(reinterpret_cast<const char*>(pKeyPkcs8), pKeyPkcs8Len);
PublicKey pKey4;
- BOOST_REQUIRE_NO_THROW(pKey4.loadPkcs8(ss4));
+ BOOST_CHECK_NO_THROW(pKey4.loadPkcs8(ss4));
OBufferStream os5;
BOOST_REQUIRE_NO_THROW(pKey1.savePkcs8Base64(os5));
@@ -117,6 +102,25 @@
os6.buf()->begin(), os6.buf()->end());
}
+// NOTE: We cannot test RSA encryption by comparing the computed ciphertext to
+// a known-good one, because OAEP padding is randomized and would produce
+// different results every time. An encrypt/decrypt round-trip test is
+// performed in private-key.t.cpp
+
+BOOST_AUTO_TEST_CASE(UnsupportedEcEncryption)
+{
+ EcKeyTestData dataSet;
+
+ PublicKey pKey;
+ pKey.loadPkcs8Base64(reinterpret_cast<const uint8_t*>(dataSet.publicKeyPkcs8.c_str()),
+ dataSet.publicKeyPkcs8.size());
+
+ OBufferStream os;
+ bufferSource("Y2lhbyFob2xhIWhlbGxvIQ==") >> base64Decode() >> streamSink(os);
+
+ BOOST_CHECK_THROW(pKey.encrypt(os.buf()->buf(), os.buf()->size()), PublicKey::Error);
+}
+
BOOST_AUTO_TEST_SUITE_END() // TestPublicKey
BOOST_AUTO_TEST_SUITE_END() // Transform
BOOST_AUTO_TEST_SUITE_END() // Security