Re: [PR] Add srvkey option to stick-table

2020-12-09 Thread Thayne McCombs
Here are my updates from the feedback.  It took me quite a while to figure out 
how to send this diff properly formatted with gmail.

If you would like a single patch with all my changes, I can send that as well.

>From 9c71b643e993dcafca36feae71cdfacc24ffaaa2 Mon Sep 17 00:00:00 2001
From: Thayne McCombs 
Date: Sat, 5 Dec 2020 23:09:24 -0700
Subject: [PATCH 1/2] Feedback from initial review

---
 doc/configuration.txt  |  4 ++--
 include/haproxy/dict.h |  1 +
 include/haproxy/server-t.h |  1 -
 src/cfgparse-listen.c  |  1 +
 src/dict.c | 24 +-
 src/peers.c|  7 ++-
 src/server.c   | 41 --
 src/stick_table.c  |  8 
 src/stream.c   |  2 +-
 src/tools.c|  3 ++-
 10 files changed, 66 insertions(+), 26 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 95c9abd8c..d1f7374ea 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -10711,8 +10711,8 @@ stick-table type {ip | integer | string [len ] 
| binary [len ]}
specifies how each server is identified for the purposes of the
stick table. The valid values are "name" and "addr". If "name" 
is
given, then  argument for the server (may be generated by
-   a template). If "addr" is given, then the server is idntified
-   by it's current network address, including the port. "addr" is
+   a template). If "addr" is given, then the server is identified
+   by its current network address, including the port. "addr" is
especially useful if you are using service discovery to generate
the addresses for servers with peered stick-tables and want
to consistently use the same host across peers for a stickiness
diff --git a/include/haproxy/dict.h b/include/haproxy/dict.h
index 59e81352c..c55834ca5 100644
--- a/include/haproxy/dict.h
+++ b/include/haproxy/dict.h
@@ -31,5 +31,6 @@
 
 struct dict *new_dict(const char *name);
 struct dict_entry *dict_insert(struct dict *d, char *str);
+void dict_entry_unref(struct dict *d, struct dict_entry *de);
 
 #endif  /* _HAPROXY_DICT_H */
diff --git a/include/haproxy/server-t.h b/include/haproxy/server-t.h
index 5a4dadfbd..13f5a5dab 100644
--- a/include/haproxy/server-t.h
+++ b/include/haproxy/server-t.h
@@ -274,7 +274,6 @@ struct server {
const struct netns_entry *netns;/* contains network namespace 
name or NULL. Network namespace comes from configuration */
/* warning, these structs are huge, keep them at the bottom */
struct sockaddr_storage addr;   /* the address to connect to, 
doesn't include the port */
-   char *addr_desc;/* string description of the 
address and port for the server */
struct xprt_ops *xprt;  /* transport-layer operations */
unsigned int svc_port;  /* the port to connect to (for 
relevant families) */
unsigned down_time; /* total time the server was 
down */
diff --git a/src/cfgparse-listen.c b/src/cfgparse-listen.c
index 97a97e746..a493e741c 100644
--- a/src/cfgparse-listen.c
+++ b/src/cfgparse-listen.c
@@ -457,6 +457,7 @@ int cfg_parse_listen(const char *file, int linenum, char 
**args, int kwm)
curproxy->grace  = defproxy.grace;
curproxy->conf.used_listener_id = EB_ROOT;
curproxy->conf.used_server_id = EB_ROOT;
+   curproxy->used_server_addr = EB_ROOT_UNIQUE;
 
if (defproxy.check_path)
curproxy->check_path = strdup(defproxy.check_path);
diff --git a/src/dict.c b/src/dict.c
index 903f07323..9b3536d96 100644
--- a/src/dict.c
+++ b/src/dict.c
@@ -87,8 +87,10 @@ struct dict_entry *dict_insert(struct dict *d, char *s)
HA_RWLOCK_RDLOCK(DICT_LOCK, &d->rwlock);
de = __dict_lookup(d, s);
HA_RWLOCK_RDUNLOCK(DICT_LOCK, &d->rwlock);
-   if (de)
+   if (de) {
+   HA_ATOMIC_ADD(&de->refcount, 1);
return de;
+   }
 
de = new_dict_entry(s);
if (!de)
@@ -105,3 +107,23 @@ struct dict_entry *dict_insert(struct dict *d, char *s)
return de;
 }
 
+
+/*
+ * Unreference a dict entry previously acquired with .
+ * If this is the last live reference to the entry, it is
+ * removed from the dictionary.
+ */
+void dict_entry_unref(struct dict *d, struct dict_entry *de)
+{
+   if (!de)
+   return;
+
+   if (HA_ATOMIC_SUB(&de->refcount, 1) != 0)
+   return;
+
+   HA_RWLOCK_WRLOCK(DICT_LOCK, &d->rwlock);
+   ebpt_delete(&de->value);
+   HA_RWLOCK_WRUNLOCK(DICT_LOCK, &d->rwlock);
+
+   free_dict_entry(de);
+}
diff --git a/src/peers.c b/src/peers.c
index 3f4e2c575..3fa1a28b4 100644
--- a/src/peers.c
+++ b/src/peers.c
@@ -1634,12 +1634,15 @@ stati

[PATCH] BUG/MINOR: haproxy: Add a check whether the given pid is "haproxy"

2020-12-09 Thread mizuta.take...@fujitsu.com
Hi, all,

Haproxy sends the signals to pidlist with the -sf/-st option.
However, since it does not check the haproxy's PID, it may kill other processes.

Reproducer is:
  ~]# sleep 1000 & sleep 1 ; haproxy -st $! -f /etc/haproxy/haproxy.cfg
  [1] 1909
  [1]+  Terminated  sleep 1000

In the -sf/-st option, I attach a patch to check whether the PID is haproxy or 
not.

Best regards,
MIZUTA Takeshi


0001-BUG-MINOR-haproxy-Add-a-check-whether-the-given-pid-.patch
Description:  0001-BUG-MINOR-haproxy-Add-a-check-whether-the-given-pid-.patch


RE: [PATCH] BUG/MINOR: mworker: delete the pidfile when the master process is stopped

2020-12-09 Thread mizuta.take...@fujitsu.com
Dear William-san,

Sorry for the late reply.

> I'm not convince that it should be done within HAProxy, you will still
> have the same problem if the master crashes.

I understand that it is the same when the master crashes.
The remaining pid file is unavoidable,
but I think it is necessary to check if the PID written in the pidfile is the 
haproxy's PID.
Otherwise, the -sf/-st option could kill other processes.

I will send a patch to solve this problem.

Best regards,
MIZUTA Takeshi

> -Original Message-
> From: William Lallemand 
> Sent: Tuesday, October 13, 2020 5:42 PM
> To: Mizuta, Takeshi 
> Cc: 'HAProxy' 
> Subject: Re: [PATCH] BUG/MINOR: mworker: delete the pidfile when the
> master process is stopped
> 
> On Thu, Oct 08, 2020 at 04:59:46AM +, mizuta.take...@fujitsu.com
> wrote:
> > Hi, all,
> >
> > haproxy does not delete the pidfile when stopped.
> > If the PID in the remaining pidfile is reused by other process,
> > the operation using the pidfile may affect other process.
> > In master-worker mode, fixed to delete pidfile when stopped.
> >
> > However, in the daemon mode, it has not been dealt with yet.
> > Is there any objection to dealing only with master-worker mode?
> >
> > Best regards,
> > MIZUTA Takeshi
> 
> 
> Hello,
> 
> I'm not convince that it should be done within HAProxy, you will still
> have the same problem if the master crashes.
> 
> It's also a big change of behavior that could break existing scripts.
> 
> In my opinion this should be done this in your init script.
> 
> --
> William Lallemand



Re: dynamic ssl certificate updates with changed intermediate

2020-12-09 Thread William Lallemand
On Tue, Dec 08, 2020 at 06:42:13PM +0100, Björn Jacke wrote:
> Hi William,
> 
>  On 08.12.20 15:13, William Lallemand wrote:> I then updated the
> certificate this way:
> > 
> > $ echo -e -n "@1 set ssl cert server1.fullchain.pem <<\n$(cat 
> > server2.fullchain.pem)\n\n" | socat - /tmp/master.socket 
> > Transaction created for certificate server1.fullchain.pem!
> > 
> > $ echo "@1 commit ssl cert server1.fullchain.pem" | socat - 
> > /tmp/master.socket 
> > Committing server1.fullchain.pem.
> > Success!
> > 
> > And checked that the certificate is correctly updated:
> 
> true, what fail though is the dynamic ocsp-response update after that,
> sorry for the unprecise problem description before. This happens after a
> dynamic cert update that *includes* an intermediate cert update if you
> then also try make a dynamic ocsp-response update:
> 
> # echo "set ssl ocsp-response $(base64 -w 1 ${DIRNAME}/ocsp.der)" |
> socat ...
> 
> OCSP single response: Certificate ID does not match any certificate or
> issuer.
> 

Hello,

Okay thanks for confirming.

Could you try this method?

$ echo -e -n "@1 set ssl cert server1.fullchain.pem <<\n$(cat 
server2.fullchain.pem)\n\n" | socat - /tmp/master.socket
$ echo -e "@1 set ssl cert server1.fullchain.pem.ocsp <<\n$(base64 -w 
1 server2.fullchain.ocsp)\n" | socat - /tmp/master.socket
$ echo "@1 commit ssl cert server1.fullchain.pem" | socat - 
/tmp/master.socket

It should activate the OCSP with the new SSL context.

-- 
William Lallemand



Re: contrib/spoa/python: A few doc typo and bug fixes

2020-12-09 Thread Christopher Faulet

Le 08/12/2020 à 18:57, Gilchrist DADAGLO a écrit :

No issue with backporting to 2.0. I just mentioned 2.2 as it's the last.



Ok, now applied and marked to be backported as far as 2.0. Thanks !

--
Christopher Faulet