Skip to content

OPENSSL_die when EVP_MD_CTX_new allocation fails in ssl_handshake_hash #15491

@trevorlarock

Description

@trevorlarock

Hi folks,

I have a question on EVP_MD_CTX_new failure handling in
ssl_lib.c::ssl_handshake_hash (observed in 1.1.1e but same in 1.1.1k)

/* Retrieve handshake hashes */
int ssl_handshake_hash(SSL *s, unsigned char *out, size_t outlen,
                       size_t *hashlen)
{
    EVP_MD_CTX *ctx = NULL;
    EVP_MD_CTX *hdgst = s->s3->handshake_dgst;
    int hashleni = EVP_MD_CTX_size(hdgst);
    int ret = 0;

    if (hashleni < 0 || (size_t)hashleni > outlen) {
        SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_SSL_HANDSHAKE_HASH,
                 ERR_R_INTERNAL_ERROR);
        goto err;
    }

    ctx = EVP_MD_CTX_new();
    if (ctx == NULL)
        goto err; // <---- SSLfatal not called before

    if (!EVP_MD_CTX_copy_ex(ctx, hdgst)
        || EVP_DigestFinal_ex(ctx, out, NULL) <= 0) {
        SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_SSL_HANDSHAKE_HASH,
                 ERR_R_INTERNAL_ERROR);
        goto err;
    }
..

In ssl/statem/statem.c there seems to be an expectation that SSLfatal is called
first:

static SUB_STATE_RETURN read_state_machine(SSL *s)
{
..
            switch (ret) {
            case MSG_PROCESS_ERROR:
                check_fatal(s, SSL_F_READ_STATE_MACHINE);
                return SUB_STATE_ERROR;

Which leads to an assert:

(gdb) bt
#0  0x00007f960cbe1625 in raise () from /lib64/libc.so.6
#1  0x00007f960cbca8d9 in abort () from /lib64/libc.so.6
#2  0x00007f960ceb2f73 in OPENSSL_die (message=0x7f960d177a50 "Assertion failed: (s)->statem.in_init && (s)->statem.state == MSG_FLOW_ERROR",
    file=0x7f960d177a38 "ssl/statem/statem.c", line=643) at crypto/cryptlib.c:421
#3  0x00007f960d14672f in ossl_assert_int (expr=0, exprstr=0x7f960d177a50 "Assertion failed: (s)->statem.in_init && (s)->statem.state == MSG_FLOW_ERROR",
    file=0x7f960d177a38 "ssl/statem/statem.c", line=643) at include/internal/cryptlib.h:33
#4  0x00007f960d147612 in read_state_machine (s=0x1162110) at ssl/statem/statem.c:643
#5  0x00007f960d147005 in state_machine (s=0x1162110, server=0) at ssl/statem/statem.c:429
#6  0x00007f960d146b0b in ossl_statem_set_hello_verify_done (s=0x1162110) at ssl/statem/statem.c:245
#7  0x00007f960d10bd80 in ssl3_write_bytes (s=0x1162110, type=23, buf_=0x1157100, len=0, written=0x7fff49f3f4b0) at ssl/record/rec_layer_s3.c:396
#8  0x00007f960d11b063 in ssl3_write (s=0x1162110, buf=0x1157100, len=0, written=0x7fff49f3f4b0) at ssl/s3_lib.c:4460
#9  0x00007f960d129e23 in ssl_write_internal (s=0x1162110, buf=0x1157100, num=0, written=0x7fff49f3f4b0) at ssl/ssl_lib.c:1952
#10 0x00007f960d129e80 in SSL_write (s=0x1162110, buf=0x1157100, num=0) at ssl/ssl_lib.c:1966
#11 0x000000000044b8cc in s_client_main (argc=0, argv=0x7fff49f40150) at apps/s_client.c:2859
#12 0x0000000000436bc0 in do_cmd (prog=0x1151ab0, argc=3, argv=0x7fff49f40150) at apps/openssl.c:570
#13 0x00000000004360d5 in main (argc=3, argv=0x7fff49f40150) at apps/openssl.c:189

Question is if SSLfatal should be called first when EVP_MD_CTX_new fails in ssl_handshake_hash.

Metadata

Metadata

Assignees

No one assigned

    Labels

    triaged: bugThe issue/pr is/fixes a bug

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions