Re: Coverity scan findings

[Dinko Korunic]

Hi Dave,

Just browse to https://scan.coverity.com/projects/haproxy 
 and make a request for access, 
I’ll gladly add you to the project.

> On 10 Sep 2019, at 16:49, Dave Chiluk  wrote:
> 
> Are these scans publicly available *(I'm looking for a link)?  They look 
> interesting, but without line numbers it looks a lot less useful.
> 
> Dave.
> 
> On Thu, Aug 8, 2019 at 2:49 AM Илья Шипицин  > wrote:
> Hello,
> 
> coverity found tens of "null pointer dereference".
> also, there's a good correlation, after "now fixed, good catch" coverity 
> usually dismiss some bug.
> 
> should we revisit those findings ?
> 
> 
> 


Kind regards,
D.

-- 
Dinko Korunic   ** Standard disclaimer applies **
Sent from OSF1 osf1v4b V4.0 564 alpha

Re: [PATCH] MINOR: dns: Set maximum DNS udp payload at 65507

[Anthonin Bonnefoy]

Hi,

Any news on this?
We have use cases with records returning 600+ IPs and the resolution fails
since it exceeds the payload.

Regards,
Anthonin

On Sun, Dec 2, 2018 at 6:46 PM Willy Tarreau  wrote:

> On Thu, Oct 18, 2018 at 05:52:47PM +0200, Anthonin Bonnefoy wrote:
> > From: Anthonin Bonnefoy 
> >
> > With EDNS, DNS packets can have a maximum size of 65507.
> > This will allow to have a bigger accepted_payload_size which is useful
> > when we have more than 100 SRV records.
>
> Baptiste, please have a look at this one, I want to be sure we don't
> miss it if needed.
>
> Thanks,
> Willy
>
> > ---
> >  include/types/dns.h | 5 -
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/types/dns.h b/include/types/dns.h
> > index 488d3996..53f24c19 100644
> > --- a/include/types/dns.h
> > +++ b/include/types/dns.h
> > @@ -42,7 +42,10 @@
> >   */
> >  #define DNS_MAX_LABEL_SIZE   63
> >  #define DNS_MAX_NAME_SIZE255
> > -#define DNS_MAX_UDP_MESSAGE  8192
> > +/* With EDNS, DNS message can reach the maximum size of an UDP message
> > + * which is (65535 - 8 bytes UDP header - 20 bytes IP header)
> > + */
> > +#define DNS_MAX_UDP_MESSAGE  65507
> >
> >  /* DNS minimun record size: 1 char + 1 NULL + type + class */
> >  #define DNS_MIN_RECORD_SIZE  (1 + 1 + 2 + 2)
> > --
> > 2.13.2
> >
>

Re: RFC uuid for log-format

[Tim Düsterhus]

Luca,

Am 10.09.19 um 15:48 schrieb Schimweg, Luca:
> sorry, didn't know about these guidelines. I packed all changes into one 
> commit in the patch file in the attachments. 

One more thing: Do not leave out the body of the commit message. See the
paragraph starting line 562 of the CONTRIBUTING file:

https://github.com/haproxy/haproxy/blob/e058f7359f3822beb8552f77a6d439cb053edb3f/CONTRIBUTING#L562-L567

> I included your changes, you are right, the code looks much better now.

I spotted a little typo:

((rnd[1] >> 16u) % 0xFFF) | 0x4000,  // highest 4 bits indicate the uuid
version

It should read `&` instead of `%` there.

> I'm open for any feedback.

I suggest to wait for Willy before sending an updated patch. I guess he
has some more feedback.

Best regards
Tim Düsterhus

Re: RFC uuid for log-format

[Schimweg, Luca]

Hey,

sorry, didn't know about these guidelines. I packed all changes into one commit 
in the patch file in the attachments. 
I included your changes, you are right, the code looks much better now.

I'm open for any feedback.

Best regards,
Luca

On 10.09.19, 14:25, "Tim Düsterhus"  wrote:

Luca,

(not an HAProxy maintainer)

please have a look at the CONTRIBUTING file regarding the commit message
format: https://github.com/haproxy/haproxy/blob/master/CONTRIBUTING

You are missing the severity indicator and the affected subsystem. It
probably should be `MINOR: sample:`.

> diff --git a/doc/configuration.txt b/doc/configuration.txt
> index ba5a8b360..e22557134 100644
> --- a/doc/configuration.txt
> +++ b/doc/configuration.txt
> @@ -14391,6 +14391,11 @@ thread : integer
>the function, between 0 and (global.nbthread-1). This is useful for 
logging
>and debugging purposes.
>  
> +uuid([]) : string
> +  Returns a random UUID, following the RFC4122 standard. If the version 
is not
   ^^
I'd remove the random here. UUID v3 while not currently supported can
hardly be called "random". UUID in itself already implies "unique".

> +  specified, a UUID version 4 (fully random) is returned.
> +  Currently, only version 4 is supported.
> +
>  var() : undefined
>Returns a variable with the stored type. If the variable is not set, 
the
>sample fetch fails. The name of the variable starts with an indication
> diff --git a/src/sample.c b/src/sample.c
> index 6327b6b08..6caa440b7 100644
> --- a/src/sample.c
> +++ b/src/sample.c
> @@ -3196,6 +3196,47 @@ static int smp_fetch_const_meth(const struct arg 
*args, struct sample *smp, cons
>   return 1;
>  }
>  
> +// Generate a RFC4122 UUID (default is v4 = fully random)
> +static int
> +smp_fetch_uuid(const struct arg *args, struct sample *smp, const char 
*kw, void *private)
> +{
> +if (!args[0].data.sint || args[0].data.sint == 4) {
> +uint32_t rnd[4] = { 0, 0, 0, 0 };
> +uint64_t last = 0;
> +int byte = 0;
> +uint8_t bits = 0;
> +unsigned int rand_max_bits = my_flsl(RAND_MAX);
> +
> +while (byte < 4) {
> +while (bits < 32) {
> +last |= (uint64_t)random() << bits;
> +bits += rand_max_bits;
> +}
> +rnd[byte++] = last;
> +last >>= 32u;
> +bits  -= 32;
> +}
> +
> +chunk_printf(, "%8.8x-%4.4x-4%3.3x-%4.4x-%12.12llx",
 ^
Instead of hardcoding the 4 here (it's easy to miss within all the
digits for the format specifiers) I suggest to instead "force" the bits
to match the version within the random values using bit operations.

> + rnd[0], // first 32 bits
> + rnd[1] % 65536, // getting fist 16 bits
 ^ 
Hexadecimal notation and a bit mask would capture the intent better:
rand[1] & 0x. The comment is redundant then.

Typo in "first".

> + (rnd[1] >> 16u) % 4096,  // getting bits 17 - 28 
(12 bits), leaving out bits 29 - 32

With the suggestion regarding the version number this turns into:

((rand[1] >> 16u) & 0xFFF) | 0x4000 // the highest 4 bits indicate the
UUID version

> + (rnd[2] % 16384) + 32768,  // getting first 14 
bits, setting the first two bits to 1 and 0...

I'd do (rnd[2] & 0x3FFF) | 0x8000. // the highest 2 bits indicate the
UUID variant (10)

> + (*(((uint64_t*) rnd) + 1) >> 14u) % 281474976710656 
// getting 48 bits (last 18 of rnd[2] + first 30 of rnd[3], 281474976710656 = 
2^48)

Instead of casting this to a 64 bit value I guess it's easier to
understand if you add the remaining 48 bits in two steps, the remaining
18 of the 3rd byte and the 30 bits of the 4th byte.

> +);
> +
> +smp->data.type = SMP_T_STR;
> +smp->flags = SMP_F_VOL_TEST | SMP_F_MAY_CHANGE;;
 ^^
Duplicate semicolon.

> +smp->data.u.str = trash;
> +
> +return 1;
> +}
> +
> +// more implementations of other uuid formats possible here
> +return 0;
> +
> +}
> +
>  /* Note: must not be declared  as its list will be overwritten.
>   * Note: fetches that may return multiple types must be declared as the 
lowest
>   * common denominator, the type that can be casted into all other ones. 
For
> @@ -3214,6 +3255,7 @@ static struct sample_fetch_kw_list smp_kws = {ILH, {
>   { 

Re: RFC uuid for log-format

[Tim Düsterhus]

Luca,

(not an HAProxy maintainer)

please have a look at the CONTRIBUTING file regarding the commit message
format: https://github.com/haproxy/haproxy/blob/master/CONTRIBUTING

You are missing the severity indicator and the affected subsystem. It
probably should be `MINOR: sample:`.

> diff --git a/doc/configuration.txt b/doc/configuration.txt
> index ba5a8b360..e22557134 100644
> --- a/doc/configuration.txt
> +++ b/doc/configuration.txt
> @@ -14391,6 +14391,11 @@ thread : integer
>the function, between 0 and (global.nbthread-1). This is useful for logging
>and debugging purposes.
>  
> +uuid([]) : string
> +  Returns a random UUID, following the RFC4122 standard. If the version is 
> not
   ^^
I'd remove the random here. UUID v3 while not currently supported can
hardly be called "random". UUID in itself already implies "unique".

> +  specified, a UUID version 4 (fully random) is returned.
> +  Currently, only version 4 is supported.
> +
>  var() : undefined
>Returns a variable with the stored type. If the variable is not set, the
>sample fetch fails. The name of the variable starts with an indication
> diff --git a/src/sample.c b/src/sample.c
> index 6327b6b08..6caa440b7 100644
> --- a/src/sample.c
> +++ b/src/sample.c
> @@ -3196,6 +3196,47 @@ static int smp_fetch_const_meth(const struct arg 
> *args, struct sample *smp, cons
>   return 1;
>  }
>  
> +// Generate a RFC4122 UUID (default is v4 = fully random)
> +static int
> +smp_fetch_uuid(const struct arg *args, struct sample *smp, const char *kw, 
> void *private)
> +{
> +if (!args[0].data.sint || args[0].data.sint == 4) {
> +uint32_t rnd[4] = { 0, 0, 0, 0 };
> +uint64_t last = 0;
> +int byte = 0;
> +uint8_t bits = 0;
> +unsigned int rand_max_bits = my_flsl(RAND_MAX);
> +
> +while (byte < 4) {
> +while (bits < 32) {
> +last |= (uint64_t)random() << bits;
> +bits += rand_max_bits;
> +}
> +rnd[byte++] = last;
> +last >>= 32u;
> +bits  -= 32;
> +}
> +
> +chunk_printf(, "%8.8x-%4.4x-4%3.3x-%4.4x-%12.12llx",
 ^
Instead of hardcoding the 4 here (it's easy to miss within all the
digits for the format specifiers) I suggest to instead "force" the bits
to match the version within the random values using bit operations.

> + rnd[0], // first 32 bits
> + rnd[1] % 65536, // getting fist 16 bits
 ^ 
Hexadecimal notation and a bit mask would capture the intent better:
rand[1] & 0x. The comment is redundant then.

Typo in "first".

> + (rnd[1] >> 16u) % 4096,  // getting bits 17 - 28 (12 
> bits), leaving out bits 29 - 32

With the suggestion regarding the version number this turns into:

((rand[1] >> 16u) & 0xFFF) | 0x4000 // the highest 4 bits indicate the
UUID version

> + (rnd[2] % 16384) + 32768,  // getting first 14 bits, 
> setting the first two bits to 1 and 0...

I'd do (rnd[2] & 0x3FFF) | 0x8000. // the highest 2 bits indicate the
UUID variant (10)

> + (*(((uint64_t*) rnd) + 1) >> 14u) % 281474976710656 // 
> getting 48 bits (last 18 of rnd[2] + first 30 of rnd[3], 281474976710656 = 
> 2^48)

Instead of casting this to a 64 bit value I guess it's easier to
understand if you add the remaining 48 bits in two steps, the remaining
18 of the 3rd byte and the 30 bits of the 4th byte.

> +);
> +
> +smp->data.type = SMP_T_STR;
> +smp->flags = SMP_F_VOL_TEST | SMP_F_MAY_CHANGE;;
 ^^
Duplicate semicolon.

> +smp->data.u.str = trash;
> +
> +return 1;
> +}
> +
> +// more implementations of other uuid formats possible here
> +return 0;
> +
> +}
> +
>  /* Note: must not be declared  as its list will be overwritten.
>   * Note: fetches that may return multiple types must be declared as the 
> lowest
>   * common denominator, the type that can be casted into all other ones. For
> @@ -3214,6 +3255,7 @@ static struct sample_fetch_kw_list smp_kws = {ILH, {
>   { "rand", smp_fetch_rand,  ARG1(0,SINT), NULL, SMP_T_SINT, 
> SMP_USE_INTRN },
>   { "stopping", smp_fetch_stopping, 0, NULL, SMP_T_BOOL, 
> SMP_USE_INTRN },
>   { "stopping", smp_fetch_stopping, 0, NULL, SMP_T_BOOL, 
> SMP_USE_INTRN },
> + { "uuid", smp_fetch_uuid,  ARG1(0, SINT),  NULL, SMP_T_STR, 
> SMP_USE_INTRN },
>  
>   { "cpu_calls",smp_fetch_cpu_calls,  0,   NULL, SMP_T_SINT, 
> SMP_USE_INTRN },
>   { "cpu_ns_avg",   smp_fetch_cpu_ns_avg, 0,   NULL, SMP_T_SINT, 
> SMP_USE_INTRN },
> -- 
> 2.23.0

[PR] Add UUID fetch

[PR Bot]

Dear list!

Author: Luca Schimweg 
Number of patches: 3

This is an automated relay of the Github pull request:
   Add UUID fetch

Patch title(s): 
   Add UUID Generator
   UUID: Use suggestions from mailing list
   Add documentation for UUID-Fetch

Link:
   https://github.com/haproxy/haproxy/pull/271

Edit locally:
   wget https://github.com/haproxy/haproxy/pull/271.patch && vi 271.patch

Apply locally:
   curl https://github.com/haproxy/haproxy/pull/271.patch | git am -

Description:
   We discussed this in the mailing list

Instructions:
   This github pull request will be closed automatically; patch should be
   reviewed on the haproxy mailing list (haproxy@formilux.org). Everyone is
   invited to comment, even the patch's author. Please keep the author and
   list CCed in replies. Please note that in absence of any response this
   pull request will be lost.

[PATCH] MINOR: connection: add "set-authority" and "authority" sample fetch

[Emmanuel Hocdet]

Hi,

Included, my first proposal for « set-authority » action, to set 
custom "authority" sample  fetch.

Use case could be to use « sni authority » in server line.
For "proxy-v2-options authority », authority is pick from custom
authority (« set-authority »), ppv2 authority or ssl_fc_sni.
Sample fetch « authority » could be do the same, but i don’t
know if it’s a good idea.

++
Manu




0001-MINOR-connection-add-set-authority-and-authority-sam.patch
Description: Binary data

Re: RFC uuid for log-format

[Schimweg, Luca]

Hey,

I just integrated your change requests into my fork and created a PR: 
https://github.com/haproxy/haproxy/pull/271, would be happy if anyone could 
give me some feedback!

Best regards,
Luca

On 06.09.19, 11:41, "Willy Tarreau"  wrote:

On Fri, Sep 06, 2019 at 11:33:32AM +0200, Geoff Simmons wrote:
> On 9/6/19 11:22, Willy Tarreau wrote:
> > 
> > We can then have 4 being the only one implemented for now, we can then
> > set it to the default one if unspecified, but I really want to support
> > such a parameter so that it's easy to extend the form once a new one
> > arrives.
> 
> Still picking nits (although I think this is a good plan) -- if uuid()
> takes a parameter, and version 4 is the only one that's implemented,
> then uuid(1) or uuid(3) etc throws an error?

Yes, that's just the argument parser which will return this.

Willy

Re: Haproxy timeouts and returns NULL as response

[Jarno Huuskonen]

Hi,

On Fri, Aug 30, Santhosh Kumar wrote:
>  We have a client-haproxy-server setup like this https://imgur.com/bxV3BA9,
> we use apache and jetty httpclient for http1.1 and http2 requests
> respectively. Our http request will take 2 secs to 10 mins for processing a
> request depends on the request type. Some of the requests returns null as
> response(whereas, the request is received and processed succesfully by
> server which I can verify via server logs) which triggers
> *org.apache.http.MalformedChunkCodingException:
> Unexpected content at the end of chunk *on the client side and this problem
> is happening with http1.1 requests for a speicifc type of requests, I tried
> tweaking timeouts and tried to fix this but its doesnt help me and timeout
> does not have a pattern. Each request timeout is having diff timeout values
> like 5 secs, 12secs, 27 secs or even 45secs. This error dissapears if I
> remove haproxy and connect directly yo server. my config file as follows,

What version of haproxy are you using (haproxy -vv) ?
If version 2.x have you tried with "no option http-use-htx" and latest
2.0.x version ?

Do you get haproxy logs for these failed req/responses ?

-Jarno

-- 
Jarno Huuskonen