Skip to content

Commit d59e266

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #2618 from danielmellado/fix_tls
OCPBUGS-58475: Enforce secure TLS settings in CMO
2 parents bc8807c + 6f7b88e commit d59e266

File tree

3 files changed

+103
-17
lines changed

3 files changed

+103
-17
lines changed

‎pkg/manifests/amcfg.go‎

Lines changed: 71 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"net/url"
66

77
"golang.org/x/net/http/httpproxy"
8+
yaml2 "gopkg.in/yaml.v2"
89
v1 "k8s.io/api/core/v1"
910
)
1011

@@ -13,10 +14,19 @@ type PrometheusAdditionalAlertmanagerConfigs []AdditionalAlertmanagerConfig
1314

1415
// MarshalYAML implements the yaml.Marshaler interface.
1516
func (a PrometheusAdditionalAlertmanagerConfigs) MarshalYAML() (interface{}, error) {
17+
return a.MarshalYAMLWithTLSConfig(nil, "")
18+
}
19+
20+
// MarshalYAMLWithTLSConfig marshals the configs with the provided TLS configuration
21+
func (a PrometheusAdditionalAlertmanagerConfigs) MarshalYAMLWithTLSConfig(cipherSuites []string, minTLSVersion string) (interface{}, error) {
1622
result := make([]interface{}, len(a))
1723

1824
for i, item := range a {
19-
promAmCfg := prometheusAdditionalAlertmanagerConfig(item)
25+
promAmCfg := prometheusAdditionalAlertmanagerConfigWithTLS{
26+
AdditionalAlertmanagerConfig: item,
27+
CipherSuites: cipherSuites,
28+
MinTLSVersion: minTLSVersion,
29+
}
2030

2131
y, err := promAmCfg.MarshalYAML()
2232
if err != nil {
@@ -29,6 +39,20 @@ func (a PrometheusAdditionalAlertmanagerConfigs) MarshalYAML() (interface{}, err
2939
return result, nil
3040
}
3141

42+
// MarshalPrometheusAdditionalAlertmanagerConfigs marshals the configs with secure TLS settings
43+
func (f *Factory) MarshalPrometheusAdditionalAlertmanagerConfigs(amConfigs []AdditionalAlertmanagerConfig) ([]byte, error) {
44+
prometheusAmConfigs := PrometheusAdditionalAlertmanagerConfigs(amConfigs)
45+
cipherSuites := f.APIServerConfig.TLSCiphers()
46+
minTLSVersion := f.APIServerConfig.MinTLSVersion()
47+
48+
result, err := prometheusAmConfigs.MarshalYAMLWithTLSConfig(cipherSuites, minTLSVersion)
49+
if err != nil {
50+
return nil, err
51+
}
52+
53+
return yaml2.Marshal(result)
54+
}
55+
3256
// amConfigPrometheus is our internal representation of the Prometheus alerting configuration.
3357
type amConfigPrometheus struct {
3458
Scheme string `yaml:"scheme,omitempty"`
@@ -46,26 +70,29 @@ type amConfigAuthorization struct {
4670
}
4771

4872
type amConfigTLS struct {
49-
CA string `yaml:"ca_file,omitempty"`
50-
Cert string `yaml:"cert_file,omitempty"`
51-
Key string `yaml:"key_file,omitempty"`
52-
ServerName string `yaml:"server_name,omitempty"`
53-
InsecureSkipVerify bool `yaml:"insecure_skip_verify,omitempty"`
73+
CA string `yaml:"ca_file,omitempty"`
74+
Cert string `yaml:"cert_file,omitempty"`
75+
Key string `yaml:"key_file,omitempty"`
76+
ServerName string `yaml:"server_name,omitempty"`
77+
InsecureSkipVerify bool `yaml:"insecure_skip_verify,omitempty"`
78+
MinVersion string `yaml:"min_version,omitempty"`
79+
CipherSuites []string `yaml:"cipher_suites,omitempty"`
5480
}
5581

5682
type amConfigStaticConfigs struct {
5783
Targets []string `yaml:"targets"`
5884
}
5985

60-
// prometheusAdditionalAlertmanagerConfig is an AdditionalAlertmanagerConfig
61-
// which can be marshaled into a yaml string, compatible with the Prometheus
62-
// configuration format
63-
type prometheusAdditionalAlertmanagerConfig AdditionalAlertmanagerConfig
86+
// prometheusAdditionalAlertmanagerConfigWithTLS is an AdditionalAlertmanagerConfig
87+
// with TLS configuration that can be marshaled with secure cipher suites and min TLS version
88+
type prometheusAdditionalAlertmanagerConfigWithTLS struct {
89+
AdditionalAlertmanagerConfig
90+
CipherSuites []string
91+
MinTLSVersion string
92+
}
6493

6594
// MarshalYAML implements the yaml.Marshaler interface.
66-
// It marshals a PrometheusAdditionalAlertmanagerConfig into a format
67-
// compatible with the Prometheus configuration.
68-
func (a prometheusAdditionalAlertmanagerConfig) MarshalYAML() (interface{}, error) {
95+
func (a prometheusAdditionalAlertmanagerConfigWithTLS) MarshalYAML() (interface{}, error) {
6996
cfg := amConfigPrometheus{
7097
Scheme: a.Scheme,
7198
PathPrefix: a.PathPrefix,
@@ -84,6 +111,20 @@ func (a prometheusAdditionalAlertmanagerConfig) MarshalYAML() (interface{}, erro
84111
},
85112
}
86113

114+
// Check if this configuration needs TLS security settings
115+
// Apply strict TLS only for HTTPS connections WITHOUT explicit TLS config
116+
// If explicit TLS config is provided, respect user's choices
117+
hasExplicitTLSConfig := a.TLSConfig.CA != nil || a.TLSConfig.Cert != nil || a.TLSConfig.Key != nil ||
118+
a.TLSConfig.ServerName != "" || a.TLSConfig.InsecureSkipVerify
119+
120+
needsTLSSecuritySettings := a.Scheme == "https" && !hasExplicitTLSConfig
121+
122+
// Apply TLS security settings only for HTTPS connections without explicit TLS config
123+
if needsTLSSecuritySettings {
124+
cfg.TLSConfig.MinVersion = a.MinTLSVersion
125+
cfg.TLSConfig.CipherSuites = a.CipherSuites
126+
}
127+
87128
caPath, err := secretPath(a.TLSConfig.CA)
88129
if err != nil {
89130
return nil, err
@@ -141,6 +182,9 @@ type amHTTPConfig struct {
141182
func (f *Factory) ConvertToThanosAlertmanagerConfiguration(ta []AdditionalAlertmanagerConfig) ([]thanosAlertmanagerConfiguration, error) {
142183
result := make([]thanosAlertmanagerConfiguration, len(ta))
143184

185+
cipherSuites := f.APIServerConfig.TLSCiphers()
186+
minTLSVersion := f.APIServerConfig.MinTLSVersion()
187+
144188
for i, a := range ta {
145189
cfg := thanosAlertmanagerConfiguration{
146190
Scheme: a.Scheme,
@@ -159,6 +203,20 @@ func (f *Factory) ConvertToThanosAlertmanagerConfiguration(ta []AdditionalAlertm
159203
},
160204
}
161205

206+
// Check if this configuration needs TLS security settings
207+
// Apply strict TLS only for HTTPS connections WITHOUT explicit TLS config
208+
// If explicit TLS config is provided, respect user's choices
209+
hasExplicitTLSConfig := a.TLSConfig.CA != nil || a.TLSConfig.Cert != nil || a.TLSConfig.Key != nil ||
210+
a.TLSConfig.ServerName != "" || a.TLSConfig.InsecureSkipVerify
211+
212+
needsTLSSecuritySettings := a.Scheme == "https" && !hasExplicitTLSConfig
213+
214+
// Apply TLS security settings only for HTTPS connections without explicit TLS config
215+
if needsTLSSecuritySettings {
216+
cfg.HTTPConfig.TLSConfig.MinVersion = minTLSVersion
217+
cfg.HTTPConfig.TLSConfig.CipherSuites = cipherSuites
218+
}
219+
162220
caPath, err := secretPath(a.TLSConfig.CA)
163221
if err != nil {
164222
return nil, err

‎pkg/manifests/manifests.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1627,9 +1627,8 @@ func (f *Factory) setupPrometheusRemoteWriteProxy(p *monv1.Prometheus) {
16271627

16281628
func (f *Factory) PrometheusK8sAdditionalAlertManagerConfigsSecret() (*v1.Secret, error) {
16291629
amConfigs := f.config.ClusterMonitoringConfiguration.PrometheusK8sConfig.AlertmanagerConfigs
1630-
prometheusAmConfigs := PrometheusAdditionalAlertmanagerConfigs(amConfigs)
16311630

1632-
config, err := yaml2.Marshal(prometheusAmConfigs)
1631+
config, err := f.MarshalPrometheusAdditionalAlertmanagerConfigs(amConfigs)
16331632
if err != nil {
16341633
return nil, err
16351634
}
@@ -1647,8 +1646,8 @@ func (f *Factory) PrometheusK8sAdditionalAlertManagerConfigsSecret() (*v1.Secret
16471646

16481647
func (f *Factory) PrometheusUserWorkloadAdditionalAlertManagerConfigsSecret() (*v1.Secret, error) {
16491648
amConfigs := f.config.AdditionalAlertmanagerConfigsForPrometheusUserWorkload()
1650-
prometheusAmConfigs := PrometheusAdditionalAlertmanagerConfigs(amConfigs)
1651-
config, err := yaml2.Marshal(prometheusAmConfigs)
1649+
1650+
config, err := f.MarshalPrometheusAdditionalAlertmanagerConfigs(amConfigs)
16521651
if err != nil {
16531652
return nil, err
16541653
}

‎pkg/manifests/manifests_test.go‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2058,6 +2058,20 @@ func TestPrometheusK8sAdditionalAlertManagerConfigsSecret(t *testing.T) {
20582058
api_version: v2
20592059
authorization:
20602060
credentials_file: /etc/prometheus/secrets/alertmanager1-bearer-token/token
2061+
tls_config:
2062+
min_version: VersionTLS12
2063+
cipher_suites:
2064+
- TLS_AES_128_GCM_SHA256
2065+
- TLS_AES_256_GCM_SHA384
2066+
- TLS_CHACHA20_POLY1305_SHA256
2067+
- ECDHE-ECDSA-AES128-GCM-SHA256
2068+
- ECDHE-RSA-AES128-GCM-SHA256
2069+
- ECDHE-ECDSA-AES256-GCM-SHA384
2070+
- ECDHE-RSA-AES256-GCM-SHA384
2071+
- ECDHE-ECDSA-CHACHA20-POLY1305
2072+
- ECDHE-RSA-CHACHA20-POLY1305
2073+
- DHE-RSA-AES128-GCM-SHA256
2074+
- DHE-RSA-AES256-GCM-SHA384
20612075
static_configs:
20622076
- targets:
20632077
- alertmanager1-remote.com
@@ -2353,6 +2367,21 @@ func TestThanosRulerAdditionalAlertManagerConfigsSecret(t *testing.T) {
23532367
- scheme: https
23542368
path_prefix: /path-prefix
23552369
api_version: v2
2370+
http_config:
2371+
tls_config:
2372+
min_version: VersionTLS12
2373+
cipher_suites:
2374+
- TLS_AES_128_GCM_SHA256
2375+
- TLS_AES_256_GCM_SHA384
2376+
- TLS_CHACHA20_POLY1305_SHA256
2377+
- ECDHE-ECDSA-AES128-GCM-SHA256
2378+
- ECDHE-RSA-AES128-GCM-SHA256
2379+
- ECDHE-ECDSA-AES256-GCM-SHA384
2380+
- ECDHE-RSA-AES256-GCM-SHA384
2381+
- ECDHE-ECDSA-CHACHA20-POLY1305
2382+
- ECDHE-RSA-CHACHA20-POLY1305
2383+
- DHE-RSA-AES128-GCM-SHA256
2384+
- DHE-RSA-AES256-GCM-SHA384
23562385
static_configs:
23572386
- alertmanager1-remote.com
23582387
- alertmanager1-remotex.com

0 commit comments

Comments
 (0)