QUIC: Miscellaneous error handling updates

Raise errors when appropriate.

Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/21700)
This commit is contained in:
Tomas Mraz 2023-08-09 17:32:49 +02:00
parent cb19528b93
commit 96014840b6
4 changed files with 78 additions and 34 deletions

View File

@ -110,6 +110,7 @@ static int gen_rand_conn_id(OSSL_LIB_CTX *libctx, size_t len, QUIC_CONN_ID *cid)
cid->id_len = (unsigned char)len;
if (RAND_bytes_ex(libctx, cid->id, len, len * 8) != 1) {
ERR_raise(ERR_LIB_SSL, ERR_R_RAND_LIB);
cid->id_len = 0;
return 0;
}
@ -1264,8 +1265,11 @@ static int ch_on_transport_params(const unsigned char *params,
if (ch->got_remote_transport_params)
goto malformed;
if (!PACKET_buf_init(&pkt, params, params_len))
if (!PACKET_buf_init(&pkt, params, params_len)) {
ossl_quic_channel_raise_protocol_error(ch, QUIC_ERR_INTERNAL_ERROR, 0,
"internal error (packet buf init)");
return 0;
}
while (PACKET_remaining(&pkt) > 0) {
if (!ossl_quic_wire_peek_transport_param(&pkt, &id))
@ -1447,7 +1451,7 @@ static int ch_on_transport_params(const unsigned char *params,
if (got_max_ack_delay) {
/* must not appear more than once */
reason = TP_REASON_DUP("MAX_ACK_DELAY");
return 0;
goto malformed;
}
if (!ossl_quic_wire_decode_transport_param_int(&pkt, &id, &v)
@ -1467,7 +1471,7 @@ static int ch_on_transport_params(const unsigned char *params,
if (got_initial_max_streams_bidi) {
/* must not appear more than once */
reason = TP_REASON_DUP("INITIAL_MAX_STREAMS_BIDI");
return 0;
goto malformed;
}
if (!ossl_quic_wire_decode_transport_param_int(&pkt, &id, &v)
@ -2202,9 +2206,11 @@ static void ch_rx_handle_packet(QUIC_CHANNEL *ch)
/* Malformed retry packet, ignore. */
return;
ch_retry(ch, ch->qrx_pkt->hdr->data,
ch->qrx_pkt->hdr->len - QUIC_RETRY_INTEGRITY_TAG_LEN,
&ch->qrx_pkt->hdr->src_conn_id);
if (!ch_retry(ch, ch->qrx_pkt->hdr->data,
ch->qrx_pkt->hdr->len - QUIC_RETRY_INTEGRITY_TAG_LEN,
&ch->qrx_pkt->hdr->src_conn_id))
ossl_quic_channel_raise_protocol_error(ch, QUIC_ERR_INTERNAL_ERROR,
0, "handling retry packet");
break;
case QUIC_PKT_TYPE_0RTT:
@ -2449,7 +2455,7 @@ static int ch_tx(QUIC_CHANNEL *ch)
* be transmitted for this reason.
*/
ossl_quic_channel_raise_protocol_error(ch, QUIC_ERR_INTERNAL_ERROR, 0,
"internal error");
"internal error (txp generate)");
break; /* Internal failure (e.g. allocation, assertion) */
}
@ -2532,8 +2538,10 @@ static OSSL_TIME ch_determine_next_tick_deadline(QUIC_CHANNEL *ch)
/* Determines whether we can support a given poll descriptor. */
static int validate_poll_descriptor(const BIO_POLL_DESCRIPTOR *d)
{
if (d->type == BIO_POLL_DESCRIPTOR_TYPE_SOCK_FD && d->value.fd < 0)
if (d->type == BIO_POLL_DESCRIPTOR_TYPE_SOCK_FD && d->value.fd < 0) {
ERR_raise(ERR_LIB_SSL, ERR_R_PASSED_INVALID_ARGUMENT);
return 0;
}
return 1;
}
@ -2673,7 +2681,7 @@ static int ch_retry(QUIC_CHANNEL *ch,
* a SCID field that is identical to the DCID field of its initial packet."
*/
if (ossl_quic_conn_id_eq(&ch->init_dcid, retry_scid))
return 0;
return 1;
/* We change to using the SCID in the Retry packet as the DCID. */
if (!ossl_quic_tx_packetiser_set_cur_dcid(ch->txp, retry_scid))
@ -2964,14 +2972,14 @@ static void free_frame_data(unsigned char *buf, size_t buf_len, void *arg)
static int ch_enqueue_retire_conn_id(QUIC_CHANNEL *ch, uint64_t seq_num)
{
BUF_MEM *buf_mem;
BUF_MEM *buf_mem = NULL;
WPACKET wpkt;
size_t l;
chan_remove_reset_token(ch, seq_num);
if ((buf_mem = BUF_MEM_new()) == NULL)
return 0;
goto err;
if (!WPACKET_init(&wpkt, buf_mem))
goto err;

View File

@ -535,7 +535,7 @@ int ossl_quic_reset(SSL *s)
if (!expect_quic(s, &ctx))
return 0;
/* Not supported. */
ERR_raise(ERR_LIB_SSL, ERR_R_UNSUPPORTED);
return 0;
}
@ -1052,8 +1052,10 @@ int ossl_quic_get_rpoll_descriptor(SSL *s, BIO_POLL_DESCRIPTOR *desc)
if (!expect_quic(s, &ctx))
return 0;
if (desc == NULL || ctx.qc->net_rbio == NULL)
if (desc == NULL || ctx.qc->net_rbio == NULL) {
ERR_raise(ERR_LIB_SSL, ERR_R_PASSED_INVALID_ARGUMENT);
return 0;
}
return BIO_get_rpoll_descriptor(ctx.qc->net_rbio, desc);
}
@ -1066,8 +1068,10 @@ int ossl_quic_get_wpoll_descriptor(SSL *s, BIO_POLL_DESCRIPTOR *desc)
if (!expect_quic(s, &ctx))
return 0;
if (desc == NULL || ctx.qc->net_wbio == NULL)
if (desc == NULL || ctx.qc->net_wbio == NULL) {
ERR_raise(ERR_LIB_SSL, ERR_R_PASSED_INVALID_ARGUMENT);
return 0;
}
return BIO_get_wpoll_descriptor(ctx.qc->net_wbio, desc);
}
@ -1708,12 +1712,16 @@ static SSL *quic_conn_stream_new(QCTX *ctx, uint64_t flags, int need_lock)
}
qs = ossl_quic_channel_new_stream_local(qc->ch, is_uni);
if (qs == NULL)
if (qs == NULL) {
ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR);
goto err;
}
xso = create_xso_from_stream(qc, qs);
if (xso == NULL)
if (xso == NULL) {
ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR);
goto err;
}
qc_touch_default_xso(qc); /* inhibits default XSO */
if (need_lock)
@ -2719,6 +2727,7 @@ int ossl_quic_set_incoming_stream_policy(SSL *s, int policy,
break;
default:
ERR_raise(ERR_LIB_SSL, ERR_R_PASSED_INVALID_ARGUMENT);
ret = 0;
break;
}
@ -3018,9 +3027,11 @@ int ossl_quic_set_write_buffer_size(SSL *ssl, size_t size)
if (!expect_quic_with_stream_lock(ssl, /*remote_init=*/-1, &ctx))
return 0;
if (!ossl_quic_stream_has_send(ctx.xso->stream))
if (!ossl_quic_stream_has_send(ctx.xso->stream)) {
/* Called on a unidirectional receive-only stream - error. */
ERR_raise(ERR_LIB_SSL, ERR_R_SHOULD_NOT_HAVE_BEEN_CALLED);
goto out;
}
if (!ossl_quic_stream_has_send_buffer(ctx.xso->stream)) {
/*
@ -3031,8 +3042,10 @@ int ossl_quic_set_write_buffer_size(SSL *ssl, size_t size)
goto out;
}
if (!ossl_quic_sstream_set_buffer_size(ctx.xso->stream->sstream, size))
if (!ossl_quic_sstream_set_buffer_size(ctx.xso->stream->sstream, size)) {
ERR_raise(ERR_LIB_SSL, ERR_R_INTERNAL_ERROR);
goto out;
}
ret = 1;
@ -3088,7 +3101,7 @@ int ossl_quic_key_update(SSL *ssl, int update_type)
break;
default:
/* Unknown type - error. */
ERR_raise(ERR_LIB_SSL, ERR_R_PASSED_INVALID_ARGUMENT);
return 0;
}

View File

@ -301,8 +301,13 @@ static int depack_do_frame_crypto(PACKET *pkt, QUIC_CHANNEL *ch,
}
if (!ossl_quic_rstream_queue_data(rstream, parent_pkt,
f.offset, f.data, f.len, 0))
f.offset, f.data, f.len, 0)) {
ossl_quic_channel_raise_protocol_error(ch,
QUIC_ERR_INTERNAL_ERROR,
OSSL_QUIC_FRAME_TYPE_CRYPTO,
"internal error (rstream queue)");
return 0;
}
*datalen = f.len;
@ -594,8 +599,13 @@ static int depack_do_frame_stream(PACKET *pkt, QUIC_CHANNEL *ch,
frame_data.offset,
frame_data.data,
frame_data.len,
frame_data.is_fin))
frame_data.is_fin)) {
ossl_quic_channel_raise_protocol_error(ch,
QUIC_ERR_INTERNAL_ERROR,
frame_type,
"internal error (rstream queue)");
return 0;
}
/*
* rs_fin will be 1 only if we can read all data up to and including the FIN
@ -603,9 +613,14 @@ static int depack_do_frame_stream(PACKET *pkt, QUIC_CHANNEL *ch,
* calling ossl_quic_rstream_available() where it is not necessary as it is
* more expensive.
*/
if (stream->recv_state == QUIC_RSTREAM_STATE_SIZE_KNOWN)
if (!ossl_quic_rstream_available(stream->rstream, &rs_avail, &rs_fin))
return 0;
if (stream->recv_state == QUIC_RSTREAM_STATE_SIZE_KNOWN
&& !ossl_quic_rstream_available(stream->rstream, &rs_avail, &rs_fin)) {
ossl_quic_channel_raise_protocol_error(ch,
QUIC_ERR_INTERNAL_ERROR,
frame_type,
"internal error (rstream available)");
return 0;
}
if (rs_fin)
ossl_quic_stream_map_notify_totally_received(&ch->qsm, stream);
@ -982,12 +997,18 @@ static int depack_do_frame_path_response(PACKET *pkt,
return 1;
}
static int depack_do_frame_conn_close(PACKET *pkt, QUIC_CHANNEL *ch)
static int depack_do_frame_conn_close(PACKET *pkt, QUIC_CHANNEL *ch,
uint64_t frame_type)
{
OSSL_QUIC_FRAME_CONN_CLOSE frame_data;
if (!ossl_quic_wire_decode_frame_conn_close(pkt, &frame_data))
if (!ossl_quic_wire_decode_frame_conn_close(pkt, &frame_data)) {
ossl_quic_channel_raise_protocol_error(ch,
QUIC_ERR_FRAME_ENCODING_ERROR,
frame_type,
"decode error");
return 0;
}
ossl_quic_channel_on_remote_conn_close(ch, &frame_data);
return 1;
@ -997,8 +1018,14 @@ static int depack_do_frame_handshake_done(PACKET *pkt,
QUIC_CHANNEL *ch,
OSSL_ACKM_RX_PKT *ackm_data)
{
if (!ossl_quic_wire_decode_frame_handshake_done(pkt))
if (!ossl_quic_wire_decode_frame_handshake_done(pkt)) {
/* This can fail only with an internal error. */
ossl_quic_channel_raise_protocol_error(ch,
QUIC_ERR_INTERNAL_ERROR,
OSSL_QUIC_FRAME_TYPE_HANDSHAKE_DONE,
"internal error (decode frame handshake done)");
return 0;
}
ossl_quic_channel_on_handshake_confirmed(ch);
return 1;
@ -1321,7 +1348,7 @@ static int depack_process_frames(QUIC_CHANNEL *ch, PACKET *pkt,
/* FALLTHRU */
case OSSL_QUIC_FRAME_TYPE_CONN_CLOSE_TRANSPORT:
/* CONN_CLOSE_TRANSPORT frames are valid in all packets */
if (!depack_do_frame_conn_close(pkt, ch))
if (!depack_do_frame_conn_close(pkt, ch, frame_type))
return 0;
break;

View File

@ -833,7 +833,7 @@ int ossl_quic_tx_packetiser_generate(OSSL_QUIC_TX_PACKETISER *txp,
&& pkt[QUIC_ENC_LEVEL_HANDSHAKE].h.bytes_appended > 0);
/* Flush & Cleanup */
res = TX_PACKETISER_RES_NO_PKT;
res = pkts_done > 0 ? TX_PACKETISER_RES_SENT_PKT : TX_PACKETISER_RES_NO_PKT;
out:
ossl_qtx_finish_dgram(txp->args.qtx);
@ -842,11 +842,7 @@ out:
++enc_level)
txp_pkt_cleanup(&pkt[enc_level], txp);
/*
* If we already successfully did at least one, make sure we report this via
* the return code.
*/
return pkts_done > 0 ? TX_PACKETISER_RES_SENT_PKT : res;
return res;
}
static const struct archetype_data archetypes[QUIC_ENC_LEVEL_NUM][TX_PACKETISER_ARCHETYPE_NUM] = {