ngit-grasp/d6ee-defensive-relay-features.md

Defensive Relay Features

ID: d6ee

⚠️ COORDINATION REQUIRED: This issue is part of the Administrator Observability and Management Strategy (issue ec1f). Before starting work: Check issue ec1f for current phase, dependencies, and coordination requirements. This issue contributes to Phase 4 (Enforcement). Only Phase 1 (config-based protection) is currently approved.

Issue Summary

Implement defensive rate limiting and resource controls to protect the relay from abuse. Focus on per-connection and per-IP rate limits using rust-nostr relay-builder capabilities and custom extensions where needed.

UPDATE 2026-01-13: Need to review and validate the current approach before implementing. Concerns about over-engineering and maintaining code that relay-builder should handle. Per-IP throttling may be more appropriate for git data fetching (separate issue) than general relay operations.

✅ DECISION: PHASE 1 ONLY (Config-Based Protection)

Status: APPROVED - Implement Phase 1, defer Phase 2/3
Priority: High (addresses critical DoS vulnerability)

Review Complete - Key Decisions

✅ Review Tasks Completed (2026-01-14):

• Task 1: Validated current state - only database + WritePolicy configured
• Task 2: Validated docs - 96.7% accurate, one minor error found
• Task 3: Gap analysis - total connection limit is critical missing piece
• Task 4: Config-only path recommended (70% protection, 15% effort)
• Task 5: Upstream contribution not recommended (no demand, specialized use case)

Decision Rationale:

1. Config-only is sufficient - Total connection limit addresses primary DoS vector
2. Defer per-IP enforcement - Implement only if abuse detected in production
3. Git throttling separate - Git endpoint needs separate IP-based throttling (analyze separately)
4. Upstream later - If Phase 2 needed, prefer PR to rust-nostr/relay-builder

Phase 1: Explicit Rate Limits & Connection Limit (IMPLEMENT NOW)

Scope:

1. Make RateLimit explicit in code (currently using defaults)
   • max_reqs: 500 (max subscriptions per connection)
   • notes_per_minute: 60 (max events per minute per connection)
2. Add max_connections configuration (default: 500)
3. Update all 4 config locations (CRITICAL - see AGENTS.md)
4. Fix documentation error (filter limit 5000→500)
5. Document Phase 2 deferral decision in defensive-measures.md

Files to modify:

• src/nostr/builder.rs - Add explicit RateLimit and max_connections
• src/config.rs - Add max_connections field
• .env.example - Add NGIT_MAX_CONNECTIONS
• nix/module.nix - Add maxConnections option
• docs/reference/configuration.md - Document new option
• docs/explanation/defensive-measures.md - Fix filter limit error + document Phase 2 deferral

Effort: 2-3 hours

Acceptance Criteria:

• RateLimit explicitly configured in builder.rs with comments
• max_connections configurable via NGIT_MAX_CONNECTIONS (default: 500)
• All 4 config locations synced
• Documentation error fixed (filter limit 5000→500)
• Documentation updated with Phase 2 deferral decision
• Integration test: Connection rejected after reaching total limit
• All tests passing
• Changes committed

Phase 2 & 3: Per-IP Enforcement (DEFERRED - Future Work)

Status: Deferred until abuse detected in production

When to implement:

• IF ConnectionTracker shows IPs exceeding 10 connections
• IF event spam patterns emerge from single IPs
• Monitor for 2-4 weeks after Phase 1 deployment

Preferred Implementation Path:

1. First choice: Contribute to rust-nostr/relay-builder as PR

   • Propose IP-based rate limiting as optional feature
   • Let upstream maintain the code
   • Benefits entire Nostr ecosystem
   • See: docs/explanation/defensive-analysis-of-other-relays.md for research

2. Fallback: Implement in ngit-grasp if upstream not interested

   • Use original Phase 2 & 3 plan (see below)
   • Per-IP connection enforcement (4-6 hours)
   • Per-IP event rate limiting (6-8 hours)

Implementation details preserved below for future reference.

Git Endpoint IP-Based Throttling (MOVED TO SEPARATE ISSUE)

Status: ✅ Spun out to issue ff38

Rationale:

• Git data fetching has different threat model than Nostr relay
• HTTP endpoints, not WebSocket connections
• Different attack vectors (bandwidth, CPU for pack generation)
• Should NOT interact with relay code

See: Issue ff38 (git-endpoint-ip-throttling) for full analysis and implementation plan

---

PREVIOUS IMPLEMENTATION PLAN (FOR REFERENCE ONLY)

Findings

rust-nostr relay-builder Capabilities

Built-in Features (✅ Available):

• Per-connection subscription limits (RateLimit.max_reqs, default: 500)
• Per-connection event rate limits (RateLimit.notes_per_minute, default: 60)
• Total connection limit (max_connections(n))
• Subscription ID length limit (max_subid_length, default: 250)
• Filter result limits (max_filter_limit, default: 5000)

Custom ngit-grasp Features (✅ Implemented):

• Per-IP connection monitoring (ConnectionTracker in src/metrics/connection.rs)
  • Status: Monitoring only, does NOT enforce limits
  • Tracks connections per IP, flags abusers at threshold (default: 10)
  • Privacy-preserving: IP addresses never exposed in Prometheus metrics
• Event blacklist (block all events from specific authors)
• Repository blacklist (block specific repositories/developers)
• WritePolicy plugin system (access to IP address for custom validation)

Not Supported:

• Per-IP subscription limits (tracked per connection, not per IP)
• Per-IP event rate limits (tracked per connection, not per IP)

Architecture Decision

Rate Limiting Strategy

Per-Connection Limits (relay-builder):

• Use built-in RateLimit configuration
• Default values: 500 concurrent subscriptions, 60 events/min
• Prevents single connection from overwhelming relay

Per-IP Limits (custom implementation):

• Connection enforcement: Extend ConnectionTracker to reject connections
• Event rate limiting: Add IpRateLimiter to Nip34WritePolicy
• Hardcoded sensible defaults (configurable in future if needed)

Privacy Model:

• IP addresses tracked internally, NEVER exposed in Prometheus metrics
• Only aggregate counts exposed (total rate limited, flagged IPs)
• Rate limit events logged with IP for operator debugging

Metrics Changes: OUT OF SCOPE

Decision: Do NOT modify metrics in this issue. Current metrics implementation needs architectural review.

Reasoning:

• Current metrics use "abuse detection" threshold model
• New rate limiting uses enforcement model
• These should be aligned but require broader metrics redesign
• Deferring to separate issue to avoid scope creep

Future work:

• Create separate issue for metrics architectural review
• Align metrics with policy enforcement triggers
• Add rate limiting specific metrics (events rejected, IPs in cooldown)

Implementation Plan

Phase 1: Configure Built-in Rate Limits ⚡ (Quick Win)

Estimated Time: 1-2 hours
Can run in parallel: No (sequential)

Scope:

1. Configure RateLimit in relay builder with defaults (500 subs, 60 events/min)
2. Add total connection limit (500 connections, configurable)

Files to modify:

• src/nostr/builder.rs - Add RateLimit and max_connections to LocalRelayBuilder
• src/config.rs - Add max_connections config field
• .env.example - Add NGIT_MAX_CONNECTIONS example
• nix/module.nix - Add maxConnections NixOS option
• docs/reference/configuration.md - Document new option

Acceptance Criteria:

• ✅ Relay configured with RateLimit (500 subs, 60 events/min)
• ✅ Total connection limit configurable via NGIT_MAX_CONNECTIONS (default: 500)
• ✅ Integration test: Connection rejected after reaching total limit
• ✅ All tests passing
• ✅ Changes committed with message: "Add per-connection rate limits and total connection limit"
• ✅ Documentation updated in defensive-measures.md

Configuration sync (CRITICAL):

• Update all FOUR locations: src/config.rs, docs/reference/configuration.md, nix/module.nix, .env.example

---

Phase 2: Per-IP Connection Enforcement

Estimated Time: 4-6 hours
Can run in parallel: No (depends on Phase 1)

Scope:

1. Extend ConnectionTracker to enforce per-IP connection limit (hardcoded: 10)
2. Add actix-web middleware to check limit before WebSocket upgrade
3. Return 429 Too Many Requests with retry-after header

Files to create/modify:

• src/metrics/connection.rs - Modify on_connect() to return Result, check limit
• src/middleware/ip_limits.rs - NEW: Actix middleware for connection enforcement
• src/middleware/mod.rs - NEW: Module exports
• src/lib.rs - Export middleware module
• src/main.rs - Add middleware to actix-web app

Implementation Details:

ConnectionTracker changes:

pub struct ConnectionTracker {
    max_per_ip: u32, // NEW: hardcoded to 10
    // ... existing fields
}

pub fn on_connect(&self, ip: IpAddr) -> Result<(), String> {
    // Check count BEFORE incrementing
    let current = self.connections.get(&ip).map(|i| i.count).unwrap_or(0);
    if current >= self.max_per_ip {
        return Err(format!("Connection limit ({}) exceeded", self.max_per_ip));
    }
    // Existing increment logic...
    Ok(())
}

Actix middleware approach:

// Middleware wraps relay handler
// Checks ConnectionTracker.on_connect() before WebSocket upgrade
// Returns 429 if limit exceeded
// Allows connection if under limit

Acceptance Criteria:

• ✅ ConnectionTracker enforces 10 connections per IP (hardcoded)
• ✅ 11th connection from same IP returns 429 Too Many Requests
• ✅ Error logged with IP address for operator debugging
• ✅ Integration test: Multiple connections from same IP rejected after limit
• ✅ Integration test: Connections from different IPs allowed
• ✅ All tests passing (including Phase 1 tests)
• ✅ Changes committed with message: "Enforce per-IP connection limit (max 10)"
• ✅ Documentation updated in defensive-measures.md

---

Phase 3: Per-IP Event Rate Limiting

Estimated Time: 6-8 hours
Can run in parallel: No (depends on Phase 1 & 2)

Scope:

1. Implement token bucket rate limiter
2. Integrate with Nip34WritePolicy to check IP rate before event validation
3. Hardcoded limit: 100 events per minute per IP

Files to create/modify:

• src/nostr/ip_rate_limiter.rs - NEW: Token bucket implementation
• src/nostr/builder.rs - Add IpRateLimiter to Nip34WritePolicy
• src/nostr/mod.rs - Export ip_rate_limiter module

Implementation Details:

Token Bucket Algorithm:

pub struct IpRateLimiter {
    limiters: DashMap<IpAddr, TokenBucket>,
    events_per_minute: u32, // Hardcoded: 100
}

struct TokenBucket {
    tokens: f64,              // Current tokens (0.0 to events_per_minute)
    last_refill: Instant,     // Last refill time
    capacity: f64,            // Max tokens (events_per_minute)
    refill_rate: f64,         // Tokens per second (events_per_minute / 60.0)
}

impl IpRateLimiter {
    pub fn check_and_consume(&self, ip: IpAddr) -> bool {
        // Refill tokens based on time elapsed
        // If tokens >= 1.0, consume 1 token and return true
        // Else return false (rate limited)
    }
}

Nip34WritePolicy integration:

impl WritePolicy for Nip34WritePolicy {
    fn admit_event(&self, event: &Event, addr: &SocketAddr) -> BoxedFuture<WritePolicyResult> {
        // Check IP rate limit FIRST (before blacklist checks)
        if !self.ip_rate_limiter.check_and_consume(addr.ip()) {
            tracing::warn!(ip = %addr.ip(), "Event rejected: IP rate limit exceeded");
            return WritePolicyResult::reject("rate limited: too many events from your IP");
        }
        
        // Existing validation (blacklist, GRASP-01, etc.)...
    }
}

Cleanup task:

• Spawn background task to remove stale IP entries (no activity for 5 minutes)
• Prevents unbounded memory growth
• Runs every 60 seconds

Acceptance Criteria:

• ✅ IpRateLimiter implements token bucket (100 events/min per IP)
• ✅ Integrated into WritePolicy (checked before event validation)
• ✅ 101st event from same IP within 1 minute rejected
• ✅ Events from different IPs allowed
• ✅ Stale IP entries cleaned up (no memory leak)
• ✅ Unit tests: Token bucket refill logic
• ✅ Unit tests: Rate limit enforcement
• ✅ Integration test: Send >100 events/min from single IP, verify rejection
• ✅ Integration test: Events accepted after rate limit window passes
• ✅ All tests passing (including Phase 1 & 2 tests)
• ✅ Changes committed with message: "Add per-IP event rate limiting (100 events/min)"
• ✅ Documentation updated in defensive-measures.md

---

Phase 4: Documentation & Polish

Estimated Time: 2-3 hours
Can run in parallel: No (depends on all phases)

Scope:

1. Update all documentation to reflect implemented features
2. Add operational notes for relay operators
3. Update design docs with architectural decisions

Files to modify:

• docs/explanation/defensive-measures.md - Move features from "Not Implemented" to "Implemented"
• README.md - Update defensive measures section
• AGENTS.md - Add notes about new rate limiting code patterns (if needed)

Acceptance Criteria:

• ✅ defensive-measures.md accurately reflects all implemented features
• ✅ README.md defensive measures section updated
• ✅ Configuration docs show all new options
• ✅ All four config locations synced (src/config.rs, docs, nix/module.nix, .env.example)
• ✅ Changes committed with message: "docs: Update defensive measures documentation"

---

Testing Strategy

Unit Tests

• ConnectionTracker::on_connect() enforcement logic (Phase 2)
• IpRateLimiter::check_and_consume() token bucket (Phase 3)
• Token refill calculations (Phase 3)
• Config parsing for max_connections (Phase 1)

Integration Tests

• Connection rejection after total limit (Phase 1)
• Connection rejection after per-IP limit (Phase 2)
• Event rejection after per-IP rate limit (Phase 3)
• Rate limit window expiry (Phase 3)
• Multi-IP scenarios (all phases)

Manual Testing (after all phases)

• Use grasp-audit to simulate attack scenarios
• Test with multiple clients from same IP
• Test with rapid event publishing
• Verify metrics don't expose IP addresses

---

Configuration Summary

Phase 1: New Config Options

# Total connection limit (default: 500)
NGIT_MAX_CONNECTIONS=500

Phase 2 & 3: Hardcoded Values

• Per-IP connection limit: 10 connections (hardcoded in ConnectionTracker)
• Per-IP event rate limit: 100 events/min (hardcoded in IpRateLimiter)

Rationale for hardcoding:

• Proven defensive values based on common relay patterns
• Reduces configuration surface area
• Can be made configurable later if operators need flexibility
• Simpler initial implementation

---

Privacy & Logging

IP Address Handling:

• ✅ Tracked internally in DashMap for rate limiting
• ✅ Logged when limits exceeded (operator debugging)
• ❌ NEVER exposed in Prometheus metrics (only aggregate counts)
• ❌ NEVER logged in normal operation (only when rate limited)

Log Examples:

WARN: Connection rejected from 192.168.1.100: Connection limit (10) exceeded
WARN: Event rejected from 192.168.1.100: IP rate limit exceeded (100 events/min)

---

Out of Scope (Future Work)

Metrics Architectural Review (Separate Issue)

Current state:

• Metrics use "abuse detection threshold" model (monitoring only)
• New rate limiting uses "enforcement" model (reject at limit)
• These should be unified

Future work:

• Create new issue for metrics redesign
• Add enforcement-specific metrics:
  • ngit_connections_rejected_total{reason="total_limit|per_ip_limit"}
  • ngit_events_rejected_total{reason="ip_rate_limit|blacklist|..."}
  • ngit_ip_rate_limited_active (gauge of IPs currently in cooldown)
• Align with policy enforcement points
• Remove or repurpose "abuse threshold" concept

Per-IP Subscription Limits

Why out of scope:

• Requires complex middleware to intercept REQ messages
• relay-builder tracks subscriptions per connection, not per IP
• Lower priority than event rate limiting

HTTP Endpoint Protection

Why out of scope:

• Different attack surface (HTTP vs WebSocket)
• Needs separate rate limiting strategy
• Lower priority for git-focused relay

Query Filtering (QueryPolicy)

Why out of scope:

• Not a critical defensive measure for git relay
• Can be added later if abuse patterns emerge

Per-IP Git Data Throttling (Potential Separate Issue)

Why might need separate issue:

• Git data fetching (packs, objects) has different characteristics than Nostr events
• Already have domain-based throttling for outbound git fetches (ThrottleManager)
• Inbound git push/fetch might benefit from per-IP limits
• Should be scoped separately from general relay rate limiting
• TODO: Consider creating new issue for git-specific per-IP throttling

---

Risks & Mitigations

Risk 1: Legitimate users behind NAT/proxy

Scenario: Multiple clients behind single IP hit connection/rate limits

Mitigation:

• Per-IP connection limit set reasonably high (10)
• Per-IP event rate limit set reasonably high (100/min)
• Monitor logs for false positives
• Can add IP whitelist in future if needed

Risk 2: Middleware integration complexity

Scenario: Actix middleware doesn't work with relay-builder

Mitigation:

• Test middleware approach early in Phase 2
• Fallback: Log warnings but don't enforce (monitoring mode)
• Alternative: Contribute hooks to relay-builder upstream

Risk 3: Memory growth from IP tracking

Scenario: DashMap grows unbounded with unique IPs

Mitigation:

• Cleanup task removes stale entries (5+ min inactive)
• Proven pattern already used in ConnectionTracker
• Monitor memory in production

---

Success Criteria

How we'll know this is working:

1. Protection active:

   • 11th connection from same IP rejected (Phase 2)
   • 101st event/min from same IP rejected (Phase 3)
   • Logs show IP addresses when limits hit

2. No false positives:

   • Legitimate users can connect and publish normally
   • Sync from other relays works without rate limiting issues
   • No user complaints about false rejections

3. Resource usage stable:

   • No memory leaks from IP tracking
   • Cleanup task removes stale entries
   • CPU usage reasonable with token bucket calculations

4. Privacy maintained:

   • IP addresses NEVER in Prometheus metrics
   • Only aggregate counts exposed
   • Metrics requests don't expose tracked IPs

---

Progress Tracking

• Phase 1: Configure built-in rate limits (1-2h)
• Phase 2: Per-IP connection enforcement (4-6h)
• Phase 3: Per-IP event rate limiting (6-8h)
• Phase 4: Documentation & polish (2-3h)

Total estimated time: 13-19 hours

---

Progress

2026-01-14 [Session 14:30] - REVIEW COMPLETE, PHASE 1 APPROVED

• Status Change: Review complete, proceeding with Phase 1 only
• Completed: All 5 review tasks using subagents
• Decision: Implement Phase 1 (config-only) with explicit rate limits
• Deferred: Phase 2 & 3 (per-IP enforcement) until abuse detected in production
• Separate concern: Git endpoint IP-based throttling (analyze separately)

Review Findings:

• Task 1: Current config uses only database + WritePolicy, all else is defaults
• Task 2: Documentation 96.7% accurate (1 minor error: filter limit 5000→500)
• Task 3: Critical gap is total connection limit (DoS vulnerability)
• Task 4: Config-only approach gives 70% protection for 15% effort
• Task 5: Upstream contribution not recommended (specialized use case, no demand)

Phase 1 Scope (APPROVED):

• Add explicit RateLimit configuration (make defaults visible in code)
• Add max_connections config option (default: 500)
• Update all 4 config locations (src/config.rs, docs, nix/module.nix, .env.example)
• Fix documentation error (filter limit)
• Effort: 2-3 hours

Phase 2 & 3 Deferred (Future Work if Abuse Detected):

• Per-IP connection enforcement (10 per IP)
• Per-IP event rate limiting (100 events/min per IP)
• Preferred approach: Contribute to rust-nostr/relay-builder as PR
• Fallback: Implement in ngit-grasp if upstream not interested
• Document implementation plan for future reference

Git Endpoint Throttling (Separate Analysis Needed):

• Git data fetching has different threat model than Nostr relay
• Needs IP-based throttling with sensible limits
• Should NOT interact with relay code
• Next: Analyze requirements with architect subagent

2026-01-13 [Session 21:15] - IMPLEMENTATION PAUSED

• Status Change: Paused implementation for validation review
• Concern: May be over-engineering with custom per-IP rate limiting
• Concern: Should relay-builder handle IP-based throttling, not ngit-grasp?
• Concern: Per-IP throttling might be more relevant for git data fetching (separate issue)
• Added: Review tasks section to validate current state before proceeding
• Decision: Complete review tasks before any implementation
• Next: Work through review tasks to determine simplest path forward

Key Questions:

• Is per-connection limiting (already in relay-builder) sufficient?
• Should we contribute IP rate limiting to upstream relay-builder?
• Should git data fetching have separate per-IP throttling issue?
• What's the minimal viable protection (prefer config over code)?

2026-01-13 [Session 20:43] - RESEARCH COMPLETED

• Completed: Comprehensive research on defensive features in major Nostr relays (strfry, nostr-rs-relay, khatru)
• Completed: Analysis of ngit-grasp's current defensive capabilities
• Completed: Research on Rust rate limiting ecosystem (governor crate)
• Created: docs/explanation/defensive-analysis-of-other-relays.md with detailed findings
• Decision: The existing implementation plan is well-aligned with industry best practices
• Next: Proceed with Phase 1 implementation (configure built-in rate limits) SUPERSEDED - see above

Key Findings:

• Most relays have permissive defaults; khatru is most opinionated (2 events per 3min/IP)
• Governor crate is industry standard for Rust rate limiting (GCRA algorithm)
• ngit-grasp already has good infrastructure (ConnectionTracker, ThrottleManager pattern)
• Proposed defaults are well-balanced compared to other relays
• However: Need to validate if this complexity is necessary for our use case

References

• rust-nostr relay-builder: nostr-relay-builder v0.44.0 (git rev 4767ad13)
• Token bucket algorithm: https://en.wikipedia.org/wiki/Token_bucket
• Actix-web middleware: https://actix.rs/docs/middleware/
• Current implementation: src/metrics/connection.rs (ConnectionTracker)
• Design docs: docs/explanation/defensive-measures.md
• Research: docs/explanation/defensive-analysis-of-other-relays.md (comprehensive analysis)