OpenPGP: KeyExpirationTime/Exportable/PrimaryUserID/Revocable length unchecked, readers throw IllegalStateException

[Arpan0995]

Summary

Four OpenPGP signature-subpacket types carry no length validation at parse time: KeyExpirationTime (tag 9), Exportable (tag 4), PrimaryUserID (tag 25) and Revocable (tag 7). SignatureSubpacketInputStream.readPacket rejects only a body length of zero or an out-of-range length (SignatureSubpacketInputStream.java:93, line numbers as of acd2178); it does not enforce the fixed body length each of these four types requires when the declared length matches the bytes actually present. A certificate whose primary self-signature carries one of these subpackets with a wrong-length body therefore parses cleanly, and the self-signature is cryptographically valid because the signer's own key signs over the malformed body. The bad length surfaces only later, when a reader consults the subpacket: the accessor routes through Utils.timeFromBytes or Utils.booleanFromByteArray, both of which throw an unchecked IllegalStateException (Utils.java:53-58 and Utils.java:33-51).

Because the content is authenticated, this is not "malformed input rejected at the door". It is malformed-but-signed content that passes signature verification and then crashes ordinary reader code with an unchecked exception. The affected accessors are on the common path for reading key expiry and export policy, including the high-level org.bouncycastle.openpgp.api certificate API and the keyserver re-export path PGPPublicKeyRing.encode(out, true), whose encode methods declare IOException rather than a RuntimeException.

The behaviour fails closed (it throws, it does not return a wrong expiry or an incorrect export decision), so this is a robustness and contract issue rather than a signature-forgery issue.

Environment

• bcpg / bcprov / bcutil 1.86.0.20700 (current 1.86 beta), main at commit acd2178
• JDK 27

Steps to reproduce

import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.security.KeyPair;
import java.security.KeyPairGenerator;
import java.util.Date;
import java.util.Iterator;

import org.bouncycastle.bcpg.HashAlgorithmTags;
import org.bouncycastle.bcpg.PublicKeyAlgorithmTags;
import org.bouncycastle.bcpg.sig.Exportable;
import org.bouncycastle.bcpg.sig.KeyExpirationTime;
import org.bouncycastle.openpgp.PGPKeyRingGenerator;
import org.bouncycastle.openpgp.PGPPublicKey;
import org.bouncycastle.openpgp.PGPPublicKeyRing;
import org.bouncycastle.openpgp.PGPSignature;
import org.bouncycastle.openpgp.PGPSignatureSubpacketGenerator;
import org.bouncycastle.openpgp.PGPSignatureSubpacketVector;
import org.bouncycastle.openpgp.api.OpenPGPCertificate;
import org.bouncycastle.openpgp.jcajce.JcaPGPPublicKeyRing;
import org.bouncycastle.openpgp.operator.PGPContentSignerBuilder;
import org.bouncycastle.openpgp.operator.PGPDigestCalculator;
import org.bouncycastle.openpgp.operator.jcajce.JcaPGPContentSignerBuilder;
import org.bouncycastle.openpgp.operator.jcajce.JcaPGPDigestCalculatorProviderBuilder;
import org.bouncycastle.openpgp.operator.jcajce.JcaPGPKeyPair;

public class Repro
{
    // Build a public key ring whose primary self-signature carries the given
    // hashed subpackets. The self-signature is cryptographically valid: the
    // signer's own key signs over the (malformed) subpacket body.
    static PGPPublicKeyRing buildRing(PGPSignatureSubpacketVector hashed) throws Exception
    {
        KeyPairGenerator kpg = KeyPairGenerator.getInstance("RSA");
        kpg.initialize(2048);
        KeyPair kp = kpg.generateKeyPair();

        JcaPGPKeyPair pgpKp = new JcaPGPKeyPair(PublicKeyAlgorithmTags.RSA_GENERAL, kp, new Date());
        PGPDigestCalculator sha1 = new JcaPGPDigestCalculatorProviderBuilder().build()
            .get(HashAlgorithmTags.SHA1);
        PGPContentSignerBuilder csb = new JcaPGPContentSignerBuilder(
            PublicKeyAlgorithmTags.RSA_GENERAL, HashAlgorithmTags.SHA256);

        PGPKeyRingGenerator gen = new PGPKeyRingGenerator(
            PGPSignature.POSITIVE_CERTIFICATION, pgpKp, "Test <test@example.com>",
            sha1, hashed, null, csb, null);

        PGPPublicKeyRing pub = gen.generatePublicKeyRing();

        // Round-trip through the wire so we exercise the parser, not the builder.
        byte[] enc = pub.getEncoded();
        return new JcaPGPPublicKeyRing(new ByteArrayInputStream(enc));
    }

    static PGPSignature primarySelfSig(PGPPublicKeyRing ring)
    {
        PGPPublicKey pk = ring.getPublicKey();
        for (Iterator<PGPSignature> it = pk.getSignatures(); it.hasNext(); )
        {
            PGPSignature s = it.next();
            if (s.getHashedSubPackets() != null)
            {
                return s;
            }
        }
        return null;
    }

    static PGPSignatureSubpacketVector vec(org.bouncycastle.bcpg.SignatureSubpacket sp)
    {
        PGPSignatureSubpacketGenerator g = new PGPSignatureSubpacketGenerator();
        g.setPreferredHashAlgorithms(false, new int[]{ HashAlgorithmTags.SHA256 });
        g.addCustomSubpacket(sp);
        return g.generate();
    }

    public static void main(String[] args) throws Exception
    {
        // ---- Control: a well-formed 4-octet KeyExpirationTime parses and reads. ----
        PGPPublicKeyRing okRing = buildRing(vec(new KeyExpirationTime(false, 3600L)));
        PGPPublicKey okKey = okRing.getPublicKey();
        System.out.println("[control] well-formed KeyExpirationTime, getValidSeconds() = "
            + okKey.getValidSeconds());
        System.out.println("[control] parse of the ring succeeded, signature is present: "
            + (primarySelfSig(okRing) != null));
        System.out.println();

        // ---- Case 1: KeyExpirationTime with a 1-octet body (needs 4). ----
        // new byte[]{1} is a valid subpacket body length (>0) so the parser accepts it.
        PGPPublicKeyRing keRing = buildRing(vec(new KeyExpirationTime(false, false, new byte[]{ 1 })));
        System.out.println("[case1] ring with malformed KeyExpirationTime parsed OK, signature present: "
            + (primarySelfSig(keRing) != null));

        PGPPublicKey keKey = keRing.getPublicKey();
        try
        {
            long s = keKey.getValidSeconds();
            System.out.println("[case1] getValidSeconds() = " + s + " (no throw)");
        }
        catch (RuntimeException e)
        {
            System.out.println("[case1] PGPPublicKey.getValidSeconds() threw "
                + e.getClass().getName() + ": " + e.getMessage());
        }

        try
        {
            long s = primarySelfSig(keRing).getHashedSubPackets().getKeyExpirationTime();
            System.out.println("[case1] vector.getKeyExpirationTime() = " + s + " (no throw)");
        }
        catch (RuntimeException e)
        {
            System.out.println("[case1] PGPSignatureSubpacketVector.getKeyExpirationTime() threw "
                + e.getClass().getName() + ": " + e.getMessage());
        }

        try
        {
            OpenPGPCertificate cert = new OpenPGPCertificate(keRing);
            Date exp = cert.getExpirationTime();
            System.out.println("[case1] OpenPGPCertificate.getExpirationTime() = " + exp + " (no throw)");
        }
        catch (RuntimeException e)
        {
            System.out.println("[case1] OpenPGPCertificate.getExpirationTime() threw "
                + e.getClass().getName() + ": " + e.getMessage());
        }
        System.out.println();

        // ---- Case 2: Exportable with a 2-octet body (needs exactly 1). ----
        PGPPublicKeyRing exRing = buildRing(vec(new Exportable(false, false, new byte[]{ 1, 0 })));
        System.out.println("[case2] ring with malformed Exportable parsed OK, signature present: "
            + (primarySelfSig(exRing) != null));

        try
        {
            boolean b = primarySelfSig(exRing).getHashedSubPackets().isExportable();
            System.out.println("[case2] vector.isExportable() = " + b + " (no throw)");
        }
        catch (RuntimeException e)
        {
            System.out.println("[case2] PGPSignatureSubpacketVector.isExportable() threw "
                + e.getClass().getName() + ": " + e.getMessage());
        }

        try
        {
            // Keyserver re-export path: encode() declares IOException, not RuntimeException.
            ByteArrayOutputStream out = new ByteArrayOutputStream();
            exRing.encode(out, true);
            System.out.println("[case2] PGPPublicKeyRing.encode(out, true) wrote "
                + out.size() + " bytes (no throw)");
        }
        catch (java.io.IOException e)
        {
            System.out.println("[case2] encode(out, true) threw IOException: " + e.getMessage());
        }
        catch (RuntimeException e)
        {
            System.out.println("[case2] PGPPublicKeyRing.encode(out, true) threw "
                + e.getClass().getName() + ": " + e.getMessage());
        }
    }
}

Actual behaviour

[control] well-formed KeyExpirationTime, getValidSeconds() = 3600
[control] parse of the ring succeeded, signature is present: true

[case1] ring with malformed KeyExpirationTime parsed OK, signature present: true
[case1] PGPPublicKey.getValidSeconds() threw java.lang.IllegalStateException: Byte array has unexpected length. Expected length 4, got 1
[case1] PGPSignatureSubpacketVector.getKeyExpirationTime() threw java.lang.IllegalStateException: Byte array has unexpected length. Expected length 4, got 1
[case1] OpenPGPCertificate.getExpirationTime() threw java.lang.IllegalStateException: Byte array has unexpected length. Expected length 4, got 1

[case2] ring with malformed Exportable parsed OK, signature present: true
[case2] PGPSignatureSubpacketVector.isExportable() threw java.lang.IllegalStateException: Byte array has unexpected length. Expected length 1, got 2
[case2] PGPPublicKeyRing.encode(out, true) threw java.lang.IllegalStateException: Byte array has unexpected length. Expected length 1, got 2

Expected behaviour

Either the parser rejects the fixed-length subpacket at parse time (the natural place, since the body length is known there), or the reader accessors surface the malformed length through a checked, documented channel rather than an unchecked IllegalStateException. In particular PGPPublicKeyRing.encode and PGPPublicKey.encode declare IOException, so a caller that catches IOException around a re-export still gets an unchecked IllegalStateException from a certificate it received over the wire.

Root cause

SignatureSubpacketInputStream.readPacket validates the declared body length only for range (bodyLen must be positive and in range, SignatureSubpacketInputStream.java:93) and for an allocation cap (SignatureSubpacketInputStream.java:101-104). There is a per-type guard for the fixed-length time and issuer subpackets (the switch at SignatureSubpacketInputStream.java:118-137, calling checkData at SignatureSubpacketInputStream.java:203-212), but it fires only when the number of bytes actually read differs from the declared body length. A subpacket whose declared length matches its own wrong-length body (for example a KeyExpirationTime declaring a 1-octet body and carrying 1 octet) passes that guard and is handed straight to the constructor.

The four subpacket classes here defer length checking to their getter: KeyExpirationTime.getTime calls Utils.timeFromBytes (KeyExpirationTime.java:47-50), and Exportable.isExportable, PrimaryUserID.isPrimaryUserID and Revocable.isRevocable call Utils.booleanFromByteArray (Exportable.java:33-36, PrimaryUserID.java:32-35, Revocable.java:32-35). Utils.timeFromBytes throws IllegalStateException when the body is not four octets (Utils.java:53-58) and Utils.booleanFromByteArray throws when the body is not a single 0 or 1 octet (Utils.java:33-51). SignatureCreationTime and SignatureExpirationTime read their time value through the same Utils.timeFromBytes path, so the same unchecked-length getter applies to them.

Because the malformed subpacket lives inside a self-signature the signer generated, signature verification passes and the bad length is only discovered when the value is read. The reader-facing accessors that hit it include PGPSignatureSubpacketVector.getKeyExpirationTime (PGPSignatureSubpacketVector.java:242-252) and isExportable (PGPSignatureSubpacketVector.java:476-479), PGPPublicKey.getValidSeconds via getExpirationTimeFromSig (PGPPublicKey.java:341, 381, 403), the high-level OpenPGPCertificate.getExpirationTime via getKeyExpirationDateAt (OpenPGPCertificate.java:1161, 1172, 1739), and the forTransfer re-encode path PGPPublicKeyRing.encode(out, true).

Impact

A certificate received from a sender (imported from a keyserver, carried with a signed message, or handed to a verifier for a third-party certification check) can carry a valid self-signature whose body is malformed in one of these four fields. Any consumer that later reads key expiry or export policy, or re-encodes the certificate for transfer, gets an unchecked IllegalStateException from code that either declares only IOException or is expected to return a value. The effect is a robustness surface in certificate-handling code, not a signature-verification bypass: the operation fails closed rather than returning a wrong expiry or export decision. Severity is low to moderate, weighted by how central expiry and forTransfer re-export are on the certificate-handling path.

Suggested direction

Validate the fixed body length for KeyExpirationTime, Exportable, PrimaryUserID, Revocable, SignatureCreationTime and SignatureExpirationTime at parse time in SignatureSubpacketInputStream (rejecting a wrong-length fixed subpacket the same way an out-of-range length is already rejected), so that a length problem is caught before the signature is treated as valid content, and downstream accessors and encode paths no longer throw an unchecked exception on an already-parsed certificate.

The program above is complete and self-contained. It compiles and runs against the bcpg jar together with its bcprov and bcutil runtime dependencies on the classpath.

[dghgit]

Thanks for the report, now up on https://downloads.bouncycastle.org/betas