Skip to content

Commit ab7a235

Browse files
committed
OpenPGP: a truncated message is now reported rather than read as a clean end of message - the AEAD decryption streams no longer let an EOFException escape to BCPGInputStream.nextPacketTag(), and OpenPGPMessageInputStream.close() now closes its layer's integrity-protected stream so the SEIPDv1 MDC is always verified.
1 parent 832eece commit ab7a235

8 files changed

Lines changed: 426 additions & 25 deletions

File tree

‎docs/releasenotes.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,8 @@ Date: 2026, TBD
158158
- The structural check on decoded ASN.1 UTCTime / GeneralizedTime content introduced in 1.85 gave no way through for the zone-less UTCTime "YYMMDDHHMMSS", so a single such field made the structure carrying it unreadable: a CMS SignedData whose signing-time attribute lacked its trailing "Z" - a shape found in signatures in circulation - failed to load through both CMSSignedData and CMSSignedDataParser.getSignerInfos() with "invalid UTCTime format", and nothing else in the message could be examined. The value is not a legal UTCTime, X.680 sec. 47.3 making the zone mandatory, but ASN1UTCTime.getTime() has always carried an explicit branch reading it as GMT, so it does denote a real instant and re-encodes unchanged. Setting the new system/security property org.bouncycastle.asn1.allow_zoneless_utctime (Properties.ASN1_ALLOW_ZONELESS_UTCTIME) to "true" admits it - and admits nothing else: every other malformed value, the zone-less "YYMMDDHHMM" without seconds included, is still rejected with the property set, and GeneralizedTime, whose zone is optional to begin with, is unaffected. The property has to be set for the value to decode at all, generation is unaffected, and the value is still not DER, so writing it through a DEROutputStream is refused when Properties.ASN1_ALLOW_NON_DER_TIME is "false". Separately, CMSSignedDataParser.getSignerInfos() let a SignerInfo that failed to decode escape as an unchecked IllegalStateException; it now reports it as the CMSException the method declares (github #2411).
159159
- The BCJSSE key managers could not select an SM2 certificate for TLS 1.3. ProvX509KeyManager registers a TLS 1.3 EC public-key filter per named curve, but only for the NIST and Brainpool curves, so a key store entry on the sm2p256v1 curve was never matched against the "EC/sm2p256v1" key type the sm2sig_sm3 signature scheme asks for, and a server configured for the RFC 8998 cipher suites failed the handshake with "found no selectable cipher suite among the 1 offered: [TLS_SM4_GCM_SM3]" despite the provider supporting both suites and holding a usable credential. The filter is now registered for curveSM2 on both the client and the server side. As before, the RFC 8998 profile requires explicit configuration: the curveSM2 named group and the sm2sig_sm3 signature scheme are not enabled by default (github #2416).
160160
- The BC "PKIX" CertPathValidator in the chain validity model (PKIXExtendedParameters.CHAIN_VALIDITY_MODEL) could not use the ISIS-MTT dateOfCertGen extension. CertPathValidatorUtilities.getValidCertDateFromValidityModel() passed the raw result of X509Certificate.getExtensionValue(), which is the DER encoding of the extnValue OCTET STRING, straight to ASN1GeneralizedTime.getInstance(), so any end-entity certificate carrying the extension failed validation with "Could not validate time of certificate." rather than having its issuer checked at the recorded generation time. The OCTET STRING is now unwrapped first, and the underlying decoding error is chained on the reported exception.
161+
- OpenPGP AEAD decryption (SEIPDv2 and the v5 AEAD packet) still accepted a truncated message without reporting an error, by a route the 1.85 final-tag fix did not cover. Truncation that leaves the enclosing packet length untouched raises an EOFException, which BCPGInputStream.nextPacketTag() reads as a clean end of message; when the literal packet ended on an AEAD chunk boundary and the consumer read in increments smaller than one chunk, the final message tag was never reached, so the caller was given the plaintext of the authenticated chunks with every following packet silently dropped and no error raised - where RFC 9580 sec. 13.7 requires a clear error as soon as the truncation is detected. BcAEADUtil and JceAEADUtil now re-throw that EOF as a plain IOException, which nextPacketTag() does not launder; the legacy jdk1.1 operator overlay, which had neither this fix nor the 1.85 one, received both.
162+
- The high-level OpenPGP API (org.bouncycastle.openpgp.api) performed no integrity check at all on a truncated SEIPDv1 (MDC) message, releasing its plaintext with no error. Same cause as the entry above: IntegrityProtectedInputStream verifies the MDC from close(), and reached close() only by closing itself when a read of it returned -1, which a truncated message never produces - so PGPEncryptedData.verify() never ran and the recipient was handed CFB-decrypted plaintext on which nothing had been checked. OpenPGPMessageInputStream.close() now closes its layer's integrity-protected stream itself, and IntegrityProtectedInputStream.close() was made idempotent as java.io.Closeable requires, which that depends on - the stream is closed twice on the ordinary path and verify() cannot be run twice. The low-level API is unaffected: a caller that invokes PGPEncryptedData.verify() directly gets the check regardless.
161163

162164
### 2.1.3 Additional Features and Functionality
163165

‎pg/src/main/java/org/bouncycastle/openpgp/IntegrityProtectedInputStream.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ public class IntegrityProtectedInputStream
1616

1717
private final PGPEncryptedData esk;
1818

19+
private boolean closed;
20+
1921
public IntegrityProtectedInputStream(InputStream in, PGPEncryptedData dataPacket)
2022
{
2123
super(in);
@@ -62,6 +64,13 @@ public int read(byte[] b, int off, int len)
6264
public void close()
6365
throws IOException
6466
{
67+
// verify once only: this stream is closed twice on the ordinary path and verify() cannot be repeated
68+
if (closed)
69+
{
70+
return;
71+
}
72+
closed = true;
73+
6574
super.close();
6675
if (esk.getEncData() instanceof SymmetricEncIntegrityPacket)
6776
{

‎pg/src/main/java/org/bouncycastle/openpgp/api/OpenPGPMessageInputStream.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@ public class OpenPGPMessageInputStream
4848
private final Layer layer; // the packet layer processed by this input stream
4949

5050
private InputStream in;
51+
// this layer's IntegrityProtectedInputStream, if it has one - closed by close(), see there
52+
private InputStream integrityProtectedIn;
5153
private final List<PacketHandler> packetHandlers = new ArrayList<PacketHandler>()
5254
{{
5355
add(new SignatureListHandler());
@@ -142,6 +144,12 @@ public void close()
142144
}
143145
}
144146

147+
// close it here: a truncated message never returns the -1 that makes it self-close and verify the MDC
148+
if (integrityProtectedIn != null)
149+
{
150+
integrityProtectedIn.close();
151+
}
152+
145153
resultBuilder.verifySignatures(processor);
146154
resultBuilder.closeLayer();
147155
}
@@ -682,6 +690,7 @@ public void handle(Object packet)
682690
OpenPGPMessageProcessor.Decrypted decrypted = processor.decrypt(encryptedDataList);
683691

684692
resultBuilder.encrypted(decrypted);
693+
integrityProtectedIn = decrypted.inputStream;
685694
processNestedStream(decrypted.inputStream);
686695
}
687696

‎pg/src/main/java/org/bouncycastle/openpgp/operator/bc/BcAEADUtil.java‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package org.bouncycastle.openpgp.operator.bc;
22

3+
import java.io.EOFException;
34
import java.io.IOException;
45
import java.io.InputStream;
56
import java.io.OutputStream;
@@ -327,7 +328,7 @@ public PGPAeadInputStream(boolean isV5StyleAEAD, InputStream in,
327328
this.aaData = aaData;
328329

329330
// prime with 2 * tag len bytes.
330-
Streams.readFully(in, buf, 0, tagLen + tagLen);
331+
readAeadFully(buf, 0, tagLen + tagLen);
331332

332333
// load the first block
333334
this.data = readBlock();
@@ -407,7 +408,7 @@ private byte[] readBlock()
407408
{
408409
// we initialise with the first 16 bytes as there is an additional 16 bytes following
409410
// the last chunk (which may not be the exact chunklength).
410-
int dataLen = Streams.readFully(in, buf, tagLen + tagLen, chunkLength);
411+
int dataLen = readAeadFully(buf, tagLen + tagLen, chunkLength);
411412
if (dataLen == 0)
412413
{
413414
if (!aeadComplete)
@@ -454,12 +455,26 @@ private byte[] readBlock()
454455
}
455456
else
456457
{
457-
Streams.readFully(in, buf, tagLen, tagLen); // read the next tag bytes
458+
readAeadFully(buf, tagLen, tagLen); // read the next tag bytes
458459
}
459460

460461
return decData;
461462
}
462463

464+
// truncation must not escape as an EOFException - nextPacketTag() reads one as a clean end of message
465+
private int readAeadFully(byte[] b, int off, int len)
466+
throws IOException
467+
{
468+
try
469+
{
470+
return Streams.readFully(in, b, off, len);
471+
}
472+
catch (EOFException e)
473+
{
474+
throw Exceptions.ioException("truncated AEAD data: " + e.getMessage(), e);
475+
}
476+
}
477+
463478
// Verify the trailing message tag, which is held in buf[0..tagLen). Called both for a short
464479
// final data chunk and for a chunk-aligned message whose final tag would otherwise be skipped.
465480
private void verifyFinalTag()

‎pg/src/main/java/org/bouncycastle/openpgp/operator/jcajce/JceAEADUtil.java‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package org.bouncycastle.openpgp.operator.jcajce;
22

3+
import java.io.EOFException;
34
import java.io.IOException;
45
import java.io.InputStream;
56
import java.io.OutputStream;
@@ -311,7 +312,7 @@ public PGPAeadInputStream(boolean isV5AEAD, InputStream in,
311312
this.aaData = aaData;
312313

313314
// prime with 2 * tag len bytes.
314-
Streams.readFully(in, buf, 0, aeadTagLength + aeadTagLength);
315+
readAeadFully(buf, 0, aeadTagLength + aeadTagLength);
315316

316317
// load the first block
317318
this.data = readBlock();
@@ -391,7 +392,7 @@ private byte[] readBlock()
391392
{
392393
// we initialise with the first 16 bytes as there is an additional 16 bytes following
393394
// the last chunk (which may not be the exact chunklength).
394-
int dataLen = Streams.readFully(in, buf, aeadTagLength + aeadTagLength, chunkLength);
395+
int dataLen = readAeadFully(buf, aeadTagLength + aeadTagLength, chunkLength);
395396
if (dataLen == 0)
396397
{
397398
if (!aeadComplete)
@@ -439,12 +440,26 @@ private byte[] readBlock()
439440
}
440441
else
441442
{
442-
Streams.readFully(in, buf, aeadTagLength, aeadTagLength); // read the next tag bytes
443+
readAeadFully(buf, aeadTagLength, aeadTagLength); // read the next tag bytes
443444
}
444445

445446
return decData;
446447
}
447448

449+
// truncation must not escape as an EOFException - nextPacketTag() reads one as a clean end of message
450+
private int readAeadFully(byte[] b, int off, int len)
451+
throws IOException
452+
{
453+
try
454+
{
455+
return Streams.readFully(in, b, off, len);
456+
}
457+
catch (EOFException e)
458+
{
459+
throw Exceptions.ioException("truncated AEAD data: " + e.getMessage(), e);
460+
}
461+
}
462+
448463
// Verify the trailing message tag, which is held in buf[0..aeadTagLength). Called both for a
449464
// short final data chunk and for a chunk-aligned message whose final tag would otherwise be skipped.
450465
private void verifyFinalTag()

‎pg/src/main/jdk1.1/org/bouncycastle/openpgp/operator/jcajce/JceAEADUtil.java‎

Lines changed: 56 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package org.bouncycastle.openpgp.operator.jcajce;
22

3+
import java.io.EOFException;
34
import java.io.IOException;
45
import java.io.InputStream;
56
import java.io.OutputStream;
@@ -292,6 +293,7 @@ static class PGPAeadInputStream
292293
private int dataOff;
293294
private long chunkIndex = 0;
294295
private long totalBytes = 0;
296+
private boolean aeadComplete = false; // set once the trailing message tag has been verified
295297
private boolean v5StyleAEAD;
296298

297299
/**
@@ -329,7 +331,7 @@ public PGPAeadInputStream(boolean isV5AEAD, InputStream in,
329331
this.aaData = aaData;
330332

331333
// prime with 2 * tag len bytes.
332-
Streams.readFully(in, buf, 0, aeadTagLength + aeadTagLength);
334+
readAeadFully(buf, 0, aeadTagLength + aeadTagLength);
333335

334336
// load the first block
335337
this.data = readBlock();
@@ -409,9 +411,20 @@ private byte[] readBlock()
409411
{
410412
// we initialise with the first 16 bytes as there is an additional 16 bytes following
411413
// the last chunk (which may not be the exact chunklength).
412-
int dataLen = Streams.readFully(in, buf, aeadTagLength + aeadTagLength, chunkLength);
414+
int dataLen = readAeadFully(buf, aeadTagLength + aeadTagLength, chunkLength);
413415
if (dataLen == 0)
414416
{
417+
if (!aeadComplete)
418+
{
419+
// A chunk-aligned message (plaintext an exact multiple of chunkLength) ends after a
420+
// full-size final data chunk, so the trailing message tag was pre-read into the
421+
// look-ahead (buf[0..aeadTagLength)) but the full-size-chunk path below never verified it.
422+
// Verify it now, before signalling EOF, so that truncation at a chunk boundary
423+
// (dropping trailing chunks plus the final tag) is rejected rather than accepted as
424+
// authentic - RFC 9580 5.13.2 relies on the final tag to authenticate the total length.
425+
verifyFinalTag();
426+
aeadComplete = true;
427+
}
415428
return null;
416429
}
417430

@@ -441,31 +454,55 @@ private byte[] readBlock()
441454

442455
if (dataLen != chunkLength) // it's our last block
443456
{
444-
adata = PGPAeadOutputStream.getAdata(v5StyleAEAD, aaData, chunkIndex, totalBytes);
445-
try
446-
{
447-
if (v5StyleAEAD)
448-
{
449-
JceAEADCipherUtil.setUpAeadCipher(c, secretKey, Cipher.DECRYPT_MODE, getNonce(iv, chunkIndex), 128, Arrays.concatenate(adata, Pack.longToBigEndian(totalBytes)));
450-
}
451-
else
452-
{
453-
JceAEADCipherUtil.setUpAeadCipher(c, secretKey, Cipher.DECRYPT_MODE, getNonce(iv, chunkIndex), 128, adata);
454-
}
457+
verifyFinalTag();
458+
aeadComplete = true;
459+
}
460+
else
461+
{
462+
readAeadFully(buf, aeadTagLength, aeadTagLength); // read the next tag bytes
463+
}
464+
465+
return decData;
466+
}
467+
468+
// Verify the trailing message tag, which is held in buf[0..aeadTagLength). Called both for a short
469+
// final data chunk and for a chunk-aligned message whose final tag would otherwise be skipped.
470+
private void verifyFinalTag()
471+
throws IOException
472+
{
473+
byte[] adata = PGPAeadOutputStream.getAdata(v5StyleAEAD, aaData, chunkIndex, totalBytes);
455474

456-
c.doFinal(buf, 0, aeadTagLength); // check final tag
475+
try
476+
{
477+
if (v5StyleAEAD)
478+
{
479+
JceAEADCipherUtil.setUpAeadCipher(c, secretKey, Cipher.DECRYPT_MODE, getNonce(iv, chunkIndex), 128, Arrays.concatenate(adata, Pack.longToBigEndian(totalBytes)));
457480
}
458-
catch (Exception e)
481+
else
459482
{
460-
throw new IOException("exception processing final tag: " + e.getMessage());
483+
JceAEADCipherUtil.setUpAeadCipher(c, secretKey, Cipher.DECRYPT_MODE, getNonce(iv, chunkIndex), 128, adata);
461484
}
485+
486+
c.doFinal(buf, 0, aeadTagLength); // check final tag
462487
}
463-
else
488+
catch (Exception e)
464489
{
465-
Streams.readFully(in, buf, aeadTagLength, aeadTagLength); // read the next tag bytes
490+
throw new IOException("exception processing final tag: " + e.getMessage());
466491
}
492+
}
467493

468-
return decData;
494+
// truncation must not escape as an EOFException - nextPacketTag() reads one as a clean end of message
495+
private int readAeadFully(byte[] b, int off, int len)
496+
throws IOException
497+
{
498+
try
499+
{
500+
return Streams.readFully(in, b, off, len);
501+
}
502+
catch (EOFException e)
503+
{
504+
throw new IOException("truncated AEAD data: " + e.getMessage());
505+
}
469506
}
470507
}
471508

0 commit comments

Comments
 (0)