From 8d55d1e4b69cbe80304acb9134a754508cd203f0 Mon Sep 17 00:00:00 2001 From: Edbert Linardi Date: Mon, 15 May 2023 21:15:07 -0700 Subject: [PATCH] Add TLS Server Handshake Failures Metrics (#239) * record tls failure metric * update to latest otel pkg * record handshake failure in wrappedtransportcredentials * remove useless code * add client handshake failure metric --- auth/mtls/client.go | 3 +++ auth/mtls/mtls.go | 24 ++++++++++++++++++++++-- auth/mtls/server.go | 3 +++ cmd/proxy-server/server/server.go | 2 +- cmd/sansshell-server/server/server.go | 2 +- go.mod | 8 ++++---- go.sum | 15 ++++++++------- telemetry/metrics/otel.go | 15 ++++++++------- telemetry/metrics/otel_test.go | 8 ++++---- 9 files changed, 54 insertions(+), 26 deletions(-) diff --git a/auth/mtls/client.go b/auth/mtls/client.go index 3f9e6549..a0ad745d 100644 --- a/auth/mtls/client.go +++ b/auth/mtls/client.go @@ -22,6 +22,7 @@ import ( "crypto/x509" "fmt" + "github.com/Snowflake-Labs/sansshell/telemetry/metrics" "github.com/go-logr/logr" "google.golang.org/grpc/credentials" ) @@ -30,6 +31,7 @@ import ( // based on the provided `loaderName` func LoadClientCredentials(ctx context.Context, loaderName string) (credentials.TransportCredentials, error) { logger := logr.FromContextOrDiscard(ctx) + recorder := metrics.RecorderFromContextOrNoop(ctx) mtlsLoader, err := Loader(loaderName) if err != nil { return nil, err @@ -44,6 +46,7 @@ func LoadClientCredentials(ctx context.Context, loaderName string) (credentials. loader: internalLoadClientCredentials, mtlsLoader: mtlsLoader, logger: logger, + recorder: recorder, } return wrapped, nil } diff --git a/auth/mtls/mtls.go b/auth/mtls/mtls.go index f4af82a1..3ca7094b 100644 --- a/auth/mtls/mtls.go +++ b/auth/mtls/mtls.go @@ -27,10 +27,21 @@ import ( "sort" "sync" + "github.com/Snowflake-Labs/sansshell/telemetry/metrics" "github.com/go-logr/logr" "google.golang.org/grpc/credentials" ) +// Metrics +var ( + clientHandshakeFailureMetrics = metrics.MetricDefinition{ + Name: "mtls_client_handshake_failure", Description: "failure during mtls client handshake", + } + serverHandshakeFailureMetrics = metrics.MetricDefinition{ + Name: "mtls_server_handshake_failure", Description: "failure during mtls server handshake", + } +) + var ( loaderMu sync.RWMutex loaders = make(map[string]CredentialsLoader) @@ -82,6 +93,7 @@ type WrappedTransportCredentials struct { mtlsLoader CredentialsLoader loader func(context.Context, string) (credentials.TransportCredentials, error) logger logr.Logger + recorder metrics.MetricsRecorder } func (w *WrappedTransportCredentials) checkRefresh() error { @@ -113,7 +125,11 @@ func (w *WrappedTransportCredentials) ClientHandshake(ctx context.Context, s str } w.mu.RLock() defer w.mu.RUnlock() - return w.creds.ClientHandshake(ctx, s, n) + conn, info, err := w.creds.ClientHandshake(ctx, s, n) + if err != nil && w.recorder != nil { + w.recorder.CounterOrLog(context.Background(), clientHandshakeFailureMetrics, 1) + } + return conn, info, err } // ServerHandshake -- see credentials.ServerHandshake @@ -123,7 +139,11 @@ func (w *WrappedTransportCredentials) ServerHandshake(n net.Conn) (net.Conn, cre } w.mu.RLock() defer w.mu.RUnlock() - return w.creds.ServerHandshake(n) + conn, info, err := w.creds.ServerHandshake(n) + if err != nil && w.recorder != nil { + w.recorder.CounterOrLog(context.Background(), serverHandshakeFailureMetrics, 1) + } + return conn, info, err } // Info -- see credentials.Info diff --git a/auth/mtls/server.go b/auth/mtls/server.go index 8d237905..416856dc 100644 --- a/auth/mtls/server.go +++ b/auth/mtls/server.go @@ -22,6 +22,7 @@ import ( "crypto/x509" "fmt" + "github.com/Snowflake-Labs/sansshell/telemetry/metrics" "github.com/go-logr/logr" "google.golang.org/grpc/credentials" ) @@ -33,6 +34,7 @@ import ( // will check at call time if new certificates are available. func LoadServerCredentials(ctx context.Context, loaderName string) (credentials.TransportCredentials, error) { logger := logr.FromContextOrDiscard(ctx) + recorder := metrics.RecorderFromContextOrNoop(ctx) mtlsLoader, err := Loader(loaderName) if err != nil { return nil, err @@ -47,6 +49,7 @@ func LoadServerCredentials(ctx context.Context, loaderName string) (credentials. loader: internalLoadServerCredentials, mtlsLoader: mtlsLoader, logger: logger, + recorder: recorder, } return wrapped, nil } diff --git a/cmd/proxy-server/server/server.go b/cmd/proxy-server/server/server.go index c9800ac9..2df08b0d 100644 --- a/cmd/proxy-server/server/server.go +++ b/cmd/proxy-server/server/server.go @@ -323,7 +323,7 @@ func WithMetricsPort(addr string) Option { func WithOtelTracing(interceptorOpts ...otelgrpc.Option) Option { return optionFunc(func(_ context.Context, r *runState) error { interceptorOpts = append(interceptorOpts, - otelgrpc.WithMeterProvider(noop.NewMeterProvider()), // We don't want otel grpc metrics so discard them + otelgrpc.WithMeterProvider(noop.MeterProvider{}), // We don't want otel grpc metrics so discard them ) r.unaryClientInterceptors = append(r.unaryClientInterceptors, otelgrpc.UnaryClientInterceptor(interceptorOpts...), diff --git a/cmd/sansshell-server/server/server.go b/cmd/sansshell-server/server/server.go index b02c6170..70218932 100644 --- a/cmd/sansshell-server/server/server.go +++ b/cmd/sansshell-server/server/server.go @@ -262,7 +262,7 @@ func WithMetricsPort(addr string) Option { func WithOtelTracing(interceptorOpts ...otelgrpc.Option) Option { return optionFunc(func(_ context.Context, r *runState) error { interceptorOpts = append(interceptorOpts, - otelgrpc.WithMeterProvider(noop.NewMeterProvider()), // We don't want otel grpc metrics so discard them + otelgrpc.WithMeterProvider(noop.MeterProvider{}), // We don't want otel grpc metrics so discard them ) r.unaryInterceptors = append(r.unaryInterceptors, otelgrpc.UnaryServerInterceptor(interceptorOpts...), diff --git a/go.mod b/go.mod index 56d5a1fe..d9114f8b 100644 --- a/go.mod +++ b/go.mod @@ -79,8 +79,8 @@ require ( github.com/kylelemons/godebug v1.1.0 // indirect github.com/matttproud/golang_protobuf_extensions v1.0.4 // indirect github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8 // indirect - github.com/prometheus/client_model v0.3.0 // indirect - github.com/prometheus/common v0.42.0 // indirect + github.com/prometheus/client_model v0.4.0 // indirect + github.com/prometheus/common v0.43.0 // indirect github.com/prometheus/procfs v0.9.0 // indirect github.com/rcrowley/go-metrics v0.0.0-20201227073835-cf1acfcdf475 // indirect github.com/tchap/go-patricia/v2 v2.3.1 // indirect @@ -90,8 +90,8 @@ require ( go.opencensus.io v0.24.0 // indirect go.opentelemetry.io/otel/sdk v1.15.1 // indirect golang.org/x/crypto v0.6.0 // indirect - golang.org/x/net v0.9.0 // indirect - golang.org/x/oauth2 v0.6.0 // indirect + golang.org/x/net v0.10.0 // indirect + golang.org/x/oauth2 v0.7.0 // indirect golang.org/x/text v0.9.0 // indirect golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 // indirect google.golang.org/api v0.114.0 // indirect diff --git a/go.sum b/go.sum index 5501d162..511ceade 100644 --- a/go.sum +++ b/go.sum @@ -1633,8 +1633,9 @@ github.com/prometheus/client_model v0.0.0-20180712105110-5c3871d89910/go.mod h1: github.com/prometheus/client_model v0.0.0-20190129233127-fd36f4220a90/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA= github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA= github.com/prometheus/client_model v0.2.0/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA= -github.com/prometheus/client_model v0.3.0 h1:UBgGFHqYdG/TPFD1B1ogZywDqEkwp3fBMvqdiQ7Xew4= github.com/prometheus/client_model v0.3.0/go.mod h1:LDGWKZIo7rky3hgvBe+caln+Dr3dPggB5dvjtD7w9+w= +github.com/prometheus/client_model v0.4.0 h1:5lQXD3cAg1OXBf4Wq03gTrXHeaV0TQvGfUooCfx1yqY= +github.com/prometheus/client_model v0.4.0/go.mod h1:oMQmHW1/JoDwqLtg57MGgP/Fb1CJEYF2imWWhWtMkYU= github.com/prometheus/common v0.0.0-20180110214958-89604d197083/go.mod h1:daVV7qP5qjZbuso7PdcryaAu0sAZbrN9i7WWcTMWvro= github.com/prometheus/common v0.0.0-20181113130724-41aa239b4cce/go.mod h1:daVV7qP5qjZbuso7PdcryaAu0sAZbrN9i7WWcTMWvro= github.com/prometheus/common v0.4.0/go.mod h1:TNfzLD0ON7rHzMJeJkieUDPYmFC7Snx/y86RQel1bk4= @@ -1650,8 +1651,8 @@ github.com/prometheus/common v0.34.0/go.mod h1:gB3sOl7P0TvJabZpLY5uQMpUqRCPPCyRL github.com/prometheus/common v0.37.0/go.mod h1:phzohg0JFMnBEFGxTDbfu3QyL5GI8gTQJFhYO5B3mfA= github.com/prometheus/common v0.38.0/go.mod h1:MBXfmBQZrK5XpbCkjofnXs96LD2QQ7fEq4C0xjC/yec= github.com/prometheus/common v0.39.0/go.mod h1:6XBZ7lYdLCbkAVhwRsWTZn+IN5AB9F/NXd5w0BbEX0Y= -github.com/prometheus/common v0.42.0 h1:EKsfXEYo4JpWMHH5cg+KOUWeuJSov1Id8zGR8eeI1YM= -github.com/prometheus/common v0.42.0/go.mod h1:xBwqVerjNdUDjgODMpudtOMwlOwf2SaTr1yjz4b7Zbc= +github.com/prometheus/common v0.43.0 h1:iq+BVjvYLei5f27wiuNiB1DN6DYQkp1c8Bx0Vykh5us= +github.com/prometheus/common v0.43.0/go.mod h1:NCvr5cQIh3Y/gy73/RdVtC9r8xxrxwJnB+2lB3BxrFc= github.com/prometheus/common/assets v0.1.0/go.mod h1:D17UVUE12bHbim7HzwUvtqm6gwBEaDQ0F+hIGbFbccI= github.com/prometheus/common/assets v0.2.0/go.mod h1:D17UVUE12bHbim7HzwUvtqm6gwBEaDQ0F+hIGbFbccI= github.com/prometheus/common/sigv4 v0.1.0/go.mod h1:2Jkxxk9yYvCkE5G1sQT7GuEXm57JrvHu9k5YwTjsNtI= @@ -2137,8 +2138,8 @@ golang.org/x/net v0.3.1-0.20221206200815-1e63c2f08a10/go.mod h1:MBQ8lrhLObU/6UmL golang.org/x/net v0.4.0/go.mod h1:MBQ8lrhLObU/6UmLb4fmbmk5OcyYmqtbGd/9yIeKjEE= golang.org/x/net v0.5.0/go.mod h1:DivGGAXEgPSlEBzxGzZI+ZLohi+xUj054jfeKui00ws= golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= -golang.org/x/net v0.9.0 h1:aWJ/m6xSmxWBx+V0XRHTlrYrPG56jKsLdTFmsSsCzOM= -golang.org/x/net v0.9.0/go.mod h1:d48xBJpPfHeWQsugry2m+kC02ZBRGRgulfHnEXEuWns= +golang.org/x/net v0.10.0 h1:X2//UzNDwYmtCLn7To6G58Wr6f5ahEAQgKNzv9Y951M= +golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= @@ -2171,8 +2172,8 @@ golang.org/x/oauth2 v0.0.0-20221014153046-6fdb5e3db783/go.mod h1:h4gKUeWbJ4rQPri golang.org/x/oauth2 v0.3.0/go.mod h1:rQrIauxkUhJ6CuwEXwymO2/eh4xz2ZWF1nBkcxS+tGk= golang.org/x/oauth2 v0.4.0/go.mod h1:RznEsdpjGAINPTOF0UH/t+xJ75L18YO3Ho6Pyn+uRec= golang.org/x/oauth2 v0.5.0/go.mod h1:9/XBHVqLaWO3/BRHs5jbpYCnOZVjj5V0ndyaAM7KB4I= -golang.org/x/oauth2 v0.6.0 h1:Lh8GPgSKBfWSwFvtuWOfeI3aAAnbXTSutYxJiOJFgIw= -golang.org/x/oauth2 v0.6.0/go.mod h1:ycmewcwgD4Rpr3eZJLSB4Kyyljb3qDh40vJ8STE5HKw= +golang.org/x/oauth2 v0.7.0 h1:qe6s0zUXlPX80/dITx3440hWZ7GwMwgDDyrSGTPJG/g= +golang.org/x/oauth2 v0.7.0/go.mod h1:hPLQkd9LyjfXTiRohC/41GhcFqxisoUQ99sCUOHO9x4= golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181108010431-42b317875d0f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= diff --git a/telemetry/metrics/otel.go b/telemetry/metrics/otel.go index c712b790..55f0a9b4 100644 --- a/telemetry/metrics/otel.go +++ b/telemetry/metrics/otel.go @@ -83,20 +83,21 @@ func NewOtelRecorder(meter metric.Meter, opts ...Option) (*OtelRecorder, error) } // Counter registers the counter if it's not registered, then increments it -func (m *OtelRecorder) Counter(ctx context.Context, metric MetricDefinition, value int64, attributes ...attribute.KeyValue) error { - metric.Name = addPrefix(m.prefix, metric.Name) - if counter, exists := m.Int64Counters.Load(metric.Name); exists { - counter.(ometric.Int64Counter).Add(ctx, value, ometric.WithAttributes(attributes...)) +func (m *OtelRecorder) Counter(ctx context.Context, definition MetricDefinition, value int64, attributes ...attribute.KeyValue) error { + definition.Name = addPrefix(m.prefix, definition.Name) + options := metric.WithAttributes(attributes...) + if counter, exists := m.Int64Counters.Load(definition.Name); exists { + counter.(ometric.Int64Counter).Add(ctx, value, options) return nil } - counter, err := m.Meter.Int64Counter(metric.Name, ometric.WithDescription(metric.Description)) + counter, err := m.Meter.Int64Counter(definition.Name, ometric.WithDescription(definition.Description)) if err != nil { return errors.Wrap(err, "failed to create Int64counter") } - counter.Add(ctx, value, ometric.WithAttributes(attributes...)) + counter.Add(ctx, value, options) - m.Int64Counters.Store(metric.Name, counter) + m.Int64Counters.Store(definition.Name, counter) return nil } diff --git a/telemetry/metrics/otel_test.go b/telemetry/metrics/otel_test.go index 1aaa710b..714ea7b0 100644 --- a/telemetry/metrics/otel_test.go +++ b/telemetry/metrics/otel_test.go @@ -79,16 +79,16 @@ func TestRegisterInt64Gauge(t *testing.T) { ctx := context.Background() diskUsage := int64(50) // recording a gauge for the first time shouldn't return an error - errRegister := recorder.Gauge(ctx, gaugeDef, func(_ context.Context, obsrv metric.Int64Observer) error { + errRegister := recorder.Gauge(ctx, gaugeDef, metric.Int64Callback(func(_ context.Context, obsrv metric.Int64Observer) error { obsrv.Observe(diskUsage) return nil - }) + })) testutil.FatalOnErr("unexpected err on RegisterInt64Gauge", errRegister, t) // recording a gauge subsequently shouldn't return an error - errRegister = recorder.Gauge(ctx, gaugeDef, func(_ context.Context, obsrv metric.Int64Observer) error { + errRegister = recorder.Gauge(ctx, gaugeDef, metric.Int64Callback(func(_ context.Context, obsrv metric.Int64Observer) error { obsrv.Observe(0) return nil - }) + })) testutil.FatalOnErr("unexpected err on RegisterInt64Counter", errRegister, t) }