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

Remove the digest from OID for the digestEncryptionAlgorithm #185

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

Conversation

bb-froggy
Copy link

The CMSSignedDataGenerator allows adding signers in two ways. One are the methods AddSigner, which work for me, while the other AddSignerInfoGenerator adds the wrong OID for the signature algorithm to the PKCS#7. In my case, it adds sha256WithRSAEncryption (1 2 840 113549 1 1 11), where other common CMS signed data files I have looked at contain only rsaEncryption (1 2 840 113549 1 1 1).

As per RFC 2315, Section 9.2, the "digestEncryptionAlgorithm identifies the digest-encryption algorithm (and any associated parameters) under which the message digest and associated information are encrypted with the signer's private key." I understand this also to say that the digest should not be part of the OID.

With this fix, the correct OID appears in the CMS, and I can use the CMS with other crypto libraries, too.

It would be very kind if a maintainer could review this change.

@jimsch
Copy link

jimsch commented Apr 9, 2019

Using rsaEncryption is no longer considered to be the correct answer and is only for PKCS #7 and not for CMS. Please do not make this change.

@bb-froggy
Copy link
Author

Ah, thank you for pointing out that I referred to PKCS#7 and that this class is for CMS. I just falsely assumed that this would make no difference.

Anyway, AddSigner adds the rsaEncryption OID if you use an RSA key. AddSigner and AddSignerInfoGenerator should behave consistently, so maybe change AddSigner instead?

The CMS RFC describes in Section 5.3 the signatureAlgorithm, but is not really clear on whether the digest algorithm should be part of it. Section 10.1.2 describes it more explicit, though, and says

"The SignatureAlgorithmIdentifier type identifies a signature algorithm, and it can also identify a message digest algorithm. Examples include RSA, DSA, DSA with SHA-1, ECDSA, and ECDSA with SHA-256."

This makes it clear in my opinion that rsaEncryption is possible, but it indicates that sha256WithRSAEncryption is also valid.

It would still be good to use rsaEncryption for backwards compatibility with PKCS #7. At least add some way to control which OID is added. I also know at least one CMS library that cannot handle sha256WithRSAEncryption: the class SignedCms in the System.Security.Cryptography.Pkcs package. This library is in preview and has a couple more bugs, so I guess it's not a good reference.

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.

None yet

2 participants