Re: req.cook_cnt() broken?
Hi Daniel, On Fri, Aug 25, 2017 at 12:47:41PM +0200, Daniel Schneller wrote: > On 24. Aug. 2017, at 01:50, Cyril Bonté wrote: > > > > You're right. currently, the code and the documentation don't say the same > > things. > > > > Can you try the attached patch ? > > > > -- > > Cyril Bonté > > > > Thanks for the patch! > > Tried against 1.8, 1.7.9, and 1.6.13 just now. Works as expected with all > three. :D > > Any chance of getting this fix backported to the 1.7 and ideally 1.6 branches? > > It would come in handy on a production system currently running 1.6 that I > cannot easily upgrade to 1.7. Don't worry, we *always* backport fixes as far as relevant (so possibly even 1.5 and 1.4). We know that haproxy is such a sensitive component which once deployed rarely experiences major upgrades, so what's most important is that what is deployed works. We're late on 1.6 fixes by the way, the bug chasing in 1.7 has made us uncertain about a few fixes for a while, causing us to wait before taking risks on 1.6. I think we'll soon be more confident in preparing 1.6.14, likely with the aforementionned fix :-) Cheers, Willy
Re: req.cook_cnt() broken?
On 24. Aug. 2017, at 01:50, Cyril Bonté wrote: > > You're right. currently, the code and the documentation don't say the same > things. > > Can you try the attached patch ? > > -- > Cyril Bonté > Thanks for the patch! Tried against 1.8, 1.7.9, and 1.6.13 just now. Works as expected with all three. :D Any chance of getting this fix backported to the 1.7 and ideally 1.6 branches? It would come in handy on a production system currently running 1.6 that I cannot easily upgrade to 1.7. Cheers, Daniel -- Daniel Schneller Principal Cloud Engineer CenterDevice GmbH | Hochstraße 11 | 42697 Solingen tel: +49 1754155711| Deutschland daniel.schnel...@centerdevice.de | www.centerdevice.de Geschäftsführung: Dr. Patrick Peschlow, Dr. Lukas Pustina, Michael Rosbach, Handelsregister-Nr.: HRB 18655, HR-Gericht: Bonn, USt-IdNr.: DE-815299431
Re: req.cook_cnt() broken?
Hi Daniel, Le 23/08/2017 à 11:50, Daniel Schneller a écrit : Kindly bumping this during the summer vacation time for potentially new recipients :) On 21. Aug. 2017, at 21:14, Daniel Schneller wrote: Hi! According to the documentation req.cook_cnt([]) : integer Returns an integer value representing the number of occurrences of the cookie in the request, or all cookies if is not specified. it should be possible to do something like this to reject a request if it contains more than cookies total. I do not know the cookie names in advance. I am trying to reject malicious requests with hundreds or thousands of cookies, trying to exhaust memory in my backend servers. Tomcat has a maximum number of cookies per request setting, but I’d like to reject these before they even get to the backends. I thought this would work (for n=2): frontend fe-test bind 0.0.0.0:8070 http-request deny deny_status 400 if { req.cook_cnt() gt 2 } http-request auth realm tomcat default_backend be-test However, it does not work. The count is always 0, hence the ACL always pass >> [...] When I change the ACL to include a cookie name, it works: http-request deny deny_status 400 if { req.cook_cnt("C1") gt 2 } [...] I tried to figure out what the code does, to see if I am doing something wrong and found this in proto_http.c: -- /* Iterate over all cookies present in a request to count how many occurrences * match the name in args and args->data.str.len. If is non-null, then * multiple cookies may be parsed on the same line. The returned sample is of * type UINT. Accepts exactly 1 argument of type string. */ static int smp_fetch_cookie_cnt(const struct arg *args, struct sample *smp, const char *kw, void *private) { struct http_txn *txn; struct hdr_idx *idx; struct hdr_ctx ctx; const struct http_msg *msg; const char *hdr_name; int hdr_name_len; int cnt; char *val_beg, *val_end; char *sol; if (!args || args->type != ARGT_STR) return 0; -- So without being very C-savvy, this appears to exit early when there is no parameter of type string passed in. I hope someone can shed some light on this. :) You're right. currently, the code and the documentation don't say the same things. Can you try the attached patch ? -- Cyril Bonté diff --git a/src/proto_http.c b/src/proto_http.c index 2b9b0d01e..0935f0145 100644 --- a/src/proto_http.c +++ b/src/proto_http.c @@ -11318,8 +11318,8 @@ extract_cookie_value(char *hdr, const char *hdr_end, * its value between val_beg and val_end. */ - if (att_end - att_beg == cookie_name_l && - memcmp(att_beg, cookie_name, cookie_name_l) == 0) { + if (cookie_name_l == 0 || (att_end - att_beg == cookie_name_l && + memcmp(att_beg, cookie_name, cookie_name_l) == 0)) { /* let's return this value and indicate where to go on from */ *value = val_beg; *value_l = val_end - val_beg; @@ -11617,7 +11617,7 @@ smp_fetch_cookie_cnt(const struct arg *args, struct sample *smp, const char *kw, char *val_beg, *val_end; char *sol; - if (!args || args->type != ARGT_STR) + if (!args || (args->type != ARGT_STR && args->type != ARGT_STOP)) return 0; CHECK_HTTP_MESSAGE_FIRST();
Re: req.cook_cnt() broken?
Kindly bumping this during the summer vacation time for potentially new recipients :) > On 21. Aug. 2017, at 21:14, Daniel Schneller > wrote: > > Hi! > > According to the documentation > > req.cook_cnt([]) : integer > Returns an integer value representing the number of occurrences of the cookie > in the request, or all cookies if is not specified. > > it should be possible to do something like this to reject a request if it > contains more than cookies total. I do not know the cookie names in > advance. I am trying to reject malicious requests with hundreds or thousands > of cookies, trying to exhaust memory in my backend servers. Tomcat has a > maximum number of cookies per request setting, but I’d like to reject these > before they even get to the backends. > > I thought this would work (for n=2): > > frontend fe-test > bind 0.0.0.0:8070 > http-request deny deny_status 400 if { req.cook_cnt() gt 2 } > http-request auth realm tomcat > default_backend be-test > > > However, it does not work. The count is always 0, hence the ACL always passes > and I get a 401 response from the next ACL in line. > > root@tomcat:~# curl -v -b 'C1=v1; C1=v2; C1=v3' tomcat:8070 > * Rebuilt URL to: tomcat:8070/ > * Hostname was NOT found in DNS cache > * Trying 127.0.1.1... > * Connected to tomcat (127.0.1.1) port 8070 (#0) >> GET / HTTP/1.1 >> User-Agent: curl/7.35.0 >> Host: tomcat:8070 >> Accept: */* >> Cookie: C1=v1; C1=v2; C1=v3 >> > * HTTP 1.0, assume close after body > < HTTP/1.0 401 Unauthorized > < Cache-Control: no-cache > < Connection: close > < Content-Type: text/html > < WWW-Authenticate: Basic realm="tomcat" > < > 401 Unauthorized > You need a valid user and password to access this content. > > * Closing connection 0 > > > When I change the ACL to include a cookie name, it works: > >http-request deny deny_status 400 if { req.cook_cnt("C1") gt 2 } > > root@tomcat:~# curl -v -b 'C1=v1; C1=v2; C1=v3' tomcat:8070 > * Rebuilt URL to: tomcat:8070/ > * Hostname was NOT found in DNS cache > * Trying 127.0.1.1... > * Connected to tomcat (127.0.1.1) port 8070 (#0) >> GET / HTTP/1.1 >> User-Agent: curl/7.35.0 >> Host: tomcat:8070 >> Accept: */* >> Cookie: C1=v1; C1=v2; C1=v3 >> > * HTTP 1.0, assume close after body > < HTTP/1.0 400 Bad request > < Cache-Control: no-cache > < Connection: close > < Content-Type: text/html > < > 400 Bad request > Your browser sent an invalid request. > > * Closing connection 0 > > > > I tried to figure out what the code does, to see if I am doing something > wrong and found this in proto_http.c: > > -- > /* Iterate over all cookies present in a request to count how many occurrences > * match the name in args and args->data.str.len. If is non-null, then > * multiple cookies may be parsed on the same line. The returned sample is of > * type UINT. Accepts exactly 1 argument of type string. > */ > static int > smp_fetch_cookie_cnt(const struct arg *args, struct sample *smp, const char > *kw, void *private) > { > struct http_txn *txn; > struct hdr_idx *idx; > struct hdr_ctx ctx; > const struct http_msg *msg; > const char *hdr_name; > int hdr_name_len; > int cnt; > char *val_beg, *val_end; > char *sol; > > if (!args || args->type != ARGT_STR) > return 0; > -- > > So without being very C-savvy, this appears to exit early when there is no > parameter of type string passed in. > > I hope someone can shed some light on this. :) > > Thanks in advance, > Daniel > > > -- > Daniel Schneller > Principal Cloud Engineer > > CenterDevice GmbH | Hochstraße 11 > | 42697 Solingen > tel: +49 1754155711| Deutschland > daniel.schnel...@centerdevice.de | www.centerdevice.de > > Geschäftsführung: Dr. Patrick Peschlow, Dr. Lukas Pustina, > Michael Rosbach, Handelsregister-Nr.: HRB 18655, > HR-Gericht: Bonn, USt-IdNr.: DE-815299431 > >
req.cook_cnt() broken?
Hi! According to the documentation req.cook_cnt([]) : integer Returns an integer value representing the number of occurrences of the cookie in the request, or all cookies if is not specified. it should be possible to do something like this to reject a request if it contains more than cookies total. I do not know the cookie names in advance. I am trying to reject malicious requests with hundreds or thousands of cookies, trying to exhaust memory in my backend servers. Tomcat has a maximum number of cookies per request setting, but I’d like to reject these before they even get to the backends. I thought this would work (for n=2): frontend fe-test bind 0.0.0.0:8070 http-request deny deny_status 400 if { req.cook_cnt() gt 2 } http-request auth realm tomcat default_backend be-test However, it does not work. The count is always 0, hence the ACL always passes and I get a 401 response from the next ACL in line. root@tomcat:~# curl -v -b 'C1=v1; C1=v2; C1=v3' tomcat:8070 * Rebuilt URL to: tomcat:8070/ * Hostname was NOT found in DNS cache * Trying 127.0.1.1... * Connected to tomcat (127.0.1.1) port 8070 (#0) > GET / HTTP/1.1 > User-Agent: curl/7.35.0 > Host: tomcat:8070 > Accept: */* > Cookie: C1=v1; C1=v2; C1=v3 > * HTTP 1.0, assume close after body < HTTP/1.0 401 Unauthorized < Cache-Control: no-cache < Connection: close < Content-Type: text/html < WWW-Authenticate: Basic realm="tomcat" < 401 Unauthorized You need a valid user and password to access this content. * Closing connection 0 When I change the ACL to include a cookie name, it works: http-request deny deny_status 400 if { req.cook_cnt("C1") gt 2 } root@tomcat:~# curl -v -b 'C1=v1; C1=v2; C1=v3' tomcat:8070 * Rebuilt URL to: tomcat:8070/ * Hostname was NOT found in DNS cache * Trying 127.0.1.1... * Connected to tomcat (127.0.1.1) port 8070 (#0) > GET / HTTP/1.1 > User-Agent: curl/7.35.0 > Host: tomcat:8070 > Accept: */* > Cookie: C1=v1; C1=v2; C1=v3 > * HTTP 1.0, assume close after body < HTTP/1.0 400 Bad request < Cache-Control: no-cache < Connection: close < Content-Type: text/html < 400 Bad request Your browser sent an invalid request. * Closing connection 0 I tried to figure out what the code does, to see if I am doing something wrong and found this in proto_http.c: -- /* Iterate over all cookies present in a request to count how many occurrences * match the name in args and args->data.str.len. If is non-null, then * multiple cookies may be parsed on the same line. The returned sample is of * type UINT. Accepts exactly 1 argument of type string. */ static int smp_fetch_cookie_cnt(const struct arg *args, struct sample *smp, const char *kw, void *private) { struct http_txn *txn; struct hdr_idx *idx; struct hdr_ctx ctx; const struct http_msg *msg; const char *hdr_name; int hdr_name_len; int cnt; char *val_beg, *val_end; char *sol; if (!args || args->type != ARGT_STR) return 0; -- So without being very C-savvy, this appears to exit early when there is no parameter of type string passed in. I hope someone can shed some light on this. :) Thanks in advance, Daniel -- Daniel Schneller Principal Cloud Engineer CenterDevice GmbH | Hochstraße 11 | 42697 Solingen tel: +49 1754155711| Deutschland daniel.schnel...@centerdevice.de | www.centerdevice.de Geschäftsführung: Dr. Patrick Peschlow, Dr. Lukas Pustina, Michael Rosbach, Handelsregister-Nr.: HRB 18655, HR-Gericht: Bonn, USt-IdNr.: DE-815299431