Re: [PATCH 1/3] MINOR: cache: Remove the `hash` part of the accept-encoding secondary key

2021-01-18 Thread William Lallemand


On Mon, Jan 18, 2021 at 01:40:44PM +0100, Tim Düsterhus wrote:
> Remi,
> 
> Am 18.01.21 um 11:24 schrieb Remi Tricot-Le Breton:
> > The patches look good to me. Just one point concerning the first one,
> > you could have removed everything related to ACCEPT_ENCODING_MAX_ENTRIES
> > since the limit was induced by the array that does not exist anymore.
> 
> I have intentionally opted to leave it as is, as a bit of a safety net.
> It is not going to make sense to send more than 16 *known* encodings at
> once. This limits processing time for an abusive client that sends:
> 
> accept-encoding: br,br,br,br,[…],br
> 
> > The comment of the accept_encoding_normalizer function does not match
> > its behavior anymore either.
> 
> Indeed. I adjusted that on v2.
> 
> Best regards
> Tim Düsterhus
> 

Thanks to both of you, applied.

-- 
William Lallemand



[PATCH v2 1/3] MINOR: cache: Remove the `hash` part of the accept-encoding secondary key

2021-01-18 Thread Tim Duesterhus
As of commit 6ca89162dc881df8fecd7713ca1fe5dbaa66b315 this hash no longer is
required, because unknown encodings are not longer stored and known encodings
do not use the cache.
---
 include/haproxy/http_ana-t.h |  2 +-
 src/cache.c  | 94 +---
 2 files changed, 24 insertions(+), 72 deletions(-)

diff --git a/include/haproxy/http_ana-t.h b/include/haproxy/http_ana-t.h
index 63085a5b8..50fbd3de8 100644
--- a/include/haproxy/http_ana-t.h
+++ b/include/haproxy/http_ana-t.h
@@ -95,7 +95,7 @@
  * request/response pairs, because they depend on the responses' optional Vary
  * header. The different sizes can be found in the vary_information object (see
  * cache.c).*/
-#define HTTP_CACHE_SEC_KEY_LEN (sizeof(int)+sizeof(int)+sizeof(int))
+#define HTTP_CACHE_SEC_KEY_LEN (sizeof(uint32_t)+sizeof(int))
 
 
 /* Redirect flags */
diff --git a/src/cache.c b/src/cache.c
index 32c99cdc4..16dc7d61c 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -100,11 +100,6 @@ struct vary_hashing_information {
int(*cmp_fn)(const void *ref_hash, const void *new_hash, unsigned int 
hash_len); /* Comparison function, should return 0 if the hashes are alike */
 };
 
-struct accept_encoding_hash {
-   unsigned int encoding_bitmap;
-   unsigned int hash;
-} __attribute__((packed));
-
 static int http_request_prebuild_full_secondary_key(struct stream *s);
 static int http_request_build_secondary_key(struct stream *s, int 
vary_signature);
 static int http_request_reduce_secondary_key(unsigned int vary_signature,
@@ -123,7 +118,7 @@ static int accept_encoding_hash_cmp(const void *ref_hash, 
const void *new_hash,
 /* Warning : do not forget to update HTTP_CACHE_SEC_KEY_LEN when new items are
  * added to this array. */
 const struct vary_hashing_information vary_information[] = {
-   { IST("accept-encoding"), VARY_ACCEPT_ENCODING, sizeof(struct 
accept_encoding_hash), &accept_encoding_normalizer, &accept_encoding_hash_cmp },
+   { IST("accept-encoding"), VARY_ACCEPT_ENCODING, sizeof(uint32_t), 
&accept_encoding_normalizer, &accept_encoding_hash_cmp },
{ IST("referer"), VARY_REFERER, sizeof(int), &default_normalizer, NULL 
},
 };
 
@@ -2151,19 +2146,6 @@ struct flt_ops cache_ops = {
 };
 
 
-int accept_encoding_cmp(const void *a, const void *b)
-{
-   unsigned int int_a = *(unsigned int*)a;
-   unsigned int int_b = *(unsigned int*)b;
-
-   if (int_a < int_b)
-   return -1;
-   if (int_a > int_b)
-   return 1;
-   return 0;
-}
-
-
 #define CHECK_ENCODING(str, encoding_name, encoding_value) \
({ \
int retval = 0; \
@@ -2274,34 +2256,30 @@ static int parse_encoding_value(struct ist encoding, 
unsigned int *encoding_valu
 
 #define ACCEPT_ENCODING_MAX_ENTRIES 16
 /*
- * Build a hash of the accept-encoding header. The hash is split into an
- * encoding bitmap and an actual hash of the different encodings.
+ * Build a bitmap of the accept-encoding header.
+ *
  * The bitmap is built by matching every sub-part of the accept-encoding value
  * with a subset of explicitly supported encodings, which all have their own 
bit
  * in the bitmap. This bitmap will be used to determine if a response can be
  * served to a client (that is if it has an encoding that is accepted by the
- * client).
- * The hash part is built out of all the sub-parts of the value, which are
- * converted to lower case, hashed, sorted and then all the unique sub-hashes
- * are XORed into a single hash.
- * Returns 0 in case of success, 1 if the hash buffer should be filled with 0s
- * and -1 in case of error.
+ * client). Any unknown encodings will be indicated by the VARY_ENCODING_OTHER
+ * bit.
+ *
+ * Returns 0 in case of success and -1 in case of error.
  */
 static int accept_encoding_normalizer(struct htx *htx, struct ist hdr_name,
  char *buf, unsigned int *buf_len)
 {
-   unsigned int values[ACCEPT_ENCODING_MAX_ENTRIES] = {};
size_t count = 0;
-   struct accept_encoding_hash hash = {};
+   uint32_t encoding_bitmap = 0;
unsigned int encoding_bmp_bl = -1;
-   unsigned int prev = 0, curr = 0;
struct http_hdr_ctx ctx = { .blk = NULL };
unsigned int encoding_value;
unsigned int rejected_encoding;
 
/* A user agent always accepts an unencoded value unless it explicitly
 * refuses it through an "identity;q=0" accept-encoding value. */
-   hash.encoding_bitmap |= VARY_ENCODING_IDENTITY;
+   encoding_bitmap |= VARY_ENCODING_IDENTITY;
 
/* Iterate over all the ACCEPT_ENCODING_MAX_ENTRIES first 
accept-encoding
 * values that might span acrosse multiple accept-encoding headers. */
@@ -2314,50 +2292,37 @@ static int accept_encoding_normalizer(struct htx *htx, 
struct ist hdr_name,
if (rejected_encoding)
encoding_bmp_bl &= ~encoding_value;

[PATCH v2 3/3] CLEANUP: cache: Use proper data types in secondary_key_cmp()

2021-01-18 Thread Tim Duesterhus
- hash_length is `unsigned int` and so should offset.
- idx is compared to a `size_t` and thus it should also be.
---
 src/cache.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/cache.c b/src/cache.c
index ab0799a5d..f03944faf 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -208,8 +208,8 @@ struct cache_entry *entry_exist(struct cache *cache, char 
*hash)
 static int secondary_key_cmp(const char *ref_key, const char *new_key)
 {
int retval = 0;
-   int idx = 0;
-   int offset = 0;
+   size_t idx = 0;
+   unsigned int offset = 0;
const struct vary_hashing_information *info;
 
for (idx = 0; idx < sizeof(vary_information)/sizeof(*vary_information) 
&& !retval; ++idx) {
-- 
2.29.0




[PATCH v2 2/3] CLEANUP: Rename accept_encoding_hash_cmp to accept_encoding_bitmap_cmp

2021-01-18 Thread Tim Duesterhus
For the `accept-encoding` header a bitmap and not a hash is stored.
---
 src/cache.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/src/cache.c b/src/cache.c
index 16dc7d61c..ab0799a5d 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -97,7 +97,7 @@ struct vary_hashing_information {
enum vary_header_bit value;  /* Bit representing the header in 
a vary signature */
unsigned int hash_length;/* Size of the sub hash for this 
header's value */
int(*norm_fn)(struct htx*,struct ist hdr_name,char* buf,unsigned int* 
buf_len);  /* Normalization function */
-   int(*cmp_fn)(const void *ref_hash, const void *new_hash, unsigned int 
hash_len); /* Comparison function, should return 0 if the hashes are alike */
+   int(*cmp_fn)(const void *ref, const void *new, unsigned int len); /* 
Comparison function, should return 0 if the hashes are alike */
 };
 
 static int http_request_prebuild_full_secondary_key(struct stream *s);
@@ -113,12 +113,12 @@ static int accept_encoding_normalizer(struct htx *htx, 
struct ist hdr_name,
 static int default_normalizer(struct htx *htx, struct ist hdr_name,
  char *buf, unsigned int *buf_len);
 
-static int accept_encoding_hash_cmp(const void *ref_hash, const void 
*new_hash, unsigned int hash_len);
+static int accept_encoding_bitmap_cmp(const void *ref, const void *new, 
unsigned int len);
 
 /* Warning : do not forget to update HTTP_CACHE_SEC_KEY_LEN when new items are
  * added to this array. */
 const struct vary_hashing_information vary_information[] = {
-   { IST("accept-encoding"), VARY_ACCEPT_ENCODING, sizeof(uint32_t), 
&accept_encoding_normalizer, &accept_encoding_hash_cmp },
+   { IST("accept-encoding"), VARY_ACCEPT_ENCODING, sizeof(uint32_t), 
&accept_encoding_normalizer, &accept_encoding_bitmap_cmp },
{ IST("referer"), VARY_REFERER, sizeof(int), &default_normalizer, NULL 
},
 };
 
@@ -2354,15 +2354,15 @@ static int default_normalizer(struct htx *htx, struct 
ist hdr_name,
 }
 
 /*
- * Accept-Encoding sub-hash comparison function.
- * Returns 0 if the hashes are alike.
+ * Accept-Encoding bitmap comparison function.
+ * Returns 0 if the bitmaps are compatible.
  */
-static int accept_encoding_hash_cmp(const void *ref_hash, const void 
*new_hash, unsigned int hash_len)
+static int accept_encoding_bitmap_cmp(const void *ref, const void *new, 
unsigned int len)
 {
-   uint32_t ref = read_u32(ref_hash);
-   uint32_t new = read_u32(new_hash);
+   uint32_t ref_bitmap = read_u32(ref);
+   uint32_t new_bitmap = read_u32(new);
 
-   if (!(ref & VARY_ENCODING_OTHER)) {
+   if (!(ref_bitmap & VARY_ENCODING_OTHER)) {
/* All the bits set in the reference bitmap correspond to the
 * stored response' encoding and should all be set in the new
 * encoding bitmap in order for the client to be able to manage
@@ -2373,7 +2373,7 @@ static int accept_encoding_hash_cmp(const void *ref_hash, 
const void *new_hash,
 * the cache (as far as the accept-encoding part is concerned).
 */
 
-   return (ref & new) != ref;
+   return (ref_bitmap & new_bitmap) != ref_bitmap;
}
else {
return 1;
-- 
2.29.0




Re: [PATCH 1/3] MINOR: cache: Remove the `hash` part of the accept-encoding secondary key

2021-01-18 Thread Tim Düsterhus
Remi,

Am 18.01.21 um 11:24 schrieb Remi Tricot-Le Breton:
> The patches look good to me. Just one point concerning the first one,
> you could have removed everything related to ACCEPT_ENCODING_MAX_ENTRIES
> since the limit was induced by the array that does not exist anymore.

I have intentionally opted to leave it as is, as a bit of a safety net.
It is not going to make sense to send more than 16 *known* encodings at
once. This limits processing time for an abusive client that sends:

accept-encoding: br,br,br,br,[…],br

> The comment of the accept_encoding_normalizer function does not match
> its behavior anymore either.

Indeed. I adjusted that on v2.

Best regards
Tim Düsterhus




Re: [PATCH] BUG/MINOR: init: enforce strict-limits when using master-worker

2021-01-18 Thread William Lallemand
On Thu, Jan 14, 2021 at 12:13:17PM +0100, William Dauchy wrote:
> On Thu, Jan 14, 2021 at 11:21 AM William Lallemand
>  wrote:
> > VTest is not really suited to test the process management, for example
> > the tests doing a reload have timing issues because VTest is not able to
> > know when HAProxy is ready.
> >
> > Could you share what you tried to do? I'm not sure what is the problem
> > you are mentionning with -expectexit.
> 
> here it is:
> 
> varnishtest "strict-limits test"
> 
> #REQUIRE_VERSION=2.1
> 
> feature ignore_unknown_macro
> 
> server s1 {
> rxreq
> txresp
> } -repeat 2 -start
> 
> haproxy h1 -conf {
> global
> strict-limits
> maxconn 100
> 
> defaults
> mode http
> timeout connect 1s
> timeout client  1s
> timeout server  1s
> 
> frontend fe
> bind "fd@${fe}"
> default_backend be
> 
> backend be
> server s1 ${s1_addr}:${s1_port}
> } -start -expectexit 42
> 
> # give it some time to start and detect failure
> shell {
> sleep 1
> }
> 
> client c1 -connect ${h1_fe_sock} {
> txreq -url "/"
> } -run
> 
> - first, the sleep 1 really depends of the process init time, like you
> mentioned in your previous message, it is not possible to detect when
> process should be ready. So it remains very ugly.
> - the client triggers the `wait4` which is correctly ending with this error:
> *h1Expected exit: 0x2a signal: 0 core: 0
>  h1Bad exit status: 0x0100 exit 0x1 signal 0 core 0
> 
> - if I switch to no strict-limits, the process won't exit by itself;
> it does not trigger the `wait4` error, quite unclear why for now.

If I remember correctly the wait() is only done after a timeout of 10
seconds letting VTest run even if HAProxy exit immediatly. So I presume
HAProxy is still present as a zombie process during this time.
I don't know why it's done this way but this is a common problem I
have when starting VTest with an invalid haproxy configuration.

-- 
William Lallemand



Re: [PATCH] improve ssl guarding by switching to macro SSL_CLIENT_HELLO_CB instead of openssl version

2021-01-18 Thread Илья Шипицин
we can do nasty thing.
SSL_CLIENT_HELLO_CB is not defined for BoringSSL, we can (in
openssl-compat.h) check whether BoringSSL is used and define that macro.

I'm not sure it is good thing.

if you thing it is, please modify patch when applying. I'm ok with such
change.

пн, 18 янв. 2021 г. в 15:53, Илья Шипицин :

>
>
> пн, 18 янв. 2021 г. в 15:09, William Lallemand :
>
>> Hello,
>>
>> On Sat, Jan 16, 2021 at 11:25:05PM +0500, Илья Шипицин wrote:
>> > Hello,
>> >
>> > next openssl guarding patch
>> >
>> > Ilya
>>
>> > From b5ff0a9f1e0d2edc84981b39050e7f21d2b08ba8 Mon Sep 17 00:00:00 2001
>> > From: Ilya Shipitsin 
>> > Date: Sat, 16 Jan 2021 23:15:12 +0500
>> > Subject: [PATCH] BUILD: ssl: guard Client Hello callbacks with
>> >  SSL_CLIENT_HELLO_CB macro instead of openssl version
>> >
>> > ---
>> >  include/haproxy/ssl_sock.h | 2 +-
>> >  src/ssl_sock.c | 2 +-
>> >  2 files changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/include/haproxy/ssl_sock.h b/include/haproxy/ssl_sock.h
>> > index ebfdb19ab..bde75b632 100644
>> > --- a/include/haproxy/ssl_sock.h
>> > +++ b/include/haproxy/ssl_sock.h
>> > @@ -92,7 +92,7 @@ int ssl_sock_load_global_dh_param_from_file(const
>> char *filename);
>> >  void ssl_free_dh(void);
>> >  #endif
>> >  void ssl_free_engines(void);
>> > -#if ((HA_OPENSSL_VERSION_NUMBER >= 0x10101000L) ||
>> defined(OPENSSL_IS_BORINGSSL))
>> > +#if (defined(SSL_CLIENT_HELLO_CB) || defined(OPENSSL_IS_BORINGSSL))
>> >  int ssl_sock_switchctx_err_cbk(SSL *ssl, int *al, void *priv);
>> >  #ifdef OPENSSL_IS_BORINGSSL
>> >  int ssl_sock_switchctx_cbk(const struct ssl_early_callback_ctx *ctx);
>> > diff --git a/src/ssl_sock.c b/src/ssl_sock.c
>> > index 5ac81d36a..3e133d423 100644
>> > --- a/src/ssl_sock.c
>> > +++ b/src/ssl_sock.c
>> > @@ -2290,7 +2290,7 @@ static void ssl_sock_switchctx_set(SSL *ssl,
>> SSL_CTX *ctx)
>> >   SSL_set_SSL_CTX(ssl, ctx);
>> >  }
>> >
>> > -#if ((HA_OPENSSL_VERSION_NUMBER >= 0x10101000L) ||
>> defined(OPENSSL_IS_BORINGSSL))
>> > +#if (defined(SSL_CLIENT_HELLO_CB) || defined(OPENSSL_IS_BORINGSSL))
>> >
>> >  int ssl_sock_switchctx_err_cbk(SSL *ssl, int *al, void *priv)
>> >  {
>>
>> We probably want to remove the defined(IS_BORINGSSL) from the ssl_sock.c
>> too.
>> Why don't you define a macro constant with the feature name in
>> openssl-compat.h and test this constant in ssl_sock.c? Like it was done
>> for various fonctions.
>>
>
> it depends. I'd consider removing OPENSSL_IS_BORINGSSL as a future
> improvements.
>
> this particular guard is used 2 times only (in *.h and *.c files),
> readability is good.
>
>
>
>>
>> Regards,
>>
>> --
>> William Lallemand
>>
>


Re: [PATCH] improve ssl guarding by switching to macro SSL_CLIENT_HELLO_CB instead of openssl version

2021-01-18 Thread Илья Шипицин
пн, 18 янв. 2021 г. в 15:09, William Lallemand :

> Hello,
>
> On Sat, Jan 16, 2021 at 11:25:05PM +0500, Илья Шипицин wrote:
> > Hello,
> >
> > next openssl guarding patch
> >
> > Ilya
>
> > From b5ff0a9f1e0d2edc84981b39050e7f21d2b08ba8 Mon Sep 17 00:00:00 2001
> > From: Ilya Shipitsin 
> > Date: Sat, 16 Jan 2021 23:15:12 +0500
> > Subject: [PATCH] BUILD: ssl: guard Client Hello callbacks with
> >  SSL_CLIENT_HELLO_CB macro instead of openssl version
> >
> > ---
> >  include/haproxy/ssl_sock.h | 2 +-
> >  src/ssl_sock.c | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/haproxy/ssl_sock.h b/include/haproxy/ssl_sock.h
> > index ebfdb19ab..bde75b632 100644
> > --- a/include/haproxy/ssl_sock.h
> > +++ b/include/haproxy/ssl_sock.h
> > @@ -92,7 +92,7 @@ int ssl_sock_load_global_dh_param_from_file(const char
> *filename);
> >  void ssl_free_dh(void);
> >  #endif
> >  void ssl_free_engines(void);
> > -#if ((HA_OPENSSL_VERSION_NUMBER >= 0x10101000L) ||
> defined(OPENSSL_IS_BORINGSSL))
> > +#if (defined(SSL_CLIENT_HELLO_CB) || defined(OPENSSL_IS_BORINGSSL))
> >  int ssl_sock_switchctx_err_cbk(SSL *ssl, int *al, void *priv);
> >  #ifdef OPENSSL_IS_BORINGSSL
> >  int ssl_sock_switchctx_cbk(const struct ssl_early_callback_ctx *ctx);
> > diff --git a/src/ssl_sock.c b/src/ssl_sock.c
> > index 5ac81d36a..3e133d423 100644
> > --- a/src/ssl_sock.c
> > +++ b/src/ssl_sock.c
> > @@ -2290,7 +2290,7 @@ static void ssl_sock_switchctx_set(SSL *ssl,
> SSL_CTX *ctx)
> >   SSL_set_SSL_CTX(ssl, ctx);
> >  }
> >
> > -#if ((HA_OPENSSL_VERSION_NUMBER >= 0x10101000L) ||
> defined(OPENSSL_IS_BORINGSSL))
> > +#if (defined(SSL_CLIENT_HELLO_CB) || defined(OPENSSL_IS_BORINGSSL))
> >
> >  int ssl_sock_switchctx_err_cbk(SSL *ssl, int *al, void *priv)
> >  {
>
> We probably want to remove the defined(IS_BORINGSSL) from the ssl_sock.c
> too.
> Why don't you define a macro constant with the feature name in
> openssl-compat.h and test this constant in ssl_sock.c? Like it was done
> for various fonctions.
>

it depends. I'd consider removing OPENSSL_IS_BORINGSSL as a future
improvements.

this particular guard is used 2 times only (in *.h and *.c files),
readability is good.



>
> Regards,
>
> --
> William Lallemand
>


Re: [PATCH 1/3] MINOR: cache: Remove the `hash` part of the accept-encoding secondary key

2021-01-18 Thread Remi Tricot-Le Breton

Hello Tim,

The patches look good to me. Just one point concerning the first one, 
you could have removed everything related to ACCEPT_ENCODING_MAX_ENTRIES 
since the limit was induced by the array that does not exist anymore.
The comment of the accept_encoding_normalizer function does not match 
its behavior anymore either.

Nothing to say apart from that, thanks for catching this.

Rémi

On 16/01/2021 15:07, Tim Duesterhus wrote:

As of commit 6ca89162dc881df8fecd7713ca1fe5dbaa66b315 this hash no longer is
required, because unknown encodings are not longer stored and known encodings
do not use the cache.
---
  include/haproxy/http_ana-t.h |  2 +-
  src/cache.c  | 80 
  2 files changed, 18 insertions(+), 64 deletions(-)

diff --git a/include/haproxy/http_ana-t.h b/include/haproxy/http_ana-t.h
index 63085a5b8..50fbd3de8 100644
--- a/include/haproxy/http_ana-t.h
+++ b/include/haproxy/http_ana-t.h
@@ -95,7 +95,7 @@
   * request/response pairs, because they depend on the responses' optional Vary
   * header. The different sizes can be found in the vary_information object 
(see
   * cache.c).*/
-#define HTTP_CACHE_SEC_KEY_LEN (sizeof(int)+sizeof(int)+sizeof(int))
+#define HTTP_CACHE_SEC_KEY_LEN (sizeof(uint32_t)+sizeof(int))
  
  
  /* Redirect flags */

diff --git a/src/cache.c b/src/cache.c
index 32c99cdc4..965509871 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -100,11 +100,6 @@ struct vary_hashing_information {
int(*cmp_fn)(const void *ref_hash, const void *new_hash, unsigned int 
hash_len); /* Comparison function, should return 0 if the hashes are alike */
  };
  
-struct accept_encoding_hash {

-   unsigned int encoding_bitmap;
-   unsigned int hash;
-} __attribute__((packed));
-
  static int http_request_prebuild_full_secondary_key(struct stream *s);
  static int http_request_build_secondary_key(struct stream *s, int 
vary_signature);
  static int http_request_reduce_secondary_key(unsigned int vary_signature,
@@ -123,7 +118,7 @@ static int accept_encoding_hash_cmp(const void *ref_hash, 
const void *new_hash,
  /* Warning : do not forget to update HTTP_CACHE_SEC_KEY_LEN when new items are
   * added to this array. */
  const struct vary_hashing_information vary_information[] = {
-   { IST("accept-encoding"), VARY_ACCEPT_ENCODING, sizeof(struct 
accept_encoding_hash), &accept_encoding_normalizer, &accept_encoding_hash_cmp },
+   { IST("accept-encoding"), VARY_ACCEPT_ENCODING, sizeof(uint32_t), 
&accept_encoding_normalizer, &accept_encoding_hash_cmp },
{ IST("referer"), VARY_REFERER, sizeof(int), &default_normalizer, NULL 
},
  };
  
@@ -2151,19 +2146,6 @@ struct flt_ops cache_ops = {

  };
  
  
-int accept_encoding_cmp(const void *a, const void *b)

-{
-   unsigned int int_a = *(unsigned int*)a;
-   unsigned int int_b = *(unsigned int*)b;
-
-   if (int_a < int_b)
-   return -1;
-   if (int_a > int_b)
-   return 1;
-   return 0;
-}
-
-
  #define CHECK_ENCODING(str, encoding_name, encoding_value) \
({ \
int retval = 0; \
@@ -2290,18 +2272,16 @@ static int parse_encoding_value(struct ist encoding, 
unsigned int *encoding_valu
  static int accept_encoding_normalizer(struct htx *htx, struct ist hdr_name,
  char *buf, unsigned int *buf_len)
  {
-   unsigned int values[ACCEPT_ENCODING_MAX_ENTRIES] = {};
size_t count = 0;
-   struct accept_encoding_hash hash = {};
+   uint32_t encoding_bitmap = 0;
unsigned int encoding_bmp_bl = -1;
-   unsigned int prev = 0, curr = 0;
struct http_hdr_ctx ctx = { .blk = NULL };
unsigned int encoding_value;
unsigned int rejected_encoding;
  
  	/* A user agent always accepts an unencoded value unless it explicitly

 * refuses it through an "identity;q=0" accept-encoding value. */
-   hash.encoding_bitmap |= VARY_ENCODING_IDENTITY;
+   encoding_bitmap |= VARY_ENCODING_IDENTITY;
  
  	/* Iterate over all the ACCEPT_ENCODING_MAX_ENTRIES first accept-encoding

 * values that might span acrosse multiple accept-encoding headers. */
@@ -2314,50 +2294,37 @@ static int accept_encoding_normalizer(struct htx *htx, 
struct ist hdr_name,
if (rejected_encoding)
encoding_bmp_bl &= ~encoding_value;
else
-   hash.encoding_bitmap |= encoding_value;
+   encoding_bitmap |= encoding_value;
}
else {
/* Unknown encoding */
-   hash.encoding_bitmap |= VARY_ENCODING_OTHER;
+   encoding_bitmap |= VARY_ENCODING_OTHER;
}
  
-		values[count++] = hash_crc32(istptr(ctx.value), istlen(ctx.value));

+   count++;
}
  
  	/* If a "*" was found in the accepted encodings (without a null wei

Re: [PATCH] improve ssl guarding by switching to macro SSL_CLIENT_HELLO_CB instead of openssl version

2021-01-18 Thread William Lallemand
Hello,

On Sat, Jan 16, 2021 at 11:25:05PM +0500, Илья Шипицин wrote:
> Hello,
> 
> next openssl guarding patch
> 
> Ilya

> From b5ff0a9f1e0d2edc84981b39050e7f21d2b08ba8 Mon Sep 17 00:00:00 2001
> From: Ilya Shipitsin 
> Date: Sat, 16 Jan 2021 23:15:12 +0500
> Subject: [PATCH] BUILD: ssl: guard Client Hello callbacks with
>  SSL_CLIENT_HELLO_CB macro instead of openssl version
> 
> ---
>  include/haproxy/ssl_sock.h | 2 +-
>  src/ssl_sock.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/haproxy/ssl_sock.h b/include/haproxy/ssl_sock.h
> index ebfdb19ab..bde75b632 100644
> --- a/include/haproxy/ssl_sock.h
> +++ b/include/haproxy/ssl_sock.h
> @@ -92,7 +92,7 @@ int ssl_sock_load_global_dh_param_from_file(const char 
> *filename);
>  void ssl_free_dh(void);
>  #endif
>  void ssl_free_engines(void);
> -#if ((HA_OPENSSL_VERSION_NUMBER >= 0x10101000L) || 
> defined(OPENSSL_IS_BORINGSSL))
> +#if (defined(SSL_CLIENT_HELLO_CB) || defined(OPENSSL_IS_BORINGSSL))
>  int ssl_sock_switchctx_err_cbk(SSL *ssl, int *al, void *priv);
>  #ifdef OPENSSL_IS_BORINGSSL
>  int ssl_sock_switchctx_cbk(const struct ssl_early_callback_ctx *ctx);
> diff --git a/src/ssl_sock.c b/src/ssl_sock.c
> index 5ac81d36a..3e133d423 100644
> --- a/src/ssl_sock.c
> +++ b/src/ssl_sock.c
> @@ -2290,7 +2290,7 @@ static void ssl_sock_switchctx_set(SSL *ssl, SSL_CTX 
> *ctx)
>   SSL_set_SSL_CTX(ssl, ctx);
>  }
>  
> -#if ((HA_OPENSSL_VERSION_NUMBER >= 0x10101000L) || 
> defined(OPENSSL_IS_BORINGSSL))
> +#if (defined(SSL_CLIENT_HELLO_CB) || defined(OPENSSL_IS_BORINGSSL))
>  
>  int ssl_sock_switchctx_err_cbk(SSL *ssl, int *al, void *priv)
>  {

We probably want to remove the defined(IS_BORINGSSL) from the ssl_sock.c too.
Why don't you define a macro constant with the feature name in
openssl-compat.h and test this constant in ssl_sock.c? Like it was done
for various fonctions.

Regards,

-- 
William Lallemand



Re: [PATCH] DOC: replace use of HA-Proxy with HAProxy

2021-01-18 Thread William Dauchy
On Mon, Jan 18, 2021 at 6:35 AM John Traweek CCNA, Sec+
 wrote:
> How do I unsubscribe?

send an email to haproxy+unsubscr...@formilux.org
-- 
William



Re: [PATCH] MINOR: build: discard echoing in help target

2021-01-18 Thread William Lallemand
On Sun, Jan 17, 2021 at 06:47:47PM +, Bertrand Jacquin wrote:
> When V=1 is used in conjuction with help, the output becomes pretty
> difficult to read properly.
> 
>   $ make TARGET=linux-glibc V=1 help
>   ..
> DEBUG_USE_ABORT: use abort() for program termination, see 
> include/haproxy/bug.h for details
>   echo; \
>  if [ -n "" ]; then \
>if [ -n "" ]; then \
>   echo "Current TARGET: "; \
>else \
>   echo "Current TARGET:  (custom target)"; \
>fi; \
>  else \
>echo "TARGET not set, you may pass 'TARGET=xxx' to set one among :";\
>echo "  linux-glibc, linux-glibc-legacy, solaris, freebsd, dragonfly, 
> netbsd,"; \
>echo "  osx, openbsd, aix51, aix52, aix72-gcc, cygwin, haiku, 
> generic,"; \
>echo "  custom"; \
>  fi
> 
>   TARGET not set, you may pass 'TARGET=xxx' to set one among :
> linux-glibc, linux-glibc-legacy, solaris, freebsd, dragonfly, netbsd,
> osx, openbsd, aix51, aix52, aix72-gcc, cygwin, haiku, generic,
> custom
>   echo;echo "Enabled features for TARGET '' (disable with 'USE_xxx=') :"
> 
>   Enabled features for TARGET '' (disable with 'USE_xxx=') :
>   set --POLL  ; echo "  $*" | (fmt || 
> cat) 2>/dev/null
> POLL
>   echo;echo "Disabled features for TARGET '' (enable with 'USE_xxx=1') :"
> 
>   Disabled features for TARGET '' (enable with 'USE_xxx=1') :
>   set -- EPOLL KQUEUE NETFILTER PCRE PCRE_JIT PCRE2 PCRE2_JIT  PRIVATE_CACHE 
> THREAD PTHREAD_PSHARED BACKTRACE STATIC_PCRE STATIC_PCRE2 TPROXY LINUX_TPROXY 
> LINUX_SPLICE LIBCRYPT CRYPT_H GETADDRINFO OPENSSL LUA FUTEX ACCEPT4 CLOSEFROM 
> ZLIB SLZ CPU_AFFINITY TFO NS DL RT DEVICEATLAS 51DEGREES WURFL SYSTEMD 
> OBSOLETE_LINKER PRCTL THREAD_DUMP EVPORTS OT QUIC; echo "  $*" | (fmt || cat) 
> 2>/dev/null
> EPOLL KQUEUE NETFILTER PCRE PCRE_JIT PCRE2 PCRE2_JIT PRIVATE_CACHE
> 
> This commit ensure the help target always discard line echoing
> regardless of V variable as done for reg-tests-help target.

Thanks, merged!

-- 
William Lallemand