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
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions