Skip to content

Conversation

rhenium
Copy link
Member

@rhenium rhenium commented Jul 31, 2025


pkcs7: refactor error handling in PKCS7#add_data

Raise an exception right after an OpenSSL function returns an error. Checking ERR_peek_error() is not reliable way to see if an error has occurred or not, as OpenSSL functions do not always populate the error queue.


pkcs7: make PKCS7#add_recipient actually useful

Add a simple test case that creates an enveloped-data structure without using the shorthand method, and fix two issues preventing this from working correctly.

First, OpenSSL::PKey::PKCS7#add_recipient currently inserts an incomplete PKCS7_RECIP_INFO object into the PKCS7 object. When duplicating an unfinalized PKCS7_RECIP_INFO, the internal X509 reference must also be copied, as it is later used by #add_data to fill the rest.

A similar issue with #add_signer was fixed in commit 20ca7a2 (pkcs7: keep private key when duplicating PKCS7_SIGNER_INFO, 2021-03-24).

Second, #add_data calls PKCS7_dataFinal(), which for enveloped-data appears to require the BIO to be flushed explicitly with BIO_flush(). Without this, the last block of the encrypted data would be missing.

rhenium added 2 commits August 1, 2025 02:43
Raise an exception right after an OpenSSL function returns an error.
Checking ERR_peek_error() is not reliable way to see if an error has
occurred or not, as OpenSSL functions do not always populate the error
queue.
Add a simple test case that creates an enveloped-data structure without
using the shorthand method, and fix two issues preventing this from
working correctly.

First, OpenSSL::PKey::PKCS7#add_recipient currently inserts an
incomplete PKCS7_RECIP_INFO object into the PKCS7 object. When
duplicating an unfinalized PKCS7_RECIP_INFO, the internal X509 reference
must also be copied, as it is later used by #add_data to fill the rest.

A similar issue with #add_signer was fixed in commit 20ca7a2
(pkcs7: keep private key when duplicating PKCS7_SIGNER_INFO,
2021-03-24).

Second, #add_data calls PKCS7_dataFinal(), which for enveloped-data
appears to require the BIO to be flushed explicitly with BIO_flush().
Without this, the last block of the encrypted data would be missing.
@rhenium rhenium merged commit dfbbac6 into ruby:master Aug 1, 2025
41 checks passed
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