Commit graph

24 commits

Author SHA1 Message Date
Amaury Denoyelle
a7a4c80ade MINOR: qpack: properly handle invalid dynamic table references
Return QPACK_DECOMPRESSION_FAILED error code when dealing with dynamic
table references. This is justified as for now haproxy does not
implement dynamic table support and advertizes a zero-sized table.

The H3 calling function will thus reuse this code in a CONNECTION_CLOSE
frame, in conformance with the QPACK RFC.

This proper error management allows to remove obsolete ABORT_NOW guards.
2022-06-30 11:51:06 +02:00
Amaury Denoyelle
46e992d795 BUG/MINOR: qpack: abort on dynamic index field line decoding
This is a complement to partial fix from commit
  debaa04f9e
  BUG/MINOR: qpack: abort on dynamic index field line decoding

The main objective is to fix coverity report about usage of
uninitialized variable when receiving dynamic table references. These
references are invalid as for the moment haproxy advertizes a 0-sized
dynamic table. An ABORT_NOW clause is present to catch this. A following
patch will clean up this in order to properly handle QPACK errors with
CONNECTION_CLOSE.

This should fix github issue #1753.

No need to backport as this was introduced in the current dev branch.
2022-06-30 11:51:06 +02:00
Amaury Denoyelle
055de23b7d BUG/MINOR: qpack: fix build with QPACK_DEBUG
The local variable 't' was renamed 'static_tbl'. Fix its name in the
qpack_debug_printf() statement which is activated only with QPACK_DEBUG
mode.

No need to backport as this was introduced in current dev branch.
2022-06-30 10:15:58 +02:00
Frédéric Lécaille
628e89cfae BUILD: quic+h3: 32-bit compilation errors fixes
In GH #1760 (which is marked as being a feature), there were compilation
errors on MacOS which could be reproduced in Linux when building 32-bit code
(-m32 gcc option). Most of them were due to variables types mixing in QUIC_MIN macro
or using size_t type in place of uint64_t type.

Must be backported to 2.6.
2022-06-24 12:13:53 +02:00
Amaury Denoyelle
debaa04f9e BUG/MINOR: qpack: abort on dynamic index field line decoding
Add an ABORT_NOW() clause if indexed field line referred to the dynamic
table. This is required as current haproxy QPACK implementation does not
support the dynamic table.

Note that this should not happen as haproxy explicitely advertizes a
null-sized dynamic table to the other peer.

This shoud fix github issue #1753.

No need to backport as this was introduced by commit
  b666c6b26e
  MINOR: qpack: improve decoding function
2022-06-20 15:56:01 +02:00
Amaury Denoyelle
b666c6b26e MINOR: qpack: improve decoding function
Adjust decoding loop by using temporary ist for header name and value.
The header is inserted at the end of an iteration, which guarantee that
we do not insert name only in the list in case of an error on value
decoding. This also helps the function readability by centralizing the
LIST insert operation.

The return value of the decoding function is also changed. Now on
success the number of headers inserted in the input list is returned.
This change as no impact as success value is not used by the caller.
This is mainly done to have a behavior similar to hpack decoding
function.
2022-06-15 15:05:22 +02:00
Amaury Denoyelle
60ef19f137 BUG/MINOR: h3/qpack: deal with too many headers
ensures that we never insert too many entries in a headers input list.
On the decoding side, a new error QPACK_ERR_TOO_LARGE is reported in
this case.

This prevents crash if headers number on a H3 request or response is
superior to tune.http.maxhdr config value. Previously, a crash would
occur in QPACK decoding function.

Note that the process still crashes later with ABORT_NOW() because error
reporting on frame parsing is not implemented for now. It should be
treated with a RESET_STREAM frame in most cases.

This can be backported up to 2.6.
2022-06-15 15:05:08 +02:00
Amaury Denoyelle
28d3c2489f MINOR: qpack: add ABORT_NOW on unimplemented decoding
Post-base indices is not supported at the moment for decoding. This
should never be encountered as it is only used with a dynamic table.
However, haproxy deactivates support for the dynamic table via its
SETTINGS.

Use ABORT_NOW() if this situation happens anyway. This should help
debugging instead of silently failed without error reporting.
2022-06-15 14:56:05 +02:00
Amaury Denoyelle
4bcaf69dca BUG/MINOR: qpack: support header litteral name decoding
Complete QPACK decoding with full implementation of litteral field name
with litteral value representation. This change is mandatory to support
decoding of headers name not present in the QPACK static table.
Previously, these headers were silently ignored and not transferred on
the backend request.

QPACK decoding should now be sufficient to deal with all real
situations. Only post-base indices representation are not handled but
this should not cause a problem as they are only used for the dynamic
table whose size is null as announced by the haproxy implementation.

The direct impact of this change is that it should now be possible to
use complex webapp through a QUIC frontend.

This must be backported up to 2.6.
2022-06-15 14:54:51 +02:00
Amaury Denoyelle
53eef46b88 MINOR: qpack: reduce dependencies on other modules
Clean up QPACK decoder API by removing dependencies on ncbuf and
MUX-QUIC. This reduces includes statements. It will also help to
implement a standalone QPACK decoder.
2022-06-15 11:20:48 +02:00
Amaury Denoyelle
c5d31ed8be MINOR: qpack: add comments and remove a useless trace
Add comments on the decoding function to facilitate code analysis.

Also remove the qpack_debug_hexdump() which prints the whole left buffer
on each header parsing. With large HEADERS frame payload, QPACK traces
are complicated to debug with this statement.
2022-06-15 11:20:42 +02:00
Amaury Denoyelle
5869cb669e BUG/MINOR: qpack: do not consider empty enc/dec stream as error
When parsing QPACK encoder/decoder streams, h3_decode_qcs() displays an
error trace if they are empty. Change the return code used in QPACK code
to avoid this trace.

To uniformize with MUX/H3 code, 0 is now used to indicate success.

Beyond this spurious error trace, this bug has no impact.
2022-05-31 15:35:06 +02:00
Amaury Denoyelle
6b92394973 MINOR: h3/qpack: use qcs as type in decode callbacks
Replace h3_uqs type by qcs in stream callbacks. This change is done in
the context of unification between bidi and uni-streams. h3_uqs type
will be unneeded when this is achieved.
2022-05-25 15:41:25 +02:00
Amaury Denoyelle
3db98e9d13 MEDIUM: mux-quic/h3/qpack: use ncbuf for uni streams
This commit is the equivalent for uni-streams of previous commit
  MEDIUM: mux-quic/h3/hq-interop: use ncbuf for bidir streams

All unidirectional streams data is now handle in MUX Rx ncbuf. The
obsolete buffer is not unused and will be cleared in the following
patches.
2022-05-13 17:29:49 +02:00
Amaury Denoyelle
d96361b270 CLEANUP: qpack: suppress by default stdout traces
Remove the definition of DEBUG_HPACK on qpack-dec.c which forces the
QPACK decoding traces on stderr. Also change the name to use a dedicated
one for QPACK decoding as DEBUG_QPACK.
2022-03-25 15:22:40 +01:00
Amaury Denoyelle
18a10d07b6 BUILD: qpack: fix unused value when not using DEBUG_HPACK
If the macro is not defined, some local variables are flagged as unused
by the compiler. Fix this by using the __maybe_unused attribute.

For now, the macro is defined in the qpack-dec.c. However, this will
change to not mess up the stderr output of haproxy with QUIC traffic.
2022-03-25 15:21:45 +01:00
Willy Tarreau
3dfb7da04b CLEANUP: tree-wide: remove a few rare non-ASCII chars
As reported by Tim in issue #1428, our sources are clean, there are
just a few files with a few rare non-ASCII chars for the paragraph
symbol, a few typos, or in Fred's name. Given that Fred already uses
the non-accentuated form at other places like on the public list,
let's uniformize all this and make sure the code displays equally
everywhere.
2022-03-04 08:58:32 +01:00
Amaury Denoyelle
ab9cec7ce1 MINOR: qpack: fix typo in trace
hanme -> hname
2022-02-15 11:08:17 +01:00
Frédéric Lécaille
6842485a84 MINOR: quic: Possible overflow in qpack_get_varint()
This should fix CID 375051 in GH 1536 where a signed integer expression (1 << bit)
which could overflow was compared to a uint64_t.
2022-02-14 15:20:54 +01:00
Frédéric Lécaille
e629cfd96a MINOR: qpack: Missing check for truncated QPACK fields
Decrementing <len> variable without checking could make haproxy crash (on abort)
when printing a huge buffer (with negative length).
2021-12-17 08:38:43 +01:00
Amaury Denoyelle
7d3aea50b8 MINOR: qpack: support litteral field line with non-huff name
Support qpack header using a non-huffman encoded name in a litteral
field line with name reference.

This format is notably used by picoquic client and should improve
haproxy interop covering.
2021-11-25 11:41:29 +01:00
Amaury Denoyelle
9c8c4fa3a2 MINOR: qpack: fix memory leak on huffman decoding
Remove an unneeded strdup invocation during QPACK huffman decoding. A
temporary storage buffer is passed by the function and exists after
decoding so no need to duplicate memory here.
2021-10-08 15:45:57 +02:00
Amaury Denoyelle
fd7cdc3e70 MINOR: qpack: generate headers list on decoder
TMP -> non-free strdup
TMP -> currently only support indexed field line or literal field line
with name reference
2021-09-23 15:27:25 +02:00
Frédéric Lécaille
b4672fb6f0 MINOR: qpack: Add QPACK compression.
Implement QPACK used for HTTP header compression by h3.
2021-09-23 15:27:25 +02:00