Skip to content

Commit

Permalink
fix: Drop recipientCertificate parameter from SessionEnvelopedData.de…
Browse files Browse the repository at this point in the history
…crypt()
  • Loading branch information
gnarea committed Jan 16, 2020
1 parent 8d0a300 commit ce6de8d
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 29 deletions.
7 changes: 1 addition & 6 deletions src/integration_tests/channelSession.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,7 @@ test('Encryption and decryption with subsequent DH keys', async () => {
// Run 1: Alice initiates contact with Bob. Bob decrypts message.
const plaintext1 = bufferToArray(Buffer.from('Hi. My name is Alice.'));
const encryptionResult1 = await SessionEnvelopedData.encrypt(plaintext1, bobDhCertificate);
const decryptedPlaintext1 = await encryptionResult1.envelopedData.decrypt(
bobKeyPair1.privateKey,
bobDhCertificate,
);
const decryptedPlaintext1 = await encryptionResult1.envelopedData.decrypt(bobKeyPair1.privateKey);
expectBuffersToEqual(decryptedPlaintext1, plaintext1);
checkRecipientInfo(encryptionResult1.envelopedData, bobDhCertificate);

Expand All @@ -82,7 +79,6 @@ test('Encryption and decryption with subsequent DH keys', async () => {
const encryptionResult2 = await SessionEnvelopedData.encrypt(plaintext2, aliceDhCert1);
const decryptedPlaintext2 = await encryptionResult2.envelopedData.decrypt(
encryptionResult1.dhPrivateKey as CryptoKey,
aliceDhCert1,
);
expectBuffersToEqual(decryptedPlaintext2, plaintext2);
checkRecipientInfo(encryptionResult2.envelopedData, aliceDhCert1);
Expand All @@ -102,7 +98,6 @@ test('Encryption and decryption with subsequent DH keys', async () => {
const encryptionResult3 = await SessionEnvelopedData.encrypt(plaintext3, bobDhCert2);
const decryptedPlaintext3 = await encryptionResult3.envelopedData.decrypt(
encryptionResult2.dhPrivateKey as CryptoKey,
bobDhCert2,
);
expectBuffersToEqual(decryptedPlaintext3, plaintext3);
checkRecipientInfo(encryptionResult3.envelopedData, bobDhCert2);
Expand Down
39 changes: 25 additions & 14 deletions src/lib/crypto_wrappers/cms/envelopedData.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -446,18 +446,11 @@ describe('SessionEnvelopedData', () => {
describe('decrypt', () => {
test('Decryption with the wrong private key should fail', async () => {
const differentDhKeyPair = await generateECDHKeyPair();
const differentCertificate = await issueInitialDHKeyCertificate({
dhPublicKey: differentDhKeyPair.publicKey,
nodeCertificate,
nodePrivateKey,
serialNumber: 3,
validityEndDate: TOMORROW,
});
const { envelopedData } = await SessionEnvelopedData.encrypt(plaintext, bobDhCertificate);

expect.hasAssertions();
try {
await envelopedData.decrypt(differentDhKeyPair.privateKey, differentCertificate);
await envelopedData.decrypt(differentDhKeyPair.privateKey);
} catch (error) {
expect(error).toBeInstanceOf(CMSError);
expect(error.message).toStartWith(`Decryption failed: ${error.cause().message}`);
Expand All @@ -467,21 +460,39 @@ describe('SessionEnvelopedData', () => {
test('Decryption should succeed with the right private key', async () => {
const { envelopedData } = await SessionEnvelopedData.encrypt(plaintext, bobDhCertificate);

const decryptedPlaintext = await envelopedData.decrypt(bobDhPrivateKey, bobDhCertificate);
const decryptedPlaintext = await envelopedData.decrypt(bobDhPrivateKey);
expectBuffersToEqual(decryptedPlaintext, plaintext);
});

test('Recipient DH public key should be used to calculate shared secret', async () => {
test('Recipient DH certificate should be faked to set the key algorithm', async () => {
const { envelopedData } = await SessionEnvelopedData.encrypt(plaintext, bobDhCertificate);

jest.spyOn(pkijs.EnvelopedData.prototype, 'decrypt');
await envelopedData.decrypt(bobDhPrivateKey, bobDhCertificate);
await envelopedData.decrypt(bobDhPrivateKey);

const pkijsDecryptCall = getMockContext(pkijs.EnvelopedData.prototype.decrypt).calls[0];
const pkijsDecryptCallArgs = pkijsDecryptCall[1];
expect(pkijsDecryptCallArgs).toHaveProperty(
'recipientCertificate',
bobDhCertificate.pkijsCertificate,
expect(
pkijsDecryptCallArgs.recipientCertificate.subjectPublicKeyInfo.algorithm.algorithmParams.valueBlock.toString(),
).toEqual(
bobDhCertificate.pkijsCertificate.subjectPublicKeyInfo.algorithm.algorithmParams.valueBlock.toString(),
);
});

test('Decryption with mismatching key algorithms should fail', async () => {
// This is a safeguard for the workaround to avoid having to pass the recipientCertificate
// to PKI.js EnvelopedData.encrypt().
const { privateKey } = await generateECDHKeyPair('P-384');
expect((privateKey.algorithm as EcKeyAlgorithm).namedCurve).not.toEqual(
(bobDhPrivateKey.algorithm as EcKeyAlgorithm).namedCurve,
);
const { envelopedData } = await SessionEnvelopedData.encrypt(plaintext, bobDhCertificate);

await expectPromiseToReject(
envelopedData.decrypt(privateKey),
// Leaving the typos unchanged because they come from a dependency of PKI.js and I
// want to make sure this scenario fails for the expected reason.
new CMSError('Decryption failed: Named corves of EC keys are not equals'),
);
});
});
Expand Down
25 changes: 16 additions & 9 deletions src/lib/crypto_wrappers/cms/envelopedData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ export abstract class EnvelopedData {
});
return contentInfo.toSchema().toBER(false);
}

public abstract async decrypt(privateKey: CryptoKey): Promise<ArrayBuffer>;
}

/**
Expand Down Expand Up @@ -174,15 +176,20 @@ export class SessionEnvelopedData extends EnvelopedData {
return convertAsn1IntegerToNumber(recipientCertificate.serialNumber);
}

public async decrypt(
privateKey: CryptoKey,
dhRecipientCertificate: Certificate,
): Promise<ArrayBuffer> {
return pkijsDecrypt(
this.pkijsEnvelopedData,
privateKey,
dhRecipientCertificate.pkijsCertificate,
);
public async decrypt(dhPrivateKey: CryptoKey): Promise<ArrayBuffer> {
// PKI.js requires the entire recipient's **certificate** to decrypt, but the only thing it
// uses it for is to get the public key algorithm. Which you can get from the private key.
const recipientCertificate = this.pkijsEnvelopedData.recipientInfos[0].value
.recipientCertificate;
const dhCertificate: pkijs.Certificate = {
subjectPublicKeyInfo: {
// @ts-ignore
algorithm: {
algorithmParams: recipientCertificate.subjectPublicKeyInfo.algorithm.algorithmParams,
},
},
};
return pkijsDecrypt(this.pkijsEnvelopedData, dhPrivateKey, dhCertificate);
}
}

Expand Down

0 comments on commit ce6de8d

Please sign in to comment.