Re: [PATCH] MINOR: send-proxy-v2: sends authority TLV according to TLV received

2019-09-02 Thread Emmanuel Hocdet


> Le 31 août 2019 à 12:29, Willy Tarreau  a écrit :
> 
> Hi Manu,
> 
> On Thu, Aug 29, 2019 at 03:22:11PM +0200, Emmanuel Hocdet wrote:
>> This patch follows Geoff's patch.
> 
> Thanks for this. I didn't remember we automatically copied the SNI
> into the PP. I'm suspecting that sooner or later we'll need a
> "set-authority" action to complete the set-dst and so-on. We'll see.
> 
> Now merged, thanks,
> Willy
> 
Thanks.

Yes, it’s the next step to set authority, but I wonder what is the right 
approach.
. simply, on server line: proxy-v2-authority  (like it is with sni)
  but I would prefer a more generic way.
. more generic (as you suggest): add a « set-authority » for srv_conn,
  and reused the new « proxy_authority » entry to store it.
 
With « set-authority », in usage we will need a sample fetch « authority » as
« bc_authority » ?
Use case could be « sni bc_authority ».
For PPv2 authority, we will automatically use, in the order: bc_authority, 
fc_pp_authority, ssl_fc_sni.

++
Manu




Re: HAProxy 2.0 "stick on src table mypeers/mytable" does not result in peers binding to socket address

2019-09-02 Thread Willy Tarreau
Hi Fred,

On Mon, Sep 02, 2019 at 02:23:19PM +0200, Frederic Lecaille wrote:
> On 8/31/19 12:20 PM, Willy Tarreau wrote:
> > Hi Bruno,
> > 
> > On Sat, Aug 31, 2019 at 12:49:15AM +, Bruno Henc wrote:
> > > Greetings,
> > > 
> > > Using "stick on src table mypeers/stickysrc" in a backend results in 
> > > HAProxy
> > > deciding not to bind to the appropriate peers address for the local host
> > > (i.e. HAProxy thinks there are no stick tables in use). However using a
> > > http-request track-sc0 line will result in haproxy listening on the peers
> > > address. Also, defining the stick table in the backend itself or in a 
> > > dummy
> > > backend also works.
> > 
> > That's interesting. I suspect that the modifications to make the
> > stick-table name resolution work recently overlooked the "stick" keyword.
> > It's possible that it used to work differently from what was done later
> > with the track-* keyword, since "stick" was the very first implementation
> > of the stick tables. We'll see this with Fred on Monday.
> 
> Willy,
> 
> You are completely true. Some initializations were missing in relation with
> "stick" keyword.
> 
> Here is a patch to fix this issue.

Now merged, thank you!
Willy



Re: HAProxy 2.0 "stick on src table mypeers/mytable" does not result in peers binding to socket address

2019-09-02 Thread Frederic Lecaille

On 8/31/19 12:20 PM, Willy Tarreau wrote:

Hi Bruno,

On Sat, Aug 31, 2019 at 12:49:15AM +, Bruno Henc wrote:

Greetings,

Using "stick on src table mypeers/stickysrc" in a backend results in HAProxy
deciding not to bind to the appropriate peers address for the local host
(i.e. HAProxy thinks there are no stick tables in use). However using a
http-request track-sc0 line will result in haproxy listening on the peers
address. Also, defining the stick table in the backend itself or in a dummy
backend also works.


That's interesting. I suspect that the modifications to make the
stick-table name resolution work recently overlooked the "stick" keyword.
It's possible that it used to work differently from what was done later
with the track-* keyword, since "stick" was the very first implementation
of the stick tables. We'll see this with Fred on Monday.


Willy,

You are completely true. Some initializations were missing in relation 
with "stick" keyword.


Here is a patch to fix this issue.

Fred.
>From f2e2968b46e7a1e98c9a52f2abb419b7650990eb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20L=C3=A9caille?= 
Date: Mon, 2 Sep 2019 14:02:28 +0200
Subject: [PATCH] BUG/MEDIUM: peers: local peer socket not bound.

This bug came with 015e4d7 commit: "MINOR: stick-tables: Add peers process
binding computing" where the "stick" rules cases were missing when computing
the peer local listener process binding. At parsing time we store in the
stick-table struct ->proxies_list the proxies which refer to this stick-table.
The process binding is computed after having parsed the entire configuration file
with this simple loop in cfgparse.c:

 /* compute the required process bindings for the peers from 
  * for all the stick-tables, the ones coming with "peers" sections included.
  */
 for (t = stktables_list; t; t = t->next) {
 struct proxy *p;

 for (p = t->proxies_list; p; p = p->next_stkt_ref) {
 if (t->peers.p && t->peers.p->peers_fe) {
 t->peers.p->peers_fe->bind_proc |= p->bind_proc;
 }
 }
 }

Note that if this process binding is not correctly initialized, the child forked
by the master-worker stops the peer local listener. Should be also the case
when daemonizing haproxy.

Must be backported to 2.0.
---
 src/cfgparse.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/src/cfgparse.c b/src/cfgparse.c
index ecf62f997..9c2ac141b 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -2726,6 +2726,10 @@ int check_config_validity()
 mrule->table.t = target;
 stktable_alloc_data_type(target, STKTABLE_DT_SERVER_ID, NULL);
 stktable_alloc_data_type(target, STKTABLE_DT_SERVER_NAME, NULL);
+if (!in_proxies_list(target->proxies_list, curproxy)) {
+	curproxy->next_stkt_ref = target->proxies_list;
+	target->proxies_list = curproxy;
+}
 			}
 		}
 
@@ -2760,6 +2764,10 @@ int check_config_validity()
 mrule->table.t = target;
 stktable_alloc_data_type(target, STKTABLE_DT_SERVER_ID, NULL);
 stktable_alloc_data_type(target, STKTABLE_DT_SERVER_NAME, NULL);
+if (!in_proxies_list(target->proxies_list, curproxy)) {
+	curproxy->next_stkt_ref = target->proxies_list;
+	target->proxies_list = curproxy;
+}
 			}
 		}
 
-- 
2.20.1



Setting SSL/TLS options but still allow some exceptions

2019-09-02 Thread Olivier D
Hello,
I'm rewriting a complex HAProxy config file and would like to be sure how
ssl-default-bind-options and bind options work together.

I would like to configure safe options by default, but still allow
less-safe protocols on some frontend. I'm puzzled by "force-X"
documentation (does it really "force" protocol or just allow it ? What if I
use several force-X options all together ?) and want to be sure of the
behaviour.

Here is what I would like to do :
frontend foo : supports TLS 1.2 and TLS 1.3
frontend foo-unsecure : supports everything from sslv3 to TLS 1.3
frontend foo-unsecure2 : supports TLS 1.1 to TLS 1.3

And here is how I would write it down :

# Default (safe) config :
ssl-default-bind-options no-sslv3 no-tls10 no-tls11

frontend foo
bind 127.0.0.1:8080 ssl

frontend foo-unsecure
bind 127.0.0.1:1234 ssl force-sslv3 force-tls10 force-tls11

frontend foo-unsecure2
bind 127.0.0.1:4321 ssl force-tls11


I dont want to use 'ssl-min-ver' or 'ssl-max-ver' because the config file
is auto-generated from a database, and it would make the code more
difficult.

Thank you for your feedback.

Olivier


BUG/MEDIUM: spoe: engine-id is necessary if not health check

2019-09-02 Thread Kevin Zhu
Hi Christopher

SPOE engine-id is all same when nbproc is more than 1, the clients all
group under same engine, and same stream-id and frame-id frames may come at
same time. So we alway need engine-id to identify client's and frames.
Best Regards
From caedabe336897704b4da39818ac7d226c7b8a9db Mon Sep 17 00:00:00 2001
From: Kevin Zhu 
Date: Mon, 2 Sep 2019 13:45:34 +0800
Subject: [PATCH] BUG/MEDIUM: spoe: engine-id is necessary if not health check

SPOE engine-id is all same when nbproc is more than 1, the clients all
group under same engine, and same stream-id and frame-id frames may come at
same time. So we alway need engine-id to identify client's and frames.
---
 doc/SPOE.txt   |  8 
 src/flt_spoe.c | 35 +--
 2 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/doc/SPOE.txt b/doc/SPOE.txt
index 19f00ad..dd8f920 100644
--- a/doc/SPOE.txt
+++ b/doc/SPOE.txt
@@ -932,6 +932,10 @@ Following items are mandatory in the KV-LIST:
 This a comma-separated list of capabilities supported by HAProxy. Spaces
 must be ignored, if any.
 
+  * "engine-id"
+
+This is a uniq string that identify a SPOE engine.
+
 Following optional items can be added in the KV-LIST:
 
   * "healthcheck"
@@ -939,10 +943,6 @@ Following optional items can be added in the KV-LIST:
 If this item is set to TRUE, then the HAPROXY-HELLO frame is sent during a
 SPOE health check. When set to FALSE, this item can be ignored.
 
-  * "engine-id"
-
-This is a uniq string that identify a SPOE engine.
-
 To finish the HELLO handshake, the agent must return an AGENT-HELLO frame with
 its supported SPOP version, the lower value between its maximum size allowed
 for a frame and the HAProxy one and capabilities it supports. If an error
diff --git a/src/flt_spoe.c b/src/flt_spoe.c
index b8323f6..06ceeae 100644
--- a/src/flt_spoe.c
+++ b/src/flt_spoe.c
@@ -267,7 +267,7 @@ generate_pseudo_uuid()
 		return NULL;
 
 	if (!init) {
-		srand(now_ms);
+		srand(now_ms + pid);
 		init = 1;
 	}
 
@@ -457,17 +457,18 @@ spoe_prepare_hahello_frame(struct appctx *appctx, char *frame, size_t size)
 	if (spoe_encode_buffer(chk->area, chk->data, , end) == -1)
 		goto too_big;
 
-	/* (optionnal) "engine-id" K/V item, if present */
-	if (agent != NULL && agent->engine_id != NULL) {
-		sz = SLEN(ENGINE_ID_KEY);
-		if (spoe_encode_buffer(ENGINE_ID_KEY, sz, , end) == -1)
-			goto too_big;
+	/* "engine-id" K/V item */
+	sz = SLEN(ENGINE_ID_KEY);
+	if (spoe_encode_buffer(ENGINE_ID_KEY, sz, , end) == -1)
+		goto too_big;
 
-		*p++ = SPOE_DATA_T_STR;
+	*p++ = SPOE_DATA_T_STR;
+	if (agent != NULL) {
 		sz = strlen(agent->engine_id);
 		if (spoe_encode_buffer(agent->engine_id, sz, , end) == -1)
 			goto too_big;
-	}
+	} else if (spoe_encode_buffer(NULL, 0, , end) == -1)
+		goto too_big;
 
 	return (p - frame);
 
@@ -3111,6 +3112,21 @@ spoe_check(struct proxy *px, struct flt_conf *fconf)
 	return 0;
 }
 
+/* Initializes the SPOE filter for a proxy for a specific thread.
+ * Returns a negative value if an error occurs. */
+static int
+spoe_init_per_thread(struct proxy *p, struct flt_conf *fconf)
+{
+	struct spoe_agent *agent = ((struct spoe_config*)fconf->conf)->agent;
+	if (agent->engine_id == NULL)
+		agent->engine_id = generate_pseudo_uuid();
+
+	if (agent->engine_id == NULL)
+		return -1;
+
+	return 0;
+}
+
 /**
  * Hooks attached to a stream
  */
@@ -3309,6 +3325,7 @@ struct flt_ops spoe_ops = {
 	.init   = spoe_init,
 	.deinit = spoe_deinit,
 	.check  = spoe_check,
+	.init_per_thread = spoe_init_per_thread,
 
 	/* Handle start/stop of SPOE */
 	.attach = spoe_start,
@@ -4177,8 +4194,6 @@ parse_spoe_flt(char **args, int *cur_arg, struct proxy *px,
 		}
 		curagent->var_pfx = strdup(curagent->id);
 	}
-	if (curagent->engine_id == NULL)
-		curagent->engine_id = generate_pseudo_uuid();
 
 	if (curagent->var_on_error) {
 		struct arg arg;
-- 
2.7.4