From 32691e7c255a944224e0ac17d0f83f217be6c5d6 Mon Sep 17 00:00:00 2001 From: Amaury Denoyelle Date: Wed, 12 Feb 2025 17:49:09 +0100 Subject: [PATCH] MINOR: quic: support frame type as a varint QUIC frame type is encoded as a variable-length integer. Thus, 64-bit integer should be used for them. Currently, this was not the case as type was represented as a 1-byte char inside quic_frame structure. This does not cause any issue with QUIC from RFC9000, as all frame types fit in this range. Furthermore, a QUIC implementation is required to use the smallest size varint when encoding a frame type. However, the current code is unable to accept QUIC extension with bigger frame types. This is notably the case for quic-on-streams draft. Thus, this commit readjusts quic_frame architecture to be able to support higher frame type values. First, type field of quic_frame is changed to a 64-bits variable. Both encoding and decoding frame functions uses variable-length integer helpers to manipulate the frame type field. Secondly, the quic_frame builders/parsers infrastructure is still preserved. However, it could be impossible to define new large frame type as an index into quic_frame_builders / quic_frame_parsers arrays. Thus, wrapper functions are now provided to access the builders and parsers. Both qf_builder() and qf_parser() wrappers can then be extended to return custom builder/parser instances for larger frame type. Finally, unknown frame type detection also uses the new wrapper quic_frame_is_known(). As with builders/parsers, for large frame type, this function must be manually completed to support a new type value. --- include/haproxy/quic_frame-t.h | 8 ++++- include/haproxy/quic_frame.h | 2 +- src/quic_frame.c | 60 ++++++++++++++++++++++++++++++---- 3 files changed, 61 insertions(+), 9 deletions(-) diff --git a/include/haproxy/quic_frame-t.h b/include/haproxy/quic_frame-t.h index 3c211e5bd..71835ce37 100644 --- a/include/haproxy/quic_frame-t.h +++ b/include/haproxy/quic_frame-t.h @@ -82,6 +82,12 @@ enum quic_frame_type { QUIC_FT_MAX }; +/* Extra frame types larger than QUIC_FT_MAX should be declared here. + * For every new value, an associated builder and parser instances should be + * defined in quic_frame.c. Do not forget to complete the associated function + * quic_frame_type_is_known() and both qf_builder()/qf_parser(). + */ + #define QUIC_FT_PKT_TYPE_I_BITMASK (1 << QUIC_PACKET_TYPE_INITIAL) #define QUIC_FT_PKT_TYPE_0_BITMASK (1 << QUIC_PACKET_TYPE_0RTT) #define QUIC_FT_PKT_TYPE_H_BITMASK (1 << QUIC_PACKET_TYPE_HANDSHAKE) @@ -246,7 +252,7 @@ struct qf_connection_close_app { struct quic_frame { struct list list; /* List elem from parent elem (typically a Tx packet instance, a PKTNS or a MUX element). */ struct quic_tx_packet *pkt; /* Last Tx packet used to send the frame. */ - unsigned char type; /* QUIC frame type. */ + uint64_t type; /* QUIC frame type. */ union { struct qf_padding padding; struct qf_ack ack; diff --git a/include/haproxy/quic_frame.h b/include/haproxy/quic_frame.h index 4bed44d87..ab1d897b9 100644 --- a/include/haproxy/quic_frame.h +++ b/include/haproxy/quic_frame.h @@ -206,7 +206,7 @@ static inline struct quic_err quic_err_app(uint64_t code) * * Returns the allocated frame or NULL on failure. */ -static inline struct quic_frame *qc_frm_alloc(int type) +static inline struct quic_frame *qc_frm_alloc(uint64_t type) { struct quic_frame *frm = NULL; diff --git a/src/quic_frame.c b/src/quic_frame.c index 2f8fff77b..d7a081e1a 100644 --- a/src/quic_frame.c +++ b/src/quic_frame.c @@ -1099,6 +1099,53 @@ const struct quic_frame_parser quic_frame_parsers[] = { [QUIC_FT_HANDSHAKE_DONE] = { .func = quic_parse_handshake_done_frame, .flags = QUIC_FL_RX_PACKET_ACK_ELICITING, .mask = QUIC_FT_PKT_TYPE____1_BITMASK, }, }; +/* Extra frame types with their associated builder and parser instances. + * Complete quic_frame_type_is_known() and both qf_builder()/qf_parser() to use them. + * + * Definition example: + * + * const uint64_t QUIC_FT_MY_CUSTOM_FRM = 0x01020304; + * const struct quic_frame_builder qf_ft_qs_tp_builder = { + * .func = quic_build_my_custom_frm, + * .mask = 0, + * .flags = 0, + * }; + * const struct quic_frame_parser qf_ft_qs_tp_parser = { + * .func = quic_parse_my_custom_frm, + * .mask = 0, + * .flags = 0, + * }; + */ + +/* Returns true if frame is supported. */ +static inline int quic_frame_type_is_known(uint64_t type) +{ + /* Complete here for extra frame types greater than QUIC_FT_MAX. */ + return type < QUIC_FT_MAX; +} + +static const struct quic_frame_parser *qf_parser(uint64_t type) +{ + if (type < QUIC_FT_MAX) + return &quic_frame_parsers[type]; + + /* Complete here for extra frame types greater than QUIC_FT_MAX. */ + + ABORT_NOW(); + return NULL; +} + +const struct quic_frame_builder *qf_builder(uint64_t type) +{ + if (type < QUIC_FT_MAX) + return &quic_frame_builders[type]; + + /* Complete here for extra frame types greater than QUIC_FT_MAX. */ + + ABORT_NOW(); + return NULL; +} + /* Decode a QUIC frame at buffer position into frame. * Returns 1 if succeeded (enough data at buffer position to parse the frame), 0 if not. */ @@ -1115,8 +1162,8 @@ int qc_parse_frm(struct quic_frame *frm, struct quic_rx_packet *pkt, goto leave; } - frm->type = *(*pos)++; - if (frm->type >= QUIC_FT_MAX) { + quic_dec_int(&frm->type, pos, end); + if (!quic_frame_type_is_known(frm->type)) { /* RFC 9000 12.4. Frames and Frame Types * * An endpoint MUST treat the receipt of a frame of unknown type as a @@ -1127,7 +1174,7 @@ int qc_parse_frm(struct quic_frame *frm, struct quic_rx_packet *pkt, goto leave; } - parser = &quic_frame_parsers[frm->type]; + parser = qf_parser(frm->type); if (!(parser->mask & (1U << pkt->type))) { TRACE_DEVEL("unauthorized frame", QUIC_EV_CONN_PRSFRM, qc, frm); goto leave; @@ -1162,21 +1209,20 @@ int qc_build_frm(unsigned char **pos, const unsigned char *end, unsigned char *p = *pos; TRACE_ENTER(QUIC_EV_CONN_BFRM, qc); - builder = &quic_frame_builders[frm->type]; + builder = qf_builder(frm->type); if (!(builder->mask & (1U << pkt->type))) { /* XXX This it a bug to send an unauthorized frame with such a packet type XXX */ TRACE_ERROR("unauthorized frame", QUIC_EV_CONN_BFRM, qc, frm); BUG_ON(!(builder->mask & (1U << pkt->type))); } - if (end <= p) { + if (!quic_enc_int(&p, end, frm->type)) { TRACE_DEVEL("not enough room", QUIC_EV_CONN_BFRM, qc, frm); goto leave; } TRACE_PROTO("TX frm", QUIC_EV_CONN_BFRM, qc, frm); - *p++ = frm->type; - if (!quic_frame_builders[frm->type].func(&p, end, frm, qc)) { + if (!builder->func(&p, end, frm, qc)) { TRACE_ERROR("frame building error", QUIC_EV_CONN_BFRM, qc, frm); goto leave; }