From dd4c5fa3fc2d309619ba1b79cdc598054b56bfef Mon Sep 17 00:00:00 2001 From: Denis Mishin Date: Sun, 1 Jan 2023 18:08:22 -0500 Subject: [PATCH 1/2] make grpc_insecure an optional bool --- authorize/grpc.go | 17 ++++++++++++----- config/options.go | 10 +++++++--- internal/controlplane/events_test.go | 3 ++- internal/databroker/config_source_test.go | 3 ++- 4 files changed, 23 insertions(+), 10 deletions(-) diff --git a/authorize/grpc.go b/authorize/grpc.go index 4a785c23f2c..13f8406efdc 100644 --- a/authorize/grpc.go +++ b/authorize/grpc.go @@ -22,7 +22,9 @@ import ( ) // Check implements the envoy auth server gRPC endpoint. -func (a *Authorize) Check(ctx context.Context, in *envoy_service_auth_v3.CheckRequest) (out *envoy_service_auth_v3.CheckResponse, err error) { +func (a *Authorize) Check(ctx context.Context, in *envoy_service_auth_v3.CheckRequest) (*envoy_service_auth_v3.CheckResponse, error) { + log.Info(ctx).Msg("grpc check ext_authz BEGIN") + ctx, span := trace.StartSpan(ctx, "authorize.grpc.Check") defer span.End() @@ -47,6 +49,7 @@ func (a *Authorize) Check(ctx context.Context, in *envoy_service_auth_v3.CheckRe var s sessionOrServiceAccount var u *user.User + var err error if sessionState != nil { s, err = a.getDataBrokerSessionOrServiceAccount(ctx, sessionState.ID, sessionState.DatabrokerRecordVersion) if err != nil { @@ -72,16 +75,20 @@ func (a *Authorize) Check(ctx context.Context, in *envoy_service_auth_v3.CheckRe log.Error(ctx).Err(err).Msg("error during OPA evaluation") return nil, err } - defer func() { - a.logAuthorizeCheck(ctx, in, out, res, s, u) - }() // if show error details is enabled, attach the policy evaluation traces if req.Policy != nil && req.Policy.ShowErrorDetails { ctx = contextutil.WithPolicyEvaluationTraces(ctx, res.Traces) } - return a.handleResult(ctx, in, req, res) + resp, err := a.handleResult(ctx, in, req, res) + if err != nil { + log.Error(ctx).Err(err).Str("request-id", requestid.FromContext(ctx)).Msg("grpc check ext_authz_error") + } else { + log.Info(ctx).Msg("grpc check ext_authz") + } + a.logAuthorizeCheck(ctx, in, resp, res, s, u) + return resp, err } func (a *Authorize) getEvaluatorRequestFromCheckRequest( diff --git a/config/options.go b/config/options.go index 8aba5076c2e..159bf4f09a2 100644 --- a/config/options.go +++ b/config/options.go @@ -18,6 +18,7 @@ import ( "github.com/rs/zerolog" "github.com/spf13/viper" "github.com/volatiletech/null/v9" + "google.golang.org/protobuf/proto" "github.com/pomerium/pomerium/internal/atomicutil" "github.com/pomerium/pomerium/internal/hashutil" @@ -217,7 +218,7 @@ type Options struct { // GRPCInsecure disables transport security. // If running in all-in-one mode, defaults to true. - GRPCInsecure bool `mapstructure:"grpc_insecure" yaml:"grpc_insecure,omitempty"` + GRPCInsecure *bool `mapstructure:"grpc_insecure" yaml:"grpc_insecure,omitempty"` GRPCClientTimeout time.Duration `mapstructure:"grpc_client_timeout" yaml:"grpc_client_timeout,omitempty"` GRPCClientDNSRoundRobin bool `mapstructure:"grpc_client_dns_roundrobin" yaml:"grpc_client_dns_roundrobin,omitempty"` @@ -819,10 +820,13 @@ func (o *Options) GetGRPCAddr() string { // GetGRPCInsecure gets whether or not gRPC is insecure. func (o *Options) GetGRPCInsecure() bool { + if o.GRPCInsecure != nil { + return *o.GRPCInsecure + } if IsAll(o.Services) { return true } - return o.GRPCInsecure + return false } // GetSignOutRedirectURL gets the SignOutRedirectURL. @@ -1457,7 +1461,7 @@ func (o *Options) ApplySettings(ctx context.Context, settings *config.Settings) o.GRPCAddr = settings.GetGrpcAddress() } if settings.GrpcInsecure != nil { - o.GRPCInsecure = settings.GetGrpcInsecure() + o.GRPCInsecure = proto.Bool(settings.GetGrpcInsecure()) } if len(settings.DatabrokerServiceUrls) > 0 { o.DataBrokerURLStrings = settings.GetDatabrokerServiceUrls() diff --git a/internal/controlplane/events_test.go b/internal/controlplane/events_test.go index d4b121c3dd4..90ebcbfa3fe 100644 --- a/internal/controlplane/events_test.go +++ b/internal/controlplane/events_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/require" "golang.org/x/sync/errgroup" "google.golang.org/grpc" + "google.golang.org/protobuf/proto" "github.com/pomerium/pomerium/config" "github.com/pomerium/pomerium/internal/atomicutil" @@ -80,7 +81,7 @@ func TestEvents(t *testing.T) { Options: &config.Options{ SharedKey: cryptutil.NewBase64Key(), DataBrokerURLString: "http://" + li.Addr().String(), - GRPCInsecure: true, + GRPCInsecure: proto.Bool(true), }, }, }), diff --git a/internal/databroker/config_source_test.go b/internal/databroker/config_source_test.go index a83d9352844..e9b22fccaaa 100644 --- a/internal/databroker/config_source_test.go +++ b/internal/databroker/config_source_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/assert" "google.golang.org/grpc" + "google.golang.org/protobuf/proto" "github.com/pomerium/pomerium/config" configpb "github.com/pomerium/pomerium/pkg/grpc/config" @@ -38,7 +39,7 @@ func TestConfigSource(t *testing.T) { base := config.NewDefaultOptions() base.DataBrokerURLString = "http://" + li.Addr().String() base.InsecureServer = true - base.GRPCInsecure = true + base.GRPCInsecure = proto.Bool(true) base.Policies = append(base.Policies, config.Policy{ From: "https://pomerium.io", To: config.WeightedURLs{ {URL: *u}, From 227bba2582343108fe080d1de59035db9c44f09d Mon Sep 17 00:00:00 2001 From: Denis Mishin Date: Sun, 1 Jan 2023 22:35:43 -0500 Subject: [PATCH 2/2] use internal addresses for all in one databroker and tls --- authorize/grpc.go | 4 ---- config/envoyconfig/clusters.go | 25 +++++++++++++++---------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/authorize/grpc.go b/authorize/grpc.go index 13f8406efdc..2496ef84a75 100644 --- a/authorize/grpc.go +++ b/authorize/grpc.go @@ -23,8 +23,6 @@ import ( // Check implements the envoy auth server gRPC endpoint. func (a *Authorize) Check(ctx context.Context, in *envoy_service_auth_v3.CheckRequest) (*envoy_service_auth_v3.CheckResponse, error) { - log.Info(ctx).Msg("grpc check ext_authz BEGIN") - ctx, span := trace.StartSpan(ctx, "authorize.grpc.Check") defer span.End() @@ -84,8 +82,6 @@ func (a *Authorize) Check(ctx context.Context, in *envoy_service_auth_v3.CheckRe resp, err := a.handleResult(ctx, in, req, res) if err != nil { log.Error(ctx).Err(err).Str("request-id", requestid.FromContext(ctx)).Msg("grpc check ext_authz_error") - } else { - log.Info(ctx).Msg("grpc check ext_authz") } a.logAuthorizeCheck(ctx, in, resp, res, s, u) return resp, err diff --git a/config/envoyconfig/clusters.go b/config/envoyconfig/clusters.go index 6c96e93cda3..44f08cbf4be 100644 --- a/config/envoyconfig/clusters.go +++ b/config/envoyconfig/clusters.go @@ -25,10 +25,10 @@ import ( // BuildClusters builds envoy clusters from the given config. func (b *Builder) BuildClusters(ctx context.Context, cfg *config.Config) ([]*envoy_config_cluster_v3.Cluster, error) { - grpcURL := &url.URL{ + grpcURLs := []*url.URL{{ Scheme: "http", Host: b.localGRPCAddress, - } + }} httpURL := &url.URL{ Scheme: "http", Host: b.localHTTPAddress, @@ -37,16 +37,21 @@ func (b *Builder) BuildClusters(ctx context.Context, cfg *config.Config) ([]*env Scheme: "http", Host: b.localMetricsAddress, } - authorizeURLs, err := cfg.Options.GetInternalAuthorizeURLs() - if err != nil { - return nil, err - } - databrokerURLs, err := cfg.Options.GetDataBrokerURLs() - if err != nil { - return nil, err + + authorizeURLs, databrokerURLs := grpcURLs, grpcURLs + if !config.IsAll(cfg.Options.Services) { + var err error + authorizeURLs, err = cfg.Options.GetInternalAuthorizeURLs() + if err != nil { + return nil, err + } + databrokerURLs, err = cfg.Options.GetDataBrokerURLs() + if err != nil { + return nil, err + } } - controlGRPC, err := b.buildInternalCluster(ctx, cfg.Options, "pomerium-control-plane-grpc", []*url.URL{grpcURL}, upstreamProtocolHTTP2) + controlGRPC, err := b.buildInternalCluster(ctx, cfg.Options, "pomerium-control-plane-grpc", grpcURLs, upstreamProtocolHTTP2) if err != nil { return nil, err }