GCMParams in EnvelopedData required to be OCTET STRING instead of SEQUENCE

[gnarea]

I have this CMS EnvelopedData generated with BouncyCastle. You'll find that encryptedContentInfo.contentEncryptionAlgorithm.algorithmParams is defined as a SEQUENCE:

This is consistent with RFC 5084 (Section 3.2), which defines the params of an AlgorithmIdentifier for AES-GCM as the following SEQUENCE:

      GCMParameters ::= SEQUENCE {
        aes-nonce        OCTET STRING, -- recommended size is 12 octets
        aes-ICVlen       AES-GCM-ICVlen DEFAULT 12 }

      AES-GCM-ICVlen ::= INTEGER (12 | 13 | 14 | 15 | 16)

However, PKI.js assumes this is an OCTET STRING:

https://github.com/PeculiarVentures/PKI.js/blob/fa83e1ef003c8b450832b8c878cd36b73aaa0dee/src/EnvelopedData.js#L735 https://github.com/PeculiarVentures/PKI.js/blob/fa83e1ef003c8b450832b8c878cd36b73aaa0dee/src/EnvelopedData.js#L1698

So if it gets a SEQUENCE, decryption fails with an error like this:

Error: iv: Must have length more than 0 and less than 2^64 - 1
    at AesGcmProvider.checkAlgorithmParams (/opt/gw/node_modules/webcrypto-core/build/webcrypto-core.js:395:19)
    at AesGcmProvider.checkDecrypt (/opt/gw/node_modules/webcrypto-core/build/webcrypto-core.js:180:14)
    at AesGcmProvider.decrypt (/opt/gw/node_modules/webcrypto-core/build/webcrypto-core.js:175:27)
    at SubtleCrypto.decrypt (/opt/gw/node_modules/webcrypto-core/build/webcrypto-core.js:809:39)
    at CryptoEngine.decrypt (/opt/gw/node_modules/pkijs/build/CryptoEngine.js:771:24)
    at /opt/gw/node_modules/pkijs/build/EnvelopedData.js:1424:21
    (...)

... Because the valueBlock for the algorithmParams doesn't have a valueHex. It does, however, a value with the two items in the SEQUENCE:

So, how can we fix this? Fixing the decryption case is easy because we could use value if valueHex is undefined. But how can we fix the encryption? This could break production code, so should it be controlled by an argument passed to the encrypt() method?

[gnarea]

Side note: It seems like AES-GCM-ICVlen isn't used.

The aes-ICVlen parameter field tells the size of the message authentication code. It MUST match the size in octets of the value in the AuthEnvelopedData mac field. A length of 12 octets is RECOMMENDED.

https://www.rfc-editor.org/rfc/rfc5084#section-3.2

[YuryStrozhevsky]

@gnarea Most probably there is an error in BouncyCastle. So, the deal is that there are different types of "encrypted data". If you will take a look at RFC5652 you will see there are:

• Signed-data Content Type;
• Enveloped-Data Content Type;
• Digested-Data Content Type;
• Encrypted-Data Content Type;
• Authenticated-Data Content Type;

The RFC8084 is for "Authenticated-Data Content Type". The class you are referencing in PKI.js is for "Enveloped-Data Content Type". But I was saying about possible error in BouncyCastle because as I can see you have incorrect OID at the beginning of your data:

OBJECT IDENTIFIER envelopedData (1 2 840 113549 1 7 3)

So, it could be an error in BouncyCastle or it was made manually by yourself somehow. In any case you have an incorrect mix in your data: OID from EnvelopedData, but ASN.1 data from "Authenticated-Data Content Type".

[gnarea]

Thanks for getting back @YuryStrozhevsky!

If PKI.js isn't implementing RFC 5084 in EnvelopedData values using AES-GCM, which RFC is it implementing?

I can't find another RFC describing the parameters of an AlgorithmIdentifier for AES-GCM. In fact, both Bouncy Castle and PKI.js are using 2.16.840.1.101.3.4.1.6 as the AlgorithmIdentifier OID, which is defined in RFC5084.

https://github.com/PeculiarVentures/PKI.js/blob/6c8bee398dd2d9d76eb584c2d25a90e3b8c98380/src/CryptoEngine.js#L985-L989

The RFC5084 is for "Authenticated-Data Content Type"

I've just taken a closer look and RFC 5084 doesn't apply to either EnvelopedData or AuthenticatedData: It applies to a new EnvelopedData-like content type defined in RFC 5083 (AuthEnvelopedData). I'd imagine that's why BouncyCastle is sticking to RFC 5084.

[YuryStrozhevsky]

@gnarea RFC5652, as a base. I did all this stuff many years ago and do not remember all details but all was tested against MS CryptoAPI and OpenSSL implementations.

[gnarea]

@YuryStrozhevsky, fair enough. How should we proceed at this point?

I think complying with RFC 5084 is very important because people using AES-GCM want authenticated encryption, and PKI.js isn't currently considering the MAC. Additionally, there's the interoperability issue I ran into. However, I understand that complying with RFC 5084 could break production code for other people if we're not careful.

On my end, I'm going to have to temporarily downgrade to a cipher without authenticated encryption (AES-CBC) so that I can move on with the integration of the Bouncy Castle code. We're not running this in production yet.

If by the time we're preparing for the production rollout this hasn't been implemented, I'll be happy to contribute a PR. Does that sound like something you'd be open to in principle?

[YuryStrozhevsky]

@gnarea All depends on @rmhrisk. I am just a freelancer. He paid - I made :)

[rmhrisk]

We want this to be RFC compliant and secure but if as Yury said this is an oddity in Bouncy Castle and PKIjs is then we should leave it as is. I don't have the time to investigate right now to draw my own conclusions though. @gnarea since you have the motivation given your current situation please help us come to a conclusion on this. As for breaking changes, that's what major version changes are for.

[gnarea]

@rmhrisk, I don't think this is just an oddity with Bouncy Castle and PKI.js. I believe PKI.js isn't RFC compliant here: I can't find any RFC that supports the schema PKI.js is using when dealing with AES-GCM. Also, the AlgorithmIdentifier OID that PKI.js/BC are using is only defined in RFC 5084, which I believe confirms they should both comply with this RFC.

However, to fully comply with RFC 5084, we also must comply with RFC 5083 and use the AuthEnvelopedData. It'd be mostly the current implementation of EnvelopedData, so you could keep that class if you want, but at least the following changes should be made when the algorithm is AES-GCM (or AES-CCM, if it's also supported):

• Use the AuthEnvelopedData OID instead of the one for EnvelopedData.
• Use the GCM/CCM MAC.

On the other hand, I've just found out that openssl cms -encrypt -aes-128-gcm (...) and openssl cms -decrypt (...) fail when using AES-GCM because the latest stable release (on Ubuntu) doesn't support authenticated encryption ciphers. Coincidentally, they've just merged a PR that adds support for AE ciphers by supporting RFCs 5083 and 5084.

[rmhrisk]

If your game to do a PR to match the OpenSSL pattern were game.

[gnarea]

Great! Yeah, that's basically my thinking. I had a cursory look at the PR and it seems to match my understanding of the two RFCs.

I'm going to try to compile the latest OpenSSL commit tonight so I can test it just in case.

[YuryStrozhevsky]

@gnarea PKIjs is fully RFC compliant. I just said you I don’t remember all details.

[rmhrisk]

@YuryStrozhevsky I am at a loss. We appear to have OpenSSL doing one thing, us doing another and @gnarea seems to have provided credible references to RFCs that suggest that what OpenSSL is doing is RFC compliant. Can you help me understand the gap?

[YuryStrozhevsky]

As is said the RFC5083 is abou AutheticatedData but we don’t support it yet. We support only EnvelopedData. Also I should say that we aligned with RFC5652 and the RFC has bigger number than RFC5083 and thus superseded older RFC for sure. The only thing @gnarea has no idea about is the RFC where stated parameters for AES-GCM we aligned with. So I will try to find the info later.

[gnarea]

RFC 5083 isn't about the Authenticated Data content type in CMS. It really is about Enveloped Data -- more precisely, it creates a new content type that extends Enveloped Data to support authenticated encryption ciphers.

From the abstract:

(...) All of the various key management techniques that are supported in the CMS enveloped-data content type are also supported by the CMS authenticated-enveloped-data content type.

[YuryStrozhevsky]

@gnarea Could you please put here a title of the RFC5083?

[rmhrisk]

"Cryptographic Message Syntax (CMS) Authenticated-Enveloped-Data Content Type"

[YuryStrozhevsky]

But PKIJS support EnvelopedData only. No support yet for the Authenticated-Enveloped Data.

[YuryStrozhevsky]

Also there is no support for this type in RFC5652.

[rmhrisk]

It seems our allowance for GCM in the use of EnvelopedData is the root of the interop issue. I agree the PR is about adding support for this new type.

[gnarea]

Agreed.

I've just managed to compile and test the master branch of OpenSSL:

$  /usr/local/bin/openssl cms -encrypt -aes-128-gcm -in /tmp/plaintext -out /tmp/enveloped-data.pem -outform PEM -recip /tmp/cert.pem

$ cat enveloped-data.pem 
-----BEGIN CMS-----
MIIBtgYLKoZIhvcNAQkQARegggGlMIIBoQIBADGCAUgwggFEAgEAMCwwFDESMBAG
A1UEAwwJbG9jYWxob3N0AhQA03w9POdZZ6fuTi4BcLTHr5kIUzANBgkqhkiG9w0B
AQEFAASCAQCVLLmHNu/iwdPmCNpmyAKzfknRrmJhEaWCE64abaNUpOdGD69E5p4X
zp1psFmL3w4hgxxH0YQZCk8agnlrYLQH97uuoJ8pFxuqJCfC5RRDJKRa47kwKVUU
mrR04Ai725UwvCdJAlR4gYb7y/wP7MsLjiJAQz0HEnuxBICO26KJermWdyPhit1Q
kGjjERLWnBCu0IwHdfCEUufSc6QhccekTFjiNWwbqSm6DHW0YnEmvyc2NnfJniTr
bOMb04sfz46e+DqGiQ8DhUrK3TpYS6ZCl9tU417jK4J6FLLy4PnYR2opGMlH6dpy
wFTxUbSJf+ETnaGnL8MVEbHZEk9y/N8VMD4GCSqGSIb3DQEHATAeBglghkgBZQME
AQYwEQQMrFOlhQ4llYNIFeOVAgEQgBG+laHXb/0s5G/TK2bMDF4/2AQQaMf8gpnx
bPwEFdcQhRJl/A==
-----END CMS-----

The PEM above complies with RFCs 5083 and 5084 as far as I can tell:

https://lapo.it/asn1js/#MIIBtgYLKoZIhvcNAQkQARegggGlMIIBoQIBADGCAUgwggFEAgEAMCwwFDESMBAGA1UEAwwJbG9jYWxob3N0AhQA03w9POdZZ6fuTi4BcLTHr5kIUzANBgkqhkiG9w0BAQEFAASCAQCVLLmHNu_iwdPmCNpmyAKzfknRrmJhEaWCE64abaNUpOdGD69E5p4Xzp1psFmL3w4hgxxH0YQZCk8agnlrYLQH97uuoJ8pFxuqJCfC5RRDJKRa47kwKVUUmrR04Ai725UwvCdJAlR4gYb7y_wP7MsLjiJAQz0HEnuxBICO26KJermWdyPhit1QkGjjERLWnBCu0IwHdfCEUufSc6QhccekTFjiNWwbqSm6DHW0YnEmvyc2NnfJniTrbOMb04sfz46e-DqGiQ8DhUrK3TpYS6ZCl9tU417jK4J6FLLy4PnYR2opGMlH6dpywFTxUbSJf-ETnaGnL8MVEbHZEk9y_N8VMD4GCSqGSIb3DQEHATAeBglghkgBZQMEAQYwEQQMrFOlhQ4llYNIFeOVAgEQgBG-laHXb_0s5G_TK2bMDF4_2AQQaMf8gpnxbPwEFdcQhRJl_A

Here's a Dockerfile if you'd like to replicate my tests:

FROM ubuntu:20.04

# Install build dependencies
RUN apt-get update && \
    apt-get upgrade -y && \
    apt-get install -y git build-essential wget unzip && \
    (echo | cpan) && \
    cpan -i Text::Template

# Install OpenSSL on /usr/local
RUN wget https://github.com/openssl/openssl/archive/master.zip && \
    unzip master.zip && \
    cd openssl-master && \
    ./Configure linux-x86_64 && \
    make && \
    make install && \
    ldconfig

# Generate a key pair and self-issue cert
RUN /usr/local/bin/openssl req -x509 -newkey rsa:2048 -keyout /tmp/key.pem -out /tmp/cert.pem -days 1 -subj '/CN=localhost' -nodes && \
    echo "such plain text" > /tmp/plaintext

RUN /usr/local/bin/openssl cms -encrypt -aes-128-gcm -in /tmp/plaintext -out /tmp/enveloped-data.pem -outform PEM -recip /tmp/cert.pem

[lukaaash]

I also encountered this issue when testing interoperability of our C# CMS implementation with PKI.js using test data generated by SMIMEEncryptionExample. I agree with @gnarea that PKI.js is not RFC-compliant. PKI.js utilizes the algorithm identifiers specified by RFC 5084, but uses them in a way that is apparently incompatible with the specification as prescribed by that RFC:

1. Instead of using GCMParameters (with nonce and optional IVlen), it uses an OCTET STRING with just the nonce. This is clearly wrong.

2. Instead of using RFC 5083's Authenticated-Enveloped-Data (which adds support to integrity check value (ICV) needed by AES/GCM), it utilizes Enveloped-Data. However, Enveloped-Data lacks the ICV field, so PKI.js instead appends the ICV to the end of encrypted content. But that approach does not seem to have basis in any RFC! Additionally, it's hardcoded to use ICV length of 16 bytes, which again has no basis in RFC 5084.

In the meantime, OpenSSL's implementation made it into OpenSSL 3.0. It seems to be correct and compliant with both RFC 5083 and 5084. On the other hand, PKI.js implementation seems to be just made-up, and it definitely should be fixed.