Skip to content

Implement external ownership for AES-GCM context - #8402

Merged
Amaury Chamayou (achamayou) merged 6 commits into
mainfrom
agents/reimplement-pr8178-with-benchmarks
Sep 21, 2026
Merged

Amaury Chamayou (achamayou) merged 6 commits into
mainfrom
agents/reimplement-pr8178-with-benchmarks

Conversation

@eddyashton

Copy link
Copy Markdown
Member

This pull request reimplements the changes from PR #8178 on the latest main branch, incorporating performance benchmarks and addressing unresolved feedback. Key updates include:

  • Added reusable KeyAesGcm::Context objects and a make_context() function.
  • Implemented pre-keyed OpenSSL encrypt/decrypt contexts while maintaining thread safety.
  • Enhanced LedgerSecret to use a reusable context with proper mutex handling.
  • Fixed issues with output-vector behavior for empty plaintext and decryption failures.
  • Introduced comprehensive tests for context lifetime, concurrency, and performance.
  • Added paired benchmarks for convenience vs. reused contexts, demonstrating significant performance improvements.
  • Updated the changelog to reflect these changes, with a note to append the new PR number upon merging.

Performance Highlights

  • Benchmarks show a reduction in operation time for both encryption and decryption using reused contexts, achieving speedups of approximately 4.6x for 64-byte and 3.4x for 1 KiB operations.

Validation

  • All tests passed successfully, including stress tests and clang-tidy checks.
  • An adversarial review confirmed no remaining issues, ensuring the implementation is robust and ready for production.

This PR is prepared for merging, pending the addition of the new PR number in the changelog.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Add the introducing PR reference to both new changelog entries before merging.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Implements reusable, thread-safe AES-GCM contexts with OpenSSL support, LedgerSecret integration, tests, and benchmarks.

Changes:

  • Adds externally owned, reusable encryption contexts.
  • Improves ledger encryption reuse and synchronization.
  • Adds lifecycle, concurrency, empty-input, and performance coverage.
  • Updates changelog documentation.
File summaries
File Summary
src/node/test/encryptor.cpp Adds encryption lifecycle and concurrency tests.
src/node/ledger_secrets.h Removes the obsolete key accessor.
src/node/ledger_secret.h Adds reusable context ownership and locking.
src/kv/encryptor.h Selects reusable or fresh contexts.
src/crypto/test/crypto.cpp Adds context, lifetime, and concurrency tests.
src/crypto/test/bench.cpp Adds AES-GCM performance benchmarks.
src/crypto/openssl/symmetric_key.h Declares OpenSSL context support.
src/crypto/openssl/symmetric_key.cpp Implements reusable contexts and buffer handling.
include/ccf/crypto/symmetric_key.h Defines the reusable context API.
CHANGELOG.md Documents the API and behavior changes.
Review details

Suppressed comments (2)

CHANGELOG.md:18

  • These new release-note entries are missing the introducing PR reference. CCF's changelog convention requires each added or modified entry to include (#<PR>) (as the existing entry below does), so append this PR's actual number to both entries before merging.
- C++ callers can use `ccf::crypto::KeyAesGcm::make_context()` to explicitly own and reuse a pre-keyed AES-GCM context when they can ensure it is not accessed concurrently.

### Changed

- AES-GCM encryption now replaces an existing output cipher with an empty cipher when encrypting empty plaintext, and AES-GCM decryption clears its plaintext output when authentication fails. Previously these calls could leave stale output from an earlier operation.

CHANGELOG.md:18

  • This new release-note entry is also missing the introducing pull-request reference required by the CCF changelog convention. Add the current PR number (#8402) before merging.
- AES-GCM encryption now replaces an existing output cipher with an empty cipher when encrypting empty plaintext, and AES-GCM decryption clears its plaintext output when authentication fails. Previously these calls could leave stale output from an earlier operation.
  • Files reviewed: 10/10 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread CHANGELOG.md Outdated
@github-actions

github-actions Bot commented Sep 18, 2026

Copy link
Copy Markdown
Description

Comparing 4 available runs from this branch (#8402) against the trend of the last 30 main runs.

Each chart plots every benchmark as an axis, with values normalized so 100 is the EWMA baseline of recent main runs, using a 7-run half-life. The 4 orange branch lines run from the oldest (faintest) to the latest (darkest and thickest); the darker blue band is the main baseline +/- 1 std dev and the lighter blue band around it is +/- 2 std dev.

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 main yet has no baseline of its own, so its earliest available run from this branch is used as its reference and its band is measured across this branch's runs. Its axis is normalized, scaled and coloured like any other, but the comparison is against this branch rather than against main.

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.30!important}
    .radarCurve-5{stroke-width:1.5px!important;stroke-opacity:0.40!important}
    .radarCurve-6{stroke-width:1.5px!important;stroke-opacity:0.50!important}
    .radarCurve-7{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:#E5484D!important}
    .radarAxisLabel:nth-of-type(4){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(5){fill:#808A94!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"
    cScale6: "#F97316"
    cScale7: "#F97316"
    radar:
      axisColor: "#9CA3AF"
      graticuleColor: "#E5E7EB"
      graticuleOpacity: 0
      axisStrokeWidth: 1
      curveOpacity: 0
---
radar-beta
  axis b0["Basic Blocking 100ms: 3,095 tx/s ▬ 0%"]
  axis b1["Basic Blocking 20ms: 15,348 tx/s ▬ 0%"]
  axis b2["Basic Blocking 2ms: 48,676 tx/s ▼ 6%"]
  axis b3["Basic JS: 16,126 tx/s ▬ +1%"]
  axis b4["Historical Queries: 929,698 tx/s ▬ -5%"]
  axis b5["L…g Certificate Blocking: 29,608 tx/s ▬ 0%"]
  axis b6["Logging JWT Blocking: 15,335 tx/s ▬ 0%"]
  curve stddev2_high["main EWMA + 2 std dev"]{100.33, 104.87, 106.99, 105.05, 112.31, 100.43, 100.27}
  curve stddev1_high["main EWMA + 1 std dev"]{100.17, 102.44, 103.50, 102.52, 106.16, 100.22, 100.13}
  curve stddev1_low["main EWMA - 1 std dev"]{99.83, 97.56, 96.50, 97.48, 93.84, 99.78, 99.87}
  curve stddev2_low["main EWMA - 2 std dev"]{99.67, 95.13, 93.01, 94.95, 87.69, 99.57, 99.73}
  curve branch_0["#8402 (3 runs earlier)"]{99.98, 100.31, 104.80, 104.93, 103.79, 100.19, 99.96}
  curve branch_1["#8402 (2 runs earlier)"]{99.84, 100.27, 100.69, 101.29, 102.04, 99.94, 100.16}
  curve branch_2["#8402 (1 run earlier)"]{99.90, 100.19, 92.17, 98.49, 89.11, 99.62, 99.87}
  curve branch_3["#8402"]{100.10, 100.18, 93.54, 100.60, 94.58, 100.37, 99.78}
  graticule polygon
  max 121
  min 79
  ticks 0
  showLegend false
Loading

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.30!important}
    .radarCurve-5{stroke-width:1.5px!important;stroke-opacity:0.40!important}
    .radarCurve-6{stroke-width:1.5px!important;stroke-opacity:0.50!important}
    .radarCurve-7{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:#E5484D!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"
    cScale6: "#F97316"
    cScale7: "#F97316"
    radar:
      axisColor: "#9CA3AF"
      graticuleColor: "#E5E7EB"
      graticuleOpacity: 0
      axisStrokeWidth: 1
      curveOpacity: 0
---
radar-beta
  axis b0["Basic Blocking 100ms: 99 ms ▬ 0%"]
  axis b1["Basic Blocking 20ms: 19 ms ▬ 0%"]
  axis b2["Basic Blocking 2ms: 5 ms ▬ 0%"]
  axis b3["Basic JS: 19 ms ▬ 0%"]
  axis b4["Historical Queries: 34 ms ▲ 11%"]
  axis b5["Logging Certificate Blocking: 19 ms ▬ 0%"]
  axis b6["Logging JWT Blocking: 19 ms ▬ 0%"]
  curve stddev2_high["main EWMA + 2 std dev"]{100.61, 100.00, 107.17, 105.90, 112.70, 100.00, 100.00}
  curve stddev1_high["main EWMA + 1 std dev"]{100.30, 100.00, 103.59, 102.95, 106.35, 100.00, 100.00}
  curve stddev1_low["main EWMA - 1 std dev"]{99.70, 100.00, 96.41, 97.05, 93.65, 100.00, 100.00}
  curve stddev2_low["main EWMA - 2 std dev"]{99.39, 100.00, 92.83, 94.10, 87.30, 100.00, 100.00}
  curve branch_0["#8402 (3 runs earlier)"]{100.04, 100.00, 99.87, 94.45, 91.52, 100.00, 100.00}
  curve branch_1["#8402 (2 runs earlier)"]{100.04, 100.00, 99.87, 99.70, 101.33, 100.00, 100.00}
  curve branch_2["#8402 (1 run earlier)"]{100.04, 100.00, 99.87, 99.70, 104.60, 100.00, 100.00}
  curve branch_3["#8402"]{100.04, 100.00, 99.87, 99.70, 111.14, 100.00, 100.00}
  graticule polygon
  max 122
  min 78
  ticks 0
  showLegend false
Loading

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.30!important}
    .radarCurve-5{stroke-width:1.5px!important;stroke-opacity:0.40!important}
    .radarCurve-6{stroke-width:1.5px!important;stroke-opacity:0.50!important}
    .radarCurve-7{stroke-width:1.75px!important;stroke-opacity:1.00!important}
    .radarAxisLabel:nth-of-type(1){fill:#2DA44E!important}
    .radarAxisLabel:nth-of-type(2){fill:#E5484D!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"
    cScale6: "#F97316"
    cScale7: "#F97316"
    radar:
      axisColor: "#9CA3AF"
      graticuleColor: "#E5E7EB"
      graticuleOpacity: 0
      axisStrokeWidth: 1
      curveOpacity: 0
---
radar-beta
  axis b0["Basic Blocking 100ms: 86.4 MiB ▼ 1%"]
  axis b1["Basic Blocking 20ms: 91.2 MiB ▲ 3%"]
  axis b2["Basic Blocking 2ms: 91.6 MiB ▬ +1%"]
  axis b3["Basic JS: 96.6 MiB ▬ 0%"]
  axis b4["Historical Queries: 148 MiB ▼ 1%"]
  axis b5["Logging Certificate Blocking: 113 MiB ▬ -1%"]
  axis b6["Logging JWT Blocking: 85.9 MiB ▬ -1%"]
  curve stddev2_high["main EWMA + 2 std dev"]{102.25, 103.32, 102.46, 101.54, 101.57, 102.27, 102.82}
  curve stddev1_high["main EWMA + 1 std dev"]{101.12, 101.66, 101.23, 100.77, 100.78, 101.13, 101.41}
  curve stddev1_low["main EWMA - 1 std dev"]{98.88, 98.34, 98.77, 99.23, 99.22, 98.87, 98.59}
  curve stddev2_low["main EWMA - 2 std dev"]{97.75, 96.68, 97.54, 98.46, 98.43, 97.73, 97.18}
  curve branch_0["#8402 (3 runs earlier)"]{101.00, 98.81, 100.70, 100.87, 100.54, 98.46, 97.55}
  curve branch_1["#8402 (2 runs earlier)"]{98.79, 100.71, 97.89, 99.97, 99.40, 101.15, 99.13}
  curve branch_2["#8402 (1 run earlier)"]{101.41, 101.99, 102.39, 99.60, 100.10, 100.38, 101.23}
  curve branch_3["#8402"]{98.59, 103.42, 101.21, 100.44, 99.06, 99.38, 99.26}
  graticule polygon
  max 108
  min 92
  ticks 0
  showLegend false
Loading

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.30!important}
    .radarCurve-5{stroke-width:1.5px!important;stroke-opacity:0.40!important}
    .radarCurve-6{stroke-width:1.5px!important;stroke-opacity:0.50!important}
    .radarCurve-7{stroke-width:1.75px!important;stroke-opacity:1.00!important}
    .radarAxisLabel:nth-of-type(1){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(2){fill:#2DA44E!important}
    .radarAxisLabel:nth-of-type(3){fill:#2DA44E!important}
    .radarAxisLabel:nth-of-type(4){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(5){fill:#2DA44E!important}
    .radarAxisLabel:nth-of-type(6){fill:#2DA44E!important}
    .radarAxisLabel:nth-of-type(7){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(8){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(9){fill:#808A94!important}
  themeVariables:
    cScale0: "#62B5E5"
    cScale1: "#62B5E5"
    cScale2: "#62B5E5"
    cScale3: "#62B5E5"
    cScale4: "#F97316"
    cScale5: "#F97316"
    cScale6: "#F97316"
    cScale7: "#F97316"
    radar:
      axisColor: "#9CA3AF"
      graticuleColor: "#E5E7EB"
      graticuleOpacity: 0
      axisStrokeWidth: 1
      curveOpacity: 0
---
radar-beta
  axis b0["CCF c…n c…t lifecycle: 21,243 ops/s ▬ +2%"]
  axis b1["CCF fresh JS invocation: 19,689 ops/s ▲ 3%"]
  axis b2["CHAMP get: 65,249,944 ops/s ▲ 4%"]
  axis b3["CHAMP put: 8,086,009 ops/s ▬ +1%"]
  axis b4["KV deserialisation: 2,750,275 ops/s ▲ 15%"]
  axis b5["KV serialisation: 2,465,483 ops/s ▲ 21%"]
  axis b6["KV s…t deserialisation: 6,096 ops/s ▬ -1%"]
  axis b7["KV snapshot serialisation: 4,708 ops/s ▬ -1%"]
  axis b8["Q…S s…d c…t lifecycle: 26,064 ops/s ▬ +2%"]
  curve stddev2_high["main EWMA + 2 std dev"]{104.67, 104.99, 104.19, 105.10, 106.09, 106.41, 104.84, 105.78, 104.70}
  curve stddev1_high["main EWMA + 1 std dev"]{102.33, 102.49, 102.09, 102.55, 103.04, 103.21, 102.42, 102.89, 102.35}
  curve stddev1_low["main EWMA - 1 std dev"]{97.67, 97.51, 97.91, 97.45, 96.96, 96.79, 97.58, 97.11, 97.65}
  curve stddev2_low["main EWMA - 2 std dev"]{95.33, 95.01, 95.81, 94.90, 93.91, 93.59, 95.16, 94.22, 95.30}
  curve branch_0["#8402 (3 runs earlier)"]{101.47, 101.89, 101.88, 104.70, 116.74, 123.46, 102.18, 101.74, 101.80}
  curve branch_1["#8402 (2 runs earlier)"]{104.02, 103.73, 101.09, 102.68, 117.06, 125.03, 100.80, 97.94, 103.70}
  curve branch_2["#8402 (1 run earlier)"]{102.52, 102.59, 102.46, 104.13, 118.75, 119.83, 100.62, 98.64, 102.85}
  curve branch_3["#8402"]{101.60, 102.88, 103.61, 101.46, 115.45, 120.72, 98.67, 99.11, 101.85}
  graticule polygon
  max 137
  min 82
  ticks 0
  showLegend false
Loading

Comment thread src/crypto/openssl/symmetric_key.cpp
@achamayou
Amaury Chamayou (achamayou) merged commit 7d97443 into main Sep 21, 2026
10 checks passed
@achamayou
Amaury Chamayou (achamayou) deleted the agents/reimplement-pr8178-with-benchmarks branch September 21, 2026 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants