From bd51fe8dbec25b1f130db3b4859895124841cfe8 Mon Sep 17 00:00:00 2001 From: Denys Fedoryshchenko Date: Mon, 7 Sep 2026 22:08:04 +0300 Subject: radius: authenticate replies against the request on the wire Verify replies before callbacks and server-health updates. Keep the secret with the request and centralize accounting signing after server selection, including retransmits and Accounting-On/Off. Reset the outbound Message-Authenticator field before recalculating it on retries. Adapted from Ritika Chopra's accel-ppp-ng PR #40, T8526/T8545, with upstream's OpenSSL API and secret-reload ownership. Existing accounting callback lifetime fixes are retained. Co-authored-by: Ritika Chopra --- accel-pppd/radius/acct.c | 33 --------------------- accel-pppd/radius/auth.c | 16 ++++------- accel-pppd/radius/packet.c | 1 + accel-pppd/radius/req.c | 71 ++++++++++++++++++++++++++++++++++++++++------ accel-pppd/radius/serv.c | 38 +++++++------------------ 5 files changed, 80 insertions(+), 79 deletions(-) diff --git a/accel-pppd/radius/acct.c b/accel-pppd/radius/acct.c index d30aae7a..e9b2147f 100644 --- a/accel-pppd/radius/acct.c +++ b/accel-pppd/radius/acct.c @@ -6,7 +6,6 @@ #include #include -#include #include "linux_ppp.h" @@ -25,30 +24,6 @@ #define INTERIM_SAFE_TIME 10 -static int req_set_RA(struct rad_req_t *req) -{ - char *secret; - MD5_CTX ctx; - - 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; -} - static int req_set_stat(struct rad_req_t *req, struct ap_session *ses) { struct timespec ts; @@ -188,9 +163,6 @@ static void rad_acct_interim_update(struct triton_timer_t *t) rpd->acct_req->ts = ts.tv_sec; rpd->acct_req->pack->id++; - if (!rpd->acct_req->before_send) - req_set_RA(rpd->acct_req); - rpd->acct_req->timeout.expire_tv.tv_sec = conf_timeout; rpd->acct_req->try = 0; @@ -221,8 +193,6 @@ 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); - return 0; } @@ -317,8 +287,6 @@ 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)) - goto out_err; req->recv = rad_acct_start_recv; req->timeout.expire = rad_acct_start_timeout; @@ -550,7 +518,6 @@ 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->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 d2cb9803..63b1345e 100644 --- a/accel-pppd/radius/auth.c +++ b/accel-pppd/radius/auth.c @@ -22,7 +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; + const char *secret; int i; if (attr->len != 32) { @@ -30,7 +30,7 @@ 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); + secret = (const char *)req->pack->secret; if (!secret) return -1; @@ -59,7 +59,6 @@ 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; } @@ -69,7 +68,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; + const char *secret; int i; if (attr->len != 34) { @@ -82,7 +81,7 @@ 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); + secret = (const char *)req->pack->secret; if (!secret) return -1; @@ -99,7 +98,6 @@ 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; } @@ -111,7 +109,6 @@ static int decrypt_mppe_key(struct rad_req_t *req, struct rad_attr_t *attr, uint plain[16] ^= md5[0]; memcpy(key, plain + 1, 16); - _free(secret); return 0; } @@ -288,17 +285,16 @@ 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; + const char *secret; if (!req) return PWDB_DENIED; - secret = rad_server_secret_dup(req->serv); + secret = (const char *)req->pack->secret; if (!secret) return PWDB_DENIED; epasswd = encrypt_password(passwd, secret, req->RA, &epasswd_len); - _free(secret); if (!epasswd) return PWDB_DENIED; diff --git a/accel-pppd/radius/packet.c b/accel-pppd/radius/packet.c index 4a0ab244..9f135507 100644 --- a/accel-pppd/radius/packet.c +++ b/accel-pppd/radius/packet.c @@ -881,6 +881,7 @@ int rad_packet_send(struct rad_packet_t *pack, int fd, struct sockaddr_in *addr) uint8_t hmac[HMAC_MD5_LEN]; uint8_t *ptr = pack->buf; uint8_t *hmac_ptr = ptr + PACKET_SIGNED_OFFSET; + memset(hmac_ptr, 0, HMAC_MD5_LEN); if (hmac_md5((const uint8_t *)pack->secret, strlen((const char *)pack->secret), pack->buf, pack->len, hmac) < 0) { log_emerg("radius:packet: failed to calculate HMAC\n"); return -1; diff --git a/accel-pppd/radius/req.c b/accel-pppd/radius/req.c index 72c46b16..9a957493 100644 --- a/accel-pppd/radius/req.c +++ b/accel-pppd/radius/req.c @@ -5,6 +5,8 @@ #include #include #include +#include +#include #include #include #include @@ -17,6 +19,49 @@ #define HMAC_MD5_LEN 16 +/* Keep the signing secret and authenticator with the packet across reloads. */ +static int rad_req_set_RA(struct rad_req_t *req) +{ + char *secret = rad_server_secret_dup(req->serv); + MD5_CTX ctx; + + if (!secret) + return -1; + + memset(req->RA, 0, sizeof(req->RA)); + 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); + memcpy(req->RA, req->pack->buf + 4, sizeof(req->RA)); + _free(req->pack->secret); + req->pack->secret = (uint8_t *)secret; + return 0; +} + +static int verify_response_authenticator(struct rad_req_t *req, struct rad_packet_t *pack) +{ + uint8_t expected[MD5_DIGEST_LENGTH]; + MD5_CTX ctx; + + if (!pack || !pack->buf || pack->len < 20 || + !req->pack->buf || !req->pack->secret) + return -1; + + MD5_Init(&ctx); + MD5_Update(&ctx, pack->buf, 4); + MD5_Update(&ctx, req->pack->buf + 4, 16); + MD5_Update(&ctx, pack->buf + 20, pack->len - 20); + MD5_Update(&ctx, req->pack->secret, strlen((char *)req->pack->secret)); + MD5_Final(expected, &ctx); + return CRYPTO_memcmp(expected, pack->buf + 4, sizeof(expected)); +} + static int make_socket(struct rad_req_t *req); static mempool_t req_pool; @@ -75,12 +120,15 @@ static struct rad_req_t *__rad_req_alloc(struct radius_pd_t *rpd, int code, cons if (!req->pack) goto out_err; - if (code == CODE_ACCESS_REQUEST && conf_blast_protection) { - uint8_t buf[HMAC_MD5_LEN] = {0}; - req->pack->message_authenticator = 1; + if (code == CODE_ACCESS_REQUEST) { req->pack->secret = (uint8_t *)rad_server_secret_dup(req->serv); if (!req->pack->secret) goto out_err; + } + + if (code == CODE_ACCESS_REQUEST && conf_blast_protection) { + uint8_t buf[HMAC_MD5_LEN] = {0}; + req->pack->message_authenticator = 1; if (rad_packet_add_octets(req->pack, NULL, "Message-Authenticator", buf, HMAC_MD5_LEN)) { _free(req->pack->secret); req->pack->secret = NULL; @@ -380,6 +428,10 @@ int __rad_req_send(struct rad_req_t *req, int async) if (req->before_send && req->before_send(req)) goto out_err; + /* Re-sign accounting after server selection, including retries without delay-time. */ + if (req->pack->code == CODE_ACCOUNTING_REQUEST && rad_req_set_RA(req)) + goto out_err; + if (!req->pack->buf && rad_packet_build(req->pack, req->RA)) goto out_err; @@ -461,12 +513,15 @@ int rad_req_read(struct triton_md_handler_t *h) if (rad_packet_recv(h->fd, &pack, NULL)) return 0; - rad_server_reply(req->serv); - - if (pack->id == req->pack->id) - break; + if (!pack) + return 0; + if (pack->id != req->pack->id || verify_response_authenticator(req, pack)) { + rad_packet_free(pack); + continue; + } - rad_packet_free(pack); + rad_server_reply(req->serv); + break; } req->reply = pack; diff --git a/accel-pppd/radius/serv.c b/accel-pppd/radius/serv.c index 71398c82..34367731 100644 --- a/accel-pppd/radius/serv.c +++ b/accel-pppd/radius/serv.c @@ -11,7 +11,6 @@ #include #include -#include #include "log.h" #include "triton.h" @@ -315,10 +314,19 @@ void rad_server_req_exit(struct rad_req_t *req) int rad_server_realloc(struct rad_req_t *req) { struct rad_server_t *s = __rad_server_get(req->type, req->serv, 0, 0); + char *secret; if (!s) return -1; + secret = rad_server_secret_dup(s); + if (!secret) { + rad_server_put(s, req->type); + return -1; + } + _free(req->pack->secret); + req->pack->secret = (uint8_t *)secret; + if (req->serv) rad_server_put(req->serv, req->type); @@ -449,30 +457,6 @@ 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) -{ - char *secret; - MD5_CTX ctx; - - 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; -} - static void acct_on_sent(struct rad_req_t *req, int res) { if (!res && !req->hnd.tpd) { @@ -556,11 +540,9 @@ 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)) + if (__rad_req_send(req, 0)) goto out_err; - __rad_req_send(req, 0); - triton_timer_add(&s->ctx, &req->timeout, 0); return; -- cgit v1.2.3