[PATCH] DOC: Improve documentation of the various hdr() fetches

2021-01-23 Thread Tim Duesterhus
GitHub issue #796 notes that many administrators miss the fact that the `hdr()`
fetch (without the `f`) splits the header value at commas. This is only
mentioned at the end of a long paragraph.

This patch attempts to improve the documentation by:
- Explaning the "comma issue" as early as possible.
- Adding newlines to split the explanation into distinct sections.
- Reducing duplication by making the `res` siblings refer to their `req`
  counterparts.

This patch may be backported as long as it applies cleanly. During the
refactoring I needed to adjust several explanations for consistency and not all
of them might be available in older branches.
---
 doc/configuration.txt | 210 +++---
 1 file changed, 116 insertions(+), 94 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 899bdf553..4dd105d44 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -18509,33 +18509,44 @@ hdr([[,]]) : string
   unambiguously apply to the request headers.
 
 req.fhdr([,]) : string
-  This extracts the last occurrence of header  in an HTTP request. When
-  used from an ACL, all occurrences are iterated over until a match is found.
+  This returns the full value of the last occurrence of header  in an
+  HTTP request. It differs from req.hdr() in that any commas present in the
+  value are returned and are not used as delimiters. This is sometimes useful
+  with headers such as User-Agent.
+
+  When used from an ACL, all occurrences are iterated over until a match is
+  found.
+
   Optionally, a specific occurrence might be specified as a position number.
   Positive values indicate a position from the first occurrence, with 1 being
   the first one. Negative values indicate positions relative to the last one,
-  with -1 being the last one. It differs from req.hdr() in that any commas
-  present in the value are returned and are not used as delimiters. This is
-  sometimes useful with headers such as User-Agent.
+  with -1 being the last one.
 
 req.fhdr_cnt([]) : integer
   Returns an integer value representing the number of occurrences of request
   header field name , or the total number of header fields if  is
-  not specified. Contrary to its req.hdr_cnt() cousin, this function returns
-  the number of full line headers and does not stop on commas.
+  not specified. Like req.fhdr() it differs from res.hdr_cnt() by not splitting
+  headers at commas.
 
 req.hdr([[,]]) : string
-  This extracts the last occurrence of header  in an HTTP request. When
-  used from an ACL, all occurrences are iterated over until a match is found.
+  This returns the last comma-separated value of the header  in an HTTP
+  request. The fetch considers any comma as a delimiter for distinct values.
+  This is useful if you need to process headers that are defined to be a list
+  of values, such as Accept, or X-Forwarded-For. If full-line headers are
+  desired instead, use req.fhdr(). Please carefully check RFC 7231 to know how
+  certain headers are supposed to be parsed. Also, some of them are case
+  insensitive (e.g. Connection).
+
+  When used from an ACL, all occurrences are iterated over until a match is
+  found.
+
   Optionally, a specific occurrence might be specified as a position number.
   Positive values indicate a position from the first occurrence, with 1 being
   the first one. Negative values indicate positions relative to the last one,
-  with -1 being the last one. A typical use is with the X-Forwarded-For header
-  once converted to IP, associated with an IP stick-table. The function
-  considers any comma as a delimiter for distinct values. If full-line headers
-  are desired instead, use req.fhdr(). Please carefully check RFC7231 to know
-  how certain headers are supposed to be parsed. Also, some of them are case
-  insensitive (e.g. Connection).
+  with -1 being the last one.
+
+  A typical use is with the X-Forwarded-For header once converted to IP,
+  associated with an IP stick-table.
 
   ACL derivatives :
 hdr([[,]]) : exact string match
@@ -18551,34 +18562,36 @@ req.hdr_cnt([]) : integer
 hdr_cnt([]) : integer (deprecated)
   Returns an integer value representing the number of occurrences of request
   header field name , or the total number of header field values if
-   is not specified. It is important to remember that one header line may
-  count as several headers if it has several values. The function considers any
-  comma as a delimiter for distinct values. If full-line headers are desired
-  instead, req.fhdr_cnt() should be used instead. With ACLs, it can be used to
-  detect presence, absence or abuse of a specific header, as well as to block
-  request smuggling attacks by rejecting requests which contain more than one
-  of certain headers. See "req.hdr" for more information on header matching.
+   is not specified. Like req.hdr() it counts each comma separated
+  part of the header's value. If counting of full-line headers 

Re: [PATCH] BUILD: ssl: guard SSL_CTX_set_msg_callback with SSL_CTRL_SET_MSG_CALLBACK macro

2021-01-23 Thread William Lallemand
Hello,

On Sat, Jan 23, 2021 at 02:06:41AM +0500, Илья Шипицин wrote:
> Hello,
> 
> another ssl guard patch
> 
> Ilya

> From f39f9f69e29570fa43d7db5a0f08ee9395b98d50 Mon Sep 17 00:00:00 2001
> From: Ilya Shipitsin 
> Date: Sat, 23 Jan 2021 00:50:59 +0500
> Subject: [PATCH] BUILD: ssl: guard SSL_CTX_set_msg_callback with
>  SSL_CTRL_SET_MSG_CALLBACK macro
> 
> ---
>  src/ssl_sock.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/ssl_sock.c b/src/ssl_sock.c
> index 24a38e47d..2bda3d765 100644
> --- a/src/ssl_sock.c
> +++ b/src/ssl_sock.c
> @@ -4224,7 +4224,7 @@ int ssl_sock_prepare_ctx(struct bind_conf *bind_conf, 
> struct ssl_bind_conf *ssl_
>  #endif /* OPENSSL_NO_DH */
>  
>   SSL_CTX_set_info_callback(ctx, ssl_sock_infocbk);
> -#if HA_OPENSSL_VERSION_NUMBER >= 0x00907000L
> +#ifdef SSL_CTRL_SET_MSG_CALLBACK
>   SSL_CTX_set_msg_callback(ctx, ssl_sock_msgcbk);
>  #endif
>  #ifdef HAVE_OPENSSL_KEYLOG
> -- 
> 2.29.2
> 

Please add a commit message in your patches, patches with only a subject
line won't be taken. See this part of the contributing rules:
https://github.com/haproxy/haproxy/blob/master/CONTRIBUTING#L455

Thanks,

-- 
William Lallemand



Re: [PATCH} improve ssl guarding

2021-01-23 Thread William Lallemand
On Sat, Jan 23, 2021 at 04:50:08PM +0500, Илья Шипицин wrote:
> Hello,
> 
> yet another guard improving patch (forgot to fix last time)
> 
> Ilya

Hello,

> From 5ce5623fac558d85c0ef0ec26dcffca754a87fae Mon Sep 17 00:00:00 2001
> From: Ilya Shipitsin 
> Date: Sat, 23 Jan 2021 16:38:33 +0500
> Subject: [PATCH 1/2] BUILD: ssl: guard SSL_CTX_add_server_custom_ext with
>  special macro
> 
> ---
>  src/ssl_sock.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/ssl_sock.c b/src/ssl_sock.c
> index 2bda3d765..803af393f 100644
> --- a/src/ssl_sock.c
> +++ b/src/ssl_sock.c
> @@ -6720,7 +6720,7 @@ static struct action_kw_list http_req_actions = {ILH, {
>  
>  INITCALL1(STG_REGISTER, http_req_keywords_register, _req_actions);
>  
> -#if (HA_OPENSSL_VERSION_NUMBER >= 0x1000200fL && !defined OPENSSL_NO_TLSEXT 
> && !defined OPENSSL_IS_BORINGSSL)
> +#ifdef HAVE_SL_CTX_ADD_SERVER_CUSTOM_EXT
>  

I believe you wanted to write "SSL_CTX" and not "SL_CTX" here?

>  static void ssl_sock_sctl_free_func(void *parent, void *ptr, CRYPTO_EX_DATA 
> *ad, int idx, long argl, void *argp)
>  {
> @@ -6818,7 +6818,7 @@ static void __ssl_sock_init(void)
>  #if defined(USE_THREAD) && (HA_OPENSSL_VERSION_NUMBER < 0x1010L)
>   ssl_locking_init();
>  #endif
> -#if (HA_OPENSSL_VERSION_NUMBER >= 0x1000200fL && !defined OPENSSL_NO_TLSEXT 
> && !defined OPENSSL_IS_BORINGSSL)
> +#ifdef HAVE_SL_CTX_ADD_SERVER_CUSTOM_EXT
>   sctl_ex_index = SSL_CTX_get_ex_new_index(0, NULL, NULL, NULL, 
> ssl_sock_sctl_free_func);
>  #endif
>  


-- 
William Lallemand



[PATCH} improve ssl guarding

2021-01-23 Thread Илья Шипицин
Hello,

yet another guard improving patch (forgot to fix last time)

Ilya
From 5ce5623fac558d85c0ef0ec26dcffca754a87fae Mon Sep 17 00:00:00 2001
From: Ilya Shipitsin 
Date: Sat, 23 Jan 2021 16:38:33 +0500
Subject: [PATCH 1/2] BUILD: ssl: guard SSL_CTX_add_server_custom_ext with
 special macro

---
 src/ssl_sock.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 2bda3d765..803af393f 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -6720,7 +6720,7 @@ static struct action_kw_list http_req_actions = {ILH, {
 
 INITCALL1(STG_REGISTER, http_req_keywords_register, _req_actions);
 
-#if (HA_OPENSSL_VERSION_NUMBER >= 0x1000200fL && !defined OPENSSL_NO_TLSEXT && !defined OPENSSL_IS_BORINGSSL)
+#ifdef HAVE_SL_CTX_ADD_SERVER_CUSTOM_EXT
 
 static void ssl_sock_sctl_free_func(void *parent, void *ptr, CRYPTO_EX_DATA *ad, int idx, long argl, void *argp)
 {
@@ -6818,7 +6818,7 @@ static void __ssl_sock_init(void)
 #if defined(USE_THREAD) && (HA_OPENSSL_VERSION_NUMBER < 0x1010L)
 	ssl_locking_init();
 #endif
-#if (HA_OPENSSL_VERSION_NUMBER >= 0x1000200fL && !defined OPENSSL_NO_TLSEXT && !defined OPENSSL_IS_BORINGSSL)
+#ifdef HAVE_SL_CTX_ADD_SERVER_CUSTOM_EXT
 	sctl_ex_index = SSL_CTX_get_ex_new_index(0, NULL, NULL, NULL, ssl_sock_sctl_free_func);
 #endif
 
-- 
2.29.2



Re: Question about substring match (*_sub)

2021-01-23 Thread Aleksandar Lazic

On 23.01.21 07:36, Илья Шипицин wrote:

the following usually works for performance profiling.


1) setup work stand (similar to what you use in production)

2) use valgrind + callgrind for collecting traces

3) put workload

4) aggregate using kcachegrind

most probably you were going to do very similar things already :)


Thanks for the tips ;-)

The issue here is that for sub-string matching are several parameters
important like pattern, pattern length, text, text length and the
alphabet.

My question was focused to hear some "common" setups to be able to
create some valid tests for the different algorithms to compare it.

I think something like the examples below. As I don't used _sub
in the past it's difficult to me alone to create some valid use
cases which are used out there. It's okay to send examples only
to me, just in case for some security or privacy reasons.

acl allow_from_int hdr(x-forwarded-for) hdr_sub("192.168.4.5")
acl admin_access   hdr(user)hdr_sub("admin")
acl test_url   path urlp_sub("test=1")

Should UTF-* be considered as valid Alphabet or only ASCII?

If _sub is a very rare case then it's okay as it is, isn't it?

Opinions?


сб, 23 янв. 2021 г. в 03:18, Aleksandar Lazic mailto:al-hapr...@none.at>>:

Hi.

I would like to take a look into the substring match implementation because 
of
the comment there.


http://git.haproxy.org/?p=haproxy.git;a=blob;f=src/pattern.c;h=8729769e5e549bcd4043ae9220ceea440445332a;hb=HEAD#l767
 


"NB: Suboptimal, should be rewritten using a Boyer-Moore method."

Now before I take a deeper look into the different algorithms about 
sub-string
match I would like to know which pattern and length is a "common" use case
for the user here?

There are so many different algorithms which are mostly implemented in the
Smart Tool ( https://github.com/smart-tool/smart ) therefore it would be
interesting to know some metrics about the use cases.

Thanks for sharing.
Best regards

Aleks