Many thanks for the additional detailas, that was very helpful. > openssl(9323) in free(): chunk is already free 0x3cb3a6a9780 > Abort trap
That's a nasty double free originally introduced in -r1.3 of x509/x509_alt.c - a2i_GENERAL_NAME(out, ...) does not necessarily return something new: it will modify and return out if out != NULL. Accordingly, so we must free the return value if and only if out == NULL. > The issue seems to be actually with the line > > permitted;IP.0=10.0.0.0/255.0.0.0 > > Not sure if that's illegal, too, but at least according to > > https://www.feistyduck.com/library/openssl-cookbook/online/ch-openssl.html > > it should work. > > I changed it to > > permitted;IP.0=10.0.0.0/8 > > just to see what would happen. That gives me a Segfault ... And that's a missing error check introduced in the same commit... > > Well, the name constraints with .personal.lan aren't legal, that's why > > they are rejected. OpenSSL will let you write things in there that will > > then fail to interoperate. > > Rejecting illegal constraints is wonderful, but they shouldn't cause > crashes, no? Leaving aside the question whether we should accept some of this, here's a patch that avoids both bugs you ran into. Can you get openssl req to crash after rebuilding libcrypto with the below? Index: x509/x509_alt.c =================================================================== RCS file: /cvs/src/lib/libcrypto/x509/x509_alt.c,v retrieving revision 1.5 diff -u -p -r1.5 x509_alt.c --- x509/x509_alt.c 28 Oct 2021 10:58:23 -0000 1.5 +++ x509/x509_alt.c 9 Feb 2022 15:30:03 -0000 @@ -649,6 +649,8 @@ v2i_GENERAL_NAME_ex(GENERAL_NAME *out, c } ret = a2i_GENERAL_NAME(out, method, ctx, type, value, is_nc); + if (ret == NULL) + return NULL; /* Validate what we have for sanity */ type = x509_constraints_general_to_bytes(ret, &bytes, &len); @@ -686,7 +688,8 @@ v2i_GENERAL_NAME_ex(GENERAL_NAME *out, c } return ret; err: - GENERAL_NAME_free(ret); + if (out == NULL) + GENERAL_NAME_free(ret); return NULL; }
