diff --git a/proxy/inbound.go b/proxy/inbound.go index e8f93fbe2..c2d1c7317 100644 --- a/proxy/inbound.go +++ b/proxy/inbound.go @@ -19,6 +19,7 @@ import ( "github.com/netbirdio/netbird/client/embed" "github.com/netbirdio/netbird/proxy/internal/auth" "github.com/netbirdio/netbird/proxy/internal/debug" + "github.com/netbirdio/netbird/proxy/internal/proxy" nbtcp "github.com/netbirdio/netbird/proxy/internal/tcp" "github.com/netbirdio/netbird/proxy/internal/types" ) @@ -141,7 +142,7 @@ func (m *inboundManager) bringUp(ctx context.Context, accountID types.AccountID, router := nbtcp.NewRouter(m.logger, accountDialResolver(accountID, client), tlsListener.Addr(), nbtcp.WithPlainHTTP(plainListener.Addr())) - scopedHandler := withTunnelLookup(m.handler, accountTunnelLookup(client)) + scopedHandler := withCapturedData(withTunnelLookup(m.handler, accountTunnelLookup(client)), accountID) // markOverlayOrigin stamps every connection accepted by an inbound // listener with a context value middlewares can read to skip @@ -530,6 +531,20 @@ func withTunnelLookup(next http.Handler, lookup auth.TunnelLookupFunc) http.Hand }) } +func withCapturedData(next http.Handler, accountID types.AccountID) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + if d := proxy.CapturedDataFromContext(ctx); d != nil { + d.SetAccountID(accountID) + } else { + d := &proxy.CapturedData{} + d.SetAccountID(accountID) + ctx = proxy.WithCapturedData(ctx, d) + } + next.ServeHTTP(w, r.WithContext(ctx)) + }) +} + // inboundDebugAdapter adapts *inboundManager to the debug.InboundProvider // interface so the debug HTTP handler can render per-account inbound // listener state without importing the proxy package. diff --git a/proxy/inbound_test.go b/proxy/inbound_test.go index 0e6081802..59bfdb009 100644 --- a/proxy/inbound_test.go +++ b/proxy/inbound_test.go @@ -20,6 +20,7 @@ import ( "google.golang.org/grpc" "github.com/netbirdio/netbird/proxy/internal/auth" + "github.com/netbirdio/netbird/proxy/internal/proxy" "github.com/netbirdio/netbird/proxy/internal/roundtrip" nbtcp "github.com/netbirdio/netbird/proxy/internal/tcp" "github.com/netbirdio/netbird/proxy/internal/types" @@ -279,6 +280,19 @@ func TestWithTunnelLookup_NilLookupIsNoop(t *testing.T) { assert.True(t, called, "wrapper without lookup must still invoke next") } +func TestWithTunnelLookup_TracksAccountID(t *testing.T) { + var called bool + inner := http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) { + called = true + assert.Equal(t, types.AccountID("test-account-id"), proxy.CapturedDataFromContext(r.Context()).GetAccountID()) + }) + + handler := withCapturedData(inner, "test-account-id") + r := httptest.NewRequest(http.MethodGet, "https://svc.example/", nil) + handler.ServeHTTP(httptest.NewRecorder(), r) + assert.True(t, called) +} + // fakeListener satisfies net.Listener for snapshot tests without binding // a real socket on the netstack. type fakeListener struct { diff --git a/proxy/internal/accesslog/middleware.go b/proxy/internal/accesslog/middleware.go index 9c644418e..a39e0209e 100644 --- a/proxy/internal/accesslog/middleware.go +++ b/proxy/internal/accesslog/middleware.go @@ -48,10 +48,16 @@ func (l *Logger) Middleware(next http.Handler) http.Handler { // Create a mutable struct to capture data from downstream handlers. // We pass a pointer in the context - the pointer itself flows down immutably, // but the struct it points to can be mutated by inner handlers. - capturedData := proxy.NewCapturedData(requestID) - capturedData.SetClientIP(sourceIp) - ctx := proxy.WithCapturedData(r.Context(), capturedData) + ctx := r.Context() + + capturedData := proxy.CapturedDataFromContext(r.Context()) + if capturedData == nil { + capturedData = &proxy.CapturedData{} + ctx = proxy.WithCapturedData(ctx, capturedData) + } + capturedData.SetRequestID(requestID) + capturedData.SetClientIP(sourceIp) start := time.Now() next.ServeHTTP(sw, r.WithContext(ctx)) diff --git a/proxy/internal/auth/middleware.go b/proxy/internal/auth/middleware.go index 735fcbac3..e4a689453 100644 --- a/proxy/internal/auth/middleware.go +++ b/proxy/internal/auth/middleware.go @@ -135,12 +135,17 @@ func (mw *Middleware) Protect(next http.Handler) http.Handler { config, exists := mw.getDomainConfig(host) mw.logger.Debugf("checking authentication for host: %s, exists: %t", host, exists) - if !exists { next.ServeHTTP(w, r) return } + // enforce account id for requests routed through the embedded client + capturedData := proxy.CapturedDataFromContext(r.Context()) + if capturedData != nil && capturedData.GetAccountID() != "" && capturedData.GetAccountID() != config.AccountID { + denyForbidden(w, config) + return + } // Set account and service IDs in captured data for access logging. setCapturedIDs(r, config) diff --git a/proxy/internal/auth/middleware_test.go b/proxy/internal/auth/middleware_test.go index cce35ae35..3970a73a6 100644 --- a/proxy/internal/auth/middleware_test.go +++ b/proxy/internal/auth/middleware_test.go @@ -1604,3 +1604,66 @@ func TestStripSessionTokenParam(t *testing.T) { }) } } + +func TestProtect_RequiresMatchingAccountIds(t *testing.T) { + mw := NewMiddleware(log.StandardLogger(), nil, nil) + kp := generateTestKeyPair(t) + + hdr := newHeaderScheme(t, "Authorization", "Bearer token", "Bearer token") + require.NoError(t, mw.AddDomain("example.com", []Scheme{hdr}, kp.PublicKey, time.Hour, "test-account-1", "", nil, false, nil)) + + var backendCalled bool + + handler := mw.Protect(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + backendCalled = true + w.WriteHeader(http.StatusOK) + })) + + tests := []struct { + description string + backendCalled bool + httpStatus int + requestFunc func(*http.Request) *http.Request + }{ + { + description: "account id is present and matching", + backendCalled: true, + httpStatus: http.StatusOK, + requestFunc: func(req *http.Request) *http.Request { + d := proxy.CapturedData{} + d.SetAccountID("test-account-1") + return req.WithContext(proxy.WithCapturedData(req.Context(), &d)) + }, + }, + { + description: "account id is present and not matching", + backendCalled: false, + httpStatus: http.StatusForbidden, + requestFunc: func(req *http.Request) *http.Request { + d := proxy.CapturedData{} + d.SetAccountID("test-account-not-matching") + return req.WithContext(proxy.WithCapturedData(req.Context(), &d)) + }, + }, + { + description: "account id not present", + backendCalled: true, + httpStatus: http.StatusOK, + requestFunc: func(req *http.Request) *http.Request { return req }, + }, + } + + for _, tt := range tests { + t.Run(tt.description, func(t *testing.T) { + backendCalled = false + req := httptest.NewRequest(http.MethodGet, "http://example.com/", nil) + req.Header.Set("Authorization", "Bearer token") + + req = tt.requestFunc(req) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + assert.Equal(t, tt.httpStatus, rec.Code) + assert.Equal(t, tt.backendCalled, backendCalled) + }) + } +} diff --git a/proxy/internal/proxy/context.go b/proxy/internal/proxy/context.go index 09bb9a8d1..98744d639 100644 --- a/proxy/internal/proxy/context.go +++ b/proxy/internal/proxy/context.go @@ -70,6 +70,12 @@ func NewCapturedData(requestID string) *CapturedData { return &CapturedData{requestID: requestID} } +func (c *CapturedData) SetRequestID(requestID string) { + c.mu.Lock() + defer c.mu.Unlock() + c.requestID = requestID +} + // GetRequestID returns the request ID. func (c *CapturedData) GetRequestID() string { c.mu.RLock()