diff --git a/docs/install.md b/docs/install.md index 400aa26..d407906 100644 --- a/docs/install.md +++ b/docs/install.md @@ -105,7 +105,8 @@ Otherwise every change made in the UI is refused with "it didn't come from this Traefik send both by default; with nginx, add `proxy_set_header X-Forwarded-Host $http_host;` (`$http_host` keeps the port, which the check needs) and `proxy_set_header X-Forwarded-Proto $scheme;`. The audit log records the client's address from -`X-Forwarded-For`. YardMaster believes these headers only from the trusted proxy: a request carrying +the last value of `X-Forwarded-For`, the one your proxy appends (with nginx, +`proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;`). YardMaster believes these headers only from the trusted proxy: a request carrying the shared secret, or one from an address in `YARDMASTER_PROXY_ADDRESSES`. ## Configuration diff --git a/internal/httpapi/httpapi_test.go b/internal/httpapi/httpapi_test.go index 0a67b43..37213e0 100644 --- a/internal/httpapi/httpapi_test.go +++ b/internal/httpapi/httpapi_test.go @@ -292,7 +292,8 @@ func TestForwardedHeadersTrustedLikeIdentity(t *testing.T) { req := func(remote string, headers ...string) *http.Request { r := httptest.NewRequest("GET", "http://ym.test/api/session", nil) r.RemoteAddr = remote - r.Header.Set("X-Forwarded-For", "192.0.2.7, 10.9.0.5") + // The client sent a made-up address; the proxy appended the one it saw. + r.Header.Set("X-Forwarded-For", "198.51.100.66, 192.0.2.7") r.Header.Set("X-Forwarded-Proto", "https") for i := 0; i+1 < len(headers); i += 2 { r.Header.Set(headers[i], headers[i+1]) @@ -320,3 +321,26 @@ func TestForwardedHeadersTrustedLikeIdentity(t *testing.T) { t.Error("untrusted: forwarded https believed") } } + +func TestHTTPSStateIsReadOnceAndSaved(t *testing.T) { + s, _ := newServer(t, settings.Settings{}) + ctx := t.Context() + s.DB.SetSetting(ctx, settingHTTPS, httpsState{Name: "ym.office.lan"}) + if st := s.httpsState(ctx); st.Name != "ym.office.lan" { + t.Fatalf("not loaded from the database: %+v", st) + } + // Later reads come from memory, not the database. + s.DB.SetSetting(ctx, settingHTTPS, httpsState{Name: "elsewhere.lan"}) + if st := s.httpsState(ctx); st.Name != "ym.office.lan" { + t.Errorf("read again from the database: %+v", st) + } + // Changes are kept in memory and saved. + if err := s.setHTTPSState(ctx, httpsState{Name: "ym.office.lan", Confirmed: true}); err != nil { + t.Fatal(err) + } + var saved httpsState + s.DB.GetSetting(ctx, settingHTTPS, &saved) + if st := s.httpsState(ctx); !st.Confirmed || !saved.Confirmed { + t.Errorf("in memory %+v, saved %+v", st, saved) + } +} diff --git a/internal/httpapi/https.go b/internal/httpapi/https.go index c640140..05178f8 100644 --- a/internal/httpapi/https.go +++ b/internal/httpapi/https.go @@ -4,6 +4,7 @@ import ( "context" _ "embed" "html/template" + "log/slog" "net/http" "strconv" @@ -23,10 +24,31 @@ type httpsState struct { const settingHTTPS = "https" +// httpsState returns the stored HTTPS state. It's read from the database once and then +// kept in memory: port 8080 asks for it on every request, and only setHTTPSState changes it. func (s *Server) httpsState(ctx context.Context) httpsState { - var st httpsState - s.DB.GetSetting(ctx, settingHTTPS, &st) - return st + s.httpsMu.Lock() + defer s.httpsMu.Unlock() + if !s.httpsLoaded { + var st httpsState + if _, err := s.DB.GetSetting(ctx, settingHTTPS, &st); err != nil { + slog.Error("reading the HTTPS state", "err", err) + return httpsState{} + } + s.https, s.httpsLoaded = st, true + } + return s.https +} + +// setHTTPSState stores the HTTPS state. +func (s *Server) setHTTPSState(ctx context.Context, st httpsState) error { + s.httpsMu.Lock() + defer s.httpsMu.Unlock() + if err := s.DB.SetSetting(ctx, settingHTTPS, st); err != nil { + return err + } + s.https, s.httpsLoaded = st, true + return nil } // httpsActive reports whether YardMaster serves HTTPS on its HTTPS port, and the name @@ -62,7 +84,7 @@ func (s *Server) SeedHTTPSName(ctx context.Context) { return } if _, err := s.TLS.SetName(s.Settings.PublicHost); err == nil { - s.DB.SetSetting(ctx, settingHTTPS, httpsState{Name: s.Settings.PublicHost}) + s.setHTTPSState(ctx, httpsState{Name: s.Settings.PublicHost}) } } @@ -111,7 +133,7 @@ func (s *Server) handleHTTPSName(w http.ResponseWriter, r *http.Request, u *auth if st.Name != in.Name { st = httpsState{Name: in.Name} } - if err := s.DB.SetSetting(r.Context(), settingHTTPS, st); err != nil { + if err := s.setHTTPSState(r.Context(), st); err != nil { logErr(w, err) return } @@ -143,7 +165,7 @@ func (s *Server) handleHTTPSConfirm(w http.ResponseWriter, r *http.Request, u *a return } st.Confirmed = true - if err := s.DB.SetSetting(r.Context(), settingHTTPS, st); err != nil { + if err := s.setHTTPSState(r.Context(), st); err != nil { logErr(w, err) return } diff --git a/internal/httpapi/server.go b/internal/httpapi/server.go index 606d2c1..7adf9ec 100644 --- a/internal/httpapi/server.go +++ b/internal/httpapi/server.go @@ -4,6 +4,7 @@ package httpapi import ( + "context" "crypto/rand" "embed" "encoding/hex" @@ -62,6 +63,10 @@ type Server struct { setupMu sync.Mutex setupCode string // first-run code printed to the log; "" once used + + httpsMu sync.Mutex + https httpsState + httpsLoaded bool } func New(d Deps) *Server { @@ -150,7 +155,7 @@ func (s *Server) Handler() http.Handler { mux.Handle("/v1/", s.gateway) mux.HandleFunc("/", s.handleStatic) - return s.securityHeaders(s.originGuard(mux)) + return s.securityHeaders(s.withForwarded(s.originGuard(mux))) } // PlainHandler serves port 8080. Until HTTPS is active it is the full application. Once @@ -292,24 +297,51 @@ func (s *Server) isSiteHost(r *http.Request, host string) bool { return fwd != "" && strings.EqualFold(host, fwd) } -// fromTrustedProxy reports whether r came through the trusted proxy, by its shared secret -// or a configured address: the same trust that lets it name the user, so its -// X-Forwarded-* headers are believed too. -func (s *Server) fromTrustedProxy(r *http.Request) bool { - return s.Settings.Auth == settings.AuthProxy && auth.ProxyTrusted(r, s.Settings.Proxy, remoteAddr(r), true) +// forwarded is what the trusted proxy reports about a request: the host and scheme the +// browser used and the client's address. It's empty unless the request came through the +// proxy, by its shared secret or a configured address: the same trust that lets it name +// the user. +type forwarded struct { + host, proto, client string +} + +type forwardedKey struct{} + +// withForwarded decides once per request whether to believe its X-Forwarded-* headers. +func (s *Server) withForwarded(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ctx := context.WithValue(r.Context(), forwardedKey{}, s.readForwarded(r)) + next.ServeHTTP(w, r.WithContext(ctx)) + }) +} + +// forwardedFor returns what withForwarded decided for r. +func (s *Server) forwardedFor(r *http.Request) forwarded { + if f, ok := r.Context().Value(forwardedKey{}).(forwarded); ok { + return f + } + return s.readForwarded(r) } -// forwarded is the first value of an X-Forwarded-* header from the trusted proxy; empty -// otherwise. -func (s *Server) forwarded(r *http.Request, header string) string { - if !s.fromTrustedProxy(r) { - return "" +func (s *Server) readForwarded(r *http.Request) forwarded { + if s.Settings.Auth != settings.AuthProxy || !auth.ProxyTrusted(r, s.Settings.Proxy, remoteAddr(r), true) { + return forwarded{} + } + // The proxy reports the browser's host and scheme, so their first value is its own. It + // appends the address it saw to whatever X-Forwarded-For the client sent, so only the + // last value is. + host := strings.Split(r.Header.Get("X-Forwarded-Host"), ",") + proto := strings.Split(r.Header.Get("X-Forwarded-Proto"), ",") + client := strings.Split(r.Header.Get("X-Forwarded-For"), ",") + return forwarded{ + host: strings.TrimSpace(host[0]), + proto: strings.TrimSpace(proto[0]), + client: strings.TrimSpace(client[len(client)-1]), } - return strings.TrimSpace(strings.Split(r.Header.Get(header), ",")[0]) } // forwardedHost is the host the browser used, as the trusted proxy reports it. -func (s *Server) forwardedHost(r *http.Request) string { return s.forwarded(r, "X-Forwarded-Host") } +func (s *Server) forwardedHost(r *http.Request) string { return s.forwardedFor(r).host } // ---- helpers ---- @@ -371,8 +403,8 @@ func remoteAddr(r *http.Request) netip.Addr { // clientIP is the address recorded in the audit log. Behind another proxy it's the // address the proxy reports. func (s *Server) clientIP(r *http.Request) string { - if f := s.forwarded(r, "X-Forwarded-For"); f != "" { - return f + if c := s.forwardedFor(r).client; c != "" { + return c } return remoteAddr(r).String() } @@ -403,5 +435,5 @@ func (s *Server) isHTTPS(r *http.Request) bool { if r.TLS != nil { return true } - return strings.EqualFold(s.forwarded(r, "X-Forwarded-Proto"), "https") + return strings.EqualFold(s.forwardedFor(r).proto, "https") }