radius: update server secret on config reload

On EV_CONFIG_RELOAD, __add_server() re-parses each server line into a
fresh rad_server_t and, when it matches an existing server by
addr/auth_port/acct_port, copied over only the timeout/limit/bind
fields before freeing the new struct. The freshly parsed secret was
discarded, so editing a shared secret and reloading had no effect until
a full restart. The strdup'd secret on the freed struct was also leaked
on every matched reload.

Adopt the new secret into the existing server (freeing the old one) so
secret changes take effect on reload. New requests read
req->serv->secret directly, so they pick up the update immediately.
This covers both the modern "server=" path and the legacy
auth-server/acct-server path, which both funnel through __add_server().

5 files changed, 75 insertions, 16 deletions

diff --git a/accel-pppd/radius/acct.c b/accel-pppd/radius/acct.c
index 8b04a03f..69bd3d56 100644
--- a/accel-pppd/radius/acct.c
+++ b/accel-pppd/radius/acct.c
@@ -25,18 +25,27 @@
 
 #define INTERIM_SAFE_TIME 10
 
-static int req_set_RA(struct rad_req_t *req, const char *secret)
+static int req_set_RA(struct rad_req_t *req)
 {
+	char *secret;
 	MD5_CTX ctx;
 
-	if (rad_packet_build(req->pack, req->RA))
+	secret = rad_server_secret_dup(req->serv);
+	if (!secret)
 		return -1;
 
+	if (rad_packet_build(req->pack, req->RA)) {
+		_free(secret);
+		return -1;
+	}
+
 	MD5_Init(&ctx);
 	MD5_Update(&ctx, req->pack->buf, req->pack->len);
 	MD5_Update(&ctx, secret, strlen(secret));
 	MD5_Final(req->pack->buf + 4, &ctx);
 
+	_free(secret);
+
 	return 0;
 }
 
@@ -180,7 +189,7 @@ static void rad_acct_interim_update(struct triton_timer_t *t)
 	rpd->acct_req->pack->id++;
 
 	if (!rpd->acct_req->before_send)
-		req_set_RA(rpd->acct_req, rpd->acct_req->serv->secret);
+		req_set_RA(rpd->acct_req);
 
 	rpd->acct_req->timeout.expire_tv.tv_sec = conf_timeout;
 	rpd->acct_req->try = 0;
@@ -212,7 +221,7 @@ static int rad_acct_before_send(struct rad_req_t *req)
 	clock_gettime(CLOCK_MONOTONIC, &ts);
 
 	rad_packet_change_int(req->pack, NULL, "Acct-Delay-Time", ts.tv_sec - req->ts + conf_acct_delay_start);
-	req_set_RA(req, req->serv->secret);
+	req_set_RA(req);
 
 	return 0;
 }
@@ -308,7 +317,7 @@ static int __rad_acct_start(struct radius_pd_t *rpd)
 
 	if (conf_acct_delay_time)
 		req->before_send = rad_acct_before_send;
-	else if (req_set_RA(req, req->serv->secret))
+	else if (req_set_RA(req))
 		goto out_err;
 
 	req->recv = rad_acct_start_recv;
@@ -524,7 +533,7 @@ int rad_acct_stop(struct radius_pd_t *rpd)
 
 	rad_packet_change_val(req->pack, NULL, "Acct-Status-Type", "Stop");
 	req_set_stat(req, rpd->ses);
-	req_set_RA(req, req->serv->secret);
+	req_set_RA(req);
 
 	req->recv = rad_acct_stop_recv;
 	req->timeout.expire = rad_acct_stop_timeout;
diff --git a/accel-pppd/radius/auth.c b/accel-pppd/radius/auth.c
index 593623ee..d2cb9803 100644
--- a/accel-pppd/radius/auth.c
+++ b/accel-pppd/radius/auth.c
@@ -22,6 +22,7 @@ static int decrypt_chap_mppe_keys(struct rad_req_t *req, struct rad_attr_t *attr
 	uint8_t md5[MD5_DIGEST_LENGTH];
 	uint8_t sha1[SHA_DIGEST_LENGTH];
 	uint8_t plain[32];
+	char *secret;
 	int i;
 
 	if (attr->len != 32) {
@@ -29,10 +30,14 @@ static int decrypt_chap_mppe_keys(struct rad_req_t *req, struct rad_attr_t *attr
 		return -1;
 	}
 
+	secret = rad_server_secret_dup(req->serv);
+	if (!secret)
+		return -1;
+
 	memcpy(plain, attr->val.octets, 32);
 
 	MD5_Init(&md5_ctx);
-	MD5_Update(&md5_ctx, req->serv->secret, strlen(req->serv->secret));
+	MD5_Update(&md5_ctx, secret, strlen(secret));
 	MD5_Update(&md5_ctx, req->pack->buf + 4, 16);
 	MD5_Final(md5, &md5_ctx);
 
@@ -40,7 +45,7 @@ static int decrypt_chap_mppe_keys(struct rad_req_t *req, struct rad_attr_t *attr
 		plain[i] ^= md5[i];
 
 	MD5_Init(&md5_ctx);
-	MD5_Update(&md5_ctx, req->serv->secret, strlen(req->serv->secret));
+	MD5_Update(&md5_ctx, secret, strlen(secret));
 	MD5_Update(&md5_ctx, attr->val.octets, 16);
 	MD5_Final(md5, &md5_ctx);
 
@@ -54,6 +59,7 @@ static int decrypt_chap_mppe_keys(struct rad_req_t *req, struct rad_attr_t *attr
 	SHA1_Final(sha1, &sha1_ctx);
 
 	memcpy(key, sha1, 16);
+	_free(secret);
 
 	return 0;
 }
@@ -63,6 +69,7 @@ static int decrypt_mppe_key(struct rad_req_t *req, struct rad_attr_t *attr, uint
 	MD5_CTX md5_ctx;
 	uint8_t md5[16];
 	uint8_t plain[32];
+	char *secret;
 	int i;
 
 	if (attr->len != 34) {
@@ -75,8 +82,12 @@ static int decrypt_mppe_key(struct rad_req_t *req, struct rad_attr_t *attr, uint
 		return -1;
 	}
 
+	secret = rad_server_secret_dup(req->serv);
+	if (!secret)
+		return -1;
+
 	MD5_Init(&md5_ctx);
-	MD5_Update(&md5_ctx, req->serv->secret, strlen(req->serv->secret));
+	MD5_Update(&md5_ctx, secret, strlen(secret));
 	MD5_Update(&md5_ctx, req->pack->buf + 4, 16);
 	MD5_Update(&md5_ctx, attr->val.octets, 2);
 	MD5_Final(md5, &md5_ctx);
@@ -88,17 +99,19 @@ static int decrypt_mppe_key(struct rad_req_t *req, struct rad_attr_t *attr, uint
 
 	if (plain[0] != 16) {
 		log_ppp_warn("radius: %s: incorrect key length (%i)\n", attr->attr->name, plain[0]);
+		_free(secret);
 		return -1;
 	}
 
 	MD5_Init(&md5_ctx);
-	MD5_Update(&md5_ctx, req->serv->secret, strlen(req->serv->secret));
+	MD5_Update(&md5_ctx, secret, strlen(secret));
 	MD5_Update(&md5_ctx, attr->val.octets + 2, 16);
 	MD5_Final(md5, &md5_ctx);
 
 	plain[16] ^= md5[0];
 
 	memcpy(key, plain + 1, 16);
+	_free(secret);
 
 	return 0;
 }
@@ -275,11 +288,17 @@ int rad_auth_pap(struct radius_pd_t *rpd, const char *username, va_list args)
 	const char *passwd = va_arg(args, const char *);
 	uint8_t *epasswd;
 	int epasswd_len;
+	char *secret;
 
 	if (!req)
 		return PWDB_DENIED;
 
-	epasswd = encrypt_password(passwd, req->serv->secret, req->RA, &epasswd_len);
+	secret = rad_server_secret_dup(req->serv);
+	if (!secret)
+		return PWDB_DENIED;
+
+	epasswd = encrypt_password(passwd, secret, req->RA, &epasswd_len);
+	_free(secret);
 	if (!epasswd)
 		return PWDB_DENIED;
 
@@ -509,4 +528,3 @@ int rad_auth_null(struct radius_pd_t *rpd, const char *username, va_list args)
 
 	return PWDB_WAIT;
 }
-
diff --git a/accel-pppd/radius/radius_p.h b/accel-pppd/radius/radius_p.h
index 4c2aedaf..d3a72204 100644
--- a/accel-pppd/radius/radius_p.h
+++ b/accel-pppd/radius/radius_p.h
@@ -249,6 +249,7 @@ int rad_dae_src_check(in_addr_t ipaddr);
 struct rad_server_t *rad_server_get(int);
 struct rad_server_t *rad_server_get2(int, in_addr_t, int);
 struct rad_server_t *rad_server_put(struct rad_server_t *, int);
+char *rad_server_secret_dup(struct rad_server_t *);
 int rad_server_req_enter(struct rad_req_t *);
 void rad_server_req_exit(struct rad_req_t *);
 int rad_server_req_cancel(struct rad_req_t *, int full);
diff --git a/accel-pppd/radius/req.c b/accel-pppd/radius/req.c
index d2051078..72c46b16 100644
--- a/accel-pppd/radius/req.c
+++ b/accel-pppd/radius/req.c
@@ -78,7 +78,9 @@ static struct rad_req_t *__rad_req_alloc(struct radius_pd_t *rpd, int code, cons
 	if (code == CODE_ACCESS_REQUEST && conf_blast_protection) {
 		uint8_t buf[HMAC_MD5_LEN] = {0};
 		req->pack->message_authenticator = 1;
-		req->pack->secret = (uint8_t *)_strdup(req->serv->secret);
+		req->pack->secret = (uint8_t *)rad_server_secret_dup(req->serv);
+		if (!req->pack->secret)
+			goto out_err;
 		if (rad_packet_add_octets(req->pack, NULL, "Message-Authenticator", buf, HMAC_MD5_LEN)) {
 			_free(req->pack->secret);
 			req->pack->secret = NULL;
diff --git a/accel-pppd/radius/serv.c b/accel-pppd/radius/serv.c
index 8005bff6..b48e53ac 100644
--- a/accel-pppd/radius/serv.c
+++ b/accel-pppd/radius/serv.c
@@ -113,6 +113,17 @@ struct rad_server_t *rad_server_put(struct rad_server_t *s, int type)
 	return (do_free || do_close) ? NULL : s;
 }
 
+char *rad_server_secret_dup(struct rad_server_t *s)
+{
+	char *secret;
+
+	pthread_mutex_lock(&s->lock);
+	secret = _strdup(s->secret);
+	pthread_mutex_unlock(&s->lock);
+
+	return secret;
+}
+
 static void req_wakeup(struct rad_req_t *req)
 {
 	struct timespec ts;
@@ -425,18 +436,27 @@ void rad_server_stat_interim_query(struct rad_server_t *s, unsigned int dt)
 	stat_accm_add(s->stat.interim_query_5m, dt);
 }
 
-static int req_set_RA(struct rad_req_t *req, const char *secret)
+static int req_set_RA(struct rad_req_t *req)
 {
+	char *secret;
 	MD5_CTX ctx;
 
-	if (rad_packet_build(req->pack, req->RA))
+	secret = rad_server_secret_dup(req->serv);
+	if (!secret)
 		return -1;
 
+	if (rad_packet_build(req->pack, req->RA)) {
+		_free(secret);
+		return -1;
+	}
+
 	MD5_Init(&ctx);
 	MD5_Update(&ctx, req->pack->buf, req->pack->len);
 	MD5_Update(&ctx, secret, strlen(secret));
 	MD5_Final(req->pack->buf + 4, &ctx);
 
+	_free(secret);
+
 	return 0;
 }
 
@@ -523,7 +543,7 @@ static void send_acct_on(struct rad_server_t *s)
 		if (rad_packet_add_ipaddr(req->pack, NULL, "NAS-IP-Address", conf_nas_ip_address))
 			goto out_err;
 
-	if (req_set_RA(req, s->secret))
+	if (req_set_RA(req))
 		goto out_err;
 
 	__rad_req_send(req, 0);
@@ -674,6 +694,7 @@ static int show_stat_exec(const char *cmd, char * const *fields, int fields_cnt,
 static void __add_server(struct rad_server_t *s)
 {
 	struct rad_server_t *s1;
+	char *old_secret;
 
 	list_for_each_entry(s1, &serv_list, entry) {
 		if (s1->addr == s->addr && s1->auth_port == s->auth_port && s1->acct_port == s->acct_port) {
@@ -683,6 +704,13 @@ static void __add_server(struct rad_server_t *s)
 			s1->need_free = 0;
 			s1->bind_default = s->bind_default;
 			strcpy(s1->bind_device, s->bind_device);
+			/* adopt the freshly parsed secret so changes take effect on reload */
+			pthread_mutex_lock(&s1->lock);
+			old_secret = s1->secret;
+			s1->secret = s->secret;
+			s->secret = NULL;
+			pthread_mutex_unlock(&s1->lock);
+			_free(old_secret);
 			_free(s);
 			return;
 		}
@@ -740,6 +768,7 @@ static void __free_server(struct rad_server_t *s)
 
 	triton_context_unregister(&s->ctx);
 
+	_free(s->secret);
 	_free(s);
 }