openpgp.api: NullPointerException verifying a prefixed (non-one-pass) signed message when the signer certificate is not available

[Arpan0995]

Repro-PrefixedSigNPE.java

Summary

OpenPGPMessageInputStream throws an unchecked NullPointerException out of read() when it processes a prefixed (old-style, non-one-pass) signed message whose signer certificate the consumer has not supplied. Verifying a message from an unknown signer should end in a handled result or a checked PGPException, not an unchecked RuntimeException.

This also occurs for a well-formed, honestly-signed message: any prefixed-signature message whose signer key the consumer has not yet fetched crashes processing.

Environment

• bcprov 1.86.0.20694 / bcpg 1.86.0.20695 (current 1.86 beta), main at commit b51452f
• JDK 27

Steps to reproduce

Sign some data in the prefixed (non-one-pass) format, then process the message without adding the signer certificate:

OpenPGPApi api = new BcOpenPGPApi();
OpenPGPKey alice = api.generateKey(PublicKeyPacket.VERSION_4).classicKey("Alice <a@a>").build();
OpenPGPCertificate.OpenPGPComponentKey signComp = alice.getSigningKeys().get(0);
PGPPublicKey signPub = signComp.getPGPPublicKey();
PGPSecretKey signSec = alice.getPGPSecretKeyRing().getSecretKey(signPub.getKeyID());
PGPPrivateKey signPriv = signSec.extractPrivateKey(
    new BcPBESecretKeyDecryptorBuilder(new BcPGPDigestCalculatorProvider()).build(new char[0]));

byte[] data = "hello world".getBytes("UTF-8");

// prefixed (old-style, non-one-pass) signed message: [Signature][Literal]
PGPSignatureGenerator sGen = new PGPSignatureGenerator(
    new BcPGPContentSignerBuilder(signPub.getAlgorithm(), HashAlgorithmTags.SHA512), signPub);
sGen.init(PGPSignature.BINARY_DOCUMENT, signPriv);
sGen.update(data);
PGPSignature sig = sGen.generate();

ByteArrayOutputStream bOut = new ByteArrayOutputStream();
BCPGOutputStream pgpOut = new BCPGOutputStream(bOut);
sig.encode(pgpOut);                                  // signature first => prefixed
PGPLiteralDataGenerator lit = new PGPLiteralDataGenerator();
OutputStream lOut = lit.open(pgpOut, PGPLiteralData.BINARY, "", data.length, new Date());
lOut.write(data); lit.close(); pgpOut.close();
byte[] message = bOut.toByteArray();

// consumer processes the message but does not hold Alice's certificate
OpenPGPMessageProcessor proc = api.decryptAndOrVerifyMessage();   // no addVerificationCertificate(...)
OpenPGPMessageInputStream in = proc.process(new ByteArrayInputStream(message));
byte[] buf = new byte[4096];
while (in.read(buf) >= 0) { }                        // throws here
in.close();

Adding proc.addVerificationCertificate(alice.toCertificate()) before process(...) makes the same message read and verify cleanly, so the crash is specific to the missing-certificate case.

Actual behaviour

java.lang.NullPointerException: Cannot invoke "java.io.OutputStream.write(byte[], int, int)" because "this.sigOut" is null
    at org.bouncycastle.openpgp.PGPDefaultSignatureGenerator.blockUpdate(...)
    at org.bouncycastle.openpgp.PGPDefaultSignatureGenerator.update(...)
    at org.bouncycastle.openpgp.PGPSignature.update(...)
    at org.bouncycastle.openpgp.api.OpenPGPMessageInputStream$PrefixedSignatures.update(...)
    at org.bouncycastle.openpgp.api.OpenPGPMessageInputStream.read(...)

Expected behaviour

Processing completes, the plaintext is returned, and no unchecked exception reaches the caller. This is what the one-pass path already does for the same missing-certificate case: it skips the unverifiable signature (OnePassSignatures.verify, line 817) and returns the plaintext with no verified signature for the unknown signer. Carrying the prefixed signature through as present-but-unverified, so the caller can see that a signature was there, would be a further improvement, but it goes beyond what the one-pass path does today; the minimal fix is simply not to crash.

Root cause

In OpenPGPMessageInputStream.PrefixedSignatures:

• init(...) (OpenPGPMessageInputStream.java:875) only initialises a signature for verification when the issuer certificate is available: signature.signature.init(...) at line 898. When the signature has no resolvable issuer id, or the certificate is not supplied, it adds a null-issuer OpenPGPDocumentSignature and continues (lines 883 and 889), leaving the underlying PGPSignature uninitialised.
• update(byte[], int, int) (line 918) then iterates the raw prefixedSignatures list and calls signature.update(buf, off, len) on every entry unconditionally (line 923). For the uninitialised signature, PGPSignature.update reaches PGPDefaultSignatureGenerator.blockUpdate, where sigOut is null, so it throws on the first data byte during read(). update(int) (line 909) has the same shape.

The sanitize(...) call in verify() is never reached, because the stream crashes during read() before close() runs.

Comparison with the one-pass path

The one-pass path already guards this. OnePassSignatures.update(...) only updates a signature whose issuer is known: if (issuers.containsKey(onePassSignature)) (lines 787 and 799), and verify() skips key == null (line 817). The prefixed path has no equivalent guard.

Impact

A consumer that verifies a prefixed-signature message from a sender whose certificate it has not supplied receives an unchecked NullPointerException out of read(), rather than the handled outcome the API otherwise provides. This is inconsistent both with the API contract, where processing yields a result or a checked PGPException, and with the one-pass path, which handles the identical missing-certificate case gracefully. The missing-certificate case is ordinary (a message received before the sender's key has been fetched), and a caller that does not wrap read() in a catch for RuntimeException will have the exception propagate.

Suggested direction

Update and verify only those prefixed signatures whose underlying PGPSignature was successfully initialised, that is, where init(...) returned without throwing. Recording a signature as updatable inside the try block only after signature.signature.init(...) succeeds keeps both the missing-certificate case (lines 883 and 889) and an init failure (the catch at line 902) out of the update and verify sets, instead of feeding an uninitialised PGPSignature to update().

The one-pass path guards the common missing-certificate case (OnePassSignatures.update, lines 787 and 799), but it registers the issuer before calling init (issuers.put precedes ops.init in OnePassSignatures.init), so a guard keyed only on the issuer being known would still admit an init-failure case. Keying on successful initialisation covers both.

A self-contained reproducer is attached.