Harden OpenSSL crypto error checking and resource management by Copilot · Pull Request #7817 · microsoft/CCF

Copilot · 2026-04-14T12:44:43Z

Harden OpenSSL error checking and resource management in ccf::crypto:

CHECK1 (openssl_wrappers.h): Threw only when error queue was non-empty. Now throws unconditionally on rc != 1; error message includes rc, ec, and best-effort error string.
CHECK0 removed: Ambiguous semantics replaced by CHECKPOSITIVE(). Removal noted in CHANGELOG (Harden OpenSSL crypto error checking and resource management #7817).
CHECKPOSITIVE (new): For OpenSSL APIs returning positive-on-success. Includes ERR_get_error() / error_string() and rc/ec in diagnostics.
RSA OAEP params (rsa_key_pair.cpp, rsa_public_key.cpp): EVP_PKEY_CTX_set_rsa_padding, _set_rsa_oaep_md, _set_rsa_mgf1_md return values were unchecked. Wrapped with CHECKPOSITIVE().
HKDF context leak (hash.cpp): Raw EVP_PKEY_CTX* leaked on throw. Replaced with Unique_EVP_PKEY_CTX RAII wrapper.
Wrong check macro (ec_key_pair.cpp): X509_REQ_sign returns signature size (positive) on success, 0 on failure — corrected from CHECK1 to CHECKPOSITIVE.
salt_length overflow (rsa_key_pair.cpp, rsa_public_key.cpp): size_t salt_length was passed directly to EVP_PKEY_CTX_set_rsa_pss_saltlen(int) without range checking. Added validation throwing std::invalid_argument if salt_length > INT_MAX.

OpenSSL doc links for CHECK1 → CHECKPOSITIVE replacements:

EVP_PKEY_CTX_set_rsa_padding / set_rsa_oaep_md / set_rsa_mgf1_md / set_rsa_pss_saltlen / set_signature_md
"return a positive value for success and 0 or a negative value for failure"
https://docs.openssl.org/master/man3/EVP_PKEY_CTX_ctrl/#return-values

EVP_PKEY_CTX_set_hkdf_md / set1_hkdf_salt / set1_hkdf_key / add1_hkdf_info
Same page as above — all are EVP_PKEY_CTX_ctrl wrappers
https://docs.openssl.org/master/man3/EVP_PKEY_CTX_ctrl/#return-values

sk_X509_push (→ OPENSSL_sk_push)
"returns the total number of elements in the stack after the push, or 0 on error"
https://docs.openssl.org/master/man3/OPENSSL_sk_push/#return-values

X509_REQ_sign
"returns the size of the signature in bytes [...] or 0 on failure"
https://docs.openssl.org/master/man3/X509_sign/#return-values

i2d_ECDSA_SIG
"returns the length of the DER encoded signature [...] or negative value on error"
https://docs.openssl.org/master/man3/i2d_ECDSA_SIG/#return-values

…CK1 macro Agent-Logs-Url: https://github.com/microsoft/CCF/sessions/50322bc9-cbe7-4e3d-ac98-08b1e2a4097e Co-authored-by: maxtropets <16566519+maxtropets@users.noreply.github.com>

…a_public_key.cpp Agent-Logs-Url: https://github.com/microsoft/CCF/sessions/6fad0029-75d6-4e92-a595-807024dc64d3 Co-authored-by: maxtropets <16566519+maxtropets@users.noreply.github.com>

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

maxtropets · 2026-04-14T13:44:34Z

@copilot based on the last commit I've done, update the PR title and description if needed

Also make them as concise as possible w/o losing any useful info

Copilot · 2026-04-14T14:02:19Z

@copilot based on the last commit I've done, update the PR title and description if needed

Also make them as concise as possible w/o losing any useful info

Updated the PR description to cover all four changes including the CHECK0 hardening and the X509_REQ_sign CHECK1→CHECK0 fix from 962d640. Kept the existing title — it still accurately covers everything.

achamayou

Goes in the right direction but please see and fix comment.

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Agent-Logs-Url: https://github.com/microsoft/CCF/sessions/3a2bc942-163a-454f-bd12-a7cd8d2060bb Co-authored-by: maxtropets <16566519+maxtropets@users.noreply.github.com>

…GELOG entry for CHECK0 removal Agent-Logs-Url: https://github.com/microsoft/CCF/sessions/4d6d118d-4cf4-46fb-93c6-3fd4806cf748 Co-authored-by: maxtropets <16566519+maxtropets@users.noreply.github.com>

Copilot · 2026-04-15T12:00:13Z

@copilot based on the last commit I've done, update the PR title and description if needed

Also make them as concise as possible w/o losing any useful info

Updated the PR description to include the latest changes: rc/ec in error messages for better diagnostics, and CHANGELOG entry for CHECK0 removal. Title still fits — kept as is.

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

…sa_pss_saltlen Agent-Logs-Url: https://github.com/microsoft/CCF/sessions/0f3e27d5-e85b-427c-9bc6-94634e2acbbb Co-authored-by: maxtropets <16566519+maxtropets@users.noreply.github.com>

Harden crypto: CHECK1 for RSA OAEP params, RAII for HKDF ctx, fix CHE…

6f29218

…CK1 macro Agent-Logs-Url: https://github.com/microsoft/CCF/sessions/50322bc9-cbe7-4e3d-ac98-08b1e2a4097e Co-authored-by: maxtropets <16566519+maxtropets@users.noreply.github.com>

Copilot AI assigned Copilot and maxtropets Apr 14, 2026

Copilot created this pull request from a session on behalf of maxtropets April 14, 2026 12:44 View session

Merge branch 'main' into copilot/add-check1-to-crypto-files

37690d4

maxtropets marked this pull request as ready for review April 14, 2026 12:45

maxtropets requested a review from a team as a code owner April 14, 2026 12:45

Copilot AI review requested due to automatic review settings April 14, 2026 12:45

maxtropets approved these changes Apr 14, 2026

View reviewed changes

maxtropets added the run-long-test Run Long Test job label Apr 14, 2026

maxtropets reviewed Apr 14, 2026

View reviewed changes

Comment thread src/crypto/openssl/hash.cpp

maxtropets reviewed Apr 14, 2026

View reviewed changes

Comment thread src/crypto/openssl/rsa_public_key.cpp Outdated

Copilot started work on behalf of maxtropets April 14, 2026 12:52 View session

Copilot started reviewing on behalf of maxtropets April 14, 2026 12:59 View session

Remove unnecessary OpenSSL:: namespace prefix from CHECK1 calls in rs…

a131a59

…a_public_key.cpp Agent-Logs-Url: https://github.com/microsoft/CCF/sessions/6fad0029-75d6-4e92-a595-807024dc64d3 Co-authored-by: maxtropets <16566519+maxtropets@users.noreply.github.com>

Copilot finished work on behalf of maxtropets April 14, 2026 13:03

Copilot AI requested a review from maxtropets April 14, 2026 13:03

Copilot AI reviewed Apr 14, 2026

maxtropets marked this pull request as draft April 14, 2026 13:34

Fix check0/1 usage:

962d640

Copilot started work on behalf of maxtropets April 14, 2026 13:44 View session

maxtropets added this to the 7.0.0-rc2 milestone Apr 14, 2026

Copilot finished work on behalf of maxtropets April 14, 2026 14:03

maxtropets marked this pull request as ready for review April 14, 2026 14:16

maxtropets approved these changes Apr 14, 2026

View reviewed changes

Merge branch 'main' into copilot/add-check1-to-crypto-files

46f250f

achamayou reviewed Apr 14, 2026

View reviewed changes

Comment thread src/crypto/openssl/rsa_key_pair.cpp Outdated

achamayou approved these changes Apr 14, 2026

View reviewed changes

More check1->+, removed check0

950434f

maxtropets requested a review from Copilot April 15, 2026 10:13

maxtropets enabled auto-merge (squash) April 15, 2026 10:18

Copilot AI reviewed Apr 15, 2026

View reviewed changes

Comment thread include/ccf/crypto/openssl/openssl_wrappers.h Outdated

Comment thread include/ccf/crypto/openssl/openssl_wrappers.h

Comment thread include/ccf/crypto/openssl/openssl_wrappers.h

Comment thread src/crypto/openssl/rsa_public_key.cpp

Copilot started work on behalf of maxtropets April 15, 2026 10:25 View session

Include OpenSSL error queue details in CHECKPOSITIVE() diagnostics

5a21def

Agent-Logs-Url: https://github.com/microsoft/CCF/sessions/3a2bc942-163a-454f-bd12-a7cd8d2060bb Co-authored-by: maxtropets <16566519+maxtropets@users.noreply.github.com>

Copilot finished work on behalf of maxtropets April 15, 2026 11:23

Copilot AI requested a review from maxtropets April 15, 2026 11:23

Copilot started work on behalf of maxtropets April 15, 2026 11:23 View session

Copilot started reviewing on behalf of maxtropets April 15, 2026 11:46 View session

Copilot finished work on behalf of maxtropets April 15, 2026 12:01

Merge branch 'main' into copilot/add-check1-to-crypto-files

de508e9

maxtropets requested a review from Copilot April 16, 2026 08:58

Copilot started reviewing on behalf of maxtropets April 16, 2026 09:00 View session

Copilot AI reviewed Apr 16, 2026

View reviewed changes

Comment thread src/tls/cert.h

Comment thread src/crypto/openssl/verifier.cpp

Comment thread src/crypto/openssl/rsa_public_key.cpp

Comment thread src/crypto/openssl/rsa_key_pair.cpp

Copilot started work on behalf of maxtropets April 16, 2026 10:05 View session

Validate salt_length fits in int before passing to EVP_PKEY_CTX_set_r…

29b003b

…sa_pss_saltlen Agent-Logs-Url: https://github.com/microsoft/CCF/sessions/0f3e27d5-e85b-427c-9bc6-94634e2acbbb Co-authored-by: maxtropets <16566519+maxtropets@users.noreply.github.com>

Copilot finished work on behalf of maxtropets April 16, 2026 10:09

Copilot started work on behalf of maxtropets April 16, 2026 10:09 View session

Copilot finished work on behalf of maxtropets April 16, 2026 10:14

Merge branch 'main' into copilot/add-check1-to-crypto-files

343d8ea

maxtropets merged commit 55e5977 into main Apr 16, 2026
19 checks passed

maxtropets deleted the copilot/add-check1-to-crypto-files branch April 16, 2026 12:59

Conversation

Copilot AI commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

maxtropets commented Apr 14, 2026

Uh oh!

Copilot AI commented Apr 14, 2026

Uh oh!

Uh oh!

achamayou left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI commented Apr 15, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Copilot AI commented Apr 14, 2026 •

edited

Loading