bcgit / bc-java

Bouncy Castle Java Distribution (Mirror)
https://www.bouncycastle.org/java.html
MIT License
2.32k stars 1.14k forks source link

Exception on valid (in openssl) CMS message : no such algorithm: SHA256WITHRSA #1553

Open izderadicka opened 11 months ago

izderadicka commented 11 months ago

Hi,

we have problem with BC validating CMS message, that is validated fine with openssl:

$ openssl version
OpenSSL 3.0.2 15 Mar 2022 (Library: OpenSSL 3.0.2 15 Mar 2022)
$ openssl cms -verify -noverify -inform der -in problem-data.p7s -out /dev/null -print
CMS Verification successful

If I try validation with BC I get following exception:

Exception in thread "main" org.bouncycastle.cms.CMSException: can't create digest calculator: exception on setup: java.security.NoSuchAlgorithmException: no such algorithm: SHA256WITHRSA for provider BC
        at org.bouncycastle.cms.SignerInformation.doVerify(Unknown Source)
        at org.bouncycastle.cms.SignerInformation.verify(Unknown Source)
        at eu.zderadicka.Verifier.verify(Verifier.java:41)
        at eu.zderadicka.App.main(App.java:23)
Caused by: org.bouncycastle.operator.OperatorCreationException: exception on setup: java.security.NoSuchAlgorithmException: no such algorithm: SHA256WITHRSA for provider BC
        at org.bouncycastle.operator.jcajce.JcaDigestCalculatorProviderBuilder$1.get(Unknown Source)
        at org.bouncycastle.cms.SignerInformationVerifier.getDigestCalculator(Unknown Source)
        ... 4 more
Caused by: java.security.NoSuchAlgorithmException: no such algorithm: SHA256WITHRSA for provider BC
        at java.base/sun.security.jca.GetInstance.getService(GetInstance.java:87)
        at java.base/sun.security.jca.GetInstance.getInstance(GetInstance.java:206)
        at java.base/java.security.MessageDigest.getInstance(MessageDigest.java:248)
        at org.bouncycastle.jcajce.util.NamedJcaJceHelper.createMessageDigest(Unknown Source)
        at org.bouncycastle.operator.jcajce.OperatorHelper.createDigest(Unknown Source)
        ... 6 more

Here is minimal code to replicate the issue (without loading bytes form file):

public class Verifier {

    private static Logger log = Logger.getLogger(Verifier.class.getName());

    static {
        Security.addProvider(new BouncyCastleProvider());
    }

    public boolean verify(byte[] bytes) throws Exception {
        boolean verify = false;
        CMSSignedData data = new CMSSignedData(bytes);
        Store<X509CertificateHolder> certStore = data.getCertificates();
        SignerInformationStore signerInfos = data.getSignerInfos();

        Collection<SignerInformation> signers = signerInfos.getSigners();
        List<java.security.cert.Certificate> result = new ArrayList<>();
        for (SignerInformation signer : signers) {

            Collection<X509CertificateHolder> matches = certStore.getMatches(signer.getSID());
            for (X509CertificateHolder holder : matches) {
                result.add(new JcaX509CertificateConverter().setProvider("BC").getCertificate(holder));
            }

            verify = signer.verify(new JcaSimpleSignerInfoVerifierBuilder().setProvider("BC")
                    .build(matches.iterator().next()));

        }

        return verify;

    }

}

I unfortunately cannot share the signed message, because of privacy reasons, but I can provide anonymized text dump of it - some texts are replaced with XXX, but i hope core information is preserved

CMS_ContentInfo: 
  contentType: pkcs7-signedData (1.2.840.113549.1.7.2)
  d.signedData: 
    version: 1
    digestAlgorithms:
        algorithm: sha256WithRSAEncryption (1.2.840.113549.1.1.11)
        parameter: NULL
    encapContentInfo: 
      eContentType: pkcs7-data (1.2.840.113549.1.7.1)
      eContent: 
       XXX
    certificates:
      d.certificate: 
        cert_info: 
          version: 2
          serialNumber: 22641254
          signature: 
            algorithm: sha256WithRSAEncryption (1.2.840.113549.1.1.11)
            parameter: NULL
          issuer: XXX
          validity: 
            notBefore: May 26 10:29:41 2022 GMT
            notAfter: Jun 14 10:29:41 2025 GMT
          subject: XXX
          key: 
            algor: 
              algorithm: rsaEncryption (1.2.840.113549.1.1.1)
              parameter: NULL
            public_key:  (0 unused bits)
             XXX
          issuerUID: <ABSENT>
          subjectUID: <ABSENT>
          extensions:
              XXX
        sig_alg: 
          algorithm: sha256WithRSAEncryption (1.2.840.113549.1.1.11)
          parameter: NULL
        signature:  (0 unused bits)
          XXX
    crls:
      <ABSENT>
    signerInfos:
        version: 1
        d.issuerAndSerialNumber: 
          issuer: XXX
          serialNumber: 22641254
        digestAlgorithm: 
          algorithm: sha256WithRSAEncryption (1.2.840.113549.1.1.11)
          parameter: NULL
        signedAttrs:
          <ABSENT>
        signatureAlgorithm: 
          algorithm: rsaEncryption (1.2.840.113549.1.1.1)
          parameter: NULL
        signature: 
          XXX
        unsignedAttrs:
          <ABSENT>

Problem seems to be with using sha256WithRSAEncryption (1.2.840.113549.1.1.11) as digest algorithm, however I have noted openssl is OK with it. I'm no expert is PKCS#7, but tend to believe openssl as kind of reference implementation.
If this is against standard, then it would be great to know exact details where it is violating standards.

BC: version 1.69 , 1.70 and latest 1.77 - same behaviour

dghgit commented 10 months ago

Digest algorithm should be 2.16.840.1.101.3.4.2.1 Using a signature OID for a digest algorithm is wrong on so many levels it is difficult to know where to start - I would guess OpenSSL is accepting it as the developers have resigned themselves to the idea that people will still insist on producing garbage messages and expecting them to work.

izderadicka commented 10 months ago

@dghgit thanks a lot for reply, that makes sense to me, do you know an exact reference, that will prove that this is against PKCS#7 standard?

We found that problematic CMS was created on Windows, with wincrypt API, the code (shortened), looks like this:

CString sObjId = szOID_RSA_SHA256RSA;
// ...
SigParams.HashAlgorithm.pszObjId = sObjId.GetBuffer();
SigParams.HashAlgorithm.Parameters.cbData = NULL;
// ...
CryptSignMessage(&SigParams, bDetachedSignature, 1, MessageArray, MessageSizeArray, pbSignature, &dwSignedMsgSize)

I see here usage of that problematic algo OID, clearly HOWEVER if you look for instance here https://learn.microsoft.com/en-us/windows/win32/seccrypto/example-c-program-signing-a-message-and-verifying-a-message-signature this is exactly what Microsoft shows as proper use of their API. (they use there SigParams.HashAlgorithm.pszObjId = szOID_RSA_SHA1RSA; I guess it quite similar - it's signature algorithm).

So is MS wrong? That would not be the first case when they ignored standard. But we need to prove it. If we just say to guys who are producing this message that it's wrong, they will come back and say, oh no - it's exactly according MS official docs and it works with openssl.

So any hint how to prove this CMS incorrectness is welcomed. Thanks.

dghgit commented 10 months ago

For SHA-2 it's documented here: https://www.ietf.org/rfc/rfc5754.txt

Although, really a hash algorithm is not the same thing as a signature algorithm, that should be proof enough. Occasionally these OIDs appear in data with either a signature or a MAC attached to it, expecting people to accept incorrect OIDs in these circumstances has real security implications.

I had a look at the MS example, words fail, they should really fix that, the encoding for SHA-1 is given in:

https://datatracker.ietf.org/doc/html/rfc3370#section-2.1

That document was published in 2002, and it's actually based on a much older one as well. It's not like any of this is a recent innovation.

izderadicka commented 10 months ago

@dghgit Thanks a lot for detailed explanation, will look at RFCs. I think I may try submit issue to openssl and check why they are validating CMS with invalid hash alg OID - that may give view of other perspective.

izderadicka commented 10 months ago

Hi there,

I have created issue with OpenSSL - see above, generally they do not consider this a security issue as needed hash algorithms is identified, though not exactly with formal compliance with RFC.

Also sent feedback on that MS page, no reply (as expected). Developers who are using wincrypto api made some tests, it looks that other libraries, apart of openssl, also validates such CMS message : BouncyCastle C#, Mozilla NSS, .Net 8. So BC Java is here rather in minority. They also noted that recently BC Java was modified to accept SHA1WITHRSA for DigestAlgorimth, not sure if this is true just passing information. Maybe last info relates to #1500

dghgit commented 10 months ago

Yes, but it should be noted that #1500 only applies to legacy signatures without signed attributes, signatures which are pretty much "stuffed" these days. Can I suggest that people stop worrying about who passes invalid messages and concentrate on producing valid ones instead? There might just be a real security reason for having a concept of a valid message.

mpancode commented 8 months ago

In BC for C#, the digest algorithm is obtained from the signature OID, and if this OID does not contain a digest algorithm, it is obtained from the digest OID using the same function. Thus, if the signature algorithm has the value sha256WithRSAEncryption, the digest algorithm is obtained from it (sha256). If the signature algorithm had a value of rsaEncryption, the digest algorithm is obtained from digestAlgorithm. And if digestAlgorithm had a value of sha256WithRSAEncryption, the sha256 digest is correctly determined from this OID: source

In contrast, in BC for Java, only a proprietary function is called that only handles cases for sha1: source

dghgit commented 7 months ago

I'll have to look into the history of the situation with C#, it's probably too late to fix it now though. You can expect the trend will not continue.

minfrin commented 4 months ago

We found that problematic CMS was created on Windows, with wincrypt API, the code (shortened), looks like this:


CString sObjId = szOID_RSA_SHA256RSA;

Just ran into the same problem.

Hard coding the digest to SHA256 like windows does in its examples is awful, did you find any API calls on windows that would convert a szOID_RSA_SHA256RSA as obtained from the certificate into szOID_NIST_sha256 as required by bouncycastle?

CryptFindOIDInfo appears to come close, but the docs aren't clear whether this is possible in the general case.