LKMA-199: Add pppd egress automation - #21
Merged
Merged
Conversation
nicolasumaras
commented
Jun 26, 2026
nicolasumaras
left a comment
Owner
Author
There was a problem hiding this comment.
Approved
Reviewer verdict for PR #21 (head 863ff33).
Inspected
deploy/ppp/ip-up,deploy/ppp/ip-down— POSIX sh; pass pppd $1 (iface), $4 (local), $5 (remote), $6 (ipparam=call id) into the helper. Fall back toPPP_IPPARAMif$6is empty.deploy/install-systemd.sh— installs/usr/lib/sipfax/sipfax-egress-apply,/etc/ppp/ip-up.d/sipfax-egress,/etc/ppp/ip-down.d/sipfax-egressas root-owned 0755.bin/sipfax-egress-apply— reads/run/sipfax/ppp-leases/<callId>.json, prefersnft -f -(atomic batch apply / atomic batch tear-down viadelete table), falls back toiptables-nft(theniptables), togglesnet.ipv4.ip_forwardplusnet.ipv4.conf.<egress-iface>.forwardingonly while a lease is active (/run/sipfax/ppp-egress-active/), and best-effort POSTsip-up/ip-downto/ppp/events.src/ppp.js—EgressPolicy.firewallRulesNft()companion emits per-callinet sipfax_<call>filter table +ip sipfax_nat_<call>table with deterministicdelete tabledown rules.leaseDescriptor()ships bothnft.{up,down}andiptables.{up,down}rule arrays, plusoutboundInterfaceandoperatorUrl.src/pppd-supervisor.js—writeEgressDescriptor()writes/run/sipfax/ppp-leases/<callId>.jsonat 0640 beforepppdstarts; the unprivileged SIPfax process never touches firewall state.src/operator.js—POST /ppp/eventsaccepts the diagnostics events, keeps the last 20.deploy/README.md— privileged install steps,nftables/ppppackage install, root-only firewall ownership, and post-disconnectnft list rulesetverification documented.test/session.test.js:egress policy renders nftables rules and per-call descriptors— asserts nft + iptables rule shapes in the descriptor.operator HTTP accepts PPP egress diagnostics events— round-trips a postedip-upevent.pppd supervisor writes egress lease descriptor before daemon start— asserts SIPfax writes the descriptor before invoking pppd.sipfax-egress-apply applies and rolls back nft rules across a PPP cycle— mocksnftandsysctl, executes up then down, and assertsip_forward=1, per-ifaceforwarding=1,add table inet sipfax_call_cycle,delete table inet sipfax_call_cycle, andip_forward=0after the final lease tear-down.
Checks on head 863ff33
- GitHub CI
test: SUCCESS (run 28251890348). mergeable=MERGEABLE,mergeStateStatus=CLEAN.- Branch
lkma-199-pppd-egressis its own task branch with exactly one commit on top oforigin/main(current main HEAD3c022c1). - Local
npm test: 36/36 passing.
Notes (non-blocking)
- Lease descriptors under
/run/sipfax/ppp-leases/<callId>.jsonpersist afterip-down; they are tmpfs and reset on reboot, but SIPfax has no explicitunlinkat session terminate. Worth a tiny follow-up to remove them inPppdSupervisor.stop()/removeSessionFiles()so an instance that does many calls between reboots doesn't accumulate stale descriptors. - iptables fallback uses
iptables -Dfordown, which is non-idempotent: a partial-apply onupwould leave residue ondown. Acceptable whilenftis the documented default; revisit if iptables-nft becomes the primary path.
Merge posture
Not merging in this heartbeat. Acceptance criteria require Operator/Board sign-off on the privileged install steps in deploy/README.md before this lands on main. After Operator/Board signs off, I will merge.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verification