Bouncy Castle 1.80 parsed a GeneralName with an incorrect tag.

[onepeople158]

In this CRL file, the GeneralName in the fullName of the distributionPoint in the IDP extension is tagged as a context-specific tag and simple encoding, with the tag value set to 4. This violates the RFC5280 specification.However, when I use Bouncy Castle 1.80 to parse this CRL file, Bouncy Castle 1.80 successfully extracts the URI value from the GeneralName without any errors. Bouncy Castle 1.80 parses its IDP extension as follows:

The string 68747470733a2f2f7777772e6578616d706c652e636f6d is the hexadecimal representation of https://www.example.com/.

Test Case:

crl_tag_gn_5.zip

Code:

import java.io.InputStream;
import java.io.FileInputStream;
import org.bouncycastle.asn1.x509.CertificateList;
import org.bouncycastle.cert.X509CRLHolder;
import org.bouncycastle.asn1.ASN1ObjectIdentifier;
import org.bouncycastle.asn1.x509.Extension;
import org.bouncycastle.asn1.*;
import org.bouncycastle.util.encoders.Hex;

public class CRLParserExample_CRL_IDP1 {
    public static void main(String[] args) throws Exception {
        if (args.length != 1) {
            System.out.println("Usage: java CRLParserExample3 <path-to-crl-file>");
            return;
        }
        String crlPath = args[0];
        
        InputStream inputStream = new FileInputStream(crlPath);

        X509CRLHolder crlHolder = new X509CRLHolder(inputStream);

        ASN1ObjectIdentifier oid = new ASN1ObjectIdentifier("2.5.29.28"); 

        Extension extension = crlHolder.getExtension(oid);
        
        ASN1Encodable parsedValue =extension.getParsedValue();
        ASN1Sequence sequence = (ASN1Sequence) parsedValue;
        System.out.println(parsedValue);
    } 
}

[winfriedgerlach]

I guess the same what @peterdettman has written in #1986 applies here:

We cannot suddenly start enforcing this restriction when parsing, since we don't know the impact on existing users [...].

If it is a concern for you to enforce this, it would probably have to be added subject to a runtime property (or multiple properties for the different high-level types), preserving current behaviour by default.

To move towards enforcing by default would require collection/analysis of real-world data e.g. in the case of certificates, the Certificate Transparency logs.

You have opened several Jira issues regarding validating various CRL fields. I think there are 2 general aspects:

• Parsing CRLs: Yes, it is annoying if the CRL does not 100% adhere to all RFCs. But should you throw an exception in that case and stop processing it? Probably not! A CRL is a list of certificates you shouldn't trust, no matter whether the list itself has some bugs in some fields or not. So IMHO it makes a lot of sense to have a lenient (permissive) default parsing behavior in BC.
• Creating CRLs: If you want to create CRLs, it may be helpful to validate it strictly, so the list you send out is 100% correct. IIUC this would be a feature request worth its own Jira issue. In that issue you could summarize all fields that you have already identified as interesting.

[onepeople158]

I guess the same what @peterdettman has written in #1986 applies here:

We cannot suddenly start enforcing this restriction when parsing, since we don't know the impact on existing users [...].
If it is a concern for you to enforce this, it would probably have to be added subject to a runtime property (or multiple properties for the different high-level types), preserving current behaviour by default.
To move towards enforcing by default would require collection/analysis of real-world data e.g. in the case of certificates, the Certificate Transparency logs.

You have opened several Jira issues regarding validating various CRL fields. I think there are 2 general aspects:

• Parsing CRLs: Yes, it is annoying if the CRL does not 100% adhere to all RFCs. But should you throw an exception in that case and stop processing it? Probably not! A CRL is a list of certificates you shouldn't trust, no matter whether the list itself has some bugs in some fields or not. So IMHO it makes a lot of sense to have a lenient (permissive) default parsing behavior in BC.
• Creating CRLs: If you want to create CRLs, it may be helpful to validate it strictly, so the list you send out is 100% correct. IIUC this would be a feature request worth its own Jira issue. In that issue you could summarize all fields that you have already identified as interesting.

Thank you for your reply, I understand now.

[peterdettman]

Similarly to #2055, the ASN.1 parser doesn't have the information to find errors like this. The parsed value should be handled by an appropriate type, in this case IssuingDistributionPoint:

IssuingDistributionPoint idp = IssuingDistributionPoint.getInstance(parsedValue);

However in this case still no error is thrown, even though it ends up creating a GeneralName.ediPartyName from an implicit sequence that it doesn't attempt to validate (which would fail because the sequence contains only [CONTEXT 6]-tagged contents - not matching EDIPartyName definition).

It might be worth adding validation for EDIPartyName in particular; similar issues apply for GeneralName.otherName and GeneralName.x400Address, but they are more complicated cases.

[onepeople158]

Similarly to #2055, the ASN.1 parser doesn't have the information to find errors like this. The parsed value should be handled by an appropriate type, in this case :IssuingDistributionPoint

IssuingDistributionPoint idp = IssuingDistributionPoint.getInstance(parsedValue);

However in this case still no error is thrown, even though it ends up creating a from an implicit sequence that it doesn't attempt to validate (which would fail because the sequence contains only [CONTEXT 6]-tagged contents - not matching definition).GeneralName.ediPartyName``EDIPartyName

It might be worth adding validation for in particular; similar issues apply for and , but they are more complicated cases.EDIPartyName``GeneralName.otherName``GeneralName.x400Address

I understand.