Skip to content

Add comprehensive input validation and per-ipset tuning parameters - #41

Draft
struanb wants to merge 4 commits into
codex/review-dockside-network-firewall.py-for-security-issuesfrom
claude/security-review-firewall-fixes-YgXxW
Draft

Add comprehensive input validation and per-ipset tuning parameters#41
struanb wants to merge 4 commits into
codex/review-dockside-network-firewall.py-for-security-issuesfrom
claude/security-review-firewall-fixes-YgXxW

Conversation

@struanb

@struanb struanb commented May 14, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR adds comprehensive input validation to the firewall daemon and introduces per-ipset configuration tuning parameters (dns_queries, stale_ttl, refresh_interval). It also implements per-ipset refresh scheduling and adds authorization checks to the management socket.

Key Changes

Input Validation Framework

  • Added a suite of validator functions (_val_identifier, _val_ip, _val_cidr, _val_mac, _val_proto, _val_port, _val_comment, _val_icmp_type, _val_host_entry) that validate all configuration inputs before any kernel mutations
  • Validators use an allow-list approach and raise ValueError with descriptive messages on invalid input
  • Applied validation to EgressRule, NatRule, and NetworkSpec constructors to catch configuration errors early

Per-Ipset Tuning Parameters

  • Introduced IpsetDef class to encapsulate ipset configuration: hostnames plus optional tuning fields
  • Added IPSET_DNS_QUERIES global constant (default: 1) to control DNS query repetition per hostname per refresh cycle
  • Ipset definitions now support both compact form (plain list of hostnames) and extended form (dict with hostnames + optional dns_queries, stale_ttl, refresh_interval fields)
  • IpsetDef.to_dict() intelligently serializes to compact form when all tuning fields match global defaults, keeping config files readable

Per-Ipset Refresh Scheduling

  • Replaced global refresh_all() with refresh_due() that only refreshes ipsets whose refresh_interval has elapsed
  • Added time_until_next_refresh() to calculate the precise sleep duration until the next ipset is due, enabling the refresh loop to wake exactly when needed rather than polling on a fixed tick
  • Updated _refresh_one() to call _resolve_hostname_multi() with per-ipset dns_queries count, allowing CDN ipsets (short interval, many queries) and stable ipsets (long interval, 1 query) to coexist
  • Added _last_refresh tracking per ipset to implement independent refresh schedules

Management Socket Authorization

  • Added SO_PEERCRED peer credential reading to extract connecting process PID/UID/GID
  • Defined _MUTATING_ACTIONS frozenset for actions that alter kernel state or persisted config
  • Implemented authorization check: mutating actions require peer UID 0 (root); read-only actions (status, refresh) are open to any peer in the socket's group
  • All requests are now logged with peer credentials for audit purposes

DNS Resolution Enhancement

  • Added _resolve_hostname_multi() function that queries DNS n times for a hostname, returning all unique IPv4 addresses discovered across queries
  • Enables faster CDN pool discovery by allowing multiple queries per refresh cycle

Git Configuration Update

  • Modified create_git_config() in launch.sh to use git config --replace-all instead of overwriting the entire .gitconfig file, preserving existing configuration while updating user name and email

Notable Implementation Details

  • All validators are intentionally strict (allow-list approach) to prevent injection attacks when values are interpolated into iptables/ipset commands
  • The IpsetDef serialization strategy maintains backward compatibility: existing configs with plain hostname lists continue to work, and only configs with non-default tuning parameters use the extended dict form
  • The refresh loop now uses monotonic time (time.monotonic()) for scheduling to avoid issues with system clock adjustments
  • Per-ipset refresh intervals are checked independently, allowing fine-grained control over DNS query frequency without a single global cadence

https://claude.ai/code/session_016QwTdAvrMgYdfJuycgobHc

claude and others added 4 commits March 25, 2026 15:17
…auth bypass

Issue 1 (High) — strict input validation before iptables-restore:
- Add validator functions (_val_identifier, _val_iface, _val_ip, _val_cidr,
  _val_mac, _val_proto, _val_port, _val_comment, _val_icmp_type,
  _val_host_entry) using allow-lists and stdlib ipaddress module.
- Call validators in EgressRule, NatRule, and NetworkSpec constructors, and
  in Config.from_dicts() for ipset names and host entries.
- Any invalid value raises ValueError before kernel mutation is attempted.

Issue 2 (Medium/High) — management socket peer credential verification:
- Read SO_PEERCRED on each accepted connection (pid, uid, gid).
- Require UID 0 (root) for all mutating actions (apply, reload, set-network,
  remove-network, set-ipset, remove-ipset, reconcile).
- Read-only actions (status, refresh) remain open to any group member.
- Log peer pid/uid/gid and action for every request (audit trail).

https://claude.ai/code/session_016QwTdAvrMgYdfJuycgobHc
…ervals

Introduces IpsetDef to carry per-ipset tuning alongside the hostname list,
replacing the bare List[str] stored in Config.ipsets.  Config format remains
fully backwards-compatible: plain-list ipset entries are unchanged; the
extended dict form is opt-in.

New fields (all optional, all env-var-defaulted):

  dns_queries      — getaddrinfo calls per hostname per refresh cycle.
                     Values > 1 accelerate discovery of CDN/anycast pools
                     that return a different IP on each query (e.g.
                     vscode.download.prss.microsoft.com).
                     Global default: IPSET_DNS_QUERIES env var (default 1).

  stale_ttl        — seconds a discovered IP stays in the live set after
                     it stops appearing in DNS.  Longer values suit CDN
                     pools with large, slowly-rotating IP lists.
                     Global default: IPSET_STALE_TTL (300 s).

  refresh_interval — seconds between refresh cycles for this ipset.
                     CDN sets can refresh every 5-10 s while stable
                     single-IP sets stay at 60 s, without forcing a
                     global speed-up that hammers DNS for all ipsets.
                     Global default: IPSET_REFRESH_INTERVAL (60 s).

Implementation:
- IpsetManager gains refresh_due() (refreshes only overdue ipsets) and
  time_until_next_refresh() (precise sleep duration for the loop).
- _refresh_loop() replaced fixed-interval sleep with a precision-wake
  approach: sleep = time_until_next_refresh(), wake = refresh_due().
- _refresh_one() calls _resolve_hostname_multi(host, dns_queries) and
  uses ipset_def.stale_ttl for seen-set per-entry timeouts.
- All dict(old_cfg.ipsets) round-trips in socket handlers replaced with
  {n: idef.to_dict() ...} to serialize IpsetDef back to JSON-ready form.

Example config for a CDN hostname:
  "vscode-cdn": {
    "hostnames": ["vscode.download.prss.microsoft.com"],
    "dns_queries": 10,
    "stale_ttl": 3600,
    "refresh_interval": 10
  }

https://claude.ai/code/session_016QwTdAvrMgYdfJuycgobHc
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.

2 participants