Skip to content

Commit 4724a25

Browse files
maxtropetseddyashton
maxtropets
authored and
eddyashton
committed
Put sha256 context init/shutdown under a RAII wrapper (microsoft#7251)
1 parent 303b3d3 commit 4724a25

27 files changed

Lines changed: 94 additions & 182 deletions

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
1818

1919
- Correctly validate the full AMD ASK endorsement chain (#7233)
2020

21+
### Removed
22+
23+
- Removed `ccf::crypt::openssl_sha256_init()` and `ccf::crypt::openssl_sha256_shutdown()` interface, as it's now implicitly called by the crypto implementation (#7251).
24+
2125
## [7.0.0-dev2]
2226

2327
[7.0.0-dev2]: https://github.com/microsoft/CCF/releases/tag/ccf-7.0.0-dev2

‎include/ccf/crypto/openssl_init.h‎

Lines changed: 0 additions & 20 deletions
This file was deleted.

‎src/crypto/openssl/hash.cpp‎

Lines changed: 89 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,95 @@
33

44
#include "crypto/openssl/hash.h"
55

6-
#include "ccf/crypto/openssl_init.h"
7-
86
#include <limits>
97
#include <openssl/evp.h>
108
#include <openssl/sha.h>
119
#include <stdexcept>
1210

11+
namespace
12+
{
13+
struct Sha256Context
14+
{
15+
Sha256Context()
16+
{
17+
openssl_sha256_init();
18+
}
19+
20+
Sha256Context(const Sha256Context&) = delete;
21+
Sha256Context& operator=(const Sha256Context&) = delete;
22+
23+
Sha256Context(Sha256Context&&) = delete;
24+
Sha256Context& operator=(Sha256Context&&) = delete;
25+
26+
~Sha256Context()
27+
{
28+
openssl_sha256_shutdown();
29+
}
30+
31+
[[nodiscard]] EVP_MD_CTX* get_basectx() const
32+
{
33+
return basectx;
34+
}
35+
36+
[[nodiscard]] EVP_MD_CTX* get_mdctx() const
37+
{
38+
return mdctx;
39+
}
40+
41+
private:
42+
void openssl_sha256_init()
43+
{
44+
if (mdctx != nullptr || basectx != nullptr)
45+
{
46+
throw std::logic_error(
47+
"openssl_sha256_init: double-init of the context");
48+
}
49+
50+
mdctx = EVP_MD_CTX_new();
51+
if (mdctx == nullptr)
52+
{
53+
throw std::logic_error("openssl_sha256_init: failed to create mdctx");
54+
}
55+
56+
basectx = EVP_MD_CTX_new();
57+
if (basectx == nullptr)
58+
{
59+
mdctx = nullptr;
60+
throw std::logic_error("openssl_sha256_init: failed to create basectx");
61+
}
62+
63+
if (EVP_DigestInit_ex(basectx, EVP_sha256(), nullptr) != 1)
64+
{
65+
mdctx = nullptr;
66+
basectx = nullptr;
67+
throw std::logic_error("EVP_DigestInit_ex failed");
68+
}
69+
70+
EVP_MD_CTX* mdctx{nullptr};
71+
EVP_MD_CTX* basectx{nullptr};
72+
}
73+
74+
void openssl_sha256_shutdown()
75+
{
76+
if (mdctx != nullptr)
77+
{
78+
EVP_MD_CTX_free(mdctx);
79+
mdctx = nullptr;
80+
}
81+
if (basectx != nullptr)
82+
{
83+
EVP_MD_CTX_free(basectx);
84+
basectx = nullptr;
85+
}
86+
}
87+
88+
EVP_MD_CTX* basectx{nullptr};
89+
EVP_MD_CTX* mdctx{nullptr};
90+
};
91+
92+
thread_local const Sha256Context sha256_context{};
93+
}
94+
1395
namespace ccf::crypto
1496
{
1597
namespace OpenSSL
@@ -55,52 +137,6 @@ namespace ccf::crypto
55137

56138
using namespace OpenSSL;
57139

58-
namespace
59-
{
60-
thread_local EVP_MD_CTX* mdctx = nullptr;
61-
thread_local EVP_MD_CTX* basectx = nullptr;
62-
}
63-
64-
void openssl_sha256_init()
65-
{
66-
if (mdctx != nullptr || basectx != nullptr)
67-
{
68-
return; // Already initialised
69-
}
70-
71-
mdctx = EVP_MD_CTX_new();
72-
if (mdctx == nullptr)
73-
{
74-
throw std::logic_error("openssl_sha256_init: failed to create mdctx");
75-
}
76-
basectx = EVP_MD_CTX_new();
77-
if (basectx == nullptr)
78-
{
79-
mdctx = nullptr;
80-
throw std::logic_error("openssl_sha256_init: failed to create basectx");
81-
}
82-
if (EVP_DigestInit_ex(basectx, EVP_sha256(), nullptr) != 1)
83-
{
84-
mdctx = nullptr;
85-
basectx = nullptr;
86-
throw std::logic_error("EVP_DigestInit_ex failed");
87-
}
88-
}
89-
90-
void openssl_sha256_shutdown()
91-
{
92-
if (mdctx != nullptr)
93-
{
94-
EVP_MD_CTX_free(mdctx);
95-
mdctx = nullptr;
96-
}
97-
if (basectx != nullptr)
98-
{
99-
EVP_MD_CTX_free(basectx);
100-
basectx = nullptr;
101-
}
102-
}
103-
104140
void openssl_sha256(const std::span<const uint8_t>& data, uint8_t* h)
105141
{
106142
// EVP_Digest calls are notoriously slow with OpenSSL 3.x (see
@@ -109,6 +145,9 @@ namespace ccf::crypto
109145
// and reusing them between calls. This is about 2x faster than EVP_Digest
110146
// for 128-byte buffers.
111147

148+
auto* const mdctx = sha256_context.get_mdctx();
149+
auto* const basectx = sha256_context.get_basectx();
150+
112151
if (mdctx == nullptr || basectx == nullptr)
113152
{
114153
throw std::logic_error(
@@ -120,11 +159,13 @@ namespace ccf::crypto
120159
{
121160
throw std::logic_error(fmt::format("EVP_MD_CTX_copy_ex failed: {}", rc));
122161
}
162+
123163
rc = EVP_DigestUpdate(mdctx, data.data(), data.size());
124164
if (rc != 1)
125165
{
126166
throw std::logic_error(fmt::format("EVP_DigestUpdate failed: {}", rc));
127167
}
168+
128169
rc = EVP_DigestFinal_ex(mdctx, h, nullptr);
129170
if (rc != 1)
130171
{

‎src/crypto/test/bench.cpp‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
#include "ccf/crypto/hash_provider.h"
77
#include "ccf/crypto/hmac.h"
88
#include "ccf/crypto/key_pair.h"
9-
#include "ccf/crypto/openssl_init.h"
109
#include "ccf/crypto/sha256.h"
1110
#include "ccf/crypto/symmetric_key.h"
1211
#include "crypto/openssl/base64.h"
@@ -343,8 +342,6 @@ namespace Hashes
343342
template <size_t size>
344343
static void sha256_bench(picobench::state& s)
345344
{
346-
ccf::crypto::openssl_sha256_init();
347-
348345
std::vector<uint8_t> v(size);
349346
for (size_t i = 0; i < size; ++i)
350347
{
@@ -359,7 +356,6 @@ static void sha256_bench(picobench::state& s)
359356
ccf::crypto::openssl_sha256(v, h.h.data());
360357
}
361358
s.stop_timer();
362-
ccf::crypto::openssl_sha256_shutdown();
363359
}
364360

365361
// Variant of the code above that uses the OpenSSL API

‎src/crypto/test/crypto.cpp‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
#include "ccf/crypto/jwk.h"
1010
#include "ccf/crypto/key_pair.h"
1111
#include "ccf/crypto/key_wrap.h"
12-
#include "ccf/crypto/openssl_init.h"
1312
#include "ccf/crypto/rsa_key_pair.h"
1413
#include "ccf/crypto/symmetric_key.h"
1514
#include "ccf/crypto/verifier.h"
@@ -1150,7 +1149,6 @@ TEST_CASE("PEM to JWK and back")
11501149

11511150
TEST_CASE("Incremental hash")
11521151
{
1153-
ccf::crypto::openssl_sha256_init();
11541152
auto simple_hash = ccf::crypto::Sha256Hash(contents);
11551153

11561154
INFO("Incremental hash");
@@ -1191,7 +1189,6 @@ TEST_CASE("Incremental hash")
11911189
REQUIRE_THROWS_AS(ihash->finalise(), std::logic_error);
11921190
}
11931191
}
1194-
ccf::crypto::openssl_sha256_shutdown();
11951192
}
11961193

11971194
TEST_CASE("Sign and verify with RSA key")

‎src/enclave/enclave.h‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
// Licensed under the Apache 2.0 License.
33
#pragma once
44
#include "ccf/app_interface.h"
5-
#include "ccf/crypto/openssl_init.h"
65
#include "ccf/ds/logger.h"
76
#include "ccf/js/core/context.h"
87
#include "ccf/node_context.h"
@@ -94,8 +93,6 @@ namespace ccf
9493
rpc_map(std::make_shared<RPCMap>()),
9594
rpcsessions(std::make_shared<RPCSessions>(*writer_factory, rpc_map))
9695
{
97-
ccf::crypto::openssl_sha256_init();
98-
9996
to_host = writer_factory->create_writer_to_outside();
10097

10198
LOG_TRACE_FMT("Creating ledger secrets");
@@ -183,7 +180,6 @@ namespace ccf
183180
~Enclave()
184181
{
185182
LOG_TRACE_FMT("Shutting down enclave");
186-
ccf::crypto::openssl_sha256_shutdown();
187183
}
188184

189185
CreateNodeStatus create_new_node(
@@ -237,7 +233,6 @@ namespace ccf
237233

238234
bool run_main()
239235
{
240-
ccf::crypto::openssl_sha256_init();
241236
LOG_DEBUG_FMT("Running main thread");
242237

243238
{
@@ -421,8 +416,6 @@ namespace ccf
421416
LOG_INFO_FMT("Enclave stopped successfully. Stopping host...");
422417
RINGBUFFER_WRITE_MESSAGE(AdminMessage::stopped, to_host);
423418

424-
ccf::crypto::openssl_sha256_shutdown();
425-
426419
return true;
427420
}
428421
}
@@ -439,7 +432,6 @@ namespace ccf
439432

440433
bool run_worker()
441434
{
442-
ccf::crypto::openssl_sha256_init();
443435
LOG_DEBUG_FMT("Running worker thread");
444436

445437
{
@@ -449,7 +441,6 @@ namespace ccf
449441
msg->data.tid, std::move(msg));
450442

451443
::threading::ThreadMessaging::instance().run();
452-
ccf::crypto::openssl_sha256_shutdown();
453444
}
454445

455446
return true;

‎src/host/run.cpp‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33

44
#include "ccf/run.h"
55

6-
#include "ccf/crypto/openssl_init.h"
76
#include "ccf/crypto/pem.h"
87
#include "ccf/crypto/symmetric_key.h"
98
#include "ccf/ds/logger.h"
@@ -112,8 +111,6 @@ namespace ccf
112111
return 1;
113112
}
114113

115-
ccf::crypto::openssl_sha256_init();
116-
117114
CLI::App app{
118115
"Run a single CCF node, based on the given configuration file.\n"
119116
"Some parameters are marked \"(security critical)\" - these must be "
@@ -1032,7 +1029,6 @@ namespace ccf
10321029
uv_walk(uv_default_loop(), cb, nullptr);
10331030
}
10341031
curl_global_cleanup();
1035-
ccf::crypto::openssl_sha256_shutdown();
10361032

10371033
return loop_close_rc;
10381034
}

‎src/host/test/ledger.cpp‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
// Licensed under the Apache 2.0 License.
33
#include "host/ledger.h"
44

5-
#include "ccf/crypto/openssl_init.h"
65
#include "ccf/crypto/sha256_hash.h"
76
#include "ccf/ds/logger.h"
87
#include "crypto/openssl/hash.h"
@@ -2029,11 +2028,9 @@ TEST_CASE("Ledger init with existing files")
20292028
int main(int argc, char** argv)
20302029
{
20312030
ccf::logger::config::default_init();
2032-
ccf::crypto::openssl_sha256_init();
20332031
doctest::Context context;
20342032
context.applyCommandLine(argc, argv);
20352033
int res = context.run();
2036-
ccf::crypto::openssl_sha256_shutdown();
20372034
if (context.shouldExit())
20382035
return res;
20392036
return res;

0 commit comments

Comments
 (0)