PKCS#7 / CMS Signature validation failure with unordered attributes

[dhensby]

Firstly, apologies if this issue is known or has been raised and closed. I had a look through existing issues and found a couple of things that could be related, but don't explicitly get to the bottom of it. #286 #761

I have a service producing detached CMS payloads, these payloads validate successfully when using openssl, but when using Bouncy Castle, the signature fails to validate. NB: I am not the one using BC, so apologies that I can't provide full errors.

After much debugging, it was found that the issue is the service generating the CMS payloads is not strictly compliant with the specification which says that the authenticated attributes must be a DER encoded SET OF, the DER spec says that a SET OF must be ordered:

11 Restrictions on BER employed by both CER and DER
…
11.6 Set-of components
The encodings of the component values of a set-of value shall appear in ascending order, the encodings being compared as octet strings with the shorter components being padded at their trailing end with 0-octets.
NOTE – The padding octets are for comparison purposes only and do not appear in the encodings.

I wondered why this was an issue in BC but not in openssl and after a bit of digging, I think the issue comes down to the fact that BC is parsing and then reconstructing what it thinks the signed ASN.1 structure should be and not what it actually is. There is a decode/encode step that causes information (order) loss and therefore the signature is not calculated over the bytes that are supplied causing an invalid signature to be calculated.

I can see that there are two valid viewpoints on this:

1. It's a non-compliant structure, so failure is understandable; and
2. BC should be validating signatures over the actual bytes received, not over reconstructed data.

I don't think these are mutually exclusive and both are true; BC should not be calculating the signatures except from the actual supplied bytes. Ultimately, the signature over the bytes supplied is valid, the bytes supplied just don't adhere strictly to the spec.

---

Version: org.bouncycastle:bcpkix-jdk15on:1.70

---

Example - here is a certificate and signature that passes with openssl but should fail with bouncy castle:

certificate.pem:

-----BEGIN CERTIFICATE-----
MIIDtDCCApwCCQDUVBxA2DXi8zANBgkqhkiG9w0BAQUFADCBmzELMAkGA1UEBhMC
REUxEjAQBgNVBAgMCUZyYW5jb25pYTEQMA4GA1UEBwwHQW5zYmFjaDEVMBMGA1UE
CgwMU3RlZmFuIFNpZWdsMRIwEAYDVQQLDAlHZWllcmxlaW4xFjAUBgNVBAMMDUdl
aWVybGVpbiBERVYxIzAhBgkqhkiG9w0BCQEWFHN0ZXNpZUBicm9rZW5waXBlLmRl
MB4XDTEyMDMxODIyNTc0M1oXDTEzMDMxODIyNTc0M1owgZsxCzAJBgNVBAYTAkRF
MRIwEAYDVQQIDAlGcmFuY29uaWExEDAOBgNVBAcMB0Fuc2JhY2gxFTATBgNVBAoM
DFN0ZWZhbiBTaWVnbDESMBAGA1UECwwJR2VpZXJsZWluMRYwFAYDVQQDDA1HZWll
cmxlaW4gREVWMSMwIQYJKoZIhvcNAQkBFhRzdGVzaWVAYnJva2VucGlwZS5kZTCC
ASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAMsAbQ4fWevHqP1K1y/ewpMS
3vYovBto7IsKBq0v3NmC2kPf3NhyaSKfjOOS5uAPONLffLck+iGdOLLFia6OSpM6
0tyQIV9lHoRh7fOEYORab0Z+aBUZcEGT9yotBOraX1YbKc5f9XO+80eG4XYvb5ua
1NHrxWqe4w2p3zGJCKO+wHpvGkbKz0nfu36jwWz5aihfHi9hp/xs8mfH86mIKiD7
f2X2KeZ1PK9RvppA0X3lLb2VLOqMt+FHWicyZ/wjhQZ4oW55ln2yYJUQ+adlgaYn
PrtnsxmbTxM+99oF0F2/HmGrNs8nLZSva1Vy+hmjmWz6/O8ZxhiIj7oBRqYcAocC
AwEAATANBgkqhkiG9w0BAQUFAAOCAQEAvfvtu31GFBO5+mFjPAoR4BlzKq/H3EPO
qS8cm/TjHgDRALwSnwKYCFs/bXqE4iOTD6otV4TusX3EPbqL2vzZQEcZn6paU/oZ
ZVXwQqMqY5tf2teQiNxqxNmSIEPRHOr2QVBVIx2YF4Po89KGUqJ9u/3/10lDqRwp
sReijr5UKv5aygEcnwcW8+Ne4rTx934UDsutKG20dr5trZfWQRVS9fS9CFwJehEX
HAMUc/0++80NhfQthmWZWlWM1R3dr4TrIPtWdn5z0MtGeDvqBk7HjGrhcVS6kAsy
Z9y/lfLPjBuxlQAHztEJCWgI4TW3/RLhgfg2gI1noM2n84Cdmisfkg==
-----END CERTIFICATE-----

pkcs7-bad.pem:

-----BEGIN PKCS7-----
MIIGRwYJKoZIhvcNAQcCoIIGODCCBjQCAQExDzANBglghkgBZQMEAgEFADAcBgkq
hkiG9w0BBwGgDwQNVG8gYmUgc2lnbmVkLqCCA7gwggO0MIICnAIJANRUHEDYNeLz
MA0GCSqGSIb3DQEBBQUAMIGbMQswCQYDVQQGEwJERTESMBAGA1UECAwJRnJhbmNv
bmlhMRAwDgYDVQQHDAdBbnNiYWNoMRUwEwYDVQQKDAxTdGVmYW4gU2llZ2wxEjAQ
BgNVBAsMCUdlaWVybGVpbjEWMBQGA1UEAwwNR2VpZXJsZWluIERFVjEjMCEGCSqG
SIb3DQEJARYUc3Rlc2llQGJyb2tlbnBpcGUuZGUwHhcNMTIwMzE4MjI1NzQzWhcN
MTMwMzE4MjI1NzQzWjCBmzELMAkGA1UEBhMCREUxEjAQBgNVBAgMCUZyYW5jb25p
YTEQMA4GA1UEBwwHQW5zYmFjaDEVMBMGA1UECgwMU3RlZmFuIFNpZWdsMRIwEAYD
VQQLDAlHZWllcmxlaW4xFjAUBgNVBAMMDUdlaWVybGVpbiBERVYxIzAhBgkqhkiG
9w0BCQEWFHN0ZXNpZUBicm9rZW5waXBlLmRlMIIBIjANBgkqhkiG9w0BAQEFAAOC
AQ8AMIIBCgKCAQEAywBtDh9Z68eo/UrXL97CkxLe9ii8G2jsiwoGrS/c2YLaQ9/c
2HJpIp+M45Lm4A840t98tyT6IZ04ssWJro5KkzrS3JAhX2UehGHt84Rg5FpvRn5o
FRlwQZP3Ki0E6tpfVhspzl/1c77zR4bhdi9vm5rU0evFap7jDanfMYkIo77Aem8a
RsrPSd+7fqPBbPlqKF8eL2Gn/GzyZ8fzqYgqIPt/ZfYp5nU8r1G+mkDRfeUtvZUs
6oy34UdaJzJn/COFBnihbnmWfbJglRD5p2WBpic+u2ezGZtPEz732gXQXb8eYas2
zyctlK9rVXL6GaOZbPr87xnGGIiPugFGphwChwIDAQABMA0GCSqGSIb3DQEBBQUA
A4IBAQC9++27fUYUE7n6YWM8ChHgGXMqr8fcQ86pLxyb9OMeANEAvBKfApgIWz9t
eoTiI5MPqi1XhO6xfcQ9uova/NlARxmfqlpT+hllVfBCoypjm1/a15CI3GrE2ZIg
Q9Ec6vZBUFUjHZgXg+jz0oZSon27/f/XSUOpHCmxF6KOvlQq/lrKARyfBxbz417i
tPH3fhQOy60obbR2vm2tl9ZBFVL19L0IXAl6ERccAxRz/T77zQ2F9C2GZZlaVYzV
Hd2vhOsg+1Z2fnPQy0Z4O+oGTseMauFxVLqQCzJn3L+V8s+MG7GVAAfO0QkJaAjh
Nbf9EuGB+DaAjWegzafzgJ2aKx+SMYICQjCCAj4CAQEwgakwgZsxCzAJBgNVBAYT
AkRFMRIwEAYDVQQIDAlGcmFuY29uaWExEDAOBgNVBAcMB0Fuc2JhY2gxFTATBgNV
BAoMDFN0ZWZhbiBTaWVnbDESMBAGA1UECwwJR2VpZXJsZWluMRYwFAYDVQQDDA1H
ZWllcmxlaW4gREVWMSMwIQYJKoZIhvcNAQkBFhRzdGVzaWVAYnJva2VucGlwZS5k
ZQIJANRUHEDYNeLzMA0GCWCGSAFlAwQCAQUAoGswGAYJKoZIhvcNAQkDMQsGCSqG
SIb3DQEHATAvBgkqhkiG9w0BCQQxIgQgL/TDBQ+1LmkIk4u0TacbQj6GvXtVN5Sr
LFvTa+Sr82MwHgYJKoZIhvcNAQkFMREYDzE5NDkxMjMxMjM1OTU5WjANBgkqhkiG
9w0BAQEFAASCAQCNB13woMM5xYY5B0k/2dDF1flyAs5JaahqXSIH04ea+qgyFkgZ
uRoialeVzXXXpSPmpgEubQimFMYFslle9Ozpu6ECrXtw53wWm1GAlj8T6w50lKyd
6Ip/wclO/lPIB9qk7Ki3kN+ISBWXSgUMTTo94IdyZH3lbBqW3xsak1FM6STrGiUS
CoinmPKajIbIQBUORZ9eF9qN5erdpKwYNMB43yLle5jNGIeq6ztbL9en9boFp04q
PU+/ZeNY2QJhnoGQVYfHkK+8X6/ZKX1fRnca2L+DUUb9g7Tc0oeW5zwB3wzIEpTN
JxV3HaZ/Jvfnx9uH9wpFYg5yFX6PBLxlHarf
-----END PKCS7-----

openssl command to verify:

$ openssl smime -verify -attime 1332945777 -in ./pkcs7-bad.pem -signer ./certificate.pem -inform PEM -CAfi
le ./certificate.pem

Example - pkcs7 that passes with both openssl and BC:

pkcs7-good.pem:

-----BEGIN PKCS7-----
MIIGRwYJKoZIhvcNAQcCoIIGODCCBjQCAQExDzANBglghkgBZQMEAgEFADAcBgkq
hkiG9w0BBwGgDwQNVG8gYmUgc2lnbmVkLqCCA7gwggO0MIICnAIJANRUHEDYNeLz
MA0GCSqGSIb3DQEBBQUAMIGbMQswCQYDVQQGEwJERTESMBAGA1UECAwJRnJhbmNv
bmlhMRAwDgYDVQQHDAdBbnNiYWNoMRUwEwYDVQQKDAxTdGVmYW4gU2llZ2wxEjAQ
BgNVBAsMCUdlaWVybGVpbjEWMBQGA1UEAwwNR2VpZXJsZWluIERFVjEjMCEGCSqG
SIb3DQEJARYUc3Rlc2llQGJyb2tlbnBpcGUuZGUwHhcNMTIwMzE4MjI1NzQzWhcN
MTMwMzE4MjI1NzQzWjCBmzELMAkGA1UEBhMCREUxEjAQBgNVBAgMCUZyYW5jb25p
YTEQMA4GA1UEBwwHQW5zYmFjaDEVMBMGA1UECgwMU3RlZmFuIFNpZWdsMRIwEAYD
VQQLDAlHZWllcmxlaW4xFjAUBgNVBAMMDUdlaWVybGVpbiBERVYxIzAhBgkqhkiG
9w0BCQEWFHN0ZXNpZUBicm9rZW5waXBlLmRlMIIBIjANBgkqhkiG9w0BAQEFAAOC
AQ8AMIIBCgKCAQEAywBtDh9Z68eo/UrXL97CkxLe9ii8G2jsiwoGrS/c2YLaQ9/c
2HJpIp+M45Lm4A840t98tyT6IZ04ssWJro5KkzrS3JAhX2UehGHt84Rg5FpvRn5o
FRlwQZP3Ki0E6tpfVhspzl/1c77zR4bhdi9vm5rU0evFap7jDanfMYkIo77Aem8a
RsrPSd+7fqPBbPlqKF8eL2Gn/GzyZ8fzqYgqIPt/ZfYp5nU8r1G+mkDRfeUtvZUs
6oy34UdaJzJn/COFBnihbnmWfbJglRD5p2WBpic+u2ezGZtPEz732gXQXb8eYas2
zyctlK9rVXL6GaOZbPr87xnGGIiPugFGphwChwIDAQABMA0GCSqGSIb3DQEBBQUA
A4IBAQC9++27fUYUE7n6YWM8ChHgGXMqr8fcQ86pLxyb9OMeANEAvBKfApgIWz9t
eoTiI5MPqi1XhO6xfcQ9uova/NlARxmfqlpT+hllVfBCoypjm1/a15CI3GrE2ZIg
Q9Ec6vZBUFUjHZgXg+jz0oZSon27/f/XSUOpHCmxF6KOvlQq/lrKARyfBxbz417i
tPH3fhQOy60obbR2vm2tl9ZBFVL19L0IXAl6ERccAxRz/T77zQ2F9C2GZZlaVYzV
Hd2vhOsg+1Z2fnPQy0Z4O+oGTseMauFxVLqQCzJn3L+V8s+MG7GVAAfO0QkJaAjh
Nbf9EuGB+DaAjWegzafzgJ2aKx+SMYICQjCCAj4CAQEwgakwgZsxCzAJBgNVBAYT
AkRFMRIwEAYDVQQIDAlGcmFuY29uaWExEDAOBgNVBAcMB0Fuc2JhY2gxFTATBgNV
BAoMDFN0ZWZhbiBTaWVnbDESMBAGA1UECwwJR2VpZXJsZWluMRYwFAYDVQQDDA1H
ZWllcmxlaW4gREVWMSMwIQYJKoZIhvcNAQkBFhRzdGVzaWVAYnJva2VucGlwZS5k
ZQIJANRUHEDYNeLzMA0GCWCGSAFlAwQCAQUAoGswGAYJKoZIhvcNAQkDMQsGCSqG
SIb3DQEHATAeBgkqhkiG9w0BCQUxERgPMTk0OTEyMzEyMzU5NTlaMC8GCSqGSIb3
DQEJBDEiBCAv9MMFD7UuaQiTi7RNpxtCPoa9e1U3lKssW9Nr5KvzYzANBgkqhkiG
9w0BAQEFAASCAQB4uY6pHBk49ZotqTDme/oj33L8f5hH4GNtvXzhNWGSt7gCXTrI
dOxgi/6+mR9J5Z5CmPaAMPitnm9PQApEQD2ecDi2MoN1XQfxqT2XS81cBZwafL9w
Wp4Wz63TqD/2ahCnDBP3PUHBt2MKCKIhUCbuzvW0Xq+9JkMfoNbEHczma+8GL4Uh
NDpvo3MVvcaG2WWI47ik/+t4TfBG1MFB4Qs08WoeCKuz9GL5YN0FnqG8ia3YXKpS
udOw9HDwsIMsaeJInVgf5hPrgNh4H9pzMw1x5bixOrUJ5AtmEBVLcflYg4NUFO1r
Gfmh+VqOs1mT0o0fz0hxcvJTyKQ11yw2S5xo
-----END PKCS7-----

openssl command to verify:

$ openssl smime -verify -attime 1332945777 -in ./pkcs7-good.pem -signer ./certificate.pem -inform PEM -CAfi
le ./certificate.pem

Example failing code:

fun verifySignature(): Boolean{
    return try {
        val jsonBytes = Files.readAllBytes(Paths.get("path/to/json"));
        val sigBytes = Files.readAllBytes(Paths.get("path/to/p7s"));
        // Create certificate from byte array
        val cert = System.getenv("PINNED_CERT")
        val certBytes: ByteArray = Base64.getDecoder().decode(cert)

        val certificateFactory = CertificateFactory.getInstance("X.509")
        println(certBytes.size)
        val certificate = certificateFactory.generateCertificate(certBytes.inputStream()) as X509Certificate

        // Verify the signature of the content byte array using CMS
        val cms = CMSSignedData(sigBytes)
        val signers: SignerInformationStore = cms.signerInfos
        val signer: SignerInformation = signers.signers.first()

        // Create a new Signature object
        val signature = Signature.getInstance(certificate.sigAlgName)
        signature.initVerify(certificate.publicKey)
        signature.update(jsonBytes)

        val sig = signer.signature
        val isValidSignature = signature.verify(sig) // <-- this always fails

        isValidSignature
    } catch (e: GeneralSecurityException) {
        println("Exception: ${e.message}")
        false
    }
}

[dghgit]

So it's 1, and 2 is fair enough. Have a look at

bc-java/pkix/src/test/java/org/bouncycastle/cms/test/NewSignedDataTest.java

Line 1320 in fd44df9

public void testSignerInformationExtension()

MyRightSignerInformation will show you what to do. Keep in mind if you are accepting messages like this people can reorder the data and add extras as it suits them - the purpose of the DER encoding is to ensure only one encoding of data can be used to verify the signature. It's better to get people to correct what they are doing if they are producing invalid messages.

[dhensby]

Keep in mind if you are accepting messages like this people can reorder the data and add extras as it suits them

Only the authenticated attributes are signed, so modification of the other parts of the message is always possible (except the content).

At the moment it would be possible for someone to sign the ordered attributes and then a third party could theoretically reorder them and BC would validate the signature (despite the signed attributes being tampered with), when I suspect openssl would fail as the actual bytes wouldn't verify.

However, this is academic and, I'd suggest, meaningless as the data would still have been signed, just someone had arbitrarily (but not meaningfully) interfered with the order of the authenticated attributes.

I have submitted a fix for this issue in the upstream library too: digitalbazaar/forge#1025

[dhensby]

In fact, I think this is an example of a PKCS#7 message that would fail validation in openssl but pass in BC because the signature is calculated across sorted attributes but the message contains unsorted attributes.

That means any library verifying signatures over the actual bytes in the message will fail, but any calculating it over expected bytes will validate it.

---

Bad PKCS#7 may pass in BC

-----BEGIN PKCS7-----
MIIGRQYJKoZIhvcNAQcCoIIGNjCCBjICAQExDzANBglghkgBZQMEAgEFADAcBgkq
hkiG9w0BBwGgDwQNVG8gYmUgc2lnbmVkLqCCA7gwggO0MIICnAIJANRUHEDYNeLz
MA0GCSqGSIb3DQEBBQUAMIGbMQswCQYDVQQGEwJERTESMBAGA1UECAwJRnJhbmNv
bmlhMRAwDgYDVQQHDAdBbnNiYWNoMRUwEwYDVQQKDAxTdGVmYW4gU2llZ2wxEjAQ
BgNVBAsMCUdlaWVybGVpbjEWMBQGA1UEAwwNR2VpZXJsZWluIERFVjEjMCEGCSqG
SIb3DQEJARYUc3Rlc2llQGJyb2tlbnBpcGUuZGUwHhcNMTIwMzE4MjI1NzQzWhcN
MTMwMzE4MjI1NzQzWjCBmzELMAkGA1UEBhMCREUxEjAQBgNVBAgMCUZyYW5jb25p
YTEQMA4GA1UEBwwHQW5zYmFjaDEVMBMGA1UECgwMU3RlZmFuIFNpZWdsMRIwEAYD
VQQLDAlHZWllcmxlaW4xFjAUBgNVBAMMDUdlaWVybGVpbiBERVYxIzAhBgkqhkiG
9w0BCQEWFHN0ZXNpZUBicm9rZW5waXBlLmRlMIIBIjANBgkqhkiG9w0BAQEFAAOC
AQ8AMIIBCgKCAQEAywBtDh9Z68eo/UrXL97CkxLe9ii8G2jsiwoGrS/c2YLaQ9/c
2HJpIp+M45Lm4A840t98tyT6IZ04ssWJro5KkzrS3JAhX2UehGHt84Rg5FpvRn5o
FRlwQZP3Ki0E6tpfVhspzl/1c77zR4bhdi9vm5rU0evFap7jDanfMYkIo77Aem8a
RsrPSd+7fqPBbPlqKF8eL2Gn/GzyZ8fzqYgqIPt/ZfYp5nU8r1G+mkDRfeUtvZUs
6oy34UdaJzJn/COFBnihbnmWfbJglRD5p2WBpic+u2ezGZtPEz732gXQXb8eYas2
zyctlK9rVXL6GaOZbPr87xnGGIiPugFGphwChwIDAQABMA0GCSqGSIb3DQEBBQUA
A4IBAQC9++27fUYUE7n6YWM8ChHgGXMqr8fcQ86pLxyb9OMeANEAvBKfApgIWz9t
eoTiI5MPqi1XhO6xfcQ9uova/NlARxmfqlpT+hllVfBCoypjm1/a15CI3GrE2ZIg
Q9Ec6vZBUFUjHZgXg+jz0oZSon27/f/XSUOpHCmxF6KOvlQq/lrKARyfBxbz417i
tPH3fhQOy60obbR2vm2tl9ZBFVL19L0IXAl6ERccAxRz/T77zQ2F9C2GZZlaVYzV
Hd2vhOsg+1Z2fnPQy0Z4O+oGTseMauFxVLqQCzJn3L+V8s+MG7GVAAfO0QkJaAjh
Nbf9EuGB+DaAjWegzafzgJ2aKx+SMYICQDCCAjwCAQEwgakwgZsxCzAJBgNVBAYT
AkRFMRIwEAYDVQQIDAlGcmFuY29uaWExEDAOBgNVBAcMB0Fuc2JhY2gxFTATBgNV
BAoMDFN0ZWZhbiBTaWVnbDESMBAGA1UECwwJR2VpZXJsZWluMRYwFAYDVQQDDA1H
ZWllcmxlaW4gREVWMSMwIQYJKoZIhvcNAQkBFhRzdGVzaWVAYnJva2VucGlwZS5k
ZQIJANRUHEDYNeLzMA0GCWCGSAFlAwQCAQUAoGkwGAYJKoZIhvcNAQkDMQsGCSqG
SIb3DQEHATAvBgkqhkiG9w0BCQQxIgQgL/TDBQ+1LmkIk4u0TacbQj6GvXtVN5Sr
LFvTa+Sr82MwHAYJKoZIhvcNAQkFMQ8XDTUwMDEwMTAwMDAwMFowDQYJKoZIhvcN
AQEBBQAEggEANPSy+zPvWv9Bv7AeOZ3cBNkyOYGuqjkRQ3i8FHZjtaJRPhPZVdwH
LLNqoxKdCfEhDoU2gcyCTKU21CGENTceoS6FkX4aH0vfTxJKZYRZqw9m/V0PpxDt
gjNpYZW+qM2pJ45oLpTp96yu96Nr//IZC5vfsvN2dR4r0Z2bu8vGGuL2rkotLZLy
gjUHIwQpVaPEOkuKxTAK2XjPoD1zwtb2oDG8ud17y2QC6UsRr/W2JQ19MNgwJd71
ejPlrMXEI+KNbdFwq3Q309rn/54woJ2XWv03o7QNgrxfci2HsEJXVx6jalZmcRbk
ekI/OKxR6l76hVhFlKHtWVSgkLHs3LfPzg==
-----END PKCS7-----

[dghgit]

Frankly, I'm surprised OpenSSL would validate an invalid message. Likewise I am extremely surprised that it would not validate a valid message with disordered signed attributes.

If it's any help, RFC 5652, section 5.4 (and it's predecessors) is quite explicit about the need to DER encode signed attributes, the message with unsorted attributes is valid providing the signature is calculated over the DER encoding. As I rule we would only write things out in DER encoding as it means the message will validate even if the parser checking it is broken, but there are occasions where people do not save the attributes in DER ordering.

Glad to hear it will be fixed.

[dhensby]

Interesting, thanks. So then the BC behaviour appears to be correct (calculate over theoretical bytes not supplied bytes) and OpenSSL is wrong.

Thanks for the feedback on this and hopefully this will help anyone else who stumbles upon this problem.