bcgit / bc-java

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

EnvelopedData: Curve name is missing from OriginatorPublicKey in KeyAgreeRecipientInfo #893

Closed gnarea closed 3 years ago

gnarea commented 3 years ago

When I generate a CMS EnvelopedData containing a KeyAgreeRecipientInfo, the recipient info will have an OriginatorPublicKey whose key's AlgorithmIdentifier is missing the curve name -- Which must be set as the algorithm parameters per RFC 5480.

Consider this CMS EnvelopedData generated with the code below:

import org.bouncycastle.cms.CMSAlgorithm
import org.bouncycastle.cms.CMSEnvelopedData
import org.bouncycastle.cms.CMSEnvelopedDataGenerator
import org.bouncycastle.cms.CMSProcessableByteArray
import org.bouncycastle.cms.jcajce.JceCMSContentEncryptorBuilder
import org.bouncycastle.cms.jcajce.JceKeyAgreeRecipientInfoGenerator
import org.bouncycastle.jce.provider.BouncyCastleProvider
import org.bouncycastle.util.encoders.Base64
import java.security.KeyPair
import java.security.KeyPairGenerator
import java.security.PublicKey
import java.security.spec.ECGenParameterSpec

val bcProvider = BouncyCastleProvider()

fun generateECDHKeyPair(curveName: String = "P-256"): KeyPair {
    val keyGen = KeyPairGenerator.getInstance("EC", bcProvider)
    val ecSpec = ECGenParameterSpec(curveName)
    keyGen.initialize(ecSpec)
    return keyGen.generateKeyPair()
}

fun bcEncrypt(
    plaintext: ByteArray,
    recipientKeyId: ByteArray,
    recipientPublicKey: PublicKey,
    senderKeyPair: KeyPair
): CMSEnvelopedData {
    val cmsEnvelopedDataGenerator = CMSEnvelopedDataGenerator()

    val recipientInfoGenerator = JceKeyAgreeRecipientInfoGenerator(
        CMSAlgorithm.ECDH_SHA256KDF,
        senderKeyPair.private,
        senderKeyPair.public,
        CMSAlgorithm.AES128_WRAP
    ).addRecipient(recipientKeyId, recipientPublicKey)
    cmsEnvelopedDataGenerator
        .addRecipientInfoGenerator(recipientInfoGenerator.setProvider(bcProvider))

    val msg = CMSProcessableByteArray(plaintext)
    val encryptorBuilder =
        JceCMSContentEncryptorBuilder(CMSAlgorithm.AES128_CBC).setProvider(bcProvider)
    return cmsEnvelopedDataGenerator.generate(msg, encryptorBuilder.build())
}

val senderKeyPair = generateECDHKeyPair()
val recipientKeyPair = generateECDHKeyPair()
Base64.toBase64String(senderKeyPair.public.encoded)

val envelopedData = bcEncrypt(
    "the plaintext".toByteArray(),
    "the key id".toByteArray(),
    recipientKeyPair.public,
    senderKeyPair
)
Base64.toBase64String(envelopedData.encoded)

Its OriginatorPublicKey has its AlgorithmIdentifier's parameters set to NULL, which is illegal per RFC 5480:

algoid

The parameter for id-ecPublicKey is as follows and MUST always be present:

ECParameters ::= CHOICE {
  namedCurve         OBJECT IDENTIFIER
  -- implicitCurve   NULL
  -- specifiedCurve  SpecifiedECDomain
}
  -- implicitCurve and specifiedCurve MUST NOT be used in PKIX.

(...) implicitCurve allows the elliptic curve domain parameters to be inherited. This choice MUST NOT be used.

Note that the public key that gets passed to JceKeyAgreeRecipientInfoGenerator() does have the curve name included (as expected), so there must be something along the way that drops the algorithm identifier parameters. But I can't find the place where that's happening. Here's what I get if I encode the senderKeyPair.public above:

algoid-param

I need the namedCurve to be present because my JVM code communicates with a JavaScript peer, which uses the WebCrypto API. And the WebCrypto API requires the curve name to be passed when decoding a public key.

dghgit commented 3 years ago

For some reason, probably the wrong one, the algorithm parameters were been set to NULL. I couldn't find any reason for this in RFC 5652 so I've fixed it so that OriginatorPublicKey always reflects everything in the SubjectPublicKeyInfo of the key being used. The fix for this will appear in 169b04, which will appear on https://www.bouncycastle.org/betas in a few hours.