Skip to content

Commit a9550f1

Browse files
authored
Replace global loggers with local loggers in tlscommon package (#334)
* Add event tag for logs from tls
1 parent 61791f1 commit a9550f1

File tree

9 files changed

+71
-44
lines changed

9 files changed

+71
-44
lines changed

transport/httpcommon/httpcommon.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,8 @@ func (settings *HTTPTransportSettings) Unpack(cfg *config.C) error {
185185
return err
186186
}
187187

188-
_, err := tlscommon.LoadTLSConfig(tmp.TLS)
188+
// TODO: use local logger here
189+
_, err := tlscommon.LoadTLSConfig(tmp.TLS, logp.NewLogger(""))
189190
if err != nil {
190191
return err
191192
}
@@ -213,6 +214,11 @@ func (settings *HTTPTransportSettings) RoundTripper(opts ...TransportOption) (ht
213214
}
214215
}
215216

217+
logger := logp.NewLogger("")
218+
if log := extra.logger; log != nil {
219+
logger = log
220+
}
221+
216222
for _, opt := range opts {
217223
if dialOpt, ok := opt.(dialerOption); ok {
218224
dialer = dialOpt.baseDialer()
@@ -223,7 +229,7 @@ func (settings *HTTPTransportSettings) RoundTripper(opts ...TransportOption) (ht
223229
dialer = transport.NetDialer(settings.Timeout)
224230
}
225231

226-
tls, err := tlscommon.LoadTLSConfig(settings.TLS)
232+
tls, err := tlscommon.LoadTLSConfig(settings.TLS, logger)
227233
if err != nil {
228234
return nil, err
229235
}

transport/tlscommon/ca_pinning_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import (
3232

3333
"github.com/elastic/elastic-agent-libs/config"
3434
"github.com/elastic/elastic-agent-libs/iobuf"
35+
"github.com/elastic/elastic-agent-libs/logp/logptest"
3536
"github.com/elastic/elastic-agent-libs/transport/tlscommontest"
3637
)
3738

@@ -51,7 +52,7 @@ func TestCAPinning(t *testing.T) {
5152
err := cfg.Unpack(config)
5253
require.NoError(t, err)
5354

54-
tlsCfg, err := LoadTLSConfig(config)
55+
tlsCfg, err := LoadTLSConfig(config, logptest.NewTestingLogger(t, ""))
5556
require.NoError(t, err)
5657

5758
tls := tlsCfg.BuildModuleClientConfig(host)
@@ -67,7 +68,7 @@ func TestCAPinning(t *testing.T) {
6768
err := cfg.Unpack(config)
6869
require.NoError(t, err)
6970

70-
tlsCfg, err := LoadTLSConfig(config)
71+
tlsCfg, err := LoadTLSConfig(config, logptest.NewTestingLogger(t, ""))
7172
require.NoError(t, err)
7273

7374
tls := tlsCfg.BuildModuleClientConfig(host)

transport/tlscommon/config.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ package tlscommon
2020
import (
2121
"crypto/tls"
2222
"errors"
23+
24+
"github.com/elastic/elastic-agent-libs/logp"
2325
)
2426

2527
// Config defines the user configurable options in the yaml file.
@@ -40,7 +42,7 @@ type Config struct {
4042
// defined. If Certificate and CertificateKey are configured, client authentication
4143
// will be configured. If no CAs are configured, the host CA will be used by go
4244
// built-in TLS support.
43-
func LoadTLSConfig(config *Config) (*TLSConfig, error) {
45+
func LoadTLSConfig(config *Config, logger *logp.Logger) (*TLSConfig, error) {
4446
if !config.IsEnabled() {
4547
return nil, nil
4648
}
@@ -86,6 +88,7 @@ func LoadTLSConfig(config *Config) (*TLSConfig, error) {
8688
Renegotiation: tls.RenegotiationSupport(config.Renegotiation),
8789
CASha256: config.CASha256,
8890
CATrustedFingerprint: config.CATrustedFingerprint,
91+
logger: logger,
8992
}, nil
9093
}
9194

transport/tlscommon/server_config.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"fmt"
2424

2525
"github.com/elastic/elastic-agent-libs/config"
26+
"github.com/elastic/elastic-agent-libs/logp"
2627
)
2728

2829
// ServerConfig defines the user configurable tls options for any TCP based service.
@@ -40,7 +41,7 @@ type ServerConfig struct {
4041

4142
// LoadTLSServerConfig tranforms a ServerConfig into a `tls.Config` to be used directly with golang
4243
// network types.
43-
func LoadTLSServerConfig(config *ServerConfig) (*TLSConfig, error) {
44+
func LoadTLSServerConfig(config *ServerConfig, logger *logp.Logger) (*TLSConfig, error) {
4445
if !config.IsEnabled() {
4546
return nil, nil
4647
}
@@ -95,6 +96,7 @@ func LoadTLSServerConfig(config *ServerConfig) (*TLSConfig, error) {
9596
CurvePreferences: curves,
9697
ClientAuth: tls.ClientAuthType(clientAuth),
9798
CASha256: config.CASha256,
99+
logger: logger,
98100
}, nil
99101
}
100102

transport/tlscommon/tls_config.go

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,8 @@ type TLSConfig struct {
8686
// time returns the current time as the number of seconds since the epoch.
8787
// If time is nil, TLS uses time.Now.
8888
time func() time.Time
89+
90+
logger *logp.Logger
8991
}
9092

9193
var (
@@ -104,7 +106,7 @@ func (c *TLSConfig) ToConfig() *tls.Config {
104106

105107
insecure := c.Verification != VerifyStrict
106108
if c.Verification == VerifyNone {
107-
logp.NewLogger("tls").Warn("SSL/TLS verifications disabled.")
109+
c.logger.Named("tls").Warn("SSL/TLS verifications disabled.")
108110
}
109111

110112
return &tls.Config{
@@ -119,7 +121,7 @@ func (c *TLSConfig) ToConfig() *tls.Config {
119121
Renegotiation: c.Renegotiation,
120122
ClientAuth: c.ClientAuth,
121123
Time: c.time,
122-
VerifyConnection: makeVerifyConnection(c),
124+
VerifyConnection: makeVerifyConnection(c, c.logger),
123125
}
124126
}
125127

@@ -133,7 +135,7 @@ func (c *TLSConfig) BuildModuleClientConfig(host string) *tls.Config {
133135
VerifyConnection: makeVerifyConnection(&TLSConfig{
134136
Verification: VerifyFull,
135137
ServerName: host,
136-
}),
138+
}, logp.NewLogger("tls")),
137139
}
138140
}
139141

@@ -175,8 +177,8 @@ func (c *TLSConfig) BuildServerConfig(host string) *tls.Config {
175177
return config
176178
}
177179

178-
func trustRootCA(cfg *TLSConfig, peerCerts []*x509.Certificate) error {
179-
logger := logp.NewLogger("tls")
180+
func trustRootCA(cfg *TLSConfig, peerCerts []*x509.Certificate, logger *logp.Logger) error {
181+
logger = logger.Named("tls")
180182
logger.Info("'ca_trusted_fingerprint' set, looking for matching fingerprints")
181183
fingerprint, err := hex.DecodeString(cfg.CATrustedFingerprint)
182184
if err != nil {
@@ -223,7 +225,7 @@ func trustRootCA(cfg *TLSConfig, peerCerts []*x509.Certificate) error {
223225
return nil
224226
}
225227

226-
func makeVerifyConnection(cfg *TLSConfig) func(tls.ConnectionState) error {
228+
func makeVerifyConnection(cfg *TLSConfig, logger *logp.Logger) func(tls.ConnectionState) error {
227229
serverName := cfg.ServerName
228230

229231
switch cfg.Verification {
@@ -233,7 +235,7 @@ func makeVerifyConnection(cfg *TLSConfig) func(tls.ConnectionState) error {
233235
// tls.Config.InsecureSkipVerify is set to true
234236
return func(cs tls.ConnectionState) error {
235237
if cfg.CATrustedFingerprint != "" {
236-
if err := trustRootCA(cfg, cs.PeerCertificates); err != nil {
238+
if err := trustRootCA(cfg, cs.PeerCertificates, logger); err != nil {
237239
return err
238240
}
239241
}
@@ -259,7 +261,7 @@ func makeVerifyConnection(cfg *TLSConfig) func(tls.ConnectionState) error {
259261
// tls.Config.InsecureSkipVerify is set to true
260262
return func(cs tls.ConnectionState) error {
261263
if cfg.CATrustedFingerprint != "" {
262-
if err := trustRootCA(cfg, cs.PeerCertificates); err != nil {
264+
if err := trustRootCA(cfg, cs.PeerCertificates, logger); err != nil {
263265
return err
264266
}
265267
}
@@ -284,7 +286,7 @@ func makeVerifyConnection(cfg *TLSConfig) func(tls.ConnectionState) error {
284286
if len(cfg.CASha256) > 0 {
285287
return func(cs tls.ConnectionState) error {
286288
if cfg.CATrustedFingerprint != "" {
287-
if err := trustRootCA(cfg, cs.PeerCertificates); err != nil {
289+
if err := trustRootCA(cfg, cs.PeerCertificates, logger); err != nil {
288290
return err
289291
}
290292
}

transport/tlscommon/tls_config_test.go

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,12 @@ import (
3030

3131
"github.com/stretchr/testify/assert"
3232
"github.com/stretchr/testify/require"
33+
"go.uber.org/zap"
34+
"go.uber.org/zap/zapcore"
35+
"go.uber.org/zap/zaptest/observer"
3336

3437
"github.com/elastic/elastic-agent-libs/logp"
38+
"github.com/elastic/elastic-agent-libs/logp/logptest"
3539
"github.com/elastic/elastic-agent-libs/transport/tlscommontest"
3640
)
3741

@@ -262,9 +266,12 @@ func TestTrustRootCA(t *testing.T) {
262266
}
263267

264268
// Capture the logs
265-
_ = logp.DevelopmentSetup(logp.ToObserverOutput())
269+
observedCore, observedLogs := observer.New(zapcore.DebugLevel)
270+
logger := logptest.NewTestingLogger(t, "test", zap.WrapCore(func(core zapcore.Core) zapcore.Core {
271+
return observedCore
272+
}))
266273

267-
err := trustRootCA(&cfg, tc.peerCerts)
274+
err := trustRootCA(&cfg, tc.peerCerts, logger)
268275
if tc.expectingError && err == nil {
269276
t.Fatal("expecting an error when calling trustRootCA")
270277
}
@@ -274,7 +281,7 @@ func TestTrustRootCA(t *testing.T) {
274281
}
275282

276283
if len(tc.expectingWarnings) > 0 {
277-
warnings := logp.ObserverLogs().FilterLevelExact(logp.WarnLevel.ZapLevel()).TakeAll()
284+
warnings := observedLogs.FilterLevelExact(logp.WarnLevel.ZapLevel()).TakeAll()
278285
if len(warnings) == 0 {
279286
t.Fatal("expecting a warning message")
280287
}
@@ -385,7 +392,7 @@ func TestMakeVerifyConnectionUsesCATrustedFingerprint(t *testing.T) {
385392
CASha256: test.CASHA256,
386393
}
387394

388-
verifier := makeVerifyConnection(cfg)
395+
verifier := makeVerifyConnection(cfg, logptest.NewTestingLogger(t, ""))
389396
if test.expectedCallback {
390397
require.NotNil(t, verifier, "makeVerifyConnection returned a nil verifier")
391398
} else {
@@ -469,7 +476,7 @@ func TestMakeVerifyServerConnectionForIPs(t *testing.T) {
469476
Verification: test.verificationMode,
470477
ServerName: test.serverName,
471478
}
472-
verifier := makeVerifyConnection(cfg)
479+
verifier := makeVerifyConnection(cfg, logptest.NewTestingLogger(t, ""))
473480

474481
err = verifier(tls.ConnectionState{
475482
PeerCertificates: []*x509.Certificate{peerCerts.Leaf},
@@ -644,6 +651,7 @@ func TestVerificationMode(t *testing.T) {
644651
Verification: test.verificationMode,
645652
RootCAs: certPool,
646653
ServerName: test.hostname,
654+
logger: logptest.NewTestingLogger(t, ""),
647655
}
648656

649657
if test.ignoreCerts {

transport/tlscommon/tls_fips_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"path/filepath"
2727
"testing"
2828

29+
"github.com/elastic/elastic-agent-libs/logp/logptest"
2930
"github.com/stretchr/testify/assert"
3031
"github.com/stretchr/testify/require"
3132
)
@@ -53,7 +54,7 @@ func TestFIPSCertificateAndKeys(t *testing.T) {
5354
cfg.Certificate.Key = string(rawKey)
5455
cfg.Certificate.Passphrase = password
5556

56-
_, err = LoadTLSConfig(cfg)
57+
_, err = LoadTLSConfig(cfg, logptest.NewTestingLogger(t, ""))
5758
require.Error(t, err)
5859
assert.ErrorIs(t, err, errors.ErrUnsupported, err)
5960
})
@@ -68,7 +69,7 @@ func TestFIPSCertificateAndKeys(t *testing.T) {
6869
cfg.Certificate.Key = key
6970
cfg.Certificate.Passphrase = password
7071

71-
_, err = LoadTLSConfig(cfg)
72+
_, err = LoadTLSConfig(cfg, logptest.NewTestingLogger(t, ""))
7273
assert.ErrorIs(t, err, errors.ErrUnsupported)
7374
})
7475
}

transport/tlscommon/tls_nofips_test.go

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"path/filepath"
2727
"testing"
2828

29+
"github.com/elastic/elastic-agent-libs/logp/logptest"
2930
"github.com/stretchr/testify/assert"
3031
"github.com/stretchr/testify/require"
3132
)
@@ -52,7 +53,7 @@ func TestNoFIPSCertificateAndKeys(t *testing.T) {
5253
cfg.Certificate.Key = string(rawKey)
5354
cfg.Certificate.Passphrase = password
5455

55-
tlsC, err := LoadTLSConfig(cfg)
56+
tlsC, err := LoadTLSConfig(cfg, logptest.NewTestingLogger(t, ""))
5657
require.NoError(t, err)
5758
assert.NotNil(t, tlsC)
5859
})
@@ -67,20 +68,21 @@ func TestNoFIPSCertificateAndKeys(t *testing.T) {
6768
cfg.Certificate.Key = key
6869
cfg.Certificate.Passphrase = password
6970

70-
tlsC, err := LoadTLSConfig(cfg)
71+
tlsC, err := LoadTLSConfig(cfg, logptest.NewTestingLogger(t, ""))
7172
require.NoError(t, err)
7273
assert.NotNil(t, tlsC)
7374
})
7475
}
7576

7677
func TestEncryptedKeyPassphrase(t *testing.T) {
7778
const passphrase = "Abcd1234!" // passphrase for testdata/ca.encrypted.key
79+
logger := logptest.NewTestingLogger(t, "")
7880
t.Run("no passphrase", func(t *testing.T) {
7981
_, err := LoadTLSConfig(mustLoad(t, `
8082
enabled: true
8183
certificate: testdata/ca.crt
8284
key: testdata/ca.encrypted.key
83-
`))
85+
`), logger)
8486
assert.ErrorContains(t, err, "no PEM blocks") // ReadPEMFile will generate an internal "no passphrase available" error that is logged and the no PEM blocks error is returned instead
8587
})
8688

@@ -90,7 +92,7 @@ func TestEncryptedKeyPassphrase(t *testing.T) {
9092
certificate: testdata/ca.crt
9193
key: testdata/ca.encrypted.key
9294
key_passphrase: "abcd1234!"
93-
`))
95+
`), logger)
9496
assert.ErrorContains(t, err, "no PEM blocks") // ReadPEMFile will fail decrypting with x509.IncorrectPasswordError that will be logged and a no PEM blocks error is returned instead
9597
})
9698

@@ -100,7 +102,7 @@ func TestEncryptedKeyPassphrase(t *testing.T) {
100102
certificate: testdata/ca.crt
101103
key: testdata/ca.encrypted.key
102104
key_passphrase: Abcd1234!
103-
`))
105+
`), logger)
104106
require.NoError(t, err)
105107
assert.Equal(t, 1, len(cfg.Certificates), "expected 1 certificate to be loaded")
106108
})
@@ -112,7 +114,7 @@ func TestEncryptedKeyPassphrase(t *testing.T) {
112114
certificate: testdata/ca.crt
113115
key: testdata/ca.encrypted.key
114116
key_passphrase_path: %s
115-
`, fileName)))
117+
`, fileName)), logger)
116118
require.NoError(t, err)
117119
assert.Equal(t, 1, len(cfg.Certificates), "expected 1 certificate to be loaded")
118120
})
@@ -124,7 +126,7 @@ func TestEncryptedKeyPassphrase(t *testing.T) {
124126
certificate: testdata/ca.crt
125127
key: testdata/ca.encrypted.key
126128
key_passphrase_path: %s
127-
`, fileName)))
129+
`, fileName)), logger)
128130
assert.ErrorContains(t, err, "no PEM blocks") // ReadPEMFile will generate an internal "no passphrase available" error that is logged and the no PEM blocks error is returned instead
129131
})
130132
}

0 commit comments

Comments
 (0)