From a6f46715b2c462ebf8c83374b32060928dcefa30 Mon Sep 17 00:00:00 2001 From: Jack Westwood Date: Tue, 11 Nov 2025 13:38:43 +0000 Subject: [PATCH] ING-1363: Ensure client cert auth fails when disabled --- .../actions/install-cbdinocluster/action.yml | 2 +- gateway/auth/cbauthauthenticator.go | 3 + gateway/dataimpl/server_v1/authhandler.go | 2 + gateway/dataimpl/server_v1/errorhandler.go | 10 ++ gateway/test/dapi_mtls_test.go | 17 +-- gateway/test/mtls_test.go | 134 +++++++++++++++++- 6 files changed, 154 insertions(+), 14 deletions(-) diff --git a/.github/actions/install-cbdinocluster/action.yml b/.github/actions/install-cbdinocluster/action.yml index 7207a258..fddfe8c6 100644 --- a/.github/actions/install-cbdinocluster/action.yml +++ b/.github/actions/install-cbdinocluster/action.yml @@ -11,7 +11,7 @@ runs: shell: bash run: | mkdir -p "$HOME/bin" - wget -nv -O $HOME/bin/cbdinocluster https://github.com/couchbaselabs/cbdinocluster/releases/download/v0.0.96/cbdinocluster-linux-amd64 + wget -nv -O $HOME/bin/cbdinocluster https://github.com/couchbaselabs/cbdinocluster/releases/download/v0.0.98/cbdinocluster-linux-amd64 chmod +x $HOME/bin/cbdinocluster echo "$HOME/bin" >> $GITHUB_PATH - name: Initialize cbdinocluster diff --git a/gateway/auth/cbauthauthenticator.go b/gateway/auth/cbauthauthenticator.go index ec65b41a..d58d0759 100644 --- a/gateway/auth/cbauthauthenticator.go +++ b/gateway/auth/cbauthauthenticator.go @@ -14,6 +14,7 @@ import ( var ( ErrInvalidCredentials = errors.New("invalid credentials") ErrInvalidCertificate = errors.New("invalid certificate") + ErrCertAuthDisabled = errors.New("client cert auth disabled") ) type CbAuthAuthenticator struct { @@ -101,6 +102,8 @@ func (a *CbAuthAuthenticator) ValidateConnStateForObo(ctx context.Context, connS if err != nil { if errors.Is(err, cbauthx.ErrInvalidAuth) { return "", "", ErrInvalidCertificate + } else if errors.Is(err, cbauthx.ErrCertAuthDisabled) { + return "", "", ErrCertAuthDisabled } return "", "", fmt.Errorf("failed to check certificate with cbauth: %w", err) diff --git a/gateway/dataimpl/server_v1/authhandler.go b/gateway/dataimpl/server_v1/authhandler.go index 4d7d0a2a..f1f0cab9 100644 --- a/gateway/dataimpl/server_v1/authhandler.go +++ b/gateway/dataimpl/server_v1/authhandler.go @@ -88,6 +88,8 @@ func (a AuthHandler) MaybeGetOboUserFromContext(ctx context.Context) (string, st if err != nil { if errors.Is(err, auth.ErrInvalidCertificate) { return "", "", a.ErrorHandler.NewInvalidCertificateStatus() + } else if errors.Is(err, auth.ErrCertAuthDisabled) { + return "", "", a.ErrorHandler.NewCertAuthDisabledStatus() } a.Logger.Error("received an unexpected cert authentication error", zap.Error(err)) diff --git a/gateway/dataimpl/server_v1/errorhandler.go b/gateway/dataimpl/server_v1/errorhandler.go index fb46cb0c..c756c1e3 100644 --- a/gateway/dataimpl/server_v1/errorhandler.go +++ b/gateway/dataimpl/server_v1/errorhandler.go @@ -937,6 +937,16 @@ func (e ErrorHandler) NewInvalidCertificateStatus() *status.Status { return st } +func (e ErrorHandler) NewCertAuthDisabledStatus() *status.Status { + st := status.New(codes.Unauthenticated, "Client cert auth disabled on the cluster.") + st = e.tryAttachStatusDetails(st, &epb.ResourceInfo{ + ResourceType: "user", + ResourceName: "", + Description: "", + }) + return st +} + func (e ErrorHandler) NewUnexpectedAuthTypeStatus() *status.Status { st := status.New(codes.InvalidArgument, "Unexpected auth type.") return st diff --git a/gateway/test/dapi_mtls_test.go b/gateway/test/dapi_mtls_test.go index 8f60a44e..1634d9ed 100644 --- a/gateway/test/dapi_mtls_test.go +++ b/gateway/test/dapi_mtls_test.go @@ -11,17 +11,8 @@ import ( "github.com/stretchr/testify/require" ) -func (s *GatewayOpsTestSuite) TestDapiClientCertAuth() { +func (s *GatewayOpsTestSuite) TestDapiClientCertAuthTools() { testutils.SkipIfNoDinoCluster(s.T()) - - s.Run("Tools", s.Tools) - - s.Run("Proxy", s.Proxy) - - s.Run("Crud", s.Crud) -} - -func (s *GatewayOpsTestSuite) Tools() { dino := testutils.StartDinoTesting(s.T(), false) username := "dapiUser" client := s.createMtlsClient(dino, username) @@ -56,7 +47,8 @@ func (s *GatewayOpsTestSuite) Tools() { }) } -func (s *GatewayOpsTestSuite) Proxy() { +func (s *GatewayOpsTestSuite) TestDapiClientCertAuthProxy() { + testutils.SkipIfNoDinoCluster(s.T()) if !s.SupportsFeature(TestFeatureQuery) { s.T().Skip() } @@ -113,7 +105,8 @@ func (s *GatewayOpsTestSuite) Proxy() { }) } -func (s *GatewayOpsTestSuite) Crud() { +func (s *GatewayOpsTestSuite) TestDapiClientCertAuthCrud() { + testutils.SkipIfNoDinoCluster(s.T()) dino := testutils.StartDinoTesting(s.T(), false) username := "crudUser" client := s.createMtlsClient(dino, username) diff --git a/gateway/test/mtls_test.go b/gateway/test/mtls_test.go index 0df2c9f5..813aa193 100644 --- a/gateway/test/mtls_test.go +++ b/gateway/test/mtls_test.go @@ -5,6 +5,8 @@ import ( "crypto/tls" "time" + "github.com/couchbase/gocbcorex/cbhttpx" + "github.com/couchbase/gocbcorex/cbmgmtx" "github.com/couchbase/goprotostellar/genproto/admin_query_v1" "github.com/couchbase/goprotostellar/genproto/admin_search_v1" "github.com/couchbase/goprotostellar/genproto/kv_v1" @@ -169,12 +171,142 @@ func (s *GatewayOpsTestSuite) TestClientCertAuth() { } requireRpcSuccess(s.T(), resp, err) return true - }, time.Second*30, time.Second) + }, time.Second*30, time.Second*5) }) }) } } +func (s *GatewayOpsTestSuite) TestClientCertAuthConfiguration() { + testutils.SkipIfNoDinoCluster(s.T()) + dino := testutils.StartDinoTesting(s.T(), false) + username := "certConfig" + conn := s.newClientCertConn(dino, username) + kvClient := kv_v1.NewKvServiceClient(conn) + + getFn := func() (*kv_v1.GetResponse, error) { + return kvClient.Get(context.Background(), &kv_v1.GetRequest{ + BucketName: s.bucketName, + ScopeName: s.scopeName, + CollectionName: s.collectionName, + Key: s.testDocId(), + }) + } + + enableReq := &cbmgmtx.ConfigureClientCertAuthRequest{ + State: "enable", + Prefixes: []cbmgmtx.Prefix{ + { + Path: "san.email", + Prefix: "", + Delimiter: "@", + }, + }, + } + + dino.AddWriteUser(username) + + // Check that client cert auth is working as expected. + s.Run("InitialSuccess", func() { + resp, err := getFn() + requireRpcSuccess(s.T(), resp, err) + }) + + ep, err := s.testClusterInfo.AdminClient.GetMgmtEndpoint(context.Background()) + require.NoError(s.T(), err) + mgmt := cbmgmtx.Management{ + Transport: ep.RoundTripper, + UserAgent: "useragent", + Endpoint: ep.Endpoint, + Auth: &cbhttpx.BasicAuth{ + Username: ep.Username, + Password: ep.Password, + }, + } + + // Change the path that cbauth will try and get the name from and check + // that the old cert fails + err = mgmt.ConfigureClientCertAuth(context.Background(), &cbmgmtx.ConfigureClientCertAuthRequest{ + State: "enable", + Prefixes: []cbmgmtx.Prefix{ + { + Path: "subject.cn", + Prefix: "", + Delimiter: "", + }, + }, + }) + assert.NoError(s.T(), err) + + s.Run("IncorrectUsernamePath", func() { + require.Eventually(s.T(), func() bool { + _, err := getFn() + if err == nil { + return false + } + + assertRpcStatus(s.T(), err, codes.PermissionDenied) + return assert.Contains(s.T(), err.Error(), "Your certificate is invalid") + }, time.Second*30, time.Second*5) + }) + + // Restore intial settings and check that the original cert works again. + err = mgmt.ConfigureClientCertAuth(context.Background(), enableReq) + assert.NoError(s.T(), err) + + s.Run("SuccessAfterSettingsReset", func() { + require.Eventually(s.T(), func() bool { + resp, err := getFn() + if err != nil { + return false + } + + requireRpcSuccess(s.T(), resp, err) + return true + }, time.Second*30, time.Second*5) + }) + + // Disable client cert auth on the cluster and make sure op fails. + err = mgmt.ConfigureClientCertAuth(context.Background(), &cbmgmtx.ConfigureClientCertAuthRequest{ + State: "disable", + Prefixes: []cbmgmtx.Prefix{ + { + Path: "san.email", + Prefix: "", + Delimiter: "@", + }, + }, + }) + assert.NoError(s.T(), err) + + s.Run("CertAuthDisabled", func() { + require.Eventually(s.T(), func() bool { + _, err := getFn() + if err == nil { + return false + } + + assertRpcStatus(s.T(), err, codes.Unauthenticated) + return assert.Contains(s.T(), err.Error(), "Client cert auth disabled on the cluster") + }, time.Second*30, time.Second*5) + }) + + err = mgmt.ConfigureClientCertAuth(context.Background(), enableReq) + assert.NoError(s.T(), err) + + // Ensure that client cert auth is full enabled for finishing the test so + // that we don't impact other cert auth tests. + require.Eventually(s.T(), func() bool { + resp, err := getFn() + if err != nil { + return false + } + + requireRpcSuccess(s.T(), resp, err) + return true + }, time.Second*30, time.Second*5) +} + func (s *GatewayOpsTestSuite) newClientCertConn(dino *testutils.DinoController, username string) *grpc.ClientConn { res := dino.GetClientCert(username)