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

Don't cache and don't forward hop-by-hop headers #650

Merged
merged 24 commits into from
Dec 29, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
69f989e
tls: update license indentifier
vankoven Oct 25, 2016
52b8531
HttpMsg: fix signed-unsigned conversions
vankoven Oct 25, 2016
44fce2c
HttpMsg: remove multiple checks for header's singularity
vankoven Nov 23, 2016
a324f43
Http Parser: parse keep-alive as special headers for both req and resp
vankoven Nov 23, 2016
9af66cb
remove and don't cache headers marked as hop-by hop
vankoven Nov 23, 2016
983a970
http parser: parse connection header and mark listed headers as hop-b…
vankoven Nov 24, 2016
66090da
unit tests: http parser: add tests for requests with hop-by-hop headers
vankoven Nov 24, 2016
4f4d636
unit tests: http parser: add tests for responses with hop-by-hop headers
vankoven Nov 24, 2016
e8c64b3
http msg: fix missed transfer-encoding header in __http_msg_hdr_val
vankoven Dec 3, 2016
be64e79
unit tests: update tests for http parser
vankoven Dec 3, 2016
1611092
Fix code style according to code review
vankoven Dec 3, 2016
69fffd8
http parser: fix wrong usage of C4_INT_LCM macro
vankoven Dec 3, 2016
298b35a
limit count of raw hop-by-hop headers to TFW_HTTP_HDR_NUM
vankoven Dec 3, 2016
e057b1a
fix code according code review
vankoven Dec 3, 2016
8585f26
http parser: don't allow raw end-to-end headers be marked as hop-by-hop
vankoven Dec 4, 2016
5db01cf
unit tests: update unit tests for hop-by hop header
vankoven Dec 4, 2016
f0df55d
http parser: optimize matching of raw hop-by-hop headers
vankoven Dec 4, 2016
ab7b431
unit tests: update tests for http parser and hop-by-hop headers
vankoven Dec 4, 2016
bd357b4
fix code according to code review
vankoven Dec 13, 2016
386f212
fix code according code review
vankoven Dec 13, 2016
dda3d56
http parser: fix too generic names of parser states and set rigth states
vankoven Dec 16, 2016
8b5f00e
fix code according code review
vankoven Dec 28, 2016
9f26a4b
http parser: block messages that lists end-to-end spec headers in Con…
vankoven Dec 28, 2016
fbf0082
http parser: get rid of macro used only once
vankoven Dec 29, 2016
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
40 changes: 15 additions & 25 deletions tempesta_fw/cache.c
Original file line number Diff line number Diff line change
Expand Up @@ -131,26 +131,6 @@ enum {
TFW_CACHE_REPLICA,
};

/*
* Non-cacheable hop-by-hop response headers in terms of RFC 2068.
* The table is used if server doesn't specify Cache-Control no-cache
* directive (RFC 7234 5.2.2.2) explicitly.
*
* Server header isn't defined as hop-by-hop by the RFC, but we don't show
* protected server to world.
*
* We don't store the headers in cache and create then from scratch.
* Adding a header is faster then modify it, so this speeds up headers
* adjusting as well as saves cache storage.
*
* TODO process Cache-Control no-cache
*/
static const int hbh_hdrs[] = {
[0 ... TFW_HTTP_HDR_RAW] = 0,
[TFW_HTTP_HDR_SERVER] = 1,
[TFW_HTTP_HDR_CONNECTION] = 1,
};

typedef struct {
int cpu[NR_CPUS];
atomic_t cpu_idx;
Expand Down Expand Up @@ -764,9 +744,15 @@ tfw_cache_copy_resp(TfwCacheEntry *ce, TfwHttpResp *resp, TfwHttpReq *req,
ce->hdr_len = 0;
ce->hdr_num = resp->h_tbl->off;
FOR_EACH_HDR_FIELD(field, end1, resp) {
n = field - resp->h_tbl->tbl;
/* Skip hop-by-hop headers. */
h = (n < TFW_HTTP_HDR_RAW && hbh_hdrs[n]) ? &empty : field;
if (!(field->flags & TFW_STR_HBH_HDR)) {
h = field;
} else if (field - resp->h_tbl->tbl < TFW_HTTP_HDR_RAW) {
h = &empty;
} else {
--ce->hdr_num;
continue;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that n is not used as an array index, perhaps it's best to postpone its calculation until the else if case.

if () {
} else if (field - resp->h_tbl->tbl < TFW_HTTP_HDR_RAW) {
} else {
}

n = tfw_cache_copy_hdr(&p, &trec, h, &tot_len);
if (n < 0) {
TFW_ERR("Cache: cannot copy HTTP header\n");
Expand Down Expand Up @@ -809,7 +795,6 @@ tfw_cache_copy_resp(TfwCacheEntry *ce, TfwHttpResp *resp, TfwHttpReq *req,
static size_t
__cache_entry_size(TfwHttpResp *resp, TfwHttpReq *req)
{
long n;
size_t size = CE_BODY_SIZE;
TfwStr *h, *hdr, *hdr_end, *dup, *dup_end, empty = {};

Expand All @@ -820,8 +805,13 @@ __cache_entry_size(TfwHttpResp *resp, TfwHttpReq *req)
/* Add all the headers size */
FOR_EACH_HDR_FIELD(hdr, hdr_end, resp) {
/* Skip hop-by-hop headers. */
n = hdr - resp->h_tbl->tbl;
h = (n < TFW_HTTP_HDR_RAW && hbh_hdrs[n]) ? &empty : hdr;
if (!(hdr->flags & TFW_STR_HBH_HDR))
h = hdr;
else if (hdr - resp->h_tbl->tbl < TFW_HTTP_HDR_RAW)
h = &empty;
else
continue;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please see the comment above regarding the use of n variable. In this case, n is not used anywhere else.


if (!TFW_STR_DUP(h)) {
size += sizeof(TfwCStr);
size += h->len ? (h->len + SLEN(S_CRLF)) : 0;
Expand Down
15 changes: 12 additions & 3 deletions tempesta_fw/http.c
Original file line number Diff line number Diff line change
Expand Up @@ -601,7 +601,8 @@ static int
tfw_http_set_hdr_connection(TfwHttpMsg *hm, int conn_flg)
{
if (((hm->flags & __TFW_HTTP_CONN_MASK) == conn_flg)
&& (!TFW_STR_EMPTY(&hm->h_tbl->tbl[TFW_HTTP_HDR_CONNECTION])))
&& (!TFW_STR_EMPTY(&hm->h_tbl->tbl[TFW_HTTP_HDR_CONNECTION]))
&& !(hm->flags & TFW_HTTP_CONN_EXTRA))
return 0;

switch (conn_flg) {
Expand All @@ -617,7 +618,7 @@ tfw_http_set_hdr_connection(TfwHttpMsg *hm, int conn_flg)
}
}

/*
/**
* Add/Replace/Remove Keep-Alive header field to/from HTTP message.
*/
static int
Expand All @@ -630,7 +631,7 @@ tfw_http_set_hdr_keep_alive(TfwHttpMsg *hm, int conn_flg)

switch (conn_flg) {
case TFW_HTTP_CONN_CLOSE:
r = TFW_HTTP_MSG_HDR_DEL(hm, "Keep-Alive", TFW_HTTP_HDR_RAW);
r = TFW_HTTP_MSG_HDR_DEL(hm, "Keep-Alive", TFW_HTTP_HDR_KEEP_ALIVE);
if (unlikely(r && r != -ENOENT)) {
TFW_WARN("Cannot delete Keep-Alive header (%d)\n", r);
return r;
Expand Down Expand Up @@ -736,6 +737,10 @@ tfw_http_adjust_req(TfwHttpReq *req)
if (r)
return r;

r = tfw_http_msg_del_hbh_hdrs(hm);
if (r < 0)
return r;

return tfw_http_set_hdr_connection(hm, TFW_HTTP_CONN_KA);
}

Expand All @@ -754,6 +759,10 @@ tfw_http_adjust_resp(TfwHttpResp *resp, TfwHttpReq *req)
if (r < 0)
return r;

r = tfw_http_msg_del_hbh_hdrs(hm);
if (r < 0)
return r;

r = tfw_http_set_hdr_keep_alive(hm, conn_flg);
if (r < 0)
return r;
Expand Down
111 changes: 73 additions & 38 deletions tempesta_fw/http.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,47 +136,16 @@ typedef struct {
time_t expires;
} TfwCacheControl;

/**
* We use goto/switch-driven automaton, so compiler typically generates binary
* search code over jump labels, so it gives log(N) lookup complexity where
* N is number of states. However, DFA for full HTTP processing can be quite
* large and log(N) becomes expensive and hard to code.
*
* So we use states space splitting to avoid states explosion.
* @_i_st is used to save current state and go to interior sub-automaton
* (e.g. process OWS using @state while current state is saved in @_i_st
* or using @_i_st parse value of a header described.
*
* @to_go - remaining number of bytes to process in the data chunk;
* (limited by single packet size and never exceeds 64KB)
* @state - current parser state;
* @_i_st - helping (interior) state;
* @to_read - remaining number of bytes to read;
* @_acc - integer accumulator for parsing chunked integers;
* @_date - accumulator for a date in date related headers;
* @_hdr_tag - stores header id which must be closed on generic EoL handling
* (see RGEN_EOL());
* @_tmp_chunk - currently parsed (sub)string, possibly chunked;
* @hdr - currently parsed header.
*/
typedef struct {
unsigned short to_go;
int state;
int _i_st;
int to_read;
unsigned long _acc;
time_t _date;
unsigned int _hdr_tag;
TfwStr _tmp_chunk;
TfwStr hdr;
} TfwHttpParser;

/**
* Http headers table.
*
* Singular headers (in terms of RFC 7230 3.2.2) go first to protect header
* repetition attacks. See __hdr_is_singular() and don't forget to
* update the static headers array when add a new singular header here.
* If the new header is hop-by-hop (must not be forwarded and cached by Tempesta)
* it must be listed in __hbh_parser_init_req()/__hbh_parser_init_resp() for
* unconditionally hop-by-hop header or in __parse_connection() otherwize.
* If the header is end-to-end it must be listed in __hbh_parser_add_data().
*
* Note: don't forget to update __http_msg_hdr_val() upon adding a new header.
*
Expand All @@ -198,6 +167,7 @@ typedef enum {

TFW_HTTP_HDR_CONNECTION = TFW_HTTP_HDR_NONSINGULAR,
TFW_HTTP_HDR_X_FORWARDED_FOR,
TFW_HTTP_HDR_KEEP_ALIVE,
TFW_HTTP_HDR_TRANSFER_ENCODING,

/* Start of list of generic (raw) headers. */
Expand All @@ -217,12 +187,76 @@ typedef struct {
+ sizeof(TfwStr) * (s))
#define TFW_HHTBL_SZ(o) TFW_HHTBL_EXACTSZ(__HHTBL_SZ(o))

/** Maximum of hop-by-hop tokens listed in Connection header. */
#define TFW_HBH_TOKENS_MAX 16

/**
* Non-cacheable hop-by-hop headers in terms of RFC 7230.
*
* We don't store the headers in cache and create them from scratch if needed.
* Adding a header is faster then modify it, so this speeds up headers
* adjusting as well as saves cache storage.
*
* Headers unconditionaly treated as hop-by-hop must be listed in
* __hbh_parser_init_req()/__hbh_parser_init_resp() functions and must be
* members of Special headers.
* group.
*
* @spec - bit array for special headers. Hop-by-hop special header is
* stored as (0x1 << tfw_http_hdr_t[hid]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps a corresponding proper BUILD_BUG_ON() is justified to ensure that the type used for the structure member can accommodate the necessary number of headers (has the required number of bits).

* @raw - table of raw headers names, parsed form connection field;
* @off - offset of last added raw header name;
*/
typedef struct {
unsigned int spec;
unsigned int off;
TfwStr raw[TFW_HBH_TOKENS_MAX];
} TfwHttpHbhHdrs;

/**
* We use goto/switch-driven automaton, so compiler typically generates binary
* search code over jump labels, so it gives log(N) lookup complexity where
* N is number of states. However, DFA for full HTTP processing can be quite
* large and log(N) becomes expensive and hard to code.
*
* So we use states space splitting to avoid states explosion.
* @_i_st is used to save current state and go to interior sub-automaton
* (e.g. process OWS using @state while current state is saved in @_i_st
* or using @_i_st parse value of a header described.
*
* @to_go - remaining number of bytes to process in the data chunk;
* (limited by single packet size and never exceeds 64KB)
* @state - current parser state;
* @_i_st - helping (interior) state;
* @to_read - remaining number of bytes to read;
* @_hdr_tag - stores header id which must be closed on generic EoL handling
* (see RGEN_EOL());
* @_acc - integer accumulator for parsing chunked integers;
* @_tmp_chunk - currently parsed (sub)string, possibly chunked;
* @hdr - currently parsed header.
* @hbh_parser - list of special and raw headers names to be treated as
* hop-by-hop
*/
typedef struct {
unsigned short to_go;
int state;
int _i_st;
int to_read;
unsigned long _acc;
time_t _date;
unsigned int _hdr_tag;
TfwStr _tmp_chunk;
TfwStr hdr;
TfwHttpHbhHdrs hbh_parser;
} TfwHttpParser;

/* Common flags for requests and responses. */
#define TFW_HTTP_CONN_CLOSE 0x000001
#define TFW_HTTP_CONN_KA 0x000002
#define __TFW_HTTP_CONN_MASK (TFW_HTTP_CONN_CLOSE | TFW_HTTP_CONN_KA)
#define TFW_HTTP_CHUNKED 0x000004
#define TFW_HTTP_MSG_SENT 0x000008
#define TFW_HTTP_CONN_EXTRA 0x000004
#define TFW_HTTP_CHUNKED 0x000008
#define TFW_HTTP_MSG_SENT 0x000010

/* Request flags */
#define TFW_HTTP_HAS_STICKY 0x000100
Expand Down Expand Up @@ -263,6 +297,7 @@ typedef struct {
* @content_length - the value of Content-Length header field;
* @conn - connection which the message was received on;
* @jtstamp - time the message has been received, in jiffies;
* @keep_alive - the value of timeout specified in Keep-Alive header;
* @crlf - pointer to CRLF between headers and body;
* @body - pointer to the body of a message;
*
Expand All @@ -278,6 +313,7 @@ typedef struct {
unsigned int flags; \
unsigned long content_length; \
unsigned long jtstamp; \
unsigned int keep_alive; \
TfwConnection *conn; \
void (*destructor)(void *msg); \
TfwStr crlf; \
Expand Down Expand Up @@ -340,7 +376,6 @@ typedef struct {
TFW_HTTP_MSG_COMMON;
TfwStr s_line;
unsigned short status;
unsigned int keep_alive;
time_t date;
} TfwHttpResp;

Expand Down
Loading