Fix a few bugs in X509v3_asid_add*()
authortb <tb@openbsd.org>
Sat, 11 Nov 2023 09:35:21 +0000 (09:35 +0000)
committertb <tb@openbsd.org>
Sat, 11 Nov 2023 09:35:21 +0000 (09:35 +0000)
commit9b790dc7bd2fca0d59a0ba1870e0a50f867b36fb
tree8fca1d02a1ac664ce8fc64e006cd27622b4b33e5
parent6e05f03d342a5059d2c326eb1de71d519fbc6808
Fix a few bugs in X509v3_asid_add*()

These 'builder' functions, usually used together, can result in corrupt
ASIdentifiers on failure. In general, no caller should ever try to recover
from OpenSSL API failure. There are simply too many traps. We can still
make an effort to leave the objects in unmodified state on failure. This
is tricky because ownership transfer happens. Unfortunately a really
clean version of this seems impossible, maybe a future iteration will
bring improvements...

The nasty bit here is that the caller of X509v3_asid_add_id_or_range()
can't know from the return value whether ownership of min and max was
transferred or not. An inspection of (*choice)->u.range is required.
If a caller frees min and max after sk_ASIdOrRange_push() failed, there
is a double free.

All these complications could have been avoided if the API interface
had simply used uint32_t instead of ASN1_INTEGERs. The entire RFC 3779
API was clearly written without proper review. I don't know if there
ever was an actual consumer before rpki-client. If it existed, nobody
with the requisite skill set looked at it in depth.

ok beck for the general direction
with a lot of input and ok jsing
lib/libcrypto/x509/x509_asid.c