Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ulimited body parsing #1983

Merged
merged 4 commits into from
Oct 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 26 additions & 3 deletions fw/apm.c
Original file line number Diff line number Diff line change
Expand Up @@ -1241,11 +1241,26 @@ tfw_apm_hm_srv_rcount_update(TfwStr *uri_path, void *apmref)
atomic64_inc(&hmctl->rcount);
}

static inline u32
__tfw_apm_crc32_calc(TfwMsgIter *it, TfwStr *chunk , struct sk_buff *skb_head,
TfwStr *body)
{
u32 crc = 0;

TFW_BODY_ITER_WALK(it, chunk)
crc = crc32(crc, chunk->data, chunk->len);

return crc;
}

bool
tfw_apm_hm_srv_alive(int status, TfwStr *body, void *apmref)
tfw_apm_hm_srv_alive(int status, TfwStr *body, struct sk_buff *skb_head,
void *apmref)
{
TfwApmHM *hm = READ_ONCE(((TfwApmData *)apmref)->hmctl.hm);
u32 crc32 = 0;
TfwMsgIter it;
TfwStr chunk = {0};

BUG_ON(!hm);
if (hm->codes && !test_bit(HTTP_CODE_BIT_NUM(status), hm->codes)) {
Expand All @@ -1260,14 +1275,22 @@ tfw_apm_hm_srv_alive(int status, TfwStr *body, void *apmref)
return true;
}

if (unlikely(tfw_body_iter_init(&it, &chunk, body->data, body->skb,
skb_head)))
{
T_WARN_NL("Invalid body. Health monitor '%s': status '%d' \n",
hm->name, status);
return false;
}

/*
* Special case for 'auto' monitor: generate crc32
* from body of first response and store it into monitor.
*/
if (!hm->crc32 && hm->auto_crc) {
hm->crc32 = tfw_str_crc32_calc(body);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tfw_str_crc32_calc is now unused, we can remove it from our code and tests

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer don't remove this function. Time to time we use crc32 for TfwStr even for debug purposes and it useful to have such function.

hm->crc32 = __tfw_apm_crc32_calc(&it, &chunk, skb_head, body);
} else if (hm->crc32) {
crc32 = tfw_str_crc32_calc(body);
crc32 = __tfw_apm_crc32_calc(&it, &chunk, skb_head, body);
if (hm->crc32 != crc32)
goto crc_err;
}
Expand Down
3 changes: 2 additions & 1 deletion fw/apm.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ int tfw_apm_stats(void *apmref, TfwPrcntlStats *pstats);
int tfw_apm_stats_bh(void *apmref, TfwPrcntlStats *pstats);
int tfw_apm_pstats_verify(TfwPrcntlStats *pstats);
void tfw_apm_hm_srv_rcount_update(TfwStr *uri_path, void *apmref);
bool tfw_apm_hm_srv_alive(int status, TfwStr *body, void *apmref);
bool tfw_apm_hm_srv_alive(int status, TfwStr *body, struct sk_buff *skb_head,
void *apmref);
bool tfw_apm_hm_srv_limit(int status, void *apmref);
void tfw_apm_hm_enable_srv(TfwServer *srv, void *hmref);
void tfw_apm_hm_disable_srv(TfwServer *srv);
Expand Down
49 changes: 10 additions & 39 deletions fw/cache.c
Original file line number Diff line number Diff line change
Expand Up @@ -1382,62 +1382,33 @@ tfw_cache_h2_copy_body(unsigned int *acc_len, char **p, TdbVRec **trec,
TfwHttpResp *resp, size_t *tot_len)
{
long n;
int r;
TfwMsgIter it;
TfwStr chunk;
TfwStr chunk = {0};
TfwStr *body = &resp->body;

BUG_ON(TFW_STR_DUP(body));

if (unlikely(!body->len))
return 0;

it.skb_head = resp->msg.skb_head;
it.skb = body->skb;
it.frag = -1;

/* Move to the beginning of body. */
tfw_http_iter_set_at(&it, body->data);
chunk.data = body->data;

if (it.frag == -1) {
unsigned int size = skb_headlen(it.skb);

chunk.len = (char*)(it.skb->data + size) - chunk.data;
} else {
skb_frag_t *f = &skb_shinfo(it.skb)->frags[it.frag];
unsigned int size = skb_frag_size(f);

chunk.len = (char*)(skb_frag_address(f) + size) - chunk.data;
}
r = tfw_body_iter_init(&it, &chunk, body->data, body->skb,
resp->msg.skb_head);
if (unlikely(r))
return r;

do {
TFW_BODY_ITER_WALK(&it, &chunk)
{
if ((n = tfw_cache_strcpy(p, trec, &chunk, *tot_len)) < 0) {
T_ERR("Cache: cannot copy chunk of HTTP body\n");
return -ENOMEM;
}

*tot_len -= n;
*acc_len += n;
}

it.frag++;
if (it.frag >= skb_shinfo(it.skb)->nr_frags) {
it.skb = it.skb->next;
if (it.skb == it.skb_head)
return 0;

it.frag = -(!!skb_headlen(it.skb));
}

if (it.frag == -1) {
chunk.data = it.skb->data;
chunk.len = skb_headlen(it.skb);
} else {
skb_frag_t *f = &skb_shinfo(it.skb)->frags[it.frag];

chunk.data = skb_frag_address(f);
chunk.len = skb_frag_size(f);
}
} while (true);
WARN_ON(*acc_len != body->len);

return 0;
}
Expand Down
5 changes: 4 additions & 1 deletion fw/http.c
Original file line number Diff line number Diff line change
Expand Up @@ -1623,8 +1623,11 @@ tfw_http_hm_control(TfwHttpResp *resp)
return;

if (!tfw_srv_suspended(srv) ||
!tfw_apm_hm_srv_alive(resp->status, &resp->body, srv->apmref))
!tfw_apm_hm_srv_alive(resp->status, &resp->body, resp->msg.skb_head,
srv->apmref))
{
return;
}

tfw_srv_mark_alive(srv);
}
Expand Down
83 changes: 74 additions & 9 deletions fw/http.h
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,8 @@ typedef struct {
* @conn - connection which the message was received on;
* @destructor - called when a connection is destroyed;
* @crlf - pointer to CRLF between headers and body;
* @body - pointer to the body of a message;
* @body - contains start of the body of a message and length of
* whole body. Do not use as regular TfwStr;
*
* TfwStr members must be the last for efficient scanning.
*
Expand Down Expand Up @@ -411,8 +412,6 @@ struct tfw_http_msg_t {
TFW_HTTP_MSG_COMMON;
};

#define __MSG_STR_START(m) (&(m)->crlf)

#define TFW_HTTP_COND_IF_MSINCE 0x0001
#define TFW_HTTP_COND_ETAG_ANY 0x0002
#define TFW_HTTP_COND_ETAG_LIST 0x0004
Expand Down Expand Up @@ -495,9 +494,6 @@ struct tfw_http_req_t {
unsigned char method_override;
};

#define TFW_HTTP_REQ_STR_START(r) __MSG_STR_START(r)
#define TFW_HTTP_REQ_STR_END(r) ((&(r)->uri_path) + 1)

#define TFW_IDX_BITS 12
#define TFW_D_IDX_BITS 4

Expand Down Expand Up @@ -600,9 +596,6 @@ typedef struct {
#define TFW_HDR_MAP_SZ(cnt) (sizeof(TfwHttpHdrMap) \
+ sizeof(TfwHdrIndex) * (cnt))

#define TFW_HTTP_RESP_STR_START(r) __MSG_STR_START(r)
#define TFW_HTTP_RESP_STR_END(r) ((&(r)->body) + 1)

#define TFW_HTTP_RESP_CUT_BODY_SZ(r) \
(r)->stream ? \
(r)->body.len - (r)->cut.len : \
Expand Down Expand Up @@ -713,6 +706,78 @@ tfw_http_msg_header_table_size(void)
return TFW_HTTP_HDR_RAW - TFW_HTTP_HDR_REGULAR - 1;
}

/**
* Initialize body iterator. Should be used as helper for iterating over
* HTTP message body.
*
* @it - Generic message iterator that be used as body iterator.
* @chunk - Current body chunk to init.
* @body_start - Position in sk_buff @start to start itarating.
* @start - sk_buff to start with.
* @end - sk_buff where stop itarating. Usually skb_head of message.
*/
static inline int
tfw_body_iter_init(TfwMsgIter* it, TfwStr* chunk, char* body_start,
struct sk_buff* start, struct sk_buff* end)
{
int r;

it->skb_head = end;
it->skb = start;
it->frag = -1;

/* Set starting position. */
r = ss_skb_find_frag_by_offset(it->skb, body_start, &it->frag);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we expect to find start of the body in the first skb?

Copy link
Contributor Author

@const-t const-t Oct 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not first skb. tfw_body_iter_init() - Initializes iterator with data obtained during body parsing, we pass to this function pointer to start of the body and pointer to sk_buff that holds the body or first chunks of the body. We use these pointers as starting point. If skb start doesn't "contain" passed pointer body_start error will be handled below. ss_skb_find_frag_by_offset() finds index of the fragment that contains body_start pointer.

if (unlikely(r))
return r;

if (it->frag == -1) {
unsigned int size = skb_headlen(it->skb);

chunk->len = (char*)(it->skb->data + size) - body_start;
} else {
skb_frag_t *f = &skb_shinfo(it->skb)->frags[it->frag];
unsigned int size = skb_frag_size(f);

chunk->len = (char*)(skb_frag_address(f) + size) - body_start;
}

chunk->data = body_start;

return 0;
}

/**
* Move to next body @chunk for iterator @it.
*/
static inline void
tfw_body_iter_next(TfwMsgIter* it, TfwStr* chunk)
{
if (++it->frag >= skb_shinfo(it->skb)->nr_frags) {
it->skb = it->skb->next;
if (it->skb == it->skb_head) {
chunk->data = NULL;
chunk->len = 0;
return;
}

it->frag = -(!!skb_headlen(it->skb));
}

if (it->frag == -1) {
chunk->data = it->skb->data;
chunk->len = skb_headlen(it->skb);
} else {
skb_frag_t *f = &skb_shinfo(it->skb)->frags[it->frag];

chunk->data = skb_frag_address(f);
chunk->len = skb_frag_size(f);
}
}

#define TFW_BODY_ITER_WALK(it, c) \
for (; (c)->data; tfw_body_iter_next((it), (c)))

typedef void (*tfw_http_cache_cb_t)(TfwHttpMsg *);

/* External HTTP functions. */
Expand Down
3 changes: 2 additions & 1 deletion fw/http_parser.c
Original file line number Diff line number Diff line change
Expand Up @@ -1705,7 +1705,8 @@ __FSM_STATE(Resp_BodyUnlimStart) { \
/* fall through */ \
} \
__FSM_STATE(Resp_BodyUnlimRead) { \
__FSM_MOVE_nf(Resp_BodyUnlimRead, __data_remain(p), &msg->body); \
msg->body.len += __data_remain(p); \
__FSM_MOVE_nofixup_n(Resp_BodyUnlimRead, __data_remain(p)); \
}

#define TFW_HTTP_PARSE_BODY(response, ...) \
Expand Down