Skip to content

Commit

Permalink
fix(SignedData): Allow eContent to be OCTET-STRING of SEQUENCE (#…
Browse files Browse the repository at this point in the history
…502)

(When verifying `SignedData` values, not when producing them)

This is actually the correct way to represent RAMF messages, but PKI.js unfortunately generates an `OCTET-STRING` containing an `OCTET-STRING` containing a `SEQUENCE` (because PKI.js parses the `OCTET-STRINGS` eagerly). Bouncy Castle (used by the JVM lib) does the right thing but the JS lib fails to process such values -- hence this PR.

This is needed in relaycorp/awala-jvm#254
  • Loading branch information
gnarea authored Aug 9, 2022
1 parent 8bb4a3e commit ac6e5de
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 26 deletions.
15 changes: 5 additions & 10 deletions src/lib/crypto_wrappers/cms/signedData.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,10 +207,7 @@ describe('sign', () => {
expect(encapContentInfo).toBeInstanceOf(pkijs.EncapsulatedContentInfo);
expect(encapContentInfo).toHaveProperty('eContentType', CMS_OIDS.DATA);
expect(encapContentInfo).toHaveProperty('eContent');
if (!encapContentInfo.eContent) {
throw new Error('encapContentInfo.eContent is empty');
}
const plaintextOctetString = encapContentInfo.eContent.valueBlock
const plaintextOctetString = encapContentInfo.eContent!.valueBlock
.value[0] as asn1js.OctetString;
expectArrayBuffersToEqual(
plaintextOctetString.valueBlock.valueHexView.slice().buffer,
Expand Down Expand Up @@ -368,25 +365,23 @@ describe('verify', () => {

describe('plaintext', () => {
test('Nothing should be output if plaintext is absent', async () => {
const signedData = await SignedData.sign(plaintext, keyPair.privateKey, certificate);
// @ts-ignore
// tslint:disable-next-line:no-delete
delete signedData.pkijsSignedData.encapContentInfo.eContent;
const pkijsSignedData = new pkijs.SignedData();
const signedData = new SignedData(pkijsSignedData);

await expect(signedData.plaintext).toBeNull();
});

test('Plaintext should be output if present', async () => {
const signedData = await SignedData.sign(plaintext, keyPair.privateKey, certificate);

expect(signedData.plaintext).toEqual(plaintext);
expectArrayBuffersToEqual(plaintext, signedData.plaintext!);
});

test('Large plaintexts chunked by PKI.js should be put back together', async () => {
const largePlaintext = arrayBufferFrom('a'.repeat(2 ** 20));
const signedData = await SignedData.sign(largePlaintext, keyPair.privateKey, certificate);

expect(signedData.plaintext).toEqual(largePlaintext);
expectArrayBuffersToEqual(largePlaintext, signedData.plaintext!);
});
});

Expand Down
19 changes: 3 additions & 16 deletions src/lib/crypto_wrappers/cms/signedData.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// tslint:disable:no-object-mutation

import * as asn1js from 'asn1js';
import bufferToArray from 'buffer-to-arraybuffer';
import * as pkijs from 'pkijs';

import { CMS_OIDS } from '../../oids';
Expand All @@ -27,22 +26,10 @@ interface SignedDataOptions extends SignatureOptions {
export class SignedData {
/**
* The signed plaintext, if it was encapsulated.
*
* TODO: Cache output because computation can be relatively expensive
*/
get plaintext(): ArrayBuffer | null {
if (this.pkijsSignedData.encapContentInfo.eContent === undefined) {
return null;
}
// ASN1.js splits the payload into 65 kib chunks, so we need to put them back together
const contentOctetStringChunks =
this.pkijsSignedData.encapContentInfo.eContent.valueBlock.value;
const contentChunks = contentOctetStringChunks.map(
(os) => (os as asn1js.OctetString).valueBlock.valueHex,
);
const content = Buffer.concat(contentChunks.map((c) => new Uint8Array(c)));

return bufferToArray(content);
const eContent = this.pkijsSignedData.encapContentInfo.eContent;
return eContent?.getValue() ?? null;
}

/**
Expand Down Expand Up @@ -240,7 +227,7 @@ export async function verifySignature(

return {
attachedCertificates: Array.from(signedData.certificates),
plaintext: signedData.plaintext as ArrayBuffer,
plaintext: signedData.plaintext!,
signerCertificate: signedData.signerCertificate as Certificate,
};
}

0 comments on commit ac6e5de

Please sign in to comment.