fix(security): take the dashboard cookie Secure flag off the broker TLS setting

README.md

@@ -513,6 +513,7 @@ curl -H "Authorization: Bearer $TOKEN" http://localhost:4041/
 | `MALACHI_DASHBOARD_AUTH_RATE_WINDOW_MS` | `60000` | Rate limit window (1 minute) |
 | `MALACHI_DASHBOARD_CORS_ENABLED` | `false` | Enable CORS for `/metrics` and `/stream` |
 | `MALACHI_DASHBOARD_CORS_ORIGINS` | `*` | Allowed CORS origins (comma-separated) |
+| `MALACHI_DASHBOARD_SECURE_COOKIE` | `false` | Mark the session cookie `Secure`. Set it only behind a TLS-terminating proxy: the dashboard listener itself serves plain HTTP, and a browser refuses to store a `Secure` cookie from a plain-HTTP origin, which makes login fail with no error. |
 | `MALACHI_DASHBOARD_CSP` | (default) | Custom Content-Security-Policy |
 | `MALACHI_HSTS_ENABLED` | `true` | Enable HTTP Strict Transport Security |
 | `MALACHI_HSTS_MAX_AGE` | `31536000` | HSTS max-age (1 year) |

config/runtime.exs

@@ -334,6 +334,12 @@ config :malachi,
      end),
   dashboard_auth_rate_limit: parse_int.(System.get_env("MALACHI_DASHBOARD_AUTH_RATE_LIMIT"), 10),
   dashboard_auth_rate_window_ms: parse_int.(System.get_env("MALACHI_DASHBOARD_AUTH_RATE_WINDOW_MS"), 60_000),
+  # Whether the session cookie is marked Secure. Off by default because `Malachi.Dashboard` listens with
+  # `:gen_tcp.listen` and has no TLS path, so the transport it actually serves is plain HTTP, and a browser
+  # refuses to store a Secure cookie from a non-trustworthy origin. Turn it on when a TLS-terminating proxy
+  # sits in front, which is the only shape where the dashboard is reached over HTTPS. This deliberately does
+  # not read `enable_tls`: that switch describes the broker listener on another port.
+  dashboard_secure_cookie: System.get_env("MALACHI_DASHBOARD_SECURE_COOKIE") == "true",
   # CORS configuration
   dashboard_cors_enabled: System.get_env("MALACHI_DASHBOARD_CORS_ENABLED") == "true",
   dashboard_cors_origins:

docs/DOCKER_README.md

@@ -84,7 +84,8 @@ Open [http://localhost:4041](http://localhost:4041) in your browser.
 | `MALACHI_TCP_PORT` | `4040` | TCP server port for clients |
 | `MALACHI_DASHBOARD_PORT` | `4041` | HTTP dashboard port |
 | `MALACHI_LOCALE` | `en_US` | Language (`en_US`, `pt_BR`) |
-| `MALACHI_ENABLE_TLS` | `false` | Enable TLS encryption |
+| `MALACHI_ENABLE_TLS` | `false` | Enable TLS encryption on the broker port. It says nothing about the dashboard, which has no TLS path. |
+| `MALACHI_DASHBOARD_SECURE_COOKIE` | `false` | Mark the dashboard session cookie `Secure`. Set it only behind a TLS-terminating proxy; see the ports note below. |
 | `MALACHI_ADMIN_PASS` | *(generated)* | Admin password; if unset, a random one is generated and logged on first boot. Without a persistent ra volume, this happens again on every restart. See Users and credentials. |
 | `MALACHI_LOG_DATA_DIR` | *(tmp)* | Directory for the durable log segments. Must be an absolute path on a volume; see Data Persistence. |
 | `MALACHI_RA_DATA_DIR` | *(tmp)* | Directory for the ra log (users, ACLs, lockouts). Must be an absolute path on a volume; see Data Persistence. |
@@ -140,6 +141,15 @@ the login form and the session cookie. Publishing it on every interface would ha
 cleartext next to a broker you had just taken the trouble to encrypt. Reach a remote dashboard
 through a reverse proxy that terminates TLS in front of it, not by widening this binding.
 
+When you do put a proxy in front, set `MALACHI_DASHBOARD_SECURE_COOKIE=true`. That is the one thing
+the server cannot work out for itself: it only ever sees a plain-HTTP connection from the proxy, so
+whether the browser reached it over HTTPS is a fact about your deployment. Leave it off for a
+dashboard reached directly, because a browser refuses to store a `Secure` cookie served over plain
+HTTP, and the resulting login answers 200 and simply does nothing. The boot log states which of the
+two is in effect. Note that this used to follow `MALACHI_ENABLE_TLS`, which describes the broker port
+and made every production build mark the cookie `Secure` over plain HTTP; if you run behind a proxy
+and login worked before without setting anything, that is the setting you now need.
+
 The mounted files are read at boot and validated then, so a certificate that is expired or unreadable
 stops the container rather than being discovered by a client later.
 

lib/malachi/dashboard.ex

@@ -32,6 +32,16 @@ defmodule Malachi.Dashboard do
     case :gen_tcp.listen(port, opts) do
       {:ok, socket} ->
         Logger.info(I18n.t(:dashboard_started, port: port))
+        # Stating the cookie policy at boot is part of the fix rather than decoration: the failure it
+        # guards against is a login that answers 200 and does nothing, which is invisible from the outside.
+        # The two states need different advice, not one sentence with a boolean in it, so they are two
+        # messages: one names the variable to set, the other names the assumption being made.
+        Logger.info(
+          if secure_cookie?(),
+            do: I18n.t(:dashboard_cookie_secure),
+            else: I18n.t(:dashboard_cookie_plain)
+        )
+
         send(self(), :accept)
         {:ok, %{socket: socket, port: port}}
 
@@ -341,14 +351,37 @@ defmodule Malachi.Dashboard do
     "HTTP/1.1 302 Found\r\nLocation: /login\r\nSet-Cookie: malachi_token=; HttpOnly; Path=/; SameSite=Strict; Max-Age=0#{secure_cookie_flag()}\r\nCache-Control: no-store\r\nContent-Length: 0\r\n\r\n"
   end
 
-  # "; Secure" only under TLS, so the cookie is not marked Secure on a plain-HTTP dev server (where the
-  # browser would then silently drop it). Shared by the login cookie and the cookie-clearing redirects, so a
-  # change to the Secure policy cannot make them disagree.
+  # "; Secure" comes from the dashboard's own setting, and from nothing else. This module listens with
+  # `:gen_tcp.listen` and has no TLS path, so the transport it serves is always plain HTTP; the only way it
+  # is reached over HTTPS is behind a proxy that terminates TLS, and that is a fact about the deployment
+  # which only the operator can state. It used to read `:enable_tls`, the broker's switch for another port,
+  # which marked the cookie Secure over plain HTTP: browsers refuse to store that, so login failed with a
+  # 200 and no error for anyone not on localhost. Shared by the login cookie and the cookie-clearing
+  # redirects, so a change to the policy cannot make them disagree.
   defp secure_cookie_flag do
-    if Application.get_env(:malachi, :enable_tls), do: "; Secure", else: ""
+    if secure_cookie?(), do: "; Secure", else: ""
+  end
+
+  defp secure_cookie?, do: Application.get_env(:malachi, :dashboard_secure_cookie, false)
+
+  # `X-Forwarded-Proto` is read here to REPORT, never to decide. The cookie policy comes from configuration
+  # and nothing a client sends can change it, which is why this needs no trusted-proxy gate. What it buys is
+  # the one diagnosis the server can otherwise not make: whether the policy matches how the browser actually
+  # reached the dashboard. Only a header that is present and disagrees is worth a line, in either direction.
+  # A proxy that forwards nothing is ordinary, so its absence has to stay silent or the warning becomes
+  # noise and stops being read.
+  defp warn_on_proto_mismatch(headers) do
+    case {secure_cookie?(), Map.get(headers, "x-forwarded-proto")} do
+      {_same, nil} -> :ok
+      {true, "https"} -> :ok
+      {false, proto} when proto != "https" -> :ok
+      {true, proto} -> Logger.warning(I18n.t(:dashboard_cookie_secure_over_plain, proto: proto))
+      {false, _https} -> Logger.warning(I18n.t(:dashboard_cookie_plain_over_https))
+    end
   end
 
-  defp send_login_success(socket, token) do
+  defp send_login_success(socket, token, headers) do
+    warn_on_proto_mismatch(headers)
     response_body = Jason.encode!(%{"s" => "ok", "token" => token})
 
     cookie_header =
@@ -468,7 +501,7 @@ defmodule Malachi.Dashboard do
                   %{}
                 )
 
-                send_login_success(socket, token)
+                send_login_success(socket, token, headers)
 
               {:error, _reason} ->
                 Metrics.increment_dashboard_auth_failed()

lib/malachi/i18n.ex

@@ -129,6 +129,38 @@ defmodule Malachi.I18n do
       "pt_BR" => "🌐 Malachi Dashboard rodando em http://localhost:%{port}",
       "en_US" => "🌐 Malachi Dashboard running at http://localhost:%{port}"
     },
+    dashboard_cookie_plain: %{
+      "pt_BR" =>
+        "Cookie de sessão do dashboard sem Secure. O listener serve HTTP puro; defina " <>
+          "MALACHI_DASHBOARD_SECURE_COOKIE=true se um proxy terminar TLS na frente dele",
+      "en_US" =>
+        "Dashboard session cookie is not marked Secure. This listener serves plain HTTP; set " <>
+          "MALACHI_DASHBOARD_SECURE_COOKIE=true when a TLS-terminating proxy sits in front of it"
+    },
+    dashboard_cookie_secure: %{
+      "pt_BR" =>
+        "Cookie de sessão do dashboard marcado como Secure. Isso pressupõe um proxy terminando TLS na " <>
+          "frente: alcançado direto por HTTP, o navegador descarta o cookie e o login falha sem erro",
+      "en_US" =>
+        "Dashboard session cookie is marked Secure. That assumes a TLS-terminating proxy in front: " <>
+          "reached directly over HTTP, the browser drops the cookie and login fails with no error"
+    },
+    dashboard_cookie_secure_over_plain: %{
+      "pt_BR" =>
+        "Cookie Secure emitido, mas a requisição chegou com X-Forwarded-Proto: %{proto}. Se o navegador " <>
+          "alcança o dashboard por HTTP puro, ele descarta o cookie e o login falha sem erro",
+      "en_US" =>
+        "Issued a Secure cookie, but the request arrived with X-Forwarded-Proto: %{proto}. If the browser " <>
+          "reaches the dashboard over plain HTTP it drops the cookie and login fails with no error"
+    },
+    dashboard_cookie_plain_over_https: %{
+      "pt_BR" =>
+        "Requisição chegou com X-Forwarded-Proto: https, mas o cookie de sessão não está marcado como " <>
+          "Secure. Defina MALACHI_DASHBOARD_SECURE_COOKIE=true para o navegador não o enviar por HTTP",
+      "en_US" =>
+        "Request arrived with X-Forwarded-Proto: https, but the session cookie is not marked Secure. Set " <>
+          "MALACHI_DASHBOARD_SECURE_COOKIE=true so the browser will not send it over plain HTTP"
+    },
     rate_limiter_started: %{
       "pt_BR" => "✅ RateLimiter iniciado",
       "en_US" => "✅ RateLimiter started"

test/dashboard_security_test.exs

@@ -1,6 +1,8 @@
 defmodule Malachi.DashboardSecurityTest do
   use ExUnit.Case, async: false
 
+  import ExUnit.CaptureLog
+
   alias Malachi.Auth.UserStore
   alias Malachi.Dashboard.SecurityHeaders
   alias Malachi.Test.DashboardHelper
@@ -424,6 +426,73 @@ defmodule Malachi.DashboardSecurityTest do
     end
   end
 
+  # Every test here sets `:enable_tls` to the opposite of the cookie policy, and that opposition is the
+  # assertion: the flag used to be read from `:enable_tls`, which describes the broker listener on 4040,
+  # while the cookie is issued on 4041, which has no TLS path at all. Pinning them against each other is
+  # what proves the broker's setting no longer participates.
+  describe "session cookie Secure flag" do
+    test "is set when the dashboard is configured for it, with the broker's TLS off" do
+      response = with_cookie_policy(true, false, &login_response/0)
+
+      assert String.contains?(response, "Set-Cookie: malachi_token=")
+      assert String.contains?(response, "; Secure")
+    end
+
+    test "is absent when the dashboard is not configured for it, with the broker's TLS on" do
+      # The regression. Marking the cookie Secure over plain HTTP makes the browser refuse to store it,
+      # so the login form posts, the server answers 200, and nothing happens. Only localhost escapes it,
+      # because browsers treat that origin as trustworthy.
+      response = with_cookie_policy(false, true, &login_response/0)
+
+      assert String.contains?(response, "Set-Cookie: malachi_token=")
+      refute String.contains?(response, "; Secure")
+    end
+
+    test "the cookie-clearing redirect follows the same policy as the login cookie" do
+      secure = with_cookie_policy(true, false, &clearing_response/0)
+      plain = with_cookie_policy(false, true, &clearing_response/0)
+
+      # They share secure_cookie_flag/0 precisely so a policy change cannot make them disagree: a clear
+      # whose attributes do not match the cookie that was set is a clear the browser can ignore.
+      assert String.downcase(secure) =~ "set-cookie: malachi_token=;"
+      assert String.contains?(secure, "; Secure")
+      assert String.downcase(plain) =~ "set-cookie: malachi_token=;"
+      refute String.contains?(plain, "; Secure")
+    end
+
+    test "warns when a forwarded protocol says the cookie will be dropped" do
+      log =
+        capture_log(fn ->
+          with_cookie_policy(true, false, fn -> login_response(%{"X-Forwarded-Proto" => "http"}) end)
+        end)
+
+      assert log =~ "X-Forwarded-Proto"
+    end
+
+    test "warns when the request arrived over HTTPS but the cookie is not Secure" do
+      log =
+        capture_log(fn ->
+          with_cookie_policy(false, true, fn -> login_response(%{"X-Forwarded-Proto" => "https"}) end)
+        end)
+
+      assert log =~ "X-Forwarded-Proto"
+    end
+
+    test "stays quiet when the forwarded protocol agrees, and when there is none" do
+      # A proxy that does not forward the header is ordinary, so its absence cannot be a warning without
+      # becoming noise. Silence here is what keeps the two warnings above worth reading.
+      agreeing =
+        capture_log(fn ->
+          with_cookie_policy(true, false, fn -> login_response(%{"X-Forwarded-Proto" => "https"}) end)
+        end)
+
+      absent = capture_log(fn -> with_cookie_policy(true, false, &login_response/0) end)
+
+      refute agreeing =~ "X-Forwarded-Proto"
+      refute absent =~ "X-Forwarded-Proto"
+    end
+  end
+
   # CORS is off by default, so each test sets exactly the configuration it exercises. The preflight must
   # agree with what a real request would get: it answers from the same builder.
   describe "CORS preflight" do
@@ -1125,6 +1194,51 @@ defmodule Malachi.DashboardSecurityTest do
     response
   end
 
+  # Applies a cookie policy for the duration of `fun` and restores both settings afterwards, including on
+  # a failing assertion: leaking either of these would silently change what the rest of the suite tests.
+  # `enable_tls` is passed explicitly rather than left alone because these tests exist to show it has no
+  # say, which only means something when it is set against the expected outcome.
+  defp with_cookie_policy(secure_cookie?, enable_tls?, fun) do
+    previous = {
+      Application.get_env(:malachi, :dashboard_secure_cookie),
+      Application.get_env(:malachi, :enable_tls)
+    }
+
+    Application.put_env(:malachi, :dashboard_secure_cookie, secure_cookie?)
+    Application.put_env(:malachi, :enable_tls, enable_tls?)
+
+    try do
+      fun.()
+    after
+      {previous_cookie, previous_tls} = previous
+      Application.put_env(:malachi, :dashboard_secure_cookie, previous_cookie)
+      Application.put_env(:malachi, :enable_tls, previous_tls)
+    end
+  end
+
+  # Both of these bind the socket with a match rather than a case: a connection that fails has to fail the
+  # test, not pass it quietly.
+  defp login_response(extra_headers \\ %{}) do
+    {:ok, socket} = DashboardHelper.connect()
+    body = Jason.encode!(%{"username" => "dashboard_admin", "password" => "admin_pass_123"})
+
+    {:ok, response} =
+      DashboardHelper.request(socket, :POST, "/login", body: body, headers: extra_headers)
+
+    :gen_tcp.close(socket)
+    response
+  end
+
+  defp clearing_response do
+    {:ok, socket} = DashboardHelper.connect()
+
+    {:ok, response} =
+      DashboardHelper.request(socket, :GET, "/", headers: %{"Cookie" => "malachi_token=not_a_real_token"})
+
+    :gen_tcp.close(socket)
+    response
+  end
+
   # Extracts the numeric status from an HTTP response, and its JSON body.
   defp status_code(response) do
     case Regex.run(~r"HTTP/1\.1 (\d{3})", response) do