Commit 3cdc99f2 authored by Mingtao Yang's avatar Mingtao Yang Committed by Facebook Github Bot

ssl: Fix SSLContext initialization test relying on undefined behavior

Summary:
The context initialization test was testing undefined behavior by asserting that
OpenSSL will return 0 (and consequently, result in SSLContext throwing) after
a SSL_CTX is freed. That is to say, it is exercising control flow based on
memory that is already freed.

This diff alters the logic to detect an SSL_CTX free via EX_DATA.

Reviewed By: igorsugak

Differential Revision: D19306728

fbshipit-source-id: cc3a94dbb0b2ab614fa72b099c585467b7156613
parent 10067433
...@@ -102,23 +102,37 @@ TEST(SSLContextInitializationTest, SSLContextLocksSetAfterInitIgnored) { ...@@ -102,23 +102,37 @@ TEST(SSLContextInitializationTest, SSLContextLocksSetAfterInitIgnored) {
TEST(SSLContextInitializationTest, SSLContext_SSL_CTX_constructor) { TEST(SSLContextInitializationTest, SSLContext_SSL_CTX_constructor) {
folly::ssl::init(); folly::ssl::init();
// Used to determine when SSL_CTX is freed.
auto onFree = [](void*, void* arg, CRYPTO_EX_DATA*, int, long, void*) {
bool* freed = static_cast<bool*>(arg);
*freed = true;
};
static int idx = SSL_CTX_get_ex_new_index(
0 /*argl */,
nullptr /*argp*/,
nullptr /*new_func*/,
nullptr /*dup_func*/,
onFree /*free_func*/);
bool freed = false;
SSL_CTX* ctx = SSL_CTX_new(TLS_method()); SSL_CTX* ctx = SSL_CTX_new(TLS_method());
EXPECT_NE(ctx, nullptr) << "SSL_CTX* creation for test failed"; EXPECT_NE(ctx, nullptr) << "SSL_CTX* creation for test failed";
(void)SSL_CTX_set_ex_data(ctx, idx, &freed);
{ {
// SSLContext takes "ownership" (read: increments the ref count), and will
// free ctx on destruction.
folly::SSLContext sslContext(ctx); folly::SSLContext sslContext(ctx);
SSL_CTX_free(ctx); }
// Shouldn't be fully freed because SSLContext should've added to the // Shouldn't be fully freed because SSLContext should've added to the
// refcount. up_ref should succed // refcount. up_ref should succed
EXPECT_EQ(SSL_CTX_up_ref(ctx), 1) EXPECT_EQ(freed, false);
<< "Incrementing ctx refcount failed, SSLContext isn't grabbing a ref on creation";
}
// Last reference, ctx should no longer be valid
SSL_CTX_free(ctx);
// Should throw because ctx is no longer valid, and the constructor should // Last reference, ctx should no longer be valid. Should trigger the ex_data
// fail on incrementing ctx refcount // free func.
EXPECT_THROW(folly::SSLContext sslContext(ctx), std::runtime_error); SSL_CTX_free(ctx);
EXPECT_EQ(freed, true);
} }
} // namespace folly } // namespace folly
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment