From 14cac40266288157047c205d138d26ebcb042aa2 Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Wed, 2 Nov 2016 17:12:45 +0100 Subject: [PATCH 1/4] proposed fix for RADSECPROXY-71 never set clsrvconf->servers=null after it has been properly set up. set servers->dynfailing=1 instead Conflicts: radsecproxy.c --- radsecproxy.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/radsecproxy.c b/radsecproxy.c index 142d069..5bcbfe7 100644 --- a/radsecproxy.c +++ b/radsecproxy.c @@ -1767,6 +1767,7 @@ void *clientwr(void *arg) { * dynamicconfig() above? */ if (!resolvehostports(conf->hostports, conf->hostaf, conf->pdef->socktype)) { debug(DBG_WARN, "%s: resolve failed, sleeping %ds", __func__, ZZZ); + server->dynfailing=1; sleep(ZZZ); goto errexit; } @@ -1780,9 +1781,9 @@ void *clientwr(void *arg) { if (conf->pdef->connecter) { if (!conf->pdef->connecter(server, NULL, server->dynamiclookuparg ? 5 : 0, "clientwr")) { + server->dynfailing = 1; if (server->dynamiclookuparg) { server->dynstartup = 0; - server->dynfailing = 1; debug(DBG_WARN, "%s: connect failed, sleeping %ds", __func__, ZZZ); sleep(ZZZ); @@ -1792,6 +1793,7 @@ void *clientwr(void *arg) { server->connectionok = 1; if (pthread_create(&clientrdth, &pthread_attr, conf->pdef->clientconnreader, (void *)server)) { debugerrno(errno, DBG_ERR, "clientwr: pthread_create failed"); + server->dynfailing=1; goto errexit; } } else @@ -1834,6 +1836,7 @@ void *clientwr(void *arg) { for (i = 0; i < MAX_REQUESTS; i++) { if (server->clientrdgone) { + server->dynfailing=1; if (conf->pdef->connecter) pthread_join(clientrdth, NULL); goto errexit; @@ -1906,8 +1909,6 @@ void *clientwr(void *arg) { free(conf); else freeclsrvconf(conf); - } else { - conf->servers = NULL; } freeserver(server, 1); return NULL; From 05ed43d8d4bf254a503cee715034a8184615af9e Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Thu, 2 Mar 2017 10:44:16 +0100 Subject: [PATCH 2/4] replace server states with enum --- dtls.c | 8 ++++---- radsecproxy.c | 28 ++++++++++++---------------- radsecproxy.h | 11 ++++++++--- tcp.c | 8 ++++---- tls.c | 8 ++++---- 5 files changed, 32 insertions(+), 31 deletions(-) diff --git a/dtls.c b/dtls.c index daeacca..fbbdd3b 100644 --- a/dtls.c +++ b/dtls.c @@ -556,8 +556,8 @@ int dtlsconnect(struct server *server, struct timeval *when, int timeout, char * return 0; } - if (server->connectionok) { - server->connectionok = 0; + if (server->state == RSP_SERVER_STATE_CONNECTED) { + server->state = RSP_SERVER_STATE_RECONNECTING; sleep(2); } else if (elapsed < 1) sleep(2); @@ -591,7 +591,7 @@ int dtlsconnect(struct server *server, struct timeval *when, int timeout, char * } X509_free(cert); debug(DBG_WARN, "dtlsconnect: DTLS connection to %s port %s up", hp->host, hp->port); - server->connectionok = 1; + server->state = RSP_SERVER_STATE_CONNECTED; gettimeofday(&server->lastconnecttry, NULL); pthread_mutex_unlock(&server->lock); return 1; @@ -603,7 +603,7 @@ int clientradputdtls(struct server *server, unsigned char *rad) { unsigned long error; struct clsrvconf *conf = server->conf; - if (!server->connectionok) + if (server->state != RSP_SERVER_STATE_CONNECTED) return 0; len = RADLEN(rad); if ((cnt = SSL_write(server->ssl, rad, len)) <= 0) { diff --git a/radsecproxy.c b/radsecproxy.c index 5bcbfe7..67e8839 100644 --- a/radsecproxy.c +++ b/radsecproxy.c @@ -1260,11 +1260,11 @@ struct clsrvconf *choosesrvconf(struct list *srvconfs) { server = (struct clsrvconf *)entry->data; if (!server->servers) return server; - if (server->servers->dynfailing) + if (server->servers->state == RSP_SERVER_STATE_FAILING) continue; if (!first) first = server; - if (!server->servers->connectionok && !server->servers->dynstartup) + if (server->servers->state == RSP_SERVER_STATE_STARTUP || server->servers->state == RSP_SERVER_STATE_RECONNECTING) continue; if (!server->servers->lostrqs) return server; @@ -1560,7 +1560,6 @@ void replyh(struct server *server, unsigned char *buf) { struct tlv *attr; struct list_node *node; - server->connectionok = 1; server->lostrqs = 0; rqout = server->requests + buf[1]; @@ -1753,10 +1752,10 @@ void *clientwr(void *arg) { #define ZZZ 900 + server->state = RSP_SERVER_STATE_STARTUP; if (server->dynamiclookuparg && !dynamicconfig(server)) { dynconffail = 1; - server->dynstartup = 0; - server->dynfailing = 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); @@ -1767,7 +1766,7 @@ void *clientwr(void *arg) { * dynamicconfig() above? */ if (!resolvehostports(conf->hostports, conf->hostaf, conf->pdef->socktype)) { debug(DBG_WARN, "%s: resolve failed, sleeping %ds", __func__, ZZZ); - server->dynfailing=1; + server->state = RSP_SERVER_STATE_FAILING; sleep(ZZZ); goto errexit; } @@ -1781,24 +1780,21 @@ void *clientwr(void *arg) { if (conf->pdef->connecter) { if (!conf->pdef->connecter(server, NULL, server->dynamiclookuparg ? 5 : 0, "clientwr")) { - server->dynfailing = 1; + server->state = RSP_SERVER_STATE_FAILING; if (server->dynamiclookuparg) { - server->dynstartup = 0; debug(DBG_WARN, "%s: connect failed, sleeping %ds", __func__, ZZZ); sleep(ZZZ); } goto errexit; } - server->connectionok = 1; if (pthread_create(&clientrdth, &pthread_attr, conf->pdef->clientconnreader, (void *)server)) { debugerrno(errno, DBG_ERR, "clientwr: pthread_create failed"); - server->dynfailing=1; + server->state = RSP_SERVER_STATE_FAILING; goto errexit; } - } else - server->connectionok = 1; - server->dynstartup = 0; + } + server->state = RSP_SERVER_STATE_CONNECTED; for (;;) { pthread_mutex_lock(&server->newrq_mutex); @@ -1836,7 +1832,7 @@ void *clientwr(void *arg) { for (i = 0; i < MAX_REQUESTS; i++) { if (server->clientrdgone) { - server->dynfailing=1; + server->state = RSP_SERVER_STATE_FAILING; if (conf->pdef->connecter) pthread_join(clientrdth, NULL); goto errexit; @@ -1888,7 +1884,7 @@ void *clientwr(void *arg) { conf->pdef->clientradput(server, rqout->rq->buf); pthread_mutex_unlock(rqout->lock); } - if (conf->statusserver && server->connectionok) { + if (conf->statusserver && server->state == RSP_SERVER_STATE_CONNECTED) { secs = server->lastrcv.tv_sec > laststatsrv.tv_sec ? server->lastrcv.tv_sec : laststatsrv.tv_sec; gettimeofday(&now, NULL); if (now.tv_sec - secs > STATUS_SERVER_PERIOD) { @@ -2189,7 +2185,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->dynstartup = 1; + srvconf->servers->state = 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))) { diff --git a/radsecproxy.h b/radsecproxy.h index c96cc69..5cf842c 100644 --- a/radsecproxy.h +++ b/radsecproxy.h @@ -58,6 +58,13 @@ enum rsp_fticks_mac_type { RSP_FTICKS_MAC_FULLY_KEY_HASHED }; +enum rsp_server_state { + RSP_SERVER_STATE_STARTUP = 0, /* default */ + RSP_SERVER_STATE_CONNECTED, + RSP_SERVER_STATE_RECONNECTING, + RSP_SERVER_STATE_FAILING +}; + struct options { char *pidfile; char *logdestination; @@ -165,10 +172,8 @@ struct server { uint8_t clientrdgone; struct timeval lastconnecttry; struct timeval lastreply; - uint8_t connectionok; + enum rsp_server_state state; uint8_t lostrqs; - uint8_t dynstartup; - uint8_t dynfailing; char *dynamiclookuparg; int nextid; struct timeval lastrcv; diff --git a/tcp.c b/tcp.c index 77a6044..c6b2f52 100644 --- a/tcp.c +++ b/tcp.c @@ -102,8 +102,8 @@ int tcpconnect(struct server *server, struct timeval *when, int timeout, char *t pthread_mutex_unlock(&server->lock); return 0; } - if (server->connectionok) { - server->connectionok = 0; + if (server->state == RSP_SERVER_STATE_CONNECTED) { + server->state = RSP_SERVER_STATE_RECONNECTING; sleep(2); } else if (elapsed < 1) sleep(2); @@ -121,7 +121,7 @@ int tcpconnect(struct server *server, struct timeval *when, int timeout, char *t if ((server->sock = connecttcphostlist(server->conf->hostports, srcres)) >= 0) break; } - server->connectionok = 1; + server->state = RSP_SERVER_STATE_CONNECTED; gettimeofday(&server->lastconnecttry, NULL); pthread_mutex_unlock(&server->lock); return 1; @@ -202,7 +202,7 @@ int clientradputtcp(struct server *server, unsigned char *rad) { size_t len; struct clsrvconf *conf = server->conf; - if (!server->connectionok) + if (server->state != RSP_SERVER_STATE_CONNECTED) return 0; len = RADLEN(rad); if ((cnt = write(server->sock, rad, len)) <= 0) { diff --git a/tls.c b/tls.c index bbf9da3..a691be5 100644 --- a/tls.c +++ b/tls.c @@ -111,8 +111,8 @@ int tlsconnect(struct server *server, struct timeval *when, int timeout, char *t pthread_mutex_unlock(&server->lock); return 0; } - if (server->connectionok) { - server->connectionok = 0; + if (server->state == RSP_SERVER_STATE_CONNECTED) { + server->state = RSP_SERVER_STATE_RECONNECTING; sleep(2); } else if (elapsed < 1) sleep(2); @@ -155,7 +155,7 @@ int tlsconnect(struct server *server, struct timeval *when, int timeout, char *t X509_free(cert); } debug(DBG_WARN, "tlsconnect: TLS connection to %s up", server->conf->name); - server->connectionok = 1; + server->state = RSP_SERVER_STATE_CONNECTED; gettimeofday(&server->lastconnecttry, NULL); pthread_mutex_unlock(&server->lock); return 1; @@ -251,7 +251,7 @@ int clientradputtls(struct server *server, unsigned char *rad) { unsigned long error; struct clsrvconf *conf = server->conf; - if (!server->connectionok) + if (server->state != RSP_SERVER_STATE_CONNECTED) return 0; len = RADLEN(rad); if ((cnt = SSL_write(server->ssl, rad, len)) <= 0) { From dbbf3d23b13fc37ca79e92ff310b8bb4b0c7e931 Mon Sep 17 00:00:00 2001 From: Fabian Mauchle Date: Wed, 15 Mar 2017 09:54:40 +0100 Subject: [PATCH 3/4] update ChangeLog --- ChangeLog | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ChangeLog b/ChangeLog index cd5c58a..715d69b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -14,6 +14,8 @@ Changes between 1.6.8 and the master branch aware that use of the DynamicLookupCommand configuration option still enables code known to be buggy. - Use a listen(2) backlog of 128 (RADSECPROXY-72). + - Replace several server status bits with a single state enum. + (RADSECPROXY-71) Bug fixes: - Detect the presence of docbook2x-man correctly. From e59a9fbd982a6ce9b00c45948444cc7acc2634b2 Mon Sep 17 00:00:00 2001 From: Linus Nordberg Date: Tue, 1 Nov 2016 10:25:02 +0100 Subject: [PATCH 4/4] Look at servers->dynamiclookuparg for deciding if a server is dynamic. The dynamiclookupcommand member of the _config_ of the server is being set to NULL when it's copied in confserver_cb(), resulting in dynamic discovery being done for realms that already have a server. Patch from Fabian Mauchle. Addresses RADSECPROXY-69. --- ChangeLog | 2 ++ radsecproxy.c | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 715d69b..0ae86bc 100644 --- a/ChangeLog +++ b/ChangeLog @@ -27,6 +27,8 @@ Changes between 1.6.8 and the master branch - Tie Access-Request log lines to response log lines (RADSECPROXY-60). - Take lock on realm refcount before updating it (RADSECPROXY-77). - Fix a couple of memory leaks and NULL ptr derefs in error cases. + - Don't forget about good dynamically discovered (TLS) connections + (RADSECPROXY-69). 2016-09-21 1.6.8 Bug fixes: diff --git a/radsecproxy.c b/radsecproxy.c index 67e8839..ef1d710 100644 --- a/radsecproxy.c +++ b/radsecproxy.c @@ -707,7 +707,7 @@ int hasdynamicserver(struct list *srvconfs) { struct list_node *entry; for (entry = list_first(srvconfs); entry; entry = list_next(entry)) - if (((struct clsrvconf *)entry->data)->dynamiclookupcommand) + if (((struct clsrvconf *)entry->data)->servers->dynamiclookuparg) return 1; return 0; }