[Accel-ppp-users] [PATCH] radius: rework Framed-Route attribute handling
Status: Beta
Brought to you by:
xebd
Menu
▾
▴
[Accel-ppp-users] [PATCH] radius: rework Framed-Route attribute handling
|
From: Guillaume N. <g....@al...> - 2018-12-12 10:16:35
|
Parse Framed-Route attributes using u_parse_*() functions. This is
heavily based on the Framed-IPv6-Route parser. As a consequence, prefix
length is now mandatory.
While there, re-arrange struct frame_route to make it more like the
IPv6 version.
I haven't tried to keep the optional prefix length because the current
implementation contradicts RFC 2865's recommendation, which is to
respect the original classfull network addressing scheme (accel-ppp
always considers prefix length to be 32 when not given explicitly).
There's no point in handling classfull networks anymore (CIDR was
introduced more than 25 years ago...), but having our own special and
incompatible interpretation can be troublesome for operators
(especially since the behaviours differ in a subtle way that isn't
immediately visible).
Signed-off-by: Guillaume Nault <g....@al...>
---
Although my preference goes into making the prefix length explicit, I
can look into making it optional to keep the current behaviour. Just
let me know.
accel-pppd/radius/radius.c | 237 ++++++++++++++++++++---------------
accel-pppd/radius/radius_p.h | 12 +-
2 files changed, 145 insertions(+), 104 deletions(-)
diff --git a/accel-pppd/radius/radius.c b/accel-pppd/radius/radius.c
index 062e3b72..da68667e 100644
--- a/accel-pppd/radius/radius.c
+++ b/accel-pppd/radius/radius.c
@@ -60,94 +60,6 @@ static struct ipdb_t ipdb;
static mempool_t rpd_pool;
static mempool_t auth_ctx_pool;
-static void parse_framed_route(struct radius_pd_t *rpd, const char *attr)
-{
- char str[32];
- char *ptr;
- long int prio = 0;
- in_addr_t dst;
- in_addr_t gw;
- int mask;
- struct framed_route *fr;
-
- ptr = strchr(attr, '/');
- if (ptr && ptr - attr > 16)
- goto out_err;
-
- if (ptr) {
- memcpy(str, attr, ptr - attr);
- str[ptr - attr] = 0;
- } else {
- ptr = strchr(attr, ' ');
- if (ptr) {
- memcpy(str, attr, ptr - attr);
- str[ptr - attr] = 0;
- } else
- strcpy(str, attr);
- }
-
- dst = inet_addr(str);
- if (dst == INADDR_NONE)
- goto out_err;
-
- if (ptr) {
- if (*ptr == '/') {
- char *ptr2;
- for (ptr2 = ++ptr; *ptr2 && *ptr2 != '.' && *ptr2 != ' '; ptr2++);
- if (*ptr2 == '.' && ptr2 - ptr <= 16) {
- in_addr_t a;
- memcpy(str, ptr, ptr2 - ptr);
- str[ptr2 - ptr] = 0;
- a = ntohl(inet_addr(str));
- if (a == INADDR_NONE)
- goto out_err;
- mask = 33 - htonl(inet_addr(str));
- if (~((1<<(32 - mask)) - 1) != a)
- goto out_err;
- } else if (*ptr2 == ' ' || *ptr2 == 0) {
- char *ptr3;
- mask = strtol(ptr, &ptr3, 10);
- if (mask < 0 || mask > 32 || ptr3 != ptr2)
- goto out_err;
- } else
- goto out_err;
- } else
- mask = 32;
-
- for (++ptr; *ptr && *ptr != ' '; ptr++);
- if (*ptr == ' ')
- gw = inet_addr(ptr + 1);
- else if (*ptr == 0)
- gw = 0;
- else
- goto out_err;
-
- /* Parse priority, if any */
- if (*ptr) {
- for (++ptr; *ptr && *ptr != ' '; ptr++);
- if (*ptr == ' ')
- if (u_readlong(&prio, ptr + 1, 0, UINT32_MAX) < 0)
- goto out_err;
- }
- } else {
- mask = 32;
- gw = 0;
- }
-
- fr = _malloc(sizeof (*fr));
- fr->dst = dst;
- fr->mask = mask;
- fr->gw = gw;
- fr->prio = prio;
- fr->next = rpd->fr;
- rpd->fr = fr;
-
- return;
-
-out_err:
- log_ppp_warn("radius: failed to parse Framed-Route=%s\n", attr);
-}
-
/* Parse a RADIUS Framed-IPv6-Route string.
*
* Full format is like: "2001:db8::/32 fc00::1 2000"
@@ -245,6 +157,103 @@ static int parse_framed_ipv6_route(const char *str,
return 0;
}
+/* Parse a RADIUS Framed-Route string.
+ *
+ * Full format is like: "192.0.2.0/24 203.0.113.1 8"
+ *
+ * * "192.0.2.0/24" is the network prefix
+ * * "203.0.113.1" is the gateway address
+ * * "8" is the route metric (priority)
+ *
+ * The route metric can be omitted, in which case it is set to 0. This let the
+ * kernel use its own default route metric.
+ * If the route metric is not set, the gateway address can be omitted too. In
+ * this case, it's set to the unspecified address ('0.0.0.0'). This makes the
+ * route use the session's network interface directly rather than an IP
+ * gateway.
+ */
+static int parse_framed_route(const char *str, struct framed_route *fr)
+{
+ const char *ptr;
+ size_t len;
+
+ /* Skip leading spaces */
+ ptr = str + u_parse_spaces(str);
+
+ /* Get network prefix and prefix length */
+ len = u_parse_ip4cidr(ptr, &fr->prefix, &fr->plen);
+ if (!len) {
+ log_ppp_warn("radius: parsing Framed-Route attribute \"%s\" failed at \"%s\":"
+ " expecting an IPv4 network prefix in CIDR notation\n",
+ str, ptr);
+ return -1;
+ }
+ ptr += len;
+
+ /* Check separator, unless string ends here */
+ len = u_parse_spaces(ptr);
+ if (!len && *ptr != '\0') {
+ log_ppp_warn("radius: parsing Framed-Route attribute \"%s\" failed at \"%s\":"
+ " missing space after network prefix\n",
+ str, ptr);
+ return -1;
+ }
+ ptr += len;
+
+ /* If end of string, use no gateway and default metric */
+ if (*ptr == '\0') {
+ fr->gw.s_addr = INADDR_ANY;
+ fr->prio = 0;
+ return 0;
+ }
+
+ /* Get the gateway address */
+ len = u_parse_ip4addr(ptr, &fr->gw);
+ if (!len) {
+ log_ppp_warn("radius: parsing Framed-Route attribute \"%s\" failed at \"%s\":"
+ " expecting a gateway IPv4 address\n",
+ str, ptr);
+ return -1;
+ }
+ ptr += len;
+
+ /* Again, separator or end of string required */
+ len = u_parse_spaces(ptr);
+ if (!len && *ptr != '\0') {
+ log_ppp_warn("radius: parsing Framed-Route attribute \"%s\" failed at \"%s\":"
+ " missing space after gateway address\n",
+ str, ptr);
+ return -1;
+ }
+ ptr += len;
+
+ /* If end of string, use default metric */
+ if (*ptr == '\0') {
+ fr->prio = 0;
+ return 0;
+ }
+
+ /* Get route metric */
+ len = u_parse_u32(ptr, &fr->prio);
+ if (!len) {
+ log_ppp_warn("radius: parsing Framed-Route attribute \"%s\" failed at \"%s\":"
+ " expecting a route metric between 0 and %u\n",
+ str, ptr, UINT32_MAX);
+ return -1;
+ }
+ ptr += len;
+
+ /* Now this must be the end of the string */
+ if (!u_parse_endstr(ptr)) {
+ log_ppp_warn("radius: parsing Framed-Route attribute \"%s\" failed at \"%s\":"
+ " unexpected data after route metric\n",
+ str, ptr + u_parse_spaces(ptr));
+ return -1;
+ }
+
+ return 0;
+}
+
static int rad_add_framed_ipv6_route(const char *str, struct radius_pd_t *rpd)
{
struct framed_ip6_route *fr6;
@@ -267,6 +276,28 @@ err:
return -1;
}
+static int rad_add_framed_route(const char *str, struct radius_pd_t *rpd)
+{
+ struct framed_route *fr;
+
+ fr = _malloc(sizeof(*fr));
+ if (!fr)
+ goto err;
+
+ if (parse_framed_route(str, fr) < 0)
+ goto err_fr;
+
+ fr->next = rpd->fr;
+ rpd->fr = fr;
+
+ return 0;
+
+err_fr:
+ _free(fr);
+err:
+ return -1;
+}
+
int rad_proc_attrs(struct rad_req_t *req)
{
struct ev_wins_t wins = {};
@@ -368,7 +399,7 @@ int rad_proc_attrs(struct rad_req_t *req)
rpd->ses->ifname_rename[attr->len] = 0;
break;
case Framed_Route:
- parse_framed_route(rpd, attr->val.string);
+ rad_add_framed_route(attr->val.string, rpd);
break;
case Framed_IPv6_Route:
rad_add_framed_ipv6_route(attr->val.string, rpd);
@@ -578,10 +609,11 @@ static void ses_started(struct ap_session *ses)
for (fr6 = rpd->fr6; fr6; fr6 = fr6->next) {
bool gw_spec = !IN6_IS_ADDR_UNSPECIFIED(&fr6->gw);
- char nbuf[INET6_ADDRSTRLEN];
- char gwbuf[INET6_ADDRSTRLEN];
if (ip6route_add(gw_spec ? 0 : rpd->ses->ifindex, &fr6->prefix, fr6->plen, gw_spec ? &fr6->gw : NULL, 3, fr6->prio)) {
+ char gwbuf[INET6_ADDRSTRLEN];
+ char nbuf[INET6_ADDRSTRLEN];
+
log_ppp_warn("radius: failed to add route %s/%hhu %s %u\n",
u_ip6str(&fr6->prefix, nbuf), fr6->plen,
u_ip6str(&fr6->gw, gwbuf), fr6->prio);
@@ -589,11 +621,15 @@ static void ses_started(struct ap_session *ses)
}
for (fr = rpd->fr; fr; fr = fr->next) {
- if (iproute_add(fr->gw ? 0 : rpd->ses->ifindex, 0, fr->dst, fr->gw, 3, fr->mask, fr->prio)) {
- char dst[17], gw[17];
- u_inet_ntoa(fr->dst, dst);
- u_inet_ntoa(fr->gw, gw);
- log_ppp_warn("radius: failed to add route %s/%i %s %u\n", dst, fr->mask, gw, fr->prio);
+ bool gw_spec = fr->gw.s_addr != INADDR_ANY;
+
+ if (iproute_add(gw_spec ? 0 : rpd->ses->ifindex, 0, fr->prefix.s_addr, fr->gw.s_addr, 3, fr->plen, fr->prio)) {
+ char gwbuf[INET_ADDRSTRLEN];
+ char nbuf[INET_ADDRSTRLEN];
+
+ log_ppp_warn("radius: failed to add route %s/%hhu %s %u\n",
+ u_ip4str(&fr->prefix, nbuf), fr->plen,
+ u_ip4str(&fr->gw, gwbuf), fr->prio);
}
}
@@ -627,8 +663,9 @@ static void ses_finishing(struct ap_session *ses)
}
for (fr = rpd->fr; fr; fr = fr->next) {
- if (fr->gw)
- iproute_del(0, fr->dst, 3, fr->mask, fr->prio);
+ /* Again, no need to remove routes with unspecified gateway. */
+ if (fr->gw.s_addr != INADDR_ANY)
+ iproute_del(0, fr->prefix.s_addr, 3, fr->plen, fr->prio);
}
if (rpd->acct_started || rpd->acct_req)
@@ -639,8 +676,8 @@ static void ses_finished(struct ap_session *ses)
{
struct radius_pd_t *rpd = find_pd(ses);
struct ipv6db_addr_t *a;
- struct framed_route *fr = rpd->fr;
struct framed_ip6_route *fr6;
+ struct framed_route *fr;
pthread_rwlock_wrlock(&sessions_lock);
pthread_mutex_lock(&rpd->lock);
@@ -699,8 +736,10 @@ static void ses_finished(struct ap_session *ses)
fr6 = next;
}
+ fr = rpd->fr;
while (fr) {
struct framed_route *next = fr->next;
+
_free(fr);
fr = next;
}
diff --git a/accel-pppd/radius/radius_p.h b/accel-pppd/radius/radius_p.h
index db8d277f..8ee30f6c 100644
--- a/accel-pppd/radius/radius_p.h
+++ b/accel-pppd/radius/radius_p.h
@@ -26,19 +26,21 @@ struct radius_auth_ctx {
};
struct framed_route {
- in_addr_t dst;
- int mask;
- in_addr_t gw;
- uint32_t prio;
struct framed_route *next;
+
+ struct in_addr prefix;
+ struct in_addr gw;
+ uint32_t prio;
+ uint8_t plen;
};
struct framed_ip6_route {
+ struct framed_ip6_route *next;
+
struct in6_addr prefix;
struct in6_addr gw;
uint32_t prio;
uint8_t plen;
- struct framed_ip6_route *next;
};
struct radius_pd_t {
--
2.20.0
|