BUG/MAJOR: mux-h1/mux-h2/htx: Fix HTTP tunnel management at the mux level

Tunnel management between the H1 and H2 multiplexers is a bit blurred. And
the HTX is not enough well defined on this point to make things clear. In
fact, Establishing a tunnel between an H2 client and an H1 server, or the
opposite is buggy because the both multiplexers don't handle the EOM block
the same way when a tunnel is established. In fact, the H2 multiplexer is
pretty strict and add an END_STREAM flag when an EOM block is found, while
the H1 multiplexer is more flexible.

The purpose of this patch is to make the EOM block usage pretty clear and to
fix the HTTP multiplexers to really handle HTTP tunnels in the right
way. Now, an EOM block is used to mark the end of an HTTP message,
semantically speaking. That means it may be followed by tunneled data. Thus,
CONNECT requests are now finished by an EOM block, just after the EOH block.

On the H1 multiplexer side, a tunnel is now only established on the response
path. So a CONNECT request remains in a DONE state waiting for the 2xx
response. On the H2 multiplexer side, a flag is used to know an HTTP tunnel
is requested, to not immediately add the END_STREAM flag on the EOM block.

All these changes are sensitives and not backportable because of recent
changes. The same problem exists on earlier versions and should be
addressed. But it will only be possible with a specific patchset.

This patch relies on the following ones :

  * MEDIUM: mux-h1: Properly handle tunnel establishments and aborts
  * MEDIUM: mux-h2: Close streams when processing data for an aborted tunnel
  * MEDIUM: mux-h2: Block client data on server side waiting tunnel establishment
  * MINOR: mux-h2: Add 2 flags to help to properly handle tunnel mode
  * MINOR: mux-h1: Split H1C_F_WAIT_OPPOSITE flag to separate input/output sides
  * MINOR: mux-h1/mux-fcgi: Don't set TUNNEL mode if payload length is unknown
This commit is contained in:
Christopher Faulet 2021-01-22 15:28:03 +01:00
parent dea2474991
commit 5be651d4d7
4 changed files with 98 additions and 110 deletions

View File

@ -43,16 +43,6 @@ static size_t h1_eval_htx_size(const struct ist p1, const struct ist p2, const s
return sz;
}
/* Switch the message to tunnel mode. On the request, it must only be called for
* a CONNECT method. On the response, this function must only be called on
* successful replies to CONNECT requests or on protocol switching.
*/
static void h1_set_tunnel_mode(struct h1m *h1m)
{
h1m->flags &= ~(H1_MF_XFER_LEN|H1_MF_CLEN|H1_MF_CHNK);
h1m->state = H1_MSG_TUNNEL;
}
/* Check the validity of the request version. If the version is valid, it
* returns 1. Otherwise, it returns 0.
*/
@ -146,8 +136,6 @@ static unsigned int h1m_htx_sl_flags(struct h1m *h1m)
else
flags |= HTX_SL_F_BODYLESS;
}
if (h1m->state == H1_MSG_TUNNEL)
flags |= HTX_SL_F_BODYLESS;
if (h1m->flags & H1_MF_CONN_UPG)
flags |= HTX_SL_F_CONN_UPG;
return flags;
@ -186,8 +174,8 @@ static int h1_postparse_req_hdrs(struct h1m *h1m, union h1_sl *h1sl, struct htx
h1m->flags |= H1_MF_XFER_LEN;
if (h1sl->rq.meth == HTTP_METH_CONNECT) {
/* Switch CONNECT requests to tunnel mode */
h1_set_tunnel_mode(h1m);
h1m->flags &= ~(H1_MF_CLEN|H1_MF_CHNK);
h1m->curr_len = h1m->body_len = 0;
}
used = htx_used_space(htx);
@ -281,9 +269,9 @@ static int h1_postparse_res_hdrs(struct h1m *h1m, union h1_sl *h1sl, struct htx
}
if (((h1m->flags & H1_MF_METH_CONNECT) && code >= 200 && code < 300) || code == 101) {
/* Switch successful replies to CONNECT requests and
* protocol switching to tunnel mode. */
h1_set_tunnel_mode(h1m);
h1m->flags &= ~(H1_MF_CLEN|H1_MF_CHNK);
h1m->flags |= H1_MF_XFER_LEN;
h1m->curr_len = h1m->body_len = 0;
}
else if ((h1m->flags & H1_MF_METH_HEAD) || (code >= 100 && code < 200) ||
(code == 204) || (code == 304)) {

View File

@ -499,11 +499,19 @@ int h2_make_htx_request(struct http_hdr *list, struct htx *htx, unsigned int *ms
}
/* now send the end of headers marker */
htx_add_endof(htx, HTX_BLK_EOH);
if (!htx_add_endof(htx, HTX_BLK_EOH))
goto fail;
/* Set bytes used in the HTX message for the headers now */
sl->hdrs_bytes = htx_used_space(htx) - used;
if (*msgf & H2_MSGF_BODY_TUNNEL) {
/* Add the EOM for tunnel requests (CONNECT) */
htx->flags |= HTX_FL_EOI; /* no more message data are expected */
if (!htx_add_endof(htx, HTX_BLK_EOM))
goto fail;
}
ret = 1;
return ret;
@ -704,11 +712,19 @@ int h2_make_htx_response(struct http_hdr *list, struct htx *htx, unsigned int *m
}
/* now send the end of headers marker */
htx_add_endof(htx, HTX_BLK_EOH);
if (!htx_add_endof(htx, HTX_BLK_EOH))
goto fail;
/* Set bytes used in the HTX message for the headers now */
sl->hdrs_bytes = htx_used_space(htx) - used;
if (*msgf & H2_MSGF_BODY_TUNNEL) {
/* Tunnel sucessfully established, add the EOM now, all data are part of the tunnel */
htx->flags |= HTX_FL_EOI; /* no more message data are expected */
if (!htx_add_endof(htx, HTX_BLK_EOM))
goto fail;
}
ret = 1;
return ret;

View File

@ -1270,78 +1270,36 @@ static void h1_emit_chunk_crlf(struct buffer *buf)
}
/*
* Switch the request to tunnel mode. This function must only be called for
* CONNECT requests. On the client side, if the response is not finished, the
* mux is mark as busy on input.
* Switch the stream to tunnel mode. This function must only be called on 2xx
* (successful) replies to CONNECT requests or on 101 (switching protocol).
*/
static void h1_set_req_tunnel_mode(struct h1s *h1s)
static void h1_set_tunnel_mode(struct h1s *h1s)
{
h1s->req.flags &= ~(H1_MF_XFER_LEN|H1_MF_CLEN|H1_MF_CHNK);
struct h1c *h1c = h1s->h1c;
h1s->req.state = H1_MSG_TUNNEL;
TRACE_STATE("switch H1 request in tunnel mode", H1_EV_TX_DATA|H1_EV_TX_HDRS, h1s->h1c->conn, h1s);
h1s->req.flags &= ~(H1_MF_XFER_LEN|H1_MF_CLEN|H1_MF_CHNK);
if (!(h1s->h1c->flags & H1C_F_IS_BACK)) {
h1s->flags &= ~H1S_F_PARSING_DONE;
if (h1s->res.state < H1_MSG_DONE) {
h1s->h1c->flags |= H1C_F_WAIT_OUTPUT;
TRACE_STATE("Disable read on h1c (wait_output)", H1_EV_RX_DATA|H1_EV_H1C_BLK, h1s->h1c->conn, h1s);
}
}
else if (h1s->h1c->flags & H1C_F_WAIT_OUTPUT) {
h1s->h1c->flags &= ~H1C_F_WAIT_OUTPUT;
tasklet_wakeup(h1s->h1c->wait_event.tasklet);
TRACE_STATE("Re-enable read on h1c", H1_EV_RX_DATA|H1_EV_H1C_BLK|H1_EV_H1C_WAKE, h1s->h1c->conn, h1s);
}
}
/*
* Switch the response to tunnel mode. This function must only be called on
* successful replies to CONNECT requests or on protocol switching. In this
* last case, this function takes care to switch the request to tunnel mode if
* possible. On the server side, if the request is not finished, the mux is mark
* as busy on input.
*/
static void h1_set_res_tunnel_mode(struct h1s *h1s)
{
h1s->res.flags &= ~(H1_MF_XFER_LEN|H1_MF_CLEN|H1_MF_CHNK);
h1s->res.state = H1_MSG_TUNNEL;
TRACE_STATE("switch H1 response in tunnel mode", H1_EV_TX_DATA|H1_EV_TX_HDRS, h1s->h1c->conn, h1s);
h1s->res.flags &= ~(H1_MF_XFER_LEN|H1_MF_CLEN|H1_MF_CHNK);
if (h1s->h1c->flags & H1C_F_IS_BACK) {
h1s->flags &= ~H1S_F_PARSING_DONE;
/* On protocol switching, switch the request to tunnel mode if it is in
* DONE state. Otherwise we will wait the end of the request to switch
* it in tunnel mode.
*/
if (h1s->req.state < H1_MSG_DONE) {
h1s->h1c->flags |= H1C_F_WAIT_OUTPUT;
TRACE_STATE("Disable read on h1c (wait_output)", H1_EV_RX_DATA|H1_EV_H1C_BLK, h1s->h1c->conn, h1s);
}
else {
h1s->req.flags &= ~(H1_MF_XFER_LEN|H1_MF_CLEN|H1_MF_CHNK);
h1s->req.state = H1_MSG_TUNNEL;
TRACE_STATE("switch H1 request in tunnel mode", H1_EV_TX_DATA|H1_EV_TX_HDRS, h1s->h1c->conn, h1s);
TRACE_STATE("switch H1 stream in tunnel mode", H1_EV_TX_DATA|H1_EV_TX_HDRS, h1c->conn, h1s);
if (h1c->flags & H1C_F_IS_BACK)
h1s->flags &= ~H1S_F_PARSING_DONE;
if (h1s->h1c->flags & H1C_F_WAIT_INPUT) {
h1s->h1c->flags &= ~H1C_F_WAIT_INPUT;
h1_wake_stream_for_send(h1s);
if (b_data(&h1s->h1c->obuf))
tasklet_wakeup(h1s->h1c->wait_event.tasklet);
TRACE_STATE("Re-enable send on h1c", H1_EV_TX_DATA|H1_EV_H1C_BLK|H1_EV_H1C_WAKE, h1s->h1c->conn, h1s);
}
}
if (h1c->flags & H1C_F_WAIT_OUTPUT) {
h1c->flags &= ~H1C_F_WAIT_OUTPUT;
if (b_data(&h1c->ibuf))
h1_wake_stream_for_recv(h1s);
tasklet_wakeup(h1c->wait_event.tasklet);
TRACE_STATE("Re-enable read on h1c", H1_EV_RX_DATA|H1_EV_H1C_BLK|H1_EV_H1C_WAKE, h1c->conn, h1s);
}
else {
h1s->req.flags &= ~(H1_MF_XFER_LEN|H1_MF_CLEN|H1_MF_CHNK);
h1s->req.state = H1_MSG_TUNNEL;
TRACE_STATE("switch H1 request in tunnel mode", H1_EV_TX_DATA|H1_EV_TX_HDRS, h1s->h1c->conn, h1s);
if (h1s->h1c->flags & H1C_F_WAIT_OUTPUT) {
h1s->h1c->flags &= ~H1C_F_WAIT_OUTPUT;
tasklet_wakeup(h1s->h1c->wait_event.tasklet);
TRACE_STATE("Re-enable read on h1c", H1_EV_RX_DATA|H1_EV_H1C_BLK|H1_EV_H1C_WAKE, h1s->h1c->conn, h1s);
}
if (h1c->flags & H1C_F_WAIT_INPUT) {
h1c->flags &= ~H1C_F_WAIT_INPUT;
h1_wake_stream_for_send(h1s);
if (b_data(&h1c->obuf))
tasklet_wakeup(h1c->wait_event.tasklet);
TRACE_STATE("Re-enable send on h1c", H1_EV_TX_DATA|H1_EV_H1C_BLK|H1_EV_H1C_WAKE, h1c->conn, h1s);
}
}
@ -1383,16 +1341,11 @@ static size_t h1_process_headers(struct h1s *h1s, struct h1m *h1m, struct htx *h
h1_capture_bad_message(h1s->h1c, h1s, h1m, buf);
}
if (!(h1m->flags & H1_MF_RESP)) {
if (!(h1m->flags & H1_MF_RESP))
h1s->meth = h1sl.rq.meth;
if (h1m->state == H1_MSG_TUNNEL)
h1_set_req_tunnel_mode(h1s);
}
else {
else
h1s->status = h1sl.st.status;
if (h1m->state == H1_MSG_TUNNEL)
h1_set_res_tunnel_mode(h1s);
}
h1_process_input_conn_mode(h1s, h1m, htx);
*ofs += ret;
@ -1511,6 +1464,9 @@ static size_t h1_process_input(struct h1c *h1c, struct buffer *buf, size_t count
if (h1s->flags & (H1S_F_PARSING_ERROR|H1S_F_NOT_IMPL_ERROR))
goto end;
if (h1c->flags & H1C_F_WAIT_OUTPUT)
goto end;
do {
size_t used = htx_used_space(htx);
@ -1565,8 +1521,9 @@ static size_t h1_process_input(struct h1c *h1c, struct buffer *buf, size_t count
H1_EV_RX_DATA|H1_EV_RX_EOI, h1c->conn, h1s, htx);
}
if (!(h1m->flags & H1_MF_RESP) && h1s->status == 101)
h1_set_req_tunnel_mode(h1s);
if ((h1m->flags & H1_MF_RESP) &&
((h1s->meth == HTTP_METH_CONNECT && h1s->status >= 200 && h1s->status < 300) || h1s->status == 101))
h1_set_tunnel_mode(h1s);
else {
if (h1s->req.state < H1_MSG_DONE || h1s->res.state < H1_MSG_DONE) {
/* Unfinished transaction: block this input side waiting the end of the output side */
@ -1596,7 +1553,8 @@ static size_t h1_process_input(struct h1c *h1c, struct buffer *buf, size_t count
}
count -= htx_used_space(htx) - used;
} while (!(h1s->flags & (H1S_F_PARSING_ERROR|H1S_F_NOT_IMPL_ERROR)));
} while (!(h1s->flags & (H1S_F_PARSING_ERROR|H1S_F_NOT_IMPL_ERROR)) && !(h1c->flags & H1C_F_WAIT_OUTPUT));
if (h1s->flags & (H1S_F_PARSING_ERROR|H1S_F_NOT_IMPL_ERROR)) {
TRACE_PROTO("parsing or not-implemented error", H1_EV_RX_DATA|H1_EV_H1S_ERR, h1c->conn, h1s);
@ -1660,10 +1618,11 @@ static size_t h1_process_input(struct h1c *h1c, struct buffer *buf, size_t count
h1s->cs->flags &= ~CS_FL_MAY_SPLICE;
}
/* Don't set EOI on the conn-stream for protocol upgrade requests, wait
* the response to do so or not depending on the status code.
/* Don't set EOI on the conn-stream for protocol upgrade or connect
* requests, wait the response to do so or not depending on the status
* code.
*/
if ((h1s->flags & H1S_F_PARSING_DONE) && !(h1m->flags & H1_MF_CONN_UPG))
if ((h1s->flags & H1S_F_PARSING_DONE) && (h1s->meth != HTTP_METH_CONNECT) && !(h1m->flags & H1_MF_CONN_UPG))
h1s->cs->flags |= CS_FL_EOI;
if (h1s_data_pending(h1s) && !htx_is_empty(htx))
@ -1966,11 +1925,13 @@ static size_t h1_process_output(struct h1c *h1c, struct buffer *buf, size_t coun
H1_EV_TX_DATA|H1_EV_TX_HDRS, h1c->conn, h1s);
if (!(h1m->flags & H1_MF_RESP) && h1s->meth == HTTP_METH_CONNECT) {
goto done;
/* Must have a EOM before tunnel data */
h1m->state = H1_MSG_DONE;
}
else if ((h1m->flags & H1_MF_RESP) &&
((h1s->meth == HTTP_METH_CONNECT && h1s->status >= 200 && h1s->status < 300) || h1s->status == 101)) {
goto done;
/* Must have a EOM before tunnel data */
h1m->state = H1_MSG_DONE;
}
else if ((h1m->flags & H1_MF_RESP) &&
h1s->status < 200 && (h1s->status == 100 || h1s->status >= 102)) {
@ -2088,10 +2049,11 @@ static size_t h1_process_output(struct h1c *h1c, struct buffer *buf, size_t coun
/* a successful reply to a CONNECT or a protocol switching is sent
* to the client. Switch the response to tunnel mode.
*/
h1_set_res_tunnel_mode(h1s);
h1_set_tunnel_mode(h1s);
TRACE_STATE("switch H1 response in tunnel mode", H1_EV_TX_DATA|H1_EV_TX_HDRS, h1c->conn, h1s);
}
else if (h1s->h1c->flags & H1C_F_WAIT_OUTPUT) {
if (h1s->h1c->flags & H1C_F_WAIT_OUTPUT) {
h1s->h1c->flags &= ~H1C_F_WAIT_OUTPUT;
h1c->conn->xprt->subscribe(h1c->conn, h1c->conn->xprt_ctx, SUB_RETRY_RECV, &h1c->wait_event);
TRACE_STATE("Re-enable read on h1c", H1_EV_TX_DATA|H1_EV_H1C_BLK|H1_EV_H1C_WAKE, h1c->conn, h1s);

View File

@ -4719,7 +4719,7 @@ next_frame:
if (!(msgf & H2_MSGF_RSP_1XX))
*flags |= H2_SF_HEADERS_RCVD;
if ((h2c->dff & H2_F_HEADERS_END_STREAM)) {
if (htx_get_tail_type(htx) != HTX_BLK_EOM && (h2c->dff & H2_F_HEADERS_END_STREAM)) {
/* Mark the end of message using EOM */
htx->flags |= HTX_FL_EOI; /* no more data are expected. Only EOM remains to add now */
if (!htx_add_endof(htx, HTX_BLK_EOM)) {
@ -4857,12 +4857,19 @@ try_again:
* transferred.
*/
if (h2c->dff & H2_F_DATA_END_STREAM) {
htx->flags |= HTX_FL_EOI; /* no more data are expected. Only EOM remains to add now */
if (!htx_add_endof(htx, HTX_BLK_EOM)) {
TRACE_STATE("h2s rxbuf is full, failed to add EOM", H2_EV_RX_FRAME|H2_EV_RX_DATA|H2_EV_H2S_BLK, h2c->conn, h2s);
h2c->flags |= H2_CF_DEM_SFULL;
goto fail;
if (!(h2s->flags & H2_SF_BODY_TUNNEL) && (h2c->dff & H2_F_DATA_END_STREAM)) {
/* no more data are expected for this message. This add the EOM
* flag but only on the response path or if no tunnel attempt
* was aborted. Otherwise (request path + tunnel abrted), the
* EOM was already reported.
*/
if ((h2c->flags & H2_CF_IS_BACK) || !(h2s->flags & H2_SF_TUNNEL_ABRT)) {
htx->flags |= HTX_FL_EOI; /* no more data are expected. Only EOM remains to add now */
if (!htx_add_endof(htx, HTX_BLK_EOM)) {
TRACE_STATE("h2s rxbuf is full, failed to add EOM", H2_EV_RX_FRAME|H2_EV_RX_DATA|H2_EV_H2S_BLK, h2c->conn, h2s);
h2c->flags |= H2_CF_DEM_SFULL;
goto fail;
}
}
}
@ -5056,7 +5063,14 @@ static size_t h2s_frt_make_resp_headers(struct h2s *h2s, struct htx *htx)
* FIXME: we should also set it when we know for sure that the
* content-length is zero as well as on 204/304
*/
if (blk_end && htx_get_blk_type(blk_end) == HTX_BLK_EOM && h2s->status >= 200)
if ((h2s->flags & H2_SF_BODY_TUNNEL) && h2s->status >= 200 && h2s->status < 300) {
/* Don't set EOM if a tunnel is successfully established
* (2xx responses to a connect). In this case, the EOM must be found
*/
if (!blk_end || htx_get_blk_type(blk_end) != HTX_BLK_EOM)
goto fail;
}
else if (blk_end && htx_get_blk_type(blk_end) == HTX_BLK_EOM && h2s->status >= 200)
es_now = 1;
if (!h2s->cs || h2s->cs->flags & CS_FL_SHW)
@ -5422,6 +5436,9 @@ static size_t h2s_bck_make_req_headers(struct h2s *h2s, struct htx *htx)
es_now = 1;
}
else {
/* For CONNECT requests, the EOM must be found and eaten without setting the ES */
if (!blk_end || htx_get_blk_type(blk_end) != HTX_BLK_EOM)
goto fail;
h2s->flags |= H2_SF_BODY_TUNNEL;
}
@ -5754,7 +5771,12 @@ static size_t h2s_make_data(struct h2s *h2s, struct buffer *buf, size_t count)
if (type == HTX_BLK_EOM) {
total++; // EOM counts as one byte
count--;
es_now = 1;
/* EOM+empty: we may need to add END_STREAM (except for tunneled
* message)
*/
if (!(h2s->flags & H2_SF_BODY_TUNNEL))
es_now = 1;
}
if (es_now)