Re: [2.1.1] http-request replace-uri does not work

2019-12-16 Thread Willy Tarreau
On Tue, Dec 17, 2019 at 08:41:55AM +0100, Julien Pivotto wrote:
> On 17 Dec 06:58, Willy Tarreau wrote:
> > On Tue, Dec 17, 2019 at 06:08:56AM +0100, Willy Tarreau wrote:
> > > But now I'm starting to suspect that most of the problem comes from the
> > > fact that people who used to rely on regex in the past will not as easily
> > > perform their rewrites using set-path as they would using a replace rule
> > > which is very similar to the old set. So probably we'd need to introduce
> > > a "replace-path" action and suggest it in the warning emitted for reqrep.
> > > 
> > > I think it is important that we properly address such needs and am
> > > willing to backport anything like this to 2.1 to ease the transition if
> > > that's the best solution.
> > 
> > What about this ? It does exactly what's needed for me. It's self-contained
> > enough that we could get it backported to 2.1 and maybe even to 2.0 (though
> > it would require some adaptations to legacy mode there).
> > 
> > Willy
> 
> I am in favour of replace-path.
> Should we have a replace-domain or so for the first part of the abs url?

Technically speaking it's called the authority. And since we take great
care to keep it synchronized with the Host header field, actually one
can simply decide to adjust the Host header to adjust this part. I'm
just realizing that we should mention this in the doc as well.

Willy



Re: [2.1.1] http-request replace-uri does not work

2019-12-16 Thread Julien Pivotto
On 17 Dec 06:58, Willy Tarreau wrote:
> On Tue, Dec 17, 2019 at 06:08:56AM +0100, Willy Tarreau wrote:
> > But now I'm starting to suspect that most of the problem comes from the
> > fact that people who used to rely on regex in the past will not as easily
> > perform their rewrites using set-path as they would using a replace rule
> > which is very similar to the old set. So probably we'd need to introduce
> > a "replace-path" action and suggest it in the warning emitted for reqrep.
> > 
> > I think it is important that we properly address such needs and am
> > willing to backport anything like this to 2.1 to ease the transition if
> > that's the best solution.
> 
> What about this ? It does exactly what's needed for me. It's self-contained
> enough that we could get it backported to 2.1 and maybe even to 2.0 (though
> it would require some adaptations to legacy mode there).
> 
> Willy

I am in favour of replace-path.
Should we have a replace-domain or so for the first part of the abs url?

-- 
 (o-Julien Pivotto
 //\Open-Source Consultant
 V_/_   Inuits - https://www.inuits.eu


signature.asc
Description: PGP signature


Re: [2.1.1] http-request replace-uri does not work

2019-12-16 Thread Willy Tarreau
On Tue, Dec 17, 2019 at 06:58:11AM +0100, Willy Tarreau wrote:
> On Tue, Dec 17, 2019 at 06:08:56AM +0100, Willy Tarreau wrote:
> > But now I'm starting to suspect that most of the problem comes from the
> > fact that people who used to rely on regex in the past will not as easily
> > perform their rewrites using set-path as they would using a replace rule
> > which is very similar to the old set. So probably we'd need to introduce
> > a "replace-path" action and suggest it in the warning emitted for reqrep.
> > 
> > I think it is important that we properly address such needs and am
> > willing to backport anything like this to 2.1 to ease the transition if
> > that's the best solution.
> 
> What about this ? It does exactly what's needed for me. It's self-contained
> enough that we could get it backported to 2.1 and maybe even to 2.0 (though
> it would require some adaptations to legacy mode there).

Actually a slightly cleaner one that should even be backportable to 2.0.
We could trivially extend it to also support the query string though I'm
not convinced at all that it would make sense to have a regex-based
"replace-query".

Willy
>From 126c472884112caa9a70ac6d2fea71062f36c5cb Mon Sep 17 00:00:00 2001
From: Willy Tarreau 
Date: Tue, 17 Dec 2019 06:52:51 +0100
Subject: MINOR: http: add a new "replace-path" action

This action is very similar to "replace-uri" except that it only acts on the
path component. This is assumed to better match users' expectations when they
used to rely on "replace-uri" in HTTP/1 because mostly origin forms were used
in H1 while mostly absolute URI form is used in H2, and their rules very often
start with a '/', and as such do not match.

It could help users to get this backported to 2.0 and 2.1.
---
 doc/configuration.txt | 27 ++-
 src/cfgparse-listen.c |  2 +-
 src/http_act.c| 20 +++-
 3 files changed, 42 insertions(+), 7 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index fdcdb04fae..3ba340c38d 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -4463,6 +4463,30 @@ http-request replace-header   

 # outputs:
 User-Agent: foo
 
+http-request replace-path  
+   [ { if | unless }  ]
+
+  This works like "replace-header" except that it works on the request's path
+  component instead of a header. The path component starts at the first '/'
+  after an optional scheme+authority. It does contain the query string if any
+  is present. The replacement does not modify the scheme nor authority.
+
+  It is worth noting that regular expressions may be more expensive to evaluate
+  than certain ACLs, so rare replacements may benefit from a condition to avoid
+  performing the evaluation at all if it does not match.
+
+  Example:
+# prefix /foo : turn /bar?q=1 into /foo/bar?q=1 :
+http-request replace-path (.*) /foo\1
+
+# suffix /foo : turn /bar?q=1 into /bar/foo?q=1 :
+http-request replace-path ([^?]*)(\?(.*))? \1/foo\2
+
+# strip /foo : turn /foo/bar?q=1 into /bar?q=1
+http-request replace-path /foo/(.*) /\1
+# or more efficient if only some requests match :
+http-request replace-path /foo/(.*) /\1 if { url_beg /foo/ }
+
 http-request replace-uri  
[ { if | unless }  ]
 
@@ -4484,7 +4508,8 @@ http-request replace-uri  
   with HTTP/2, clients are encouraged to send absolute URIs only, which look
   like the ones HTTP/1 clients use to talk to proxies. Such partial replace-uri
   rules may then fail in HTTP/2 when they work in HTTP/1. Either the rules need
-  to be adapted to optionally match a scheme and authority.
+  to be adapted to optionally match a scheme and authority, or replace-path
+  should be used.
 
   Example:
 # rewrite all "http" absolute requests to "https":
diff --git a/src/cfgparse-listen.c b/src/cfgparse-listen.c
index 507e071734..9975e46876 100644
--- a/src/cfgparse-listen.c
+++ b/src/cfgparse-listen.c
@@ -3694,7 +3694,7 @@ stats_error_parsing:
}
else if (!strcmp(args[0], "cliexp") || !strcmp(args[0], "reqrep")) {  
/* replace request header from a regex */
ha_alert("parsing [%s:%d] : The '%s' directive is not supported 
anymore since HAProxy 2.1. "
-"Use 'http-request replace-uri' and 'http-request 
replace-header' instead.\n",
+"Use 'http-request replace-path', 'http-request 
replace-uri' or 'http-request replace-header' instead.\n",
 file, linenum, args[0]);
err_code |= ERR_ALERT | ERR_FATAL;
goto out;
diff --git a/src/http_act.c b/src/http_act.c
index d6015d3624..c8d9220feb 100644
--- a/src/http_act.c
+++ b/src/http_act.c
@@ -133,6 +133,8 @@ static enum act_parse_ret parse_set_req_line(const char 
**args, int *orig_arg, s
  * .arg.act.p[]. It builds a string in the trash from the format string
  * previously filled by function parse_replace_uri() 

Re: [2.1.1] http-request replace-uri does not work

2019-12-16 Thread Willy Tarreau
On Tue, Dec 17, 2019 at 06:08:56AM +0100, Willy Tarreau wrote:
> But now I'm starting to suspect that most of the problem comes from the
> fact that people who used to rely on regex in the past will not as easily
> perform their rewrites using set-path as they would using a replace rule
> which is very similar to the old set. So probably we'd need to introduce
> a "replace-path" action and suggest it in the warning emitted for reqrep.
> 
> I think it is important that we properly address such needs and am
> willing to backport anything like this to 2.1 to ease the transition if
> that's the best solution.

What about this ? It does exactly what's needed for me. It's self-contained
enough that we could get it backported to 2.1 and maybe even to 2.0 (though
it would require some adaptations to legacy mode there).

Willy
>From 290a8473a9a8d65685a6a3c4922a581eaa7c3377 Mon Sep 17 00:00:00 2001
From: Willy Tarreau 
Date: Tue, 17 Dec 2019 06:52:51 +0100
Subject: MINOR: http: add a new "replace-path" action

This action is very similar to "replace-uri" except that it only acts on the
path component. This is assumed to better match users' expectations when they
used to rely on "replace-uri" in HTTP/1 because mostly origin forms were used
in H1 while mostly absolute URI form is used in H2, and their rules very often
start with a '/', and as such do not match.

It could help users to get this backported to 2.0 and 2.1.
---
 doc/configuration.txt | 27 ++-
 src/cfgparse-listen.c |  2 +-
 src/http_act.c| 21 -
 3 files changed, 43 insertions(+), 7 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index fdcdb04fae..3ba340c38d 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -4463,6 +4463,30 @@ http-request replace-header   

 # outputs:
 User-Agent: foo
 
+http-request replace-path  
+   [ { if | unless }  ]
+
+  This works like "replace-header" except that it works on the request's path
+  component instead of a header. The path component starts at the first '/'
+  after an optional scheme+authority. It does contain the query string if any
+  is present. The replacement does not modify the scheme nor authority.
+
+  It is worth noting that regular expressions may be more expensive to evaluate
+  than certain ACLs, so rare replacements may benefit from a condition to avoid
+  performing the evaluation at all if it does not match.
+
+  Example:
+# prefix /foo : turn /bar?q=1 into /foo/bar?q=1 :
+http-request replace-path (.*) /foo\1
+
+# suffix /foo : turn /bar?q=1 into /bar/foo?q=1 :
+http-request replace-path ([^?]*)(\?(.*))? \1/foo\2
+
+# strip /foo : turn /foo/bar?q=1 into /bar?q=1
+http-request replace-path /foo/(.*) /\1
+# or more efficient if only some requests match :
+http-request replace-path /foo/(.*) /\1 if { url_beg /foo/ }
+
 http-request replace-uri  
[ { if | unless }  ]
 
@@ -4484,7 +4508,8 @@ http-request replace-uri  
   with HTTP/2, clients are encouraged to send absolute URIs only, which look
   like the ones HTTP/1 clients use to talk to proxies. Such partial replace-uri
   rules may then fail in HTTP/2 when they work in HTTP/1. Either the rules need
-  to be adapted to optionally match a scheme and authority.
+  to be adapted to optionally match a scheme and authority, or replace-path
+  should be used.
 
   Example:
 # rewrite all "http" absolute requests to "https":
diff --git a/src/cfgparse-listen.c b/src/cfgparse-listen.c
index 507e071734..9975e46876 100644
--- a/src/cfgparse-listen.c
+++ b/src/cfgparse-listen.c
@@ -3694,7 +3694,7 @@ stats_error_parsing:
}
else if (!strcmp(args[0], "cliexp") || !strcmp(args[0], "reqrep")) {  
/* replace request header from a regex */
ha_alert("parsing [%s:%d] : The '%s' directive is not supported 
anymore since HAProxy 2.1. "
-"Use 'http-request replace-uri' and 'http-request 
replace-header' instead.\n",
+"Use 'http-request replace-path', 'http-request 
replace-uri' or 'http-request replace-header' instead.\n",
 file, linenum, args[0]);
err_code |= ERR_ALERT | ERR_FATAL;
goto out;
diff --git a/src/http_act.c b/src/http_act.c
index d6015d3624..61200ba368 100644
--- a/src/http_act.c
+++ b/src/http_act.c
@@ -142,6 +142,7 @@ static enum act_return http_action_replace_uri(struct 
act_rule *rule, struct pro
enum act_return ret = ACT_RET_ERR;
struct buffer *replace, *output;
struct ist uri;
+   int action;
int len;
 
replace = alloc_trash_chunk();
@@ -149,6 +150,13 @@ static enum act_return http_action_replace_uri(struct 
act_rule *rule, struct pro
if (!replace || !output)
goto leave;
uri = htx_sl_req_uri(http_get_stline(htxbuf(>req.buf)));
+   action = 3; // replace uri

Re: [2.1.1] http-request replace-uri does not work

2019-12-16 Thread Willy Tarreau
Hi guys,

On Mon, Dec 16, 2019 at 11:01:19PM +0100, Cyril Bonté wrote:
> Hi Willy,
> 
> Le 16/12/2019 à 22:06, Artur a écrit :
> > > > [...]
> > > > URLs like https://q.d/PPDSlide/testfile are correctly rewritten to
> > > > https://q.d/p3/PPDSlide/testfile and forwarded to the backend.
> > > > 
> > > > Once I switched to 2.1.1, haproxy no longer rewrites the URI and the
> > > > URIs remains unchanged while forwarded to the backend. I had to
> > > > downgrade to have the usual behaviour.
> > > > 
> > > > Is it a bug or something changed in normal haproxy behaviour with 2.1
> > > > release ?
> > > 
> > > I can confirm the issue.
> > > 
> > > It seems to happen with h2 requests only, since commit #30ee1efe67.
> > > haproxy normalizes the URI but replace-uri doesn't take into account
> > > this information. The fix should be easy for replace-uri (If someone
> > > wants to work on it, I won't have time this week).
> > > 
> > > http://git.haproxy.org/?p=haproxy-2.1.git;a=commit;h=30ee1efe67
> 
> I'm not 100% sure it's the right approach to fix the issue. Can you check if
> this patch may fix the issue in all conditions ?
> 
> From what I've observed, it seems we can rely on the HTX_SL_F_NORMALIZED_URI
> flag to detect if the URI was normalized by haproxy and in that case, we
> should start at the path instead of the URI.

In fact it's not a bug and is the intended behavior. The rule in Artur's
config only matches origin URIs, not absolute ones, which most H2 clients
use.

In HTTP/1, there are two ways to send a request URI:
  origin form:GET /path/to/resource HTTP/1.1
  absolute form:  GET http://server.domain.tld/path/to/resource HTTP/1.1

Clients mostly use the origin form with servers, and mostly use the
absolute form with proxies, and even then, the mapping is 99.99% and
not 100% in either case. It's worth noting that Artur's rule already
does not work with the absolute form, it only handles the origin form.

In H2 you have the same distinction, except that clients were initially
encouraged to use the absolute form only. The first non-HTX implementation
used to implement only a subset of H2 and to map H2 to H1 using the Host
header field, transforming incoming absolute URIs to origin URIs. This
broke gRPC and end-to-end H2 because the scheme was lost. This also
prevented such requests from being passed to proxies. So starting with
2.1 we now respect the format used by the client.

It turns out that there seems to exist a number of configs in field which
are already broken with H1 but which work by pure luck as they only handle
the origin form. Arguably, many of these have evolved over the ages because
initially they were designed to use the old reqrep matching which was very
painful to deal with, and writing extra config just to handle an almost
always absent scheme was not worth it. Nowadays we have multiple actions
available depending on what we want to handle:

  - set-path to set only the path component
  - set-uri to set the whole URI
  - set-query to set the query string only
  - replace-uri to replace a part of the URI using parts from it

In theory in order to perform some path rewrites, one would only need
set-path. In Artur's case, it should be as simple as :

http-request set-path /p3%[path]

But now I'm starting to suspect that most of the problem comes from the
fact that people who used to rely on regex in the past will not as easily
perform their rewrites using set-path as they would using a replace rule
which is very similar to the old set. So probably we'd need to introduce
a "replace-path" action and suggest it in the warning emitted for reqrep.

I think it is important that we properly address such needs and am
willing to backport anything like this to 2.1 to ease the transition if
that's the best solution.

Please advise,
Willy



Re: CORS support

2019-12-16 Thread Willy Tarreau
Hello Alex,

On Mon, Dec 16, 2019 at 01:22:42PM -0500, Alex Evonosky wrote:
> Hello Haproxy group-
> 
> migrating from haproxy 2.0 to 2.1 and noticed some directives changed:
> 
> === 2.0.10 ===
> 
> capture request header origin len 128
> http-response add-header Access-Control-Allow-Origin %[capture.req.hdr(0)]
> if { capture.req.hdr(0) -m end aiqwest.com }
> http-response add-header Access-Control-Allow-Headers:\ Origin,\
> X-Requested-With,\ Content-Type,\ Accept if { capture.req.hdr(0) -m found }
> 
> 
> === 2.1 ===
> 
> capture request header origin len 128
> http-response add-header Access-Control-Allow-Origin %[capture.req.hdr(0)]
> if { capture.req.hdr(0) -m end aiqwest.com }
> rspadd add-header Access-Control-Allow-Headers:\ Origin,\
> X-Requested-With,\ Content-Type,\ Accept if { capture.req.hdr(0) -m found }
> 
> 
> getting:
> 
> The 'rspadd' directive is not supported anymore since HAProxy 2.1. Use
> 'http-response add-header' instead.
> 
> when adding the said above, getting an error:
> 
> 'http-response add-header' expects exactly 2 arguments.

You almost had it right, actually it's exactly the same as in your
first rule: first arg is the header name, second one is the value:

   http-response add-header Access-Control-Allow-Headers Origin,\ 
X-Requested-With,\ Content-Type,\ Accept if { capture.req.hdr(0) -m found }

Please let us know if anything is not clear enough in the message or
in the doc so that we can improve it.

Willy



Re: ModSecurity testing

2019-12-16 Thread Igor Cicimov
Hi Joao,

On Sat, Dec 14, 2019 at 11:30 PM Joao Morais  wrote:

>
>
> > Em 13 de dez de 2019, à(s) 10:09, Christopher Faulet <
> cfau...@haproxy.com> escreveu:
> >
> > Le 10/12/2019 à 05:24, Igor Cicimov a écrit :
> >>
> >> Testing with Haproxy 2.0.10 but same result with 1.8.23. The versions
> of ModSecurity is 2.9.2 and the OWASP rules v3.0.2
> >> What am I doing wrong? Can anyone provide a request that should confirm
> if the module is working or not from or share the experience from their own
> setup?
> >
> > Hi Igor,
> >
> > First of all, I don't know how the modsecurity agent really work. But
> I'm surprised to see it returns -101. In the code, -1, 0 or an HTTP status
> code is expected. And only 0 or the HTTP status code is returned to
> HAProxy. I don't know if -101 is a valid return value from modsecurity
> point of view. But it is not from the agent one.
> >
> > Then, You don't have an error 403 because the variable txn.modsec.code
> is negative, so the deny http-request rule is never triggered. So, I guess
> your error 400 comes from your webserver. You can enabled HTTP log to have
> more information.
> >
> > Finally, I notice some requests to the SPOA agent seems to have failed.
> The variable is not set (- in the logs). You can try to enable SPOE logs in
> your SPOE engine configuration. Take a look at the SPOE documentation
> (doc/SPOE.txt) for more information.
>
>
> Hi, perhaps this thread helps:
>
> https://www.mail-archive.com/haproxy@formilux.org/msg30061.html
>
> And perhaps this building of ModSecurity SPOA will also help:
>
>
> https://github.com/jcmoraisjr/modsecurity-spoa/blob/v0.5/rootfs/Dockerfile
>
> ~jm
>

First thanks for your reply, I've been following your work on
haproxy-ingress for Kubernetes (where I can see you have incorporated
ModSecurity) and your input is certainly appreciated on this matter.

I had some time today to quickly run the test again after enabling the log
for SPOE:

[modsecurity]
spoe-agent modsecurity-agent
log global
messagescheck-request
option  var-prefix  modsec
option  continue-on-error
timeout hello   100ms
timeout idle30s
timeout processing  1s
use-backend spoe-modsecurity

spoe-message check-request
args unique-id method path query req.ver req.hdrs_bin req.body_size
req.body
event on-frontend-http-request

and can see that the empty values coming from SPOE are legit and are due to
SEARCH method that is not allowed by Haproxy:

Dec 17 00:40:01 ip-172-31-17-121 haproxy[17468]:
1345:my-front.accept(000a)=0016 from [127.0.0.1:53206] ALPN=
Dec 17 00:40:01 ip-172-31-17-121 haproxy[17468]:
1345:my-front.clireq[0016:]: SEARCH / HTTP/1.1
Dec 17 00:40:01 ip-172-31-17-121 haproxy[17468]:
1345:my-front.clihdr[0016:]: user-agent: Mozilla/5.0 (Windows
NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko)
Chrome/74.0.3729.169 Safari/537.36
Dec 17 00:40:01 ip-172-31-17-121 haproxy[17468]:
1345:my-front.clihdr[0016:]: content-type:
application/x-www-form-urlencoded
Dec 17 00:40:01 ip-172-31-17-121 haproxy[17468]:
1345:my-front.clihdr[0016:]: content-length: 1
Dec 17 00:40:01 ip-172-31-17-121 haproxy[17468]:
1345:my-front.clihdr[0016:]: host: localhost:9080
Dec 17 00:40:01 ip-172-31-17-121 haproxy[17468]:
1343:my-front.srvcls[0017:0013]
Dec 17 00:40:01 ip-172-31-17-121 haproxy[17468]:
1343:my-front.clicls[0017:0013]
Dec 17 00:40:01 ip-172-31-17-121 haproxy[17468]:
1343:my-front.closed[0017:0013]
Dec 17 00:40:01 ip-172-31-17-121 haproxy[17468]:
1345:my-front.srvrep[0016:0013]: HTTP/1.1 405 Method Not Allowed
Dec 17 00:40:01 ip-172-31-17-121 haproxy[21508]: SPOE: [modsecurity-agent]
 sid=4933 st=0 0/0/0/0/0 4/4 0/0 0/2768
Dec 17 00:40:01 ip-172-31-17-121 haproxy[21508]: The txn.modsec.code is: -
Dec 17 00:40:01 ip-172-31-17-121 haproxy[17468]:
1345:my-front.srvhdr[0016:0013]: server: ecstatic-3.3.2
Dec 17 00:40:01 ip-172-31-17-121 haproxy[21508]: The txn.modsec.code is: -
Dec 17 00:40:01 ip-172-31-17-121 haproxy[21508]: SPOE: [modsecurity-agent]
 sid=4935 st=0 0/0/0/0/0 4/4 0/0 0/2769
Dec 17 00:40:01 ip-172-31-17-121 haproxy[17468]:
1345:my-front.srvhdr[0016:0013]: date: Tue, 17 Dec 2019 00:40:01 GMT
Dec 17 00:40:01 ip-172-31-17-121 haproxy[17468]:
1345:my-front.srvhdr[0016:0013]: content-length: 0
Dec 17 00:40:01 ip-172-31-17-121 haproxy[17468]:
1345:my-front.srvcls[0016:0013]
Dec 17 00:40:01 ip-172-31-17-121 haproxy[17468]:
1345:my-front.clicls[0016:0013]
Dec 17 00:40:01 ip-172-31-17-121 haproxy[17468]:
1345:my-front.closed[0016:0013]

and some other methods like TRACK, PROPFIND, DEBUG that NIkto tries out.

Apart from that, I'm still equally stumped as I was before about the rules
not working as expected. For example this request:

$ curl -I -H "User-Agent: nikto" http://localhost:9080/
HTTP/1.1 200 OK
server: ecstatic-3.3.2
cache-control: max-age=3600
last-modified: 

Re: PROXY protocol and check port

2019-12-16 Thread Igor Cicimov
Hi,

On Tue, Dec 17, 2019 at 2:55 AM Olivier D  wrote:

> Hello,
>
> I found what was wrong : I was using "load-server-state-from-file" and
> previous config file was using port 80 as server port.
> It seems using this instruction loads previous server state but also
> previous srv_port.
> Is this an expected behaviour ?
>

Yes, basically it is your responsibility to dump the current state into the
file otherwise you'll gate outdated data as you noticed. For example I add:

ExecReload=/bin/echo "show servers state" | /usr/bin/socat stdio
/run/haproxy/admin.sock > /var/lib/haproxy/state

in the systemd service file.


Re: [2.1.1] http-request replace-uri does not work

2019-12-16 Thread Cyril Bonté

Hi Willy,

Le 16/12/2019 à 22:06, Artur a écrit :

[...]
URLs like https://q.d/PPDSlide/testfile are correctly rewritten to
https://q.d/p3/PPDSlide/testfile and forwarded to the backend.

Once I switched to 2.1.1, haproxy no longer rewrites the URI and the
URIs remains unchanged while forwarded to the backend. I had to
downgrade to have the usual behaviour.

Is it a bug or something changed in normal haproxy behaviour with 2.1
release ?


I can confirm the issue.

It seems to happen with h2 requests only, since commit #30ee1efe67.
haproxy normalizes the URI but replace-uri doesn't take into account
this information. The fix should be easy for replace-uri (If someone
wants to work on it, I won't have time this week).

http://git.haproxy.org/?p=haproxy-2.1.git;a=commit;h=30ee1efe67


I'm not 100% sure it's the right approach to fix the issue. Can you 
check if this patch may fix the issue in all conditions ?


From what I've observed, it seems we can rely on the 
HTX_SL_F_NORMALIZED_URI flag to detect if the URI was normalized by 
haproxy and in that case, we should start at the path instead of the URI.


--
Cyril Bonté
diff --git a/src/http_act.c b/src/http_act.c
index d6015d362..797d75275 100644
--- a/src/http_act.c
+++ b/src/http_act.c
@@ -141,6 +141,7 @@ static enum act_return http_action_replace_uri(struct act_rule *rule, struct pro
 {
 	enum act_return ret = ACT_RET_ERR;
 	struct buffer *replace, *output;
+	struct htx_sl *sl;
 	struct ist uri;
 	int len;
 
@@ -148,7 +149,12 @@ static enum act_return http_action_replace_uri(struct act_rule *rule, struct pro
 	output  = alloc_trash_chunk();
 	if (!replace || !output)
 		goto leave;
-	uri = htx_sl_req_uri(http_get_stline(htxbuf(>req.buf)));
+
+	sl = http_get_stline(htxbuf(>req.buf));
+	uri = htx_sl_req_uri(sl);
+	if (sl->flags & HTX_SL_F_NORMALIZED_URI)
+		uri = http_get_path(uri);
+
 	if (!regex_exec_match2(rule->arg.act.p[1], uri.ptr, uri.len, MAX_MATCH, pmatch, 0))
 		goto leave;
 


Re: HAProxy 2.0.10 and 2.1.0 RPM's

2019-12-16 Thread Julien Pivotto
On 16 Dec 15:00, Ryan O'Hara wrote:
> On Tue, Nov 26, 2019 at 2:40 PM Russell Eason  wrote:
> 
> > Hello,
> >
> > Fedora upstream added it
> > https://src.fedoraproject.org/rpms/haproxy/c/45c57ba71174f308a5f59569bac0598bb31ef767
> > , and can be seen as far back as F24 here
> > https://src.fedoraproject.org/rpms/haproxy/blob/f24/f/haproxy.spec . LUA
> > support is in the RHEL 8 version of HAProxy, but not in 7 (yet?).
> >
> 
> Sorry for the late reply. I believe the reason that RHEL7 lacks LUA support
> is because the required version of LUA is not available in RHEL7. Enabling
> LUA support in haproxy at build time is easy, but if the underlying bits
> aren't available, there is nothing I can do. Cheers.
> 
> Ryan

Alternatively I have updated
https://copr.fedorainfracloud.org/coprs/roidelapluie/haproxy/
to 2.1.1

In ELEL7 it uses
https://copr.fedorainfracloud.org/coprs/roidelapluie/lua/
which is the LUA for RHEL8 compiled for RHEL7.

We statically compile it within haproxy.

-- 
 (o-Julien Pivotto
 //\Open-Source Consultant
 V_/_   Inuits - https://www.inuits.eu


signature.asc
Description: PGP signature


haproxy 2.1 package for Debian 9 Stretch oldstable

2019-12-16 Thread Artur
Hello,

While checking for haproxy 2.1 package for Debian Stretch on
https://haproxy.debian.net/, I saw it wasn't available (yet ?).

Do you plan to build haproxy deb packages for this version of Debian,
it's still supported as oldstable for now ?

-- 
Best regards,
Artur




Re: HAProxy 2.0.10 and 2.1.0 RPM's

2019-12-16 Thread Ryan O'Hara
On Tue, Nov 26, 2019 at 9:20 PM Willy Tarreau  wrote:

>
> Indeed that looks good. We'll need to include Ryan in this discussion,
> he's the maintainer of the official RPMs for RHEL. I'm purposely not CCing
> him as I know he's very busy this week, but I sense that we're starting to
> see the light at the end of the tunnel here.
>

Indeed, I have been busy. Then on vacation. Then back in time for another
deadline. I'm always open to hear feedback on the RHEL and Fedora packages
(I maintain Fedora packages as well as RHEL). Thanks to Julien for creating
those copr builds. That makes my life so much easier!

A couple items about Fedora/RHEL packages. For Fedora, I try to be
responsive for CVEs, minor release rebases, and enabling features (eg.
Prometheus support). Lesser things like minor spec file changes usually get
a much lower priority. Note that we do not rebase to a new, major release
within a stable release. For example, if Fedora 31 has haproxy-2.0.x, it
will never have haproxy-2.1.x. That is what copr is for. For RHEL, it is
even more restrictive. I don't make the rules, just wanted to explain this
since I get a lot of email asking about these things. Cheers.

Ryan


Re: [PATCH] openssl-compat: Fix getm_ defines

2019-12-16 Thread Илья Шипицин
вт, 17 дек. 2019 г. в 00:55, Rosen Penev :

> On Mon, Dec 16, 2019 at 10:21 AM Илья Шипицин 
> wrote:
> >
> >
> >
> > пн, 16 дек. 2019 г. в 22:40, Rosen Penev :
> >>
> >> On Mon, Dec 16, 2019 at 4:49 AM Lukas Tribus  wrote:
> >> >
> >> > Hello Rosen,
> >> >
> >> > > пн, 16 дек. 2019 г. в 12:07, Rosen Penev :
> >> > >>
> >> > >> LIBRESSL_VERSION_NUMBER evaluates to 0 under OpenSSL, making the
> condition
> >> > >> always true. Check for the define before checking it.
> >> >
> >> > I cannot find this in the openssl sources, not in master and not in
> >> > the 1.1.1 branch. Please clarify where this is defined.
> >> Compile with -Wundef. Missing macros evaluate to 0.
> >
> >
> > I checked haproxy source, it does not use such compiler flag. Any reason
> for introducing it ?
> >
> > if we want to make it first class citizen, maybe we should add it to
> proper Makefile ? or to our CI ?
> >
> > assuming "undefined macros may ACCIDENTLY become equal to 0" scares me
> You serious? This is basic C. Undefined macros always evaluate to 0.
>
>
indeed, you're right. I checked both gcc and clang.
unfortunately, I neither learned nor used that area of C preprocessor
specification before.


> -Wundef only warns about it.
> >
> >>
> >> >
> >> > The SSL compatibility layer is already complex enough and needs
> >> > continuous adjustments, we need to understand the reason for changes
> >> > very well. Fast fixes are continually coming back to hunt us.
> >> >
> >> >
> >> > On Mon, 16 Dec 2019 at 08:19, Илья Шипицин 
> wrote:
> >> > > please have a look at https://github.com/haproxy/haproxy/issues/367
> (it still misses germ part, I tried things like you send, but reg-tests
> fail. do you have travis-ci passed ?)
> >> > > also, there's a patch already sent, Lukas Tribus promised to review
> it
> >> >
> >> > Yeah, this one fell through the cracks. Give me a few days to catch
> up.
> >> >
> >> > Thanks,
> >> > Lukas
>


Re: [2.1.1] http-request replace-uri does not work

2019-12-16 Thread Artur
Hello Cyril,

Thanks a lot for the confirmation.

Le 16/12/2019 à 20:20, Cyril Bonté a écrit :
> Hi Artur,
>
> Le 16/12/2019 à 19:06, Artur a écrit :
>> Hello,
>>
>> This is an extract of my frontend configuration working perfectly on
>> 2.0.11.
>>
>> frontend wwws
>>  bind 0.0.0.0:443 ssl crt /etc/haproxy/ssl/server.pem alpn
>> h2,http/1.1
>>  mode http
>>  acl is_dev_qd hdr(host) -i dev.q.d dev.qs.d
>>  acl is_qd hdr(host) -i q.d qs.d www.q.d www.qs.d
>>  http-request replace-uri ^/PPDSlide/(.*) /d3/PPDSlide/\1 if
>> is_dev_qd
>>  http-request replace-uri ^/PPDSlide/(.*) /p3/PPDSlide/\1 if
>> is_qd
>>  
>>
>> URLs like https://q.d/PPDSlide/testfile are correctly rewritten to
>> https://q.d/p3/PPDSlide/testfile and forwarded to the backend.
>>
>> Once I switched to 2.1.1, haproxy no longer rewrites the URI and the
>> URIs remains unchanged while forwarded to the backend. I had to
>> downgrade to have the usual behaviour.
>>
>> Is it a bug or something changed in normal haproxy behaviour with 2.1
>> release ?
>
> I can confirm the issue.
>
> It seems to happen with h2 requests only, since commit #30ee1efe67.
> haproxy normalizes the URI but replace-uri doesn't take into account
> this information. The fix should be easy for replace-uri (If someone
> wants to work on it, I won't have time this week).
>
> http://git.haproxy.org/?p=haproxy-2.1.git;a=commit;h=30ee1efe67
>
>
-- 
Best regards,
Artur




Re: HAProxy 2.0.10 and 2.1.0 RPM's

2019-12-16 Thread Ryan O'Hara
On Tue, Nov 26, 2019 at 2:40 PM Russell Eason  wrote:

> Hello,
>
> Fedora upstream added it
> https://src.fedoraproject.org/rpms/haproxy/c/45c57ba71174f308a5f59569bac0598bb31ef767
> , and can be seen as far back as F24 here
> https://src.fedoraproject.org/rpms/haproxy/blob/f24/f/haproxy.spec . LUA
> support is in the RHEL 8 version of HAProxy, but not in 7 (yet?).
>

Sorry for the late reply. I believe the reason that RHEL7 lacks LUA support
is because the required version of LUA is not available in RHEL7. Enabling
LUA support in haproxy at build time is easy, but if the underlying bits
aren't available, there is nothing I can do. Cheers.

Ryan


Re: [PATCH] openssl-compat: Fix getm_ defines

2019-12-16 Thread Rosen Penev
On Mon, Dec 16, 2019 at 10:21 AM Илья Шипицин  wrote:
>
>
>
> пн, 16 дек. 2019 г. в 22:40, Rosen Penev :
>>
>> On Mon, Dec 16, 2019 at 4:49 AM Lukas Tribus  wrote:
>> >
>> > Hello Rosen,
>> >
>> > > пн, 16 дек. 2019 г. в 12:07, Rosen Penev :
>> > >>
>> > >> LIBRESSL_VERSION_NUMBER evaluates to 0 under OpenSSL, making the 
>> > >> condition
>> > >> always true. Check for the define before checking it.
>> >
>> > I cannot find this in the openssl sources, not in master and not in
>> > the 1.1.1 branch. Please clarify where this is defined.
>> Compile with -Wundef. Missing macros evaluate to 0.
>
>
> I checked haproxy source, it does not use such compiler flag. Any reason for 
> introducing it ?
>
> if we want to make it first class citizen, maybe we should add it to proper 
> Makefile ? or to our CI ?
>
> assuming "undefined macros may ACCIDENTLY become equal to 0" scares me
You serious? This is basic C. Undefined macros always evaluate to 0.

-Wundef only warns about it.
>
>>
>> >
>> > The SSL compatibility layer is already complex enough and needs
>> > continuous adjustments, we need to understand the reason for changes
>> > very well. Fast fixes are continually coming back to hunt us.
>> >
>> >
>> > On Mon, 16 Dec 2019 at 08:19, Илья Шипицин  wrote:
>> > > please have a look at https://github.com/haproxy/haproxy/issues/367 (it 
>> > > still misses germ part, I tried things like you send, but reg-tests 
>> > > fail. do you have travis-ci passed ?)
>> > > also, there's a patch already sent, Lukas Tribus promised to review it
>> >
>> > Yeah, this one fell through the cracks. Give me a few days to catch up.
>> >
>> > Thanks,
>> > Lukas



Re: [PATCH] openssl-compat: Fix getm_ defines

2019-12-16 Thread Rosen Penev
On Mon, Dec 16, 2019 at 10:09 AM Lukas Tribus  wrote:
>
> On Mon, 16 Dec 2019 at 19:00, Илья Шипицин  wrote:
> >
> >
> >
> > пн, 16 дек. 2019 г. в 22:42, Rosen Penev :
> >>
> >> LIBRESSL_VERSION_NUMBER evaluates to 0 under OpenSSL, making the condition
> >> always true. Check for the define before checking it.
> >>
> >> Signed-off-by: Rosen Penev 
> >> ---
> >>  include/common/openssl-compat.h | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/include/common/openssl-compat.h 
> >> b/include/common/openssl-compat.h
> >> index 25102fbe3..c5029d133 100644
> >> --- a/include/common/openssl-compat.h
> >> +++ b/include/common/openssl-compat.h
> >> @@ -278,7 +278,7 @@ static inline void EVP_PKEY_up_ref(EVP_PKEY *pkey)
> >>  #define TLSEXT_signature_ecdsa  3
> >>  #endif
> >>
> >> -#if (OPENSSL_VERSION_NUMBER < 0x1010L) || (LIBRESSL_VERSION_NUMBER < 
> >> 0x2070L)
> >> +#if (HA_OPENSSL_VERSION_NUMBER < 0x101fL) && (LIBRESSL_VERSION_NUMBER 
> >> < 0x207fL)
> >
> >
> > assuming "&& (LIBRESSL_VERSION_NUMBER < 0x207fL)" part ... it is only 
> > relevant for LibreSSL, right ?
> > if so, should we leave just second part and omit first ?
>
> Any reason why would not just #ifndef X509_getm_notBefore, testing for
> what we actually want instead of those backbreaking version
> assumptions?
X509_getm_notBefore is a function, not a define.
>
>
> Lukas



Re: [2.1.1] http-request replace-uri does not work

2019-12-16 Thread Cyril Bonté

Hi Artur,

Le 16/12/2019 à 19:06, Artur a écrit :

Hello,

This is an extract of my frontend configuration working perfectly on 2.0.11.

frontend wwws
     bind 0.0.0.0:443 ssl crt /etc/haproxy/ssl/server.pem alpn
h2,http/1.1
     mode http
     acl is_dev_qd hdr(host) -i dev.q.d dev.qs.d
     acl is_qd hdr(host) -i q.d qs.d www.q.d www.qs.d
     http-request replace-uri ^/PPDSlide/(.*) /d3/PPDSlide/\1 if
is_dev_qd
     http-request replace-uri ^/PPDSlide/(.*) /p3/PPDSlide/\1 if is_qd
     

URLs like https://q.d/PPDSlide/testfile are correctly rewritten to
https://q.d/p3/PPDSlide/testfile and forwarded to the backend.

Once I switched to 2.1.1, haproxy no longer rewrites the URI and the
URIs remains unchanged while forwarded to the backend. I had to
downgrade to have the usual behaviour.

Is it a bug or something changed in normal haproxy behaviour with 2.1
release ?


I can confirm the issue.

It seems to happen with h2 requests only, since commit #30ee1efe67.
haproxy normalizes the URI but replace-uri doesn't take into account 
this information. The fix should be easy for replace-uri (If someone 
wants to work on it, I won't have time this week).


http://git.haproxy.org/?p=haproxy-2.1.git;a=commit;h=30ee1efe67


--
Cyril Bonté



CORS support

2019-12-16 Thread Alex Evonosky
Hello Haproxy group-

migrating from haproxy 2.0 to 2.1 and noticed some directives changed:

=== 2.0.10 ===

capture request header origin len 128
http-response add-header Access-Control-Allow-Origin %[capture.req.hdr(0)]
if { capture.req.hdr(0) -m end aiqwest.com }
http-response add-header Access-Control-Allow-Headers:\ Origin,\
X-Requested-With,\ Content-Type,\ Accept if { capture.req.hdr(0) -m found }


=== 2.1 ===

capture request header origin len 128
http-response add-header Access-Control-Allow-Origin %[capture.req.hdr(0)]
if { capture.req.hdr(0) -m end aiqwest.com }
rspadd add-header Access-Control-Allow-Headers:\ Origin,\
X-Requested-With,\ Content-Type,\ Accept if { capture.req.hdr(0) -m found }


getting:

The 'rspadd' directive is not supported anymore since HAProxy 2.1. Use
'http-response add-header' instead.

when adding the said above, getting an error:

'http-response add-header' expects exactly 2 arguments.


main question is, how can I convert the resadd statement to work with the
new 2.1?


Thank you!


Re: [PATCH] openssl-compat: Fix getm_ defines

2019-12-16 Thread Илья Шипицин
пн, 16 дек. 2019 г. в 22:40, Rosen Penev :

> On Mon, Dec 16, 2019 at 4:49 AM Lukas Tribus  wrote:
> >
> > Hello Rosen,
> >
> > > пн, 16 дек. 2019 г. в 12:07, Rosen Penev :
> > >>
> > >> LIBRESSL_VERSION_NUMBER evaluates to 0 under OpenSSL, making the
> condition
> > >> always true. Check for the define before checking it.
> >
> > I cannot find this in the openssl sources, not in master and not in
> > the 1.1.1 branch. Please clarify where this is defined.
> Compile with -Wundef. Missing macros evaluate to 0.
>

I checked haproxy source, it does not use such compiler flag. Any reason
for introducing it ?

if we want to make it first class citizen, maybe we should add it to proper
Makefile ? or to our CI ?

assuming "undefined macros may ACCIDENTLY become equal to 0" scares me


> >
> > The SSL compatibility layer is already complex enough and needs
> > continuous adjustments, we need to understand the reason for changes
> > very well. Fast fixes are continually coming back to hunt us.
> >
> >
> > On Mon, 16 Dec 2019 at 08:19, Илья Шипицин  wrote:
> > > please have a look at https://github.com/haproxy/haproxy/issues/367
> (it still misses germ part, I tried things like you send, but reg-tests
> fail. do you have travis-ci passed ?)
> > > also, there's a patch already sent, Lukas Tribus promised to review it
> >
> > Yeah, this one fell through the cracks. Give me a few days to catch up.
> >
> > Thanks,
> > Lukas
>


Re: [PATCH] openssl-compat: Fix getm_ defines

2019-12-16 Thread Lukas Tribus
On Mon, 16 Dec 2019 at 19:00, Илья Шипицин  wrote:
>
>
>
> пн, 16 дек. 2019 г. в 22:42, Rosen Penev :
>>
>> LIBRESSL_VERSION_NUMBER evaluates to 0 under OpenSSL, making the condition
>> always true. Check for the define before checking it.
>>
>> Signed-off-by: Rosen Penev 
>> ---
>>  include/common/openssl-compat.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/common/openssl-compat.h 
>> b/include/common/openssl-compat.h
>> index 25102fbe3..c5029d133 100644
>> --- a/include/common/openssl-compat.h
>> +++ b/include/common/openssl-compat.h
>> @@ -278,7 +278,7 @@ static inline void EVP_PKEY_up_ref(EVP_PKEY *pkey)
>>  #define TLSEXT_signature_ecdsa  3
>>  #endif
>>
>> -#if (OPENSSL_VERSION_NUMBER < 0x1010L) || (LIBRESSL_VERSION_NUMBER < 
>> 0x2070L)
>> +#if (HA_OPENSSL_VERSION_NUMBER < 0x101fL) && (LIBRESSL_VERSION_NUMBER < 
>> 0x207fL)
>
>
> assuming "&& (LIBRESSL_VERSION_NUMBER < 0x207fL)" part ... it is only 
> relevant for LibreSSL, right ?
> if so, should we leave just second part and omit first ?

Any reason why would not just #ifndef X509_getm_notBefore, testing for
what we actually want instead of those backbreaking version
assumptions?


Lukas



Re: [PATCH] openssl-compat: Fix getm_ defines

2019-12-16 Thread Rosen Penev
On Mon, Dec 16, 2019 at 10:00 AM Илья Шипицин  wrote:
>
>
>
> пн, 16 дек. 2019 г. в 22:42, Rosen Penev :
>>
>> LIBRESSL_VERSION_NUMBER evaluates to 0 under OpenSSL, making the condition
>> always true. Check for the define before checking it.
>>
>> Signed-off-by: Rosen Penev 
>> ---
>>  include/common/openssl-compat.h | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/common/openssl-compat.h 
>> b/include/common/openssl-compat.h
>> index 25102fbe3..c5029d133 100644
>> --- a/include/common/openssl-compat.h
>> +++ b/include/common/openssl-compat.h
>> @@ -278,7 +278,7 @@ static inline void EVP_PKEY_up_ref(EVP_PKEY *pkey)
>>  #define TLSEXT_signature_ecdsa  3
>>  #endif
>>
>> -#if (OPENSSL_VERSION_NUMBER < 0x1010L) || (LIBRESSL_VERSION_NUMBER < 
>> 0x2070L)
>> +#if (HA_OPENSSL_VERSION_NUMBER < 0x101fL) && (LIBRESSL_VERSION_NUMBER < 
>> 0x207fL)
>
>
> assuming "&& (LIBRESSL_VERSION_NUMBER < 0x207fL)" part ... it is only 
> relevant for LibreSSL, right ?
> if so, should we leave just second part and omit first ?
No. As I said previously, undefined macros evaluate to 0. OpenSSL does
not define LIBRESSL_VERSION_NUMBER.
>
>
>>
>>  #define X509_getm_notBefore X509_get_notBefore
>>  #define X509_getm_notAfter  X509_get_notAfter
>>  #endif
>> --
>> 2.23.0
>>
>>



[2.1.1] http-request replace-uri does not work

2019-12-16 Thread Artur
Hello,

This is an extract of my frontend configuration working perfectly on 2.0.11.

frontend wwws
    bind 0.0.0.0:443 ssl crt /etc/haproxy/ssl/server.pem alpn
h2,http/1.1
    mode http
    acl is_dev_qd hdr(host) -i dev.q.d dev.qs.d
    acl is_qd hdr(host) -i q.d qs.d www.q.d www.qs.d
    http-request replace-uri ^/PPDSlide/(.*) /d3/PPDSlide/\1 if
is_dev_qd
    http-request replace-uri ^/PPDSlide/(.*) /p3/PPDSlide/\1 if is_qd
    

URLs like https://q.d/PPDSlide/testfile are correctly rewritten to
https://q.d/p3/PPDSlide/testfile and forwarded to the backend.

Once I switched to 2.1.1, haproxy no longer rewrites the URI and the
URIs remains unchanged while forwarded to the backend. I had to
downgrade to have the usual behaviour.

Is it a bug or something changed in normal haproxy behaviour with 2.1
release ?

-- 
Best regards,
Artur




Re: [PATCH] openssl-compat: Fix getm_ defines

2019-12-16 Thread Илья Шипицин
пн, 16 дек. 2019 г. в 22:42, Rosen Penev :

> LIBRESSL_VERSION_NUMBER evaluates to 0 under OpenSSL, making the condition
> always true. Check for the define before checking it.
>
> Signed-off-by: Rosen Penev 
> ---
>  include/common/openssl-compat.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/common/openssl-compat.h
> b/include/common/openssl-compat.h
> index 25102fbe3..c5029d133 100644
> --- a/include/common/openssl-compat.h
> +++ b/include/common/openssl-compat.h
> @@ -278,7 +278,7 @@ static inline void EVP_PKEY_up_ref(EVP_PKEY *pkey)
>  #define TLSEXT_signature_ecdsa  3
>  #endif
>
> -#if (OPENSSL_VERSION_NUMBER < 0x1010L) || (LIBRESSL_VERSION_NUMBER <
> 0x2070L)
> +#if (HA_OPENSSL_VERSION_NUMBER < 0x101fL) && (LIBRESSL_VERSION_NUMBER
> < 0x207fL)
>

assuming "&& (LIBRESSL_VERSION_NUMBER < 0x207fL)" part ... it is only
relevant for LibreSSL, right ?
if so, should we leave just second part and omit first ?



>  #define X509_getm_notBefore X509_get_notBefore
>  #define X509_getm_notAfter  X509_get_notAfter
>  #endif
> --
> 2.23.0
>
>
>


Re: [PATCH] openssl-compat: Fix getm_ defines

2019-12-16 Thread Rosen Penev
On Mon, Dec 16, 2019 at 4:49 AM Lukas Tribus  wrote:
>
> Hello Rosen,
>
> > пн, 16 дек. 2019 г. в 12:07, Rosen Penev :
> >>
> >> LIBRESSL_VERSION_NUMBER evaluates to 0 under OpenSSL, making the condition
> >> always true. Check for the define before checking it.
>
> I cannot find this in the openssl sources, not in master and not in
> the 1.1.1 branch. Please clarify where this is defined.
Compile with -Wundef. Missing macros evaluate to 0.
>
> The SSL compatibility layer is already complex enough and needs
> continuous adjustments, we need to understand the reason for changes
> very well. Fast fixes are continually coming back to hunt us.
>
>
> On Mon, 16 Dec 2019 at 08:19, Илья Шипицин  wrote:
> > please have a look at https://github.com/haproxy/haproxy/issues/367 (it 
> > still misses germ part, I tried things like you send, but reg-tests fail. 
> > do you have travis-ci passed ?)
> > also, there's a patch already sent, Lukas Tribus promised to review it
>
> Yeah, this one fell through the cracks. Give me a few days to catch up.
>
> Thanks,
> Lukas



[PATCH] openssl-compat: Fix getm_ defines

2019-12-16 Thread Rosen Penev
LIBRESSL_VERSION_NUMBER evaluates to 0 under OpenSSL, making the condition
always true. Check for the define before checking it.

Signed-off-by: Rosen Penev 
---
 include/common/openssl-compat.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/common/openssl-compat.h b/include/common/openssl-compat.h
index 25102fbe3..c5029d133 100644
--- a/include/common/openssl-compat.h
+++ b/include/common/openssl-compat.h
@@ -278,7 +278,7 @@ static inline void EVP_PKEY_up_ref(EVP_PKEY *pkey)
 #define TLSEXT_signature_ecdsa  3
 #endif
 
-#if (OPENSSL_VERSION_NUMBER < 0x1010L) || (LIBRESSL_VERSION_NUMBER < 
0x2070L)
+#if (HA_OPENSSL_VERSION_NUMBER < 0x101fL) && (LIBRESSL_VERSION_NUMBER < 
0x207fL)
 #define X509_getm_notBefore X509_get_notBefore
 #define X509_getm_notAfter  X509_get_notAfter
 #endif
-- 
2.23.0




Re: [RFC PATCH] MINOR: debug: allow debug converter in default build

2019-12-16 Thread Lukas Tribus
Hello,

On Mon, 16 Dec 2019 at 09:20, Willy Tarreau  wrote:
>
> Hi Lukas,
>
> On Sun, Dec 15, 2019 at 05:23:38PM +0100, Lukas Tribus wrote:
> > Currently this debug converter is only enabled when DEBUG_EXPR is
> > defined at build time (which is different than other debug build
> > options and unclear from the documentation).
> >
> > This moves the patch to the default build, so everyone can use it.
>
> I was thinking about repurposing this converter to use the ring buffer
> instead, so that it can be used even with live traffic and let one
> consult some history from the CLI. We could then have a few info
> such as the request ID, and config file+line number where the
> converter is present, followed by the pattern.
>
> We could imagine adding one (or multiple) optional arguments to force
> the output to another destination (e.g. stdout, stderr, other ring),
> and maybe another one for the format or to prepend a prefix. Then most
> likely we'd use the ring buffer by default as it's the least impacting
> one and the only self-sustaining output. And probably that we could
> switch to stderr by default in backports (or make it mandatory to
> force the destination).
>
> What do you think ?

I like it, more flexibility to debug in production is always a good.

Removing those 2 ifdef's is just a "low hanging fruit".


Lukas



Re: PROXY protocol and check port

2019-12-16 Thread Olivier D
Hello,

I found what was wrong : I was using "load-server-state-from-file" and
previous config file was using port 80 as server port.
It seems using this instruction loads previous server state but also
previous srv_port.
Is this an expected behaviour ?

Olivier


Le ven. 13 déc. 2019 à 18:32, Olivier D  a écrit :

> Hello all,
> I struggle with what seemed a very easy config :
>
> listen test:443
> id 20609
> bind-process 16
> balance source
> hash-type consistent
> mode tcp
> bind x.x.x.x:443
> server s1 192.168.x.x:443 id 2158 check weight 5 send-proxy port 80
> server s2 192.168.x.x:443 id 2168 check weight 5 send-proxy port 80
>
> I expect traffic to be routed to port 443 of backend server, and checks to
> be performed without proxy-protocol on port 80. tests seem ok, but
> traffic seems also routed to port 80 (I checked with tcpdump) instead of
> 443.
>
> Can you at least confirm that my expectations are correct ? Or am I just
> very tired on a friday evening ?
>
> I'm using HAProxy 1.9.12
>
> Thank you !
>


Re: [PATCH] openssl-compat: Fix getm_ defines

2019-12-16 Thread Lukas Tribus
Hello Rosen,

> пн, 16 дек. 2019 г. в 12:07, Rosen Penev :
>>
>> LIBRESSL_VERSION_NUMBER evaluates to 0 under OpenSSL, making the condition
>> always true. Check for the define before checking it.

I cannot find this in the openssl sources, not in master and not in
the 1.1.1 branch. Please clarify where this is defined.

The SSL compatibility layer is already complex enough and needs
continuous adjustments, we need to understand the reason for changes
very well. Fast fixes are continually coming back to hunt us.


On Mon, 16 Dec 2019 at 08:19, Илья Шипицин  wrote:
> please have a look at https://github.com/haproxy/haproxy/issues/367 (it still 
> misses germ part, I tried things like you send, but reg-tests fail. do you 
> have travis-ci passed ?)
> also, there's a patch already sent, Lukas Tribus promised to review it

Yeah, this one fell through the cracks. Give me a few days to catch up.

Thanks,
Lukas



Re: [PATCH] openssl-compat: Fix getm_ defines

2019-12-16 Thread Илья Шипицин
also, BoringSSL fails after applying your patch

https://travis-ci.com/chipitsine/haproxy/jobs/267601286

пн, 16 дек. 2019 г. в 12:07, Rosen Penev :

> LIBRESSL_VERSION_NUMBER evaluates to 0 under OpenSSL, making the condition
> always true. Check for the define before checking it.
>
> Signed-off-by: Rosen Penev 
> ---
>  include/common/openssl-compat.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/include/common/openssl-compat.h
> b/include/common/openssl-compat.h
> index 25102fbe3..8b1630110 100644
> --- a/include/common/openssl-compat.h
> +++ b/include/common/openssl-compat.h
> @@ -278,7 +278,8 @@ static inline void EVP_PKEY_up_ref(EVP_PKEY *pkey)
>  #define TLSEXT_signature_ecdsa  3
>  #endif
>
> -#if (OPENSSL_VERSION_NUMBER < 0x1010L) || (LIBRESSL_VERSION_NUMBER <
> 0x2070L)
> +#if (OPENSSL_VERSION_NUMBER < 0x1010L) || \
> +   (defined(LIBRESSL_VERSION_NUMBER) && (LIBRESSL_VERSION_NUMBER <
> 0x2070L))
>  #define X509_getm_notBefore X509_get_notBefore
>  #define X509_getm_notAfter  X509_get_notAfter
>  #endif
> --
> 2.23.0
>
>
>


Re: [RFC PATCH] MINOR: debug: allow debug converter in default build

2019-12-16 Thread Willy Tarreau
On Mon, Dec 16, 2019 at 09:30:38AM +0100, Baptiste wrote:
> My 2 cents.
> I personally use a lot this converter, si I'd be more than happy to get it
> available in default built!

OK.

> I think Willy's idea to route its output wherever we want is great too for
> production purpose.

At least the thing is that you cannot leave it enabled forever when it's
on stdout or stderr because either this will be dropped entirely or will
inflate the service logs. With the rotating buffer you can leave it enabled
full time and only go to the CLI to consult it when needed. Later we could
imagine passing an extra ID to selectively enable/disable each of them via
the CLI. It would then provide something more or less comparable to what
Cyril did many years ago with his ACL debugging patchset.

> Can we also use an env variable? So we can easily switch from stdout to
> ring buffer without updating the config file?

It's orthogonal, environment variables are evaluated by the config parser :-)
You could even imagine having something like  :

 http-request some-action "sample_fetch${DEBUG},converter"

Where DEBUG is either empty or ",debug".

Willy



Re: [RFC PATCH] MINOR: debug: allow debug converter in default build

2019-12-16 Thread Baptiste
On Mon, Dec 16, 2019 at 9:22 AM Willy Tarreau  wrote:

> Hi Lukas,
>
> On Sun, Dec 15, 2019 at 05:23:38PM +0100, Lukas Tribus wrote:
> > Currently this debug converter is only enabled when DEBUG_EXPR is
> > defined at build time (which is different than other debug build
> > options and unclear from the documentation).
> >
> > This moves the patch to the default build, so everyone can use it.
>
> I was thinking about repurposing this converter to use the ring buffer
> instead, so that it can be used even with live traffic and let one
> consult some history from the CLI. We could then have a few info
> such as the request ID, and config file+line number where the
> converter is present, followed by the pattern.
>
> We could imagine adding one (or multiple) optional arguments to force
> the output to another destination (e.g. stdout, stderr, other ring),
> and maybe another one for the format or to prepend a prefix. Then most
> likely we'd use the ring buffer by default as it's the least impacting
> one and the only self-sustaining output. And probably that we could
> switch to stderr by default in backports (or make it mandatory to
> force the destination).
>
> What do you think ?
>
> Cheers,
> Willy
>
>

Hi,

My 2 cents.
I personally use a lot this converter, si I'd be more than happy to get it
available in default built!

I think Willy's idea to route its output wherever we want is great too for
production purpose.
Can we also use an env variable? So we can easily switch from stdout to
ring buffer without updating the config file?

Baptiste


Re: [RFC PATCH] MINOR: debug: allow debug converter in default build

2019-12-16 Thread Willy Tarreau
Hi Lukas,

On Sun, Dec 15, 2019 at 05:23:38PM +0100, Lukas Tribus wrote:
> Currently this debug converter is only enabled when DEBUG_EXPR is
> defined at build time (which is different than other debug build
> options and unclear from the documentation).
> 
> This moves the patch to the default build, so everyone can use it.

I was thinking about repurposing this converter to use the ring buffer
instead, so that it can be used even with live traffic and let one
consult some history from the CLI. We could then have a few info
such as the request ID, and config file+line number where the
converter is present, followed by the pattern.

We could imagine adding one (or multiple) optional arguments to force
the output to another destination (e.g. stdout, stderr, other ring),
and maybe another one for the format or to prepend a prefix. Then most
likely we'd use the ring buffer by default as it's the least impacting
one and the only self-sustaining output. And probably that we could
switch to stderr by default in backports (or make it mandatory to
force the destination).

What do you think ?

Cheers,
Willy



Re: [PATCH] openssl-compat: Fix getm_ defines

2019-12-16 Thread Илья Шипицин
пн, 16 дек. 2019 г. в 12:47, William Lallemand :

> Hello Rosen,
>
> On Sun, Dec 15, 2019 at 11:04:37PM -0800, Rosen Penev wrote:
> > -#if (OPENSSL_VERSION_NUMBER < 0x1010L) || (LIBRESSL_VERSION_NUMBER
> < 0x2070L)
> > +#if (OPENSSL_VERSION_NUMBER < 0x1010L) || \
> > + (defined(LIBRESSL_VERSION_NUMBER) && (LIBRESSL_VERSION_NUMBER <
> 0x2070L))
> >
>
> It's probably cleaner to use the HA_OPENSSL_VERSION_NUMBER function and
> something like this:
>

@Rosen Penev 

there was some discussion whether to trust OPENSSL_VERSION_NUMBER or not.
LibreSSL pollutes that macro, it sets it to 2.0.0

so ... new macro was introduced instead: HA_OPENSSL_VERSION_NUMBER

https://github.com/haproxy/haproxy/blob/master/include/common/openssl-compat.h#L27-L36


>
>#if (HA_OPENSSL_VERSION_NUMBER < 0x101fL) &&
> (LIBRESSL_VERSION_NUMBER < 0x207fL)
>
> --
> William Lallemand
>
>