On Thu, Sep 11, 2025 at 10:53:56PM +0000, wufan@xxxxxxxxxx wrote: > Fix the X.509 Basic Constraints CA flag parsing to correctly handle > the ASN.1 DER encoded structure. The parser was incorrectly treating > the length field as the boolean value. > > According to ITU-T X.690 section 8.2, a BOOLEAN is encoded as: > > Tag (0x01), Length (0x01), Value (0x00 for FALSE, non-zero for TRUE) > > The basicConstraints extension with CA:TRUE is encoded as: > > SEQUENCE (0x30) | Length | BOOLEAN (0x01) | Length (0x01) | Value (0xFF) > ^-- v[2] ^-- v[3] ^-- v[4] > > The parser was checking v[3] (the length field, always 0x01) instead > of v[4] (the actual boolean value, 0xFF for TRUE). Excellent catch! How did you find it? > +++ 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. Up to you whether to respin this patch or make those changes in a separate patch on top. And up to Herbert whether to take this patch as is or wait for a respin. Reviewed-by: Lukas Wunner <lukas@xxxxxxxxx> I note that parsing the v[] array is quite error-prone and it might have been better to either declare a packed struct for the BasicConstraints object with human-readable member names, or create a separate ASN.1 module for it. Thanks, Lukas
