Skip to content

Security review follow-ups: public /metrics, IP-match unification, negentropy buffer default #83

Description

@kwsantiago

Non-blocking items from the pre-v1 security review (the HIGHs and two MEDIUMs are fixed in PR #82). None are critical; grouped here for later.

LOW — /metrics is publicly scrapeable by default

server.zig getMetrics only applies ipDenied; with no ip_whitelist configured (default), operational metrics are exposed to anyone. Information disclosure.
Options: bind /metrics to loopback or a separate port, gate behind an opt-in config flag, or require auth. Document the reverse-proxy pattern either way.

LOW — IP matching is inconsistent across layers; IPv6 not prefix-matched

Two access-control layers disagree on what a stored "IP" means:

  • rate_limiter.zig IpFilter.matchesPrefix (now: exact, or explicit trailing-./: prefix, per PR Security hardening: query rate limiting, IP matching, proxy headers, negentropy auth #82).
  • management_store.zig isIpBlocked (~:317) does IPv4 dot-boundary prefix matching, and its . scan never fires for IPv6, so an IPv6 subnet cannot be blocked the way IPv4 can.
    Fix: unify both on numeric IP/CIDR parsing and matching (proper subnet semantics for v4 and v6).

MEDIUM — negentropy_max_sync_events default (1,000,000) is high for an in-memory buffer

Each negentropy session buffers up to this many (i64, id) pairs (~40 MB at 1M). PR #82 added a per-connection session cap + query rate limit, so exposure is now bounded, but the per-session default is still large.
Fix: lower the default to a saner value (e.g. 100k) after confirming it does not break legitimate large relay-to-relay syncs, or make the buffer grow lazily.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions