linux-kernel - Re: [PATCH] KEYS: X.509: Fix Basic Constraints CA flag parsing

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAKtyLkGYfnBnAandEOsTDVHt7oL8LXCWq6+7zEndZ-eQRGAtag@mail.gmail.com>
Date: Fri, 12 Sep 2025 22:37:24 -0700
From: Fan Wu <wufan@...nel.org>
To: Lukas Wunner <lukas@...ner.de>
Cc: Fan Wu <wufan@...nel.org>, dhowells@...hat.com, ignat@...udflare.com, 
	herbert@...dor.apana.org.au, davem@...emloft.net, jarkko@...nel.org, 
	zohar@...ux.ibm.com, eric.snowberg@...cle.com, keyrings@...r.kernel.org, 
	linux-crypto@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] KEYS: X.509: Fix Basic Constraints CA flag parsing

On Fri, Sep 12, 2025 at 9:38 PM Lukas Wunner <lukas@...ner.de> wrote:
>
> On Fri, Sep 12, 2025 at 02:14:49PM -0700, Fan Wu wrote:
> > On Fri, Sep 12, 2025 at 6:14 AM Lukas Wunner <lukas@...ner.de> wrote:
> > > On Thu, Sep 11, 2025 at 10:53:56PM +0000, wufan@...nel.org wrote:
> > > > +++ b/crypto/asymmetric_keys/x509_cert_parser.c
> > > > @@ -623,7 +625,7 @@ int x509_process_extension(void *context, size_t hdrlen,
> > > >               if (v[0] != (ASN1_CONS_BIT | ASN1_SEQ))
> > > >                       return -EBADMSG;
> > > >               if (vlen < 2)
> > > >                       return -EBADMSG;
> > > >               if (v[1] != vlen - 2)
> > > >                       return -EBADMSG;
> > > > -             if (vlen >= 4 && v[1] != 0 && v[2] == ASN1_BOOL && v[3] == 1)
> > > > +             if (vlen >= 5 && v[1] != 0 && v[2] == ASN1_BOOL && v[3] == 1 && v[4] != 0)
> > > >                       ctx->cert->pub->key_eflags |= 1 << KEY_EFLAG_CA;
> > > >               return 0;
> > > >       }
> > >
> > > Your patch is correct, however the conditions ...
> > >
> > >   vlen >= 5 && v[1] != 0 && v[2] == ASN1_BOOL && v[3] == 1
> > >
> > > ... all check well-formedness of the BasicConstraints object,
> > > so it seems if any of those checks fails, -EBADMSG should be returned.
> > >
> > > The check "if (vlen < 2)" could be changed to "if (vlen < 5)" because
> > > 5 bytes seems to be the minimum size of a well-formed BasicConstraints
> > > object.  Then the "vlen >= 5" and "v[1] != 0" checks can be dropped.
> >
> > Actually, we need to be careful here. OpenSSL produces
> > BasicConstraints with CA:FALSE as just an empty SEQUENCE:
> >
> > 06 03 55 1d 13 | 01 01 ff | 04 02 | 30 00
> > [----OID------] [critical] [OCTET] [empty SEQ]
>
> I see, thanks for the explanation.
>
> This behavior of OpenSSL doesn't seem spec-compliant, or is it?
> RFC 5280 sec 4.2.1.9 says the pathLenConstraint is optional,
> but the cA boolean is not optional.  Is there a rule that booleans
> need not be rendered if they are false?
>
> BTW, I note that X.690 sec 11.1 says that for DER encoding,
> all bits of a "true" boolean must be set, hence the 0xff value.
> But I'm fine with your more permissive approach which checks for
> a non-zero value, hence also allows BER encoding per X.690 sec 8.2.2.
>
> Thanks!
>
> Lukas

I double-checked RFC 5280 and X.690 after your comment. I found RFC
5280 section 4.1 explicitly requires X.509 certificates to use DER
encoding, so my original patch allowing any non-zero value was
incorrect. I'll update the patch to check specifically for 0xFF and
return -EBADMSG for any other value.

For OpenSSL's empty SEQUENCE, X.690 section 11.5 states: "The encoding
of a set value or sequence value shall not include an encoding for any
component value which is equal to its default value." Since RFC 5280
defines BasicConstraints with "cA BOOLEAN DEFAULT FALSE", the field
must be omitted when false under DER encoding. So OpenSSL's behavior
is correct.

Thanks for catching the boolean encoding issue.

-Fan