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
CMS: Add support for sha-1WithRSAEncryption *digest* algorithm + aliases. #1500
base: main
Are you sure you want to change the base?
Conversation
dad4e9a
to
73c4b78
Compare
73c4b78
to
b0f9377
Compare
I've had a chance to have a quick look at this, essentially we'd be adding support for a specific kind of broken CMS signed data message. sha-1WithRSAEncryption is a signature algorithm, the reason for the nomenclature lies in the original drafts of PKCS#7 where the algorithm used for processing the hash was referred to an an "encryption algorithm" since the process of signing with RSA is the same as "encrypting with the private key". Neither of the OIDs being added to map to SHA-1 are, or ever have been, considered to be identifying the SHA-1 hash algorithm, both RFCs are explicit about requiring 1.3.14.3.2.26 as being the OID used to represent SHA-1. The move to calling it a signatureAlgorithm was a result of other signing algorithms being required which made it obvious that the RSA centric view of the universe was not always correct. It doesn't look like this would present a security issue, but I would prefer to keep the number of non-compliant OIDs to a minimum. Are you saying that you have message where both the OIW and PKCS definitions for SHA-1withRSA are used to represent the SHA-1 OID? Or is it just one variant that is present (I'd guess the OIW arc) and the other is given "for completeness"? |
Yup, unfortunately we have clients sending CMS messages where the the digest algorithm OID (what BouncyCastle's CMS library calls the "ContentDigestAlgorithm") is sha-1WithRSAEncryption, even though they really should be setting it as SHA1 (because that's what it really is, they're just using a weird name / alias for it), and we're trying to support processing and verifying these messages. Apparently googling around there are other (non-Java) pkcs7 implementations that run into this so provide support for these aliases. |
We'd be better off if people just did it correctly. In some situations it is actually possible to weaken the security of the message when issues like this have to be dealt with. One last question, do they use sha-1WithRSAEncryption as the encryption algorithm in the signed data message or is it just rsaEncryption? |
Do you mean the signature algorithm? They appear to use rsaEncryption as the identifier. Here's an example of a partial ASN.1 dump (just the metadata, actual data content redacted): |
So I've pushed an update to SignerInformation.java which should deal with this - in a limited fashion. Is it possible to provide a sample of one of these messages? |
Thanks! It does seem like it allows decoding and verification if CMS messages using the weird digest algo name. This does seem like it'll allow us to handle these messages being sent. Unfortunately, it still doesn't allow you to sign messages using the OID, which would be helpful for writing code for tests. :) Trying to use it with CMSSignedDataGenerator / JcaSignerInfoGeneratorBuilder still gives you:
Is it possible in addition for the changes you already made for you to pull in the
Here's an example of a random CMS message using this:
|
…t algorithm. Per RFC 3279 / RFC 8017, this digest algorithm computes the SHA-1 digest on the encoded `DigestInfo` whose `digestAlgorithm` is set to the `id-sha1` OID.
bf3509d
to
61f6bfe
Compare
Thanks for the sample. With hindsight, I've got mixed feelings about the fact it works, I note it's using signed attributes, I guess since you've generated it locally - but for that particular use case it does mean we're weakening an already weak algorithm further. With further changes, I hate break bad news, but the generator is resolving the alias correctly - producing messages like this should not be encouraged (I'll leave the idea of using SHA-1 in general now for another discussion...). In the (hopefully) unlikely event you're actually supposed to send broken messages to who ever was responsible for this in the first place, one interesting difference between the generated sample and the message the dump was provided of early, is the original message used a raw signature and no signing attributes. You can actually create one of these using JcaJcaSimpleSignerInfoGeneratorBuilder.setDirectSignature() and then doing (for the simplest case):
One further question, are these message types always direct signatures? If possible, I would rather go back to failing for signatures using signed attributes, people having to rely on SHA-1 based signatures already have enough issues without us further compromising the security of the signature. |
Ah you're right, I generated it locally with signed attributes. I wanted to avoid posting a real example of messages we're seeing because of PII.
Ah you're right, using direct signature works: val cert: X509Certificate = // ...
val privateKey: PrivateKey = // ...
val contentSigner: ContentSigner =
JcaContentSignerBuilder("SHA1WithRSAEncryption")
.setProvider(BouncyCastleProvider())
.build(privateKey)
val content: CMSTypedData = CMSProcessableByteArray("My message".toByteArray())
val signedData: CMSSignedData =
CMSSignedDataGenerator()
.apply {
addSignerInfoGenerator(
JcaSignerInfoGeneratorBuilder(
JcaDigestCalculatorProviderBuilder().setProvider(BouncyCastleProvider()).build()
)
.setDirectSignature(true)
.setContentDigest(AlgorithmIdentifier(OIWObjectIdentifiers.sha1WithRSA))
.build(contentSigner, cert)
)
addCertificates(JcaCertStore(setOf(cert)))
}
.generate(content, true) I updated my example in my comment above to be a CMS message using direct signature w/ the sha1WithRsa digest algo alias.
I've only looked at a few samples of CMS messages we get, but of the ones I've seen they don't seem to have signed attributes. Still it's hard to be sure that no client would send them... |
added check for signed attributes on dodgey SHA-1 OID relates to github bcgit#1500
This adds support to the CMS libraries for the
sha-1WithRSAEncryption
digest algorithm (OID1.3.14.3.2.29
) and its aliases. This is not to be confused with theSHA1withRSA
signature algorithm which is currently already supported.Per RFC 3279 2.2.1 and RFC 8017 A.2.4, this digest algorithm computes the SHA-1 digest on the encoded
DigestInfo
whosedigestAlgorithm
field is set to theid-sha1
OID.Bouncy Castle doesn't currently recognize this digest algorithm (either for building
CMSSignedData
or verifying) and fails withIn versions 1.69 and earlier, recognition for OID
1.3.14.3.2.29
inOperatorHelper.java
was present, but the data encoding was not performed correctly (setting OID1.3.14.3.2.29
during the data encoding step, rather than OID1.3.14.3.2.26
as RFC 3279 requires), so the computed digest would be wrong and not verify.