-
-
Notifications
You must be signed in to change notification settings - Fork 11.1k
Closed
prchander/openssl
#3Labels
triaged: bugThe issue/pr is/fixes a bugThe issue/pr is/fixes a bug
Description
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.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
triaged: bugThe issue/pr is/fixes a bugThe issue/pr is/fixes a bug