update mem leak in crypto helpers
Change-Id: Id9294dba9f1f2639edc58b5112b619eeee80af55
diff --git a/src/ca-module.cpp b/src/ca-module.cpp
index 98fb2ab..67b034b 100644
--- a/src/ca-module.cpp
+++ b/src/ca-module.cpp
@@ -19,7 +19,7 @@
*/
#include "ca-module.hpp"
-#include "detail/crypto-helper.hpp"
+#include "detail/crypto-helpers.hpp"
#include "identity-challenge/challenge-module.hpp"
#include "name-assignment/assignment-func.hpp"
#include "detail/challenge-encoder.hpp"
diff --git a/src/ca-module.hpp b/src/ca-module.hpp
index 0e6ea06..e1cedaf 100644
--- a/src/ca-module.hpp
+++ b/src/ca-module.hpp
@@ -22,7 +22,7 @@
#define NDNCERT_CA_MODULE_HPP
#include "configuration.hpp"
-#include "detail/crypto-helper.hpp"
+#include "detail/crypto-helpers.hpp"
#include "detail/ca-storage.hpp"
namespace ndn {
diff --git a/src/detail/crypto-helper.cpp b/src/detail/crypto-helpers.cpp
similarity index 84%
rename from src/detail/crypto-helper.cpp
rename to src/detail/crypto-helpers.cpp
index ae86796..2437d16 100644
--- a/src/detail/crypto-helper.cpp
+++ b/src/detail/crypto-helpers.cpp
@@ -18,27 +18,48 @@
* See AUTHORS.md for complete list of ndncert authors and contributors.
*/
-#include "crypto-helper.hpp"
+#include "crypto-helpers.hpp"
+
+#include <boost/endian/conversion.hpp>
#include <cmath>
-#include <openssl/err.h>
-#include <openssl/hmac.h>
-#include <openssl/pem.h>
-#include <openssl/ec.h>
-#include <openssl/evp.h>
-#include <openssl/kdf.h>
#include <ndn-cxx/encoding/buffer-stream.hpp>
#include <ndn-cxx/security/transform/base64-decode.hpp>
#include <ndn-cxx/security/transform/base64-encode.hpp>
#include <ndn-cxx/security/transform/buffer-source.hpp>
#include <ndn-cxx/security/transform/stream-sink.hpp>
#include <ndn-cxx/util/random.hpp>
-#include <boost/endian/conversion.hpp>
+#include <openssl/ec.h>
+#include <openssl/err.h>
+#include <openssl/evp.h>
+#include <openssl/hmac.h>
+#include <openssl/kdf.h>
+#include <openssl/pem.h>
namespace ndn {
namespace ndncert {
struct ECDHState::ECDH_CTX
{
+ ~ECDH_CTX()
+ {
+ // Contexts
+ if (ctx_params != nullptr) {
+ EVP_PKEY_CTX_free(ctx_params);
+ }
+ if (ctx_keygen != nullptr) {
+ EVP_PKEY_CTX_free(ctx_keygen);
+ }
+ // Keys
+ if (privkey != nullptr) {
+ EVP_PKEY_free(privkey);
+ }
+ if (peerkey != nullptr) {
+ EVP_PKEY_free(peerkey);
+ }
+ if (params != nullptr) {
+ EVP_PKEY_free(params);
+ }
+ }
EVP_PKEY_CTX* ctx_params = nullptr;
EVP_PKEY_CTX* ctx_keygen = nullptr;
EVP_PKEY* privkey = nullptr;
@@ -47,88 +68,57 @@
};
ECDHState::ECDHState()
- : m_publicKeyLen(0)
- , m_sharedSecretLen(0)
{
- OpenSSL_add_all_algorithms();
context = std::make_unique<ECDH_CTX>();
auto EC_NID = NID_X9_62_prime256v1;
- // Create the context for parameter generation
if (nullptr == (context->ctx_params = EVP_PKEY_CTX_new_id(EVP_PKEY_EC, nullptr))) {
- NDN_THROW(std::runtime_error("Could not create context contexts."));
+ NDN_THROW(std::runtime_error("Could not create context."));
}
-
- // Initialise the parameter generation
if (EVP_PKEY_paramgen_init(context->ctx_params) != 1) {
+ context.reset();
NDN_THROW(std::runtime_error("Could not initialize parameter generation."));
}
-
- // We're going to use the ANSI X9.62 Prime 256v1 curve
if (1 != EVP_PKEY_CTX_set_ec_paramgen_curve_nid(context->ctx_params, EC_NID)) {
+ context.reset();
NDN_THROW(std::runtime_error("Likely unknown elliptical curve ID specified."));
}
-
- // Create the parameter object params
if (!EVP_PKEY_paramgen(context->ctx_params, &context->params)) {
- // the generated key is written to context->params
+ context.reset();
NDN_THROW(std::runtime_error("Could not create parameter object parameters."));
}
-
- // Create the context for the key generation
if (nullptr == (context->ctx_keygen = EVP_PKEY_CTX_new(context->params, nullptr))) {
- //The EVP_PKEY_CTX_new() function allocates public key algorithm context using
- //the algorithm specified in pkey and ENGINE e (in this case nullptr).
+ context.reset();
NDN_THROW(std::runtime_error("Could not create the context for the key generation"));
}
-
- // initializes a public key algorithm context
if (1 != EVP_PKEY_keygen_init(context->ctx_keygen)) {
+ context.reset();
NDN_THROW(std::runtime_error("Could not init context for key generation."));
}
if (1 != EVP_PKEY_keygen(context->ctx_keygen, &context->privkey)) {
- //performs a key generation operation, the generated key is written to context->privkey.
+ context.reset();
NDN_THROW(std::runtime_error("Could not generate DHE keys in final step"));
}
}
ECDHState::~ECDHState()
-{
- // Contexts
- if (context->ctx_params != nullptr) {
- EVP_PKEY_CTX_free(context->ctx_params);
- }
- if (context->ctx_keygen != nullptr) {
- EVP_PKEY_CTX_free(context->ctx_keygen);
- }
-
- // Keys
- if (context->privkey != nullptr) {
- EVP_PKEY_free(context->privkey);
- }
- if (context->peerkey != nullptr) {
- EVP_PKEY_free(context->peerkey);
- }
- if (context->params != nullptr) {
- EVP_PKEY_free(context->params);
- }
-}
+{}
uint8_t*
ECDHState::getRawSelfPubKey()
{
auto privECKey = EVP_PKEY_get1_EC_KEY(context->privkey);
-
if (privECKey == nullptr) {
+ context.reset();
NDN_THROW(std::runtime_error("Could not get key when calling EVP_PKEY_get1_EC_KEY()."));
}
-
auto ecPoint = EC_KEY_get0_public_key(privECKey);
const EC_GROUP* group = EC_KEY_get0_group(privECKey);
m_publicKeyLen = EC_POINT_point2oct(group, ecPoint, POINT_CONVERSION_COMPRESSED,
m_publicKey, 256, nullptr);
EC_KEY_free(privECKey);
if (m_publicKeyLen == 0) {
+ context.reset();
NDN_THROW(std::runtime_error("Could not convert EC_POINTS to octet string when calling EC_POINT_point2oct."));
}
return m_publicKey;
@@ -137,12 +127,11 @@
std::string
ECDHState::getBase64PubKey()
{
- namespace t = ndn::security::transform;
-
if (m_publicKeyLen == 0) {
this->getRawSelfPubKey();
}
std::ostringstream os;
+ namespace t = ndn::security::transform;
t::bufferSource(m_publicKey, m_publicKeyLen) >> t::base64Encode(false) >> t::streamSink(os);
return os.str();
}
@@ -151,24 +140,24 @@
ECDHState::deriveSecret(const uint8_t* peerkey, size_t peerKeySize)
{
auto privECKey = EVP_PKEY_get1_EC_KEY(context->privkey);
-
if (privECKey == nullptr) {
+ context.reset();
NDN_THROW(std::runtime_error("Could not get key when calling EVP_PKEY_get1_EC_KEY()"));
}
-
auto group = EC_KEY_get0_group(privECKey);
auto peerPoint = EC_POINT_new(group);
int result = EC_POINT_oct2point(group, peerPoint, peerkey, peerKeySize, nullptr);
if (result == 0) {
EC_POINT_free(peerPoint);
EC_KEY_free(privECKey);
+ context.reset();
NDN_THROW(std::runtime_error("Cannot convert peer's key into a EC point when calling EC_POINT_oct2point()"));
}
-
result = ECDH_compute_key(m_sharedSecret, 256, peerPoint, privECKey, nullptr);
if (result == -1) {
EC_POINT_free(peerPoint);
EC_KEY_free(privECKey);
+ context.reset();
NDN_THROW(std::runtime_error("Cannot generate ECDH secret when calling ECDH_compute_key()"));
}
m_sharedSecretLen = static_cast<size_t>(result);
@@ -181,11 +170,9 @@
ECDHState::deriveSecret(const std::string& peerKeyStr)
{
namespace t = ndn::security::transform;
-
OBufferStream os;
t::bufferSource(peerKeyStr) >> t::base64Decode(false) >> t::streamSink(os);
auto result = os.buf();
-
return this->deriveSecret(result->data(), result->size());
}
@@ -243,52 +230,39 @@
EVP_CIPHER_CTX* ctx;
int len;
int ciphertext_len;
-
- // Create and initialise the context
if (!(ctx = EVP_CIPHER_CTX_new())) {
NDN_THROW(std::runtime_error("Cannot create and initialise the context when calling EVP_CIPHER_CTX_new()"));
}
-
- // Initialise the encryption operation.
if (1 != EVP_EncryptInit_ex(ctx, EVP_aes_128_gcm(), nullptr, nullptr, nullptr)) {
+ EVP_CIPHER_CTX_free(ctx);
NDN_THROW(std::runtime_error("Cannot initialise the encryption operation when calling EVP_EncryptInit_ex()"));
}
-
- // Set IV length if default 12 bytes (96 bits) is not appropriate
if (1 != EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_IVLEN, 12, nullptr)) {
+ EVP_CIPHER_CTX_free(ctx);
NDN_THROW(std::runtime_error("Cannot set IV length when calling EVP_CIPHER_CTX_ctrl()"));
}
-
- // Initialise key and IV
if (1 != EVP_EncryptInit_ex(ctx, nullptr, nullptr, key, iv)) {
+ EVP_CIPHER_CTX_free(ctx);
NDN_THROW(std::runtime_error("Cannot initialize key and IV when calling EVP_EncryptInit_ex()"));
}
-
- // Provide any AAD data. This can be called zero or more times as required
if (1 != EVP_EncryptUpdate(ctx, nullptr, &len, associated, associated_len)) {
+ EVP_CIPHER_CTX_free(ctx);
NDN_THROW(std::runtime_error("Cannot set associated authentication data when calling EVP_EncryptUpdate()"));
}
-
- // Provide the message to be encrypted, and obtain the encrypted output.
- // EVP_EncryptUpdate can be called multiple times if necessary
if (1 != EVP_EncryptUpdate(ctx, ciphertext, &len, plaintext, plaintext_len)) {
+ EVP_CIPHER_CTX_free(ctx);
NDN_THROW(std::runtime_error("Cannot encrypt when calling EVP_EncryptUpdate()"));
}
ciphertext_len = len;
-
- // Finalise the encryption. Normally ciphertext bytes may be written at
- // this stage, but this does not occur in GCM mode
if (1 != EVP_EncryptFinal_ex(ctx, ciphertext + len, &len)) {
+ EVP_CIPHER_CTX_free(ctx);
NDN_THROW(std::runtime_error("Cannot finalise the encryption when calling EVP_EncryptFinal_ex()"));
}
ciphertext_len += len;
-
- // Get the tag
if (1 != EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_GET_TAG, 16, tag)) {
+ EVP_CIPHER_CTX_free(ctx);
NDN_THROW(std::runtime_error("Cannot get tag when calling EVP_CIPHER_CTX_ctrl()"));
}
-
- // Clean up
EVP_CIPHER_CTX_free(ctx);
return ciphertext_len;
}
@@ -301,51 +275,39 @@
int len;
int plaintext_len;
int ret;
-
- // Create and initialise the context
if (!(ctx = EVP_CIPHER_CTX_new())) {
NDN_THROW(std::runtime_error("Cannot create and initialise the context when calling EVP_CIPHER_CTX_new()"));
}
-
- // Initialise the decryption operation.
if (!EVP_DecryptInit_ex(ctx, EVP_aes_128_gcm(), nullptr, nullptr, nullptr)) {
+ EVP_CIPHER_CTX_free(ctx);
NDN_THROW(std::runtime_error("Cannot initialise the decryption operation when calling EVP_DecryptInit_ex()"));
}
-
- // Set IV length. Not necessary if this is 12 bytes (96 bits)
if (!EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_IVLEN, 12, nullptr)) {
+ EVP_CIPHER_CTX_free(ctx);
NDN_THROW(std::runtime_error("Cannot set IV length when calling EVP_CIPHER_CTX_ctrl"));
}
-
- // Initialise key and IV
if (!EVP_DecryptInit_ex(ctx, nullptr, nullptr, key, iv)) {
+ EVP_CIPHER_CTX_free(ctx);
NDN_THROW(std::runtime_error("Cannot initialise key and IV when calling EVP_DecryptInit_ex()"));
}
-
- // Provide any AAD data. This can be called zero or more times as required
if (!EVP_DecryptUpdate(ctx, nullptr, &len, associated, associated_len)) {
+ EVP_CIPHER_CTX_free(ctx);
NDN_THROW(std::runtime_error("Cannot set associated authentication data when calling EVP_EncryptUpdate()"));
}
-
- // Provide the message to be decrypted, and obtain the plaintext output.
- // EVP_DecryptUpdate can be called multiple times if necessary
if (!EVP_DecryptUpdate(ctx, plaintext, &len, ciphertext, ciphertext_len)) {
+ EVP_CIPHER_CTX_free(ctx);
NDN_THROW(std::runtime_error("Cannot decrypt when calling EVP_DecryptUpdate()"));
}
plaintext_len = len;
-
- // Set expected tag value. Works in OpenSSL 1.0.1d and later
if (!EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_TAG, 16, (void*)tag)) {
+ EVP_CIPHER_CTX_free(ctx);
NDN_THROW(std::runtime_error("Cannot set tag value when calling EVP_CIPHER_CTX_ctrl"));
}
-
// Finalise the decryption. A positive return value indicates success,
// anything else is a failure - the plaintext is not trustworthy.
ret = EVP_DecryptFinal_ex(ctx, plaintext + len, &len);
-
// Clean up
EVP_CIPHER_CTX_free(ctx);
-
if (ret > 0) {
// Success
plaintext_len += len;
diff --git a/src/detail/crypto-helper.hpp b/src/detail/crypto-helpers.hpp
similarity index 98%
rename from src/detail/crypto-helper.hpp
rename to src/detail/crypto-helpers.hpp
index ecf9327..00b91b1 100644
--- a/src/detail/crypto-helper.hpp
+++ b/src/detail/crypto-helpers.hpp
@@ -38,11 +38,6 @@
uint8_t*
deriveSecret(const std::string& peerKeyStr);
- uint8_t m_publicKey[256];
- size_t m_publicKeyLen;
- uint8_t m_sharedSecret[256];
- size_t m_sharedSecretLen;
-
NDNCERT_PUBLIC_WITH_TESTS_ELSE_PRIVATE:
uint8_t*
deriveSecret(const uint8_t* peerkey, size_t peerKeySize);
@@ -50,6 +45,12 @@
uint8_t*
getRawSelfPubKey();
+public:
+ uint8_t m_publicKey[256];
+ size_t m_publicKeyLen = 0;
+ uint8_t m_sharedSecret[256];
+ size_t m_sharedSecretLen = 0;
+
private:
struct ECDH_CTX;
unique_ptr<ECDH_CTX> context;
diff --git a/src/requester-state.hpp b/src/requester-state.hpp
index ffebd8f..2c90fac 100644
--- a/src/requester-state.hpp
+++ b/src/requester-state.hpp
@@ -22,7 +22,7 @@
#define NDNCERT_REQUESTER_STATE_HPP
#include "detail/ca-state.hpp"
-#include "detail/crypto-helper.hpp"
+#include "detail/crypto-helpers.hpp"
#include "configuration.hpp"
namespace ndn {
diff --git a/src/requester.cpp b/src/requester.cpp
index 412b7f3..b4cc929 100644
--- a/src/requester.cpp
+++ b/src/requester.cpp
@@ -20,7 +20,7 @@
#include "requester.hpp"
#include "identity-challenge/challenge-module.hpp"
-#include "detail/crypto-helper.hpp"
+#include "detail/crypto-helpers.hpp"
#include "detail/challenge-encoder.hpp"
#include "detail/error-encoder.hpp"
#include "detail/info-encoder.hpp"
diff --git a/tests/unit-tests/crypto-helper.t.cpp b/tests/unit-tests/crypto-helper.t.cpp
index 349800f..8f10361 100644
--- a/tests/unit-tests/crypto-helper.t.cpp
+++ b/tests/unit-tests/crypto-helper.t.cpp
@@ -18,7 +18,7 @@
* See AUTHORS.md for complete list of ndncert authors and contributors.
*/
-#include "detail/crypto-helper.hpp"
+#include "detail/crypto-helpers.hpp"
#include "test-common.hpp"
namespace ndn {