From fbefe301cdfdb053a3b621e50d8e855342cb7ddb Mon Sep 17 00:00:00 2001 From: Jason Ernst Date: Sat, 3 Oct 2026 09:12:38 -0700 Subject: [PATCH] Session cookie: not Secure over plain http on a local-network host The session cookie was always Secure unless SESSION_SECURE=false, and a browser silently drops a Secure cookie over plain http anywhere but localhost. On a home server reached as http://192.168.1.20:7000 nobody could get past the setup code or log in, with nothing saying why. The attribute is now decided per request. SESSION_SECURE=true or false is final. Otherwise the cookie stays Secure except for a plain-http request whose host can only be on a local network: an IP address, a bare name, or a private-use suffix. A public host name stays Secure even over what looks like plain http, since that is also what a TLS-terminating proxy that forwards no headers looks like. Closes #665. Co-Authored-By: Claude Fable 5.1 --- README.md | 2 +- csrf_test.go | 86 +++++++++++++++++++-- goblog.go | 67 ++++++++++++++-- scripts/install-smoke-test.sh | 8 +- themes/default/templates/wizard_unlock.html | 2 +- 5 files changed, 150 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 0055a13..3a3bcf9 100644 --- a/README.md +++ b/README.md @@ -95,7 +95,7 @@ Set `TRUSTED_PROXIES` so `X-Forwarded-For` headers are trusted for client IP res TRUSTED_PROXIES=172.16.0.0/12 ./goblog ``` -The session cookie is `HttpOnly`, `SameSite=Lax` and `Secure`, so it is only sent over HTTPS (browsers exempt `localhost`, so local development on `http://localhost:7000` still works). If you serve goblog over plain HTTP on any other host, set `SESSION_SECURE=false` or logins will not stick. Mutating `/api/v1` requests must be sent as `application/json` (`/api/v1/upload` as `multipart/form-data`); anything else gets `415 Unsupported Media Type`. +The session cookie is `HttpOnly`, `SameSite=Lax` and `Secure`, so it is only sent over HTTPS. The one exception goblog makes by itself is a request over plain HTTP for a host that can only be on a local network (an IP address, a bare host name, or a name ending in `.local`, `.lan`, `.internal`, `.home.arpa` or `.localhost`): there the cookie is not marked `Secure`, so a home-server install at `http://192.168.1.20:7000` can log in. `SESSION_SECURE=true` or `SESSION_SECURE=false` overrides that decision either way; if you serve goblog over plain HTTP on a public host name, set `SESSION_SECURE=false` or logins will not stick. Mutating `/api/v1` requests must be sent as `application/json` (`/api/v1/upload` as `multipart/form-data`); anything else gets `415 Unsupported Media Type`. ### Admin Password The admin account the wizard creates signs in on the login page with its email and password. The email is only a sign-in name; goblog sends nothing to it. Passwords are at least 10 characters and stored as bcrypt hashes, and sign-in attempts are rate limited per client address. diff --git a/csrf_test.go b/csrf_test.go index 5b060bc..d6ef60e 100644 --- a/csrf_test.go +++ b/csrf_test.go @@ -1,25 +1,99 @@ package main import ( + "crypto/tls" "net/http" "net/http/httptest" "os" "strings" "testing" + "github.com/gin-contrib/sessions" + "github.com/gin-contrib/sessions/cookie" "github.com/gin-gonic/gin" ) func TestSessionOptions(t *testing.T) { - o := sessionOptions("") + o := sessionOptions(true) if !o.HttpOnly || !o.Secure || o.SameSite != http.SameSiteLaxMode || o.Path != "/" || o.MaxAge <= 0 { - t.Errorf("default options: %+v", o) + t.Errorf("options: %+v", o) } - if sessionOptions("false").Secure { - t.Error("SESSION_SECURE=false should clear Secure") + if sessionOptions(false).Secure { + t.Error("sessionOptions(false) is Secure") } - if !sessionOptions("true").Secure { - t.Error("SESSION_SECURE=true should keep Secure") +} + +// TestCookieSecure: the session cookie is Secure unless the operator said +// otherwise or the request is plain http for a host on a local network, +// where a Secure cookie would be dropped and nobody could log in (#665). +func TestCookieSecure(t *testing.T) { + type req struct { + env, host, proto string + tls bool + } + for name, tc := range map[string]struct { + req + want bool + }{ + "LAN address": {req{host: "192.168.1.20:7000"}, false}, + "loopback address": {req{host: "127.0.0.1:7000"}, false}, + "IPv6 address": {req{host: "[fd00::1]:7000"}, false}, + "bare host name": {req{host: "nas:7000"}, false}, + "mDNS name": {req{host: "blog.local"}, false}, + "localhost": {req{host: "localhost:7000"}, false}, + "public name, plain http": {req{host: "blog.example.com"}, true}, // or a proxy that forwards no headers + "public name behind https proxy": {req{host: "blog.example.com", proto: "https"}, true}, + "LAN address behind https proxy": {req{host: "192.168.1.20", proto: "HTTPS, http"}, true}, + "LAN address over TLS": {req{host: "192.168.1.20:7000", tls: true}, true}, + "no host": {req{host: ""}, true}, + "forced on, LAN address": {req{env: "true", host: "192.168.1.20:7000"}, true}, + "forced off, public name": {req{env: "false", host: "blog.example.com", proto: "https"}, false}, + } { + r := httptest.NewRequest(http.MethodGet, "/", nil) + r.Host = tc.host + if tc.proto != "" { + r.Header.Set("X-Forwarded-Proto", tc.proto) + } + if tc.tls { + r.TLS = &tls.ConnectionState{} + } + if got := cookieSecure(tc.env, r); got != tc.want { + t.Errorf("%s: Secure = %v, want %v", name, got, tc.want) + } + } +} + +// TestSessionCookieSecurity: the middleware's decision is what ends up on +// the Set-Cookie header. +func TestSessionCookieSecurity(t *testing.T) { + gin.SetMode(gin.TestMode) + r := gin.New() + store := cookie.NewStore([]byte("test")) + store.Options(sessionOptions(true)) + r.Use(sessions.Sessions("session", store)) + r.Use(sessionCookieSecurity("")) + r.GET("/", func(c *gin.Context) { + s := sessions.Default(c) + s.Set("k", "v") + if err := s.Save(); err != nil { + t.Fatal(err) + } + }) + for host, wantSecure := range map[string]bool{"192.168.1.20:7000": false, "blog.example.com": true} { + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.Host = host + w := httptest.NewRecorder() + r.ServeHTTP(w, req) + header := w.Header().Get("Set-Cookie") + if header == "" { + t.Fatalf("%s: no Set-Cookie", host) + } + if got := strings.Contains(header, "Secure"); got != wantSecure { + t.Errorf("%s: Secure = %v, want %v (%s)", host, got, wantSecure, header) + } + if !strings.Contains(header, "HttpOnly") || !strings.Contains(header, "SameSite=Lax") { + t.Errorf("%s: cookie lost HttpOnly or SameSite: %s", host, header) + } } } diff --git a/goblog.go b/goblog.go index b7ebb9f..f9b83f3 100644 --- a/goblog.go +++ b/goblog.go @@ -21,6 +21,7 @@ import ( "gorm.io/gorm" "log" "mime" + "net" "net/http" "os" "strings" @@ -479,9 +480,10 @@ func main() { router.Use(requireJSON()) router.Use(gplugin.Middleware(registry)) store := cookie.NewStore([]byte(sessionKey)) - store.Options(sessionOptions(os.Getenv("SESSION_SECURE"))) + store.Options(sessionOptions(true)) hostname, err := os.Hostname() router.Use(sessions.Sessions(hostname, store)) + router.Use(sessionCookieSecurity(os.Getenv("SESSION_SECURE"))) log.Println("Hostname: ", hostname) // Load templates from the active theme directory, falling back to "default". // activeTheme is read by the static handler on every /theme/* request and @@ -720,18 +722,73 @@ func CORS() gin.HandlerFunc { // sessionOptions returns the session cookie's attributes: HttpOnly and // SameSite=Lax, so a cross-site form post or fetch does not carry an admin's -// session, and Secure unless SESSION_SECURE=false (plain http on a host -// other than localhost, which browsers exempt). -func sessionOptions(secureEnv string) sessions.Options { +// session, and Secure as cookieSecure decides for the request. +func sessionOptions(secure bool) sessions.Options { return sessions.Options{ Path: "/", MaxAge: 30 * 24 * 60 * 60, HttpOnly: true, - Secure: secureEnv != "false", + Secure: secure, SameSite: http.SameSiteLaxMode, } } +// cookieSecure decides whether the session cookie for this request is +// marked Secure. SESSION_SECURE=true or false is the operator's answer and +// is final. Otherwise the cookie is Secure, with one exception: a request +// that arrived over plain http for a host that only exists on a local +// network (an IP address, a bare name, .local and the like). A browser +// there would silently drop a Secure cookie, so nobody could get past the +// setup code or log in on a home server (#665), and there is no https for +// the flag to protect. +// +// A public host name stays Secure even when the request looks like plain +// http, because that is also what a TLS-terminating proxy that forwards no +// headers looks like. +func cookieSecure(secureEnv string, r *http.Request) bool { + switch secureEnv { + case "true": + return true + case "false": + return false + } + forwarded, _, _ := strings.Cut(r.Header.Get("X-Forwarded-Proto"), ",") + if r.TLS != nil || strings.EqualFold(strings.TrimSpace(forwarded), "https") { + return true + } + return !localNetworkHost(r.Host) +} + +// localNetworkHost reports whether host (a Host header, port allowed) can +// only be a machine on a local network: an IP address, a name with no dot, +// or a name under a suffix reserved for private use. +func localNetworkHost(host string) bool { + if h, _, err := net.SplitHostPort(host); err == nil { + host = h + } + host = strings.ToLower(strings.TrimSuffix(strings.Trim(host, "[]"), ".")) + if host == "" { + return false + } + if net.ParseIP(host) != nil || !strings.Contains(host, ".") { + return true + } + for _, suffix := range []string{".local", ".lan", ".internal", ".home.arpa", ".localhost"} { + if strings.HasSuffix(host, suffix) { + return true + } + } + return false +} + +// sessionCookieSecurity sets the session cookie's attributes for each +// request. It has to run after sessions.Sessions. +func sessionCookieSecurity(secureEnv string) gin.HandlerFunc { + return func(c *gin.Context) { + sessions.Default(c).Options(sessionOptions(cookieSecure(secureEnv, c.Request))) + } +} + // requireJSON rejects mutating /api/v1 requests whose body is not JSON // (multipart is allowed for /api/v1/upload only). Every goblog client sends // application/json; an HTML form on another site can only send form diff --git a/scripts/install-smoke-test.sh b/scripts/install-smoke-test.sh index c35ecb9..29e6941 100755 --- a/scripts/install-smoke-test.sh +++ b/scripts/install-smoke-test.sh @@ -120,8 +120,9 @@ esac step "starting goblog ($DB${LEGACY:+, no data directory})" start_app -# The session cookie is Secure, which curl will not send over plain http, so -# it is carried by hand. login stores whatever cookies the last response set. +# The session cookie is carried by hand, so the test does not depend on what +# curl's cookie jar makes of its attributes. keep_cookies stores whatever +# cookies the last response set. COOKIE="" keep_cookies() { set_cookies=$(grep -i '^set-cookie:' "$TMP/headers" | sed -E 's/^[^:]*: *([^;]*).*/\1/' | paste -sd ';' - | sed 's/;/; /g' || true) @@ -154,6 +155,9 @@ SETUP_CODE=$(docker logs "$APP" 2>&1 | sed -n 's/.*GoBlog setup code: \([A-Z0-9- [ -n "$SETUP_CODE" ] || fail "no setup code in the log" as_owner "POST /wizard/unlock" 303 -X POST "$BASE/wizard/unlock" -d "setup_code=$SETUP_CODE" [ -n "$COOKIE" ] || fail "POST /wizard/unlock set no cookie" +# Plain http to an IP address: a Secure cookie would be dropped by a browser, +# and the install could not get past this page (#665). +! grep -i '^set-cookie:' "$TMP/headers" | grep -qi '; *secure' || fail "the session cookie is Secure over plain http on an IP address" as_owner "GET /" 200 "$BASE/" body_has 'name="dbtype"' "GET / with the setup code" diff --git a/themes/default/templates/wizard_unlock.html b/themes/default/templates/wizard_unlock.html index 20b9972..b5fe997 100644 --- a/themes/default/templates/wizard_unlock.html +++ b/themes/default/templates/wizard_unlock.html @@ -29,7 +29,7 @@
Setup code
-

The code changes every time goblog restarts. If this page comes back after you enter the right code, your browser is not keeping the session cookie: goblog marks it Secure, which browsers only accept over https or on localhost. Use one of those, or start goblog with SESSION_SECURE=false.

+

The code changes every time goblog restarts. If this page comes back after you enter the right code, your browser is not keeping the session cookie: on a public host name goblog marks it Secure, which browsers only accept over https. Use https, or start goblog with SESSION_SECURE=false.

{{ template "_powered_by" . }}