Skip to content

Commit

Permalink
revert wdt changes introduced in D3707470
Browse files Browse the repository at this point in the history 
Summary: See title. Apparently my changes are causing crashes.

Reviewed By: uddipta

Differential Revision: D3752985

fbshipit-source-id: 71fff8d3252b10fcc1bd52faefe30c81de31ed5f
  • Loading branch information
@ivmaykov
ivmaykov authored and Facebook Github Bot 5 committed Aug 22, 2016
1 parent ccf86c8 commit 65e04e6
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 89 deletions.
111 changes: 43 additions & 68 deletions util/EncryptionUtils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
*/
#include <wdt/util/EncryptionUtils.h>
#include <folly/Conv.h>
#include <folly/ScopeGuard.h>
#include <folly/SpinLock.h>
#include <folly/String.h> // for humanify

Expand Down Expand Up @@ -257,21 +256,13 @@ EncryptionParams EncryptionParams::generateEncryptionParams(
return EncryptionParams(type, std::string(key, key + kAESBlockSize));
}

AESBase::AESBase()
: type_{ENC_NONE}, evpCtx_{EVP_CIPHER_CTX_new()}, started_{false} {
if (evpCtx_ == nullptr) {
throw std::runtime_error("Unable to allocate an EVP_CIPHER_CTX object");
}
}

AESBase::~AESBase() = default;

bool AESBase::cloneCtx(EVP_CIPHER_CTX* const ctxOut) const {
bool AESBase::cloneCtx(EVP_CIPHER_CTX* ctxOut) const {
WDT_CHECK(encryptionTypeToTagLen(type_));
int status = EVP_CIPHER_CTX_copy(ctxOut, evpCtx_.get());
EVP_CIPHER_CTX_init(ctxOut);
int status = EVP_CIPHER_CTX_copy(ctxOut, &evpCtx_);
if (status != 1) {
WLOG(ERROR) << "Cipher ctx copy failed " << status;
resetCipherCtx(ctxOut);
EVP_CIPHER_CTX_cleanup(ctxOut);
return false;
}
return true;
Expand All @@ -288,17 +279,6 @@ const EVP_CIPHER* AESBase::getCipher(const EncryptionType encryptionType) {
return nullptr;
}

void AESBase::resetCipherCtx(EVP_CIPHER_CTX* const ctx) {
if (ctx != nullptr) {
#if OPENSSL_VERSION_NUMBER < 0x10100000L
EVP_CIPHER_CTX_cleanup(ctx);
EVP_CIPHER_CTX_init(ctx);
#else
EVP_CIPHER_CTX_reset(ctx);
#endif
}
}

bool AESEncryptor::start(const EncryptionParams& encryptionData,
std::string& ivOut) {
WDT_CHECK(!started_);
Expand All @@ -322,6 +302,8 @@ bool AESEncryptor::start(const EncryptionParams& encryptionData,
return false;
}

EVP_CIPHER_CTX_init(&evpCtx_);

const EVP_CIPHER* cipher = getCipher(type_);
if (cipher == nullptr) {
return false;
Expand All @@ -333,19 +315,18 @@ bool AESEncryptor::start(const EncryptionParams& encryptionData,
// gcm only uses 96 out of the 128 bits of IV. Let's use all of it to
// reduce chances of attacks on large data transfers.
if (type_ == ENC_AES128_GCM) {
if (EVP_EncryptInit_ex(evpCtx_.get(), cipher, nullptr, nullptr, nullptr) !=
1) {
if (EVP_EncryptInit_ex(&evpCtx_, cipher, nullptr, nullptr, nullptr) != 1) {
WLOG(ERROR) << "GCM First init error";
}
if (EVP_CIPHER_CTX_ctrl(evpCtx_.get(), EVP_CTRL_GCM_SET_IVLEN, ivOut.size(),
if (EVP_CIPHER_CTX_ctrl(&evpCtx_, EVP_CTRL_GCM_SET_IVLEN, ivOut.size(),
nullptr) != 1) {
WLOG(ERROR) << "Encrypt Init ivlen set failed";
}
}

if (EVP_EncryptInit_ex(evpCtx_.get(), cipher, nullptr, keyPtr, ivPtr) != 1) {
if (EVP_EncryptInit_ex(&evpCtx_, cipher, nullptr, keyPtr, ivPtr) != 1) {
WLOG(ERROR) << "Encrypt Init failed";
resetCipherCtx(evpCtx_.get());
EVP_CIPHER_CTX_cleanup(&evpCtx_);
return false;
}
started_ = true;
Expand All @@ -356,41 +337,39 @@ bool AESEncryptor::encrypt(const char* in, const int inLength, char* out) {
WDT_CHECK(started_);

int outLength;
if (EVP_EncryptUpdate(evpCtx_.get(), (uint8_t*)out, &outLength, (uint8_t*)in,
if (EVP_EncryptUpdate(&evpCtx_, (uint8_t*)out, &outLength, (uint8_t*)in,
inLength) != 1) {
WLOG(ERROR) << "EncryptUpdate failed";
resetCipherCtx(evpCtx_.get());
return false;
}
WDT_CHECK_EQ(inLength, outLength);
return true;
}

/* static */
bool AESEncryptor::finishInternal(EVP_CIPHER_CTX* const ctx,
bool AESEncryptor::finishInternal(EVP_CIPHER_CTX& ctx,
const EncryptionType type,
std::string& tagOut) {
int outLength;
SCOPE_EXIT {
resetCipherCtx(ctx);
};
int status = EVP_EncryptFinal_ex(ctx, nullptr, &outLength);
int status = EVP_EncryptFinal(&ctx, nullptr, &outLength);
if (status != 1) {
WLOG(ERROR) << "EncryptFinal failed";
EVP_CIPHER_CTX_cleanup(&ctx);
return false;
}
WDT_CHECK_EQ(0, outLength);
size_t tagSize = encryptionTypeToTagLen(type);
if (tagSize) {
tagOut.resize(tagSize);
status = EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_GET_TAG, tagOut.size(),
status = EVP_CIPHER_CTX_ctrl(&ctx, EVP_CTRL_GCM_GET_TAG, tagOut.size(),
&(tagOut.front()));
if (status != 1) {
WLOG(ERROR) << "EncryptFinal Tag extraction error "
<< folly::humanify(tagOut);
tagOut.clear();
}
}
EVP_CIPHER_CTX_cleanup(&ctx);
return true;
}

Expand All @@ -400,31 +379,25 @@ bool AESEncryptor::finish(std::string& tagOut) {
return true;
}
started_ = false;
bool status = finishInternal(evpCtx_.get(), type_, tagOut);
bool status = finishInternal(evpCtx_, type_, tagOut);
WLOG_IF(INFO, status) << "Encryption finish tag = "
<< folly::humanify(tagOut);
return status;
}

std::string AESEncryptor::computeCurrentTag() {
std::unique_ptr<EVP_CIPHER_CTX, CipherCtxDeleter> ctx(EVP_CIPHER_CTX_new());
if (ctx == nullptr) {
throw std::runtime_error("Unable to allocate an EVP_CIPHER_CTX object");
}
EVP_CIPHER_CTX ctx;
std::string tag;
if (!cloneCtx(ctx.get())) {
if (!cloneCtx(&ctx)) {
return tag;
}
finishInternal(ctx.get(), type_, tag);
finishInternal(ctx, type_, tag);
return tag;
}

AESEncryptor::~AESEncryptor() = default;

AESDecryptor::AESDecryptor() : AESBase(), savedCtx_{EVP_CIPHER_CTX_new()} {
if (savedCtx_ == nullptr) {
throw std::runtime_error("Unable to allocate saved EVP_CIPHER_CTX object");
}
AESEncryptor::~AESEncryptor() {
std::string tag;
finish(tag);
}

bool AESDecryptor::start(const EncryptionParams& encryptionData,
Expand All @@ -447,6 +420,7 @@ bool AESDecryptor::start(const EncryptionParams& encryptionData,

uint8_t* ivPtr = (uint8_t*)(&iv.front());
uint8_t* keyPtr = (uint8_t*)(&key.front());
EVP_CIPHER_CTX_init(&evpCtx_);

const EVP_CIPHER* cipher = getCipher(type_);
if (cipher == nullptr) {
Expand All @@ -457,19 +431,18 @@ bool AESDecryptor::start(const EncryptionParams& encryptionData,
WDT_CHECK_EQ(1, cipherBlockSize);

if (type_ == ENC_AES128_GCM) {
if (EVP_DecryptInit_ex(evpCtx_.get(), cipher, nullptr, nullptr, nullptr) !=
1) {
if (EVP_EncryptInit_ex(&evpCtx_, cipher, nullptr, nullptr, nullptr) != 1) {
WLOG(ERROR) << "GCM Decryptor First init error";
}
if (EVP_CIPHER_CTX_ctrl(evpCtx_.get(), EVP_CTRL_GCM_SET_IVLEN, iv.size(),
if (EVP_CIPHER_CTX_ctrl(&evpCtx_, EVP_CTRL_GCM_SET_IVLEN, iv.size(),
nullptr) != 1) {
WLOG(ERROR) << "Encrypt Init ivlen set failed";
}
}

if (EVP_DecryptInit_ex(evpCtx_.get(), cipher, nullptr, keyPtr, ivPtr) != 1) {
if (EVP_DecryptInit_ex(&evpCtx_, cipher, nullptr, keyPtr, ivPtr) != 1) {
WLOG(ERROR) << "Decrypt Init failed";
resetCipherCtx(evpCtx_.get());
EVP_CIPHER_CTX_cleanup(&evpCtx_);
return false;
}
started_ = true;
Expand All @@ -480,10 +453,9 @@ bool AESDecryptor::decrypt(const char* in, const int inLength, char* out) {
WDT_CHECK(started_);

int outLength;
if (EVP_DecryptUpdate(evpCtx_.get(), (uint8_t*)out, &outLength, (uint8_t*)in,
if (EVP_DecryptUpdate(&evpCtx_, (uint8_t*)out, &outLength, (uint8_t*)in,
inLength) != 1) {
WLOG(ERROR) << "DecryptUpdate failed";
resetCipherCtx(evpCtx_.get());
return false;
}
WDT_CHECK_EQ(inLength, outLength);
Expand All @@ -492,7 +464,7 @@ bool AESDecryptor::decrypt(const char* in, const int inLength, char* out) {

bool AESDecryptor::saveContext() {
WDT_CHECK(!ctxSaved_);
if (!cloneCtx(savedCtx_.get())) {
if (!cloneCtx(&savedCtx_)) {
return false;
}
ctxSaved_ = true;
Expand All @@ -502,36 +474,36 @@ bool AESDecryptor::saveContext() {
bool AESDecryptor::verifyTag(const std::string& tag) {
WDT_CHECK_EQ(ENC_AES128_GCM, type_);
WDT_CHECK(ctxSaved_);
bool status = finishInternal(savedCtx_.get(), type_, tag);
bool status = finishInternal(savedCtx_, type_, tag);
ctxSaved_ = false;
return status;
}

/* static */
bool AESDecryptor::finishInternal(EVP_CIPHER_CTX* const ctx,
bool AESDecryptor::finishInternal(EVP_CIPHER_CTX& ctx,
const EncryptionType type,
const std::string& tag) {
int status;
size_t tagSize = encryptionTypeToTagLen(type);
SCOPE_EXIT {
resetCipherCtx(ctx);
};
if (tagSize) {
if (tag.size() != tagSize) {
WLOG(ERROR) << "Need tag for gcm mode " << folly::humanify(tag);
EVP_CIPHER_CTX_cleanup(&ctx);
return false;
}
// EVP_CIPHER_CTX_ctrl takes a non const buffer. But, for set tag the buffer
// will not be modified. So, it is safe to use const_cast here.
char* tagBuf = const_cast<char*>(tag.data());
status = EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GCM_SET_TAG, tag.size(), tagBuf);
status =
EVP_CIPHER_CTX_ctrl(&ctx, EVP_CTRL_GCM_SET_TAG, tag.size(), tagBuf);
if (status != 1) {
WLOG(ERROR) << "Decrypt final tag set error " << folly::humanify(tag);
}
}

int outLength = 0;
status = EVP_DecryptFinal_ex(ctx, nullptr, &outLength);
status = EVP_DecryptFinal(&ctx, nullptr, &outLength);
EVP_CIPHER_CTX_cleanup(&ctx);
if (status != 1) {
WLOG(ERROR) << "DecryptFinal failed " << outLength;
return false;
Expand All @@ -545,16 +517,19 @@ bool AESDecryptor::finish(const std::string& tag) {
return true;
}
started_ = false;
bool status = finishInternal(evpCtx_.get(), type_, tag);
bool status = finishInternal(evpCtx_, type_, tag);
WLOG_IF(INFO, status) << "Successful end of decryption with tag = "
<< folly::humanify(tag);
if (ctxSaved_) {
resetCipherCtx(savedCtx_.get());
EVP_CIPHER_CTX_cleanup(&savedCtx_);
ctxSaved_ = false;
}
return status;
}

AESDecryptor::~AESDecryptor() = default;
AESDecryptor::~AESDecryptor() {
std::string tag;
finish(tag);
}
}
} // end of namespaces
30 changes: 9 additions & 21 deletions util/EncryptionUtils.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,9 @@
*/
#pragma once

#include <memory>
#include <string>

#include <folly/Memory.h>
#include <openssl/evp.h>
#include <wdt/ErrorCodes.h>
#include <string>

namespace facebook {
namespace wdt {
Expand Down Expand Up @@ -100,24 +97,16 @@ class EncryptionParams {
/// base class to share code between encyptor and decryptor
class AESBase {
protected:
AESBase();
virtual ~AESBase();

/// evpCtx_ is copied into ctxOut
/// @return whether the cloning was successful
bool cloneCtx(EVP_CIPHER_CTX* const ctxOut) const;

using CipherCtxDeleter =
folly::static_function_deleter<EVP_CIPHER_CTX, &EVP_CIPHER_CTX_free>;
bool cloneCtx(EVP_CIPHER_CTX* ctxOut) const;

protected:
/// @return cipher for a encryption type
const EVP_CIPHER* getCipher(const EncryptionType encryptionType);

static void resetCipherCtx(EVP_CIPHER_CTX* const ctx);

EncryptionType type_;
std::unique_ptr<EVP_CIPHER_CTX, CipherCtxDeleter> evpCtx_;
bool started_;
EncryptionType type_{ENC_NONE};
EVP_CIPHER_CTX evpCtx_;
bool started_{false};
};

/// encryptor class
Expand Down Expand Up @@ -159,7 +148,7 @@ class AESEncryptor : public AESBase {
virtual ~AESEncryptor();

private:
static bool finishInternal(EVP_CIPHER_CTX* const ctx, EncryptionType type,
static bool finishInternal(EVP_CIPHER_CTX& ctx, EncryptionType type,
std::string& tagOut);
};

Expand Down Expand Up @@ -201,16 +190,15 @@ class AESDecryptor : public AESBase {
/// verify whether the given tag matches previously saved context
bool verifyTag(const std::string& tag);

AESDecryptor();
/// destructor
virtual ~AESDecryptor();

private:
static bool finishInternal(EVP_CIPHER_CTX* const ctx, EncryptionType type,
static bool finishInternal(EVP_CIPHER_CTX& ctx, EncryptionType type,
const std::string& tag);

/// saved cipher ctx
std::unique_ptr<EVP_CIPHER_CTX, CipherCtxDeleter> savedCtx_;
EVP_CIPHER_CTX savedCtx_;

/// whether ctx has been saved
bool ctxSaved_{false};
Expand Down

0 comments on commit 65e04e6

Please sign in to comment.