Welcome! Log In Create A New Profile

Advanced

Dynamic cookies support

Posted by Olivier Houchard 
Olivier Houchard
Dynamic cookies support
March 14, 2017 08:30PM
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.

Thanks !

Olivier
From a29344438de3777ab692978b5195adfd100f219f Mon Sep 17 00:00:00 2001
From: Olivier Houchard <[email protected]>
Date: Tue, 14 Mar 2017 20:01:29 +0100
Subject: [PATCH 1/2] MINOR: server: Add dynamic session cookies.
X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4

This adds a new "dynamic" keyword for the cookie option. If set, a cookie
will be generated for each server (assuming one isn't already provided on
the "server" line), from the IP of the server, the TCP port, and a secret
key provided. To provide the secret key, a new keyword as been added,
"dynamic-cookie-key", for backends.

Example :
backend bk_web
balance roundrobin
dynamic-cookie-key "bla"
cookie WEBSRV insert dynamic
server s1 127.0.0.1:80 check
server s2 192.168.56.1:80 check

This is a first step to be able to dynamically add and remove servers,
without modifying the configuration file, and still have all the load
balancers redirect the traffic to the right server.

Provide a way to generate session cookies, based on the IP address of the
server, the TCP port, and a secret key provided.
---
doc/configuration.txt | 21 +++++++++++++
include/proto/server.h | 5 +++
include/types/proxy.h | 2 ++
include/types/server.h | 1 +
src/cfgparse.c | 42 ++++++++++++++++++++++++-
src/server.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++--
6 files changed, 152 insertions(+), 4 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index a505f70..5cc1bdd 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -2771,6 +2771,7 @@ contimeout <timeout> (deprecated)
cookie <name> [ rewrite | insert | prefix ] [ indirect ] [ nocache ]
[ postonly ] [ preserve ] [ httponly ] [ secure ]
[ domain <domain> ]* [ maxidle <idle> ] [ maxlife <life> ]
+ [ dynamic ]
Enable cookie-based persistence in a backend.
May be used in sections : defaults | frontend | listen | backend
yes | no | yes | yes
@@ -2922,6 +2923,13 @@ cookie <name> [ rewrite | insert | prefix ] [ indirect ] [ nocache ]
is stronger than the maxidle method in that it forces a
redispatch after some absolute delay.

+ dynamic Activate dynamic cookies. When used, a session cookie is
+ dynamically created for each server, based on the IP and port
+ of the server, and a secret key, specified in the
+ "dynamic-cookie-key" backend directive.
+ The cookie will be regenerated each time the IP address change,
+ and is only generated for IPv4/IPv6.
+
There can be only one persistence cookie per HTTP backend, and it can be
declared in a defaults section. The value of the cookie will be the value
indicated after the "cookie" keyword in a "server" statement. If no cookie
@@ -3042,6 +3050,19 @@ dispatch <address>:<port>
See also : "server"


+dynamic-cookie-key <string>
+ Set the dynamic cookie secret key for a backend.
+ May be used in sections : defaults | frontend | listen | backend
+ yes | no | yes | yes
+ Arguments : The secret key to be used.
+
+ When dynamic cookies are enabled (see the "dynamic" directive for cookie),
+ a dynamic cookie is created for each server (unless one is explicitely
+ specified on the "server" line), using a hash of the IP address of the
+ server, the TCP port, and the secret key.
+ That way, we can ensure session persistence accross multiple load-balancers,
+ even if servers are dynamically added or removed.
+
enabled
Enable a proxy, frontend or backend.
May be used in sections : defaults | frontend | listen | backend
diff --git a/include/proto/server.h b/include/proto/server.h
index 7c9574e..6e3ccf3 100644
--- a/include/proto/server.h
+++ b/include/proto/server.h
@@ -204,6 +204,11 @@ void srv_set_admin_flag(struct server *s, enum srv_admin mode, const char *cause
*/
void srv_clr_admin_flag(struct server *s, enum srv_admin mode);

+/* Calculates the dynamic persitent cookie for a server, if a secret key has
+ * been provided.
+ */
+void srv_set_dyncookie(struct server *s);
+
/* Puts server <s> into maintenance mode, and propagate that status down to all
* tracking servers.
*/
diff --git a/include/types/proxy.h b/include/types/proxy.h
index 3f848a0..5306a3b 100644
--- a/include/types/proxy.h
+++ b/include/types/proxy.h
@@ -189,6 +189,7 @@ enum PR_SRV_STATE_FILE {
#define PR_CK_PSV 0x00000040 /* cookie ... preserve */
#define PR_CK_HTTPONLY 0x00000080 /* emit the "HttpOnly" attribute */
#define PR_CK_SECURE 0x00000100 /* emit the "Secure" attribute */
+#define PR_CK_DYNAMIC 0x00000200 /* create dynamic cookies for each server */

/* bits for sticking rules */
#define STK_IS_MATCH 0x00000001 /* match on request fetch */
@@ -282,6 +283,7 @@ struct proxy {
char *cookie_domain; /* domain used to insert the cookie */
char *cookie_name; /* name of the cookie to look for */
int cookie_len; /* strlen(cookie_name), computed only once */
+ char *dyncookie_key; /* Secret key used to generate dynamic persistent cookies */
unsigned int cookie_maxidle; /* max idle time for this cookie */
unsigned int cookie_maxlife; /* max life time for this cookie */
int rdp_cookie_len; /* strlen(rdp_cookie_name), computed only once */
diff --git a/include/types/server.h b/include/types/server.h
index 51b7e53..feede6d 100644
--- a/include/types/server.h
+++ b/include/types/server.h
@@ -117,6 +117,7 @@ enum srv_initaddr {
#define SRV_F_CHECKADDR 0x0020 /* this server has a check addr configured */
#define SRV_F_CHECKPORT 0x0040 /* this server has a check port configured */
#define SRV_F_AGENTADDR 0x0080 /* this server has a agent addr configured */
+#define SRV_F_COOKIESET 0x0100 /* this server has a cookie configured, so don't generate dynamic cookies */

/* configured server options for send-proxy (server->pp_opts) */
#define SRV_PP_V1 0x0001 /* proxy protocol version 1 */
diff --git a/src/cfgparse.c b/src/cfgparse.c
index 9d04f02..832fa88 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -2632,6 +2632,9 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int kwm)
if (defproxy.cookie_name)
curproxy->cookie_name = strdup(defproxy.cookie_name);
curproxy->cookie_len = defproxy.cookie_len;
+
+ if (defproxy.dyncookie_key)
+ curproxy->dyncookie_key = strdup(defproxy.dyncookie_key);
if (defproxy.cookie_domain)
curproxy->cookie_domain = strdup(defproxy.cookie_domain);

@@ -2793,6 +2796,7 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int kwm)
free(defproxy.check_path);
free(defproxy.cookie_name);
free(defproxy.rdp_cookie_name);
+ free(defproxy.dyncookie_key);
free(defproxy.cookie_domain);
free(defproxy.url_param_name);
free(defproxy.hh_name);
@@ -3159,6 +3163,20 @@ int cfg_parse_listen(const char *file, int linenum, char **args, int kwm)
goto out;
}
}
+ else if (!strcmp(args[0], "dynamic-cookie-key")) { /* Dynamic cookies secret key */
+
+ if (warnifnotcap(curproxy, PR_CAP_BE, file, linenum, args[0], NULL))
+ err_code |= ERR_WARN;
+
+ if (*(args[1]) == 0) {
+ Alert("parsing [%s:%d] : '%s' expects <secret_key> as argument.\n",
+ file, linenum, args[0]);
+ err_code |= ERR_ALERT | ERR_FATAL;
+ goto out;
+ }
+ free(curproxy->dyncookie_key);
+ curproxy->dyncookie_key = strdup(args[1]);
+ }
else if (!strcmp(args[0], "cookie")) { /* cookie name */
int cur_arg;

@@ -3292,8 +3310,15 @@ 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 */
+
+ if (warnifnotcap(curproxy, PR_CAP_BE, file, linenum, args[cur_arg], NULL))
+ err_code |= ERR_WARN;
+ curproxy->ck_opts |= PR_CK_DYNAMIC;
+ }
+
else {
- Alert("parsing [%s:%d] : '%s' supports 'rewrite', 'insert', 'prefix', 'indirect', 'nocache', 'postonly', 'domain', 'maxidle, and 'maxlife' options.\n",
+ Alert("parsing [%s:%d] : '%s' supports 'rewrite', 'insert', 'prefix', 'indirect', 'nocache', 'postonly', 'domain', 'maxidle', 'dynamic' and 'maxlife' options.\n",
file, linenum, args[0]);
err_code |= ERR_ALERT | ERR_FATAL;
goto out;
@@ -8442,6 +8467,21 @@ out_uri_auth_compat:
newsrv = newsrv->next;
}

+ /*
+ * Try to generate dynamic cookies for servers now.
+ * It couldn't be done earlier, since at the time we parsed
+ * the server line, we may not have known yet that we
+ * should use dynamic cookies, or the secret key may not
+ * have been provided yet.
+ */
+ if (curproxy->ck_opts & PR_CK_DYNAMIC) {
+ newsrv = curproxy->srv;
+ while (newsrv != NULL) {
+ srv_set_dyncookie(newsrv);
+ newsrv = newsrv->next;
+ }
+
+ }
/* We have to initialize the server lookup mechanism depending
* on what LB algorithm was choosen.
*/
diff --git a/src/server.c b/src/server.c
index 9cc02f7..531f4c1 100644
--- a/src/server.c
+++ b/src/server.c
@@ -14,6 +14,8 @@
#include <ctype.h>
#include <errno.h>

+#include <import/xxhash.h>
+
#include <common/cfgparse.h>
#include <common/config.h>
#include <common/errors.h>
@@ -77,6 +79,73 @@ int srv_getinter(const struct check *check)
return (check->fastinter)?(check->fastinter):(check->inter);
}

+void srv_set_dyncookie(struct server *s)
+{
+ struct proxy *p = s->proxy;
+ struct server *tmpserv;
+ char *tmpbuf;
+ unsigned long long hash_value;
+ size_t key_len = strlen(p->dyncookie_key);
+ size_t buffer_len;
+ int addr_len;
+ int port;
+
+ if ((s->flags & SRV_F_COOKIESET) ||
+ !(s->proxy->ck_opts & PR_CK_DYNAMIC) ||
+ s->proxy->dyncookie_key == NULL)
+ return;
+
+ if (s->addr.ss_family != AF_INET &&
+ s->addr.ss_family != AF_INET6)
+ return;
+ /*
+ * Buffer to calculate the cookie value.
+ * The buffer contains the secret key + the server IP address
+ * + the TCP port.
+ */
+ addr_len = (s->addr.ss_family == AF_INET) ? 4 : 16;
+ /*
+ * The TCP port should use only 2 bytes, but is stored in
+ * an unsigned int in struct server, so let's use 4, to be
+ * on the safe side.
+ */
+ buffer_len = key_len + addr_len + 4;
+ tmpbuf = trash.str;
+ memcpy(tmpbuf, p->dyncookie_key, key_len);
+ 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);
+ /*
+ * Make sure it's the same across all the load balancers,
+ * no matter their endianness.
+ */
+ port = htonl(s->svc_port);
+ memcpy(&tmpbuf[key_len + addr_len], &port, 4);
+ hash_value = XXH64(tmpbuf, buffer_len, 0);
+ memprintf(&s->cookie, "%016llx", hash_value);
+ if (!s->cookie)
+ return;
+ s->cklen = 16;
+ /*
+ * Check that we did not get a hash collision.
+ * Unlikely, but it can happen.
+ */
+ for (p = proxy; p != NULL; p = p->next)
+ for (tmpserv = proxy->srv; tmpserv != NULL;
+ tmpserv = tmpserv->next) {
+ if (tmpserv == s)
+ continue;
+ if (tmpserv->cookie &&
+ strcmp(tmpserv->cookie, s->cookie) == 0) {
+ Warning("We generated two equal cookies for two different servers.\n"
+ "Please change the secret key for '%s'.\n",
+ s->proxy->id);
+ }
+ }
+}
+
/*
* Registers the server keyword list <kwl> as a list of valid keywords for next
* parsing sessions.
@@ -1175,6 +1244,7 @@ int parse_server(const char *file, int linenum, char **args, struct proxy *curpr
else if (!defsrv && !strcmp(args[cur_arg], "cookie")) {
newsrv->cookie = strdup(args[cur_arg + 1]);
newsrv->cklen = strlen(args[cur_arg + 1]);
+ newsrv->flags |= SRV_F_COOKIESET;
cur_arg += 2;
}
else if (!strcmp(args[cur_arg], "init-addr")) {
@@ -2730,6 +2800,7 @@ int update_server_addr(struct server *s, void *ip, int ip_sin_family, const char
memcpy(((struct sockaddr_in6 *)&s->addr)->sin6_addr.s6_addr, ip, 16);
break;
};
+ srv_set_dyncookie(s);

return 0;
}
@@ -2758,6 +2829,7 @@ const char *update_server_addr_port(struct server *s, const char *addr, const ch
char current_addr[INET6_ADDRSTRLEN];
uint16_t current_port, new_port;
struct chunk *msg;
+ int changed = 0;

msg = get_trash_chunk();
chunk_reset(msg);
@@ -2792,6 +2864,7 @@ const char *update_server_addr_port(struct server *s, const char *addr, const ch
goto port;
}
ipcpy(&sa, &s->addr);
+ changed = 1;

/* we also need to update check's ADDR only if it uses the server's one */
if ((s->check.state & CHK_ST_CONFIGURED) && (s->flags & SRV_F_CHECKADDR)) {
@@ -2859,6 +2932,7 @@ const char *update_server_addr_port(struct server *s, const char *addr, const ch
if (port_change_required) {
/* apply new port */
s->svc_port = new_port;
+ changed = 1;

/* prepare message */
chunk_appendf(msg, "port changed from '");
@@ -2893,6 +2967,8 @@ const char *update_server_addr_port(struct server *s, const char *addr, const ch
}

out:
+ if (changed)
+ srv_set_dyncookie(s);
if (updater)
chunk_appendf(msg, " by '%s'", updater);
chunk_appendf(msg, "\n");
@@ -3279,7 +3355,7 @@ static int srv_iterate_initaddr(struct server *srv)
if (!srv->lastaddr)
continue;
if (srv_apply_lastaddr(srv, &err_code) == 0)
- return return_code;
+ goto out;
return_code |= err_code;
break;

@@ -3287,7 +3363,7 @@ static int srv_iterate_initaddr(struct server *srv)
if (!srv->hostname)
continue;
if (srv_set_addr_via_libc(srv, &err_code) == 0)
- return return_code;
+ goto out;
return_code |= err_code;
break;

@@ -3305,7 +3381,7 @@ static int srv_iterate_initaddr(struct server *srv)
Warning("parsing [%s:%d] : 'server %s' : could not resolve address '%s', falling back to configured address.\n",
srv->conf.file, srv->conf.line, srv->id, srv->hostname);
}
- return return_code;
+ goto out;

default: /* unhandled method */
break;
@@ -3323,6 +3399,9 @@ static int srv_iterate_initaddr(struct server *srv)

return_code |= ERR_ALERT | ERR_FATAL;
return return_code;
+out:
+ srv_set_dyncookie(srv);
+ return return_code;
}

/*
--
2.9.3

From 51fd74e2b0ea97ce93e623af2009dcec5f657837 Mon Sep 17 00:00:00 2001
From: Olivier Houchard <[email protected]>
Date: Tue, 14 Mar 2017 20:08:46 +0100
Subject: [PATCH 2/2] MINOR: cli: Let configure the dynamic cookies from the
cli.
X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4

This adds 3 new commands to the cli :
enable dynamic-cookie backend <backend> that enables dynamic cookies for a
specified backend
disable dynamic-cookie backend <backend> that disables dynamic cookies for a
specified backend
set dynamic-cookie-key backend <backend> that lets one change the dynamic
cookie secret key, for a specified backend.
---
doc/management.txt | 11 +++++
include/proto/proxy.h | 1 +
src/proxy.c | 109 ++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 121 insertions(+)

diff --git a/doc/management.txt b/doc/management.txt
index 791d297..1d34f84 100644
--- a/doc/management.txt
+++ b/doc/management.txt
@@ -1401,6 +1401,9 @@ disable agent <backend>/<server>
This command is restricted and can only be issued on sockets configured for
level "admin".

+disable dynamic-cookie backend <backend>
+ Disable the generation of dynamic cookies fot the backend <backend>
+
disable frontend <frontend>
Mark the frontend as temporarily stopped. This corresponds to the mode which
is used during a soft restart : the frontend releases the port but can be
@@ -1450,6 +1453,10 @@ enable agent <backend>/<server>
This command is restricted and can only be issued on sockets configured for
level "admin".

+enable dynamic-cookie backend <backend>
+ Enable the generation of dynamic cookies fot the backend <backend>
+ A secret key must also be provided
+
enable frontend <frontend>
Resume a frontend which was temporarily stopped. It is possible that some of
the listening ports won't be able to bind anymore (eg: if another process
@@ -1540,6 +1547,10 @@ prompt
quit
Close the connection when in interactive mode.

+set dynamic-cookie-key backend <backend> <value>
+ Modify the secret key used to generate the dynamic persistent cookies.
+ This will break the existing sessions.
+
set map <map> [<key>|#<ref>] <value>
Modify the value corresponding to each key <key> in a map <map>. <map> is the
#<id> or <file> returned by "show map". If the <ref> is used in place of
diff --git a/include/proto/proxy.h b/include/proto/proxy.h
index a0fa454..72f1e1d 100644
--- a/include/proto/proxy.h
+++ b/include/proto/proxy.h
@@ -58,6 +58,7 @@ void init_new_proxy(struct proxy *p);
int get_backend_server(const char *bk_name, const char *sv_name,
struct proxy **bk, struct server **sv);
struct proxy *cli_find_frontend(struct appctx *appctx, const char *arg);
+struct proxy *cli_find_frontend(struct appctx *appctx, const char *arg);

/*
* This function returns a string containing the type of the proxy in a format
diff --git a/src/proxy.c b/src/proxy.c
index 41e40e6..c76d55d 100644
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -46,6 +46,7 @@
#include <proto/proto_tcp.h>
#include <proto/proto_http.h>
#include <proto/proxy.h>
+#include <proto/server.h>
#include <proto/signal.h>
#include <proto/stream.h>
#include <proto/stream_interface.h>
@@ -1244,6 +1245,30 @@ struct proxy *cli_find_frontend(struct appctx *appctx, const char *arg)
return px;
}

+/* Expects to find a backend named <arg> and returns it, otherwise displays various
+ * adequate error messages and returns NULL. This function is designed to be used by
+ * functions requiring a frontend on the CLI.
+ */
+struct proxy *cli_find_backend(struct appctx *appctx, const char *arg)
+{
+ struct proxy *px;
+
+ if (!*arg) {
+ appctx->ctx.cli.msg = "A backend name is expected.\n";
+ appctx->st0 = CLI_ST_PRINT;
+ return NULL;
+ }
+
+ px = proxy_be_by_name(arg);
+ if (!px) {
+ appctx->ctx.cli.msg = "No such backend.\n";
+ appctx->st0 = CLI_ST_PRINT;
+ return NULL;
+ }
+ return px;
+}
+
+
/* parse a "show servers" CLI line, returns 0 if it wants to start the dump or
* 1 if it stops immediately. If an argument is specified, it will set the proxy
* pointer into cli.p0 and its ID into cli.i0.
@@ -1413,6 +1438,87 @@ static int cli_io_handler_show_backend(struct appctx *appctx)
return 1;
}

+/* Parses the "enable dynamic-cookies backend" directive, it always returns 1 */
+static int cli_parse_enable_dyncookie_backend(char **args, struct appctx *appctx, void *private)
+{
+ struct proxy *px;
+ struct server *s;
+
+ if (!cli_has_level(appctx, ACCESS_LVL_ADMIN))
+ return 1;
+
+ px = cli_find_backend(appctx, args[3]);
+ if (!px)
+ return 1;
+
+ px->ck_opts |= PR_CK_DYNAMIC;
+
+ for (s = px->srv; s != NULL; s = s->next)
+ srv_set_dyncookie(s);
+
+ return 1;
+}
+
+/* Parses the "disable dynamic-cookies backend" directive, it always returns 1 */
+static int cli_parse_disable_dyncookie_backend(char **args, struct appctx *appctx, void *private)
+{
+ struct proxy *px;
+ struct server *s;
+
+ if (!cli_has_level(appctx, ACCESS_LVL_ADMIN))
+ return 1;
+
+ px = cli_find_backend(appctx, args[3]);
+ if (!px)
+ return 1;
+
+ px->ck_opts &= ~PR_CK_DYNAMIC;
+
+ for (s = px->srv; s != NULL; s = s->next) {
+ if (!(s->flags & SRV_F_COOKIESET)) {
+ free(s->cookie);
+ s->cookie = NULL;
+ }
+ }
+
+ return 1;
+}
+
+/* Parses the "set dynamic-cookie-key backend" directive, it always returns 1 */
+static int cli_parse_set_dyncookie_key_backend(char **args, struct appctx *appctx, void *private)
+{
+ struct proxy *px;
+ struct server *s;
+ char *newkey;
+
+ if (!cli_has_level(appctx, ACCESS_LVL_ADMIN))
+ return 1;
+
+ px = cli_find_backend(appctx, args[3]);
+ if (!px)
+ return 1;
+
+ if (!*args[4]) {
+ appctx->ctx.cli.msg = "String value expected.\n";
+ appctx->st0 = CLI_ST_PRINT;
+ return 1;
+ }
+
+ newkey = strdup(args[4]);
+ if (!newkey) {
+ appctx->ctx.cli.msg = "Failed to allocate memory.\n";
+ appctx->st0 = CLI_ST_PRINT;
+ return 1;
+ }
+ free(px->dyncookie_key);
+ px->dyncookie_key = newkey;
+
+ for (s = px->srv; s != NULL; s = s->next)
+ srv_set_dyncookie(s);
+
+ return 1;
+}
+
/* Parses the "set maxconn frontend" directive, it always returns 1 */
static int cli_parse_set_maxconn_frontend(char **args, struct appctx *appctx, void *private)
{
@@ -1554,6 +1660,9 @@ static struct cli_kw_list cli_kws = {{ },{
{ { "show","servers", "state", NULL }, "show servers state [id]: dump volatile server information (for backend <id>)", cli_parse_show_servers, cli_io_handler_servers_state },
{ { "show", "backend", NULL }, "show backend : list backends in the current running config", NULL, cli_io_handler_show_backend },
{ { "shutdown", "frontend", NULL }, "shutdown frontend : stop a specific frontend", cli_parse_shutdown_frontend, NULL, NULL },
+ { { "set", "dynamic-cookie-key", "backend", NULL }, "set dynamic-cookie-key backend : change a backend secret key for dynamic cookies", cli_parse_set_dyncookie_key_backend, NULL },
+ { { "enable", "dynamic-cookie", "backend", NULL }, "enable dynamic-cookie backend : enable dynamic cookies on a specific backend", cli_parse_enable_dyncookie_backend, NULL },
+ { { "disable", "dynamic-cookie", "backend", NULL }, "disable dynamic-cookie backend : disable dynamic cookies on a specific backend", cli_parse_disable_dyncookie_backend, NULL },
{{},}
}};

--
2.9.3
Willy Tarreau
Re: Dynamic cookies support
March 14, 2017 11:30PM
Hi Olivier,

On Tue, Mar 14, 2017 at 08:23:56PM +0100, 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.

Well that looks pretty good, I have no comment to make about it.
Do you want me to merge them now or are you seeking more comments first ?

thanks,
Willy
Willy Tarreau
Re: Dynamic cookies support
March 15, 2017 11:50AM
On Tue, Mar 14, 2017 at 11:22:35PM +0100, Willy Tarreau wrote:
> Well that looks pretty good, I have no comment to make about it.
> Do you want me to merge them now or are you seeking more comments first ?

now applied, thanks!
Willy
Jarno Huuskonen
Re: Dynamic cookies support
March 15, 2017 03:00PM
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 ([email protected]=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>, [email protected]=4,
argv=<optimized out>, [email protected]=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

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);

-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/)

--
Jarno Huuskonen
Olivier Houchard
Re: Dynamic cookies support
March 15, 2017 03:30PM
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 ([email protected]=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>, [email protected]=4,
> argv=<optimized out>, [email protected]=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 <[email protected]>
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 <[email protected]>
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
Willy Tarreau
Re: Dynamic cookies support
March 15, 2017 04:10PM
On Wed, Mar 15, 2017 at 03:19:15PM +0100, Olivier Houchard wrote:
> Oops my bad, I'm an idiot.

Great, that's exactly what we were missing here, I needed some help.

> Willy, can you commit the attached patch ?

Both patches merged, thanks guys.

Willy
Sorry, only registered users may post in this forum.

Click here to login