Skip to content
This repository has been archived by the owner on Oct 5, 2021. It is now read-only.

protocol: Allow for PKCS #7 backwards compatibility #22

Closed
wants to merge 1 commit into from

Conversation

josephlr
Copy link
Contributor

@josephlr josephlr commented May 20, 2020

Depends on #23 (to fix the CI)

Many signature protocols use a format that is technically backwards
compatible with CMS. However, these incompatibilities are minor and can
be avoided by removing the check for octets.Tag == asn1.TagOctetString.

By just returning the octets.Bytes without checking the tag, old-style
PKCS #7 signatures can now be created and verified without issue.

I tested this with Microsoft Authenticode Signatures (which use non-CMS PKCS7), both creating and verifying. Note that verifying non-standard structures also requires #21 (to call sd.psd.EncapContentInfo.EContentValue on a cms.SignedData).

@mastahyeti how would you like me to add test cases here?

EDIT: Note that this procedure of not requiring the OCTET STRING to keep backwards compatiblity with PKCS7 is described in the CMS RFC, RFC 5652 Section 5.2.1:

   The following strategy can be used to achieve backward compatibility
   with PKCS #7 when processing SignedData content types.  If the
   implementation is unable to ASN.1 decode the SignedData type using
   the CMS SignedData encapContentInfo eContent OCTET STRING syntax,
   then the implementation MAY attempt to decode the SignedData type
   using the PKCS #7 SignedData contentInfo content ANY syntax and
   compute the message digest accordingly.

Many signature protocols use a format that is _technically_ backwards
compatible with CMS. However, these incompaiblities are minor and can
be avoided by removing the check for octets.Tag == asn1.TagOctetString.

By just returning the octets.Bytes without checking the tag, old-style
PKCS github#7 signatures can now be created and verified without issue.

Signed-off-by: Joe Richey <joerichey@google.com>
@btoews
Copy link
Contributor

btoews commented Jun 1, 2020

Sorry for the slow response. I don't work for GitHub any long and am not maintaining this library. /cc @ptoomey3 for 👀 on this and #23

@lgarron
Copy link
Contributor

lgarron commented Oct 4, 2021

This project is now part of smimesign.
Please send this PR to https://github.com/github/smimesign if it's still relevant!

@lgarron lgarron closed this Oct 4, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants