Skip to content

Commit 1859885

Browse files
committed
fix: invalid TLS CA cert should error immediately
Previously if a consumer specified the CA cert to verify the command connection but that CA wasn't valid then system would log at Debug (default hidden) and proceed anyways. I don't believe this is good behavior. If the consumer is directly specifying a CA cert then that is the CA that should be used, not silently ignored. This patch returns the error up, which is now caught and swallowed at a higher level, but at least it is more visible: > time=2025-05-21T15:41:33.547Z level=ERROR msg="Unable to add transport credentials to gRPC dial options, adding default transport credentials" error="invalid CA cert while building transport credentials: read CA file (/etc/nginx-agent/bad.crt): open /etc/nginx-agent/bad.crt: no such file or directory"
1 parent 50e82ed commit 1859885

File tree

2 files changed

+6
-11
lines changed

2 files changed

+6
-11
lines changed

internal/grpc/grpc.go

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -382,14 +382,12 @@ func getTLSConfigForCredentials(c *config.TLSConfig) (*tls.Config, error) {
382382
InsecureSkipVerify: c.SkipVerify,
383383
}
384384

385-
err := appendRootCAs(tlsConfig, c.Ca)
386-
if err != nil {
387-
slog.Debug("Unable to append root CA", "error", err)
385+
if err := appendRootCAs(tlsConfig, c.Ca); err != nil {
386+
return nil, fmt.Errorf("invalid CA cert while building transport credentials: %w", err)
388387
}
389388

390-
err = appendCertKeyPair(tlsConfig, c.Cert, c.Key)
391-
if err != nil {
392-
return nil, fmt.Errorf("append cert and key pair failed: %w", err)
389+
if err := appendCertKeyPair(tlsConfig, c.Cert, c.Key); err != nil {
390+
return nil, fmt.Errorf("invalid client cert while building transport credentials: %w", err)
393391
}
394392

395393
return tlsConfig, nil

internal/grpc/grpc_test.go

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -447,14 +447,11 @@ func Test_getTLSConfig(t *testing.T) {
447447
require.False(t, c.InsecureSkipVerify, "InsecureSkipVerify should not be set")
448448
},
449449
},
450-
"Test 3: incorrect CA should not error": { // REALLY ?!
450+
"Test 3: incorrect CA should not error": {
451451
conf: &config.TLSConfig{
452452
Ca: "customca.pem",
453453
},
454-
wantErr: false,
455-
verify: func(t require.TestingT, c *tls.Config) {
456-
require.Nil(t, c.RootCAs, "RootCAs should be nil to use system")
457-
},
454+
wantErr: true,
458455
},
459456
"Test 4: incorrect key path should error": {
460457
conf: &config.TLSConfig{

0 commit comments

Comments
 (0)