Implement external ownership for AES-GCM context - #8178
Implement external ownership for AES-GCM context#8178Eddy Ashton (eddyashton) wants to merge 5 commits into
Conversation
Co-authored-by: eddyashton <6000239+eddyashton@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR extends CCF’s AES-GCM symmetric key abstraction to support externally owned, reusable “pre-keyed” cipher contexts (KeyAesGcm::Context via make_context()), and wires this into ledger encryption by giving each LedgerSecret a reusable context protected by a mutex. It also adds new concurrency/behavior tests and a microbenchmark to validate and measure the new approach.
Changes:
- Introduce
ccf::crypto::KeyAesGcm::Contextandmake_context()for reusable, non-concurrently-used AES-GCM contexts. - Update the OpenSSL AES-GCM implementation to create and reuse pre-keyed
EVP_CIPHER_CTXinstances. - Update ledger encryption to use
LedgerSecret-owned reusable contexts, plus add concurrent tests and an AES-GCM benchmark.
Custom instructions used:
.github/copilot-instructions.md.github/instructions/reviewing.instructions.md
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/node/test/encryptor.cpp | Adds a concurrent encrypt/decrypt test and strengthens rollback lifetime checks. |
| src/node/ledger_secret.h | Adds per-LedgerSecret reusable AES-GCM context with mutex-guarded encrypt/decrypt helpers. |
| src/kv/encryptor.h | Switches to retrieving LedgerSecret and calling its encrypt/decrypt wrappers. |
| src/crypto/test/crypto.cpp | Adds AES-GCM context-reuse, empty-input, and concurrency tests. |
| src/crypto/test/bench.cpp | Adds AES-GCM encryption microbench using a reusable context. |
| src/crypto/openssl/symmetric_key.h | Adds make_context() to the OpenSSL-backed AES-GCM key and moves cleansing to out-of-line dtor. |
| src/crypto/openssl/symmetric_key.cpp | Refactors AES-GCM encrypt/decrypt around reusable pre-keyed contexts. |
| include/ccf/crypto/symmetric_key.h | Extends public API with KeyAesGcm::Context and make_context(). |
| CHANGELOG.md | Documents the new public API for reusable AES-GCM contexts. |
Suppressed comments (4)
src/crypto/openssl/symmetric_key.cpp:152
- On authentication failure, decrypt_with_context() returns false without clearing the output buffer. Callers that reuse the plain vector can accidentally observe stale plaintext from a previous successful decrypt.
if (EVP_DecryptFinal_ex(ctx, nullptr, &final_outl) != 1)
{
return false;
}
src/crypto/openssl/symmetric_key.cpp:165
- decrypt_with_context() only assigns to the output plain when cipher is non-empty. For valid AAD-only messages (empty cipher), the function returns true but leaves plain unchanged, so callers reusing the vector may see stale data.
if (!cipher.empty())
{
plain = std::move(plaintext);
}
src/crypto/openssl/symmetric_key.cpp:157
- Typo in comment: should refer to GCM (not GSM) and fix grammar (“As long as …”).
// As long a we use GSM cipher, the final outl must be 0, because there's
// no padding and the block size is equal to 1, so EncryptUpdate() always
// does the whole thing. Final is still a must to finalize and check the
// error.
src/crypto/test/crypto.cpp:921
- Similarly, this empty-cipher decrypt assertion seeds decrypted as empty, so it won’t catch a bug where decrypt() returns true but leaves stale data in the output buffer. Seed the output with non-empty data first.
decrypted.clear();
REQUIRE(empty_aes_gcm_key->decrypt(iv, empty_tag, {}, {}, decrypted));
REQUIRE(decrypted.empty());
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…m-contexts' into agents/context-object-ownership-implementation
7b91ee1 to
9d78605
Compare
DescriptionComparing 2 available runs from this branch (#8178) against the trend of the last 30 Each chart plots every benchmark as an axis, with values normalized so 100 is the EWMA baseline of recent Axis labels show the latest branch value and its difference from the main EWMA baseline, where 0% is on the baseline. They are coloured green where the latest run improves on the baseline, red where it regresses, and grey where the difference is within one std dev of the baseline (within noise). Higher is better for throughput and rate, lower for latency and memory. A benchmark which does not exist on Throughput (tx/s)---
config:
radar:
width: 620
height: 620
marginTop: 90
marginRight: 220
marginBottom: 60
marginLeft: 220
axisLabelFactor: 1.12
curveTension: 0.08
theme: base
themeCSS: |
.radarCurve-0{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
.radarCurve-1{fill:color-mix(in srgb, #62B5E5 40%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
.radarCurve-2{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
.radarCurve-3{fill:var(--color-canvas-default,var(--bgColor-default,#fff))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
.radarAxisLabel,.radarTitle{fill:var(--color-fg-default,var(--fgColor-default,#111827))!important;color:var(--color-fg-default,var(--fgColor-default,#111827))!important}
.radarCurve-4{stroke-width:1.5px!important;stroke-opacity:0.50!important}
.radarCurve-5{stroke-width:1.75px!important;stroke-opacity:1.00!important}
.radarAxisLabel:nth-of-type(1){fill:#2DA44E!important}
.radarAxisLabel:nth-of-type(2){fill:#808A94!important}
.radarAxisLabel:nth-of-type(3){fill:#808A94!important}
.radarAxisLabel:nth-of-type(4){fill:#E5484D!important}
.radarAxisLabel:nth-of-type(5){fill:#808A94!important}
.radarAxisLabel:nth-of-type(6){fill:#E5484D!important}
.radarAxisLabel:nth-of-type(7){fill:#808A94!important}
.radarAxisLabel:nth-of-type(8){fill:#2DA44E!important}
.radarAxisLabel:nth-of-type(9){fill:#E5484D!important}
themeVariables:
cScale0: "#62B5E5"
cScale1: "#62B5E5"
cScale2: "#62B5E5"
cScale3: "#62B5E5"
cScale4: "#F97316"
cScale5: "#F97316"
radar:
axisColor: "#9CA3AF"
graticuleColor: "#E5E7EB"
graticuleOpacity: 0
axisStrokeWidth: 1
curveOpacity: 0
---
radar-beta
axis b0["Basic: 73,912 tx/s ▲ 5%"]
axis b2["Basic Blocking Locust 100ms: 3,057 tx/s ▬ 0%"]
axis b3["Basic Blocking Locust 20ms: 15,159 tx/s ▬ 0%"]
axis b4["Basic Blocking Locust 2ms: 30,355 tx/s ▼ 5%"]
axis b5["Basic JS: 4,872 tx/s ▬ +3%"]
axis b6["Basic Multi-Threaded: 83,768 tx/s ▼ 4%"]
axis b7["Historical Queries: 200,320 tx/s ▬ -4%"]
axis b8["Logging: 68,953 tx/s ▲ 4%"]
axis b10["L…g JWT Blocking Locust: 14,568 tx/s ▼ 1%"]
curve stddev2_high["main EWMA + 2 std dev"]{107.51, 100.59, 100.68, 109.19, 107.14, 106.81, 111.50, 108.56, 101.60}
curve stddev1_high["main EWMA + 1 std dev"]{103.76, 100.29, 100.34, 104.60, 103.57, 103.40, 105.75, 104.28, 100.80}
curve stddev1_low["main EWMA - 1 std dev"]{96.24, 99.71, 99.66, 95.40, 96.43, 96.60, 94.25, 95.72, 99.20}
curve stddev2_low["main EWMA - 2 std dev"]{92.49, 99.41, 99.32, 90.81, 92.86, 93.19, 88.50, 91.44, 98.40}
curve branch_0["#8178 (1 run earlier)"]{108.33, 99.90, 99.96, 94.86, 102.67, 100.99, 99.10, 107.33, 101.24}
curve branch_1["#8178"]{104.70, 100.10, 100.01, 94.78, 102.73, 96.47, 95.94, 104.31, 99.08}
graticule polygon
max 120
min 80
ticks 0
showLegend false
Latency (ms)---
config:
radar:
width: 620
height: 620
marginTop: 90
marginRight: 220
marginBottom: 60
marginLeft: 220
axisLabelFactor: 1.12
curveTension: 0.08
theme: base
themeCSS: |
.radarCurve-0{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
.radarCurve-1{fill:color-mix(in srgb, #62B5E5 40%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
.radarCurve-2{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
.radarCurve-3{fill:var(--color-canvas-default,var(--bgColor-default,#fff))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
.radarAxisLabel,.radarTitle{fill:var(--color-fg-default,var(--fgColor-default,#111827))!important;color:var(--color-fg-default,var(--fgColor-default,#111827))!important}
.radarCurve-4{stroke-width:1.5px!important;stroke-opacity:0.50!important}
.radarCurve-5{stroke-width:1.75px!important;stroke-opacity:1.00!important}
.radarAxisLabel:nth-of-type(1){fill:#808A94!important}
.radarAxisLabel:nth-of-type(2){fill:#808A94!important}
.radarAxisLabel:nth-of-type(3){fill:#808A94!important}
.radarAxisLabel:nth-of-type(4){fill:#808A94!important}
.radarAxisLabel:nth-of-type(5){fill:#2DA44E!important}
.radarAxisLabel:nth-of-type(6){fill:#808A94!important}
.radarAxisLabel:nth-of-type(7){fill:#808A94!important}
themeVariables:
cScale0: "#62B5E5"
cScale1: "#62B5E5"
cScale2: "#62B5E5"
cScale3: "#62B5E5"
cScale4: "#F97316"
cScale5: "#F97316"
radar:
axisColor: "#9CA3AF"
graticuleColor: "#E5E7EB"
graticuleOpacity: 0
axisStrokeWidth: 1
curveOpacity: 0
---
radar-beta
axis b0["Basic Blocking Locust 100ms: 99 ms ▬ 0%"]
axis b1["Basic Blocking Locust 20ms: 19 ms ▬ 0%"]
axis b2["Basic Blocking Locust 2ms: 9 ms ▬ 0%"]
axis b3["Commit Latency 16ms: 5.07 ms ▬ +9%"]
axis b4["Commit Latency 1ms: 1.89 ms ▼ 3%"]
axis b5["Commit Latency 256ms: 214 ms ▬ 0%"]
axis b6["Logging JWT Blocking Locust: 20 ms ▬ 0%"]
curve stddev2_high["main EWMA + 2 std dev"]{100.00, 100.00, 108.27, 152.75, 104.41, 102.84, 100.00}
curve stddev1_high["main EWMA + 1 std dev"]{100.00, 100.00, 104.13, 126.38, 102.21, 101.42, 100.00}
curve stddev1_low["main EWMA - 1 std dev"]{100.00, 100.00, 95.87, 73.62, 97.79, 98.58, 100.00}
curve stddev2_low["main EWMA - 2 std dev"]{100.00, 100.00, 91.73, 47.25, 95.59, 97.16, 100.00}
curve branch_0["#8178 (1 run earlier)"]{100.00, 100.00, 100.19, 138.10, 99.31, 100.10, 100.00}
curve branch_1["#8178"]{100.00, 100.00, 100.19, 109.13, 96.98, 100.47, 100.00}
graticule polygon
max 190
min 10
ticks 0
showLegend false
Memory (bytes)---
config:
radar:
width: 620
height: 620
marginTop: 90
marginRight: 220
marginBottom: 60
marginLeft: 220
axisLabelFactor: 1.12
curveTension: 0.08
theme: base
themeCSS: |
.radarCurve-0{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
.radarCurve-1{fill:color-mix(in srgb, #62B5E5 40%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
.radarCurve-2{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
.radarCurve-3{fill:var(--color-canvas-default,var(--bgColor-default,#fff))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
.radarAxisLabel,.radarTitle{fill:var(--color-fg-default,var(--fgColor-default,#111827))!important;color:var(--color-fg-default,var(--fgColor-default,#111827))!important}
.radarCurve-4{stroke-width:1.5px!important;stroke-opacity:0.50!important}
.radarCurve-5{stroke-width:1.75px!important;stroke-opacity:1.00!important}
.radarAxisLabel:nth-of-type(1){fill:#808A94!important}
.radarAxisLabel:nth-of-type(2){fill:#808A94!important}
.radarAxisLabel:nth-of-type(3){fill:#808A94!important}
.radarAxisLabel:nth-of-type(4){fill:#808A94!important}
.radarAxisLabel:nth-of-type(5){fill:#808A94!important}
.radarAxisLabel:nth-of-type(6){fill:#E5484D!important}
.radarAxisLabel:nth-of-type(7){fill:#2DA44E!important}
.radarAxisLabel:nth-of-type(8){fill:#E5484D!important}
themeVariables:
cScale0: "#62B5E5"
cScale1: "#62B5E5"
cScale2: "#62B5E5"
cScale3: "#62B5E5"
cScale4: "#F97316"
cScale5: "#F97316"
radar:
axisColor: "#9CA3AF"
graticuleColor: "#E5E7EB"
graticuleOpacity: 0
axisStrokeWidth: 1
curveOpacity: 0
---
radar-beta
axis b0["Basic: 90 MiB ▬ +2%"]
axis b2["Basic Blocking Locust 100ms: 90.6 MiB ▬ 0%"]
axis b3["Basic Blocking Locust 20ms: 90.6 MiB ▬ 0%"]
axis b4["Basic Blocking Locust 2ms: 93.9 MiB ▬ 0%"]
axis b5["Basic JS: 69.7 MiB ▬ -1%"]
axis b6["Basic Multi-Threaded: 90.4 MiB ▲ 1%"]
axis b7["Logging: 75.4 MiB ▼ 1%"]
axis b9["Logging JWT Blocking Locust: 93.3 MiB ▲ 1%"]
curve stddev2_high["main EWMA + 2 std dev"]{104.24, 102.15, 101.64, 101.83, 103.83, 102.57, 101.43, 102.24}
curve stddev1_high["main EWMA + 1 std dev"]{102.12, 101.07, 100.82, 100.91, 101.91, 101.29, 100.72, 101.12}
curve stddev1_low["main EWMA - 1 std dev"]{97.88, 98.93, 99.18, 99.09, 98.09, 98.71, 99.28, 98.88}
curve stddev2_low["main EWMA - 2 std dev"]{95.76, 97.85, 98.36, 98.17, 96.17, 97.43, 98.57, 97.76}
curve branch_0["#8178 (1 run earlier)"]{97.91, 100.55, 100.67, 100.90, 100.52, 98.14, 100.77, 98.94}
curve branch_1["#8178"]{101.94, 100.10, 99.73, 100.30, 98.55, 101.40, 98.91, 101.21}
graticule polygon
max 109
min 91
ticks 0
showLegend false
Rate (ops/s)---
config:
radar:
width: 620
height: 620
marginTop: 90
marginRight: 220
marginBottom: 60
marginLeft: 220
axisLabelFactor: 1.12
curveTension: 0.08
theme: base
themeCSS: |
.radarCurve-0{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
.radarCurve-1{fill:color-mix(in srgb, #62B5E5 40%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
.radarCurve-2{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
.radarCurve-3{fill:var(--color-canvas-default,var(--bgColor-default,#fff))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
.radarAxisLabel,.radarTitle{fill:var(--color-fg-default,var(--fgColor-default,#111827))!important;color:var(--color-fg-default,var(--fgColor-default,#111827))!important}
.radarCurve-4{stroke-width:1.5px!important;stroke-opacity:0.50!important}
.radarCurve-5{stroke-width:1.75px!important;stroke-opacity:1.00!important}
.radarAxisLabel:nth-of-type(1){fill:#808A94!important}
.radarAxisLabel:nth-of-type(2){fill:#808A94!important}
.radarAxisLabel:nth-of-type(3){fill:#2DA44E!important}
.radarAxisLabel:nth-of-type(4){fill:#2DA44E!important}
.radarAxisLabel:nth-of-type(5){fill:#808A94!important}
.radarAxisLabel:nth-of-type(6){fill:#808A94!important}
themeVariables:
cScale0: "#62B5E5"
cScale1: "#62B5E5"
cScale2: "#62B5E5"
cScale3: "#62B5E5"
cScale4: "#F97316"
cScale5: "#F97316"
radar:
axisColor: "#9CA3AF"
graticuleColor: "#E5E7EB"
graticuleOpacity: 0
axisStrokeWidth: 1
curveOpacity: 0
---
radar-beta
axis b0["CHAMP get: 38,188,294 ops/s ▬ +1%"]
axis b1["CHAMP put: 5,452,812 ops/s ▬ 0%"]
axis b2["KV deserialisation: 1,920,123 ops/s ▲ 18%"]
axis b3["KV serialisation: 1,645,007 ops/s ▲ 14%"]
axis b4["KV s…t deserialisation: 4,220 ops/s ▬ +2%"]
axis b5["KV snapshot serialisation: 4,248 ops/s ▬ -5%"]
curve stddev2_high["main EWMA + 2 std dev"]{105.58, 106.40, 106.20, 107.16, 105.65, 111.89}
curve stddev1_high["main EWMA + 1 std dev"]{102.79, 103.20, 103.10, 103.58, 102.83, 105.94}
curve stddev1_low["main EWMA - 1 std dev"]{97.21, 96.80, 96.90, 96.42, 97.17, 94.06}
curve stddev2_low["main EWMA - 2 std dev"]{94.42, 93.60, 93.80, 92.84, 94.35, 88.11}
curve branch_0["#8178 (1 run earlier)"]{100.42, 100.67, 117.43, 114.65, 101.65, 97.68}
curve branch_1["#8178"]{100.87, 100.36, 118.33, 114.46, 101.55, 95.49}
graticule polygon
max 129
min 77
ticks 0
showLegend false
|
This pull request implements an alternative approach to the context object ownership as discussed in #8170. The key changes include:
KeyAesGcm::Contextthat can be created throughmake_context(), allowing for faster encryption and decryption calls.KeyAesGcm::encrypt/decryptcalls.LedgerSecretnow owns a reusable context, synchronized with a mutex to ensure thread safety.EVP_aes_*_gcm()calls for improved performance.Validation
This implementation enhances the performance and usability of the AES-GCM encryption while ensuring thread safety and compatibility with existing APIs.