Skip to content

Replace WipeMode enum with SecretInputArray / SecretOutputArray subclasses - #549

Draft
WillChilds-Klein wants to merge 1 commit into
corretto:mainfrom
WillChilds-Klein:jbac-secret-array-subclasses
Draft

Replace WipeMode enum with SecretInputArray / SecretOutputArray subclasses#549
WillChilds-Klein wants to merge 1 commit into
corretto:mainfrom
WillChilds-Klein:jbac-secret-array-subclasses

Conversation

@WillChilds-Klein

@WillChilds-Klein WillChilds-Klein commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Status

Draft.

Summary

Follow-up to #547 per prasden's suggestion. Encodes wipe semantics in the type rather than a runtime WipeMode flag, so callers can't accidentally tag a secret with NO_WIPE.


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

…asses

Per follow-up agreed in PR 547 review. Encode wipe semantics in the
type rather than a runtime flag:

- JByteArrayCritical (base): non-sensitive critical region. No |len|
  parameter. Release does mode-0 ReleasePrimitiveArrayCritical. Use
  for salt, info, ciphertext, etc.

- SecretInputArray: sensitive read-only buffer (key, password,
  plaintext-in). Caller must not write through get(). On release, if
  the JVM returned a copy, OPENSSL_cleanse the native buffer and
  discard via JNI_ABORT; the Java array is preserved unchanged.

- SecretOutputArray: sensitive caller-writable buffer (derived key,
  plaintext-out). On release, if the JVM returned a copy, stash the
  caller's writes (allocation reserved in the ctor), cleanse the
  native buffer, JNI_ABORT, and SetByteArrayRegion to commit the
  stashed bytes back to the Java array. SetByteArrayRegion runs only
  after every critical region on the thread is closed; for multi-
  output scopes the caller drives release()/commitBack() manually.

Implementation uses a virtual doRelease() hook on the base; release()
itself stays in the base with idempotency and noexcept guarantees.
Subclass dtors run their own release() before chaining to the base
dtor so that virtual dispatch resolves to the derived override (the
base dtor would otherwise see a sliced vtable).

The thread-local debug critical-region counter and the assert in
SecretOutputArray::commitBack() are preserved. cleanse_and_stash
remains a static member of the base for use by csrc/test_buffer.cpp
unit tests.

Updates pbkdf2.cpp, hkdf.cpp, and aes_xts.cpp to use the new types.
GetArrayLength is no longer called for non-sensitive (base-class)
arrays at all; the secret subclasses take a |len| parameter that
callers must compute BEFORE constructing any JBAC subclass on the
thread.
@WillChilds-Klein
WillChilds-Klein force-pushed the jbac-secret-array-subclasses branch from b0bf28c to 3e3d7f8 Compare June 19, 2026 21:40
@codecov

codecov Bot commented Jun 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 72.72727% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.55%. Comparing base (2f28348) to head (3e3d7f8).

Files with missing lines Patch % Lines
csrc/buffer.cpp 51.61% 13 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main     #549      +/-   ##
============================================
- Coverage     78.58%   78.55%   -0.04%     
  Complexity     1211     1211              
============================================
  Files           127      127              
  Lines          7734     7737       +3     
  Branches        981      980       -1     
============================================
  Hits           6078     6078              
- Misses         1126     1130       +4     
+ Partials        530      529       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant