Hey look ma, I'm doing crypto!

In the same way infoleaks usually aren’t “found” but created, Adi Shamir once said,

Cryptography is typically bypassed, not penetrated.

In this post, we’ll talk about encryption from that exact perspective. In the first part of this series, we’re focusing specifically on identity validation issues in the context of Public Key Infrastructure (PKI).

Exploring encryption

Vulnerability research is a game of specificity, the deeper you go down the rabbit hole, the more impact in terms of bug quantity and severity. This is especially true if your research focuses on the areas of the code base that are traditionally hard to reach by fuzzers and static analysis. This often leads researchers to become subject matter experts in systems that are uncommon and may also be complex in nature.

This concept has me pondering whether I’m developing tunnel vision regarding my approach to vulnerability research: Am I too specific and neglecting other areas that could be interesting?

It’s for this reason that I decided to broaden my horizon these past couple of months and started exploring areas of software security that I never explored before. One area I’ve developed a deeper understanding for is encryption, more specifically anything related to TLS.

Motivation

Something that’s always amazed me about cryptography flaws is how binary things are. Any issue, no matter how small, can ultimately lead to the complete collapse of a system. You rarely find shades of grey in crypto—the system is either safe or it isn’t.

Examples of these situations are timing attacks and padding oracles. An attacker’s ability to observe and measure timing differences in the way certain data is processed allows them to extract secrets from the system. In the case of padding oracles, if an attacker can find a way to make the system respond to the question “is the message correctly padded?” they can also decrypt messages. It almost feels like magic.

Small unforeseen issues resulting in a large impact implies that cryptographic software must be as correct as possible in order to be secure. And its implementation is where we usually find most of the practical flaws with cryptography. Even when the math is sound, most programmers are not seasoned cryptographers and many issues can appear in the translation from theory to code.

Considering all of these points, cryptographic software seems like a fertile field for bug hunting! Let’s give it a try.

Security at scale

Security is a problem that needs to be addressed at scale. Fixing issues one-by-one is a task that never ends. For this reason, we need to think about security from a comprehensive perspective, which is coincidentally our mission statement for the GitHub Security Lab (GHSL).

GitHub is the home for millions of open source projects, which puts the GHSL in a privileged position. In this post, I’ll explain one of the ways we’re approaching software security in a broader way.

CodeQL

Let’s query the code of some C/C++ projects using CodeQL to find out how many of those are using some form of TLS. To answer this question, first we need to define what we consider “using TLS” to be. In our case, we’ll define this as “a project that creates an OpenSSL context”. There are several other popular libraries that implement TLS but, to date, OpenSSL is still the most widely deployed.

A simple CodeQL query can answer this question for us:

import cpp

from FunctionCall call
where call.getTarget().getName() = "SSL_CTX_new"
select call

A first dive

Cryptography is a vast field, so we need to decide where to start. In this case, after reading a number of advisories related to using OpenSSL, I decided to investigate hostname validation since it’s a very important part of TLS. To my surprise, the API to perform such an important function appeared to be a bit convoluted and very OpenSSL version specific.

If developers want to correctly perform hostname verification they need to read the hostname validation section of the OpenSSL wiki and then correctly implement OpenSSL version granular code, since their software could be linked against an old version. Confusing, complex, and version dependent…it ticks all the marks. There’s bound to be bugs in there!

Hostname validation

Hostname validation is kind of a misnomer since a hostname is only one of the ways in which the identity of a certificate can be validated. Alternative identifiers include e-mail, IP addresses, and URIs.

This validation process is one of the fundamental pillars of PKI since failure to do so can lead to complete compromise of the encrypted communications.

In order to verify the identity of a certificate, a client must get all the available identities and match the connecting address to one of the fields inside the certificate. Here is where things get even more confusing: the way in which a certificate gets an identifier has evolved over time and has gone from a mere unique Common Name (CN) to containing multiple Subject Alternative Name (SAN) sections. Moreover, per RFC6125, TLS implementations are encouraged to favor checking for a SAN instead of the CN, so there’s potential for non-conforming implementations.

In older versions of OpenSSL, mechanisms were provided to manually verify the identity of certificates. In newer versions of the library, built-in functionality is provided that’s intended to replace manual checking.

The X509_check_host function is part of a family of functions called certificate matching functions. They’re used to check whether a certificate matches a given hostname, email, or IP address. This is fundamental to the identity verification of a TLS endpoint. Failure to do so generally means that the software is vulnerable to a person-in-the-middle attack.

There are several ways that this function can be used incorrectly, but we’ll focus on misinterpreting its return value, which is the main mechanism in how these functions relay whether or not a given hostname is valid.

If we visit the man page of X509_check_host we can see that the function can return different values. The function returns any of the following:

• 1 for a successful match
• 0 for a failed match
• -1 for an internal error: typically a memory allocation failure or an ASN.1 decoding error

All functions can also return -2 if the input is malformed. For example, X509_check_host() returns -2 if the provided name contains embedded NULs.

Usually, when I identify a function with such return value complexity and especially when that return value is not properly typed, it always raises my interest. I start to think in ways a developer can use this function incorrectly and what the impact could be. I form hypotheses about every way a developer may introduce security issues when misusing the function, and then I attempt to prove or disprove the hypothesis in the code.

This process, when performed manually, can take a lot of time. And while grepping (or in my case, ripgrepping) for flawed code constructs is certainly useful, it’s slow and sometimes incomplete.

Modeling behavior

We usually have two options when modeling code behavior: model the correct cases or the flawed cases. Deciding which to do depends on the type of pattern you’re looking at. A good way to make this decision is to choose the path with the least amount of options. In our case, it’s easier to detect the absence of a “good” return value check.

So, what’s a “good” return value check? Let’s think about this for a second. What are the ways a software developer can express the desired behavior in a correct way? In my case, I like to think about things visually so I decided to build a table with all the options:

Since the return value type of the function is an integer, the operators that are likely to be used to check the return value are !, ==, !=, <, >, <=, and >=. A quick calculation tells us that we have 4 * 6 ways in which a return value check can be expressed. Let’s enumerate the cases that are important for us—match and no match:

From the previous table, we can identify six cases in which developers can correctly express their intention. Any deviation from this is likely an error.

Let’s model one of the correct expressions in CodeQL, for instance ret != 1:

predicate isPhrasedAsNEOne(Expr expression) {
  expression instanceof NEExpr and
  expression.getAChild().getValue() = "1"
}

This predicate will be satisfiable by any NEExpr (not equal expression) that has a value of 1 in any of its operands (children). The same kind of modeling can be applied to the rest of the expressions we’re interested in.

Now that we’re informed with a modeled set of predicates that express the things we want to find, we can build a query to get some real results. In order to do this, we’ll use two CodeQL utilities named DataFlow and GuardCondition. The DataFlow module was featured before so I’m going to avoid explaining it poorly and direct you to Analyzing data flow in C/C++, instead. The second utility called GuardCondition is also detailed in Using the guards library in C and C++, but we can briefly summarize this and say that a GuardCondition is an expression that’s used in some kind of control flow condition, like an if statement.

Now we need to get all the GuardConditions that involve the return value of any of the certificate matching functions. Simply put, we need to trace the data flow of the return of a function into an if expression.

Since this pattern (an Expr flowing into a GuardCondition) is common enough for me, I decided to put it into a Utilities.qll library:

Utilities.qll

import cpp

import semmle.code.cpp.controlflow.Guards
import semmle.code.cpp.dataflow.DataFlow

module Utilities {
  predicate flowsToGuard(GuardCondition guard, Expr expression) {
    exists(DataFlow::Node source, DataFlow::Node sink |
      DataFlow::localFlow(source, sink) and
      source.asExpr() = expression and
      sink.asExpr() = guard.getAChild*()
    )
  }
}

In order to make our analysis more complete, I modeled the certificate matching functions as a custom CodeQL class. This way I won’t need to repeat myself, and the query will give me more results:

OpenSSL.qll

import cpp

class X509_check_host extends FunctionCall {
  X509_check_host() { this.getTarget().getName() = "X509_check_host" }
}

class X509_check_email extends FunctionCall {
  X509_check_email() { this.getTarget().getName() = "X509_check_email" }
}

class X509_check_ip extends FunctionCall {
  X509_check_ip() { this.getTarget().getName() = "X509_check_ip" }
}

class X509_check_ip_asc extends FunctionCall {
  X509_check_ip_asc() { this.getTarget().getName() = "X509_check_ip_asc" }
}

class X509_matching_function extends FunctionCall {
  X509_matching_function() {
    this instanceof X509_check_host or
    this instanceof X509_check_email or
    this instanceof X509_check_ip or
    this instanceof X509_check_ip_asc
  }
}

Now we have all the pieces that we need to express our final query:

openssl-certificate-matching.ql

import cpp
import OpenSSL
import Utilities

predicate isPhrasedAsNEOne(Expr expression) {
  expression instanceof NEExpr and
  expression.getAChild().getValue() = "1"
}

predicate isPhrasedAsEQOne(Expr expression) {
  expression instanceof EQExpr and
  expression.getAChild().getValue() = "1"
}

predicate isPhrasedAsLEZero(Expr expression) {
  expression instanceof LEExpr and
  expression.getAChild().getValue() = "0"
}

predicate isPhrasedAsGTZero(Expr expression) {
  expression instanceof GTExpr and
  expression.getAChild().getValue() = "0"
}

predicate isPhrasedAsEQZero(Expr expression) {
  expression instanceof EQExpr and
  expression.getAChild().getValue() = "0"
}

from X509_matching_function call, GuardCondition guard, string error
where
  Utilities::flowsToGuard(guard, call) and
  not (
    isPhrasedAsLEZero(guard)
    or
    isPhrasedAsNEOne(guard)
    or
    isPhrasedAsEQOne(guard)
    or
    isPhrasedAsGTZero(guard)
    or
    isPhrasedAsEQZero(guard)
  ) and
  error = "Return value check is insecure."
select call, "Incorrect host check: " + error, guard

Results

The query gave us 26 projects where there are calls to certificate matching functions in a potentially incorrect way, which represents roughly eight percent of the projects that use OpenSSL.

Interestingly we got results in OpenSSL itself, let’s check those first:

OpenSSL

Fortunately these issues are either in unit tests or in example applications. As you can see, the return value of the target functions is used in an erroneous way:

/test/v3nametest.c

match = -1;
ret = X509_check_email(crt, name, namelen, 0);
if (fn->email) {
    if (ret && !samename)
        match = 1;
    if (!ret && samename && strchr(nameincert, '@') != NULL)
        match = 0;
} else if (ret)
    match = 1;
if (!TEST_true(check_message(fn, "email", nameincert, match, *pname)))
    failed = 1;
OPENSSL_free(name);

/apps/lib/apps.c

void print_cert_checks(BIO *bio, X509 *x,
                       const char *checkhost,
                       const char *checkemail, const char *checkip)
{
    if (x == NULL)
        return;
    if (checkhost) {
        BIO_printf(bio, "Hostname %s does%s match certificate\n",
                   checkhost,
                   X509_check_host(x, checkhost, 0, 0, NULL) == 1
                       ? "" : " NOT");
    }

    if (checkemail) {
        BIO_printf(bio, "Email %s does%s match certificate\n",
                   checkemail, X509_check_email(x, checkemail, 0, 0)
                   ? "" : " NOT");
    }

    if (checkip) {
        BIO_printf(bio, "IP %s does%s match certificate\n",
                   checkip, X509_check_ip_asc(x, checkip, 0) ? "" : " NOT");
    }
}

Boring SSL

Similar to what we have found in OpenSSL, this code is a unit test so there is no security impact.

/crypto/x509v3/v3name_test.cc

ret = X509_check_email(crt, name, namelen, 0);
match = -1;
if (fn->email) {
    if (ret && !samename)
        match = 1;
    if (!ret && samename && strchr(nameincert, '@') != NULL)
        match = 0;
} else if (ret)
    match = 1;
check_message(fn, "email", nameincert, match, *pname);
++pname;
free(name);

openfortivpn

openfortivpn is an open source VPN client that uses TLS. Unfortunately, if the client is compiled with a modern version of OpenSSL it uses the certificate matching function X509_check_host in a way that allows an invalid certificate to be valid.

/src/tunnel.c

#ifdef HAVE_X509_CHECK_HOST
    _// Use OpenSSL native host validation if v >= 1.0.2._
    if (X509_check_host(cert, common_name, FIELD_SIZE, 0, NULL))
      cert_valid = 1;
#else
    _// Use explicit Common Name check if native validation not available._
    _// Note: this will ignore Subject Alternative Name fields._
    if (subj
        && X509_NAME_get_text_by_NID(subj, NID_commonName, common_name,
                                      FIELD_SIZE) > 0
        && strncasecmp(common_name, tunnel->config->gateway_host,
                        FIELD_SIZE) == 0)
      cert_valid = 1;
#endif

You can find a detailed explanation of these bugs along with proof of concepts at GHSL-2020-003.

lua-openssl

This project is a series of bindings for use in the Lua programming language. In the following code snippet, you can see that they provide wrappers for the certificate matching routines, but they cast the return value of these functions to a bool. The problem with this is when these functions return a negative value (due to an internal error) the end result is that the wrapper interprets the result as a match.

/src/x509.c

static LUA_FUNCTION(openssl_x509_check_host)
{
  X509 * cert = CHECK_OBJECT(1, X509, "openssl.x509");
  if (lua_isstring(L, 2))
  {
    const char *hostname = lua_tostring(L, 2);
    lua_pushboolean(L, X509_check_host(cert, hostname, strlen(hostname), 0, NULL));
  }
  else
  {
    lua_pushboolean(L, 0);
  }
  return 1;
}

static LUA_FUNCTION(openssl_x509_check_email)
{
  X509 * cert = CHECK_OBJECT(1, X509, "openssl.x509");
  if (lua_isstring(L, 2))
  {
    const char *email = lua_tostring(L, 2);
    lua_pushboolean(L, X509_check_email(cert, email, strlen(email), 0));
  }
  else
  {
    lua_pushboolean(L, 0);
  }
  return 1;
}

static LUA_FUNCTION(openssl_x509_check_ip_asc)
{
  X509 * cert = CHECK_OBJECT(1, X509, "openssl.x509");
  if (lua_isstring(L, 2))
  {
    const char *ip_asc = lua_tostring(L, 2);
    lua_pushboolean(L, X509_check_ip_asc(cert, ip_asc, 0));
  }
  else
  {
    lua_pushboolean(L, 0);
  }
  return 1;
}

You can find a detailed explanation of these bugs along with proof of concepts from GHSL-2020-026.

False positives

About 20 other projects were embedding OpenSSL, so the results we found were duplicated. The rest of the results were basically false positives because of the way I developed the query. Those results were discarded after a manual auditing pass.

Conclusion

In this post, we’ve explored how we can take a relatively straightforward vulnerability hypothesis and check it against a large number of projects. We then triaged those results down into actual findings with real world impact. In the process, I learned a lot about the way TLS works along with its potential pitfalls. Hopefully you learned a few things from what I shared for the next time you want to test your own vulnerability hypothesis.

P.S. Thanks Joe, keep sending it.