Skip to content

Commit

Permalink
Replace references to s->wbio with rl->bio
Browse files Browse the repository at this point in the history
We use the record layer reference to the BIO rather than the SSL object
reference. This removes an unneeded SSL object usage.

Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from #19198)
  • Loading branch information
mattcaswell committed Sep 23, 2022
1 parent 3105901 commit b5cf81f
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 34 deletions.
6 changes: 6 additions & 0 deletions ssl/record/methods/ktls_meth.c
Expand Up @@ -517,6 +517,12 @@ ktls_new_record_layer(OSSL_LIB_CTX *libctx, const char *propq, int vers,

(*retrl)->funcs = &ossl_ktls_funcs;

/*
* TODO(RECLAYER): We're not ready to set the crypto state for the write
* record layer. Fix this once we are
*/
if (direction == OSSL_RECORD_DIRECTION_WRITE)
return 1;
ret = (*retrl)->funcs->set_crypto_state(*retrl, level, key, keylen, iv,
ivlen, mackey, mackeylen, ciph,
taglen, mactype, md, comp);
Expand Down
12 changes: 6 additions & 6 deletions ssl/record/methods/tls_common.c
Expand Up @@ -1365,7 +1365,7 @@ int tls_write_records(OSSL_RECORD_LAYER *rl, OSSL_RECORD_TEMPLATE *templates,
}
}

using_ktls = BIO_get_ktls_send(s->wbio);
using_ktls = BIO_get_ktls_send(rl->bio);
if (!ossl_assert(!using_ktls || !prefix)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
goto err;
Expand Down Expand Up @@ -1781,21 +1781,21 @@ int tls_retry_write_records(OSSL_RECORD_LAYER *rl)
continue;
}
clear_sys_error();
if (s->wbio != NULL) {
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
*/
if (BIO_get_ktls_send(s->wbio)
if (BIO_get_ktls_send(rl->bio)
&& thiswb->type != SSL3_RT_APPLICATION_DATA) {
i = BIO_flush(s->wbio);
i = BIO_flush(rl->bio);
if (i <= 0)
return i;
BIO_set_ktls_ctrl_msg(s->wbio, thiswb->type);
BIO_set_ktls_ctrl_msg(rl->bio, thiswb->type);
}
i = BIO_write(s->wbio, (char *)
i = BIO_write(rl->bio, (char *)
&(SSL3_BUFFER_get_buf(thiswb)
[SSL3_BUFFER_get_offset(thiswb)]),
(unsigned int)SSL3_BUFFER_get_left(thiswb));
Expand Down
47 changes: 21 additions & 26 deletions ssl/record/rec_layer_s3.c
Expand Up @@ -1256,9 +1256,8 @@ int ssl_set_new_record_layer(SSL_CONNECTION *s, int version,
{
OSSL_PARAM options[5], *opts = options;
OSSL_PARAM settings[6], *set = settings;
const OSSL_RECORD_METHOD *origmeth = s->rlayer.rrlmethod;
const OSSL_RECORD_METHOD **thismethod;
OSSL_RECORD_LAYER **thisrl;
OSSL_RECORD_LAYER **thisrl, *newrl = NULL;
BIO *thisbio;
SSL_CTX *sctx = SSL_CONNECTION_GET_CTX(s);
const OSSL_RECORD_METHOD *meth;
Expand All @@ -1279,16 +1278,10 @@ int ssl_set_new_record_layer(SSL_CONNECTION *s, int version,
thisbio = s->wbio;
}

if (*thismethod != NULL && !(*thismethod)->free(*thisrl)) {
ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR);
return 0;
}

*thisrl = NULL;
if (meth != NULL)
*thismethod = meth;
if (meth == NULL)
meth = *thismethod;

if (!ossl_assert(*thismethod != NULL)) {
if (!ossl_assert(meth != NULL)) {
ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR);
return 0;
}
Expand Down Expand Up @@ -1370,7 +1363,6 @@ int ssl_set_new_record_layer(SSL_CONNECTION *s, int version,
BIO *next = NULL;
unsigned int epoch = 0;;


if (direction == OSSL_RECORD_DIRECTION_READ) {
prev = s->rlayer.rrlnext;
if (SSL_CONNECTION_IS_DTLS(s)
Expand All @@ -1390,31 +1382,26 @@ int ssl_set_new_record_layer(SSL_CONNECTION *s, int version,
s->rlayer.rrlnext = next;
}

rlret = (*thismethod)->new_record_layer(sctx->libctx,
sctx->propq,
version, s->server,
direction, level, epoch,
key, keylen, iv, ivlen,
mackey, mackeylen, ciph,
taglen, mactype, md, comp,
prev, thisbio,
next, NULL,
NULL, settings, options,
rlayer_dispatch, s,
thisrl);
rlret = meth->new_record_layer(sctx->libctx, sctx->propq, version,
s->server, direction, level, epoch,
key, keylen, iv, ivlen, mackey,
mackeylen, ciph, taglen, mactype, md,
comp, prev, thisbio, next, NULL, NULL,
settings, options, rlayer_dispatch, s,
&newrl);
BIO_free(prev);
switch (rlret) {
case OSSL_RECORD_RETURN_FATAL:
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_R_RECORD_LAYER_FAILURE);
return 0;

case OSSL_RECORD_RETURN_NON_FATAL_ERR:
if (*thismethod != origmeth && origmeth != NULL) {
if (*thismethod != meth && *thismethod != NULL) {
/*
* We tried a new record layer method, but it didn't work out,
* so we fallback to the original method and try again
*/
*thismethod = origmeth;
meth = *thismethod;
continue;
}
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_R_NO_SUITABLE_RECORD_LAYER);
Expand All @@ -1431,5 +1418,13 @@ int ssl_set_new_record_layer(SSL_CONNECTION *s, int version,
break;
}

if (*thismethod != NULL && !(*thismethod)->free(*thisrl)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
return 0;
}

*thisrl = newrl;
*thismethod = meth;

return ssl_post_record_layer_select(s, direction);
}
5 changes: 5 additions & 0 deletions ssl/s3_msg.c
Expand Up @@ -87,6 +87,11 @@ int ssl3_dispatch_alert(SSL *s)

sc->s3.alert_dispatch = 0;

if (sc->rlayer.wrlmethod == NULL) {
/* No write record layer so we can't sent and alert. We just ignore it */
return 1;
}

templ.type = SSL3_RT_ALERT;
templ.buf = &sc->s3.send_alert[0];
templ.buflen = 2;
Expand Down
10 changes: 8 additions & 2 deletions ssl/ssl_lib.c
Expand Up @@ -1358,8 +1358,6 @@ void ossl_ssl_connection_free(SSL *ssl)
X509_VERIFY_PARAM_free(s->param);
dane_final(&s->dane);

RECORD_LAYER_clear(&s->rlayer);

/* Ignore return value */
ssl_free_wbio_buffer(s);

Expand All @@ -1368,6 +1366,8 @@ void ossl_ssl_connection_free(SSL *ssl)
BIO_free_all(s->rbio);
s->rbio = NULL;

RECORD_LAYER_clear(&s->rlayer);

BUF_MEM_free(s->init_buf);

/* add extra stuff */
Expand Down Expand Up @@ -1463,6 +1463,8 @@ void SSL_set0_wbio(SSL *s, BIO *wbio)
/* Re-attach |bbio| to the new |wbio|. */
if (sc->bbio != NULL)
sc->wbio = BIO_push(sc->bbio, sc->wbio);

sc->rlayer.wrlmethod->set1_bio(sc->rlayer.wrl, sc->wbio);
}

void SSL_set_bio(SSL *s, BIO *rbio, BIO *wbio)
Expand Down Expand Up @@ -4809,6 +4811,8 @@ int ssl_init_wbio_buffer(SSL_CONNECTION *s)
s->bbio = bbio;
s->wbio = BIO_push(bbio, s->wbio);

s->rlayer.wrlmethod->set1_bio(s->rlayer.wrl, s->wbio);

return 1;
}

Expand All @@ -4819,6 +4823,8 @@ int ssl_free_wbio_buffer(SSL_CONNECTION *s)
return 1;

s->wbio = BIO_pop(s->wbio);
s->rlayer.wrlmethod->set1_bio(s->rlayer.wrl, s->wbio);

BIO_free(s->bbio);
s->bbio = NULL;

Expand Down

0 comments on commit b5cf81f

Please sign in to comment.