Adding custom TLV to proxy protocol sent to backend servers?

2017-03-23 Thread Christian Rohmann

Hello!

Is there a way to make use of the extensibility of the PROXY protocol 
version 2 and to add a TLV-field which is then sent to the backend 
servers? I would like to i.e. generate a random id value per request, 
add that to the access log and also forward this very value to the 
backend service (speaking / understanding PROXYv2 of cause). This way 
it's then possible to correlate between the incoming request handled and 
distributed by HAProxy and the corresponding request which is logged on 
the backend server.


1) Are there config options for adding things to PROXY anywhere?
2) I did not find any LUA API bindings to access the PROXY protocol 
values, maybe that would be the right approach?


Thanks in advance,
Regards

Christian



Individual or even dynamic tcp-check send (binary) commands per backend server? rand()? unique-id()?

2017-03-23 Thread Christian Rohmann

Hello HAProxy community!

I configured a protocol specific tcp-check using send-binary and expect 
binary. Just like described on https://blog.danman.eu/mongodb-haproxy/ 
for MongoDB. What a cool feature!


The check itself works just fine. But, to make it perfect I need to send 
an individual string or binary for each backend server.


1) Is it possible to configure a check individually for each server in a 
backend?


2) Or even better / easier, is it possible somehow to use rand(), 
unique-id() or any reference to some variable in those "tcp-check send" 
lines?



Thanks in advance for any ideas!
Regards

Christian



Re: All "server" settings supported on "default-server" lines

2017-03-23 Thread Emmanuel Hocdet

> Le 22 mars 2017 à 22:58, Willy Tarreau  a écrit :
> 
> On Wed, Mar 22, 2017 at 05:30:09PM +0100, Emmanuel Hocdet wrote:
>> I have patches sent in the ML who change the internal implementation of
>> no/force-tlsxx and add min/max-tlsxx (who can replace no/force usage).
>> It could simplify (or not)  what you want to do, but there will be an impact
>> on your patches if they are accepted.
> 
> Yes, as I said in the other mail I think that's on a good track but as
> Emeric suggested we'd rather have them provide an argument instead of
> using the keyword name, that will make it much easier to process. We
> can still support most older valid use cases and use warnings to explain
> how to convert that to the new mode (if really needed, not even sure) and
> emit errors explaining what to do for the situations that openssl does
> not support anymore (holes in the range).
> 

Emeric's suggestion is not on the ML.
If no- and force- are defined as deprecated it can make a difference.
I'm not used to seeing this kind of proposal for haproxy configuration ;-)
(for the hole, openssl low leveling the range:  no-tlsv1.1 become max-tlsv1.0)

++
Manu





Re: All "server" settings supported on "default-server" lines

2017-03-23 Thread Frederic Lecaille

On 03/21/2017 07:54 PM, Frederic Lecaille wrote:

Hello HAProxy ML,

I am starting this new thread to publish a serie of patches to make
all "server" settings be supported on "default-server" lines.

This is a preliminary work for "server templates" feature.

New boolean settings have been added to disable others. Most of them
have "no-" as prefix.


[snipped]


So, from now on, all server "settings" are supported by "default-server"
except "id" which is only supported on "server" lines.


Before all theses patches, if an unknown "foo" keyword was provided on 
"default-server" or "server" lines, such error messages were displayed:


[ALERT] 081/110011 (11626) : parsing [haproxy.cfg:97] : 'server srv1' 
unknown keyword 'foo'. Registered keywords :

[ ALL] id 
[ TCP] tcp-ut 
[ SSL] ca-file 
.
.
etc.

[ALERT] 081/110011 (11626) : Error(s) found in configuration file : 
haproxy.cfg

[ALERT] 081/110011 (11626) : Fatal errors found in configuration.

Only registered by srv_register_keywords() in 'srv_kwst list keywords 
were displayed.


Most of this thread patches register new keywords in this lists.

So now, the new error messages are:

[ALERT] 081/111458 (12190) : parsing [haproxy.cfg:97] : 'server srv1' 
unknown keyword 'foo'. Registered keywords :

[ SSL] ca-file  [dflt_ok]
[ SSL] check-ssl [dflt_ok]
[ SSL] ciphers  [dflt_ok]
[ SSL] crl-file  [dflt_ok]
[ SSL] crt  [dflt_ok]
[ SSL] force-sslv3 [dflt_ok]
[ SSL] force-tlsv10 [dflt_ok]
.
.
[snipped]
.
[ ALL] id 
[ ALL] namespace  [dflt_ok]
[ ALL] no-agent-check [dflt_ok]
[ ALL] no-backup [dflt_ok]

From my point of view, this is not acceptable to flag all these 
settings as supported on "default-server" lines.


At this time, as "id" remains as unique setting supported by "server", 
the patch attached to this mail now produces these error messages:


[ALERT] 081/111458 (12190) : parsing [haproxy.cfg:97] : 'server srv1' 
unknown keyword 'foo'. Registered keywords :

[ SSL] ca-file 
[ SSL] check-ssl
[ SSL] ciphers 
[ SSL] crl-file 
[ SSL] crt 
[ SSL] force-sslv3
[ SSL] force-tlsv10
.
.
[snipped]
.
[ ALL] id  [server_only]
[ ALL] namespace 
[ ALL] no-agent-check
[ ALL] no-backup [dflt_ok]


It does not make sens to backport this patch. It is only supposed to be
used to fix the boring error messages this thread patches introduced.


>From 94f81538cc6b9f042800b47bb57cc5e1e4a9ee24 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20L=C3=A9caille?= 
Date: Thu, 23 Mar 2017 11:39:10 +0100
Subject: [PATCH] MINOR: server: Display only server keywords which are only
 supported on "server" lines.
X-Bogosity: Ham, tests=bogofilter, spamicity=0.01, version=1.2.4

When displaying error messages when parsing unknown "server" keywords,
in place of flagging keywords which are also supported on "default-server" lines,,
and as in the future almost all "server" setting will be supported by "default-server",
this patch make the error message handlers flag only keywords which are only
supported on server lines.

Does not make sense to be backported.
---
 src/server.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/server.c b/src/server.c
index 5589723..1b02ee5 100644
--- a/src/server.c
+++ b/src/server.c
@@ -206,7 +206,7 @@ void srv_dump_kws(char **out)
   kwl->scope,
   kwl->kw[index].kw,
   kwl->kw[index].skip ? " " : "",
-  kwl->kw[index].default_ok ? " [dflt_ok]" : "",
+  kwl->kw[index].default_ok ? "" : " [server_only]",
   kwl->kw[index].parse ? "" : " (not supported)");
 			}
 		}
-- 
2.1.4



Re: All "server" settings supported on "default-server" lines

2017-03-23 Thread Frederic Lecaille

On 03/23/2017 12:03 PM, Frederic Lecaille wrote:

On 03/21/2017 07:54 PM, Frederic Lecaille wrote:


[snipped]


At this time, as "id" remains as unique setting supported by "server",
the patch attached to this mail now produces these error messages:

[ALERT] 081/111458 (12190) : parsing [haproxy.cfg:97] : 'server srv1'
unknown keyword 'foo'. Registered keywords :
[ SSL] ca-file 
[ SSL] check-ssl
[ SSL] ciphers 
[ SSL] crl-file 
[ SSL] crt 
[ SSL] force-sslv3
[ SSL] force-tlsv10
.
.
[snipped]
.
[ ALL] id  [server_only]
[ ALL] namespace 
[ ALL] no-agent-check
[ ALL] no-backup [dflt_ok]


ooop! wrong copy and paste... well remove this previous line...




Re: All "server" settings supported on "default-server" lines

2017-03-23 Thread Willy Tarreau
On Thu, Mar 23, 2017 at 11:26:50AM +0100, Emmanuel Hocdet wrote:
> Emeric's suggestion is not on the ML.

I transcripted it in the other e-mail of this same thread.

> If no- and force- are defined as deprecated it can make a difference.
> I'm not used to seeing this kind of proposal for haproxy configuration ;-)

The purpose is not to deprecate them but to "emulate" them and only
to deprecate those causing trouble.

> (for the hole, openssl low leveling the range:  no-tlsv1.1 become max-tlsv1.0)

Here for me, no-tlsv1.1 alone would be rejected because it's ambigous as it
either means max-tlsv1.0 or min-tlsv1.2. However if you have :

no-sslv3 no-tlsv1.0 no-tlsv1.1

Then there's no ambiguity, only tlsv1.2 remains. Conversely :

no-tlsv1.1 no-tlsv1.2

Restricts us to sslv3 and tlsv1.0.

So in fact we know the bounds we support and we convert a contigous range
into a min+max. Discontinuities are rejected and it should not be a problem.
Emeric told me that he knows some deployments where people rejected recent
versions due to interoperability issues, but there's no reason for blocking
only a middle version and not either older or newer ones.

I know Emeric is busy at the moment so I don't expect him to read this
thread yet :-)

Willy



Send PROXY protocol header from HAProxy

2017-03-23 Thread Dave J
I've probably got lost in the masses of documentation on this subject, but I'm 
trying to configure my HAProxy process to send the PROXY protocol header as 
described at http://www.haproxy.org/download/1.8/doc/proxy-protocol.txt. This 
is because I am having to write support for the PROXY protocol into a C++ 
server (in order for it to have access to the client IP/port) and I want to 
test my code is working properly with the parsing of the PROXY header.

Here is my minimal config file:

---

global
   maxconn 4096

defaults
   log   global
   mode   http
   retries   3
   option redispatch
   maxconn   2000
   timeout connect 5000
   timeout client  5
   timeout server  5

frontend TestServerTest
    bind 10.6.186.24:54781
    mode tcp
    default_backend TestServernodes

backend TestServernodes
    mode tcp
    # Note there is no 'check' after the below line unlike the others as we 
don't want to send the
    # healthcheck ("OPTIONS / HTTP/1.0"...) string to the TestServer as it 
doesn't understand it!
    server TestServer01 10.6.186.24:48080

---

What I am finding is that when I start HAProxy and connect to 54781, the first 
data that TestServer at 48080 receives is the data which is sent from my 
client; it is not the PROXY header described at the link I posted.

Can someone please tell me what I am missing in my configuration that is 
preventing the PROXY header being sent to my backend server?

Thanks in advance.

Dave J




Re: [PATCH] MEDIUM: global: add a 'hard-stop-after' option to cap the soft-stop time

2017-03-23 Thread Olivier Doucet
Hello there,

2017-03-23 7:42 GMT+01:00 Willy Tarreau :

> Additionally, since it helps improving the reliability of the service
>
during reloads without adding particular features and the patch is
> reasonably small and isolated, I was thinking we could backport it to
> 1.7 and even possibly 1.6. What do you and others think ?
>
> Thanks!
> Willy
>
>

That would be really great. I have processes that stays alive several days
after a reload, and it's always a pain to kill them by hand / with a custom
bash loop.
With the code from Cyril, would it be possible to define very long timeout
(several hours) without side-effect ? If so, backport would be great.
I was also thinking about dumping how many connections were force-killed in
log, if it's easy.

Olivier


Re: All "server" settings supported on "default-server" lines

2017-03-23 Thread Emmanuel Hocdet

> Le 23 mars 2017 à 12:25, Willy Tarreau  a écrit :
> 
> On Thu, Mar 23, 2017 at 11:26:50AM +0100, Emmanuel Hocdet wrote:
>> Emeric's suggestion is not on the ML.
> 
> I transcripted it in the other e-mail of this same thread.
> 
>> If no- and force- are defined as deprecated it can make a difference.
>> I'm not used to seeing this kind of proposal for haproxy configuration ;-)
> 
> The purpose is not to deprecate them but to "emulate" them and only
> to deprecate those causing trouble.
> 
Ok, this is what I propose.

>> (for the hole, openssl low leveling the range:  no-tlsv1.1 become 
>> max-tlsv1.0)
> 
> Here for me, no-tlsv1.1 alone would be rejected because it's ambigous as it
> either means max-tlsv1.0 or min-tlsv1.2. However if you have :
> 

I agree, haproxy should, at least, warm in this case.
It’s in my todo, waiting for more feedback on my patches.

>no-sslv3 no-tlsv1.0 no-tlsv1.1
> 
> Then there's no ambiguity, only tlsv1.2 remains. Conversely :
> 
>no-tlsv1.1 no-tlsv1.2
> 
> Restricts us to sslv3 and tlsv1.0.
> 
> So in fact we know the bounds we support and we convert a contigous range
> into a min+max. Discontinuities are rejected and it should not be a problem.
> Emeric told me that he knows some deployments where people rejected recent
> versions due to interoperability issues, but there's no reason for blocking
> only a middle version and not either older or newer ones.
> 
Yep, perhaps continue to discuss in the treads with my patches.

> I know Emeric is busy at the moment so I don't expect him to read this
> thread yet :-)
> 


Manu




Re: [PATCH] MEDIUM: global: add a 'hard-stop-after' option to cap the soft-stop time

2017-03-23 Thread Willy Tarreau
Hi Olivier,

On Thu, Mar 23, 2017 at 02:17:38PM +0100, Olivier Doucet wrote:
> Hello there,
> 
> 2017-03-23 7:42 GMT+01:00 Willy Tarreau :
> 
> > Additionally, since it helps improving the reliability of the service
> >
> during reloads without adding particular features and the patch is
> > reasonably small and isolated, I was thinking we could backport it to
> > 1.7 and even possibly 1.6. What do you and others think ?
> >
> > Thanks!
> > Willy
> >
> >
> 
> That would be really great. I have processes that stays alive several days
> after a reload, and it's always a pain to kill them by hand / with a custom
> bash loop.

OK so I count this as a +1 vote :-)

> With the code from Cyril, would it be possible to define very long timeout
> (several hours) without side-effect ?

Sure, it's a task as any other task so it would not cause any particular
problem once you've taken this into account :
  - stats are not available anymore (you only see those from the new
process)
  - health checks are not performed anymore (avoids hammering servers from
many older processes not supposed to get any new request anyway)

These ones are normally totally fine for an old process (and you're living
with this now), these just limit the reasonable amount of time one would be
willing to run in this situation. A few hours definitely sounds totally
appropriate to me for most use cases with pure TCP connections (ldap, sql).

> If so, backport would be great.
> I was also thinking about dumping how many connections were force-killed in
> log, if it's easy.

I thought about it as well while responding but forgot to mention it. That's
also why I'd prefer to leave one extra second for the connections to cleanly
close and log. Yes it's important.

Cheers,
willy



Re: [Patches] TLS methods configuration reworked

2017-03-23 Thread Emeric Brun
Hi Manu,

On 03/22/2017 06:24 PM, Emmanuel Hocdet wrote:
> 
>> Le 22 mars 2017 à 16:30, Emmanuel Hocdet  a écrit :
>> […]
>> 0005 force-tlsxx implementation compatibility (Emeric first point)
>>
>> For the second point 
>>> But we will face issue using 'force-' when openssl will support further tls 
>>> versions not yet handled by haproxy. This problem was correctly handled by 
>>> the previous implementation.
>>
>> I can provide a patch for that but it will not useful for years until a new 
>> TLS will be implemented. It can generate build breaks until this time.
>> . all TLS methods are known in haproxy (set_options usage is safe)
>> . Haproxy must be run with the same version as the compilation. Change the 
>> openssl version (other than for bug fix) is not supported.
> 
> By testing TLSv1.3 i noticed that per default, the version is disable and 
> can’t be used until SSL_CTX_set_max_proto_version is set with TLS1_3_VERSION.
> So i will add a patch for SSL_CTX_set_max_proto_version with TLSv1.3 disable 
> per default.
> 
> This look like what you might have encountered with initial openssl 
> development and force-tlsxx: activate a pending TLS version.
> Does that tell you something Emeric?
> 
> Manu
> 
> 
> 

I said that using  SSL_CTX_set_max_proto_version and  
SSL_CTX_set_min_proto_version to handle force-tlsxx will keep it compliant for 
any newcoming protocol version.

Using ~knownbitflags is not the case cause you have to predict further 
versions, to disable them.

R,
Emeric
 



Re: Propagating agent-check weight change to tracking servers

2017-03-23 Thread Willy Tarreau
Hi Michal,

On Wed, Mar 15, 2017 at 10:13:01PM +0100, Michal wrote:
> Hello!
> Any news in this topic? Is there anything wrong with my patch?

So I checked it but it still has the problem of propagating absolute
weights, which, as I explained earlier, will break lots of setups. I
tend to think that doing it only for relative weight changes could be
OK (provided this is properly documented in the "track" and "agent-check"
keyword sections). The principle of the relative weight change is what
most users are seeking : the server wants to say "I'm running my backups
now, please cut my load in half" or "I'm swapping, I estimate that by
shrinking my load by 33% it will be OK". Regardless of the configured
weigths in different farms, we could propagate this relative weight
change.

Also I'm seeing that your patch only propagates to the first layer of
tracking servers, and stops there without updating the next layer, you
need a recursive propagation here.

Last, if you implement this, it's absolutely mandatory that the same is
done for the CLI since the CLI is the only way to fix the bad effects
of wrong agent changes. Thus having a function dedicated to propagating
relative weight changes would help, it would solve the case for the CLI
and for the agent.

I continue to think that such a change will definitely reintroduce problems
that took several years to get rid of, but hopefully with proper documentation
that can be worked around. Ie when a use complains that the weight change
applied to a server seems to regularly be ignored, it probably is because
of the fact that they're tracking another server whose weight changes.

Thanks,
Willy



Re: Send PROXY protocol header from HAProxy

2017-03-23 Thread Aaron West
Hi Dave,

I don't see the "send-proxy" directive in your config, have you tried it :
https://cbonte.github.io/haproxy-dconv/1.8/configuration.html#5.2-send-proxy

Sorry if I'm misunderstanding something already...

Aaron West

Loadbalancer.org Limited
+44 (0)330 380 1064
www.loadbalancer.org

On 23 March 2017 at 12:38, Dave J  wrote:

> I've probably got lost in the masses of documentation on this subject, but
> I'm trying to configure my HAProxy process to send the PROXY protocol
> header as described at http://www.haproxy.org/download/1.8/doc/proxy-
> protocol.txt. This is because I am having to write support for the PROXY
> protocol into a C++ server (in order for it to have access to the client
> IP/port) and I want to test my code is working properly with the parsing of
> the PROXY header.
>
> Here is my minimal config file:
>
> ---
>
> global
>maxconn 4096
>
> defaults
>log   global
>mode   http
>retries   3
>option redispatch
>maxconn   2000
>timeout connect 5000
>timeout client  5
>timeout server  5
>
> frontend TestServerTest
> bind 10.6.186.24:54781
> mode tcp
> default_backend TestServernodes
>
> backend TestServernodes
> mode tcp
> # Note there is no 'check' after the below line unlike the others as
> we don't want to send the
> # healthcheck ("OPTIONS / HTTP/1.0"...) string to the TestServer as it
> doesn't understand it!
> server TestServer01 10.6.186.24:48080
>
> ---
>
> What I am finding is that when I start HAProxy and connect to 54781, the
> first data that TestServer at 48080 receives is the data which is sent from
> my client; it is not the PROXY header described at the link I posted.
>
> Can someone please tell me what I am missing in my configuration that is
> preventing the PROXY header being sent to my backend server?
>
> Thanks in advance.
>
> Dave J
>
>
>


Re: Send PROXY protocol header from HAProxy

2017-03-23 Thread Dave J
Thanks very much Aaron, that did the trick :)  My config file now has the 
updated line


server TestServer01 10.6.186.24:48080 send-proxy





From: Aaron West 
Sent: 23 March 2017 17:59
To: Dave J
Cc: haproxy@formilux.org
Subject: Re: Send PROXY protocol header from HAProxy

Hi Dave,

I don't see the "send-proxy" directive in your config, have you tried it : 
https://cbonte.github.io/haproxy-dconv/1.8/configuration.html#5.2-send-proxy
HAProxy version 1.8-dev0 - Configuration 
Manual
cbonte.github.io
This document covers the configuration language as implemented in the version 
specified above. It does not provide any hint, example or advice.


Sorry if I'm misunderstanding something already...

Aaron West

Loadbalancer.org Limited
+44 (0)330 380 1064
www.loadbalancer.org

On 23 March 2017 at 12:38, Dave J 
mailto:mr_wad...@hotmail.com>> wrote:
I've probably got lost in the masses of documentation on this subject, but I'm 
trying to configure my HAProxy process to send the PROXY protocol header as 
described at http://www.haproxy.org/download/1.8/doc/proxy-protocol.txt. This 
is because I am having to write support for the PROXY protocol into a C++ 
server (in order for it to have access to the client IP/port) and I want to 
test my code is working properly with the parsing of the PROXY header.

Here is my minimal config file:

---

global
   maxconn 4096

defaults
   log   global
   mode   http
   retries   3
   option redispatch
   maxconn   2000
   timeout connect 5000
   timeout client  5
   timeout server  5

frontend TestServerTest
bind 10.6.186.24:54781
mode tcp
default_backend TestServernodes

backend TestServernodes
mode tcp
# Note there is no 'check' after the below line unlike the others as we 
don't want to send the
# healthcheck ("OPTIONS / HTTP/1.0"...) string to the TestServer as it 
doesn't understand it!
server TestServer01 10.6.186.24:48080

---

What I am finding is that when I start HAProxy and connect to 54781, the first 
data that TestServer at 48080 receives is the data which is sent from my 
client; it is not the PROXY header described at the link I posted.

Can someone please tell me what I am missing in my configuration that is 
preventing the PROXY header being sent to my backend server?

Thanks in advance.

Dave J





[PATCH] improve DNS response parsing

2017-03-23 Thread Baptiste
Hi all,

Currently, HAProxy picks up the first IP available in the response which
matches a familiy preference or a subnet preference.
That said, there are chances that this IP is already assigned to an other
server in the backend while some other IPs are unassigned in the same
response.
This patch aims at improving this situation: it tries to look for an IP
which is not assigned already.

Baptiste
From 79e032d6428bc900b12e99af64c7ce4608432c8c Mon Sep 17 00:00:00 2001
From: Baptiste 
Date: Mon, 26 Dec 2016 23:21:08 +0100
Subject: [PATCH] MINOR: dns: improve DNS response parsing to use as many
 available records as possible

A "weakness" exist in the first implementation of the parsing of the DNS
responses: HAProxy always choses the first IP available matching family
preference, or as a failover, the first IP.

It should be good enough, since most DNS servers do round robin on the
records they send back to clients.
That said, some servers does not do proper round robin, or we may be
unlucky too and deliver the same IP to all the servers sharing the same
hostname.

Let's take the simple configuration below:

  backend bk
srv s1 www:80 check resolvers R
srv s2 www:80 check resolvers R

The DNS server configured with 2 IPs for 'www'.
If you're unlucky, then HAProxy may apply the same IP to both servers.

Current patch improves this situation by weighting the decision
algorithm to ensure we'll prefer use first an IP found in the response
which is not already affected to any server.

The new algorithm does not guarantee that the chosen IP is healthy,
neither a fair distribution of IPs amongst the servers in the farm,
etc...
It only guarantees that if the DNS server returns many records for a
hostname and that this hostname is being resolved by multiple servers in
the same backend, then we'll use as many records as possible.
If a server fails, HAProxy won't pick up an other record from the
response.
---
 src/dns.c | 49 +++--
 1 file changed, 43 insertions(+), 6 deletions(-)

diff --git a/src/dns.c b/src/dns.c
index 5542b17..075a701 100644
--- a/src/dns.c
+++ b/src/dns.c
@@ -782,19 +782,23 @@ int dns_get_ip_from_response(struct dns_response_packet *dns_p,
 	 *
 	 * For these three priorities, a score is calculated. The
 	 * weight are:
-	 *  4 - prefered netwok ip version.
-	 *  2 - prefered network.
+	 *  8 - prefered netwok ip version.
+	 *  4 - prefered network.
+	 *  2 - if the ip in the record is not affected to any other server in the same backend (duplication)
 	 *  1 - current ip.
 	 * The result with the biggest score is returned.
 	 */
 	max_score = -1;
 	for (i = 0; i < rec_nb; i++) {
+		struct server *srv, *tmpsrv;
+		struct proxy *be;
+		int record_ip_already_affected = 0;
 
 		score = 0;
 
 		/* Check for prefered ip protocol. */
 		if (rec[i].type == family_priority)
-			score += 4;
+			score += 8;
 
 		/* Check for prefered network. */
 		for (j = 0; j < resol->opts->pref_net_nb; j++) {
@@ -811,11 +815,43 @@ int dns_get_ip_from_response(struct dns_response_packet *dns_p,
 			 in_net_ipv6(rec[i].ip,
 			 &resol->opts->pref_net[j].mask.in6,
 			 &resol->opts->pref_net[j].addr.in6))) {
-score += 2;
+score += 4;
 break;
 			}
 		}
 
+		/* Check if the IP found in the record is already affected to an other server. */
+		srv = resol->requester;
+		be = srv->proxy;
+		for (tmpsrv = be->srv; tmpsrv; tmpsrv = tmpsrv->next) {
+			/* We want to compare the IP in the record with the IP of the servers in the
+			 * same backend, only if:
+			 *   * DNS resolution is enabled on the server
+			 *   * the hostname used for the resolution by our server is the same than the
+			 * one used for the server found in the backend
+			 *   * the server found in the backend is not our current server
+			 */
+			if ((tmpsrv->resolution == NULL) ||
+			(srv->resolution->hostname_dn_len != tmpsrv->resolution->hostname_dn_len) ||
+			(strcmp(srv->resolution->hostname_dn, tmpsrv->resolution->hostname_dn) != 0) ||
+			(srv->puid == tmpsrv->puid))
+continue;
+
+			/* At this point, we have 2 different servers using the same DNS hostname
+			 * for their respective resolution.
+			 */
+			if (rec[i].type == tmpsrv->addr.ss_family &&
+			((tmpsrv->addr.ss_family == AF_INET &&
+			  memcmp(rec[i].ip, &((struct sockaddr_in *)&tmpsrv->addr)->sin_addr, 4) == 0) ||
+			 (tmpsrv->addr.ss_family == AF_INET6 &&
+			  memcmp(rec[i].ip, &((struct sockaddr_in6 *)&tmpsrv->addr)->sin6_addr, 16) == 0))) {
+record_ip_already_affected = 1;
+break;
+			}
+		}
+		if (!record_ip_already_affected)
+			score += 2;
+
 		/* Check for current ip matching. */
 		if (rec[i].type == currentip_sin_family &&
 		((currentip_sin_family == AF_INET &&
@@ -827,8 +863,9 @@ int dns_get_ip_from_response(struct dns_response_packet *dns_p,
 		} else
 			currentip_sel = 0;
 
+
 		/* Keep the address if the score is better tha

Re: [PATCH] MEDIUM: global: add a 'hard-stop-after' option to cap the soft-stop time

2017-03-23 Thread Cyril Bonté

Hi Willy,

Le 23/03/2017 à 07:42, Willy Tarreau a écrit :

Hi Cyril,

I have a few comments below :


diff --git a/src/cfgparse.c b/src/cfgparse.c
index 2eb25edb..9681e06b 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -1011,6 +1011,23 @@ int cfg_parse_global(const char *file, int linenum, char 
**args, int kwm)
}
}
/* end of user/group name handling*/
+   else if (strcmp(args[0], "hard-stop-after") == 0) {
+   const char *res;
+
+   if (!*args[1]) {
+   Alert("parsing [%s:%d] : '%s' expects  as 
argument.\n",
+   file, linenum, args[0]);
+   err_code |= ERR_ALERT | ERR_FATAL;
+   goto out;
+   }
+   res = parse_time_err(args[1], &global.hard_stop_after, 
TIME_UNIT_MS);
+   if (res) {
+   Alert("parsing [%s:%d]: unexpected character '%c' in argument to 
<%s>.\n",
+   file, linenum, *res, args[0]);
+   err_code |= ERR_ALERT | ERR_FATAL;
+   goto out;
+   }
+   }


You can place this directly as a keyword parser in proxy.c. For this
you can simply add the following line to cfg_kws :

   { CFG_GLOBAL, "hard-stop-after", proxy_parse_hard_stop_after },


Right, I missed that one for global keywords. It's ready in a new patch 
version.



+struct task *hard_stop(struct task *t)
+{
+   struct proxy *p;
+   struct stream *s;
+
+   Warning("soft-stop running for too long, performing a hard-stop.\n");
+   send_log(NULL, LOG_WARNING, "soft-stop running for too long, performing a 
hard-stop.\n");
+   p = proxy;
+   while (p) {
+   if (!(p->cap & PR_CAP_FE))
+   continue;
+
+   list_for_each_entry(s, &streams, list) {
+   stream_shutdown(s, SF_ERR_KILLED);
+   }
+   p = p->next;
+   }


You don't use "p" in this loop, I think you started using it initially
and now you don't need it anymore, so in fact when you visit the first
proxy you shut down all streams and then nothing is done when looping
over the remaining ones. So all the streams are visited once per
frontend.


Oops, I copied a portion of code from an older patch supposed to close 
the HTTP keep-alived connections, but I didn't clean everything.



That said, I'm not convinced by the benefit of killing all streams like
this given that at the moment it does nothing, it only wakes up their
task and immediately dies a few lines later in exit() without giving
them any chance to run. However I admit that if we later want to give a
chance to these tasks to complete (eg: by waiting one extra second and
coming back here again to exit for real) then it could be useful to do
it.

I'd be tempted to suggest that we set a flag and re-arm the timer to
come back here to exit so that tasks can stop cleanly, this would give
an opportunity to get a log for each of them, but on the other hand
almost all the tasks being killed late will be purely TCP and most of
the time users don't log pure TCP connections, so the benefit becomes
very low. Otherwise we can simply exit and not even kill them.

Hmmm just thinking, in fact there could be a benefit in letting them
run : ensuring that we close outgoing connections with an RST to avoid
accumulating TIME_WAITs.


Yes I was not convinced by the benefit too, but I prefered to close them 
exactly to avoid TIME_WAITs.



What do you think about trying something like this (not even compile-tested) :

static int killed;
struct stream *s;

if (killed) {
Warning("Some tasks resisted to hard-stop, exiting now.\n");
send_log(NULL, LOG_WARNING, "Some tasks resisted to hard-stop, 
exiting now.\n");
/* Do some cleanup and explicitely quit */
deinit();
exit(0);
}

Warning("soft-stop running for too long, performing a hard-stop.\n");
send_log(NULL, LOG_WARNING, "soft-stop running for too long, performing a 
hard-stop.\n");

/* kill the tasks and give them one second to terminate cleanly */
list_for_each_entry(s, &streams, list) {
stream_shutdown(s, SF_ERR_KILLED);
}

killed = 1;
t->expire = tick_add(now_ms, MS_TO_TICKS(1000));
return t;

Normally if all tasks properly die, we'll automatically break out of
run_poll_loops(). If we have to come back here, it means we didn't
quit so it's just a safety belt to exit here.


OK I see, I've put the logic in the next patch version.


Additionally, since it helps improving the reliability of the service
during reloads without adding particular features and the patch is
reasonably small and isolated, I was thinking we could backport it to
1.7 and even possibly 1.6. What do you and others think ?


+1 for backporting it i

Re: [PATCH] MEDIUM: global: add a 'hard-stop-after' option to cap the soft-stop time

2017-03-23 Thread Cyril Bonté

Hi Olivier and Willy,

Le 23/03/2017 à 14:50, Willy Tarreau a écrit :

Hi Olivier,

On Thu, Mar 23, 2017 at 02:17:38PM +0100, Olivier Doucet wrote:

I was also thinking about dumping how many connections were force-killed in
log, if it's easy.


I thought about it as well while responding but forgot to mention it. That's
also why I'd prefer to leave one extra second for the connections to cleanly
close and log. Yes it's important.


Of course I can add some logs. Currently, when haproxy enters 
"soft-stop", it sends one log line for each proxy, indicating it's 
stopping :

[WARNING] 081/213056 (19942) : Stopping proxy test in 0 ms.
[WARNING] 081/213056 (19942) : Proxy test stopped (FE: 500 conns, BE: 0 
conns).


I think we can add a new log line for each proxy having opened 
connections, where the number of remaining connections is read from 
"feconn":
[WARNING] 081/213101 (19942) : Proxy test hard-stopped (500 remaining 
conns will be closed).


or do you prefer a global counter ?

--
Cyril Bonté



Re: [PATCH] MEDIUM: global: add a 'hard-stop-after' option to cap the soft-stop time

2017-03-23 Thread Willy Tarreau
On Thu, Mar 23, 2017 at 09:35:28PM +0100, Cyril Bonté wrote:
> Hi Olivier and Willy,
> 
> Le 23/03/2017 à 14:50, Willy Tarreau a écrit :
> > Hi Olivier,
> > 
> > On Thu, Mar 23, 2017 at 02:17:38PM +0100, Olivier Doucet wrote:
> > > I was also thinking about dumping how many connections were force-killed 
> > > in
> > > log, if it's easy.
> > 
> > I thought about it as well while responding but forgot to mention it. That's
> > also why I'd prefer to leave one extra second for the connections to cleanly
> > close and log. Yes it's important.
> 
> Of course I can add some logs. Currently, when haproxy enters "soft-stop",
> it sends one log line for each proxy, indicating it's stopping :
> [WARNING] 081/213056 (19942) : Stopping proxy test in 0 ms.
> [WARNING] 081/213056 (19942) : Proxy test stopped (FE: 500 conns, BE: 0
> conns).
> 
> I think we can add a new log line for each proxy having opened connections,
> where the number of remaining connections is read from "feconn":
> [WARNING] 081/213101 (19942) : Proxy test hard-stopped (500 remaining conns
> will be closed).
> 
> or do you prefer a global counter ?

I didn't think about the per-proxy approach but I think that emitting a
log only for those having got killed connections would be nice given that
most of the time there will be very few such proxies. But I think it can
make your patch more complicated, especially if we target simplicity for
the backport. So maybe in fact having only the global counter first is a
good first step and if we can later improve on this we'll see if it's
worth backporting.

Thanks!
Willy



Re: [PATCH] MEDIUM: global: add a 'hard-stop-after' option to cap the soft-stop time

2017-03-23 Thread Cyril Bonté

Le 23/03/2017 à 21:40, Willy Tarreau a écrit :

On Thu, Mar 23, 2017 at 09:35:28PM +0100, Cyril Bonté wrote:
I didn't think about the per-proxy approach but I think that emitting a
log only for those having got killed connections would be nice given that
most of the time there will be very few such proxies. But I think it can
make your patch more complicated, especially if we target simplicity for
the backport. So maybe in fact having only the global counter first is a
good first step and if we can later improve on this we'll see if it's
worth backporting.


No, remember the useless loop on proxies in the patch I submitted ;-) I 
just have to leave it as is, send a log line for each proxy where 
fe_conn > 0 and move the stream_shutdown calls outside of this loop.


--
Cyril Bonté



Re: [PATCH] MEDIUM: global: add a 'hard-stop-after' option to cap the soft-stop time

2017-03-23 Thread Willy Tarreau
On Thu, Mar 23, 2017 at 09:43:44PM +0100, Cyril Bonté wrote:
> Le 23/03/2017 à 21:40, Willy Tarreau a écrit :
> > On Thu, Mar 23, 2017 at 09:35:28PM +0100, Cyril Bonté wrote:
> > I didn't think about the per-proxy approach but I think that emitting a
> > log only for those having got killed connections would be nice given that
> > most of the time there will be very few such proxies. But I think it can
> > make your patch more complicated, especially if we target simplicity for
> > the backport. So maybe in fact having only the global counter first is a
> > good first step and if we can later improve on this we'll see if it's
> > worth backporting.
> 
> No, remember the useless loop on proxies in the patch I submitted ;-) I just
> have to leave it as is, send a log line for each proxy where fe_conn > 0 and
> move the stream_shutdown calls outside of this loop.

Ah yes good point!

Willy



[PATCH] MEDIUM: global: add a 'hard-stop-after' option to cap the soft-stop time

2017-03-23 Thread Cyril Bonté
When SIGUSR1 is received, haproxy enters in soft-stop and quits when no
connection remains.
It can happen that the instance remains alive for a long time, depending
on timeouts and traffic. This option ensures that soft-stop won't run
for too long.

Example:
  global
hard-stop-after 30s  # Once in soft-stop, the instance will remain
 # alive for at most 30 seconds.
---
 doc/configuration.txt  | 17 ++
 include/types/global.h |  3 +++
 src/haproxy.c  |  5 +++-
 src/proxy.c| 64 ++
 4 files changed, 88 insertions(+), 1 deletion(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 73a4f4b8..fb3e691d 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -536,6 +536,7 @@ The following keywords are supported in the "global" 
section :
- external-check
- gid
- group
+   - hard-stop-after
- log
- log-tag
- log-send-hostname
@@ -703,6 +704,22 @@ gid 
   will only be able to drop these groups if started with superuser privileges.
   See also "group" and "uid".
 
+hard-stop-after 
+  Defines the maximum time allowed to perform a clean soft-stop.
+
+  Arguments :
+  is the maximum time (by default in milliseconds) for which the
+instance will remain alive when a soft-stop is received via the
+SIGUSR1 signal.
+
+  This may be used to ensure that the instance will quit even if connections
+  remain opened during a soft-stop (for example with long timeouts for a proxy
+  in tcp mode). It applies both in TCP and HTTP mode.
+
+  Example:
+global
+  hard-stop-after 30s
+
 group 
   Similar to "gid" but uses the GID of group name  from /etc/group.
   See also "gid" and "user".
diff --git a/include/types/global.h b/include/types/global.h
index e14a2add..df8e2c69 100644
--- a/include/types/global.h
+++ b/include/types/global.h
@@ -80,6 +80,7 @@ struct global {
int gid;
int external_check;
int nbproc;
+   unsigned int hard_stop_after;   /* maximum time allowed to perform a 
soft-stop */
int maxconn, hardmaxconn;
int maxsslconn;
int ssl_session_max_cost;   /* how many bytes an SSL session may cost */
@@ -170,6 +171,7 @@ extern const int zero;
 extern const int one;
 extern const struct linger nolinger;
 extern int stopping;   /* non zero means stopping in progress */
+extern int killed; /* non zero means a hard-stop is triggered */
 extern char hostname[MAX_HOSTNAME_LEN];
 extern char localpeer[MAX_HOSTNAME_LEN];
 extern struct list global_listener_queue; /* list of the temporarily limited 
listeners */
@@ -194,6 +196,7 @@ static inline int already_warned(unsigned int warning)
return 0;
 }
 
+void deinit(void);
 void hap_register_build_opts(const char *str, int must_free);
 void hap_register_post_check(int (*fct)());
 void hap_register_post_deinit(void (*fct)());
diff --git a/src/haproxy.c b/src/haproxy.c
index 559b4811..4f30d727 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -117,6 +117,7 @@ int  relative_pid = 1;  /* process id starting 
at 1 */
 
 /* global options */
 struct global global = {
+   .hard_stop_after = TICK_ETERNITY,
.nbproc = 1,
.req_count = 0,
.logsrvs = LIST_HEAD_INIT(global.logsrvs),
@@ -157,6 +158,7 @@ struct global global = {
 /*/
 
 int stopping;  /* non zero means stopping in progress */
+int killed;/* non zero means a hard-stop is triggered */
 int jobs = 0;   /* number of active jobs (conns, listeners, active tasks, ...) 
*/
 
 /* Here we store informations about the pids of the processes we may pause
@@ -593,6 +595,7 @@ static void init(int argc, char **argv)
 */
 
totalconn = actconn = maxfd = listeners = stopping = 0;
+   killed = 0;
 
 
 #ifdef HAPROXY_MEMMAX
@@ -1225,7 +1228,7 @@ static void deinit_stick_rules(struct list *rules)
}
 }
 
-static void deinit(void)
+void deinit(void)
 {
struct proxy *p = proxy, *p0;
struct cap_hdr *h,*h_next;
diff --git a/src/proxy.c b/src/proxy.c
index 19eddcac..d158fac0 100644
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -914,6 +914,58 @@ struct task *manage_proxy(struct task *t)
 }
 
 
+static int proxy_parse_hard_stop_after(char **args, int section_type, struct 
proxy *curpx,
+struct proxy *defpx, const char *file, int 
line,
+char **err)
+{
+   const char *res;
+
+   if (!*args[1]) {
+   memprintf(err, "'%s' expects  as argument.\n", args[0]);
+   return -1;
+   }
+   res = parse_time_err(args[1], &global.hard_stop_after, TIME_UNIT_MS);
+   if (res) {
+   memprintf(err, "unexpected character '%c' in argument to 
<%s>.\n", *res, args[0]);
+   return -1;
+   }
+   return 0;
+}
+
+struct task *hard_stop(struct task *

Re: [PATCH] MEDIUM: global: add a 'hard-stop-after' option to cap the soft-stop time

2017-03-23 Thread Willy Tarreau
On Thu, Mar 23, 2017 at 10:44:13PM +0100, Cyril Bonté wrote:
> When SIGUSR1 is received, haproxy enters in soft-stop and quits when no
> connection remains.
> It can happen that the instance remains alive for a long time, depending
> on timeouts and traffic. This option ensures that soft-stop won't run
> for too long.
(...)

Looks pretty good, I've just merged it now so that it's available in
tomorrow morning's snapshot for others to give it a try. But I'm not
worried at all.

Thanks Cyril!
Willy



Binding to interface as non-root user

2017-03-23 Thread Ankit Malp
tldr; Is there a way to bind a frontend to interface and still be able to
start HAProxy as root and later lower privileges to a non root user?

I asked this question at
http://serverfault.com/questions/840039/haproxy-interface-eth-aware-binding-as-non-root-user
but
did not get replies and thought this community might be a better place. I
have scenario where i need to listen explicitly on network interfaces. This
works great if i do not set an explicit lower privileged user (proxy runs
as root throughout its life).

However, I would prefer to not run the proxy as root.

Config snippet

global
#Works only without below line but its implication is running as root user
user haproxy

frontend frontend_tcp_eth1
mode tcp
bind 0.0.0.0:80 interface eth1


Reading through the docs, i only see root permissions necessary to bind for
outgoing connections but not for listening to an interface. Am I missing
something?

https://cbonte.github.io/haproxy-dconv/1.6/management.html#13
"HAProxy will need to be started as root in order to :
   - adjust the file descriptor limits
   - bind to privileged port numbers
   - bind to a specific network interface
   - transparently listen to a foreign address
   - isolate itself inside the chroot jail
   - drop to another non-privileged UID
HAProxy may require to be run as root in order to :
   - bind to an interface for outgoing connections
   - bind to privileged source ports for outgoing connections
   - transparently bind to a foreing address for outgoing connections
Most users will never need the "run as root" case. But the "start as root"
covers most usages."

Thanks,
Ankit