Skip to content

Commit

Permalink
QUIC QSM/STREAM: Refactor to use RFC stream states
Browse files Browse the repository at this point in the history 
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from #21135)
  • Loading branch information
@hlandau @paulidale
hlandau authored and paulidale committed Jul 16, 2023
1 parent 64b1d2f commit 2f018d1
Show file tree
Hide file tree
Showing 9 changed files with 676 additions and 81 deletions.
355 changes: 325 additions & 30 deletions include/internal/quic_stream_map.h
View file

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions ssl/quic/quic_channel.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2817,6 +2817,7 @@ QUIC_STREAM *ossl_quic_channel_new_stream_local(QUIC_CHANNEL *ch, int is_uni)
if ((qs = ossl_quic_stream_map_alloc(&ch->qsm, stream_id, type)) == NULL)
return NULL;


/* Locally-initiated stream, so we always want a send buffer. */
if (!ch_init_new_stream(ch, qs, /*can_send=*/1, /*can_recv=*/!is_uni))
goto err;
Expand Down
31 changes: 19 additions & 12 deletions ssl/quic/quic_impl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -415,13 +415,15 @@ void ossl_quic_free(SSL *s)
--ctx.qc->num_xso;

/* If a stream's send part has not been finished, auto-reset it. */
if (ctx.xso->stream->sstream != NULL
if (( ctx.xso->stream->send_state == QUIC_SSTREAM_STATE_READY
|| ctx.xso->stream->send_state == QUIC_SSTREAM_STATE_SEND)
&& !ossl_quic_sstream_get_final_size(ctx.xso->stream->sstream, NULL))
ossl_quic_stream_map_reset_stream_send_part(ossl_quic_channel_get_qsm(ctx.qc->ch),
ctx.xso->stream, 0);

/* Do STOP_SENDING for the receive part, if applicable. */
if (ctx.xso->stream->rstream != NULL)
if ( ctx.xso->stream->recv_state == QUIC_RSTREAM_STATE_RECV
|| ctx.xso->stream->recv_state == QUIC_RSTREAM_STATE_SIZE_KNOWN)
ossl_quic_stream_map_stop_sending_recv_part(ossl_quic_channel_get_qsm(ctx.qc->ch),
ctx.xso->stream, 0);

Expand Down Expand Up @@ -1960,7 +1962,8 @@ int ossl_quic_write(SSL *s, const void *buf, size_t len, size_t *written)
goto out;
}

if (ctx.xso->stream == NULL || ctx.xso->stream->sstream == NULL) {
if (ctx.xso->stream == NULL
|| !ossl_quic_stream_has_send_buffer(ctx.xso->stream)) {
ret = QUIC_RAISE_NON_NORMAL_ERROR(&ctx, ERR_R_INTERNAL_ERROR, NULL);
goto out;
}
Expand Down Expand Up @@ -2004,7 +2007,7 @@ static int quic_read_actual(QCTX *ctx,
if (stream->recv_fin_retired)
return QUIC_RAISE_NORMAL_ERROR(ctx, SSL_ERROR_ZERO_RETURN);

if (stream->rstream == NULL)
if (!ossl_quic_stream_has_recv_buffer(stream))
return QUIC_RAISE_NON_NORMAL_ERROR(ctx, ERR_R_INTERNAL_ERROR, NULL);

if (peek) {
Expand Down Expand Up @@ -2198,7 +2201,8 @@ static size_t ossl_quic_pending_int(const SSL *s, int check_channel)
if (!expect_quic_with_stream_lock(s, /*remote_init=*/-1, &ctx))
return 0;

if (ctx.xso->stream == NULL || ctx.xso->stream->rstream == NULL)
if (ctx.xso->stream == NULL
|| !ossl_quic_stream_has_recv_buffer(ctx.xso->stream))
/* Cannot raise errors here because we are const, just fail. */
goto out;

Expand Down Expand Up @@ -2239,13 +2243,15 @@ int ossl_quic_conn_stream_conclude(SSL *s)

qs = ctx.xso->stream;

if (qs == NULL || qs->sstream == NULL) {
if (qs == NULL
|| !ossl_quic_channel_is_active(ctx.qc->ch)
|| !ossl_quic_stream_has_send(qs)
|| ossl_quic_stream_send_is_reset(qs)) {
quic_unlock(ctx.qc);
return 0;
}

if (!ossl_quic_channel_is_active(ctx.qc->ch)
|| ossl_quic_sstream_get_final_size(qs->sstream, NULL)) {
if (ossl_quic_sstream_get_final_size(qs->sstream, NULL)) {
quic_unlock(ctx.qc);
return 1;
}
Expand Down Expand Up @@ -2646,6 +2652,7 @@ int ossl_quic_stream_reset(SSL *ssl,
QUIC_STREAM_MAP *qsm;
QUIC_STREAM *qs;
uint64_t error_code;
int ok;

if (!expect_quic_with_stream_lock(ssl, /*remote_init=*/0, &ctx))
return 0;
Expand All @@ -2654,10 +2661,10 @@ int ossl_quic_stream_reset(SSL *ssl,
qs = ctx.xso->stream;
error_code = (args != NULL ? args->quic_error_code : 0);

ossl_quic_stream_map_reset_stream_send_part(qsm, qs, error_code);
ok = ossl_quic_stream_map_reset_stream_send_part(qsm, qs, error_code);

quic_unlock(ctx.qc);
return 1;
return ok;
}

/*
Expand Down Expand Up @@ -2693,7 +2700,7 @@ static void quic_classify_stream(QUIC_CONNECTION *qc,
/* Application has read a FIN. */
*state = SSL_STREAM_STATE_FINISHED;
} else if ((!is_write && qs->stop_sending)
|| (is_write && qs->reset_stream)) {
|| (is_write && ossl_quic_stream_send_is_reset(qs))) {
/*
* Stream has been reset locally. FIN takes precedence over this for the
* read case as the application need not care if the stream is reset
Expand All @@ -2703,7 +2710,7 @@ static void quic_classify_stream(QUIC_CONNECTION *qc,
*app_error_code = !is_write
? qs->stop_sending_aec
: qs->reset_stream_aec;
} else if ((!is_write && qs->peer_reset_stream)
} else if ((!is_write && ossl_quic_stream_recv_is_reset(qs))
|| (is_write && qs->peer_stop_sending)) {
/*
* Stream has been reset remotely. */
Expand Down
18 changes: 12 additions & 6 deletions ssl/quic/quic_rx_depack.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ static int depack_do_frame_reset_stream(PACKET *pkt,
if (stream == NULL)
return 1; /* old deleted stream, not a protocol violation, ignore */

if (stream->rstream == NULL) {
if (!ossl_quic_stream_has_recv(stream)) {
ossl_quic_channel_raise_protocol_error(ch,
QUIC_ERR_STREAM_STATE_ERROR,
OSSL_QUIC_FRAME_TYPE_RESET_STREAM,
Expand All @@ -166,8 +166,14 @@ static int depack_do_frame_reset_stream(PACKET *pkt,
return 0;
}

stream->peer_reset_stream = 1;
stream->peer_reset_stream_aec = frame_data.app_error_code;
/*
* Depending on the receive part state this is handled either as a reset
* transition or a no-op (e.g. if a reset has already been received before,
* or the application already retired a FIN). Best effort - there are no
* protoocol error conditions we need to check for here.
*/
ossl_quic_stream_map_notify_reset_recv_part(&ch->qsm, stream,
frame_data.app_error_code);

ossl_quic_stream_map_update_state(&ch->qsm, stream);
return 1;
Expand Down Expand Up @@ -199,7 +205,7 @@ static int depack_do_frame_stop_sending(PACKET *pkt,
if (stream == NULL)
return 1; /* old deleted stream, not a protocol violation, ignore */

if (stream->sstream == NULL) {
if (!ossl_quic_stream_has_send(stream)) {
ossl_quic_channel_raise_protocol_error(ch,
QUIC_ERR_STREAM_STATE_ERROR,
OSSL_QUIC_FRAME_TYPE_STOP_SENDING,
Expand Down Expand Up @@ -452,7 +458,7 @@ static int depack_do_frame_stream(PACKET *pkt, QUIC_CHANNEL *ch,
*/
return 1;

if (stream->rstream == NULL) {
if (!ossl_quic_stream_has_recv(stream)) {
ossl_quic_channel_raise_protocol_error(ch,
QUIC_ERR_STREAM_STATE_ERROR,
frame_type,
Expand Down Expand Up @@ -576,7 +582,7 @@ static int depack_do_frame_max_stream_data(PACKET *pkt,
if (stream == NULL)
return 1; /* old deleted stream, not a protocol violation, ignore */

if (stream->sstream == NULL) {
if (!ossl_quic_stream_has_send(stream)) {
ossl_quic_channel_raise_protocol_error(ch,
QUIC_ERR_STREAM_STATE_ERROR,
OSSL_QUIC_FRAME_TYPE_MAX_STREAM_DATA,
Expand Down

0 comments on commit 2f018d1

Please sign in to comment.