Skip to content

Commit

Permalink
Add TLS Server Handshake Failures Metrics (#239)
Browse files Browse the repository at this point in the history 
* record tls failure metric

* update to latest otel pkg

* record handshake failure in wrappedtransportcredentials

* remove useless code

* add client handshake failure metric
  • Loading branch information
@sfc-gh-elinardi
sfc-gh-elinardi authored May 16, 2023
1 parent 443cde9 commit 8d55d1e
Show file tree
Hide file tree
Showing 9 changed files with 54 additions and 26 deletions.
3 changes: 3 additions & 0 deletions auth/mtls/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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
Expand All @@ -44,6 +46,7 @@ func LoadClientCredentials(ctx context.Context, loaderName string) (credentials.
loader: internalLoadClientCredentials,
mtlsLoader: mtlsLoader,
logger: logger,
recorder: recorder,
}
return wrapped, nil
}
Expand Down
24 changes: 22 additions & 2 deletions auth/mtls/mtls.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
3 changes: 3 additions & 0 deletions auth/mtls/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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
Expand All @@ -47,6 +49,7 @@ func LoadServerCredentials(ctx context.Context, loaderName string) (credentials.
loader: internalLoadServerCredentials,
mtlsLoader: mtlsLoader,
logger: logger,
recorder: recorder,
}
return wrapped, nil
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/proxy-server/server/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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...),
Expand Down
2 changes: 1 addition & 1 deletion cmd/sansshell-server/server/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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...),
Expand Down
8 changes: 4 additions & 4 deletions go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
15 changes: 8 additions & 7 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand All @@ -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=
Expand Down Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand Down
15 changes: 8 additions & 7 deletions telemetry/metrics/otel.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
8 changes: 4 additions & 4 deletions telemetry/metrics/otel_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

0 comments on commit 8d55d1e

Please sign in to comment.