Skip to content

Commit

Permalink
Fix context verification with expectSigned: true: throw on invalid …
Browse files Browse the repository at this point in the history
…context

Verification wouldn't throw if the OpenPGP signature was valid
but the context was not matching.

This bug had no actual impact since `expectSigned` isn't used by any app yet.
  • Loading branch information
larabr committed Dec 5, 2024
1 parent 1f0c4ea commit d25ce43
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 6 deletions.
2 changes: 1 addition & 1 deletion lib/message/decrypt.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export default async function decryptMessage({
}

const decryptionResult = await decrypt(sanitizedOptions);
const verificationResult = handleVerificationResult(decryptionResult, context);
const verificationResult = handleVerificationResult(decryptionResult, context, options.expectSigned);

let verified = verificationResult.then((result) => result.verified);
let verifiedSignatures = verificationResult.then((result) => result.signatures);
Expand Down
3 changes: 2 additions & 1 deletion lib/message/verify.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ export function verifyMessage<DataType extends Data, FormatType extends VerifyOp
>;
export function handleVerificationResult<DataType extends Data>(
verificationResult: openpgp_VerifyMessageResult<DataType>,
context: ContextVerificationOptions
context?: ContextVerificationOptions,
expectSigned?: boolean
): Promise<VerifyMessageResult<DataType>>;

export interface VerifyCleartextOptionsPmcrypto extends Omit<VerifyOptions, 'message' | 'signature' | 'format'> {
Expand Down
14 changes: 10 additions & 4 deletions lib/message/verify.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const { NOT_SIGNED, SIGNED_AND_VALID, SIGNED_AND_INVALID } = VERIFICATION_STATUS
* signature: Promise<openpgp.signature.Signature>
* }
* @param {Object} context - context verification options
* @param {Boolean} expectSigned - whether a valid signature is expected; it causes the function to throw otherwise
* @returns {{
* data: Uint8Array|string|ReadableStream|NodeStream - message data,
* verified: constants.VERIFICATION_STATUS - message verification status,
Expand All @@ -26,7 +27,7 @@ const { NOT_SIGNED, SIGNED_AND_VALID, SIGNED_AND_INVALID } = VERIFICATION_STATUS
* errors: Error[]|undefined - verification errors if all signatures are invalid
* }}
*/
export async function handleVerificationResult(verificationResult, context) {
export async function handleVerificationResult(verificationResult, context, expectSigned) {
const { data, signatures: sigsInfo } = verificationResult;
const signatures = [];
const errors = [];
Expand All @@ -52,7 +53,7 @@ export async function handleVerificationResult(verificationResult, context) {
if (!context || isValidSignatureContext(context, verifiedSigPacket)) {
verificationStatus = SIGNED_AND_VALID;
} else {
errors.push(new ContextError());
errors.push(new ContextError('context verification error'));
}

if (!signatureTimestamp) {
Expand All @@ -63,6 +64,11 @@ export async function handleVerificationResult(verificationResult, context) {
}
}

if (expectSigned && verificationStatus !== SIGNED_AND_VALID) {
// this is primarily intended to throw context error, which isn't checked by OpenPGP.js
throw errors[0];
}

return {
data,
verified: verificationStatus,
Expand Down Expand Up @@ -106,7 +112,7 @@ export async function verifyMessage({
};

const verificationResult = await verify(sanitizedOptions);
return handleVerificationResult(verificationResult, context);
return handleVerificationResult(verificationResult, context, options.expectSigned);
}

/**
Expand All @@ -131,5 +137,5 @@ export async function verifyCleartextMessage({ cleartextMessage, date = serverTi
const sanitizedOptions = { ...options, date, message: cleartextMessage };

const verificationResult = await verify(sanitizedOptions);
return handleVerificationResult(verificationResult);
return handleVerificationResult(verificationResult, undefined, options.expectSigned);
}
9 changes: 9 additions & 0 deletions test/message/context.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,15 @@ describe('context', () => {
context: { value: 'unexpected-context', required: true }
});

// verificationWrongContext with `expectSigned`
await expect(verifyMessage({
textData,
signature: await readSignature({ armoredSignature }),
verificationKeys: [publicKey],
context: { value: 'unexpected-context', required: true },
expectSigned: true
})).to.be.rejectedWith(ContextError);

const verificationMissingContext = await verifyMessage({
textData,
signature: await readSignature({ armoredSignature }),
Expand Down

0 comments on commit d25ce43

Please sign in to comment.