Skip to content

Commit 35ea132

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #591 from lmiccini/instanceha-security-hardening
InstanceHA: security hardening, HMAC heartbeat authentication, and controller lifecycle fixes
2 parents 48b751f + 440caaf commit 35ea132

23 files changed

Lines changed: 1086 additions & 267 deletions

‎apis/bases/instanceha.openstack.org_instancehas.yaml‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,10 +95,14 @@ spec:
9595
description: InstanceHaHeartbeatPort is the UDP port for compute node
9696
heartbeat packets
9797
format: int32
98+
maximum: 65535
99+
minimum: 1
98100
type: integer
99101
instanceHaKdumpPort:
100102
default: 7410
101103
format: int32
104+
maximum: 65535
105+
minimum: 1
102106
type: integer
103107
metricsTLS:
104108
description: MetricsTLS - Parameters related to TLS for the metrics
@@ -108,6 +112,20 @@ spec:
108112
description: CaBundleSecretName - holding the CA certs in a pre-created
109113
bundle file
110114
type: string
115+
cipherSuites:
116+
default: HIGH:!aNULL:!MD5:!RC4:!3DES:!kRSA
117+
description: CipherSuites - Allowed TLS cipher suites (OpenSSL
118+
format)
119+
minLength: 1
120+
type: string
121+
minTLSVersion:
122+
default: "1.2"
123+
description: MinTLSVersion - Minimum TLS version for the metrics
124+
endpoint
125+
enum:
126+
- "1.2"
127+
- "1.3"
128+
type: string
111129
secretName:
112130
description: SecretName - holding the cert, key for the service
113131
type: string
@@ -210,6 +228,11 @@ spec:
210228
- type
211229
type: object
212230
type: array
231+
heartbeatHMACSecret:
232+
description: |-
233+
HeartbeatHMACSecret is the name of the auto-generated Secret
234+
containing the HMAC key for heartbeat authentication
235+
type: string
213236
lastAppliedTopology:
214237
description: LastAppliedTopology - the last applied Topology
215238
properties:
@@ -238,7 +261,9 @@ spec:
238261
format: int64
239262
type: integer
240263
podName:
241-
description: PodName
264+
description: |-
265+
PodName
266+
Deprecated: this field is no longer used
242267
type: string
243268
type: object
244269
type: object

‎apis/instanceha/v1beta1/instanceha_types.go‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,23 @@ type AuthSpec struct {
4646
ApplicationCredentialSecret string `json:"applicationCredentialSecret,omitempty"`
4747
}
4848

49+
// InstanceHaMetricsTLS extends tls.SimpleService with TLS hardening parameters.
50+
type InstanceHaMetricsTLS struct {
51+
tls.SimpleService `json:",inline"`
52+
53+
// +kubebuilder:validation:Optional
54+
// +kubebuilder:validation:Enum="1.2";"1.3"
55+
// +kubebuilder:default="1.2"
56+
// MinTLSVersion - Minimum TLS version for the metrics endpoint
57+
MinTLSVersion string `json:"minTLSVersion"`
58+
59+
// +kubebuilder:validation:Optional
60+
// +kubebuilder:validation:MinLength=1
61+
// +kubebuilder:default="HIGH:!aNULL:!MD5:!RC4:!3DES:!kRSA"
62+
// CipherSuites - Allowed TLS cipher suites (OpenSSL format)
63+
CipherSuites string `json:"cipherSuites"`
64+
}
65+
4966
// InstanceHaSpec defines the desired state of InstanceHa
5067
type InstanceHaSpec struct {
5168
// +kubebuilder:validation:Optional
@@ -79,10 +96,14 @@ type InstanceHaSpec struct {
7996

8097
// +kubebuilder:validation:Required
8198
// +kubebuilder:default=7410
99+
// +kubebuilder:validation:Minimum=1
100+
// +kubebuilder:validation:Maximum=65535
82101
InstanceHaKdumpPort int32 `json:"instanceHaKdumpPort"`
83102

84103
// +kubebuilder:validation:Optional
85104
// +kubebuilder:default=7411
105+
// +kubebuilder:validation:Minimum=1
106+
// +kubebuilder:validation:Maximum=65535
86107
// InstanceHaHeartbeatPort is the UDP port for compute node heartbeat packets
87108
InstanceHaHeartbeatPort int32 `json:"instanceHaHeartbeatPort"`
88109

@@ -119,12 +140,14 @@ type InstanceHaSpec struct {
119140
// +kubebuilder:validation:Optional
120141
//+operator-sdk:csv:customresourcedefinitions:type=spec
121142
// MetricsTLS - Parameters related to TLS for the metrics endpoint
122-
MetricsTLS tls.SimpleService `json:"metricsTLS,omitempty"`
143+
MetricsTLS InstanceHaMetricsTLS `json:"metricsTLS,omitempty"`
123144
}
124145

125146
// InstanceHaStatus defines the observed state of InstanceHa
126147
type InstanceHaStatus struct {
127148
// PodName
149+
// +kubebuilder:validation:Optional
150+
// Deprecated: this field is no longer used
128151
PodName string `json:"podName,omitempty"`
129152

130153
// Conditions
@@ -138,6 +161,10 @@ type InstanceHaStatus struct {
138161

139162
// LastAppliedTopology - the last applied Topology
140163
LastAppliedTopology *topologyv1.TopoRef `json:"lastAppliedTopology,omitempty"`
164+
165+
// HeartbeatHMACSecret is the name of the auto-generated Secret
166+
// containing the HMAC key for heartbeat authentication
167+
HeartbeatHMACSecret string `json:"heartbeatHMACSecret,omitempty"`
141168
}
142169

143170
//+kubebuilder:object:root=true

‎apis/instanceha/v1beta1/instanceha_webhook.go‎

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ package v1beta1
2222
import (
2323
"context"
2424
"fmt"
25+
"strings"
2526

2627
"k8s.io/apimachinery/pkg/runtime"
2728
logf "sigs.k8s.io/controller-runtime/pkg/log"
@@ -84,7 +85,12 @@ func (spec *InstanceHaSpec) Default() {
8485
if spec.InstanceHaHeartbeatPort == 0 {
8586
spec.InstanceHaHeartbeatPort = 7411
8687
}
87-
88+
if spec.MetricsTLS.MinTLSVersion == "" {
89+
spec.MetricsTLS.MinTLSVersion = "1.2"
90+
}
91+
if spec.MetricsTLS.CipherSuites == "" {
92+
spec.MetricsTLS.CipherSuites = "HIGH:!aNULL:!MD5:!RC4:!3DES:!kRSA"
93+
}
8894
}
8995

9096
// ValidateCreate validates the InstanceHa resource on creation.
@@ -97,6 +103,7 @@ func (r *InstanceHa) ValidateCreate(ctx context.Context, c client.Client) (admis
97103

98104
allErrs = append(allErrs, r.Spec.ValidateTopology(basePath, r.Namespace)...)
99105
allErrs = append(allErrs, r.validateUniqueOpenStackCloud(ctx, c, basePath)...)
106+
allErrs = append(allErrs, r.validateCipherSuites(basePath)...)
100107

101108
if len(allErrs) != 0 {
102109
return allWarn, apierrors.NewInvalid(
@@ -116,6 +123,7 @@ func (r *InstanceHa) ValidateUpdate(ctx context.Context, c client.Client, old ru
116123

117124
allErrs = append(allErrs, r.Spec.ValidateTopology(basePath, r.Namespace)...)
118125
allErrs = append(allErrs, r.validateUniqueOpenStackCloud(ctx, c, basePath)...)
126+
allErrs = append(allErrs, r.validateCipherSuites(basePath)...)
119127

120128
if len(allErrs) != 0 {
121129
return allWarn, apierrors.NewInvalid(
@@ -159,6 +167,33 @@ func (r *InstanceHa) validateUniqueOpenStackCloud(
159167
return allErrs
160168
}
161169

170+
// validateCipherSuites rejects cipher strings that would result in unencrypted connections.
171+
func (r *InstanceHa) validateCipherSuites(basePath *field.Path) field.ErrorList {
172+
var allErrs field.ErrorList
173+
ciphers := r.Spec.MetricsTLS.CipherSuites
174+
if ciphers == "" {
175+
return allErrs
176+
}
177+
fldPath := basePath.Child("metricsTLS", "cipherSuites")
178+
upper := strings.ToUpper(ciphers)
179+
for _, token := range strings.Split(upper, ":") {
180+
trimmed := strings.TrimLeft(token, "+")
181+
switch trimmed {
182+
case "NULL", "ENULL", "ANULL":
183+
allErrs = append(allErrs, field.Invalid(fldPath, ciphers,
184+
fmt.Sprintf("cipher string must not include %q without a '!' prefix", token)))
185+
case "ALL":
186+
allErrs = append(allErrs, field.Invalid(fldPath, ciphers,
187+
"cipher string must not use 'ALL' (includes NULL ciphers); use 'HIGH' instead"))
188+
}
189+
if strings.HasPrefix(trimmed, "@SECLEVEL=0") {
190+
allErrs = append(allErrs, field.Invalid(fldPath, ciphers,
191+
"cipher string must not use '@SECLEVEL=0' (disables all security requirements)"))
192+
}
193+
}
194+
return allErrs
195+
}
196+
162197
// ValidateDelete implements webhook.Validator so a webhook will be registered for the type
163198
func (r *InstanceHa) ValidateDelete() (admission.Warnings, error) {
164199
instancehalog.Info("validate delete", "name", r.Name)

‎apis/instanceha/v1beta1/zz_generated.deepcopy.go‎

Lines changed: 16 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎config/crd/bases/instanceha.openstack.org_instancehas.yaml‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,10 +95,14 @@ spec:
9595
description: InstanceHaHeartbeatPort is the UDP port for compute node
9696
heartbeat packets
9797
format: int32
98+
maximum: 65535
99+
minimum: 1
98100
type: integer
99101
instanceHaKdumpPort:
100102
default: 7410
101103
format: int32
104+
maximum: 65535
105+
minimum: 1
102106
type: integer
103107
metricsTLS:
104108
description: MetricsTLS - Parameters related to TLS for the metrics
@@ -108,6 +112,20 @@ spec:
108112
description: CaBundleSecretName - holding the CA certs in a pre-created
109113
bundle file
110114
type: string
115+
cipherSuites:
116+
default: HIGH:!aNULL:!MD5:!RC4:!3DES:!kRSA
117+
description: CipherSuites - Allowed TLS cipher suites (OpenSSL
118+
format)
119+
minLength: 1
120+
type: string
121+
minTLSVersion:
122+
default: "1.2"
123+
description: MinTLSVersion - Minimum TLS version for the metrics
124+
endpoint
125+
enum:
126+
- "1.2"
127+
- "1.3"
128+
type: string
111129
secretName:
112130
description: SecretName - holding the cert, key for the service
113131
type: string
@@ -210,6 +228,11 @@ spec:
210228
- type
211229
type: object
212230
type: array
231+
heartbeatHMACSecret:
232+
description: |-
233+
HeartbeatHMACSecret is the name of the auto-generated Secret
234+
containing the HMAC key for heartbeat authentication
235+
type: string
213236
lastAppliedTopology:
214237
description: LastAppliedTopology - the last applied Topology
215238
properties:
@@ -238,7 +261,9 @@ spec:
238261
format: int64
239262
type: integer
240263
podName:
241-
description: PodName
264+
description: |-
265+
PodName
266+
Deprecated: this field is no longer used
242267
type: string
243268
type: object
244269
type: object

0 commit comments

Comments
 (0)