PeculiarVentures / PKI.js

PKI.js is a pure JavaScript library implementing the formats that are used in PKI applications (signing, encryption, certificate requests, OCSP and TSP requests/responses). It is built on WebCrypto (Web Cryptography API) and requires no plug-ins.
http://pkijs.org
Other
1.25k stars 204 forks source link

How can I use RecipientKeyIdentifier in a CMS EnvelopedData value? #304

Closed gnarea closed 2 years ago

gnarea commented 3 years ago

I'm using the KeyAgreeRecipientInfo type, but I want to use a RecipientKeyIdentifier instead of a IssuerAndSerialNumber in the KeyAgreeRecipientIdentifier choice. However, it seems like only IssuerAndSerialNumber is supported (in envelopedData.addRecipientByCertificate()):

https://github.com/PeculiarVentures/PKI.js/blob/0095250f7ca9bb29370d97793d430ff0b7158631/src/EnvelopedData.js#L444-L450

I think envelopedData.addRecipientByCertificate() already does almost everything I need, so I could add the RecipientInfo manually by using most of that code and changing the lines above with:

 rid: new KeyAgreeRecipientIdentifier({ 
    variant: 2, 
    value: new RecipientKeyIdentifier({ 
        subjectKeyIdentifier: new asn1js.OctetString(...)
    }) 
 }) 

However, the next issue I found is that KeyAgreeRecipientInfo takes a recipientCertificate, which I don't have here and isn't part of the spec:

https://github.com/PeculiarVentures/PKI.js/blob/0095250f7ca9bb29370d97793d430ff0b7158631/src/EnvelopedData.js#L494 https://github.com/PeculiarVentures/PKI.js/blob/682ef8d148e03adc4d7fe5ea1a0cdce4294f5e38/src/KeyAgreeRecipientInfo.js#L50-L54

Am I missing something and RecipientKeyIdentifier is actually supported?

If it indeed isn't supported, I'd be happy to propose a PR, which should be fairly straightforward (e.g., EnvelopedData.addRecipientBy[Subject]Key()), but I'm a bit concerned about what to do with KeyAgreeRecipientInfo.recipientCertificate, so any pointers would be much appreciated. Presumably, what we actually care about is the subject's public key in the certificate, not the whole certificate, right?

microshine commented 3 years ago

As I can see from the source code recipientCertificate is an extra field that keeps Certificate for some reasons. I think for parsed EnvelopedData that field will be empty.

I'm not sure about EnvelopedData.addRecipientBy[Subject]Key(). Looks like you need a certificate to fill RecipientKeyIdentifier

RFC5662 - KeyAgreeRecipientInfo Type subjectKeyIdentifier identifies the recipient's certificate by a key identifier. When an X.509 certificate is referenced, the key identifier matches the X.509 subjectKeyIdentifier extension value. When other certificate formats are referenced, the documents that specify the certificate format and their use with the CMS must include details on matching the key identifier to the appropriate certificate field.

I like idea of CMSRecipient class from .NET. It allows selecting how to keep information about the recipient in CMS message.

microshine commented 3 years ago

Maybe improve addRecipientByCertificate function and support option with recipient type (eg. SubjectIdentifierType) instead of variant option

Current function

addRecipientByCertificate(certificate, parameters, variant);

New version

addRecipientByCertificate(certificate, parameters, type);

// Example

envelopedData.addRecipientByCertificate(cert, encParams, SubjectIdentifierType.subjectKeyIdentifier);
gnarea commented 3 years ago

Thanks for looking into this @microshine!

If you're using RecipientKeyIdentifier, you don't have an X.509 certificate. It might not even exist (e.g., it could be a non-X.509 certificate). All you have is an identifier, plus some optional metadata:

      RecipientKeyIdentifier ::= SEQUENCE {
        subjectKeyIdentifier SubjectKeyIdentifier,
        date GeneralizedTime OPTIONAL,
        other OtherKeyAttribute OPTIONAL }

So I think repurposing addRecipientByCertificate() would be inconvenient for the developer because we'd have to create a fake, temporary certificate. For example:

async function addRecipientToEnvData(envelopedData, recipientKeyId, recipientPublicKey) {
    const fakeCertificate = new pkijs.Certificate({
        extensions: [
            new pkijs.Extension({
                extnID: '2.5.29.14',
                extnValue: new asn1js.OctetString({ valueHex: recipientKeyId }).toBER(false),
            }),
        ]
    });
    await fakeCertificate.subjectPublicKeyInfo.importKey(recipientPublicKey);

    envelopedData.addRecipientByCertificate(fakeCertificate, encParams, SubjectIdentifierType.subjectKeyIdentifier);

    return envelopedData;
}

By contrast, if you create a new method, there would be no need for the developer to create a fake certificate, so I could simply do:

async function addRecipientToEnvData(envelopedData, recipientKeyId, recipientPublicKey) {
    envelopedData.addRecipientBySubjectKey(recipientKeyId, recipientPublicKey);

    return envelopedData;
}
rmhrisk commented 3 years ago

Seems reasonable.

@gnarea would you be interested in doing a PR we would review?

microshine commented 3 years ago

@gnarea Could you share a simple example of code how you are creating an enveloped CMS?

gnarea commented 3 years ago

I would, @rmhrisk!

I think the tricky thing here is what to do about KeyAgreeRecipientInfo.recipientCertificate -- I think it should be replaced with a public key (i.e., a CryptoKey instance). But I'd like to hear your thoughts before I start the implementation.


@microshine, here's what I have, but it's using IssuerAndSerialNumber instead of RecipientKeyIdentifier:

  async function encrypt(
    plaintext: ArrayBuffer,
    pkijsCertificate: Certificate,
  ) {
    const pkijsEnvelopedData = new pkijs.EnvelopedData({});

    pkijsEnvelopedData.addRecipientByCertificate(pkijsCertificate, {}, 2);

    const [pkijsEncryptionResult]: ReadonlyArray<{
      readonly ecdhPrivateKey: CryptoKey;
    }> = await pkijsEnvelopedData.encrypt(
      { name: 'AES-CBC', length: 128 },
      plaintext,
    );
    const dhPrivateKey = pkijsEncryptionResult.ecdhPrivateKey;

    return { dhPrivateKey, pkijsEnvelopedData };
  }

So I'm just testing this and I'm creating a cert just to make the above work.

microshine commented 3 years ago

Please review that CMS. Is it what you need? CMS ASN.1

3082029806092a864886f70d010703a082028930820285020102a082013ba0820137308201333081d9a003020102020101300c06082a8648ce3d040302050030123110300e0603550403130754657374204341301e170d3139313233313231303030305a170d3230303130313231303030305a300f310d300b06035504031304546573743059301306072a8648ce3d020106082a8648ce3d03010703420004d5d702e3c471b92cad05a22ba017429915f5fed85215be0f72dc87f3fae14e344c058b2f897a77c288a195abd6131d3fdd6c0ed40004ce6d5521c179cb6ec393a321301f301d0603551d0e04160414c8d546eebc222d0b895407e38c01c87c16995bb7300c06082a8648ce3d040302050003470030440220978ed80118450387fed537e27c205f07f6e18651fa10e64c044597db3bfc97a90220922c8c5bbbba9ca6335f23653962be69f5813fd9c812dd97f327ce6056e6968c3181fca181f9020103a051a14f300906072a8648ce3d0201034200044540d7fef2c8acccdc96a03604750acd8d7526c3e11716e5c2750805af280a4e2a486286e7959554374d46050c29b014000e950bb15e428e400985378bb4debba142044060aac835b5336463311931a4b6aff9ab98f1267c881667cafc9add2993ce5f6bf5239f8a42375718aad8e13141935359f8eddf7994556b0dbd8eba579d92610d301706062b8104010b03300d060960864801650304012d050030443042a0160414c8d546eebc222d0b895407e38c01c87c16995bb704282aa217d9ffa6274801bfa58622a5cb626e759fca937442fc6dd5e3f70539983eb9613063b5ac071f308006092a864886f70d010701301d060960864801650304012a04102daaa87b7c1c81505e8eebc2dc633bada08004101087b56576e53bf5cb4e14994f6c66dc00000000

Here is my code

const asn = require("asn1js");
const { Crypto } = require("@peculiar/webcrypto");
const { EnvelopedData, Certificate, setEngine, CryptoEngine, OriginatorInfo, CertificateSet, ContentInfo } = require("pkijs");

const name = "Crypto";
const crypto = new Crypto;
setEngine(name, new CryptoEngine({ name, crypto, subtle: crypto.subtle }), new CryptoEngine({ name, crypto, subtle: crypto.subtle }));

async function main() {
  const certRaw = Buffer.from("MIIBMzCB2aADAgECAgEBMAwGCCqGSM49BAMCBQAwEjEQMA4GA1UEAxMHVGVzdCBDQTAeFw0xOTEyMzEyMTAwMDBaFw0yMDAxMDEyMTAwMDBaMA8xDTALBgNVBAMTBFRlc3QwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAATV1wLjxHG5LK0FoiugF0KZFfX+2FIVvg9y3Ifz+uFONEwFiy+JenfCiKGVq9YTHT/dbA7UAATObVUhwXnLbsOToyEwHzAdBgNVHQ4EFgQUyNVG7rwiLQuJVAfjjAHIfBaZW7cwDAYIKoZIzj0EAwIFAANHADBEAiCXjtgBGEUDh/7VN+J8IF8H9uGGUfoQ5kwERZfbO/yXqQIgkiyMW7u6nKYzXyNlOWK+afWBP9nIEt2X8yfOYFbmlow=", "base64");
  const certAsn = asn.fromBER(new Uint8Array(certRaw).buffer);
  const cert = new Certificate({ schema: certAsn.result });

  const data = new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8, 9, 0]).buffer;

  const cmsEnveloped = new EnvelopedData({
    originatorInfo: new OriginatorInfo({
      certs: new CertificateSet({
        certificates: [cert]
      }),
    })
  });

  cmsEnveloped.addRecipientByCertificate(cert, {});
  await cmsEnveloped.encrypt({
    name: "AES-CBC",
    length: 256,
  }, data);

  const cmsContent = new ContentInfo();
  cmsContent.contentType = "1.2.840.113549.1.7.3";
  cmsContent.content = cmsEnveloped.toSchema();

  const cmsRaw = cmsContent.toSchema().toBER(false);
  console.log(Buffer.from(cmsRaw).toString("hex"));
}

main().catch(e => console.error(e));

I changed source code from IssuerAndSerialNumber to RecipientKeyIdentifier image

microshine commented 3 years ago

I see the problem for addRecipientByCertificate. In the current version that function is sync. It's fine if the certificate has got SKI extension otherwise it must calculate that value from the certificate's public key. For that case, we need to use crypto.digest which is async.

microshine commented 3 years ago

It could be like this

enum RecipientType {
  issuerAndSerial,
  subjectKeyIdentifier,
}

declare class Recipient {

  /**
   * Create the new instance of Recipient.
   * - For SKI type uses SPI extension from the certificate otherwise computes hash from the certificate's public key
   * @param cert Certificate
   * @param type
   */
  static create(cert: Certificate, type: RecipientType): Promise<Recipient>;

  type: RecipientType;
  certificate: Certificate;

  // @internal
  // Keeps IssuerAndSerialNumber or RecipientKeyIdentifier structure
  asn: any;
}

declare class EnvelopedData {

  addRecipientCertificate(cert: Certificate | Recipient, parameters?: any, variant?: number): void

}

// Examples
const cert = new Certificate();
const envelopedData = new EnvelopedData();

envelopedData.addRecipientCertificate(cert); // from certificate

const recipient = await Recipient.create(cert, RecipientType.subjectKeyIdentifier);
envelopedData.addRecipientCertificate(recipient); // from recipient
gnarea commented 3 years ago

@microshine, the ASN.1 value you shared above looks good to me: The RecipientEncryptedKeys is indeed a sequence of one RecipientEncryptedKey whose rid is a RecipientKeyIdentifier:

ans1

      RecipientEncryptedKeys ::= SEQUENCE OF RecipientEncryptedKey

      RecipientEncryptedKey ::= SEQUENCE {
        rid KeyAgreeRecipientIdentifier,
        encryptedKey EncryptedKey }

      KeyAgreeRecipientIdentifier ::= CHOICE {
        issuerAndSerialNumber IssuerAndSerialNumber,
        rKeyId [0] IMPLICIT RecipientKeyIdentifier }

      RecipientKeyIdentifier ::= SEQUENCE {
        subjectKeyIdentifier SubjectKeyIdentifier,
        date GeneralizedTime OPTIONAL,
        other OtherKeyAttribute OPTIONAL }

      SubjectKeyIdentifier ::= OCTET STRING

As for the proposed API, what do you think if the developer didn't have to create an empty Certificate when using RecipientKeyIdentifiers? I think it'd be confusing to have to pass a certificate when adding a RecipientKeyIdentifier recipient to EnvelopedData.

EDIT: Or do you mean that the empty certificate would be created internally by PKI.js?

microshine commented 3 years ago

EDIT: Or do you mean that the empty certificate would be created internally by PKI.js? It's impossible to create a certificate using only the recipient's public key.

I'm trying to understand your task. Do you need to encrypt data to the recipient with a public key only?

gnarea commented 3 years ago

I'm trying to understand your task. Do you need to encrypt data to the recipient with a public key only?

Gotcha. Yes, a public key and an identifier for that key.

microshine commented 3 years ago

if so maybe

static create(key: CryptoKey): Promise<Recipient>;
static create(cert: Certificate, type: RecipientType): Promise<Recipient>;

EDIT:

static async create(key: CryptoKey): Promise<Recipient> {
  const spki = await crypto.subtle.exportKey("spki", key);
  const ski = await crypto.subtle.digest("SHA-1", spki);
  // ...
}
gnarea commented 3 years ago

Almost -- I think the first method also needs the key id:

static create(key: CryptoKey, keyId: ArrayBuffer): Promise<Recipient>;
static create(cert: Certificate, type: RecipientType): Promise<Recipient>;
gnarea commented 3 years ago

In my specific use case, I don't mind if the key id is generated for me or I create it, so having PKI.js generate the key id from the digest of the public key would work for me :+1:

microshine commented 3 years ago

In your API keyId allows to set the wrong value, otherwise, we need to compute the hash and compare it with the incomming option

gnarea commented 3 years ago

I'm not sure there's a right or wrong value -- I believe the key id could be a pseudo-random number, for example. It doesn't have to be the SHA-1/256/etc of the public key.

Here's what Bouncy Castle does, for example:

https://github.com/bcgit/bc-java/blob/2c0804d04e34adc68b589d3007cf4cf8b677e763/pkix/src/main/java/org/bouncycastle/cms/jcajce/JceKeyAgreeRecipientInfoGenerator.java#L116-L131

Note that it'd just take whatever subjectKeyID I pass.

But again, either way would work in my code. I just wanted to flag it in case it'd be problematic for other people.

microshine commented 3 years ago

Interesting BC implementation. Thank you for that link.

static create(key: CryptoKey, keyId?: ArrayBuffer): Promise<Recipient>;
gnarea commented 3 years ago

Great! That'd work for me :+1:

gnarea commented 2 years ago

FYI, I'm starting to work on this and I'll be implementing the interface above:

static create(key: CryptoKey, keyId?: ArrayBuffer): Promise<Recipient>;

I'll create a PR once I've had a chance to check the interoperability of the implementation with Bouncy Castle.

gnarea commented 2 years ago

@microshine, I'm re-reading our conversation above and I'm not sure it'd make sense to create the static method Recipient.create() based on the last few messages we exchanged.

As I understand it, we decided not to use certificates at all, so that to me means not using EnvelopedData.addRecipientByCertificate(). But the reason to define Recipient.create() was to pass its result to EnvelopedData.addRecipientByCertificate().

All we need is a public key and an id for that key, so we could just create a method like this in EnvelopedData:

addRecipientByKeyIdentifier(key: CryptoKey, keyId: ArrayBuffer)

What do you think?

gnarea commented 2 years ago

I went for the approach I suggested in my last comment and just created a PR (#333) with the changes.