From 9d71d0b36d83f475deb7f88d63c947aa7ad72e77 Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Wed, 28 Apr 2021 16:34:49 +0200 Subject: [PATCH 1/7] ensure proper timeouts on dynamic server startup --- dtls.c | 11 +++++------ tcp.c | 11 ++++++----- tls.c | 6 ++++++ 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/dtls.c b/dtls.c index 620259d..ccf0264 100644 --- a/dtls.c +++ b/dtls.c @@ -561,16 +561,15 @@ int dtlsconnect(struct server *server, int timeout, char *text) { server->ssl = NULL; wait = connect_wait(start, server->connecttime, firsttry); - debug(DBG_INFO, "Next connection attempt to %s in %lds", server->conf->name, wait); - sleep(wait); - firsttry = 0; - gettimeofday(&now, NULL); - if (timeout && (now.tv_sec - start.tv_sec) > timeout) { - debug(DBG_DBG, "tlsconnect: timeout"); + if (timeout && (now.tv_sec - start.tv_sec) + wait > timeout) { + debug(DBG_DBG, "dtlsconnect: timeout"); if (source) freeaddrinfo(source); return 0; } + debug(DBG_INFO, "Next connection attempt to %s in %lds", server->conf->name, wait); + sleep(wait); + firsttry = 0; debug(DBG_INFO, "dtlsconnect: connecting to %s port %s", hp->host, hp->port); diff --git a/tcp.c b/tcp.c index aa6a6ce..4ea3725 100644 --- a/tcp.c +++ b/tcp.c @@ -106,16 +106,17 @@ int tcpconnect(struct server *server, int timeout, char *text) { pthread_mutex_unlock(&server->lock); wait = connect_wait(start, server->connecttime, firsttry); - debug(DBG_INFO, "Next connection attempt to %s in %lds", server->conf->name, wait); - sleep(wait); - firsttry = 0; - gettimeofday(&now, NULL); - if (timeout && (now.tv_sec - start.tv_sec) > timeout) { + if (timeout && (now.tv_sec - start.tv_sec) + wait > timeout) { debug(DBG_DBG, "tcpconnect: timeout"); if (source) freeaddrinfo(source); return 0; } + debug(DBG_INFO, "Next connection attempt to %s in %lds", server->conf->name, wait); + sleep(wait); + firsttry = 0; + + pthread_mutex_lock(&server->lock); debug(DBG_INFO, "tcpconnect: connecting to %s", server->conf->name); diff --git a/tls.c b/tls.c index f717d98..a2d9e1a 100644 --- a/tls.c +++ b/tls.c @@ -118,6 +118,12 @@ int tlsconnect(struct server *server, int timeout, char *text) { server->ssl = NULL; wait = connect_wait(start, server->connecttime, firsttry); + gettimeofday(&now, NULL); + if (timeout && (now.tv_sec - start.tv_sec) + wait > timeout) { + debug(DBG_DBG, "tlsconnect: timeout"); + if (source) freeaddrinfo(source); + return 0; + } debug(DBG_INFO, "Next connection attempt to %s in %lds", server->conf->name, wait); sleep(wait); firsttry = 0; From 54ecbd70a45abf01c6101fd212a3875d0dcf07cb Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Wed, 28 Apr 2021 16:55:17 +0200 Subject: [PATCH 2/7] add BlockingStartup option --- radsecproxy.c | 6 ++++-- radsecproxy.h | 2 ++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/radsecproxy.c b/radsecproxy.c index e8df372..9db19b0 100644 --- a/radsecproxy.c +++ b/radsecproxy.c @@ -1586,7 +1586,8 @@ void *clientwr(void *arg) { #define ZZZ 900 - server->state = RSP_SERVER_STATE_STARTUP; + if (server->state != RSP_SERVER_STATE_BLOCKING_STARTUP) + server->state = RSP_SERVER_STATE_STARTUP; if (server->dynamiclookuparg && !dynamicconfig(server)) { dynconffail = 1; server->state = RSP_SERVER_STATE_FAILING; @@ -2059,7 +2060,7 @@ struct list *createsubrealmservers(struct realm *realm, struct list *srvconfs) { * the srvconfs list. */ if (addserver(srvconf)) { srvconf->servers->dynamiclookuparg = stringcopy(realm->name, 0); - srvconf->servers->state = RSP_SERVER_STATE_STARTUP; + srvconf->servers->state = srvconf->blockingstartup ? RSP_SERVER_STATE_BLOCKING_STARTUP : RSP_SERVER_STATE_STARTUP; debug(DBG_DBG, "%s: new client writer for %s", __func__, srvconf->servers->conf->name); if (pthread_create(&clientth, &pthread_attr, clientwr, (void *)(srvconf->servers))) { @@ -2573,6 +2574,7 @@ int confserver_cb(struct gconffile **cf, void *arg, char *block, char *opt, char "RetryCount", CONF_LINT, &retrycount, "DynamicLookupCommand", CONF_STR, &conf->dynamiclookupcommand, "LoopPrevention", CONF_BLN, &conf->loopprevention, + "BlockingStartup", CONF_BLN, &conf->blockingstartup, NULL )) { debug(DBG_ERR, "configuration error"); diff --git a/radsecproxy.h b/radsecproxy.h index 5917faa..cf493b4 100644 --- a/radsecproxy.h +++ b/radsecproxy.h @@ -71,6 +71,7 @@ enum rsp_mac_type { enum rsp_server_state { RSP_SERVER_STATE_STARTUP = 0, /* default */ + RSP_SERVER_STATE_BLOCKING_STARTUP, RSP_SERVER_STATE_CONNECTED, RSP_SERVER_STATE_RECONNECTING, RSP_SERVER_STATE_FAILING @@ -166,6 +167,7 @@ struct clsrvconf { uint8_t addttl; uint8_t keepalive; uint8_t loopprevention; + uint8_t blockingstartup; struct rewrite *rewritein; struct rewrite *rewriteout; pthread_mutex_t *lock; /* only used for updating clients so far */ From d517079ca94d833405dc51432e7f4cc42e03dd19 Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Wed, 28 Apr 2021 17:08:51 +0200 Subject: [PATCH 3/7] document BlockingStartup on manpage --- radsecproxy.conf.5.in | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/radsecproxy.conf.5.in b/radsecproxy.conf.5.in index 35f3f3d..3b9d4a7 100644 --- a/radsecproxy.conf.5.in +++ b/radsecproxy.conf.5.in @@ -609,6 +609,16 @@ See section \fBBASIC OPTIONS\fR for details on this option. .RE +.BR "BlockingStartup (" on | off) +.RS +Start the server in blocking mode, treating it as if it was already connected and enqueue +requests to this server. Queued requests will be sent out when the connection is establised. +This is only considered for dynamic lookup servers (ie when DynamicLookupCommand is specified) +and is disabled by default. +Warning: when the dynamic lookup and connection process is slow, this wil block the +respective realm for that time. +.RE + The meaning and syntax of the following options are exactly the same as for the client block. The details are not repeated here. Please refer to the definitions in the \fBCLIENT BLOCK\fR section. From 5aa2126b615cade65f914e1beff03b512c76ea87 Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Wed, 28 Apr 2021 17:17:34 +0200 Subject: [PATCH 4/7] override BlockingStartup from dynamic config snippet --- radsecproxy.c | 1 + 1 file changed, 1 insertion(+) diff --git a/radsecproxy.c b/radsecproxy.c index 9db19b0..9b059fd 100644 --- a/radsecproxy.c +++ b/radsecproxy.c @@ -2301,6 +2301,7 @@ int mergesrvconf(struct clsrvconf *dst, struct clsrvconf *src) { dst->retryinterval = src->retryinterval; if (src->retrycount != 255) dst->retrycount = src->retrycount; + dst->blockingstartup = src->blockingstartup; return 1; } From 04abf8689fe5cba7113e954a4223c044c6e9c02c Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Thu, 29 Apr 2021 10:23:06 +0200 Subject: [PATCH 5/7] flush request queue when server startup failed --- radsecproxy.c | 51 +++++++++++++++++++++++++++++---------------------- 1 file changed, 29 insertions(+), 22 deletions(-) diff --git a/radsecproxy.c b/radsecproxy.c index 9b059fd..ca160aa 100644 --- a/radsecproxy.c +++ b/radsecproxy.c @@ -1589,21 +1589,19 @@ void *clientwr(void *arg) { if (server->state != RSP_SERVER_STATE_BLOCKING_STARTUP) server->state = RSP_SERVER_STATE_STARTUP; if (server->dynamiclookuparg && !dynamicconfig(server)) { - dynconffail = 1; - server->state = RSP_SERVER_STATE_FAILING; - debug(DBG_WARN, "%s: dynamicconfig(%s: %s) failed, sleeping %ds", + dynconffail = 1; + server->state = RSP_SERVER_STATE_FAILING; + debug(DBG_WARN, "%s: dynamicconfig(%s: %s) failed, sleeping %ds", __func__, server->conf->name, server->dynamiclookuparg, ZZZ); - sleep(ZZZ); - goto errexit; + goto errexitwait; } /* FIXME: Is resolving not always done by compileserverconfig(), * either as part of static configuration setup or by * dynamicconfig() above? */ if (!resolvehostports(conf->hostports, conf->hostaf, conf->pdef->socktype)) { debug(DBG_WARN, "%s: resolve failed, sleeping %ds", __func__, ZZZ); - server->state = RSP_SERVER_STATE_FAILING; - sleep(ZZZ); - goto errexit; + server->state = RSP_SERVER_STATE_FAILING; + goto errexitwait; } memset(&timeout, 0, sizeof(struct timespec)); @@ -1613,20 +1611,19 @@ void *clientwr(void *arg) { laststatsrv = server->lastreply; if (conf->pdef->connecter) { - if (!conf->pdef->connecter(server, server->dynamiclookuparg ? 5 : 0, "clientwr")) { - server->state = RSP_SERVER_STATE_FAILING; - if (server->dynamiclookuparg) { - debug(DBG_WARN, "%s: connect failed, sleeping %ds", - __func__, ZZZ); - sleep(ZZZ); - } - goto errexit; - } - if (pthread_create(&clientrdth, &pthread_attr, conf->pdef->clientconnreader, (void *)server)) { - debugerrno(errno, DBG_ERR, "clientwr: pthread_create failed"); - server->state = RSP_SERVER_STATE_FAILING; - goto errexit; - } + if (!conf->pdef->connecter(server, server->dynamiclookuparg ? 5 : 0, "clientwr")) { + server->state = RSP_SERVER_STATE_FAILING; + if (server->dynamiclookuparg) { + debug(DBG_WARN, "%s: connect failed, sleeping %ds", __func__, ZZZ); + goto errexitwait; + } + goto errexit; + } + if (pthread_create(&clientrdth, &pthread_attr, conf->pdef->clientconnreader, (void *)server)) { + debugerrno(errno, DBG_ERR, "clientwr: pthread_create failed"); + server->state = RSP_SERVER_STATE_FAILING; + goto errexit; + } } server->state = RSP_SERVER_STATE_CONNECTED; @@ -1761,6 +1758,16 @@ void *clientwr(void *arg) { } } } + +errexitwait: + /* flush request queue so we don't block incoming retries by removing blocked duplicates*/ + for (i=0; i < MAX_REQUESTS; i++) { + rqout = server->requests + i; + pthread_mutex_lock(rqout->lock); + freerqoutdata(rqout); + pthread_mutex_unlock(rqout->lock); + } + sleep(ZZZ); errexit: if (server->dynamiclookuparg) { removeserversubrealms(realms, conf); From 7cd6676786dccdd58ddb866b5b455317b1f2eb6c Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Fri, 30 Apr 2021 13:09:44 +0200 Subject: [PATCH 6/7] add timeout for dynamic lookup --- radsecproxy.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/radsecproxy.c b/radsecproxy.c index ca160aa..eb9fd79 100644 --- a/radsecproxy.c +++ b/radsecproxy.c @@ -56,6 +56,7 @@ #include #include #include +#include #include #include #include @@ -2125,10 +2126,12 @@ struct realm *adddynamicrealmserver(struct realm *realm, char *id) { } int dynamicconfig(struct server *server) { - int ok, fd[2], status; + int ok = 0, fd[2], status; pid_t pid; struct clsrvconf *conf = server->conf; struct gconffile *cf = NULL; + struct pollfd fds[1]; + FILE *pipein; /* for now we only learn hostname/address */ debug(DBG_DBG, "dynamicconfig: need dynamic server config for %s", server->dynamiclookuparg); @@ -2156,10 +2159,20 @@ int dynamicconfig(struct server *server) { } close(fd[1]); - pushgconffile(&cf, fdopen(fd[0], "r"), conf->dynamiclookupcommand); - ok = getgenericconfig(&cf, NULL, "Server", CONF_CBK, confserver_cb, - (void *) conf, NULL); - freegconf(&cf); + pipein = fdopen(fd[0], "r"); + fds[0].fd = fd[0]; + fds[0].events = POLLIN; + status = poll(fds, 1, 5000); + if (status < 0) { + debugerrno(errno, DBG_ERR, "dynamicconfig: error while waiting for command output"); + } else if (status ==0) { + debug(DBG_WARN, "dynamicconfig: command did not return anything in time"); + kill(pid, SIGKILL); + } else { + pushgconffile(&cf, pipein, conf->dynamiclookupcommand); + ok = getgenericconfig(&cf, NULL, "Server", CONF_CBK, confserver_cb, (void *) conf, NULL); + freegconf(&cf); + } if (waitpid(pid, &status, 0) < 0) { debugerrno(errno, DBG_ERR, "dynamicconfig: wait error"); From 38901959ee8bd3375528225bfc22688bb6228311 Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Mon, 3 May 2021 07:52:13 +0200 Subject: [PATCH 7/7] update manpage --- radsecproxy.conf.5.in | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/radsecproxy.conf.5.in b/radsecproxy.conf.5.in index 3b9d4a7..08c21c5 100644 --- a/radsecproxy.conf.5.in +++ b/radsecproxy.conf.5.in @@ -611,10 +611,11 @@ See section .BR "BlockingStartup (" on | off) .RS -Start the server in blocking mode, treating it as if it was already connected and enqueue -requests to this server. Queued requests will be sent out when the connection is establised. -This is only considered for dynamic lookup servers (ie when DynamicLookupCommand is specified) -and is disabled by default. +Start the dynamic server in blocking mode (default off), treating it as if it was already +connected and enqueue requests to this server. Queued requests will be sent out when the +connection is established. If however the dynamic lookup or the connection fails, the queued +requests will be lost. +(This is only considered for dynamic lookup servers. Ie when DynamicLookupCommand is specified) Warning: when the dynamic lookup and connection process is slow, this wil block the respective realm for that time. .RE