Skip to content

Make secret-key cleanup and replacement exception-safe - #29

Open
AKG451 wants to merge 6 commits into
open-quantum-safe:mainfrom
AKG451:fix/cleanse-secret-key-lifetime
Open

AKG451 wants to merge 6 commits into
open-quantum-safe:mainfrom
AKG451:fix/cleanse-secret-key-lifetime

Conversation

@AKG451

@AKG451 AKG451 commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Secret-key storage in KeyEncapsulation and Signature could be released without cleansing during assignment, regeneration, or failed construction. The move operations also copied secret vectors inside noexcept functions, potentially terminating the process on allocation failure.

This PR updates both wrappers to:

  • Cleanse the constructor’s by-value secret-key parameter when initialization fails.
  • Complete metadata copying before copying secret bytes during copy construction.
  • Use copy-and-swap assignment to preserve the destination on failure and cleanse its previous key after successful replacement.
  • Transfer secret-key storage without allocation during moves.
  • Generate replacement keys in guarded temporary storage, preserving the existing key on failure and cleansing temporary or replaced secrets.
  • Check backend creation results before dereferencing them.

The added lifecycle tests cover copying, moving, self-assignment, empty-key assignment, regeneration, and assignment across algorithms when the selected alternatives are enabled. They check that resulting keys remain usable for encapsulation/decapsulation or signing/verification.

Moves now transfer ownership of the existing allocation rather than copying and wiping the source allocation. The destination becomes responsible for cleansing that storage. Private member order also changes, so consumers should rebuild.

Validation completed:

  • C++11 syntax/type checks using dependency stubs.
  • Supplemental mock-backend allocation-failure and cleansing tests.
  • ASan/UBSan checks of that harness, with LeakSanitizer disabled due to environment limitations.

AKG451 and others added 4 commits November 7, 2026 03:07
Secret keys were moved into wrapper objects before algorithm validation, so constructor exceptions could leave caller-provided key material uncleansed. Existing keys were also overwritten during assignment and key regeneration without cleansing the old bytes first.\n\nRetain constructor ownership until validation and metadata initialization complete, cleanse keys before copy/move assignment and regeneration, and add lifecycle coverage for both KEM and signature wrappers.

Signed-off-by: AKG451 <217809809+AKG451@users.noreply.github.com>
Refactor secret key management for KeyEncapsulation and Signature classes. Introduce SecretKeyGuard for automatic cleansing of secret keys upon destruction.

Signed-off-by: Aryan K. Gupta <akg451154@gmail.com>
Refactor KEM tests to use TestSecret for memory management and improve thread safety.

Signed-off-by: Aryan K. Gupta <akg451154@gmail.com>
Signed-off-by: Aryan K. Gupta <akg451154@gmail.com>
@AKG451 AKG451 closed this Nov 6, 2026
@AKG451 AKG451 reopened this Nov 6, 2026
Refactor KEM tests to improve clarity and correctness. Removed TestSecret class usage in favor of direct comparison of shared secrets. Added NotSupported test case for unsupported KEMs.

Signed-off-by: Aryan K. Gupta <akg451154@gmail.com>
Refactor signature tests to improve readability and error handling. Update verification checks to log failures and ensure proper thread handling.

Signed-off-by: Aryan K. Gupta <akg451154@gmail.com>
@AKG451 AKG451 changed the title Cleanse secret keys before replacement Make secret-key cleanup and replacement exception-safe Nov 6, 2026
@AKG451

AKG451 commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor Author

@dstebila @baentsch The Windows CI job is failing while compiling liboqs’s SQIsign code:

liboqs/src/sig/sqisign/the-sqisign_sqisign_common/sign.c
error C2065: 'ssize_t': undeclared identifier

This appears unrelated to the files changed in this PR, although I haven’t confirmed the same failure on the base branch. Could you please take a look when convenient? Thank you.

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