PR#224: Don't encode critical=FALSE in X509v3 extensions -

certmonger

#224 Don't encode critical=FALSE in X509v3 extensions

Merged Oct 5, 2021 by rcritten. Opened Oct 4, 2021 by cheimes.

cheimes/certmonger ext_critical into master

Use implicit, empty FALSE for extensions

Christian Heimes • Oct 5, 2021

e3e4679693efc60bc7a25983909ddfa6883ab2ec

Use extensions template from NSS

Christian Heimes • Oct 4, 2021

9312d1892c611d9f0e814cb915488182da2b76cc

Download 224.patch

cheimes commented Oct 4, 2021

The X509v3 extension is defined as

   Extension  ::=  SEQUENCE  {
        extnID      OBJECT IDENTIFIER,
        critical    BOOLEAN DEFAULT FALSE,
        extnValue   OCTET STRING
                    -- contains the DER encoding of an ASN.1 value
                    -- corresponding to the extension type identified
                    -- by extnID
        }

Since the critical element has a default value of FALSE, a DER
encoder must not encode the value. Only TRUE value must be encoded.

Fixes: https://pagure.io/certmonger/issue/223
Signed-off-by: Christian Heimes cheimes@redhat.com

1 new commit added

Use extensions template from NSS

Oct 4, 2021

rcritten commented Oct 4, 2021

Change looks ok. Can the commits be squashed? The first one updates the existing template and the second eliminates it.

rebased onto 9312d1892c611d9f0e814cb915488182da2b76cc

Oct 4, 2021

rcritten commented Oct 4, 2021

I'm still seeing BOOLEAN: 0 in the generated CSR:

402:d=7 hl=2 l= 3 prim: OBJECT :X509v3 Subject Alternative Name
407:d=7 hl=2 l= 1 prim: BOOLEAN :0
410:d=7 hl=3 l= 161 prim: OCTET STRING [HEX DUMP]:....

1 new commit added

Use implicit, empty FALSE for extensions

Oct 4, 2021

rcritten commented Oct 4, 2021

OK, this drops the critical extension altogether as desired.

asn1crypto.csr still fails to parse the CSR with a very similar failure as reported in the ticket.

Can you update the commit message on the new commit to include the ticket # and I think the 1st comment from this PR would be good to context to include as well.

rcritten commented Oct 4, 2021

I tested this on F35 with and without this patch applied. An IPA replica is installable with the patch.

2 new commits added