1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
|
# Additional safeguards from accel-ppp-ng PR #40
Compared against freshly fetched accel-ppp `upstream/master` at
`c414f0a7` (2026-09-07). Reference:
[accel-ppp-ng PR #40](https://github.com/accel-ppp/accel-ppp-ng/pull/40/commits),
base `c7b67eb010a99ceefe13d2f74fc672cb91ce0af9`, head
`3843773f15af477c39a326720ac5f3e713b373fb`.
The reference contains 20 commits. Several reported vulnerabilities already
have independent upstream fixes by Denys Fedoryshchenko. This branch adds the
remaining safeguards while retaining upstream's existing parsing and policy.
## What was missing
- PPPoE rejected missing Service-Name tags but accepted duplicates. A matching
earlier tag could validate a different final selection. Require exactly one
tag. Preserve silent rejection before cookie validation; no error response
is sent for an unauthenticated request.
- DHCPv6 accepted duplicate/nonempty Rapid-Commit options and emitted one
response option per occurrence. Reject malformed requests and emit at most
one Rapid-Commit. AFTR printing was already bounded, but parsing did not
validate the uncompressed, terminated DNS name or reject duplicate AFTR
options. Add those checks and an eight-layer relay nesting limit.
- DHCPv6 reply bounds were measured from a moving inner-message pointer.
Bound relay headers and both option allocators by the actual 4096-byte
allocation, including the inner header. Propagate allocation failures
through both reply builders, status insertion, and ORO expansion; discard
an incomplete reply. Encode Relay-Message length in network byte order,
excluding its option header, so valid relayed replies remain usable.
- RADIUS accepted replies based only on the identifier. Verify the Response
Authenticator using the request's wire authenticator and secret snapshot
before updating server health, releasing the request slot, or invoking its
callback. Use constant-time comparison. This is independent of the existing
outbound Message-Authenticator/`blast-protection` setting.
- Accounting signing did not preserve its computed Request Authenticator.
Centralize signing after server selection and delay updates, zero the
authenticator before each hash, and retain the result and signing secret.
This covers Start/Interim/Stop and Accounting-On/Off, including retransmits
with `acct-delay-time` disabled. Snapshot Access-Request secrets too; use
that snapshot for PAP encryption and MPPE decryption, refresh it on server
changes, and zero Message-Authenticator before each retransmit calculation.
- Accel-VRF-Name could reach an unbounded interface-name copy. Reject oversized
and embedded-NUL Access-Accept values and handle allocation failure. Enforce
the same boundary at interface lookup and the length-delimited session API.
Preserve the CoA attribute length instead of silently accepting a prefix
before NUL. Only the literal CoA value `0` removes the VRF; names such as
`0blue` are now treated as interface names. Empty names and `(NULL, 0)` still
select the default VRF.
- LCP already bounded received packets and mandatory fields, but an Echo-Request
larger than the peer's MTU produced a truncated response with the old length
field. Discard it before modifying the packet or echo state.
- Normal pooled allocations retained their former contents after free. Clear
the payload before returning it to the pool as defense in depth. This adds a
write proportional to object size; it does not replace protocol bounds checks.
The separate MEMDEBUG allocator is unchanged.
## Reference commit coverage
Reference hashes link to the original authors' commits. “Present” identifies
upstream code already providing the relevant safeguard, not necessarily an
identical patch.
| Reference commit | Topic | Upstream baseline and this branch |
| --- | --- | --- |
| [ef7323fd](https://github.com/accel-ppp/accel-ppp-ng/commit/ef7323fd6dfe5655a46581733c86f4abcbfee548) | T8464 Service-Name | Missing-name rejection present (`f8dfbeca`, `4deb615d`); add duplicate rejection. |
| [371e7f0a](https://github.com/accel-ppp/accel-ppp-ng/commit/371e7f0a3e21d3040941330858274e104f1dfdcd) | T8473 Rapid-Commit | Add parser checks and single response option. |
| [10a3e72d](https://github.com/accel-ppp/accel-ppp-ng/commit/10a3e72d523c7fe910a23febe979a3de9cd55d8f) | T8474 AFTR | Printer bounds present (`3004db8b`); add parser semantics and duplicate checks. |
| [bd1806e7](https://github.com/accel-ppp/accel-ppp-ng/commit/bd1806e74baff0ef9b6a24a1a73741f8b7ca5125) | T8475 relay layers | Inner bounds/progress present (`b0e7444c`, `a9bfcdb8`, `94993558`); add depth and physical reply bounds, including caller failure handling. |
| [248e7caa](https://github.com/accel-ppp/accel-ppp-ng/commit/248e7caa2cf5c2e1fcaad60abedd7ccfc6365d3b) | T8526 response authentication | Add verification with upstream's OpenSSL API and reload-safe secret ownership. |
| [7ce6244f](https://github.com/accel-ppp/accel-ppp-ng/commit/7ce6244f044f10650adadfcb34e8b6f095a8967b) | T8543 Framed-Route | Present (`a4c79494`, `027bff94`); retain configurable strictness. |
| [be2f54db](https://github.com/accel-ppp/accel-ppp-ng/commit/be2f54dbf8272d6bdd3da1c98c7d9838b95dc27b) | T8545 accounting RA | Add shared zero/hash/save signing. |
| [24afb859](https://github.com/accel-ppp/accel-ppp-ng/commit/24afb859e0a28a509d930a6cf9bea1d8f5baf732) | T8549 status printing | Present (`6019e1d1`, `52a01730`). |
| [2fa64759](https://github.com/accel-ppp/accel-ppp-ng/commit/2fa647594e9d2c64e2f75e3e1537a748e5568d73) | T8550 SSTP lengths | Present (`1719b4ab`). |
| [55a804af](https://github.com/accel-ppp/accel-ppp-ng/commit/55a804afaaddd671fbb58290977a59c03369ab5e) | T8611 VRF | Add Access-Accept, CoA, session API, and shared lookup safeguards. |
| [c93b3675](https://github.com/accel-ppp/accel-ppp-ng/commit/c93b36755c285ef05400fa7be34f7560f02fe2e3) | T8825 tag copy | Present (`c32518d7`); no duplicate port. |
| [1a42ab40](https://github.com/accel-ppp/accel-ppp-ng/commit/1a42ab408da86acde9292120835bd76eb82e8809) | T8830 PPP lengths/pool clear | Packet bounds present (`028b942d`, `d22666a8`); add pool payload clearing. |
| [1957e7d1](https://github.com/accel-ppp/accel-ppp-ng/commit/1957e7d1c131b65768f9fc4b3881e5fdb544f159) | T8830 LCP fields/Echo | Mandatory-field access checks present (`028b942d`); add Echo MTU rejection. |
| [64311652](https://github.com/accel-ppp/accel-ppp-ng/commit/64311652003f719a16cecbb1d5be71af02f57370) | T8545 Accounting-On/Off | Use shared signing; callback UAF already fixed by `7bd6843c`. |
| [01baa010](https://github.com/accel-ppp/accel-ppp-ng/commit/01baa010a45b01ff067405646da89411fd2cc02a) | T8464 cookie ordering | Upstream silently rejects malformed Service-Name; preserve that safe behavior. |
| [8305387d](https://github.com/accel-ppp/accel-ppp-ng/commit/8305387d80c7a5009988106702b23c6eeca8f57a) | T8825 tag fixup | Existing capacity check protects the write; no duplicate port. |
| [f52607d4](https://github.com/accel-ppp/accel-ppp-ng/commit/f52607d4ae216e7d84d7bdacd8581a136248b3e2) | T8545 EVP cleanup | Upstream uses stack MD5 contexts; no EVP allocation to free here. |
| [177238b1](https://github.com/accel-ppp/accel-ppp-ng/commit/177238b1799456510d56637eaa210d62eedef353) | T8611 allocation failure | Include VRF allocation failure handling. |
| [2f18c76d](https://github.com/accel-ppp/accel-ppp-ng/commit/2f18c76d92eef9d09805146847ee690234371963) | T8550 cast fixup | SSTP safeguard already present. |
| [3843773f](https://github.com/accel-ppp/accel-ppp-ng/commit/3843773f15af477c39a326720ac5f3e713b373fb) | T8474 log indentation | New parser log follows upstream style; no standalone cosmetic port. |
## Credits
Original safeguard patches: **Ritika Chopra** `<r.chopra@vyos.io>`.
The tag fixup `8305387d` is authored as **RC** `<ritika0313@gmail.com>` and
carries the original trailer
`Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>`.
That fixup is already covered upstream and was not copied into this branch.
Adaptations retain Ritika's credit in their commit messages. Existing upstream
fixes and this integration are by **Denys Fedoryshchenko**.
## Validation
Run from the repository root:
```sh
cmake -S . -B /tmp/accel-ppp-ng40-build -DCMAKE_BUILD_TYPE=Debug \
-DBUILD_IPOE_DRIVER=FALSE -DBUILD_VLAN_MON_DRIVER=FALSE
cmake --build /tmp/accel-ppp-ng40-build -j 8
sh tests/unit/run_safeguards.sh /tmp/accel-ppp-ng40-build
git diff --check
```
Build, the existing IPv6 DNS selection test, and five standalone suites passed. Tests compile production code with
small external-service stubs. DHCPv6, RADIUS, VRF, and pool tests use ASan,
UBSan, and LeakSanitizer; the LCP test uses UBSan (ASan's global registration
otherwise retains the entire daemon's layer table). No sanitizer failures.
The VRF harness emits an unused-path compiler warning about the uninitialized
namespace configuration in `alloc_net`; that path is not executed.
Covered: duplicate/nonempty Rapid-Commit, valid single Rapid-Commit replies,
malformed/duplicate AFTR, direct and eight-layer replies, excessive nesting,
exact buffer fill and overflow rejection, negative allocation lengths, ORO
expansion exhaustion in both reply builders, response digests for Accept/
Reject/Challenge/Accounting, forged reply rejection before callbacks/health,
valid reply delivery, Message-Authenticator retransmission, accounting digest
recomputation and secret changes,
VRF boundaries/embedded NUL/CoA/default removal, Echo MTU boundaries, and pool
payload clearing on reuse.
Baseline reproductions against `c414f0a7` failed as expected: the duplicate
Rapid-Commit rejection assertion and the forged Access-Accept rejection
assertion. The corresponding patched checks pass.
Limits: these are focused in-process tests, not a complete deployed NAS or
network-namespace session suite. PPPoE duplicate rejection was reviewed against
its real parser and callers but was not exercised over an Ethernet interface.
Live RADIUS server failover, full PAP/MPPE interoperability, and pool-clearing
throughput were not measured. In particular, upstream already sends PAP's
original encrypted password when changing to a server with a different secret;
this branch does not redesign that pre-existing failover behavior.
|
