OCAP audit of OpenSSL

The audit took place during 2015 in two phases while the OpenSSL project was working on the development of the (now released) 1.1.0 version. We chose to audit the “master” branch of the code as it stood at the time. OpenSSL 1.1.0 has made some extensive internal changes, most notably in libssl with the new state machine code, as well as the new packet parsing code. We wanted the audit team to review that code to give us confidence that what we were delivering was sound.

The first phase of the audit concentrated primarily on areas of libcrypto including (for example) the ASN.1, x509, BIOs, BIGNUM, RSA and elliptic curve components. Some areas of libssl were also looked at. Code was manually reviewed, as well as fuzzed using AFL. The full report from phase 1 can be found here.

The second phase of the audit occurred later in the year after much of the refactoring work in libssl had been completed (including for example the state machine refactor and the packet parsing refactor). The focus for this phase was on the TLS stack. The full report from phase 2 can be found here.

The audit identified a number of issues, including: buffer overreads and memory leaks. Any significant security issues, such as CVE-2016-2181 (DTLS replay protection DoS), have already been addressed.

It also outlined a number of recommendations designed to further security harden OpenSSL.

There were no MODERATE, HIGH or CRITICAL severity security defects identified in any released version of OpenSSL as a result of this audit. The recommendations ranged from specific short-term issues such as memory leaks and crashes to longer-term goals and objectives, including: refactoring of message construction, fuzz testing and general code quality issues.

Most of these have already been addressed in the current 1.1.0 version. Others are addressed in the forthcoming 1.1.1 version. Furthermore, we are continuing to work toward other longer-term recommendations, including code quality improvements and better fuzz testing.

The reports details a number of “vulnerabilities”. The term here is used in a looser sense than typically used by the OpenSSL project. Since this audit was taking place on a development (i.e. not released at the time) version of the code these issues would not qualify for a CVE assignment unless they were also present in older (released) versions of the code. Even where they are present in older versions of the code, many would still not qualify for a CVE assignment. For example some are not practically exploitable; some can only occur as a result of a programmer error in the application using OpenSSL; some are really suggestions for “hardening” and are not directly security issues in themselves; etc. Each of the vulnerabilities listed in the reports has been assessed and, where a change was deemed necessary, that has been done.

I’d like to finish off by looking at the strategic recommendations listed in the second phase report and give some updates on our progress towards achieving these.

“Recommendation 1 - Message Construction: Message construction is still being performed by manual pointer and buffer manipulation. This has lead to one identified vulnerability (finding NCC-OSSL2-006 on page 8). Consider rewriting message construction using safe primitives similar to those recently introduced for message parsing.”

This has been a major focus for the forthcoming 1.1.1 development release with the introduction of the new “WPACKET” message construction primitive. WPACKET provides safe functions for constructing packets and ensuring that buffer over (and under) flows do not occur. The whole of the libssl packet construction code has now been converted to use the new primitives.

“Recommendation 2 - Code Quality: Address the code quality issues enumerated in Appendix B on page 17. Two of the more important strategic changes would be to break complex functions into smaller units and to further separate out message parsing and construction from business logic. These changes should increase the long term readability, maintainability, and security of OpenSSL, but risk introducing new bugs in the short term.”

Since the report was produced, we’ve focused extensively on code quality. For example a major effort has been undertaken to “size_t-ify” libssl and harmonise the usage of types throughout. Previously there were many instances of multiple casts of types as calls progress through the stack (e.g. a variable held as an int in one function, is later cast to a size_t in another function, and cast to a long in yet another, etc). A large proportion of these issues were in the handling of sizes. The size_t work has sought to standardise the types used and greatly reduce the number of casts. This type harmonisation has also reduced instances of signed/unsigned mismatches.

Most of the functions identified as being overly long have now been broken into smaller units. This is an ongoing effort. For example the recent extensions refactor has addressed some other areas of very long functions.

“Recommendation 3 - Fuzz Testing: Consider incorporating fuzz testing into the OpenSSL development process. Develop a fuzzer that covers more code cases and invest in OpenSSL code changes that make fuzzing and testing more productive, such as using assertions to detect violations of implementation assumptions.”

We have made significant progress in the fuzz testing of OpenSSL. Fuzz testing code is now an integral part of the project (e.g. see the “fuzz” sub-directory in the distribution). We now provide built-in support for fuzz testing using both libfuzzer and AFL. We are really excited by these developments and are keen to extend this out further in the future. We are exploring the possibilities of performing “continuous fuzzing” - and some of this is already taking place through the OSS-Fuzz project.

“Recommendation 4 - Compiler Warnings: Visual Studio emits a number of warnings when OpenSSL is built in it’s standard configuration, and more when the warning level is increased. Review and address compiler warnings in Visual Studio. Consider further increasing the warning level when compiling with GCC and Clang and addressing any new warnings that are generated.”

All of the OpenSSL development team currently build with our “–strict-warnings” flag to “Configure” which automatically treats all warnings as errors when building with gcc or clang. Our Continuous Integration environments also build using this flag on numerous different platforms and with various configuration options. This enables us to produce warning free builds on many of our primary platforms. Currently this does not yet extend to Visual Studio builds although there has been significant progress towards this with our recent size_t work. Currently all of the libssl and engines code is compiling without warnings on a default VC-WIN32 build.

“Recommendation 5 - Zero buffer allocations: As documented in Appendix D on page 35, there are a number of buffer overreads in buffers during protocol parsing in the 1.0.1 and earlier branches. These overreads are not believed to be exploitable, even by oracle attacks - but zeroing the buffer allocations would provide defense in depth against possible attacks.”

There has been a lot of work to reduce buffer over and under reads and writes with the introduction of our PACKET and WPACKET primitives. These are safe alternatives to the old “open coded” approach for reading from and writing to buffers. They have been developed to prevent errors occurring through buffer over/under reads and writes.

Finally I would like to thank OCAP and the CII for their support in performing this audit.