I reviewed several SSL implementations for coding style: OpenSSL, NSS, GnuTLS, JSSE, Botan, MatrixSSL and PolarSSL. I looked at how buffers are handled in parsers and writers. Of all of them, I think only JSSE, i.e. pure Java, can be trusted to be free of buffer overflows. It suggests that a good webserver for security-critical applications would be Tomcat, without native extensions.
In OpenSSL, the Heartbleed patch itself is a good example of what not to do:
/* Read type and payload length first */ if (1 + 2 + 16 > s->s3->rrec.length) return 0; /* silently discard */ hbtype = *p++; n2s(p, payload); if (1 + 2 + payload + 16 > s->s3->rrec.length) return 0; /* silently discard per RFC 6520 sec. 4 */ pl = p;
Bounds checks are rolled up into an obscure calculation, then the code proceeds to memcpy() straight out of the buffer.
NSS has a similar style. For example, ssl3_ComputeDHKeyHash() has:
bufLen = 2*SSL3_RANDOM_LENGTH + 2 + dh_p.len + 2 + dh_g.len + 2 + dh_Ys.len;
In GnuTLS, a similar example of precalculation can be found in this Heartbleed-inspired patch:
response = gnutls_malloc(1 + 2 + data_size + DEFAULT_PADDING_SIZE);PolarSSL was also similar in style.
An embedded, open-core SSL library called MatrixSSL shows an alternative style, which proves that there are alternatives to precalculation. It has bounds checks distributed throughout the parser. Before each read, the remaining length is calculated from the cursor position, and compared with the read length:
if (end - c < SSL2_HEADER_LEN) {
return SSL_PARTIAL;
}
if (ssl->majVer != 0 || (*c & 0x80) == 0) {
if (end - c < ssl->recordHeadLen) {
return SSL_PARTIAL;
}
ssl->rec.type = *c; c++;
ssl->rec.majVer = *c; c++;
ssl->rec.minVer = *c; c++;
ssl->rec.len = *c << 8; c++;
ssl->rec.len += *c; c++;
} else {
ssl->rec.type = SSL_RECORD_TYPE_HANDSHAKE;
ssl->rec.majVer = 2;
ssl->rec.minVer = 0;
ssl->rec.len = (*c & 0x7f) << 8; c++;
ssl->rec.len += *c; c++;
}
This makes auditing easier, and should be commended. However, it’s still a long way short of actually following CERT security guidelines by using a safe string library. And since this is an embedded library, it would have been nice to see a backend free of dynamic allocation, to allow verification tools like Polyspace Code Prover to provide strong guarantees.
Botan is written in C++ by a single author who claims to be aware of security issues, which sounded very promising. In C++, implementing bounds checks should be trivial. However, the code didn’t live up to my expectations. It does pass around std::vector<byte> instead of char* (often by value!), but the author makes extensive use of a memcpy() wrapper to actually read and write those vectors. For example:
std::vector<byte> Heartbeat_Message::contents() const
{
std::vector<byte> send_buf(3 + m_payload.size() + 16);
send_buf[0] = m_type;
send_buf[1] = get_byte<u16bit>(0, m_payload.size());
send_buf[2] = get_byte<u16bit>(1, m_payload.size());
copy_mem(&send_buf[3], &m_payload[0], m_payload.size());
// leave padding as all zeros
return send_buf;
}
The functions are short, with short code paths between length determination and buffer use, which gives it similar auditability to MatrixSSL. But the potential security advantages of using C++ over C were squandered, and frequent copying and small dynamic allocations means that performance will not be comparable to the C libraries.
So it seems that in every case, authors of C and C++ SSL libraries have found unbounded memory access primitives like memcpy() to be too tempting to pass up. Thus, if we want to have an SSL library with implicit, pervasive bounds checking, apparently the only option is to use a library written in a language which forces bounds checking. The best example of this is surely JSSE, also known as javax.net.ssl. This SSL library is written in pure Java code — the implementation can be found here. As I noted in my introduction, it is used by Tomcat as long as you don’t use the native library. The native library gives you “FIPS 140-2 support for TLS/SSL”; that is to say, it links to a library that probably has undiscovered buffer overflow vulnerabilities.
