diff --git a/Makefile b/Makefile index 2534a1d6..6044328b 100644 --- a/Makefile +++ b/Makefile @@ -122,4 +122,4 @@ lint-webui: # Format the code fmt: - go fmt ./... \ No newline at end of file + go fmt ./... diff --git a/internal/bootstrap/router_bootstrap.go b/internal/bootstrap/router_bootstrap.go index ae82c2d3..688e419a 100644 --- a/internal/bootstrap/router_bootstrap.go +++ b/internal/bootstrap/router_bootstrap.go @@ -22,6 +22,8 @@ func (app *BootstrapApp) setupRouter() error { gin.SetMode(gin.ReleaseMode) engine := gin.New() + // Platform headers must not bypass configured proxy trust. + engine.TrustedPlatform = "" engine.Use(gin.Recovery()) if len(app.config.Auth.TrustedProxies) > 0 { @@ -39,7 +41,7 @@ func (app *BootstrapApp) setupRouter() error { return fmt.Errorf("failed to set trusted proxies: %w", err) } - app.log.App.Warn().Msg("Trusted proxies are not configured, IP access controls will NOT work") + app.log.App.Warn().Msg("Trusted proxies are not configured, IP access controls will NOT work and only direct client connections can safely use IP-based authentication") } middlewareProvideFor := []any{ diff --git a/internal/middleware/context_middleware.go b/internal/middleware/context_middleware.go index 38955d17..0d1bbac3 100644 --- a/internal/middleware/context_middleware.go +++ b/internal/middleware/context_middleware.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "net" "net/http" "strings" "time" @@ -79,7 +80,7 @@ func (m *ContextMiddleware) Middleware() gin.HandlerFunc { uuid, err := c.Cookie(m.runtime.SessionCookieName) if err == nil { - userContext, cookie, err := m.cookieAuth(c.Request.Context(), uuid, c.ClientIP()) + userContext, cookie, err := m.cookieAuth(c.Request.Context(), uuid, c.ClientIP(), c.RemoteIP()) if err == nil { if cookie != nil { @@ -129,10 +130,11 @@ func (m *ContextMiddleware) Middleware() gin.HandlerFunc { // Lastly check if we have a tailscale session to add if m.tailscale != nil { - tailscaleContext, err := m.tailscaleWhois(c.ClientIP()) + ip := tailscaleClientIP(c.ClientIP(), c.RemoteIP(), m.config.Auth.TrustedProxies) + tailscaleContext, err := m.tailscaleWhois(ip) if err != nil { - m.log.App.Error().Err(err).Msgf("Error performing tailscale whois for IP %s: %v", c.ClientIP(), err) + m.log.App.Error().Err(err).Msgf("Error performing tailscale whois for IP %s: %v", ip, err) } if tailscaleContext != nil { @@ -148,7 +150,44 @@ func (m *ContextMiddleware) Middleware() gin.HandlerFunc { } } -func (m *ContextMiddleware) cookieAuth(ctx context.Context, uuid string, ip string) (*model.UserContext, *http.Cookie, error) { +func tailscaleClientIP(clientIP, peerIP string, trustedProxies []string) string { + parsedClientIP := net.ParseIP(clientIP) + + if parsedClientIP == nil { + return "" + } + + if len(trustedProxies) == 0 { + return clientIP + } + + parsedPeerIP := net.ParseIP(peerIP) + + if parsedClientIP.Equal(parsedPeerIP) || isTrustedProxy(parsedClientIP, trustedProxies) { + return "" + } + + return clientIP +} + +func isTrustedProxy(ip net.IP, trustedProxies []string) bool { + for _, trustedProxy := range trustedProxies { + parsedTrustedProxy := net.ParseIP(trustedProxy) + + if parsedTrustedProxy != nil && parsedTrustedProxy.Equal(ip) { + return true + } + + _, network, err := net.ParseCIDR(trustedProxy) + if err == nil && network.Contains(ip) { + return true + } + } + + return false +} + +func (m *ContextMiddleware) cookieAuth(ctx context.Context, uuid, clientIP, peerIP string) (*model.UserContext, *http.Cookie, error) { session, err := m.auth.GetSession(ctx, uuid) if err != nil { @@ -184,6 +223,7 @@ func (m *ContextMiddleware) cookieAuth(ctx context.Context, uuid string, ip stri userContext.Local.Attributes.Email = utils.CompileUserEmail(user.Username, m.runtime.CookieDomain) } case model.ProviderTailscale: + ip := tailscaleClientIP(clientIP, peerIP, m.config.Auth.TrustedProxies) tailscaleContext, err := m.tailscaleWhois(ip) if err != nil { @@ -363,7 +403,7 @@ func (m *ContextMiddleware) isIgnorePath(path string) bool { } func (m *ContextMiddleware) tailscaleWhois(ip string) (*model.TailscaleContext, error) { - if m.tailscale == nil { + if m.tailscale == nil || ip == "" { return nil, nil } diff --git a/internal/middleware/context_middleware_test.go b/internal/middleware/context_middleware_test.go index f2193cc9..f9a4a507 100644 --- a/internal/middleware/context_middleware_test.go +++ b/internal/middleware/context_middleware_test.go @@ -2,6 +2,7 @@ package middleware import ( "context" + "net" "net/http" "net/http/httptest" "testing" @@ -71,7 +72,7 @@ func TestContextMiddleware(t *testing.T) { }, }, { - description: "Valid session cookie sets authenticated local context", + description: "Valid local session cookie authenticates with unusable client addresses", run: func(t *testing.T, args runArgs) { uuid := "session-valid-local" seedSession(t, args.queries, repository.CreateSessionParams{ @@ -83,6 +84,8 @@ func TestContextMiddleware(t *testing.T) { }) req := httptest.NewRequest("GET", "/api/test", nil) + req.RemoteAddr = "invalid" + req.Header.Set("X-Forwarded-For", "invalid") req.AddCookie(&http.Cookie{Name: "tinyauth-session", Value: uuid}) userCtx, _ := args.do(req) @@ -355,3 +358,48 @@ func TestContextMiddleware(t *testing.T) { }) } } + +func TestTailscaleClientIP(t *testing.T) { + tests := []struct { + description string + peer string + proxies []string + forwarded string + unix bool + expected string + }{ + {description: "Trusted forwarded client", peer: "100.101.0.1:1234", proxies: []string{"100.101.0.1"}, forwarded: "100.101.0.2", expected: "100.101.0.2"}, + {description: "Direct client", peer: "100.101.0.1:1234", expected: "100.101.0.1"}, + {description: "No trusted proxies", peer: "100.101.0.1:1234", forwarded: "100.101.0.2", expected: "100.101.0.1"}, + {description: "Unlisted proxy in proxy mode", peer: "100.101.0.1:1234", proxies: []string{"192.0.2.1"}, forwarded: "100.101.0.2"}, + {description: "Missing forwarding", peer: "100.101.0.1:1234", proxies: []string{"100.101.0.1"}}, + {description: "Malformed forwarding", peer: "100.101.0.1:1234", proxies: []string{"100.101.0.1"}, forwarded: "invalid"}, + {description: "Trusted proxy network", peer: "100.101.0.1:1234", proxies: []string{"100.101.0.0/24"}}, + {description: "Exhausted exact trusted proxy chain", peer: "100.101.0.1:1234", proxies: []string{"100.101.0.1", "100.101.0.2"}, forwarded: "100.101.0.2"}, + {description: "Exhausted trusted proxy network", peer: "100.101.0.1:1234", proxies: []string{"100.101.0.0/24"}, forwarded: "100.101.0.2"}, + {description: "Mapped IPv4 peer equality", peer: "[::ffff:100.101.0.1]:1234", proxies: []string{"100.101.0.1"}, forwarded: "100.101.0.1"}, + {description: "Expanded IPv6 peer equality", peer: "[fd7a:115c:a1e0:0:0:0:0:1]:1234", proxies: []string{"fd7a:115c:a1e0::1"}, forwarded: "fd7a:115c:a1e0::1"}, + {description: "Invalid address", peer: "invalid"}, + {description: "Unix forwarded client", peer: "@", forwarded: "100.101.0.2", unix: true, expected: "100.101.0.2"}, + {description: "Unix missing forwarding", peer: "@", unix: true}, + {description: "Unix malformed forwarding", peer: "@", forwarded: "invalid", unix: true}, + } + + for _, tc := range tests { + t.Run(tc.description, func(t *testing.T) { + c, engine := gin.CreateTestContext(httptest.NewRecorder()) + require.NoError(t, engine.SetTrustedProxies(tc.proxies)) + + c.Request = httptest.NewRequest(http.MethodGet, "/", nil) + c.Request.RemoteAddr = tc.peer + c.Request.Header.Set("X-Forwarded-For", tc.forwarded) + + if tc.unix { + ctx := context.WithValue(c.Request.Context(), http.LocalAddrContextKey, &net.UnixAddr{Name: "/tmp/tinyauth-test.sock", Net: "unix"}) + c.Request = c.Request.WithContext(ctx) + } + + assert.Equal(t, tc.expected, tailscaleClientIP(c.ClientIP(), c.RemoteIP(), tc.proxies)) + }) + } +}