OpenSSL bug exposed up to 255 bytes of server heap and existed since 2011

An interesting aspect of this is that it's a bug in NPN, which is more or less a historic artifact.

When SPDY, the precursor of HTTP/2, got introduced, it needed a mechanism to signal that within TLS, a different protocol (SPDY instead of HTTP/1.1) was spoken. That mechanism was originally NPN. I don't know the exact details and motivation, but eventually people seem to have decided that NPN wasn't exactly what they wanted, and they invented a new mechanism called ALPN.

Now, that was a decade ago (the ALPN RFC is from 2014), so the question is: why do we still have NPN code in OpenSSL? I don't think anyone uses it any more. Shouldn't it have been removed long ago?

To put this in a larger context: it appears to me that OpenSSL still has a strong tendency to bloat. Heartbleed was essentially a "we added this feature, although noone knows why we need it" kind of bug, but it doesn't look to me they've changed. I still get the feeling that OpenSSL adds many features that they probably just should ignore (e.g. obscure "not invented here"-type algorithms), and they don't remove features that are obsolete.

The title: "bug exposed up to 255 bytes of server heap and existed since 2011"

The post: "Silently sends up to 255 bytes of the client’s heap to the server."

I started writing s2n the day after Heartbleed and the first lines of code were for the stuffer interface. A stuffer is a buffer for stuff, and it's like Java buffered I/O for C. You can get a flavor from reading the header: https://github.com/aws/s2n-tls/blob/main/stuffer/s2n_stuffer...

The implementation is incredibly simple. Treat all blocks of memory as blob with a known size and then read/write into those blobs with a cursor to track progress and bounds checks on every access. Fence all serialization/deserialization through a safe low level interface. Not only do you get memory safety (which we later proved using formal reasoning) ... but when you're parsing message formats it lends itself to a declarative coding style that makes it very clear what the structure is. You can also do lifecycle things, like erasing sensitive memory with zeroes when you're done with it, making sure things don't show up in core dumps, etc. BoringSSL introduced a Crypto_bytes API that also did some of this plus bounds checking, and retrofit it into OpenSSL.

OpenSSL on the other hand is a horrific mash up of raw pointer arithmetic, ad-hoc parsers interleaved with business logic and control flow. I could never keep it straight, and it always scared me to review.

A few excerpts from the blog post (CVE-2024-5535 was assigned):

> Meeting those constraints is quite unlikely nowadays:

> NPN is a precursor to ALPN and was abandoned in 2012. It is very uncommon on internet servers now.

> Node.js 10 and later removed NPN support, and is well past end-of-life.

> Python 3.10 and later removed NPN support.

OpenSSL advisory: https://openssl-library.org/news/secadv/20240627.txt

> Severity: Low

> Issue summary: Calling the OpenSSL API function SSL_select_next_proto with an empty supported client protocols buffer may cause a crash or memory contents to be sent to the peer.

BoringSSL fix: https://boringssl.googlesource.com/boringssl/+/c1d9ac02514a1...

The heap leak was independently observed in 2014 in the Android okhttp library: https://github.com/square/okhttp/issues/437#issuecomment-358...

Funny that this bug was discovered by rewriting it in Rust!

> (though likely with a huge false-positive rate in other code)

That's what I doubt at all. I rather trust the compilers than the openssl devs.