Empty Extensions Sequence

[mtgag]

It is possible to create an empty sequence of extensions in certificates, CRLs and probably other PKI structures that use extensions.
The ASN.1 specification requires that the sequence is not empty:

Extensions ::= SEQUENCE SIZE (1..MAX) OF Extension

A small programm that creates an empty sequence of extensions is the following:

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Paths;

import org.bouncycastle.asn1.ASN1Encoding;
import org.bouncycastle.asn1.x509.Extension;
import org.bouncycastle.asn1.x509.Extensions;

public class CreateEmptyExtenionsSequence {

    public static void main(String[] args) throws IOException {

        Extension[] emptyExtensions = new Extension[0];
        Extensions extensions = new Extensions(emptyExtensions);
        Files.write(Paths.get("emptySeq.der"), extensions.getEncoded(ASN1Encoding.DER));
    }

}

The caller of the constructor should not place empty extensions at all on the first place. However, if this done then the extenions are created as an empty sequence. We propose a more strict cheking in BC to disallow this behaviour, for example by checking if the length of the array is 0 and throwing an appropriate exception.
For example

bc-java/core/src/main/java/org/bouncycastle/asn1/x509/Extensions.java

Line 86 in e04ee7f

public Extensions(

could be changed to:

public Extensions(Extension[] extensions)
{

    if (extensions == null || extensions.length == 0) {
        throw new IllegalArgumentException("extension array cannot be null or empty");
    }

    for (int i = 0; i != extensions.length; i++)
    {
        Extension ext = extensions[i];

        this.ordering.addElement(ext.getExtnId());
        this.extensions.put(ext.getExtnId(), ext);
    }
}

Thic could also be addittionally captured here

bc-java/core/src/main/java/org/bouncycastle/asn1/x509/Extensions.java

Line 50 in e04ee7f

private Extensions(

private Extensions(
        ASN1Sequence seq)
{

    if (seq == null || seq.size() == 0) {
        throw new IllegalArgumentException("extension sequence cannot be null or empty");
    }

    Enumeration e = seq.getObjects();

    while (e.hasMoreElements())
    {
        Extension ext = Extension.getInstance(e.nextElement());

        if (extensions.containsKey(ext.getExtnId()))
        {
            throw new IllegalArgumentException("repeated extension found: " + ext.getExtnId());
        }

        extensions.put(ext.getExtnId(), ext);
        ordering.addElement(ext.getExtnId());
    }
}

Also a minor change could be done here:

bc-java/core/src/main/java/org/bouncycastle/asn1/x509/Extension.java

Line 17 in e04ee7f

* an object for the elements in the X.509 V3 extension block.

to describe that the extension object can be used in CRLs and CRL entries, OCSP etc., e.g.

/**
 * an object for the elements in the X.509 extension block of V3 certificates, the crlExtensions and crlEntryExtensions blocks of V2 CRLs and extension blocks in OCSP requests and responses.
 */

or simply:

/**
 * an object for the elements in the X.509 extensions.
 * Extensions  ::=  SEQUENCE SIZE (1..MAX) OF Extension
 */

[mtgag]

We could also consider such positions for more strict checks:

bc-java/prov/src/main/java/org/bouncycastle/jcajce/provider/asymmetric/x509/X509CRLEntryObject.java

Line 265 in 91c3c60

return c.getExtensions() != null;

to change to:

 return c.getExtensions() != null && c.getExtensions().getExtensionOIDs() != null && c.getExtensions().getExtensionOIDs().length > 0

And similar positions like

bc-java/prov/src/main/java/org/bouncycastle/jce/provider/X509CRLEntryObject.java

Line 256 in 91c3c60

return c.getExtensions() != null;

[dghgit]

I think this is now covered. Thanks for the report.