From be3d653c646bdbf19d4e843824e92b3467ab6b49 Mon Sep 17 00:00:00 2001 From: Pawel Kosiec Date: Wed, 13 Dec 2023 18:23:47 +0100 Subject: [PATCH 1/2] Support TLS encryption for gRPC connection in Cloud Slack and Teams --- pkg/bot/slack_cloud.go | 19 +++++++++++--- pkg/bot/teams_cloud.go | 4 +-- pkg/bot/teams_cloud_grpc.go | 23 +++++++++++++---- pkg/config/config.go | 11 ++++++++- pkg/grpcx/credentials.go | 49 +++++++++++++++++++++++++++++++++++++ 5 files changed, 95 insertions(+), 11 deletions(-) create mode 100644 pkg/grpcx/credentials.go diff --git a/pkg/bot/slack_cloud.go b/pkg/bot/slack_cloud.go index 10fc3c8ca..f23714d4a 100644 --- a/pkg/bot/slack_cloud.go +++ b/pkg/bot/slack_cloud.go @@ -19,7 +19,6 @@ import ( "github.com/sourcegraph/conc/pool" "google.golang.org/grpc" "google.golang.org/grpc/codes" - "google.golang.org/grpc/credentials/insecure" "google.golang.org/grpc/status" "github.com/kubeshop/botkube/internal/analytics" @@ -33,6 +32,7 @@ import ( "github.com/kubeshop/botkube/pkg/execute" "github.com/kubeshop/botkube/pkg/execute/command" "github.com/kubeshop/botkube/pkg/formatx" + "github.com/kubeshop/botkube/pkg/grpcx" "github.com/kubeshop/botkube/pkg/multierror" "github.com/kubeshop/botkube/pkg/sliceutil" ) @@ -169,8 +169,21 @@ func (b *CloudSlack) start(ctx context.Context) error { return fmt.Errorf("while getting remote config for %s", config.CloudSlackCommPlatformIntegration) } - creds := grpc.WithTransportCredentials(insecure.NewCredentials()) - opts := []grpc.DialOption{creds, + b.log.WithFields(logrus.Fields{ + "url": b.cfg.Server.URL, + "disableSecurity": b.cfg.Server.DisableSecurity, + "tlsUseSystemCertPool": b.cfg.Server.TLS.UseSystemCertPool, + "tlsCACertificateLen": len(b.cfg.Server.TLS.CACertificate), + "tlsSkipVerify": b.cfg.Server.TLS.InsecureSkipVerify, + }).Debug("Creating gRPC connection to Cloud Teams...") + + creds, err := grpcx.ClientTransportCredentials(b.log, b.cfg.Server) + if err != nil { + return fmt.Errorf("while creating gRPC credentials: %w", err) + } + + opts := []grpc.DialOption{ + grpc.WithTransportCredentials(creds), grpc.WithStreamInterceptor(cloudplatform.AddStreamingClientCredentials(remoteConfig)), grpc.WithUnaryInterceptor(cloudplatform.AddUnaryClientCredentials(remoteConfig)), } diff --git a/pkg/bot/teams_cloud.go b/pkg/bot/teams_cloud.go index 1a2e3ec84..8a7062970 100644 --- a/pkg/bot/teams_cloud.go +++ b/pkg/bot/teams_cloud.go @@ -182,9 +182,9 @@ func (b *CloudTeams) GetStatus() health.PlatformStatus { } func (b *CloudTeams) start(ctx context.Context) error { - svc, err := newGrpcCloudTeamsConnector(b.log, b.cfg.Server.URL) + svc, err := newGrpcCloudTeamsConnector(b.log, b.cfg.Server) if err != nil { - return fmt.Errorf("while creating gRPC connector") + return fmt.Errorf("while creating gRPC connector: %w", err) } defer svc.Shutdown() diff --git a/pkg/bot/teams_cloud_grpc.go b/pkg/bot/teams_cloud_grpc.go index 9f4eb8e8e..93e60d2b7 100644 --- a/pkg/bot/teams_cloud_grpc.go +++ b/pkg/bot/teams_cloud_grpc.go @@ -10,13 +10,13 @@ import ( "github.com/sourcegraph/conc/pool" "google.golang.org/grpc" "google.golang.org/grpc/codes" - "google.golang.org/grpc/credentials/insecure" "google.golang.org/grpc/status" "github.com/kubeshop/botkube/internal/config/remote" "github.com/kubeshop/botkube/pkg/api/cloudplatform" pb "github.com/kubeshop/botkube/pkg/api/cloudteams" "github.com/kubeshop/botkube/pkg/config" + "github.com/kubeshop/botkube/pkg/grpcx" ) type grpcCloudTeamsConnector struct { @@ -30,19 +30,32 @@ type grpcCloudTeamsConnector struct { activityClient pb.CloudTeams_StreamActivityClient } -func newGrpcCloudTeamsConnector(log logrus.FieldLogger, url string) (*grpcCloudTeamsConnector, error) { +func newGrpcCloudTeamsConnector(log logrus.FieldLogger, cfg config.GRPCServer) (*grpcCloudTeamsConnector, error) { remoteConfig, ok := remote.GetConfig() if !ok { return nil, fmt.Errorf("while getting remote config for %q", config.CloudTeamsCommPlatformIntegration) } - creds := grpc.WithTransportCredentials(insecure.NewCredentials()) + + log.WithFields(logrus.Fields{ + "url": cfg.URL, + "disableSecurity": cfg.DisableSecurity, + "tlsUseSystemCertPool": cfg.TLS.UseSystemCertPool, + "tlsCACertificateLen": len(cfg.TLS.CACertificate), + "tlsSkipVerify": cfg.TLS.InsecureSkipVerify, + }).Debug("Creating gRPC connection to Cloud Teams...") + + creds, err := grpcx.ClientTransportCredentials(log, cfg) + if err != nil { + return nil, fmt.Errorf("while creating gRPC credentials: %w", err) + } + opts := []grpc.DialOption{ - creds, + grpc.WithTransportCredentials(creds), grpc.WithStreamInterceptor(cloudplatform.AddStreamingClientCredentials(remoteConfig)), grpc.WithUnaryInterceptor(cloudplatform.AddUnaryClientCredentials(remoteConfig)), } - conn, err := grpc.Dial(url, opts...) + conn, err := grpc.Dial(cfg.URL, opts...) if err != nil { return nil, fmt.Errorf("while creating gRCP connection: %w", err) } diff --git a/pkg/config/config.go b/pkg/config/config.go index c2d23b536..d49f1c2eb 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -495,7 +495,16 @@ type CloudSlack struct { // GRPCServer config for gRPC server type GRPCServer struct { - URL string `yaml:"url"` + URL string `yaml:"url"` + DisableSecurity bool `yaml:"insecure"` + TLS GRPCServerTLSConfig `yaml:"tls"` +} + +// GRPCServerTLSConfig describes gRPC server TLS configuration.m +type GRPCServerTLSConfig struct { + CACertificate []byte `yaml:"caCertificate"` + UseSystemCertPool bool `yaml:"useSystemCertPool"` + InsecureSkipVerify bool `yaml:"insecureSkipVerify"` } // Elasticsearch config auth settings diff --git a/pkg/grpcx/credentials.go b/pkg/grpcx/credentials.go new file mode 100644 index 000000000..a93682e26 --- /dev/null +++ b/pkg/grpcx/credentials.go @@ -0,0 +1,49 @@ +package grpcx + +import ( + "crypto/tls" + "crypto/x509" + "fmt" + + "github.com/sirupsen/logrus" + "google.golang.org/grpc/credentials" + "google.golang.org/grpc/credentials/insecure" + + "github.com/kubeshop/botkube/pkg/config" +) + +// ClientTransportCredentials returns gRPC client transport credentials based on the provided configuration. +func ClientTransportCredentials(log logrus.FieldLogger, cfg config.GRPCServer) (credentials.TransportCredentials, error) { + if cfg.DisableSecurity { + log.Warn("Server CA certificate is not provided. Using insecure gRPC connection...") + return insecure.NewCredentials(), nil + } + + var ( + certPool *x509.CertPool + err error + ) + if cfg.TLS.UseSystemCertPool { + certPool, err = x509.SystemCertPool() + if err != nil { + return nil, fmt.Errorf("while getting system certificate pool: %w", err) + } + } else { + certPool = x509.NewCertPool() + } + + if len(cfg.TLS.CACertificate) != 0 { + log.Debug("Adding custom CA certificate for gRPC connection") + if !certPool.AppendCertsFromPEM(cfg.TLS.CACertificate) { + return nil, fmt.Errorf("failed to append CA certificate for gRPC connection") + } + } + + if cfg.TLS.InsecureSkipVerify { + log.Warn("InsecureSkipVerify is enabled. Skipping TLS certificate verification...") + } + + //nolint:gosec // G402: TLS InsecureSkipVerify may be true. - Yes, indeed - just for development purposes. + tlsCfg := &tls.Config{MinVersion: tls.VersionTLS13, InsecureSkipVerify: cfg.TLS.InsecureSkipVerify, RootCAs: certPool} + return credentials.NewTLS(tlsCfg), nil +} From 2e79af2ee09d7965d511e888fe783b9e8b4b7a7a Mon Sep 17 00:00:00 2001 From: Pawel Kosiec Date: Fri, 15 Dec 2023 10:23:00 +0100 Subject: [PATCH 2/2] Improve log and improve naming --- pkg/bot/slack_cloud.go | 2 +- pkg/bot/teams_cloud_grpc.go | 2 +- pkg/config/config.go | 6 +++--- pkg/grpcx/credentials.go | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/pkg/bot/slack_cloud.go b/pkg/bot/slack_cloud.go index f23714d4a..68e6de050 100644 --- a/pkg/bot/slack_cloud.go +++ b/pkg/bot/slack_cloud.go @@ -171,7 +171,7 @@ func (b *CloudSlack) start(ctx context.Context) error { b.log.WithFields(logrus.Fields{ "url": b.cfg.Server.URL, - "disableSecurity": b.cfg.Server.DisableSecurity, + "disableSecurity": b.cfg.Server.DisableTransportSecurity, "tlsUseSystemCertPool": b.cfg.Server.TLS.UseSystemCertPool, "tlsCACertificateLen": len(b.cfg.Server.TLS.CACertificate), "tlsSkipVerify": b.cfg.Server.TLS.InsecureSkipVerify, diff --git a/pkg/bot/teams_cloud_grpc.go b/pkg/bot/teams_cloud_grpc.go index 93e60d2b7..db91efaa6 100644 --- a/pkg/bot/teams_cloud_grpc.go +++ b/pkg/bot/teams_cloud_grpc.go @@ -38,7 +38,7 @@ func newGrpcCloudTeamsConnector(log logrus.FieldLogger, cfg config.GRPCServer) ( log.WithFields(logrus.Fields{ "url": cfg.URL, - "disableSecurity": cfg.DisableSecurity, + "disableSecurity": cfg.DisableTransportSecurity, "tlsUseSystemCertPool": cfg.TLS.UseSystemCertPool, "tlsCACertificateLen": len(cfg.TLS.CACertificate), "tlsSkipVerify": cfg.TLS.InsecureSkipVerify, diff --git a/pkg/config/config.go b/pkg/config/config.go index d49f1c2eb..054cf619a 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -495,9 +495,9 @@ type CloudSlack struct { // GRPCServer config for gRPC server type GRPCServer struct { - URL string `yaml:"url"` - DisableSecurity bool `yaml:"insecure"` - TLS GRPCServerTLSConfig `yaml:"tls"` + URL string `yaml:"url"` + DisableTransportSecurity bool `yaml:"disableTransportSecurity"` + TLS GRPCServerTLSConfig `yaml:"tls"` } // GRPCServerTLSConfig describes gRPC server TLS configuration.m diff --git a/pkg/grpcx/credentials.go b/pkg/grpcx/credentials.go index a93682e26..8264fdb63 100644 --- a/pkg/grpcx/credentials.go +++ b/pkg/grpcx/credentials.go @@ -14,8 +14,8 @@ import ( // ClientTransportCredentials returns gRPC client transport credentials based on the provided configuration. func ClientTransportCredentials(log logrus.FieldLogger, cfg config.GRPCServer) (credentials.TransportCredentials, error) { - if cfg.DisableSecurity { - log.Warn("Server CA certificate is not provided. Using insecure gRPC connection...") + if cfg.DisableTransportSecurity { + log.Warn("gRPC encryption is disabled. Disabling transport security...") return insecure.NewCredentials(), nil }