Skip to content

Commit f8af86c

Browse files
areebahmedddcodeboten
areebahmeddd
and
codeboten
authored
feat(configtls): add validation to fail fast on missing certificates (#13245)
**Signed-off-by**: Areeb Ahmed [areebshariff@acm.org](mailto:areebshariff@acm.org) ## Summary of Changes * Enhanced `Config.Validate()` to validate certificate/key pairs and prevent mixed file/PEM configurations. * Added `ServerConfig.Validate()` to require certificates when TLS settings are configured. * Added `ClientConfig.Validate()` for a consistent validation interface. ## Related Issue(s) Closes #13130 --------- Co-authored-by: Alex Boten <223565+codeboten@users.noreply.github.com>
1 parent 0b25883 commit f8af86c

5 files changed

Lines changed: 295 additions & 2 deletions

File tree

‎.chloggen/configtls-cert-validation.yaml‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: enhancement
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
7+
component: configtls
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Add early validation for TLS server configurations to fail fast when certificates are missing instead of failing at runtime.
11+
12+
# One or more tracking issues or pull requests related to the change
13+
issues: [13130, 13245]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext:
19+
20+
# Optional: The change log or logs in which this entry should be included.
21+
# e.g. '[user]' or '[user, api]'
22+
# Include 'user' if the change is relevant to end users.
23+
# Include 'api' if there is a change to a library API.
24+
# Default: '[user]'
25+
change_logs: [user]

‎config/configtls/configtls.go‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,21 @@ func (c Config) Validate() error {
194194
return errors.New("provide either a CA file or the PEM-encoded string, but not both")
195195
}
196196

197+
// Ensure certificate is not set using both file and PEM
198+
if c.hasCertFile() && c.hasCertPem() {
199+
return errors.New("provide either certificate file or PEM, but not both")
200+
}
201+
202+
// Ensure key is not set using both file and PEM
203+
if c.hasKeyFile() && c.hasKeyPem() {
204+
return errors.New("provide either key file or PEM, but not both")
205+
}
206+
207+
// Fail if only one of cert/key is provided (mismatch case)
208+
if c.hasCert() != c.hasKey() {
209+
return errors.New("TLS configuration must include both certificate and key (CertFile/CertPem and KeyFile/KeyPem)")
210+
}
211+
197212
minTLS, err := convertVersion(c.MinVersion, defaultMinTLSVersion)
198213
if err != nil {
199214
return fmt.Errorf("invalid TLS min_version: %w", err)
@@ -211,6 +226,16 @@ func (c Config) Validate() error {
211226
return nil
212227
}
213228

229+
func (c ServerConfig) Validate() error {
230+
// For servers, both certificate and key are required:
231+
// - If both are missing, error.
232+
// - If only one is provided (mismatch), error.
233+
if !c.hasCert() && !c.hasKey() {
234+
return errors.New("TLS configuration must include both certificate and key for server connections")
235+
}
236+
return nil
237+
}
238+
214239
// loadTLSConfig loads TLS certificates and returns a tls.Config.
215240
// This will set the RootCAs and Certificates of a tls.Config.
216241
func (c Config) loadTLSConfig() (*tls.Config, error) {

‎config/configtls/configtls_test.go‎

Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"github.com/stretchr/testify/require"
2020

2121
"go.opentelemetry.io/collector/config/configopaque"
22+
"go.opentelemetry.io/collector/confmap/xconfmap"
2223
)
2324

2425
func TestNewDefaultConfig(t *testing.T) {
@@ -664,6 +665,16 @@ func TestConfigValidate(t *testing.T) {
664665
{name: `TLS Config ["0.4", ""] to give [Error]`, tlsConfig: Config{MinVersion: "0.4", MaxVersion: ""}, errorTxt: `invalid TLS min_version: unsupported TLS version: "0.4"`},
665666
{name: `TLS Config ["1.2", "1.1"] to give [Error]`, tlsConfig: Config{MinVersion: "1.2", MaxVersion: "1.1"}, errorTxt: `invalid TLS configuration: min_version cannot be greater than max_version`},
666667
{name: `TLS Config with both CA File and PEM`, tlsConfig: Config{CAFile: "test", CAPem: "test"}, errorTxt: `provide either a CA file or the PEM-encoded string, but not both`},
668+
{name: `TLS Config with cert file but no key`, tlsConfig: Config{CertFile: "cert.pem"}, errorTxt: `TLS configuration must include both certificate and key (CertFile/CertPem and KeyFile/KeyPem)`},
669+
{name: `TLS Config with key file but no cert`, tlsConfig: Config{KeyFile: "key.pem"}, errorTxt: `TLS configuration must include both certificate and key (CertFile/CertPem and KeyFile/KeyPem)`},
670+
{name: `TLS Config with cert PEM but no key`, tlsConfig: Config{CertPem: "cert-pem"}, errorTxt: `TLS configuration must include both certificate and key (CertFile/CertPem and KeyFile/KeyPem)`},
671+
{name: `TLS Config with key PEM but no cert`, tlsConfig: Config{KeyPem: "key-pem"}, errorTxt: `TLS configuration must include both certificate and key (CertFile/CertPem and KeyFile/KeyPem)`},
672+
{name: `TLS Config with both cert file and cert PEM`, tlsConfig: Config{CertFile: "cert.pem", CertPem: "cert-pem", KeyFile: "key.pem"}, errorTxt: `provide either certificate file or PEM, but not both`},
673+
{name: `TLS Config with both key file and key PEM`, tlsConfig: Config{CertFile: "cert.pem", KeyFile: "key.pem", KeyPem: "key-pem"}, errorTxt: `provide either key file or PEM, but not both`},
674+
{name: `TLS Config with cert file and key PEM`, tlsConfig: Config{CertFile: "cert.pem", KeyPem: "key-pem"}},
675+
{name: `TLS Config with cert PEM and key file`, tlsConfig: Config{CertPem: "cert-pem", KeyFile: "key.pem"}},
676+
{name: `TLS Config with valid cert and key files`, tlsConfig: Config{CertFile: "cert.pem", KeyFile: "key.pem"}},
677+
{name: `TLS Config with valid cert and key PEM`, tlsConfig: Config{CertPem: "cert-pem", KeyPem: "key-pem"}},
667678
}
668679

669680
for _, test := range tests {
@@ -924,3 +935,199 @@ func TestCurvePreferences(t *testing.T) {
924935
}
925936
}
926937
}
938+
939+
func TestServerConfigValidate(t *testing.T) {
940+
tests := []struct {
941+
name string
942+
serverConfig ServerConfig
943+
errorTxt string
944+
}{
945+
{
946+
name: "server config without certificates",
947+
serverConfig: ServerConfig{},
948+
errorTxt: "TLS configuration must include both certificate and key for server connections",
949+
},
950+
{
951+
name: "server config with cert file but no key",
952+
serverConfig: ServerConfig{
953+
Config: Config{
954+
CertFile: "cert.pem",
955+
},
956+
},
957+
errorTxt: "config: TLS configuration must include both certificate and key (CertFile/CertPem and KeyFile/KeyPem)",
958+
},
959+
{
960+
name: "server config with key file but no cert",
961+
serverConfig: ServerConfig{
962+
Config: Config{
963+
KeyFile: "key.pem",
964+
},
965+
},
966+
errorTxt: "config: TLS configuration must include both certificate and key (CertFile/CertPem and KeyFile/KeyPem)",
967+
},
968+
{
969+
name: "server config with cert PEM but no key",
970+
serverConfig: ServerConfig{
971+
Config: Config{
972+
CertPem: "cert-pem",
973+
},
974+
},
975+
errorTxt: "config: TLS configuration must include both certificate and key (CertFile/CertPem and KeyFile/KeyPem)",
976+
},
977+
{
978+
name: "server config with key PEM but no cert",
979+
serverConfig: ServerConfig{
980+
Config: Config{
981+
KeyPem: "key-pem",
982+
},
983+
},
984+
errorTxt: "config: TLS configuration must include both certificate and key (CertFile/CertPem and KeyFile/KeyPem)",
985+
},
986+
{
987+
name: "server config with both cert file and cert PEM",
988+
serverConfig: ServerConfig{
989+
Config: Config{
990+
CertFile: "cert.pem",
991+
CertPem: "cert-pem",
992+
KeyFile: "key.pem",
993+
},
994+
},
995+
errorTxt: "config: provide either certificate file or PEM, but not both",
996+
},
997+
{
998+
name: "server config with both key file and key PEM",
999+
serverConfig: ServerConfig{
1000+
Config: Config{
1001+
CertFile: "cert.pem",
1002+
KeyFile: "key.pem",
1003+
KeyPem: "key-pem",
1004+
},
1005+
},
1006+
errorTxt: "config: provide either key file or PEM, but not both",
1007+
},
1008+
{
1009+
name: "valid server config with cert and key files",
1010+
serverConfig: ServerConfig{
1011+
Config: Config{
1012+
CertFile: "cert.pem",
1013+
KeyFile: "key.pem",
1014+
},
1015+
},
1016+
},
1017+
{
1018+
name: "valid server config with cert and key PEM",
1019+
serverConfig: ServerConfig{
1020+
Config: Config{
1021+
CertPem: "cert-pem",
1022+
KeyPem: "key-pem",
1023+
},
1024+
},
1025+
},
1026+
{
1027+
name: "valid server config with mixed cert file and key PEM",
1028+
serverConfig: ServerConfig{
1029+
Config: Config{
1030+
CertFile: "cert.pem",
1031+
KeyPem: "key-pem",
1032+
},
1033+
},
1034+
},
1035+
}
1036+
1037+
for _, test := range tests {
1038+
t.Run(test.name, func(t *testing.T) {
1039+
err := xconfmap.Validate(test.serverConfig)
1040+
1041+
if test.errorTxt == "" {
1042+
assert.NoError(t, err)
1043+
} else {
1044+
assert.ErrorContains(t, err, test.errorTxt)
1045+
}
1046+
})
1047+
}
1048+
}
1049+
1050+
func TestClientConfigValidate(t *testing.T) {
1051+
tests := []struct {
1052+
name string
1053+
clientConfig ClientConfig
1054+
errorTxt string
1055+
}{
1056+
{
1057+
name: "valid empty client config",
1058+
clientConfig: ClientConfig{},
1059+
},
1060+
{
1061+
name: "valid client config with insecure connection",
1062+
clientConfig: ClientConfig{
1063+
Insecure: true,
1064+
},
1065+
},
1066+
{
1067+
name: "valid client config with cert and key files",
1068+
clientConfig: ClientConfig{
1069+
Config: Config{
1070+
CertFile: "cert.pem",
1071+
KeyFile: "key.pem",
1072+
},
1073+
},
1074+
},
1075+
{
1076+
name: "valid client config with mixed cert file and key PEM",
1077+
clientConfig: ClientConfig{
1078+
Config: Config{
1079+
CertFile: "cert.pem",
1080+
KeyPem: "key-pem",
1081+
},
1082+
},
1083+
},
1084+
{
1085+
name: "client config with only cert file",
1086+
clientConfig: ClientConfig{
1087+
Config: Config{
1088+
CertFile: "cert.pem",
1089+
},
1090+
},
1091+
errorTxt: "config: TLS configuration must include both certificate and key (CertFile/CertPem and KeyFile/KeyPem)",
1092+
},
1093+
{
1094+
name: "client config with only key file",
1095+
clientConfig: ClientConfig{
1096+
Config: Config{
1097+
KeyFile: "key.pem",
1098+
},
1099+
},
1100+
errorTxt: "config: TLS configuration must include both certificate and key (CertFile/CertPem and KeyFile/KeyPem)",
1101+
},
1102+
{
1103+
name: "client config with only cert PEM",
1104+
clientConfig: ClientConfig{
1105+
Config: Config{
1106+
CertPem: "cert-pem",
1107+
},
1108+
},
1109+
errorTxt: "config: TLS configuration must include both certificate and key (CertFile/CertPem and KeyFile/KeyPem)",
1110+
},
1111+
{
1112+
name: "client config with only key PEM",
1113+
clientConfig: ClientConfig{
1114+
Config: Config{
1115+
KeyPem: "key-pem",
1116+
},
1117+
},
1118+
errorTxt: "config: TLS configuration must include both certificate and key (CertFile/CertPem and KeyFile/KeyPem)",
1119+
},
1120+
}
1121+
1122+
for _, test := range tests {
1123+
t.Run(test.name, func(t *testing.T) {
1124+
err := xconfmap.Validate(test.clientConfig)
1125+
1126+
if test.errorTxt == "" {
1127+
assert.NoError(t, err)
1128+
} else {
1129+
assert.ErrorContains(t, err, test.errorTxt)
1130+
}
1131+
})
1132+
}
1133+
}

‎config/configtls/go.mod‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,35 @@ require (
88
github.com/google/go-tpm v0.9.5
99
github.com/stretchr/testify v1.10.0
1010
go.opentelemetry.io/collector/config/configopaque v1.41.0
11+
go.opentelemetry.io/collector/confmap/xconfmap v0.135.0
1112
)
1213

1314
require (
1415
github.com/davecgh/go-spew v1.1.1 // indirect
16+
github.com/go-viper/mapstructure/v2 v2.4.0 // indirect
17+
github.com/gobwas/glob v0.2.3 // indirect
1518
github.com/google/go-tpm-tools v0.4.4 // indirect
16-
github.com/kr/text v0.2.0 // indirect
19+
github.com/hashicorp/go-version v1.7.0 // indirect
20+
github.com/knadh/koanf/maps v0.1.2 // indirect
21+
github.com/knadh/koanf/providers/confmap v1.0.0 // indirect
22+
github.com/knadh/koanf/v2 v2.2.2 // indirect
23+
github.com/mitchellh/copystructure v1.2.0 // indirect
24+
github.com/mitchellh/reflectwalk v1.0.2 // indirect
1725
github.com/pmezard/go-difflib v1.0.0 // indirect
26+
go.opentelemetry.io/collector/confmap v1.41.0 // indirect
27+
go.opentelemetry.io/collector/featuregate v1.41.0 // indirect
28+
go.uber.org/multierr v1.11.0 // indirect
29+
go.uber.org/zap v1.27.0 // indirect
30+
go.yaml.in/yaml/v3 v3.0.4 // indirect
1831
golang.org/x/crypto v0.35.0 // indirect
1932
golang.org/x/sys v0.30.0 // indirect
2033
gopkg.in/yaml.v3 v3.0.1 // indirect
2134
)
2235

2336
replace go.opentelemetry.io/collector/config/configopaque => ../configopaque
37+
38+
replace go.opentelemetry.io/collector/featuregate => ../../featuregate
39+
40+
replace go.opentelemetry.io/collector/confmap => ../../confmap
41+
42+
replace go.opentelemetry.io/collector/confmap/xconfmap => ../../confmap/xconfmap

‎config/configtls/go.sum‎

Lines changed: 18 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)
⚡