From c3ad63e03c385d90cb5229715b52fd659bab97c4 Mon Sep 17 00:00:00 2001 From: johnabass Date: Thu, 21 Nov 2024 16:26:32 -0800 Subject: [PATCH 1/2] moved zap-x509 integration to its own package; log individual certificate verification errors --- token/claimBuilder.go | 9 +++++++++ xhttp/xhttpserver/server.go | 3 ++- xzap/doc.go | 7 +++++++ {xhttp/xhttpserver => xzap}/zap.go | 14 ++++++++++++-- 4 files changed, 30 insertions(+), 3 deletions(-) create mode 100644 xzap/doc.go rename {xhttp/xhttpserver => xzap}/zap.go (86%) diff --git a/token/claimBuilder.go b/token/claimBuilder.go index 33e3578..b7f1691 100644 --- a/token/claimBuilder.go +++ b/token/claimBuilder.go @@ -14,6 +14,8 @@ import ( "github.com/xmidt-org/themis/random" "github.com/xmidt-org/themis/xhttp/xhttpclient" "github.com/xmidt-org/themis/xhttp/xhttpserver" + "github.com/xmidt-org/themis/xzap" + "go.uber.org/zap" "github.com/go-kit/kit/endpoint" kithttp "github.com/go-kit/kit/transport/http" @@ -238,6 +240,13 @@ func (cb *clientCertificateClaimBuilder) AddClaims(_ context.Context, r *Request } _, verifyErr := pc.Verify(vo) + if verifyErr != nil { + r.Logger.Warn( + "certificate verification failed", + xzap.Certificate("cert", pc), + zap.Error(verifyErr), + ) + } switch { case expired && verifyErr != nil: diff --git a/xhttp/xhttpserver/server.go b/xhttp/xhttpserver/server.go index 02cd1fa..42755f2 100644 --- a/xhttp/xhttpserver/server.go +++ b/xhttp/xhttpserver/server.go @@ -10,6 +10,7 @@ import ( "github.com/xmidt-org/sallust" "github.com/xmidt-org/sallust/sallusthttp" + "github.com/xmidt-org/themis/xzap" "go.uber.org/zap" "github.com/justinas/alice" @@ -59,7 +60,7 @@ func NewServerChain(o Options, l *zap.Logger, fbs ...sallusthttp.FieldBuilder) a return l.With(zap.String("userAgent", r.UserAgent())) }) bs.Add(func(r *http.Request, l *zap.Logger) *zap.Logger { - return l.With(connectionStateField("state", r.TLS)) + return l.With(xzap.ConnectionState("state", r.TLS)) }) chain := alice.New( diff --git a/xzap/doc.go b/xzap/doc.go new file mode 100644 index 0000000..26bf5e8 --- /dev/null +++ b/xzap/doc.go @@ -0,0 +1,7 @@ +// SPDX-FileCopyrightText: 2019 Comcast Cable Communications Management, LLC +// SPDX-License-Identifier: Apache-2.0 + +/* +Package xzap provides some zap logging integrations. +*/ +package xzap diff --git a/xhttp/xhttpserver/zap.go b/xzap/zap.go similarity index 86% rename from xhttp/xhttpserver/zap.go rename to xzap/zap.go index 7cac1f7..8610631 100644 --- a/xhttp/xhttpserver/zap.go +++ b/xzap/zap.go @@ -1,6 +1,6 @@ // SPDX-FileCopyrightText: 2017 Comcast Cable Communications Management, LLC // SPDX-License-Identifier: Apache-2.0 -package xhttpserver +package xzap import ( "crypto/tls" @@ -53,6 +53,14 @@ func (c certificate) MarshalLogObject(enc zapcore.ObjectEncoder) error { return nil } +func Certificate(field string, cert *x509.Certificate) zap.Field { + if cert != nil { + return zap.Object(field, certificate(*cert)) + } else { + return zap.Skip() + } +} + type certificates []*x509.Certificate func (cs certificates) MarshalLogArray(enc zapcore.ArrayEncoder) error { @@ -96,7 +104,9 @@ func (cstate connectionState) MarshalLogObject(enc zapcore.ObjectEncoder) error return nil } -func connectionStateField(field string, v *tls.ConnectionState) zap.Field { +// ConnectionState produces a zap logging Field that produces an object representation +// of a TLS connection state. +func ConnectionState(field string, v *tls.ConnectionState) zap.Field { if v != nil { return zap.Object(field, connectionState(*v)) } else { From 039fc7b53464648ccc8a5fabb6fb6c00b0de47dd Mon Sep 17 00:00:00 2001 From: johnabass Date: Thu, 21 Nov 2024 16:27:40 -0800 Subject: [PATCH 2/2] reordered fields for more easy reading --- token/claimBuilder.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/token/claimBuilder.go b/token/claimBuilder.go index b7f1691..241fd86 100644 --- a/token/claimBuilder.go +++ b/token/claimBuilder.go @@ -243,8 +243,8 @@ func (cb *clientCertificateClaimBuilder) AddClaims(_ context.Context, r *Request if verifyErr != nil { r.Logger.Warn( "certificate verification failed", - xzap.Certificate("cert", pc), zap.Error(verifyErr), + xzap.Certificate("cert", pc), ) }