diff --git a/management/internals/shared/grpc/proxy.go b/management/internals/shared/grpc/proxy.go index 4cae872e2..b5d6590f8 100644 --- a/management/internals/shared/grpc/proxy.go +++ b/management/internals/shared/grpc/proxy.go @@ -61,6 +61,17 @@ type ProxyTokenChecker interface { IsProxyAccessTokenValid(ctx context.Context, tokenID string) (bool, error) } +// ProxyConnectAuthorizer authorizes a proxy's claim to the cluster address it +// declares at connect time. Implementations are supplied by integrations; none +// is installed by default, so every well-formed claim is authorized — the +// declared address is otherwise only checked for availability. token is nil +// when the connection carries no proxy access token. A returned status error +// is sent to the proxy unchanged; any other error is wrapped as +// PermissionDenied. +type ProxyConnectAuthorizer interface { + AuthorizeProxyConnect(ctx context.Context, token *types.ProxyAccessToken, proxyID, address string) error +} + // ProxyServiceServer implements the ProxyService gRPC server // AgentNetworkSynthesizer produces in-memory reverse-proxy services from // Agent Network provider/policy state for the proxy snapshot path; synthesised @@ -99,6 +110,9 @@ type ProxyServiceServer struct { // and the post-flight consumption write (RecordLLMUsage). Optional — when // nil both RPCs return Unimplemented. agentNetworkLimits AgentNetworkLimitsService + // connectAuthorizer authorizes address claims at proxy connect time. + // Optional — when nil every well-formed claim is authorized. + connectAuthorizer ProxyConnectAuthorizer // ProxyController for service updates and cluster management proxyController proxy.Controller @@ -262,6 +276,23 @@ func (s *ProxyServiceServer) agentNetworkSynthesizer() AgentNetworkSynthesizer { return s.agentNetworkSynth } +// SetProxyConnectAuthorizer wires the connect-time address-claim authorizer. +// Optional — when nil (the default) every well-formed claim is authorized, +// which is the behavior without the hook. The modules layer injects this +// after the proxy server is constructed, like the other setters. +func (s *ProxyServiceServer) SetProxyConnectAuthorizer(authorizer ProxyConnectAuthorizer) { + s.mu.Lock() + s.connectAuthorizer = authorizer + s.mu.Unlock() +} + +// proxyConnectAuthorizer returns the connect authorizer under read lock. +func (s *ProxyServiceServer) proxyConnectAuthorizer() ProxyConnectAuthorizer { + s.mu.RLock() + defer s.mu.RUnlock() + return s.connectAuthorizer +} + // CheckLLMPolicyLimits is the pre-flight policy gate the proxy calls before // forwarding an LLM request upstream. Delegates to the agent-network selector, // which scores applicable policies by remaining headroom and returns the @@ -446,8 +477,9 @@ func recvSyncInit(stream proto.ProxyService_SyncMappingsServer) (*proto.SyncMapp return init, nil } -// validateProxyConnect validates the proxy ID and address, and checks cluster -// address availability for account-scoped tokens. +// validateProxyConnect validates the proxy ID and address, checks cluster +// address availability for account-scoped tokens, and finally consults the +// connect authorizer (when installed) on the address claim. func (s *ProxyServiceServer) validateProxyConnect(proxyID, address string, ctx context.Context) (proxyConnectParams, error) { if proxyID == "" { return proxyConnectParams{}, status.Errorf(codes.InvalidArgument, "proxy_id is required") @@ -467,6 +499,19 @@ func (s *ProxyServiceServer) validateProxyConnect(proxyID, address string, ctx c } } + // The authorizer runs last, outside the account-scoped branch, so it also + // sees management-wide and token-less connects. PermissionDenied keeps an + // authorization rejection distinguishable from the AlreadyExists address + // conflict above in proxy logs. + if authorizer := s.proxyConnectAuthorizer(); authorizer != nil { + if err := authorizer.AuthorizeProxyConnect(ctx, token, proxyID, address); err != nil { + if _, ok := status.FromError(err); ok { + return proxyConnectParams{}, err + } + return proxyConnectParams{}, status.Errorf(codes.PermissionDenied, "proxy connect not authorized: %v", err) + } + } + return proxyConnectParams{proxyID: proxyID, address: address}, nil } diff --git a/management/internals/shared/grpc/proxy_connect_authorizer_test.go b/management/internals/shared/grpc/proxy_connect_authorizer_test.go new file mode 100644 index 000000000..ff618227e --- /dev/null +++ b/management/internals/shared/grpc/proxy_connect_authorizer_test.go @@ -0,0 +1,168 @@ +package grpc + +import ( + "context" + "errors" + "testing" + + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + grpcstatus "google.golang.org/grpc/status" + + "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/proxy" + "github.com/netbirdio/netbird/management/server/types" +) + +// capturingAuthorizer records the arguments of the last AuthorizeProxyConnect +// call and returns a fixed error. +type capturingAuthorizer struct { + called int + token *types.ProxyAccessToken + proxyID string + address string + err error +} + +func (a *capturingAuthorizer) AuthorizeProxyConnect(_ context.Context, token *types.ProxyAccessToken, proxyID, address string) error { + a.called++ + a.token = token + a.proxyID = proxyID + a.address = address + return a.err +} + +// authorizerServer builds a ProxyServiceServer whose proxy manager reports +// every cluster address as available, so the authorizer is the only thing +// standing between a claim and success. +func authorizerServer(t *testing.T) *ProxyServiceServer { + t.Helper() + ctrl := gomock.NewController(t) + mgr := proxy.NewMockManager(ctrl) + mgr.EXPECT().IsClusterAddressAvailable(gomock.Any(), gomock.Any(), gomock.Any()).Return(true, nil).AnyTimes() + return &ProxyServiceServer{proxyManager: mgr} +} + +// TestValidateProxyConnect_NilAuthorizerUnchanged guards the no-behavior-change +// claim: with no authorizer installed — the OSS default — a well-formed claim +// succeeds and malformed input is rejected exactly as before the hook existed. +func TestValidateProxyConnect_NilAuthorizerUnchanged(t *testing.T) { + s := authorizerServer(t) + + params, err := s.validateProxyConnect("proxy-1", "cluster.example.com", scopedCtx("acc-1")) + require.NoError(t, err) + assert.Equal(t, "proxy-1", params.proxyID) + assert.Equal(t, "cluster.example.com", params.address) + + _, err = s.validateProxyConnect("", "cluster.example.com", scopedCtx("acc-1")) + require.Error(t, err) + st, ok := grpcstatus.FromError(err) + require.True(t, ok) + assert.Equal(t, codes.InvalidArgument, st.Code(), "missing proxy_id must stay InvalidArgument") + + _, err = s.validateProxyConnect("proxy-1", "not a hostname", scopedCtx("acc-1")) + require.Error(t, err) + st, ok = grpcstatus.FromError(err) + require.True(t, ok) + assert.Equal(t, codes.InvalidArgument, st.Code(), "invalid address must stay InvalidArgument") +} + +// TestValidateProxyConnect_AuthorizerReceivesClaim pins the hook contract: the +// authorizer sees the presented token and the claimed proxy ID and address, +// and an authorized claim proceeds. +func TestValidateProxyConnect_AuthorizerReceivesClaim(t *testing.T) { + s := authorizerServer(t) + auth := &capturingAuthorizer{} + s.SetProxyConnectAuthorizer(auth) + + params, err := s.validateProxyConnect("proxy-1", "cluster.example.com", scopedCtx("acc-1")) + require.NoError(t, err) + assert.Equal(t, "cluster.example.com", params.address) + + require.Equal(t, 1, auth.called, "authorizer must be consulted exactly once per connect") + assert.Equal(t, "proxy-1", auth.proxyID) + assert.Equal(t, "cluster.example.com", auth.address) + require.NotNil(t, auth.token, "the presented token must be handed to the authorizer") + require.NotNil(t, auth.token.AccountID) + assert.Equal(t, "acc-1", *auth.token.AccountID) +} + +// TestValidateProxyConnect_PlainErrorBecomesPermissionDenied pins the error +// mapping: a non-status error from the authorizer surfaces as +// PermissionDenied — distinguishable from the AlreadyExists used for address +// conflicts — and the claim does not proceed even though the address itself +// was available. +func TestValidateProxyConnect_PlainErrorBecomesPermissionDenied(t *testing.T) { + s := authorizerServer(t) + s.SetProxyConnectAuthorizer(&capturingAuthorizer{err: errors.New("not the assigned credential")}) + + _, err := s.validateProxyConnect("proxy-1", "cluster.example.com", scopedCtx("acc-1")) + require.Error(t, err) + st, ok := grpcstatus.FromError(err) + require.True(t, ok) + assert.Equal(t, codes.PermissionDenied, st.Code()) + assert.Contains(t, st.Message(), "not the assigned credential", "the authorizer's reason must survive into the status message") +} + +// TestValidateProxyConnect_StatusErrorPassesThrough pins that an authorizer +// which chooses its own status code is not second-guessed. +func TestValidateProxyConnect_StatusErrorPassesThrough(t *testing.T) { + s := authorizerServer(t) + s.SetProxyConnectAuthorizer(&capturingAuthorizer{ + err: grpcstatus.Errorf(codes.ResourceExhausted, "try later"), + }) + + _, err := s.validateProxyConnect("proxy-1", "cluster.example.com", scopedCtx("acc-1")) + require.Error(t, err) + st, ok := grpcstatus.FromError(err) + require.True(t, ok) + assert.Equal(t, codes.ResourceExhausted, st.Code(), "a status error must pass through unchanged") + assert.Equal(t, "try later", st.Message()) +} + +// TestValidateProxyConnect_AuthorizerSeesGlobalAndTokenlessConnects pins the +// call-site placement: the authorizer sits outside the account-scoped branch, +// so management-wide tokens (AccountID == nil) and connections without any +// token are also presented to it rather than bypassing policy. +func TestValidateProxyConnect_AuthorizerSeesGlobalAndTokenlessConnects(t *testing.T) { + s := &ProxyServiceServer{} // no proxy manager: neither path may reach the availability check + + auth := &capturingAuthorizer{} + s.SetProxyConnectAuthorizer(auth) + + _, err := s.validateProxyConnect("proxy-1", "cluster.example.com", globalCtx()) + require.NoError(t, err) + require.Equal(t, 1, auth.called, "a management-wide token must still be presented to the authorizer") + require.NotNil(t, auth.token) + assert.Nil(t, auth.token.AccountID) + + _, err = s.validateProxyConnect("proxy-1", "cluster.example.com", context.Background()) + require.NoError(t, err) + require.Equal(t, 2, auth.called, "a token-less connect must still be presented to the authorizer") + assert.Nil(t, auth.token, "no token in context must surface as a nil token, not a zero value") +} + +// TestValidateProxyConnect_AuthorizerRunsLast pins the ordering: input +// validation and the availability check precede policy, so the authorizer is +// never consulted about a claim that is malformed or already rejected. +func TestValidateProxyConnect_AuthorizerRunsLast(t *testing.T) { + ctrl := gomock.NewController(t) + mgr := proxy.NewMockManager(ctrl) + mgr.EXPECT().IsClusterAddressAvailable(gomock.Any(), gomock.Any(), gomock.Any()).Return(false, nil) + s := &ProxyServiceServer{proxyManager: mgr} + + auth := &capturingAuthorizer{} + s.SetProxyConnectAuthorizer(auth) + + _, err := s.validateProxyConnect("proxy-1", "not a hostname", scopedCtx("acc-1")) + require.Error(t, err) + assert.Zero(t, auth.called, "a malformed address must be rejected before policy runs") + + _, err = s.validateProxyConnect("proxy-1", "cluster.example.com", scopedCtx("acc-1")) + require.Error(t, err) + st, ok := grpcstatus.FromError(err) + require.True(t, ok) + assert.Equal(t, codes.AlreadyExists, st.Code(), "an address conflict must keep its own status") + assert.Zero(t, auth.called, "a conflicting address must be rejected before policy runs") +}