On Wed, Mar 15, 2017 at 03:52:04PM +0200, Jarno Huuskonen wrote:
> Hi Olivier,
>
> On Tue, Mar 14, Olivier Houchard wrote:
> > Hi guys,
> >
> > You'll find attached patches to add support for dynamically-generated
> > session
> > cookies for each server, the said cookies will be a hash of the IP, the
> > TCP port, and a secret key provided.
> > This adds 2 keywords to the config file, a "dynamic" keyword in the cookie
> > line, which just enables dynamic cookie, and a "dynamic-cookie-key" keyword,
> > for backends, which specifies the secret key to use.
> > This is a first step to be able to add and remove servers on the fly,
> > without modifying the configuration. That way, we can have cookies that are
> > automatically usable for all the load-balancers.
> >
> > Any comment would be welcome.
>
> If I omit dynamic-cookie-key then I'll get a crash:
> (gdb) bt
> #0 0x00007ffff6a72e71 in __strlen_sse2_pminub () from /lib64/libc.so.6
> #1 0x000000000044bbe4 in srv_set_dyncookie (s=s@entry=0x720050)
> at src/server.c:88
> (I think p->dyncookie_key is NULL).
>
> #2 0x0000000000429720 in check_config_validity () at src/cfgparse.c:8480
> #3 0x0000000000487b25 in init (argc=<optimized out>, argc@entry=4,
> argv=<optimized out>, argv@entry=0x7fffffffdcb8) at src/haproxy.c:814
> #4 0x000000000040820b in main (argc=4, argv=0x7fffffffdcb8)
> at src/haproxy.c:1643
>
> with this config(defaults/global omitted):
> frontend test
> bind ipv4@127.0.0.1:8080
> default_backend test_be
>
> backend test_be
> balance roundrobin
> #dynamic-cookie-key "dymmykey"
> cookie WEBSRV insert dynamic
> server wp1 127.0.0.1:8084 id 1
> server wp2 127.0.0.1:8085 id 2
>
Oops my bad, I'm an idiot.
Willy, can you commit the attached patch ?
> Is the hash same with little/big endian processors ?
> memcpy(&(tmpbuf[key_len]),
> s->addr.ss_family == AF_INET ?
> (void *)&((struct sockaddr_in *)&s->addr)->sin_addr.s_addr :
> (void *)&(((struct sockaddr_in6 *)&s->addr)->sin6_addr.s6_addr),
> addr_len);
>
I expect those to be stored in network byte order, so it should be.
> -Jarno
>
> (Also there's a minor typo in comments:
> > +/* Calculates the dynamic persitent cookie for a server, if a secret key
> > has
> > + * been provided.
> > + */
> ...
> > + else if (!strcmp(args[cur_arg], "dynamic")) { /*
> > Dynamic persitent cookies secret key */
>
> s/persitent/persistent/)
>
Oops, patch #2 :)
Thanks a lot !
Olivier
>From f9a92fd9a624962860af9c61208d60e304e60a52 Mon Sep 17 00:00:00 2001
From: Olivier Houchard <ohouch...@haproxy.com>
Date: Wed, 15 Mar 2017 15:11:06 +0100
Subject: [PATCH 1/2] BUG/MEDIUM server: Fix crash when dynamic is defined, but
not key is provided.
X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4
Wait until we're sure we have a key before trying to calculate its length.
---
src/server.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/server.c b/src/server.c
index 4e03e50..5589723 100644
--- a/src/server.c
+++ b/src/server.c
@@ -85,7 +85,7 @@ void srv_set_dyncookie(struct server *s)
struct server *tmpserv;
char *tmpbuf;
unsigned long long hash_value;
- size_t key_len = strlen(p->dyncookie_key);
+ size_t key_len;
size_t buffer_len;
int addr_len;
int port;
@@ -94,6 +94,7 @@ void srv_set_dyncookie(struct server *s)
!(s->proxy->ck_opts & PR_CK_DYNAMIC) ||
s->proxy->dyncookie_key == NULL)
return;
+ key_len = strlen(p->dyncookie_key);
if (s->addr.ss_family != AF_INET &&
s->addr.ss_family != AF_INET6)
--
2.9.3
>From c576bb09e643b34798056c83182752e872e578c1 Mon Sep 17 00:00:00 2001
From: Olivier Houchard <ohouch...@haproxy.com>
Date: Wed, 15 Mar 2017 15:12:06 +0100
Subject: [PATCH 2/2] MINOR: Typo in comment.
X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4
---
src/cfgparse.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/cfgparse.c b/src/cfgparse.c
index 5a09589..2eb25ed 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -3310,7 +3310,7 @@ int cfg_parse_listen(const char *file, int linenum, char
**args, int kwm)
curproxy->cookie_maxlife = maxlife;
cur_arg++;
}
- else if (!strcmp(args[cur_arg], "dynamic")) { /*
Dynamic persitent cookies secret key */
+ else if (!strcmp(args[cur_arg], "dynamic")) { /*
Dynamic persistent cookies secret key */
if (warnifnotcap(curproxy, PR_CAP_BE, file,
linenum, args[cur_arg], NULL))
err_code |= ERR_WARN;
--
2.9.3
