Convert the write record layer to supply proper return values

This also means we can convert SSLfatal calls to RLAYERfatal

Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/19198)
This commit is contained in:
Matt Caswell 2022-08-31 17:37:48 +01:00
parent 5361a5a966
commit 320145d5b3
5 changed files with 88 additions and 67 deletions

View File

@ -107,7 +107,7 @@ static int tls_setup_write_buffer(OSSL_RECORD_LAYER *rl, size_t numwpipes,
rl->numwpipes = numwpipes;
if (len == 0) {
if (SSL_CONNECTION_IS_DTLS(s))
if (rl->isdtls)
headerlen = DTLS1_RT_HEADER_LENGTH + 1;
else
headerlen = SSL3_RT_HEADER_LENGTH;
@ -145,7 +145,7 @@ static int tls_setup_write_buffer(OSSL_RECORD_LAYER *rl, size_t numwpipes,
* buffers. We assume we're so doomed that we won't even be able
* to send an alert.
*/
SSLfatal(s, SSL_AD_NO_ALERT, ERR_R_MALLOC_FAILURE);
RLAYERfatal(rl, SSL_AD_NO_ALERT, ERR_R_MALLOC_FAILURE);
return 0;
}
} else {
@ -1438,7 +1438,7 @@ int tls_write_records(OSSL_RECORD_LAYER *rl, OSSL_RECORD_TEMPLATE *templates,
/* Check we don't have pending data waiting to write */
if (!ossl_assert(rl->nextwbuf >= rl->numwpipes
|| SSL3_BUFFER_get_left(&rl->wbuf[rl->nextwbuf]) == 0)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
goto err;
}
@ -1452,7 +1452,7 @@ int tls_write_records(OSSL_RECORD_LAYER *rl, OSSL_RECORD_TEMPLATE *templates,
} else {
mac_size = EVP_MD_CTX_get_size(s->write_hash);
if (mac_size < 0) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
goto err;
}
}
@ -1472,14 +1472,14 @@ int tls_write_records(OSSL_RECORD_LAYER *rl, OSSL_RECORD_TEMPLATE *templates,
* smaller. It is wasteful to allocate a full sized buffer here
*/
if (!tls_setup_write_buffer(rl, numtempl + prefix, 0)) {
/* SSLfatal() already called */
return -1;
/* RLAYERfatal() already called */
goto err;
}
}
using_ktls = BIO_get_ktls_send(rl->bio);
if (!ossl_assert(!using_ktls || !prefix)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
goto err;
}
@ -1509,7 +1509,7 @@ int tls_write_records(OSSL_RECORD_LAYER *rl, OSSL_RECORD_TEMPLATE *templates,
if (!WPACKET_init_static_len(&pkt[0], SSL3_BUFFER_get_buf(wb),
SSL3_BUFFER_get_len(wb), 0)
|| !WPACKET_allocate_bytes(&pkt[0], align, NULL)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
goto err;
}
wpinited = 1;
@ -1541,7 +1541,7 @@ int tls_write_records(OSSL_RECORD_LAYER *rl, OSSL_RECORD_TEMPLATE *templates,
if (!WPACKET_init_static_len(thispkt, SSL3_BUFFER_get_buf(wb),
SSL3_BUFFER_get_len(wb), 0)
|| !WPACKET_allocate_bytes(thispkt, align, NULL)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
goto err;
}
wpinited++;
@ -1556,7 +1556,7 @@ int tls_write_records(OSSL_RECORD_LAYER *rl, OSSL_RECORD_TEMPLATE *templates,
if (mode == EVP_CIPH_CBC_MODE) {
eivlen = EVP_CIPHER_CTX_get_iv_length(s->enc_write_ctx);
if (eivlen < 0) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_R_LIBRARY_BUG);
RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, SSL_R_LIBRARY_BUG);
goto err;
}
if (eivlen <= 1)
@ -1615,7 +1615,7 @@ int tls_write_records(OSSL_RECORD_LAYER *rl, OSSL_RECORD_TEMPLATE *templates,
|| (maxcomplen > 0
&& !WPACKET_reserve_bytes(thispkt, maxcomplen,
&compressdata)))) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
goto err;
}
@ -1638,7 +1638,7 @@ int tls_write_records(OSSL_RECORD_LAYER *rl, OSSL_RECORD_TEMPLATE *templates,
if (s->compress != NULL) {
if (!ssl3_do_compress(s, thiswr)
|| !WPACKET_allocate_bytes(thispkt, thiswr->length, NULL)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_R_COMPRESSION_FAILURE);
RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, SSL_R_COMPRESSION_FAILURE);
goto err;
}
} else {
@ -1646,7 +1646,7 @@ int tls_write_records(OSSL_RECORD_LAYER *rl, OSSL_RECORD_TEMPLATE *templates,
SSL3_RECORD_reset_data(&wr[j]);
} else {
if (!WPACKET_memcpy(thispkt, thiswr->input, thiswr->length)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
goto err;
}
SSL3_RECORD_reset_input(&wr[j]);
@ -1661,7 +1661,7 @@ int tls_write_records(OSSL_RECORD_LAYER *rl, OSSL_RECORD_TEMPLATE *templates,
size_t rlen, max_send_fragment;
if (!WPACKET_put_bytes_u8(thispkt, thistempl->type)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
goto err;
}
SSL3_RECORD_add_length(thiswr, 1);
@ -1695,8 +1695,8 @@ int tls_write_records(OSSL_RECORD_LAYER *rl, OSSL_RECORD_TEMPLATE *templates,
if (padding > max_padding)
padding = max_padding;
if (!WPACKET_memset(thispkt, 0, padding)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR,
ERR_R_INTERNAL_ERROR);
RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR,
ERR_R_INTERNAL_ERROR);
goto err;
}
SSL3_RECORD_add_length(thiswr, padding);
@ -1715,7 +1715,7 @@ int tls_write_records(OSSL_RECORD_LAYER *rl, OSSL_RECORD_TEMPLATE *templates,
if (!WPACKET_allocate_bytes(thispkt, mac_size, &mac)
|| !ssl->method->ssl3_enc->mac(s, thiswr, mac, 1)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
goto err;
}
}
@ -1729,13 +1729,13 @@ int tls_write_records(OSSL_RECORD_LAYER *rl, OSSL_RECORD_TEMPLATE *templates,
if (!WPACKET_reserve_bytes(thispkt,
SSL_RT_MAX_CIPHER_BLOCK_SIZE,
NULL)
/*
* We also need next the amount of bytes written to this
* sub-packet
*/
|| !WPACKET_get_length(thispkt, &len)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
goto err;
/*
* We also need next the amount of bytes written to this
* sub-packet
*/
|| !WPACKET_get_length(thispkt, &len)) {
RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
goto err;
}
/* Get a pointer to the start of this record excluding header */
@ -1752,26 +1752,23 @@ int tls_write_records(OSSL_RECORD_LAYER *rl, OSSL_RECORD_TEMPLATE *templates,
* send early data - so we need to use the tls13enc function.
*/
if (tls13_enc(s, wr, numtempl, 1, NULL, mac_size) < 1) {
if (!ossl_statem_in_error(s)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
}
if (!ossl_statem_in_error(s))
RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
goto err;
}
} else {
if (!using_ktls) {
if (prefix) {
if (ssl->method->ssl3_enc->enc(s, wr, 1, 1, NULL, mac_size) < 1) {
if (!ossl_statem_in_error(s)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
}
if (!ossl_statem_in_error(s))
RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
goto err;
}
}
if (ssl->method->ssl3_enc->enc(s, wr + prefix, numtempl, 1, NULL,
mac_size) < 1) {
if (!ossl_statem_in_error(s)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
}
if (!ossl_statem_in_error(s))
RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
goto err;
}
}
@ -1796,7 +1793,7 @@ int tls_write_records(OSSL_RECORD_LAYER *rl, OSSL_RECORD_TEMPLATE *templates,
&& !WPACKET_allocate_bytes(thispkt,
thiswr->length - origlen,
NULL))) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
goto err;
}
if (rl->use_etm && mac_size != 0) {
@ -1804,7 +1801,7 @@ int tls_write_records(OSSL_RECORD_LAYER *rl, OSSL_RECORD_TEMPLATE *templates,
if (!WPACKET_allocate_bytes(thispkt, mac_size, &mac)
|| !ssl->method->ssl3_enc->mac(s, thiswr, mac, 1)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
goto err;
}
SSL3_RECORD_add_length(thiswr, mac_size);
@ -1812,7 +1809,7 @@ int tls_write_records(OSSL_RECORD_LAYER *rl, OSSL_RECORD_TEMPLATE *templates,
if (!WPACKET_get_length(thispkt, &len)
|| !WPACKET_close(thispkt)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
goto err;
}
@ -1831,7 +1828,7 @@ int tls_write_records(OSSL_RECORD_LAYER *rl, OSSL_RECORD_TEMPLATE *templates,
}
if (!WPACKET_finish(thispkt)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
goto err;
}
@ -1856,30 +1853,23 @@ int tls_write_records(OSSL_RECORD_LAYER *rl, OSSL_RECORD_TEMPLATE *templates,
err:
for (j = 0; j < wpinited; j++)
WPACKET_cleanup(&pkt[j]);
return -1;
return OSSL_RECORD_RETURN_FATAL;
}
/* if SSL3_BUFFER_get_left() != 0, we need to call this
*
* Return values are as per SSL_write()
*/
int tls_retry_write_records(OSSL_RECORD_LAYER *rl)
{
int i;
int i, ret;
SSL3_BUFFER *thiswb;
size_t tmpwrit = 0;
SSL_CONNECTION *s = rl->cbarg;
if (rl->nextwbuf >= rl->numwpipes)
return 1;
return OSSL_RECORD_RETURN_SUCCESS;
for (;;) {
thiswb = &rl->wbuf[rl->nextwbuf];
clear_sys_error();
if (rl->bio != NULL) {
s->rwstate = SSL_WRITING;
/*
* To prevent coalescing of control and data messages,
* such as in buffer_write, we flush the BIO
@ -1887,18 +1877,34 @@ int tls_retry_write_records(OSSL_RECORD_LAYER *rl)
if (BIO_get_ktls_send(rl->bio)
&& thiswb->type != SSL3_RT_APPLICATION_DATA) {
i = BIO_flush(rl->bio);
if (i <= 0)
return i;
if (i <= 0) {
if (BIO_should_retry(rl->bio))
ret = OSSL_RECORD_RETURN_RETRY;
else
ret = OSSL_RECORD_RETURN_FATAL;
return ret;
}
BIO_set_ktls_ctrl_msg(rl->bio, thiswb->type);
}
i = BIO_write(rl->bio, (char *)
&(SSL3_BUFFER_get_buf(thiswb)
[SSL3_BUFFER_get_offset(thiswb)]),
(unsigned int)SSL3_BUFFER_get_left(thiswb));
if (i >= 0)
if (i >= 0) {
tmpwrit = i;
if (i == 0 && BIO_should_retry(rl->bio))
ret = OSSL_RECORD_RETURN_RETRY;
else
ret = OSSL_RECORD_RETURN_SUCCESS;
} else {
if (BIO_should_retry(rl->bio))
ret = OSSL_RECORD_RETURN_RETRY;
else
ret = OSSL_RECORD_RETURN_FATAL;
}
} else {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_R_BIO_NOT_SET);
RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, SSL_R_BIO_NOT_SET);
ret = OSSL_RECORD_RETURN_FATAL;
i = -1;
}
@ -1914,12 +1920,11 @@ int tls_retry_write_records(OSSL_RECORD_LAYER *rl)
SSL3_BUFFER_add_offset(thiswb, tmpwrit);
if (++(rl->nextwbuf) < rl->numwpipes)
continue;
s->rwstate = SSL_NOTHING;
if (rl->nextwbuf == rl->numwpipes
&& (rl->mode & SSL_MODE_RELEASE_BUFFERS) != 0)
tls_release_write_buffer(rl);
return 1;
return OSSL_RECORD_RETURN_SUCCESS;
} else if (i <= 0) {
if (rl->isdtls) {
/*
@ -1932,7 +1937,7 @@ int tls_retry_write_records(OSSL_RECORD_LAYER *rl)
tls_release_write_buffer(rl);
}
return i;
return ret;
}
SSL3_BUFFER_add_offset(thiswb, tmpwrit);
SSL3_BUFFER_sub_left(thiswb, tmpwrit);

View File

@ -270,7 +270,7 @@ int dtls1_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
do {
rr = &sc->rlayer.tlsrecs[sc->rlayer.num_recs];
ret = HANDLE_RLAYER_RETURN(sc,
ret = HANDLE_RLAYER_READ_RETURN(sc,
sc->rlayer.rrlmethod->read_record(sc->rlayer.rrl,
&rr->rechandle,
&rr->version, &rr->type,

View File

@ -266,7 +266,8 @@ int ssl3_write_bytes(SSL *ssl, int type, const void *buf_, size_t len,
return i;
} else if (i > 0) {
/* Retry needed */
i = s->rlayer.wrlmethod->retry_write_records(s->rlayer.wrl);
i = HANDLE_RLAYER_WRITE_RETURN(s,
s->rlayer.wrlmethod->retry_write_records(s->rlayer.wrl));
if (i <= 0)
return i;
tot += s->rlayer.wpend_tot;
@ -539,7 +540,8 @@ int ssl3_write_bytes(SSL *ssl, int type, const void *buf_, size_t len,
s->rlayer.wpend_tot = n;
}
i = s->rlayer.wrlmethod->write_records(s->rlayer.wrl, tmpls, numpipes);
i = HANDLE_RLAYER_WRITE_RETURN(s,
s->rlayer.wrlmethod->write_records(s->rlayer.wrl, tmpls, numpipes));
if (i <= 0) {
/* SSLfatal() already called if appropriate */
s->rlayer.wnum = tot;
@ -559,18 +561,28 @@ int ssl3_write_bytes(SSL *ssl, int type, const void *buf_, size_t len,
}
}
int ossl_tls_handle_rlayer_return(SSL_CONNECTION *s, int ret, char *file,
int line)
int ossl_tls_handle_rlayer_return(SSL_CONNECTION *s, int writing, int ret,
char *file, int line)
{
SSL *ssl = SSL_CONNECTION_GET_SSL(s);
if (ret == OSSL_RECORD_RETURN_RETRY) {
s->rwstate = SSL_READING;
s->rwstate = writing ? SSL_WRITING : SSL_READING;
ret = -1;
} else {
s->rwstate = SSL_NOTHING;
if (ret == OSSL_RECORD_RETURN_EOF) {
if (s->options & SSL_OP_IGNORE_UNEXPECTED_EOF) {
if (writing) {
/*
* This shouldn't happen with a writing operation. We treat it
* as fatal.
*/
ERR_new();
ERR_set_debug(file, line, 0);
ossl_statem_fatal(s, SSL_AD_INTERNAL_ERROR,
ERR_R_INTERNAL_ERROR, NULL);
ret = OSSL_RECORD_RETURN_FATAL;
} else if ((s->options & SSL_OP_IGNORE_UNEXPECTED_EOF) != 0) {
SSL_set_shutdown(ssl, SSL_RECEIVED_SHUTDOWN);
s->s3.warn_alert = SSL_AD_CLOSE_NOTIFY;
} else {
@ -725,7 +737,7 @@ int ssl3_read_bytes(SSL *ssl, int type, int *recvd_type, unsigned char *buf,
do {
rr = &s->rlayer.tlsrecs[s->rlayer.num_recs];
ret = HANDLE_RLAYER_RETURN(s,
ret = HANDLE_RLAYER_READ_RETURN(s,
s->rlayer.rrlmethod->read_record(s->rlayer.rrl,
&rr->rechandle,
&rr->version, &rr->type,

View File

@ -252,11 +252,14 @@ int do_dtls1_write(SSL_CONNECTION *s, int type, const unsigned char *buf,
void dtls1_reset_seq_numbers(SSL_CONNECTION *s, int rw);
void ssl_release_record(SSL_CONNECTION *s, TLS_RECORD *rr);
# define HANDLE_RLAYER_RETURN(s, ret) \
ossl_tls_handle_rlayer_return(s, ret, OPENSSL_FILE, OPENSSL_LINE)
# define HANDLE_RLAYER_READ_RETURN(s, ret) \
ossl_tls_handle_rlayer_return(s, 0, ret, OPENSSL_FILE, OPENSSL_LINE)
int ossl_tls_handle_rlayer_return(SSL_CONNECTION *s, int ret, char *file,
int line);
# define HANDLE_RLAYER_WRITE_RETURN(s, ret) \
ossl_tls_handle_rlayer_return(s, 1, ret, OPENSSL_FILE, OPENSSL_LINE)
int ossl_tls_handle_rlayer_return(SSL_CONNECTION *s, int writing, int ret,
char *file, int line);
int ssl_set_new_record_layer(SSL_CONNECTION *s, int version, int direction,
int level, unsigned char *key, size_t keylen,

View File

@ -108,7 +108,8 @@ int ssl3_dispatch_alert(SSL *s)
if (RECORD_LAYER_write_pending(&sc->rlayer))
return -1;
i = sc->rlayer.wrlmethod->write_records(sc->rlayer.wrl, &templ, 1);
i = HANDLE_RLAYER_WRITE_RETURN(sc,
sc->rlayer.wrlmethod->write_records(sc->rlayer.wrl, &templ, 1));
if (i <= 0) {
sc->s3.alert_dispatch = 1;
} else {