From b85ebc4b279ff0abe81c3a64eafc4f3c6c00605e Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Tue, 9 Aug 2022 15:52:27 +0100 Subject: [PATCH] Check record layer callbacks are non-null The current libssl code always ensures that the callbacks are non-null. However, the record layer itself wasn't checkthing this. We ensure it does. Reviewed-by: Hugo Landau Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/18132) --- ssl/record/methods/dtls_meth.c | 5 +++-- ssl/record/methods/tls_common.c | 18 ++++++++++++------ 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/ssl/record/methods/dtls_meth.c b/ssl/record/methods/dtls_meth.c index 7ac49a0a9a..0964dfe98d 100644 --- a/ssl/record/methods/dtls_meth.c +++ b/ssl/record/methods/dtls_meth.c @@ -431,8 +431,9 @@ int dtls_get_more_records(OSSL_RECORD_LAYER *rl) p = rl->packet; - rl->msg_callback(0, 0, SSL3_RT_HEADER, p, DTLS1_RT_HEADER_LENGTH, - rl->cbarg); + if (rl->msg_callback != NULL) + rl->msg_callback(0, 0, SSL3_RT_HEADER, p, DTLS1_RT_HEADER_LENGTH, + rl->cbarg); /* Pull apart the header into the DTLS1_RECORD */ rr->type = *(p++); diff --git a/ssl/record/methods/tls_common.c b/ssl/record/methods/tls_common.c index 2c6c73a2a0..c32d1e37bd 100644 --- a/ssl/record/methods/tls_common.c +++ b/ssl/record/methods/tls_common.c @@ -89,7 +89,8 @@ static int tls_allow_compression(OSSL_RECORD_LAYER *rl) if (rl->options & SSL_OP_NO_COMPRESSION) return 0; - return rl->security(rl->cbarg, SSL_SECOP_COMPRESSION, 0, 0, NULL); + return rl->security == NULL + || rl->security(rl->cbarg, SSL_SECOP_COMPRESSION, 0, 0, NULL); } #endif @@ -500,7 +501,8 @@ int tls_get_more_records(OSSL_RECORD_LAYER *rl) if (!PACKET_get_1(&pkt, &type) || !PACKET_get_net_2(&pkt, &version) || !PACKET_get_net_2_len(&pkt, &thisrr->length)) { - rl->msg_callback(0, 0, SSL3_RT_HEADER, p, 5, rl->cbarg); + if (rl->msg_callback != NULL) + rl->msg_callback(0, 0, SSL3_RT_HEADER, p, 5, rl->cbarg); RLAYERfatal(rl, SSL_AD_DECODE_ERROR, ERR_R_INTERNAL_ERROR); return OSSL_RECORD_RETURN_FATAL; } @@ -519,7 +521,8 @@ int tls_get_more_records(OSSL_RECORD_LAYER *rl) return OSSL_RECORD_RETURN_FATAL; } - rl->msg_callback(0, version, SSL3_RT_HEADER, p, 5, rl->cbarg); + if (rl->msg_callback != NULL) + rl->msg_callback(0, version, SSL3_RT_HEADER, p, 5, rl->cbarg); if (thisrr->length > SSL3_BUFFER_get_len(rbuf) - SSL3_RT_HEADER_LENGTH) { @@ -693,7 +696,9 @@ int tls_get_more_records(OSSL_RECORD_LAYER *rl) /* RLAYERfatal() already got called */ goto end; } - if (num_recs == 1 && rl->skip_early_data(rl->cbarg)) { + if (num_recs == 1 + && rl->skip_early_data != NULL + && rl->skip_early_data(rl->cbarg)) { /* * Valid early_data that we cannot decrypt will fail here. We treat * it like an empty record. @@ -912,8 +917,9 @@ int tls13_common_post_process_record(OSSL_RECORD_LAYER *rl, SSL3_RECORD *rec) return 0; } - rl->msg_callback(0, rl->version, SSL3_RT_INNER_CONTENT_TYPE, &rec->type, - 1, rl->cbarg); + if (rl->msg_callback != NULL) + rl->msg_callback(0, rl->version, SSL3_RT_INNER_CONTENT_TYPE, &rec->type, + 1, rl->cbarg); /* * TLSv1.3 alert and handshake records are required to be non-zero in