Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for verifying PKCS7 signed attributes #2264

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

samuel40791765
Copy link
Contributor

Issues:

Resolves CryptoAlg-2946

Description of changes:

I discovered this while trying to fix our PKCS7 implementation to use indefinite encoding. The PKCS7 file that Ruby's PKCS7 tests uses signed attributes, but PKCS7_verify bails out whenever it encounters files that have signed attributes. There are still other issues with the Ruby PKCS7 test that we'll have to fix (indefinite length ASN1), but I believe we should fix the missing support for verifying signed attributes first.
AWS-LC turns on PKCS7_NOATTR by default in PKCS7_sign, so our existing PKCS7_verify implementation can do a successful sign/verify round trip against itself. However, OpenSSL does not turn on PKCS7_NOATTR by default and signed attributes are added automatically to the PKCS7 file if no flags are set. This means that the current state of AWS-LC's PKCS7_verify would fail against files generated by the default of OpenSSL's PKCS7_sign. This PR adds support for verifying PKCS7 signed attributes to fix the misalignment.

Call-outs:

N/A

Testing:

  1. Test file from Ruby. The intention is to build upon the test to fix the ASN1 indefinite length issues later on.
  2. Test file generated by OpenSSL. Our PKCS7_sign does not support signed attributes, so we can only test against generated files by OpenSSL.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

@samuel40791765 samuel40791765 requested a review from a team as a code owner March 12, 2025 00:14
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