| Age | Commit message (Collapse) | Author |
|---|
|
The legacy limit matches RFC 6106's recommendation that hosts treat three learned RDNSS addresses as sufficient. That was host-side repository guidance, not a limit in the RADIUS, DHCPv6, or RDNSS encodings.
RFC 8106 obsoletes RFC 6106 and explicitly removes the recommendation to limit learned RDNSS addresses to three, leaving the number to local policy. It instead recommends the ability to store at least three addresses when DNS information comes from multiple sources.
Store both configured and per-session servers in reusable contiguous arrays. Start with a four-address, 64-byte allocation, grow geometrically, retain capacity across reload or reauthorization, and free session storage only at teardown. Bound advertised lists by actual DHCPv6 and router-advertisement packet space.
Warn when servers have to be dropped for lack of room, as the fixed limit used to, skip the DNS search list rather than write it past the end of a full advertisement, and drop the stale three-server wording from the [ipv6-dns] documentation.
Link: https://github.com/accel-ppp/accel-ppp/commit/4f562467dbdf819395e138617c2a057e02595b9e#r197212083
Link: https://www.rfc-editor.org/rfc/rfc6106.html#section-5.3.1
Link: https://www.rfc-editor.org/rfc/rfc8106.html#section-5.3.1
Link: https://www.rfc-editor.org/rfc/rfc8106.html#appendix-A
|
|
DNS-Server-IPv6-Address carries a full IPv6 address, so storing it in
ipv6db_addr_t also carried an unused prefix length and flags. Introduce
dedicated DNS list and address types so the representation matches the data.
Suggested-by: Vladislav Grishenko <566150+themiron@users.noreply.github.com>
Link: https://github.com/accel-ppp/accel-ppp/commit/4f562467dbdf819395e138617c2a057e02595b9e#r197215804
|
|
Additional safeguards
|
|
Closes #354
|
|
Reject oversized and embedded-NUL VRF attributes and allocation failures.
Keep explicit CoA lengths, restrict removal to literal 0, and bound the
session API and shared interface lookup. Preserve default VRF removal.
Adapted from Ritika Chopra's accel-ppp-ng PR #40, T8611, with the same
validation boundary extended to the CoA path.
Co-authored-by: Ritika Chopra <r.chopra@vyos.io>
|
|
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 <r.chopra@vyos.io>
|
|
|
|
Use memcpy for scalar backup headers and restored session, pool, and RADIUS values because variable-length tags do not guarantee native integer alignment.
|
|
Serialize integer fields via memcpy, restrict attribute parsing to the RADIUS header's declared packet length, reject undersized packet lengths, and reject truncated attribute headers.
|
|
The ipv6_nd and ipv6_dhcp modules could only advertise the DNS servers
configured in [ipv6-dns], the same set for every subscriber. RFC 6911
defines DNS-Server-IPv6-Address (attribute 169) for exactly this, and
the attribute was already in the shipped dictionary and in attr_defs.h;
nothing read it.
Give struct ap_session an ipv6_dns list, filled by the radius module
from that attribute, and have both modules advertise it when the session
has one: in the RDNSS option of the router advertisements, and in the
DNS_SERVERS option of DHCPv6 replies. Sessions without a list of their
own keep getting the configured servers, so nothing changes for anyone
not sending the attribute.
The selection is a single ipv6_dns_get() shared by both modules rather
than a copy in each: they already duplicate the whole [ipv6-dns] parser,
and two copies of a precedence rule are two chances to drift. It caps
what it returns, so neither the RDNSS option length (one byte, in units
of 8) nor the router advertisement buffer can be pushed around by what a
RADIUS server sends. The radius module caps at the same 3 servers as
[ipv6-dns] accepts and warns once when a reply carries more.
An Access-Accept which carries the attribute replaces the whole
previously assigned list rather than appending to it, so a re-authorized
session ends up with the servers of the latest reply and not with a
concatenation. One which does not carry it leaves the current list
alone, which is how the IPv4 MS-Primary-DNS-Server attribute already
behaves.
ipv6_dns_test.c covers the selection: assigned wins over configured,
empty list means "nothing assigned" rather than "no DNS", the cap holds
for both sources, and a caller with no room gets nothing rather than a
stomped buffer. Wired into the ASAN/UBSAN workflow.
Inspired by the per-session IPv6 DNS support in accel-ppp-ng (commit
2df6eb99), reimplemented against mainline's ap_session and ipdb types.
|
|
c912d090 ("crypto: Removed internal tomcat crypto.") deleted the
crypto/ tree but left accel-pppd/include/crypto.h behind, a tracked
symlink to ../../crypto/crypto.h which has pointed at nothing since.
backup_file.c still includes it, so it fails to compile with
fatal error: crypto.h: No such file or directory
That goes unnoticed because accel-pppd/CMakeLists.txt has
ADD_SUBDIRECTORY(backup) commented out, i.e. backup_file.c is not part
of any build; the breakage only shows up for whoever re-enables it.
Include <openssl/md5.h> instead, which is what the file actually needs
(MD5_CTX and friends) and what c912d090 did for every file it touched.
sstp.c and radius/packet.c defer to crypto.h for the rationale behind
their OPENSSL_API_COMPAT define. Spell it out locally instead, and point
at the project wide ADD_DEFINITIONS() in the top level CMakeLists.txt
added by c1689506 ("openssl: suppress deprecated API warnings"), noting
why the local define is kept despite being redundant with it: it has to
be visible before the first OpenSSL header.
Then remove the symlink, which nothing references anymore.
|
|
req_wakeup() ignored the return value of req->send(req, 1). When a
request dequeued from the req-limit queue failed at socket setup
(__rad_req_send returns -2, e.g. under ephemeral port exhaustion or a
routing error), the server's req_cnt slot taken in
rad_server_req_exit() was never released and the request was orphaned
with no callback and no timer.
Leaked slots accumulate until req_cnt permanently saturates req-limit,
after which every request queues forever and the server is effectively
dead until restart.
Handle -2 the same way rad_server_req_enter() does: release the slot,
mark the server failed and drive the request through the regular
failover path so it either retries on another server or reports the
failure to its owner.
Signed-off-by: Denys Fedoryshchenko <denys.f@collabora.com>
|
|
When an accounting Stop request cannot be retransmitted because no
server is available (single server inside its fail-timeout window,
server removed on config reload, or a transient socket/connect error
that marks the server failed), rad_acct_stop_timeout() reset req->try
and returned. The retransmit timer is one-shot (no period), so it
never fired again: the request leaked forever together with its open
UDP socket, epoll registration and timerfd.
The same dead end existed in rad_acct_stop_sent(): a deferred Stop
request (req->rpd == NULL) whose queued send was cancelled by
rad_server_fail() fell through the failure branch without freeing the
request or scheduling a retry.
With the RADIUS client bound to a source address (bind=/nas-ip-address)
every leaked socket pins one ephemeral port. On a busy NAS each session
terminating during a short RADIUS outage leaks one socket; after months
of uptime the ephemeral port range is exhausted and every new request
fails with "radius:bind: Address already in use" followed by
"no available servers", requiring a restart.
Fix by re-arming the one-shot timer on send failure instead of
resetting the try counter, so retries are bounded by max-try and the
request is freed cleanly once attempts are exhausted.
Both defects date back to the accounting rewrite (62e89248, 2014).
Fixes: https://github.com/accel-ppp/accel-ppp/issues/324
Signed-off-by: Denys Fedoryshchenko <denys.f@collabora.com>
|
|
radius: update server secret on config reload
|
|
Several bugfixes for problems reported by Khodor Tahech
|
|
|
|
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().
|
|
If a RADIUS dictionary contains a line consisting of exactly one
word with no trailing spaces (for example, standard keywords like
"END-VENDOR\n" or "END-TLV\n" ), this bug getting triggered.
Triggering crash is compiler dependent, it might not happen now,
but a bit different compiler, flags, and it might crash on load.
Reported-by: Khedor <khedor@gmail.com>
Signed-off-by: Denys Fedoryshchenko <denys.f@collabora.com>
|
|
Group the per-server RADIUS statistics in struct rad_server_stat_t under rad_server_t.stat instead of keeping auth, accounting, interim, and failure counters as separate fields on rad_server_t. This keeps the statistics state collected behind one ownership boundary and makes the relationship between the total counters and their rolling accumulators explicit.
Route counter updates through rad_server_stat_*() helpers. Auth, accounting, interim, and server-failure paths no longer open-code individual counter increments and accumulator updates; the update policy now lives in serv.c with the rest of the RADIUS server accounting logic.
Make the CLI show-stat path render from a local snapshot. The displayed totals are loaded with relaxed atomic reads, the rolling one-minute/five-minute values are collected in one place, and the in-flight request/queue counters are copied under the server lock before printing. Future changes to synchronization or accumulator storage can stay inside the snapshot/update helpers instead of leaking into the CLI formatting code.
Signed-off-by: Denys Fedoryshchenko <denys.f@collabora.com>
|
|
Various radius fixes
|
|
When sending accounting STOP requests, the timer callback was
incorrectly set to the START timeout handler. This caused stop
retries to follow the wrong termination path.
Also clear rpd->acct_req before freeing on stop timeout/shutdown
failures to avoid leaving a stale pointer.
This bug is very nasty, revealed during stress tests, leading to
memory corruption and other bad stuff when there is noticeable
loss of radius "Stop" packets.
Signed-off-by: Denys Fedoryshchenko <denys.f@collabora.com>
|
|
Situation is a bit complex. Even one problem we fix easily,
such as saving server as variable, rad_req_free might free
server in theory, and s will be invalid.
This is a bit of rewrite, but proper fix.
Fixes problem in 2 functions.
Signed-off-by: Denys Fedoryshchenko <denys.f@collabora.com>
|
|
Actually 3 fixes in same place:
INTEGER: size mismatch now breaks instead of falling through - a malformed INTEGER attribute is rejected rather than silently parsed with a wrong size
INTEGER/DATE split: each case has its own break, no more fallthrough
DATE: strictly requires len == 4 (per RFC 2865), warns and skips otherwise instead of silently accepting 1 or 2-byte dates
Signed-off-by: Denys Fedoryshchenko <denys.f@collabora.com>
|
|
Signed-off-by: Denys Fedoryshchenko <denys.f@collabora.com>
|
|
This check left old, relevant to mmap, migrate to proper check.
Signed-off-by: Denys Fedoryshchenko <denys.f@collabora.com>
|
|
Also fix small typo.
Signed-off-by: Denys Fedoryshchenko <denys.f@collabora.com>
|
|
When parsing vendor-specific attributes (type 26),
the code reads internal structure without checking that the attribute data is long enough.
Signed-off-by: Denys Fedoryshchenko <denys.f@collabora.com>
|
|
netlink: Added VRF support
|
|
Added VRF support to the iproute routines.
Updated iproute* calls in the ipoe, radius, and dhcpv6 code.
Signed-off-by: Andrii Melnychenko <a.melnychenko@vyos.io>
|
|
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
|
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
|
Signed-off-by: Denys Fedoryshchenko <denys.f@collabora.com>
|
|
Signed-off-by: Denys Fedoryshchenko <denys.f@collabora.com>
|
|
Signed-off-by: Denys Fedoryshchenko <denys.f@collabora.com>
|
|
Signed-off-by: Denys Fedoryshchenko <denys.f@collabora.com>
|
|
Signed-off-by: Denys Fedoryshchenko <denys.f@collabora.com>
|
|
acct: Fix losing some data on interface down due wrong sequence
|
|
Ensure accounting values include the most recent traffic sample when a session disconnects,
preventing the final interval from being dropped and avoiding under-reported totals in usage/billing.
Big thanks Dmitriy Eshenko for patch and testing
Author: Dmitriy Eshenko <dmitriy.eshenko@accel-ppp.org>
Signed-off-by: Denys Fedoryshchenko <denys.f@collabora.com>
|
|
Signed-off-by: Andrii Melnychenko <a.melnychenko@vyos.io>
|
|
Suppress OpenSSL 3.0 deprecation warnings for legacy crypto APIs
|
|
Not a bug, but to supress warnings.
Signed-off-by: Denys Fedoryshchenko <denys.f@collabora.com>
|
|
We are using similar approach as in other projects, easiest one,
but probably in future it will break as soon as this functions
will be removed completely.
Signed-off-by: Denys Fedoryshchenko <denys.f@collabora.com>
|
|
Recently FreeRadius started to complain accel-ppp doesn't pass
BlastRADIUS check. This commit fixes that.
This commit implements protection against RADIUS blast attacks
by adding support for the Message-Authenticator attribute in
Access-Request packets. This security enhancement helps
prevent unauthorized access attempts and replay attacks
on RADIUS authentication.
- Added new configuration option `blast-protection=1`
in [radius] to enable Message-Authenticator inclusion
- Implemented HMAC-MD5 calculation for
Message-Authenticator attribute (RFC 2869)
- Modified packet building to include 18-byte Message-Authenticator
attribute when enabled
- Updated packet structure to support signing with shared secret
Enable blast protection by adding to the `[radius]` section:
```
blast-protection=1
```
When enabled, all Access-Request packets will include a
Message-Authenticator attribute with HMAC-MD5 signature,
providing cryptographic integrity verification and protection
against packet modification attacks.
Signed-off-by: Denys Fedoryshchenko <denys.f@collabora.com>
|
|
This patch fixes compile errors on GCC 14 like the following
/root/accel-ppp/accel-pppd/radius/packet.c: In function 'rad_packet_recv':
/root/accel-ppp/accel-pppd/radius/packet.c:142:72: error: passing argument 5 of 'recvfrom' from incompatible pointer type [-Wincompatible-pointer-types]
142 | n = recvfrom(fd, pack->buf, REQ_LENGTH_MAX, 0, addr, &addr_len);
| ^~~~
| |
| struct sockaddr_in *
In file included from /usr/include/netinet/in.h:10,
from /usr/include/arpa/inet.h:9,
from /root/accel-ppp/accel-pppd/radius/packet.c:10:
/usr/include/sys/socket.h:397:55: note: expected 'struct sockaddr * restrict' but argument is of type 'struct sockaddr_in *'
Reference: https://gcc.gnu.org/gcc-14/porting_to.html
|
|
This change enables ipv6 connections between accel-ppp and radius server
|
|
|
|
|
|
handling.
When a link has a relatively high throughput, the 32-bit packet and byte counters could overflow multiple times between accounting runs.
To accommodate this limitation, directly use 64-bit interface statistics.
This also gets rid of the internal giga-word counters.
|
|
This patch introduces nas-port-id-in-req switch to disable sending
NAS-Port-Id attribute in radauth and radacct messages. New switch
might be useful if radius server cannot properly handle this
attrubite.
Signed-off-by: Sergey V. Lobanov <sergey@lobanov.in>
|
|
fix buffer overflow when receive radius packet
|
