Skip to content

Cleanse sensitive key material in freed memory#30729

Open
cyberkittens4u wants to merge 1 commit intoopenssl:masterfrom
cyberkittens4u:fix/evp-cipher-ctx-cleanse
Open

Cleanse sensitive key material in freed memory#30729
cyberkittens4u wants to merge 1 commit intoopenssl:masterfrom
cyberkittens4u:fix/evp-cipher-ctx-cleanse

Conversation

@cyberkittens4u
Copy link
Copy Markdown

4 fixes ensuring sensitive cryptographic material is zeroed before freeing:

  • bn_blind.c: BN_freeBN_clear_free for RSA blinding factors A and Ai, OPENSSL_freeOPENSSL_clear_free for BN_BLINDING struct
  • ecdh_ossl.c: OPENSSL_freeOPENSSL_clear_free on shared secret buffer in error path
  • cms_kari.c: OPENSSL_cleanse full kek buffer (sizeof(kek)) instead of only keklen bytes
  • statem_srvr.c + statem_clnt.c: add OPENSSL_cleanse for sctpauthkey stack buffers after BIO_ctrl (3 instances)

These reduce the blast radius of memory disclosure vulnerabilities in applications using OpenSSL, by ensuring key material does not persist in freed heap or stack frames.

@openssl-machine openssl-machine added the hold: cla required The contributor needs to submit a license agreement label Apr 8, 2026
@t8m
Copy link
Copy Markdown
Member

t8m commented Apr 8, 2026

@cyberkittens4u Please fix the CI failure and add CLA: trivial to the commit message on a separate line in the commit message body.

This would be OK with CLA: trivial.

@t8m t8m added approval: review pending This pull request needs review by a committer triaged: feature The issue/pr requests/adds a feature tests: exempted The PR is exempt from requirements for testing branch: master Applies to master branch labels Apr 8, 2026
@cyberkittens4u cyberkittens4u force-pushed the fix/evp-cipher-ctx-cleanse branch from 5a8cdba to fd15f38 Compare April 8, 2026 13:05
@openssl-machine openssl-machine removed the hold: cla required The contributor needs to submit a license agreement label Apr 8, 2026
Comment thread crypto/bn/bn_blind.c Outdated
Comment thread crypto/cms/cms_kari.c Outdated
Comment thread crypto/ec/ecdh_ossl.c
BN_CTX_end(ctx);
BN_CTX_free(ctx);
OPENSSL_free(buf);
OPENSSL_clear_free(buf, buflen);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If you are going to do this here, then buflen needs to be initialized, since there are early exit paths that goto err.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Sorry, the kittens got overexcited with their very first commit ever and knocked the vase off the table :)

You're right on all three points and we appreciate the thorough review:

  1. bn_blind.c — struct only holds pointers and metadata after A/Ai are cleared. Dropped the OPENSSL_clear_free, keeping BN_clear_free on A and Ai only.

  2. cms_kari.c — only keklen bytes were written by EVP_DecryptUpdate, bytes beyond that are uninitialized stack. Reverted entirely.

  3. ecdh_ossl.c — good catch on the uninitialized buflen on early exit paths. Initialized to 0 at declaration.

Compiled locally this time before pushing. Lesson learned.

- bn_blind.c: BN_free -> BN_clear_free for RSA blinding factors A and Ai
- ecdh_ossl.c: OPENSSL_free -> OPENSSL_clear_free on shared secret
  buffer in error path, initialize buflen to 0 for early exit safety
- statem_srvr.c, statem_clnt.c: add OPENSSL_cleanse for sctpauthkey
  stack buffers after BIO_ctrl (3 instances)

These reduce the blast radius of memory disclosure vulnerabilities
in applications using OpenSSL, by ensuring key material does not
persist in freed heap or stack frames.

CLA: trivial

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@cyberkittens4u cyberkittens4u force-pushed the fix/evp-cipher-ctx-cleanse branch from fd15f38 to 0a1e405 Compare April 9, 2026 02:14
@github-actions github-actions Bot added the severity: fips change The pull request changes FIPS provider sources label Apr 9, 2026
@cyberkittens4u
Copy link
Copy Markdown
Author

Shane, sorry for the notification delay — all three issues are fixed (compiles locally).

These PRs are essentially cleanup to reduce blast radius if a third-party service using OpenSSL were ever compromised.

— cyberkittens 🐾

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

Labels

approval: review pending This pull request needs review by a committer branch: master Applies to master branch severity: fips change The pull request changes FIPS provider sources tests: exempted The PR is exempt from requirements for testing triaged: feature The issue/pr requests/adds a feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants