# Phase 4b: Security Hardening **Architect**: Claude (Architect Agent) **Date**: 2025-11-20 **Status**: Design Complete - Clarifications Provided **Phase**: 4b (Security Hardening) **Design References**: - `/docs/reports/2025-11-20-gap-analysis-v1.0.0.md` - Gap analysis Components 4-5 - `/docs/designs/phase-4-5-critical-components.md` - Phase 4-5 overview - `/docs/architecture/security.md` - Security architecture - `/docs/roadmap/v1.0.0.md` - Phase 4 requirements (lines 189-218) - `/docs/designs/phase-4b-clarifications.md` - Implementation clarifications (2025-11-20) --- ## Purpose Implement production-grade security hardening to protect against common web vulnerabilities and ensure compliance with OAuth 2.0 and IndieAuth security best practices. This phase adds the final security layer required for v1.0.0 production readiness. **Critical Requirements**: 1. Security headers middleware (HSTS, CSP, X-Frame-Options, etc.) 2. HTTPS enforcement in production mode 3. Comprehensive security test suite 4. PII logging audit and remediation 5. Input validation verification **Success Outcome**: Application is production-ready with defense-in-depth security measures, passing all security tests, and compliant with OWASP Top 10 recommendations. --- ## Specification References **W3C IndieAuth**: - Security Considerations section (HTTPS requirement) - Token security (hashing, constant-time comparison) **OAuth 2.0 Security Best Practices** (RFC 8252): - HTTPS enforcement for non-localhost - State parameter CSRF protection - Open redirect prevention - Token theft mitigation **OWASP Top 10**: - A01:2021 Broken Access Control (token validation) - A03:2021 Injection (SQL injection, XSS) - A05:2021 Security Misconfiguration (headers, HTTPS) - A07:2021 Identification and Authentication Failures (timing attacks) **v1.0.0 Roadmap Requirements**: - Phase 4, lines 198-203: HTTPS enforcement, security headers, constant-time comparison, input sanitization, PII logging audit - Exit criteria, lines 212-218: All security tests passing, no PII in logs --- ## Design Overview Phase 4b implements four major components: 1. **Security Headers Middleware**: FastAPI middleware that adds security-related HTTP headers to all responses 2. **HTTPS Enforcement**: Production-mode middleware that redirects HTTP → HTTPS and validates TLS usage 3. **Security Test Suite**: Comprehensive tests covering timing attacks, injection prevention, XSS, open redirects, CSRF 4. **PII Logging Audit**: Review all logging statements and ensure no personally identifiable information is logged **Architecture Pattern**: Defense-in-depth with multiple layers: - Layer 1: Network (HTTPS/TLS) - Layer 2: HTTP headers (prevent common attacks) - Layer 3: Input validation (already implemented via Pydantic) - Layer 4: Token security (hashing, constant-time comparison) - Layer 5: Logging (no sensitive data exposure) **Implementation Approach**: - Middleware-based for headers and HTTPS enforcement (FastAPI middleware pattern) - Pytest-based security tests using standard pytest markers - Logging audit via grep and manual code review - Configuration-driven (production vs development mode) --- ## Component 4: Security Headers Middleware ### Purpose Add HTTP security headers to all responses to protect against clickjacking, XSS, MIME sniffing, and other client-side attacks. Headers are the first line of defense for browser-based vulnerabilities. ### Security Headers to Implement | Header | Value | Purpose | Standard | |--------|-------|---------|----------| | `X-Frame-Options` | `DENY` | Prevent clickjacking attacks | RFC 7034 | | `X-Content-Type-Options` | `nosniff` | Prevent MIME type sniffing | WHATWG | | `X-XSS-Protection` | `1; mode=block` | Enable XSS filter (legacy browsers) | Non-standard but widely supported | | `Strict-Transport-Security` | `max-age=31536000; includeSubDomains` | Force HTTPS for 1 year (production only) | RFC 6797 | | `Content-Security-Policy` | `default-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' https:; frame-ancestors 'none'` | Restrict resource loading | W3C CSP Level 3 | | `Referrer-Policy` | `strict-origin-when-cross-origin` | Control referrer information leakage | W3C Referrer Policy | | `Permissions-Policy` | `geolocation=(), microphone=(), camera=()` | Disable unnecessary browser features | W3C Permissions Policy | **Rationale for Values**: - **X-Frame-Options: DENY**: IndieAuth server should never be embedded in frames (phishing risk) - **X-Content-Type-Options: nosniff**: Force browser to respect Content-Type header (prevent XSS via misinterpreted files) - **X-XSS-Protection: 1; mode=block**: Enable legacy XSS filter for older browsers (modern browsers have CSP) - **Strict-Transport-Security**: Enforce HTTPS for 1 year including subdomains (production only, not development) - **Content-Security-Policy**: - `default-src 'self'`: Only load resources from same origin - `style-src 'self' 'unsafe-inline'`: Allow inline styles for simplicity (templates may use inline styles) - `img-src 'self' https:`: Allow images from same origin + HTTPS (for client logos from h-app) - `frame-ancestors 'none'`: Equivalent to X-Frame-Options DENY - **Referrer-Policy: strict-origin-when-cross-origin**: Send origin on cross-origin requests (not full URL, privacy) - **Permissions-Policy**: Disable geolocation, microphone, camera (not needed for auth server) ### Middleware Design **File**: `/src/gondulf/middleware/security_headers.py` **Implementation**: ```python """Security headers middleware for Gondulf IndieAuth server.""" import logging from typing import Callable from fastapi import Request, Response from starlette.middleware.base import BaseHTTPMiddleware from gondulf.config import Config logger = logging.getLogger("gondulf.middleware.security_headers") class SecurityHeadersMiddleware(BaseHTTPMiddleware): """ Add security-related HTTP headers to all responses. Headers protect against clickjacking, XSS, MIME sniffing, and other client-side attacks. HSTS is only added in production mode (non-DEBUG). References: - OWASP Secure Headers Project - Mozilla Web Security Guidelines """ def __init__(self, app, debug: bool = False): """ Initialize security headers middleware. Args: app: FastAPI application debug: If True, skip HSTS header (development mode) """ super().__init__(app) self.debug = debug async def dispatch(self, request: Request, call_next: Callable) -> Response: """ Process request and add security headers to response. Args: request: Incoming HTTP request call_next: Next middleware/handler in chain Returns: Response with security headers added """ # Process request response = await call_next(request) # Add security headers response.headers["X-Frame-Options"] = "DENY" response.headers["X-Content-Type-Options"] = "nosniff" response.headers["X-XSS-Protection"] = "1; mode=block" response.headers["Referrer-Policy"] = "strict-origin-when-cross-origin" # CSP: Allow self, inline styles (for templates), and HTTPS images (for h-app logos) response.headers["Content-Security-Policy"] = ( "default-src 'self'; " "style-src 'self' 'unsafe-inline'; " "img-src 'self' https:; " "frame-ancestors 'none'" ) # Permissions Policy: Disable unnecessary browser features response.headers["Permissions-Policy"] = ( "geolocation=(), microphone=(), camera=()" ) # HSTS: Only in production (not development) if not self.debug: response.headers["Strict-Transport-Security"] = ( "max-age=31536000; includeSubDomains" ) logger.debug("Added HSTS header (production mode)") return response ``` ### Configuration Integration **Update**: `/src/gondulf/main.py` Add middleware registration after app initialization: ```python from gondulf.middleware.security_headers import SecurityHeadersMiddleware # ... existing imports and setup ... # Add security headers middleware app.add_middleware( SecurityHeadersMiddleware, debug=Config.DEBUG ) logger.info(f"Security headers middleware registered (debug={Config.DEBUG})") ``` **Middleware Order**: Security headers should be added early in the middleware chain, but after any error-handling middleware. FastAPI applies middleware in reverse order of registration, so register security headers early. ### Testing Requirements **File**: `/tests/integration/test_security_headers.py` ```python """Integration tests for security headers middleware.""" import pytest from fastapi.testclient import TestClient from gondulf.main import app @pytest.fixture def client(): """FastAPI test client.""" return TestClient(app) class TestSecurityHeaders: """Test security headers middleware.""" def test_x_frame_options_header(self, client): """Test X-Frame-Options header is present.""" response = client.get("/health") assert "X-Frame-Options" in response.headers assert response.headers["X-Frame-Options"] == "DENY" def test_x_content_type_options_header(self, client): """Test X-Content-Type-Options header is present.""" response = client.get("/health") assert "X-Content-Type-Options" in response.headers assert response.headers["X-Content-Type-Options"] == "nosniff" def test_x_xss_protection_header(self, client): """Test X-XSS-Protection header is present.""" response = client.get("/health") assert "X-XSS-Protection" in response.headers assert response.headers["X-XSS-Protection"] == "1; mode=block" def test_csp_header(self, client): """Test Content-Security-Policy header is present and configured correctly.""" response = client.get("/health") assert "Content-Security-Policy" in response.headers csp = response.headers["Content-Security-Policy"] assert "default-src 'self'" in csp assert "style-src 'self' 'unsafe-inline'" in csp assert "img-src 'self' https:" in csp assert "frame-ancestors 'none'" in csp def test_referrer_policy_header(self, client): """Test Referrer-Policy header is present.""" response = client.get("/health") assert "Referrer-Policy" in response.headers assert response.headers["Referrer-Policy"] == "strict-origin-when-cross-origin" def test_permissions_policy_header(self, client): """Test Permissions-Policy header is present.""" response = client.get("/health") assert "Permissions-Policy" in response.headers policy = response.headers["Permissions-Policy"] assert "geolocation=()" in policy assert "microphone=()" in policy assert "camera=()" in policy def test_hsts_header_not_in_debug_mode(self, client, monkeypatch): """Test HSTS header is NOT present in debug mode.""" # This test assumes DEBUG=True in test environment # In production, DEBUG=False and HSTS should be present response = client.get("/health") # Check current mode from Config from gondulf.config import Config if Config.DEBUG: # HSTS should NOT be present in debug mode assert "Strict-Transport-Security" not in response.headers else: # HSTS should be present in production mode assert "Strict-Transport-Security" in response.headers assert "max-age=31536000" in response.headers["Strict-Transport-Security"] assert "includeSubDomains" in response.headers["Strict-Transport-Security"] def test_headers_on_all_endpoints(self, client): """Test security headers are present on all endpoints.""" endpoints = [ "/", "/health", "/.well-known/oauth-authorization-server", ] for endpoint in endpoints: response = client.get(endpoint) # All endpoints should have security headers assert "X-Frame-Options" in response.headers assert "X-Content-Type-Options" in response.headers assert "Content-Security-Policy" in response.headers def test_headers_on_error_responses(self, client): """Test security headers are present even on error responses.""" # Request non-existent endpoint (404) response = client.get("/nonexistent") assert response.status_code == 404 # Security headers should still be present assert "X-Frame-Options" in response.headers assert "X-Content-Type-Options" in response.headers ``` **Test Coverage**: - Each security header individually - Headers present on all endpoints - HSTS conditional on production mode - Headers present on error responses --- ## Component 5: HTTPS Enforcement ### Purpose Ensure all production traffic uses HTTPS (TLS) to prevent credential theft, token interception, and man-in-the-middle attacks. Allow HTTP only for localhost development. ### Design Approach **Strategy**: Configuration-driven enforcement based on `Config.DEBUG` and `Config.BASE_URL`: - **Production Mode** (`DEBUG=False`): Reject HTTP requests or redirect to HTTPS - **Development Mode** (`DEBUG=True`): Allow HTTP for localhost only - **Validation**: Check `request.url.scheme` and `request.url.hostname` **Implementation**: Middleware that runs before request processing. ### Middleware Design **File**: `/src/gondulf/middleware/https_enforcement.py` ```python """HTTPS enforcement middleware for Gondulf IndieAuth server.""" import logging from typing import Callable from fastapi import Request, Response from starlette.middleware.base import BaseHTTPMiddleware from starlette.responses import RedirectResponse from gondulf.config import Config logger = logging.getLogger("gondulf.middleware.https_enforcement") def is_https_request(request: Request) -> bool: """ Check if request is HTTPS, considering reverse proxy headers. Args: request: Incoming HTTP request Returns: True if HTTPS, False otherwise """ # Direct HTTPS if request.url.scheme == "https": return True # Behind proxy - check forwarded header # Only trust this header in production with TRUST_PROXY=true if Config.TRUST_PROXY: forwarded_proto = request.headers.get("X-Forwarded-Proto", "").lower() return forwarded_proto == "https" return False class HTTPSEnforcementMiddleware(BaseHTTPMiddleware): """ Enforce HTTPS in production mode. In production (DEBUG=False), reject or redirect HTTP requests to HTTPS. In development (DEBUG=True), allow HTTP for localhost only. Supports reverse proxy deployments via X-Forwarded-Proto header when Config.TRUST_PROXY is enabled. References: - OAuth 2.0 Security Best Practices: HTTPS required - W3C IndieAuth: TLS required for production - Clarifications: See /docs/designs/phase-4b-clarifications.md section 2 """ def __init__(self, app, debug: bool = False, redirect: bool = True): """ Initialize HTTPS enforcement middleware. Args: app: FastAPI application debug: If True, allow HTTP for localhost (development mode) redirect: If True, redirect HTTP to HTTPS. If False, return 400. """ super().__init__(app) self.debug = debug self.redirect = redirect async def dispatch(self, request: Request, call_next: Callable) -> Response: """ Process request and enforce HTTPS if in production mode. Args: request: Incoming HTTP request call_next: Next middleware/handler in chain Returns: Response (redirect to HTTPS, error, or normal response) """ hostname = request.url.hostname or "" # Debug mode: Allow HTTP for localhost only if self.debug: if not is_https_request(request) and hostname not in ["localhost", "127.0.0.1", "::1"]: logger.warning( f"HTTP request to non-localhost in debug mode: {hostname}" ) # Allow but log warning (for development on local networks) # Continue processing return await call_next(request) # Production mode: Enforce HTTPS if not is_https_request(request): logger.warning( f"HTTP request blocked in production mode: " f"{request.method} {request.url}" ) if self.redirect: # Redirect HTTP → HTTPS https_url = request.url.replace(scheme="https") logger.info(f"Redirecting to HTTPS: {https_url}") return RedirectResponse(url=str(https_url), status_code=301) else: # Return 400 Bad Request (strict mode) from fastapi.responses import JSONResponse return JSONResponse( status_code=400, content={ "error": "invalid_request", "error_description": "HTTPS is required" } ) # HTTPS or allowed HTTP: Continue processing return await call_next(request) ``` ### Configuration Options **Update `/src/gondulf/config.py`** to add security configuration: ```python class Config: """Application configuration.""" # Existing configuration... # Security configuration (Phase 4b) HTTPS_REDIRECT: bool = Field( default=True, description="Redirect HTTP requests to HTTPS in production" ) TRUST_PROXY: bool = Field( default=False, description="Trust X-Forwarded-* headers from reverse proxy" ) SECURE_COOKIES: bool = Field( default=True, description="Set secure flag on cookies (requires HTTPS)" ) @validator("HTTPS_REDIRECT") def validate_https_redirect(cls, v, values): """Disable HTTPS redirect in development.""" if values.get("ENV") == "development": return False return v ``` **Environment Variables**: - `GONDULF_HTTPS_REDIRECT=true` - Enable HTTPS redirect (default: true) - `GONDULF_TRUST_PROXY=true` - Trust X-Forwarded-Proto header (default: false) - `GONDULF_SECURE_COOKIES=true` - Set secure flag on cookies (default: true) **Clarification**: See `/docs/designs/phase-4b-clarifications.md` section 2 for reverse proxy support details. **Integration**: Add to `main.py`: ```python from gondulf.middleware.https_enforcement import HTTPSEnforcementMiddleware # Add HTTPS enforcement middleware (before security headers) app.add_middleware( HTTPSEnforcementMiddleware, debug=Config.DEBUG, redirect=Config.HTTPS_REDIRECT ) logger.info(f"HTTPS enforcement middleware registered (debug={Config.DEBUG})") ``` **Middleware Order**: 1. HTTPS enforcement (first, redirects before processing) 2. Security headers (second, adds headers to all responses) 3. Application routers (last) ### Testing Requirements **File**: `/tests/integration/test_https_enforcement.py` ```python """Integration tests for HTTPS enforcement middleware.""" import pytest from fastapi.testclient import TestClient from gondulf.main import app @pytest.fixture def client(): """FastAPI test client.""" return TestClient(app) class TestHTTPSEnforcement: """Test HTTPS enforcement middleware.""" def test_https_allowed_in_production(self, client, monkeypatch): """Test HTTPS requests are allowed in production mode.""" # Simulate production mode from gondulf.config import Config monkeypatch.setattr(Config, "DEBUG", False) # HTTPS request should succeed # Note: TestClient uses http by default, so this test is illustrative # In real production, requests come from a reverse proxy (nginx) with HTTPS response = client.get("/health") assert response.status_code == 200 def test_http_localhost_allowed_in_debug(self, client, monkeypatch): """Test HTTP to localhost is allowed in debug mode.""" from gondulf.config import Config monkeypatch.setattr(Config, "DEBUG", True) # HTTP to localhost should succeed in debug mode response = client.get("http://localhost:8000/health") assert response.status_code == 200 def test_https_always_allowed(self, client): """Test HTTPS requests are always allowed regardless of mode.""" # HTTPS should work in both debug and production response = client.get("/health") # TestClient doesn't enforce HTTPS, but middleware should allow it assert response.status_code == 200 ``` **Note**: Full HTTPS testing requires integration tests with a real server (uvicorn with TLS). The TestClient doesn't enforce scheme validation, so these tests are illustrative. Real-world testing should be done in Phase 5 deployment testing. ### Production Deployment Notes **Reverse Proxy Configuration** (nginx example): ```nginx server { listen 80; server_name auth.example.com; # Redirect all HTTP to HTTPS return 301 https://$server_name$request_uri; } server { listen 443 ssl http2; server_name auth.example.com; ssl_certificate /etc/letsencrypt/live/auth.example.com/fullchain.pem; ssl_certificate_key /etc/letsencrypt/live/auth.example.com/privkey.pem; # Proxy to Gondulf location / { proxy_pass http://localhost:8000; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; proxy_set_header X-Forwarded-Proto $scheme; proxy_set_header Host $host; } } ``` **Important**: The reverse proxy handles TLS termination. Gondulf sees HTTP from nginx but with `X-Forwarded-Proto: https` header. FastAPI's middleware respects this header. --- ## Component 6: Security Test Suite ### Purpose Comprehensive security tests to verify protection against common vulnerabilities: timing attacks, SQL injection, XSS, open redirects, CSRF. Tests demonstrate compliance with OWASP Top 10 and OAuth 2.0 security best practices. ### Test Organization **Directory**: `/tests/security/` **Test Files**: - `test_timing_attacks.py` - Timing attack resistance (token comparison) - `test_sql_injection.py` - SQL injection prevention - `test_xss_prevention.py` - Cross-site scripting prevention - `test_open_redirect.py` - Open redirect prevention - `test_csrf_protection.py` - CSRF protection (state parameter) - `test_input_validation.py` - Input validation edge cases **Pytest Marker**: Add `@pytest.mark.security` to all security tests for easy execution: ```bash # Run only security tests pytest -m security # Run security tests with coverage pytest -m security --cov=src/gondulf --cov-report=html ``` **Marker Registration**: See `/docs/designs/phase-4b-clarifications.md` section 7. Register the `security` marker in `pytest.ini` or `pyproject.toml` to prevent warnings and enable targeted test runs. ### Test 1: Timing Attack Resistance **File**: `/tests/security/test_timing_attacks.py` **Purpose**: Verify that token comparison uses constant-time algorithms to prevent timing side-channel attacks. ```python """Security tests for timing attack resistance.""" import secrets import time from statistics import mean, stdev import pytest from gondulf.services.token_service import TokenService @pytest.mark.security class TestTimingAttackResistance: """Test timing attack resistance in token validation.""" def test_token_verification_constant_time(self, db_session): """ Test that token verification takes similar time for valid and invalid tokens. Timing attacks exploit differences in processing time to guess secrets. This test verifies that token verification uses constant-time comparison. """ token_service = TokenService(db_session) # Generate valid token me = "https://user.example.com" client_id = "https://client.example.com" token = token_service.generate_access_token( me=me, client_id=client_id, scope="" ) # Measure time for valid token (hits database, passes validation) valid_times = [] for _ in range(100): start = time.perf_counter() result = token_service.verify_access_token(token) end = time.perf_counter() valid_times.append(end - start) assert result is not None # Valid token # Measure time for invalid token (misses database, fails validation) invalid_token = secrets.token_urlsafe(32) invalid_times = [] for _ in range(100): start = time.perf_counter() result = token_service.verify_access_token(invalid_token) end = time.perf_counter() invalid_times.append(end - start) assert result is None # Invalid token # Statistical analysis: times should be similar valid_mean = mean(valid_times) invalid_mean = mean(invalid_times) valid_stdev = stdev(valid_times) invalid_stdev = stdev(invalid_times) # Difference in means should be small relative to standard deviations # Allow 3x stdev difference (99.7% confidence interval) max_allowed_diff = 3 * max(valid_stdev, invalid_stdev) actual_diff = abs(valid_mean - invalid_mean) assert actual_diff < max_allowed_diff, ( f"Timing difference suggests timing attack vulnerability: " f"valid={valid_mean:.6f}s, invalid={invalid_mean:.6f}s, " f"diff={actual_diff:.6f}s, max_allowed={max_allowed_diff:.6f}s" ) def test_hash_comparison_uses_constant_time(self): """ Test that hash comparison uses secrets.compare_digest. This is a code inspection test (verified by reading token_service.py). """ import inspect from gondulf.services.token_service import TokenService source = inspect.getsource(TokenService.verify_access_token) # Verify that secrets.compare_digest is used (or equivalent) # OR that comparison happens in SQL (which is also constant-time) assert "compare_digest" in source or "SELECT" in source, ( "Token verification should use constant-time comparison or SQL lookup" ) def test_authorization_code_verification_constant_time(self, code_store): """ Test that authorization code verification is constant-time. Similar to token verification test. """ # Store valid code code = secrets.token_urlsafe(32) code_data = { "client_id": "https://client.example.com", "redirect_uri": "https://client.example.com/callback", "me": "https://user.example.com", "state": "test-state" } code_store.store_authorization_code(code, code_data) # Measure valid code lookup valid_times = [] for _ in range(100): start = time.perf_counter() result = code_store.get_authorization_code(code) end = time.perf_counter() valid_times.append(end - start) # Measure invalid code lookup invalid_code = secrets.token_urlsafe(32) invalid_times = [] for _ in range(100): start = time.perf_counter() result = code_store.get_authorization_code(invalid_code) end = time.perf_counter() invalid_times.append(end - start) # Times should be similar valid_mean = mean(valid_times) invalid_mean = mean(invalid_times) # In-memory dict lookup is very fast, so allow tighter bounds # But still should be constant-time assert abs(valid_mean - invalid_mean) < 0.001, ( "Authorization code lookup timing difference detected" ) ``` **Acceptance Criteria**: - Token verification timing difference < 3x standard deviation - Hash comparison uses `secrets.compare_digest` or SQL lookup - Authorization code lookup is constant-time **Implementation Note**: See `/docs/designs/phase-4b-clarifications.md` section 4 for handling timing test flakiness in CI environments (relaxed thresholds, increased samples, skip markers). ### Test 2: SQL Injection Prevention **File**: `/tests/security/test_sql_injection.py` **Purpose**: Verify that all database queries use parameterized queries and are not vulnerable to SQL injection. ```python """Security tests for SQL injection prevention.""" import pytest from sqlalchemy.exc import SQLAlchemyError from gondulf.services.token_service import TokenService from gondulf.services.domain_verification import DomainVerificationService @pytest.mark.security class TestSQLInjectionPrevention: """Test SQL injection prevention in database queries.""" def test_token_service_sql_injection_in_me(self, db_session): """Test token service prevents SQL injection in 'me' parameter.""" token_service = TokenService(db_session) # Attempt SQL injection via 'me' parameter malicious_me = "https://user.example.com'; DROP TABLE tokens; --" client_id = "https://client.example.com" # Should not raise exception, should treat as literal string token = token_service.generate_access_token( me=malicious_me, client_id=client_id, scope="" ) assert token is not None # Verify token was stored safely (not executed as SQL) result = token_service.verify_access_token(token) assert result is not None assert result["me"] == malicious_me # Stored as literal string def test_domain_service_sql_injection_in_domain(self, db_session): """Test domain service prevents SQL injection in domain parameter.""" domain_service = DomainVerificationService(db_session) # Attempt SQL injection via domain parameter malicious_domain = "example.com'; DROP TABLE domains; --" # Should not raise exception # Note: This will fail validation (not a valid domain), but should not execute SQL try: domain_service.verify_domain_ownership(malicious_domain) except ValueError: # Expected: invalid domain format pass # Verify tables still exist (not dropped by injection) result = db_session.execute("SELECT COUNT(*) FROM tokens") assert result.scalar() >= 0 # Table exists def test_token_lookup_sql_injection(self, db_session): """Test token lookup prevents SQL injection in token parameter.""" token_service = TokenService(db_session) # Attempt SQL injection via token parameter malicious_token = "' OR '1'='1" # Should return None (not found), not execute malicious SQL result = token_service.verify_access_token(malicious_token) assert result is None def test_parameterized_queries_used(self): """ Test that all database queries use parameterized queries. This is a code inspection test. """ import ast import inspect from pathlib import Path # Check all service files services_dir = Path("src/gondulf/services") violations = [] for service_file in services_dir.glob("*.py"): source = service_file.read_text() # Check for dangerous patterns (string formatting in SQL) if 'f"SELECT' in source or "f'SELECT" in source: violations.append(f"{service_file}: f-string in SQL query") if '.format(' in source and 'SELECT' in source: violations.append(f"{service_file}: .format() in SQL query") if 'f"INSERT' in source or "f'INSERT" in source: violations.append(f"{service_file}: f-string in INSERT query") assert not violations, ( "SQL injection vulnerabilities detected:\n" + "\n".join(violations) ) ``` **Acceptance Criteria**: - Malicious SQL strings treated as literal values - No SQL execution from injected parameters - All queries use parameterized format (behavioral testing, not source inspection) **Implementation Note**: See `/docs/designs/phase-4b-clarifications.md` section 5. Tests should verify actual behavior (injection attempts fail safely) rather than inspecting source code patterns. ### Test 3: XSS Prevention **File**: `/tests/security/test_xss_prevention.py` **Purpose**: Verify that user input is properly escaped in HTML templates to prevent cross-site scripting. ```python """Security tests for XSS prevention.""" import pytest from fastapi.testclient import TestClient from gondulf.main import app @pytest.mark.security class TestXSSPrevention: """Test XSS prevention in HTML templates.""" def test_client_name_xss_escaped(self, client): """Test that client name is HTML-escaped in consent screen.""" # Mock h-app parser to return malicious client name malicious_name = '' # Request authorization with malicious client metadata # (This requires mocking the h-app parser) # For now, test that templates auto-escape by inspecting template from jinja2 import Environment env = Environment(autoescape=True) template_source = '{{ client_name }}' template = env.from_string(template_source) rendered = template.render(client_name=malicious_name) # Should be escaped assert '") assert not is_valid def test_url_validation_rejects_file_protocol(self): """Test that file: URLs are rejected.""" is_valid, _ = validate_url("file:///etc/passwd") assert not is_valid def test_url_validation_handles_unicode(self): """Test that URL validation handles Unicode safely.""" # IDN homograph attack is_valid, _ = validate_url("https://аpple.com") # Cyrillic 'а' # Should either reject or normalize to punycode # For v1.0.0, basic validation (may accept) def test_url_validation_handles_very_long_urls(self): """Test that URL validation handles very long URLs.""" long_url = "https://example.com/" + "a" * 10000 # Should handle without crashing (may reject) try: is_valid, _ = validate_url(long_url) except Exception as e: pytest.fail(f"URL validation crashed on long URL: {e}") def test_email_validation_rejects_injection(self): """Test that email validation rejects injection attempts.""" malicious_emails = [ "user@example.com\nBcc: attacker@evil.com", "user@example.com\r\nSubject: Injected", "user@example.com", ] for email in malicious_emails: is_valid = validate_email_format(email) assert not is_valid, f"Email injection allowed: {email}" def test_null_byte_injection_rejected(self): """Test that null byte injection is rejected.""" malicious_url = "https://example.com\x00.attacker.com" is_valid, _ = validate_url(malicious_url) assert not is_valid ``` **Acceptance Criteria**: - Dangerous URL protocols rejected (javascript:, data:, file:) - Email injection attempts rejected - Null byte injection rejected - Very long inputs handled safely --- ## Component 7: PII Logging Audit ### Purpose Review all logging statements throughout the codebase to ensure no personally identifiable information (PII) is logged. PII includes email addresses, tokens, codes, and IP addresses. ### Audit Process **Step 1: Grep for Logging Statements** ```bash # Find all logging statements grep -rn "logger\." src/gondulf/ > /tmp/logging_audit.txt # Find potential PII in logs grep -rn "logger.*email" src/gondulf/ grep -rn "logger.*token" src/gondulf/ grep -rn "logger.*code" src/gondulf/ grep -rn "logger.*password" src/gondulf/ ``` **Step 2: Manual Review** Review each logging statement for: - Email addresses (PII) - Full tokens or codes (security) - Passwords (security) - IP addresses (PII in some jurisdictions) **Step 3: Remediation Patterns** **SAFE Logging Patterns**: ```python # GOOD: Domain only (public information) logger.info(f"Authorization granted for {domain} to {client_id}") # GOOD: Token prefix for correlation (first 8 chars) logger.debug(f"Token generated: {token[:8]}...") # GOOD: Error without sensitive data logger.error(f"Email send failed for domain {domain}") # GOOD: Count/aggregate data logger.info(f"Cleaned up {count} expired tokens") ``` **UNSAFE Logging Patterns** (to fix): ```python # BAD: Full email address logger.info(f"Verification sent to {email}") # ← REMOVE email # BAD: Full token logger.debug(f"Token: {token}") # ← Use token[:8] only # BAD: Full authorization code logger.info(f"Code generated: {code}") # ← Use code[:8] only # BAD: IP address logger.info(f"Request from {request.client.host}") # ← Remove IP ``` **Step 4: Code Changes** Create a checklist of files to update: ```markdown ## PII Logging Audit Checklist - [ ] `/src/gondulf/services/email.py` - Remove email addresses from logs - [ ] `/src/gondulf/services/token_service.py` - Use token[:8] prefix only - [ ] `/src/gondulf/routers/authorization.py` - No PII in logs - [ ] `/src/gondulf/routers/token.py` - No PII in logs - [ ] `/src/gondulf/routers/verification.py` - Remove email from logs - [ ] `/src/gondulf/services/domain_verification.py` - Remove email from logs - [ ] `/src/gondulf/main.py` - Review startup logs ``` ### Testing Requirements **File**: `/tests/security/test_pii_logging.py` ```python """Security tests for PII in logging.""" import logging import re import pytest @pytest.mark.security class TestPIILogging: """Test that no PII is logged.""" def test_no_email_addresses_in_logs(self, caplog): """Test that email addresses are not logged.""" # Email regex pattern email_pattern = r'\b[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Z|a-z]{2,}\b' caplog.set_level(logging.DEBUG) # Trigger various operations that might log email # ... (requires integration test) # Check logs for email addresses for record in caplog.records: match = re.search(email_pattern, record.message) assert match is None, ( f"Email address found in log: {record.message}" ) def test_no_full_tokens_in_logs(self, caplog): """Test that full tokens are not logged (only prefixes).""" caplog.set_level(logging.DEBUG) # Trigger token generation # ... (requires integration test) # Check logs for long alphanumeric strings (potential tokens) for record in caplog.records: # Tokens are 32+ chars of base64url long_tokens = re.findall(r'[A-Za-z0-9_-]{32,}', record.message) # If found, should be prefixed with "..." or truncated for token in long_tokens: assert "..." in record.message, ( f"Full token logged: {record.message}" ) def test_no_passwords_in_logs(self, caplog): """Test that passwords are never logged.""" caplog.set_level(logging.DEBUG) # Check all logs for "password" keyword for record in caplog.records: if "password" in record.message.lower(): # Should only be in config messages like "SMTP password: ***" assert "***" in record.message or "password" in record.levelname.lower() def test_logging_guidelines_documented(self): """Test that logging guidelines are documented.""" # Check for LOGGING.md or similar documentation from pathlib import Path docs_dir = Path("docs/standards") coding_doc = docs_dir / "coding.md" assert coding_doc.exists(), "Coding standards documentation missing" content = coding_doc.read_text() assert "logging" in content.lower() assert "PII" in content or "personal" in content.lower() ``` **Acceptance Criteria**: - No email addresses in logs (grep verification) - No full tokens in logs (only 8-char prefixes with ellipsis suffix) - No passwords in logs - No IP addresses in logs (except rate limiting, temporary) - Logging guidelines documented in `/docs/standards/coding.md` **Implementation Notes**: - See `/docs/designs/phase-4b-clarifications.md` section 3 for consistent token prefix format (exactly 8 chars + ellipsis) - See section 8 for secure logging guidelines structure and examples to add to coding standards --- ## Error Handling ### Security Header Errors **Scenario**: Middleware fails to add headers **Handling**: Log error but continue processing (fail open, not fail closed). Security headers are defense-in-depth, not primary security. ```python try: response.headers["X-Frame-Options"] = "DENY" except Exception as e: logger.error(f"Failed to add security header: {e}") # Continue anyway ``` ### HTTPS Enforcement Errors **Scenario**: Cannot determine request scheme **Handling**: Assume HTTPS in production (conservative). Log warning. ```python scheme = request.url.scheme or "https" # Default to HTTPS if not scheme: logger.warning("Cannot determine request scheme, assuming HTTPS") ``` ### Security Test Failures **Scenario**: Security test fails **Handling**: Block merge/deployment. Security tests are blocking gates. ```yaml # CI/CD - name: Run Security Tests run: pytest -m security # If fails, pipeline stops (no merge) ``` --- ## Security Considerations ### Defense-in-Depth This phase implements multiple layers of security: 1. **Network**: HTTPS enforcement 2. **Headers**: Browser-level protections 3. **Input**: Validation (Pydantic, already implemented) 4. **Processing**: Constant-time operations 5. **Output**: HTML escaping 6. **Storage**: Hashed tokens, parameterized SQL 7. **Logging**: No PII exposure ### Threat Coverage | Threat | Mitigation | Component | |--------|------------|-----------| | Clickjacking | X-Frame-Options: DENY | Security Headers | | XSS | CSP + HTML escaping | Security Headers + Templates | | MIME Sniffing | X-Content-Type-Options | Security Headers | | Man-in-the-Middle | HTTPS + HSTS | HTTPS Enforcement + Headers | | SQL Injection | Parameterized queries | Already implemented (verified by tests) | | Timing Attacks | Constant-time comparison | Already implemented (verified by tests) | | Open Redirects | redirect_uri validation | Already implemented (verified by tests) | | CSRF | State parameter | Already implemented (verified by tests) | | Privacy Violation | No PII logging | PII Audit | ### Residual Risks **Accepted Risks** (v1.0.0): - DDoS attacks (handled by infrastructure/CDN) - Zero-day vulnerabilities in dependencies (monitoring required) - Social engineering (user education) - Compromised client applications (trust model limitation) **Future Mitigations** (v1.1.0+): - Rate limiting (prevent brute force) - PKCE (additional token security) - Token revocation (limit blast radius) --- ## Testing Strategy ### Test Organization **Pytest Markers**: ```python # Mark all security tests @pytest.mark.security # Run security tests only pytest -m security ``` **Test Levels**: - **Integration**: Security headers, HTTPS enforcement (middleware) - **Security**: Timing attacks, injection, XSS, redirects, CSRF - **Unit**: PII logging (caplog inspection) **Coverage Target**: 100% of security-critical code - Token validation: 100% - Input validation: 100% - Middleware: 100% ### Test Execution **Local Development**: ```bash # Run all security tests pytest -m security -v # Run with coverage pytest -m security --cov=src/gondulf --cov-report=html # Run specific security test file pytest tests/security/test_timing_attacks.py -v ``` **CI/CD Pipeline**: ```yaml security_tests: stage: test script: - pytest -m security --cov=src/gondulf --cov-report=term --cov-report=xml - bandit -r src/gondulf/ # Security linter - pip-audit # Dependency vulnerability scan artifacts: reports: coverage: coverage.xml ``` **Blocking Gate**: Security test failures block merge to main branch. --- ## Acceptance Criteria ### Component 4: Security Headers Middleware - [ ] `SecurityHeadersMiddleware` class implemented in `/src/gondulf/middleware/security_headers.py` - [ ] All 7 security headers added to responses: - [ ] X-Frame-Options: DENY - [ ] X-Content-Type-Options: nosniff - [ ] X-XSS-Protection: 1; mode=block - [ ] Strict-Transport-Security (production only) - [ ] Content-Security-Policy (configured for templates + h-app logos) - [ ] Referrer-Policy: strict-origin-when-cross-origin - [ ] Permissions-Policy: geolocation=(), microphone=(), camera=() - [ ] HSTS header conditional on production mode (not in DEBUG mode) - [ ] Middleware registered in `main.py` - [ ] All integration tests pass (`tests/integration/test_security_headers.py`) - [ ] Headers present on all endpoints (tested) - [ ] Headers present on error responses (tested) ### Component 5: HTTPS Enforcement - [ ] `HTTPSEnforcementMiddleware` class implemented in `/src/gondulf/middleware/https_enforcement.py` - [ ] HTTP requests blocked or redirected in production mode - [ ] HTTP allowed for localhost in development mode - [ ] Configuration option `GONDULF_HTTPS_REDIRECT` (default: true) - [ ] Middleware registered in `main.py` (before security headers) - [ ] Integration tests pass (`tests/integration/test_https_enforcement.py`) - [ ] Production deployment documentation updated (nginx example) ### Component 6: Security Test Suite - [ ] Security test directory created: `/tests/security/` - [ ] All 6 security test files implemented: - [ ] `test_timing_attacks.py` - Timing attack resistance tests pass - [ ] `test_sql_injection.py` - SQL injection prevention tests pass - [ ] `test_xss_prevention.py` - XSS prevention tests pass - [ ] `test_open_redirect.py` - Open redirect prevention tests pass - [ ] `test_csrf_protection.py` - CSRF protection tests pass - [ ] `test_input_validation.py` - Input validation edge case tests pass - [ ] All security tests marked with `@pytest.mark.security` - [ ] All security tests pass: `pytest -m security` - [ ] Security test coverage ≥95% for security-critical code - [ ] CI/CD pipeline runs security tests (blocking gate) ### Component 7: PII Logging Audit - [ ] All logging statements reviewed (grep audit completed) - [ ] No email addresses in logs (verified by grep + tests) - [ ] No full tokens in logs (only 8-char prefixes, verified) - [ ] No passwords in logs (verified) - [ ] No IP addresses in logs except rate limiting (verified) - [ ] PII logging test implemented: `/tests/security/test_pii_logging.py` - [ ] Logging guidelines documented in `/docs/standards/coding.md` - [ ] Code changes committed with clear commit message ### Overall Phase 4b Completion - [ ] All components implemented and tested - [ ] All security tests passing - [ ] No security warnings from bandit or pip-audit - [ ] Code review completed (security focus) - [ ] Documentation updated: - [ ] `/docs/architecture/security.md` - Implementation confirmed - [ ] `/docs/standards/coding.md` - Logging guidelines added - [ ] Deployment guide includes HTTPS configuration - [ ] Implementation report created: `/docs/reports/YYYY-MM-DD-phase-4b-security-hardening.md` --- ## Dependencies **Upstream Dependencies** (must be complete before starting): - Phase 1 (Foundation): COMPLETE ✅ - Phase 2 (Domain Verification): COMPLETE ✅ - Phase 3 (IndieAuth Protocol): COMPLETE ✅ - Phase 4a (Client Metadata): IN PROGRESS (parallel track) **Downstream Dependencies** (blocked until complete): - Phase 5 (Deployment & Testing): Requires Phase 4b complete - v1.0.0 Release: Requires Phase 4b complete **Parallel Work**: - Phase 4a (Metadata Endpoint + Client Metadata): Can proceed in parallel - Phase 4b (Security Hardening): Can proceed immediately --- ## Implementation Notes ### Estimated Effort **Component 4: Security Headers Middleware** - Implementation: 2-3 hours - Testing: 2-3 hours - **Total**: 0.5 days **Component 5: HTTPS Enforcement** - Implementation: 2-3 hours - Testing: 1-2 hours - **Total**: 0.5 days **Component 6: Security Test Suite** - Test 1 (Timing Attacks): 3-4 hours - Test 2 (SQL Injection): 2-3 hours - Test 3 (XSS Prevention): 2-3 hours - Test 4 (Open Redirect): 2-3 hours - Test 5 (CSRF Protection): 2-3 hours - Test 6 (Input Validation): 2-3 hours - **Total**: 2-2.5 days **Component 7: PII Logging Audit** - Grep audit: 1 hour - Manual review: 2-3 hours - Code changes: 2-3 hours - Testing: 1-2 hours - **Total**: 0.5-1 day **Total Phase 4b Effort**: 3.5-4.5 days ### Recommended Implementation Order 1. **Day 1 Morning**: Security Headers Middleware (Component 4) - Quick win, immediate security improvement - Tests straightforward 2. **Day 1 Afternoon**: HTTPS Enforcement (Component 5) - Pairs well with security headers - Complete middleware layer 3. **Day 2**: PII Logging Audit (Component 7) - Requires careful review - Must be thorough - Best done fresh 4. **Days 3-4**: Security Test Suite (Component 6) - Most time-consuming - Multiple test files - Save for when middleware is complete **Rationale**: Implement protections before verifying them. Security headers and HTTPS enforcement provide immediate value. PII audit is independent. Security tests verify everything at the end. ### Developer Notes **Testing Considerations**: - TestClient doesn't enforce HTTPS (limitation of test framework) - Timing attack tests require statistical analysis (may be flaky) - PII logging tests require caplog fixture and log level configuration - Security headers tests are straightforward integration tests **Code Review Focus**: - Verify all security headers are correct - Check HTTPS enforcement doesn't break development - Ensure PII audit is complete (grep verification) - Review timing attack test methodology **Documentation**: - Update deployment guide with HTTPS configuration (nginx) - Add logging guidelines to coding standards - Document security test execution in testing standards --- ## References **Standards**: - RFC 6797: HTTP Strict Transport Security (HSTS) - RFC 7034: X-Frame-Options Header - RFC 8414: OAuth 2.0 Authorization Server Metadata - W3C Content Security Policy Level 3 - W3C IndieAuth Specification (Security Considerations) - OWASP Top 10 (2021) - OWASP Secure Headers Project **Tools**: - bandit: Python security linter - pip-audit: Dependency vulnerability scanner - pytest: Testing framework with security marker support **Internal References**: - `/docs/architecture/security.md` - Security architecture - `/docs/standards/testing.md` - Testing standards - `/docs/standards/coding.md` - Coding standards (to be updated) - `/docs/reports/2025-11-20-gap-analysis-v1.0.0.md` - Gap analysis --- **Design Status**: COMPLETE - CLARIFICATIONS PROVIDED **Implementation Clarifications**: All Developer questions answered in `/docs/designs/phase-4b-clarifications.md` (2025-11-20): 1. CSP img-src allows any HTTPS source for client logos 2. HTTPS middleware checks X-Forwarded-Proto when TRUST_PROXY configured 3. Token logging uses consistent 8-char + ellipsis format 4. Timing tests use relaxed thresholds and increased samples in CI 5. SQL injection tests use behavioral testing, not source inspection 6. Security configuration (HTTPS_REDIRECT, TRUST_PROXY, SECURE_COOKIES) added to Config class 7. Pytest markers registered in pytest.ini/pyproject.toml 8. Comprehensive secure logging guidelines added to coding standards **Next Steps**: Developer may proceed with implementation. Upon completion, Developer should create implementation report in `/docs/reports/YYYY-MM-DD-phase-4b-security-hardening.md`. --- **Architect Sign-off**: This design is complete, all clarification questions are answered, and implementation may proceed. All security requirements from the v1.0.0 roadmap Phase 4 are addressed. Implementation of this design will satisfy exit criteria for Phase 4 (lines 212-218) and move v1.0.0 toward production readiness.