Skip to content

LKMA-199: Add pppd egress automation - #21

Merged
nicolasumaras merged 1 commit into
mainfrom
lkma-199-pppd-egress
Jun 26, 2026
Merged

LKMA-199: Add pppd egress automation#21
nicolasumaras merged 1 commit into
mainfrom
lkma-199-pppd-egress

Conversation

@nicolasumaras

Copy link
Copy Markdown
Owner

Summary

  • add per-call PPP egress descriptors, nftables rendering, and iptables-nft fallback rules
  • install pppd ip-up/ip-down hooks plus the privileged sipfax-egress-apply helper
  • add loopback operator PPP event diagnostics and deploy documentation for nftables/privilege review

Verification

  • npm test

@nicolasumaras nicolasumaras left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 to PPP_IPPARAM if $6 is 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-egress as root-owned 0755.
  • bin/sipfax-egress-apply — reads /run/sipfax/ppp-leases/<callId>.json, prefers nft -f - (atomic batch apply / atomic batch tear-down via delete table), falls back to iptables-nft (then iptables), toggles net.ipv4.ip_forward plus net.ipv4.conf.<egress-iface>.forwarding only while a lease is active (/run/sipfax/ppp-egress-active/), and best-effort POSTs ip-up/ip-down to /ppp/events.
  • src/ppp.jsEgressPolicy.firewallRulesNft() companion emits per-call inet sipfax_<call> filter table + ip sipfax_nat_<call> table with deterministic delete table down rules. leaseDescriptor() ships both nft.{up,down} and iptables.{up,down} rule arrays, plus outboundInterface and operatorUrl.
  • src/pppd-supervisor.jswriteEgressDescriptor() writes /run/sipfax/ppp-leases/<callId>.json at 0640 before pppd starts; the unprivileged SIPfax process never touches firewall state.
  • src/operator.jsPOST /ppp/events accepts the diagnostics events, keeps the last 20.
  • deploy/README.md — privileged install steps, nftables/ppp package install, root-only firewall ownership, and post-disconnect nft list ruleset verification 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 posted ip-up event.
    • 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 — mocks nft and sysctl, executes up then down, and asserts ip_forward=1, per-iface forwarding=1, add table inet sipfax_call_cycle, delete table inet sipfax_call_cycle, and ip_forward=0 after the final lease tear-down.

Checks on head 863ff33

  • GitHub CI test: SUCCESS (run 28251890348).
  • mergeable=MERGEABLE, mergeStateStatus=CLEAN.
  • Branch lkma-199-pppd-egress is its own task branch with exactly one commit on top of origin/main (current main HEAD 3c022c1).
  • Local npm test: 36/36 passing.

Notes (non-blocking)

  • Lease descriptors under /run/sipfax/ppp-leases/<callId>.json persist after ip-down; they are tmpfs and reset on reboot, but SIPfax has no explicit unlink at session terminate. Worth a tiny follow-up to remove them in PppdSupervisor.stop() / removeSessionFiles() so an instance that does many calls between reboots doesn't accumulate stale descriptors.
  • iptables fallback uses iptables -D for down, which is non-idempotent: a partial-apply on up would leave residue on down. Acceptable while nft is 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.

@nicolasumaras
nicolasumaras merged commit 2be3f1c into main Jun 26, 2026
1 check passed
@nicolasumaras
nicolasumaras deleted the lkma-199-pppd-egress branch June 26, 2026 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant