diff --git a/AGENTS.md b/AGENTS.md index 3cd3ce1d..b08e9089 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -54,7 +54,7 @@ Before proposing a new dependency or version, check these public APIs with `curl ## Secure coding - Read [THREAT_MODEL.md](THREAT_MODEL.md) before changing endpoints, `SecurityConfig`, descriptor ingestion, outbound calls (Collector, DNS, metrics) or deployment defaults. Follow its review checklist and update the model when a trust boundary or assumption changes. -- Don't loosen `SecurityConfig`: no wider `permitAll`, no `csrf.disable`, no new unauthenticated actuator or admin paths. Any change there needs a matching change in `SecurityRulesTest`. +- Don't loosen `SecurityConfig`: no wider `permitAll`, no new unauthenticated actuator or admin paths, and keep CSRF protection on the actuator chain. Any change there needs a matching change in `SecurityRulesTest`. - Descriptors, request parameters and headers are untrusted. Use parameterized queries (no string-built JPQL or SQL), validate input at the controller, and never log secrets or raw untrusted input. - Workflows: keep `permissions:` least-privilege per job, never check out PR head code under `pull_request_target`, and treat any change under `.github/workflows/` as security-relevant (pin actions by SHA). - GitHub secret scanning and push protection are enabled. If a push is blocked, remove the secret from history. Don't bypass the protection. diff --git a/THREAT_MODEL.md b/THREAT_MODEL.md index 47876111..fabc8121 100644 --- a/THREAT_MODEL.md +++ b/THREAT_MODEL.md @@ -29,7 +29,7 @@ Browser ──HTTPS──> Frontend (static) Tor Collector (HTTPS, in | Service availability | The main security property: public API, scheduler, DB. Low-effort DoS matters. | | Data integrity | Correct relay, family and location data. Users draw conclusions about Tor. | | Actuator admin plane | Log file, thread dumps, recent HTTP exchanges, metrics, request mappings, cache eviction. | -| Secrets | `TORMAP_ADMIN_PASSWORD(_BCRYPT)`, DB password, `NEW_RELIC_INGEST_KEY`, CI secrets (Docker Hub, Firebase). | +| Secrets | `TORMAP_ADMIN_PASSWORD`, DB password, `NEW_RELIC_INGEST_KEY`, CI secrets (Docker Hub, Firebase). | | Host and volume | `/tormap-data` (tens of GB of descriptors), log directory. | | Release artifacts | `tormap/backend` and `tormap/frontend` images, tormap.org frontend. | @@ -40,7 +40,7 @@ Relay data is public by design, so confidentiality of relay data is not an asset | # | Boundary | Input (attacker control) | Main threats | Code | |---|----------------------------------|------------------------------------------------------------------------------------------|----------------------------------------------------------------|------| | 1 | Public API `/relay/**`, `/` | Path params `id`, `day`; JSON arrays of IDs (≤ 50 000 / 5 000); headers (direct) | Memory/CPU/DB exhaustion, cache thrash, error leakage | `adapter/controller/`, `config/AppConfig.kt`, `config/CacheConfig.kt` | -| 2 | HTTP Basic authentication | `Authorization` header (direct) | Brute force, password-hashing CPU cost, cleartext creds without TLS | `config/SecurityConfig.kt` | +| 2 | HTTP Basic authentication | `Authorization` header on `/actuator/**` (direct) | Brute force, password-hashing CPU cost, cleartext creds without TLS | `config/SecurityConfig.kt` | | 3 | Admin plane `/actuator/**` | Requests from whoever holds the admin password | Info disclosure (log file, thread dumps, exchanges), cache eviction | `application.yml` `management.*` | | 4 | Reverse proxy headers | `Forwarded`, `X-Forwarded-*` (direct if the backend port is exposed) | Spoofed client IP/scheme/host in logs, redirects, exchanges | `ForwardedHeaderFilter` in `SecurityConfig.kt` | | 5 | Relay-published descriptor data | Nickname, contact, platform, family entries, address (indirect: any relay operator) | Parser edge cases, expensive family computation, stored XSS | `service/Descriptor*`, `util/RelayFamilyUtil.kt`, vendored `org.torproject.descriptor` | @@ -66,11 +66,13 @@ Relay data is public by design, so confidentiality of relay data is not an asset ## Existing controls -- `SecurityConfig`: only `/`, `/error`, `/relay/**` and static files are public. Actuator requires role `ADMIN`. With no - admin password configured, no user exists and the actuator is unreachable. Swagger is public only when enabled. - Stateless sessions, no form login. CSRF (double-submit cookie) for unsafe actuator methods. `SecurityRulesTest` pins - these rules. -- Admin password is read only from the environment, plaintext or BCrypt. Never stored in config files. +- `SecurityConfig`: two filter chains. The actuator chain requires role `ADMIN` via HTTP Basic. The public chain has no + authentication: only `/`, `/error`, `/relay/**` and static files are allowed, everything else is denied, and + `Authorization` headers are ignored. With no admin password configured, no user exists and the actuator is + unreachable. Swagger is public only when enabled. Stateless sessions, no form login. CSRF (double-submit cookie) for + unsafe actuator methods. `SecurityRulesTest` pins these rules. +- Admin password is read only from the environment (`TORMAP_ADMIN_PASSWORD`) and only kept BCrypt-hashed in memory. + Never stored in config files. - Security headers: `nosniff`, `X-Frame-Options: DENY`, `Referrer-Policy: no-referrer`, CSP `default-src 'none'` when Swagger is off. - Input validation: `@Size` limits on POST ID lists, typed `Long` path variables, strict `LocalDate.parse`. diff --git a/backend/README.md b/backend/README.md index 711f6b52..2bd91a89 100644 --- a/backend/README.md +++ b/backend/README.md @@ -71,28 +71,24 @@ properties: - `spring.security.user.name` (default: `admin`) -The admin password is **env-only** and must be provided via one of the following environment variables: +The admin password is **env-only** and must be provided via the `TORMAP_ADMIN_PASSWORD` environment variable +(plaintext; it is BCrypt-hashed in memory at startup). -- `TORMAP_ADMIN_PASSWORD` (plaintext) -- `TORMAP_ADMIN_PASSWORD_BCRYPT` (BCrypt hash starting with `$2a$`, `$2b$`, or `$2y$`) - -If neither env var is set, a warning is logged and actuator endpoints remain unavailable. +If it is not set, a warning is logged and actuator endpoints remain unavailable. `POST`/`PUT`/`PATCH`/`DELETE` requests to actuator endpoints are CSRF protected (double submit cookie, no session): send the same value as cookie `XSRF-TOKEN` and header `X-XSRF-TOKEN`, e.g. `curl -u admin -X POST -H 'X-XSRF-TOKEN: x' --cookie 'XSRF-TOKEN=x' ...`. `GET` requests are unaffected. +`GET` endpoints can also be opened in a browser (e.g. `https://api.tormap.org/actuator`), which prompts for the +admin credentials. Recommended (production): -- Provide `TORMAP_ADMIN_PASSWORD_BCRYPT` via your secret manager (Kubernetes Secret, Docker secret, env etc.). -- Do not put passwords/hashes into `application.yml`. - -Generate a BCrypt hash: - -```bash -# Requires Docker; will prompt for password and print the hash to stdout -docker run --rm -it httpd:2.4-alpine htpasswd -nBC 12 admin -``` +- Provide a generated `TORMAP_ADMIN_PASSWORD` of at least 16 characters via your secret manager (Kubernetes Secret, + Docker secret, env etc.). Shorter passwords log a warning at startup. +- Only `/actuator/**` uses HTTP Basic. Restrict it at your reverse proxy (IP allowlist and/or rate limit) if it does not + need to be reachable from the internet. +- Do not put passwords into `application.yml`. ## Config diff --git a/backend/src/main/kotlin/org/tormap/config/SecurityConfig.kt b/backend/src/main/kotlin/org/tormap/config/SecurityConfig.kt index 2add10af..1d6edb4c 100644 --- a/backend/src/main/kotlin/org/tormap/config/SecurityConfig.kt +++ b/backend/src/main/kotlin/org/tormap/config/SecurityConfig.kt @@ -3,6 +3,7 @@ package org.tormap.config import org.springframework.beans.factory.annotation.Value import org.springframework.context.annotation.Bean import org.springframework.context.annotation.Configuration +import org.springframework.core.annotation.Order import org.springframework.http.HttpMethod import org.springframework.security.config.Customizer import org.springframework.security.config.annotation.web.builders.HttpSecurity @@ -17,8 +18,6 @@ import org.springframework.security.web.SecurityFilterChain import org.springframework.security.web.csrf.CookieCsrfTokenRepository import org.springframework.security.web.csrf.CsrfTokenRequestAttributeHandler import org.springframework.security.web.header.writers.XXssProtectionHeaderWriter -import org.springframework.security.web.servlet.util.matcher.PathPatternRequestMatcher -import org.springframework.security.web.util.matcher.RequestMatchers import org.springframework.web.cors.CorsConfiguration import org.springframework.web.cors.CorsConfigurationSource import org.springframework.web.cors.UrlBasedCorsConfigurationSource @@ -46,10 +45,8 @@ class SecurityConfig( companion object { private const val ENV_ADMIN_PASSWORD = "TORMAP_ADMIN_PASSWORD" - private const val ENV_ADMIN_PASSWORD_BCRYPT = "TORMAP_ADMIN_PASSWORD_BCRYPT" - - private fun isBcrypt(passwordOrHash: String): Boolean = - passwordOrHash.startsWith("$2a$") || passwordOrHash.startsWith("$2b$") || passwordOrHash.startsWith("$2y$") + private const val ENV_REMOVED_ADMIN_PASSWORD_BCRYPT = "TORMAP_ADMIN_PASSWORD_BCRYPT" + private const val MIN_ADMIN_PASSWORD_LENGTH = 16 } @Bean @@ -57,26 +54,26 @@ class SecurityConfig( @Bean fun userDetailsService(passwordEncoder: PasswordEncoder): UserDetailsService { - val bcryptFromEnv = System.getenv(ENV_ADMIN_PASSWORD_BCRYPT)?.trim().orEmpty() - val passwordFromEnv = System.getenv(ENV_ADMIN_PASSWORD)?.trim().orEmpty() - - val secret = when { - bcryptFromEnv.isNotBlank() -> bcryptFromEnv - passwordFromEnv.isNotBlank() -> passwordFromEnv - else -> "" + if (!System.getenv(ENV_REMOVED_ADMIN_PASSWORD_BCRYPT).isNullOrBlank()) { + logger.warn("$ENV_REMOVED_ADMIN_PASSWORD_BCRYPT is no longer supported and is ignored. Use $ENV_ADMIN_PASSWORD instead.") } - if (secret.isBlank()) { + val password = System.getenv(ENV_ADMIN_PASSWORD)?.trim().orEmpty() + + if (password.isBlank()) { logger.warn("------------------------------------------------------") logger.warn("No admin password configured; actuator endpoints will not be accessible.") - logger.warn("Set $ENV_ADMIN_PASSWORD (plaintext) or $ENV_ADMIN_PASSWORD_BCRYPT (bcrypt hash) to enable actuator admin access.") + logger.warn("Set $ENV_ADMIN_PASSWORD to enable actuator admin access.") logger.warn("------------------------------------------------------") return InMemoryUserDetailsManager() } - val hashedPassword = if (isBcrypt(secret)) secret else passwordEncoder.encode(secret) + if (password.length < MIN_ADMIN_PASSWORD_LENGTH) { + logger.warn("$ENV_ADMIN_PASSWORD is shorter than $MIN_ADMIN_PASSWORD_LENGTH characters. Use a long generated password.") + } + val user = User.withUsername(adminUserName) - .password(hashedPassword) + .password(passwordEncoder.encode(password)) .roles("ADMIN") .build() @@ -100,24 +97,34 @@ class SecurityConfig( @Bean fun forwardedHeaderFilter(): ForwardedHeaderFilter = ForwardedHeaderFilter() + /** Admin actuator: the only chain that authenticates (HTTP Basic). */ @Bean - fun securityFilterChain(http: HttpSecurity): SecurityFilterChain { - http - .cors(Customizer.withDefaults()) - // The public API is stateless and uses no cookies, so CSRF only matters for the admin actuator, which - // authenticates with HTTP Basic (browsers resend cached credentials). Unsafe methods there require the - // XSRF-TOKEN cookie value to be echoed in the X-XSRF-TOKEN header (double submit, no server-side session). + @Order(1) + fun adminFilterChain(http: HttpSecurity): SecurityFilterChain { + applyCommonConfig(http) + .securityMatcher("$actuatorPath/**") + // Browsers resend cached HTTP Basic credentials, so unsafe methods require the XSRF-TOKEN cookie value to be + // echoed in the X-XSRF-TOKEN header (double submit, no server-side session). GET stays usable in a browser. .csrf { csrf -> csrf.csrfTokenRepository(CookieCsrfTokenRepository.withHttpOnlyFalse()) csrf.csrfTokenRequestHandler(CsrfTokenRequestAttributeHandler()) - csrf.requireCsrfProtectionMatcher( - RequestMatchers.anyOf( - *listOf(HttpMethod.POST, HttpMethod.PUT, HttpMethod.PATCH, HttpMethod.DELETE) - .map { PathPatternRequestMatcher.withDefaults().matcher(it, "$actuatorPath/**") } - .toTypedArray() - ) - ) } + // Will deny all if no users exist + .authorizeHttpRequests { auth -> auth.anyRequest().hasRole("ADMIN") } + // Form login is not configured, so there are no login pages + .httpBasic(Customizer.withDefaults()) + return http.build() + } + + /** Everything else: public paths without authentication, all other paths denied. */ + @Bean + @Order(2) + fun publicFilterChain(http: HttpSecurity): SecurityFilterChain { + applyCommonConfig(http) + // The frontend calls the public API from other origins + .cors(Customizer.withDefaults()) + // No authentication, sessions or cookies on this chain, so there is nothing for CSRF to protect + .csrf { it.disable() } .authorizeHttpRequests { auth -> auth // Allow CORS preflight @@ -137,12 +144,13 @@ class SecurityConfig( ).permitAll() } } - // Admin-only actuator; will deny all if no users exist - .requestMatchers("$actuatorPath/**").hasRole("ADMIN") - .anyRequest().authenticated() + .anyRequest().denyAll() } - // HTTP Basic for admin endpoints; form login is not configured, so there are no login pages - .httpBasic(Customizer.withDefaults()) + return http.build() + } + + private fun applyCommonConfig(http: HttpSecurity): HttpSecurity = + http // Fully stateless sessions .sessionManagement { sessions -> sessions.sessionCreationPolicy(SessionCreationPolicy.STATELESS) @@ -158,6 +166,4 @@ class SecurityConfig( headers.contentSecurityPolicy { it.policyDirectives("default-src 'none'; frame-ancestors 'none'; base-uri 'none'") } } } - return http.build() - } } diff --git a/backend/src/test/kotlin/org/tormap/config/SecurityConfigTest.kt b/backend/src/test/kotlin/org/tormap/config/SecurityConfigTest.kt index bd7b0c9c..13e90119 100644 --- a/backend/src/test/kotlin/org/tormap/config/SecurityConfigTest.kt +++ b/backend/src/test/kotlin/org/tormap/config/SecurityConfigTest.kt @@ -22,7 +22,6 @@ class SecurityConfigTest( private val userDetailsService: UserDetailsService, private val corsConfigurationSource: CorsConfigurationSource, private val forwardedHeaderFilter: ForwardedHeaderFilter, - private val securityFilterChain: SecurityFilterChain, private val shallowEtagHeaderFilterRegistration: FilterRegistrationBean, ) : StringSpec({ @@ -32,7 +31,7 @@ class SecurityConfigTest( userDetailsService shouldNotBe null corsConfigurationSource shouldNotBe null forwardedHeaderFilter shouldNotBe null - securityFilterChain shouldNotBe null + applicationContext.getBeansOfType(SecurityFilterChain::class.java).size shouldBe 2 } "shallow ETag filter is not registered for large relay location responses" { diff --git a/backend/src/test/kotlin/org/tormap/config/SecurityRulesTest.kt b/backend/src/test/kotlin/org/tormap/config/SecurityRulesTest.kt index a87a81b3..320cc81f 100644 --- a/backend/src/test/kotlin/org/tormap/config/SecurityRulesTest.kt +++ b/backend/src/test/kotlin/org/tormap/config/SecurityRulesTest.kt @@ -4,6 +4,7 @@ import io.kotest.core.spec.style.StringSpec import jakarta.servlet.http.Cookie import io.kotest.matchers.shouldBe import io.kotest.matchers.shouldNotBe +import io.kotest.matchers.string.shouldStartWith import org.springframework.beans.factory.annotation.Value import org.springframework.boot.webmvc.test.autoconfigure.AutoConfigureMockMvc import org.springframework.boot.test.context.SpringBootTest @@ -13,6 +14,7 @@ import org.springframework.test.web.servlet.MockMvc import org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get import org.springframework.test.web.servlet.request.MockMvcRequestBuilders.options import org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post +import java.util.Base64 /** * Verifies the access rules of [SecurityConfig] over HTTP. @@ -36,11 +38,24 @@ class SecurityRulesTest( } } + "public paths ignore HTTP Basic credentials" { + val basicAuth = "Basic " + Base64.getEncoder().encodeToString("admin:wrong".toByteArray()) + listOf("/", "/relay/missing").forEach { path -> + val response = mockMvc.perform(get(path).header("Authorization", basicAuth)).andReturn().response + response.status shouldNotBe 401 + response.getHeader("WWW-Authenticate") shouldBe null + } + } + "actuator endpoints require authentication" { statusOf("/actuator/health") shouldBe 401 statusOf("/actuator/metrics") shouldBe 401 } + "actuator asks browsers for HTTP Basic credentials" { + mockMvc.perform(get("/actuator/health")).andReturn().response.getHeader("WWW-Authenticate") shouldStartWith "Basic" + } + "actuator rejects unsafe methods without CSRF token before authentication" { mockMvc.perform(post("/actuator/loggers/ROOT")).andReturn().response.status shouldBe 403 } @@ -60,13 +75,13 @@ class SecurityRulesTest( status shouldNotBe 401 } - "other paths require authentication" { - statusOf("/not-a-public-path") shouldBe 401 + "other paths are denied" { + statusOf("/not-a-public-path") shouldBe 403 } "swagger and openapi are not public when disabled" { listOf("/swagger-ui.html", "/swagger-ui/index.html", "/v3/api-docs", "/openapi/api.yaml").forEach { path -> - statusOf(path) shouldBe 401 + statusOf(path) shouldBe 403 } }