Allow partially releasing a record for TLS

This enables the cleansing of plaintext to occur in the record layer and
avoids the need to cast away const above the record layer.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/20404)
This commit is contained in:
Matt Caswell 2023-02-27 09:19:16 +00:00 committed by Pauli
parent 2eb91b0ec3
commit 7a4e109ebe
7 changed files with 114 additions and 62 deletions

View File

@ -225,7 +225,8 @@ struct ossl_record_method_st {
* filled in with the epoch and sequence number from the record.
* An opaque record layer handle for the record is returned in |*rechandle|
* which is used in a subsequent call to |release_record|. The buffer must
* remain available until release_record is called.
* remain available until all the bytes from record are released via one or
* more release_record calls.
*
* Internally the the OSSL_RECORD_METHOD the implementation may read/process
* multiple records in one go and buffer them.
@ -234,11 +235,12 @@ struct ossl_record_method_st {
int *type, const unsigned char **data, size_t *datalen,
uint16_t *epoch, unsigned char *seq_num);
/*
* Release a buffer associated with a record previously read with
* read_record. Records are guaranteed to be released in the order that they
* are read.
* Release length bytes from a buffer associated with a record previously
* read with read_record. Once all the bytes from a record are released, the
* whole record and its associated buffer is released. Records are
* guaranteed to be released in the order that they are read.
*/
int (*release_record)(OSSL_RECORD_LAYER *rl, void *rechandle);
int (*release_record)(OSSL_RECORD_LAYER *rl, void *rechandle, size_t length);
/*
* In the event that a fatal error is returned from the functions above then

View File

@ -58,9 +58,12 @@ struct ossl_record_layer_st {
*/
int alert;
/* Amount of crypto stream data we have received but not yet released */
/* Amount of crypto stream data we read in the last call to quic_read_record */
size_t recread;
/* Amount of crypto stream data read but not yet released */
size_t recunreleased;
/* Callbacks */
OSSL_FUNC_rlayer_msg_callback_fn *msg_callback;
void *cbarg;
@ -336,7 +339,7 @@ static int quic_read_record(OSSL_RECORD_LAYER *rl, void **rechandle,
size_t *datalen, uint16_t *epoch,
unsigned char *seq_num)
{
if (rl->recread != 0)
if (rl->recread != 0 || rl->recunreleased != 0)
return OSSL_RECORD_RETURN_FATAL;
BIO_clear_retry_flags(rl->dummybio);
@ -355,7 +358,7 @@ static int quic_read_record(OSSL_RECORD_LAYER *rl, void **rechandle,
*rechandle = rl;
*rversion = TLS1_3_VERSION;
*type = SSL3_RT_HANDSHAKE;
rl->recread = *datalen;
rl->recread = rl->recunreleased = *datalen;
/* epoch/seq_num are not relevant for TLS */
if (rl->msg_callback != NULL) {
@ -386,22 +389,30 @@ static int quic_read_record(OSSL_RECORD_LAYER *rl, void **rechandle,
return OSSL_RECORD_RETURN_SUCCESS;
}
static int quic_release_record(OSSL_RECORD_LAYER *rl, void *rechandle)
static int quic_release_record(OSSL_RECORD_LAYER *rl, void *rechandle,
size_t length)
{
if (!ossl_assert(rl->recread > 0) || !ossl_assert(rl == rechandle)) {
if (!ossl_assert(rl->recread > 0)
|| !ossl_assert(rl->recunreleased <= rl->recread)
|| !ossl_assert(rl == rechandle)
|| !ossl_assert(length <= rl->recunreleased)) {
QUIC_TLS_FATAL(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
return 0;
return OSSL_RECORD_RETURN_FATAL;
}
rl->recunreleased -= length;
if (rl->recunreleased > 0)
return OSSL_RECORD_RETURN_SUCCESS;
if (!rl->qtls->args.crypto_release_rcd_cb(rl->recread,
rl->qtls->args.crypto_release_rcd_cb_arg)) {
QUIC_TLS_FATAL(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
return OSSL_RECORD_RETURN_FATAL;
}
rl->recread = 0;
return 1;
return OSSL_RECORD_RETURN_SUCCESS;
}
static int quic_get_alert_code(OSSL_RECORD_LAYER *rl)

View File

@ -461,7 +461,7 @@ int tls_set1_bio(OSSL_RECORD_LAYER *rl, BIO *bio);
int tls_read_record(OSSL_RECORD_LAYER *rl, void **rechandle, int *rversion,
int *type, const unsigned char **data, size_t *datalen,
uint16_t *epoch, unsigned char *seq_num);
int tls_release_record(OSSL_RECORD_LAYER *rl, void *rechandle);
int tls_release_record(OSSL_RECORD_LAYER *rl, void *rechandle, size_t length);
int tls_default_set_protocol_version(OSSL_RECORD_LAYER *rl, int version);
int tls_set_protocol_version(OSSL_RECORD_LAYER *rl, int version);
void tls_set_plain_alerts(OSSL_RECORD_LAYER *rl, int allow);

View File

@ -1132,15 +1132,32 @@ int tls_read_record(OSSL_RECORD_LAYER *rl, void **rechandle, int *rversion,
return OSSL_RECORD_RETURN_SUCCESS;
}
int tls_release_record(OSSL_RECORD_LAYER *rl, void *rechandle)
int tls_release_record(OSSL_RECORD_LAYER *rl, void *rechandle, size_t length)
{
TLS_RL_RECORD *rec = &rl->rrec[rl->num_released];
if (!ossl_assert(rl->num_released < rl->curr_rec)
|| !ossl_assert(rechandle == &rl->rrec[rl->num_released])) {
|| !ossl_assert(rechandle == rec)) {
/* Should not happen */
RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, SSL_R_INVALID_RECORD);
return OSSL_RECORD_RETURN_FATAL;
}
if (rec->length < length) {
/* Should not happen */
RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
return OSSL_RECORD_RETURN_FATAL;
}
if ((rl->options & SSL_OP_CLEANSE_PLAINTEXT) != 0)
OPENSSL_cleanse(rec->data + rec->off, length);
rec->off += length;
rec->length -= length;
if (rec->length > 0)
return OSSL_RECORD_RETURN_SUCCESS;
rl->num_released++;
if (rl->curr_rec == rl->num_released

View File

@ -293,7 +293,8 @@ int dtls1_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
/* SSLfatal() already called */
return -1;
}
ssl_release_record(sc, rr);
if (!ssl_release_record(sc, rr, 0))
return -1;
goto start;
}
@ -302,7 +303,8 @@ int dtls1_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
* 'peek' mode)
*/
if (sc->shutdown & SSL_RECEIVED_SHUTDOWN) {
ssl_release_record(sc, rr);
if (!ssl_release_record(sc, rr, 0))
return -1;
sc->rwstate = SSL_NOTHING;
return 0;
}
@ -335,8 +337,8 @@ int dtls1_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
* SSL_read() with a zero length buffer will eventually cause
* SSL_pending() to report data as being available.
*/
if (rr->length == 0)
ssl_release_record(sc, rr);
if (rr->length == 0 && !ssl_release_record(sc, rr, 0))
return -1;
return 0;
}
@ -347,16 +349,11 @@ int dtls1_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
memcpy(buf, &(rr->data[rr->off]), n);
if (peek) {
if (rr->length == 0)
ssl_release_record(sc, rr);
if (rr->length == 0 && !ssl_release_record(sc, rr, 0))
return -1;
} else {
/* TODO(RECLAYER): Casting away the const is wrong! FIX ME */
if (sc->options & SSL_OP_CLEANSE_PLAINTEXT)
OPENSSL_cleanse((unsigned char *)&(rr->data[rr->off]), n);
rr->length -= n;
rr->off += n;
if (rr->length == 0)
ssl_release_record(sc, rr);
if (!ssl_release_record(sc, rr, n))
return -1;
}
#ifndef OPENSSL_NO_SCTP
/*
@ -409,7 +406,8 @@ int dtls1_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
if (alert_level == SSL3_AL_WARNING) {
sc->s3.warn_alert = alert_descr;
ssl_release_record(sc, rr);
if (!ssl_release_record(sc, rr, 0))
return -1;
sc->rlayer.alert_count++;
if (sc->rlayer.alert_count == MAX_WARN_ALERT_COUNT) {
@ -444,7 +442,8 @@ int dtls1_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
SSL_AD_REASON_OFFSET + alert_descr,
"SSL alert number %d", alert_descr);
sc->shutdown |= SSL_RECEIVED_SHUTDOWN;
ssl_release_record(sc, rr);
if (!ssl_release_record(sc, rr, 0))
return -1;
SSL_CTX_remove_session(sc->session_ctx, sc->session);
return 0;
} else {
@ -458,7 +457,8 @@ int dtls1_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
if (sc->shutdown & SSL_SENT_SHUTDOWN) { /* but we have not received a
* shutdown */
sc->rwstate = SSL_NOTHING;
ssl_release_record(sc, rr);
if (!ssl_release_record(sc, rr, 0))
return -1;
return 0;
}
@ -467,7 +467,8 @@ int dtls1_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
* We can't process a CCS now, because previous handshake messages
* are still missing, so just drop it.
*/
ssl_release_record(sc, rr);
if (!ssl_release_record(sc, rr, 0))
return -1;
goto start;
}
@ -483,7 +484,8 @@ int dtls1_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
*/
if (rr->epoch != sc->rlayer.d->r_epoch
|| rr->length < DTLS1_HM_HEADER_LENGTH) {
ssl_release_record(sc, rr);
if (!ssl_release_record(sc, rr, 0))
return -1;
goto start;
}
@ -504,7 +506,8 @@ int dtls1_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
if (ossl_statem_in_error(sc))
return -1;
}
ssl_release_record(sc, rr);
if (!ssl_release_record(sc, rr, 0))
return -1;
if (!(sc->mode & SSL_MODE_AUTO_RETRY)) {
if (!sc->rlayer.rrlmethod->unprocessed_read_pending(sc->rlayer.rrl)) {
/* no read-ahead left? */

View File

@ -10,6 +10,7 @@
#include <stdio.h>
#include <limits.h>
#include <errno.h>
#include <assert.h>
#include "../ssl_local.h"
#include <openssl/evp.h>
#include <openssl/buffer.h>
@ -496,17 +497,35 @@ int ossl_tls_handle_rlayer_return(SSL_CONNECTION *s, int writing, int ret,
return ret;
}
void ssl_release_record(SSL_CONNECTION *s, TLS_RECORD *rr)
int ssl_release_record(SSL_CONNECTION *s, TLS_RECORD *rr, size_t length)
{
assert(rr->length >= length);
if (rr->rechandle != NULL) {
if (length == 0)
length = rr->length;
/* The record layer allocated the buffers for this record */
s->rlayer.rrlmethod->release_record(s->rlayer.rrl, rr->rechandle);
} else {
if (HANDLE_RLAYER_READ_RETURN(s,
s->rlayer.rrlmethod->release_record(s->rlayer.rrl,
rr->rechandle,
length)) <= 0) {
/* RLAYER_fatal already called */
return 0;
}
if (length == rr->length)
s->rlayer.curr_rec++;
} else if (length == 0 || length == rr->length) {
/* We allocated the buffers for this record (only happens with DTLS) */
OPENSSL_free(rr->allocdata);
rr->allocdata = NULL;
}
s->rlayer.curr_rec++;
rr->length -= length;
if (rr->length > 0)
rr->off += length;
else
rr->off = 0;
return 1;
}
/*-
@ -700,8 +719,8 @@ int ssl3_read_bytes(SSL *ssl, int type, int *recvd_type, unsigned char *buf,
* SSL_read() with a zero length buffer will eventually cause
* SSL_pending() to report data as being available.
*/
if (rr->length == 0)
ssl_release_record(s, rr);
if (rr->length == 0 && !ssl_release_record(s, rr, 0))
return -1;
return 0;
}
@ -718,16 +737,11 @@ int ssl3_read_bytes(SSL *ssl, int type, int *recvd_type, unsigned char *buf,
buf += n;
if (peek) {
/* Mark any zero length record as consumed CVE-2016-6305 */
if (rr->length == 0)
ssl_release_record(s, rr);
if (rr->length == 0 && !ssl_release_record(s, rr, 0))
return -1;
} else {
/* TODO(RECLAYER) Casting away the const here is wrong! FIX ME */
if (s->options & SSL_OP_CLEANSE_PLAINTEXT)
OPENSSL_cleanse((unsigned char *)&(rr->data[rr->off]), n);
rr->length -= n;
rr->off += n;
if (rr->length == 0)
ssl_release_record(s, rr);
if (!ssl_release_record(s, rr, n))
return -1;
}
if (rr->length == 0
|| (peek && n == rr->length)) {
@ -814,7 +828,8 @@ int ssl3_read_bytes(SSL *ssl, int type, int *recvd_type, unsigned char *buf,
if ((!is_tls13 && alert_level == SSL3_AL_WARNING)
|| (is_tls13 && alert_descr == SSL_AD_USER_CANCELLED)) {
s->s3.warn_alert = alert_descr;
ssl_release_record(s, rr);
if (!ssl_release_record(s, rr, 0))
return -1;
s->rlayer.alert_count++;
if (s->rlayer.alert_count == MAX_WARN_ALERT_COUNT) {
@ -841,7 +856,8 @@ int ssl3_read_bytes(SSL *ssl, int type, int *recvd_type, unsigned char *buf,
SSL_AD_REASON_OFFSET + alert_descr,
"SSL alert number %d", alert_descr);
s->shutdown |= SSL_RECEIVED_SHUTDOWN;
ssl_release_record(s, rr);
if (!ssl_release_record(s, rr, 0))
return -1;
SSL_CTX_remove_session(s->session_ctx, s->session);
return 0;
} else if (alert_descr == SSL_AD_NO_RENEGOTIATION) {
@ -876,7 +892,8 @@ int ssl3_read_bytes(SSL *ssl, int type, int *recvd_type, unsigned char *buf,
* sent close_notify.
*/
if (!SSL_CONNECTION_IS_TLS13(s)) {
ssl_release_record(s, rr);
if (!ssl_release_record(s, rr, 0))
return -1;
if ((s->mode & SSL_MODE_AUTO_RETRY) != 0)
goto start;
@ -895,7 +912,8 @@ int ssl3_read_bytes(SSL *ssl, int type, int *recvd_type, unsigned char *buf,
* above.
* No alert sent because we already sent close_notify
*/
ssl_release_record(s, rr);
if (!ssl_release_record(s, rr, 0))
return -1;
SSLfatal(s, SSL_AD_NO_ALERT,
SSL_R_APPLICATION_DATA_AFTER_CLOSE_NOTIFY);
return -1;
@ -918,12 +936,12 @@ int ssl3_read_bytes(SSL *ssl, int type, int *recvd_type, unsigned char *buf,
n = rr->length; /* available bytes */
/* now move 'n' bytes: */
memcpy(dest + *dest_len, rr->data + rr->off, n);
rr->off += n;
rr->length -= n;
*dest_len += n;
if (rr->length == 0)
ssl_release_record(s, rr);
if (n > 0) {
memcpy(dest + *dest_len, rr->data + rr->off, n);
*dest_len += n;
if (!ssl_release_record(s, rr, n))
return -1;
}
if (*dest_len < dest_maxlen)
goto start; /* fragment was too small */
@ -1027,7 +1045,8 @@ int ssl3_read_bytes(SSL *ssl, int type, int *recvd_type, unsigned char *buf,
/* SSLfatal() already called */
return -1;
}
ssl_release_record(s, rr);
if (!ssl_release_record(s, rr, 0))
return -1;
goto start;
} else {
SSLfatal(s, SSL_AD_UNEXPECTED_MESSAGE, SSL_R_UNEXPECTED_RECORD);

View File

@ -165,7 +165,7 @@ __owur int dtls1_write_bytes(SSL_CONNECTION *s, int type, const void *buf,
int do_dtls1_write(SSL_CONNECTION *s, int type, const unsigned char *buf,
size_t len, size_t *written);
void dtls1_increment_epoch(SSL_CONNECTION *s, int rw);
void ssl_release_record(SSL_CONNECTION *s, TLS_RECORD *rr);
int ssl_release_record(SSL_CONNECTION *s, TLS_RECORD *rr, size_t length);
# define HANDLE_RLAYER_READ_RETURN(s, ret) \
ossl_tls_handle_rlayer_return(s, 0, ret, OPENSSL_FILE, OPENSSL_LINE)