Skip to content

Commit 284d611

Browse files
committed
address PR review comments
Signed-off-by: Evgeny Slutsky <eslutsky@redhat.com>
1 parent 9e1e64a commit 284d611

3 files changed

Lines changed: 148 additions & 5 deletions

File tree

pkg/components/controllers.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ func detectFIPS() bool {
6868
klog.Warningf("Got empty /proc/sys/crypto/fips_enabled; assuming FIPS is not enabled")
6969
return result
7070
}
71-
result = data[0] == '1'
71+
result = strings.TrimSpace(string(data)) == "1"
7272
klog.Infof("Read /proc/sys/crypto/fips_enabled: data=%s, result=%v", string(data), result)
7373
return result
7474
}
@@ -266,7 +266,7 @@ func startIngressController(ctx context.Context, cfg *config.Config, kubeconfigP
266266
return err
267267
}
268268

269-
extraParams, err := generateIngressParams(cfg)
269+
extraParams, err := generateIngressParams(cfg, isFIPSEnabled)
270270
if err != nil {
271271
return err
272272
}
@@ -467,7 +467,7 @@ func validateClientTLS(patterns []string) error {
467467
return nil
468468
}
469469

470-
func generateIngressParams(cfg *config.Config) (assets.RenderParams, error) {
470+
func generateIngressParams(cfg *config.Config, fipsEnabled bool) (assets.RenderParams, error) {
471471
routerMode := "v4"
472472
if cfg.IsIPv6() {
473473
routerMode = "v4v6"
@@ -512,7 +512,7 @@ func generateIngressParams(cfg *config.Config) (assets.RenderParams, error) {
512512
// suites (e.g. TLS_CHACHA20_POLY1305_SHA256). HAProxy would fail TLS
513513
// handshakes when a client offers a non-FIPS cipher first if that cipher
514514
// is listed in ssl-default-bind-ciphersuites but excluded by the OS FIPS policy.
515-
if isFIPSEnabled {
515+
if fipsEnabled {
516516
fipsCiphers := tls13Ciphers[:0]
517517
for _, c := range tls13Ciphers {
518518
if fipsApprovedTLS13Ciphers.Has(c) {
@@ -532,7 +532,7 @@ func generateIngressParams(cfg *config.Config) (assets.RenderParams, error) {
532532
// post-quantum readiness. In FIPS mode, ML-KEM and X25519 are not
533533
// supported by OpenSSL FIPS 140-3.
534534
tlsCurves := "X25519MLKEM768:X25519:P-256:P-384:P-521"
535-
if isFIPSEnabled {
535+
if fipsEnabled {
536536
tlsCurves = "P-256:P-384:P-521"
537537
}
538538

pkg/components/controllers_test.go

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
package components
2+
3+
import (
4+
"strings"
5+
"testing"
6+
"time"
7+
8+
configv1 "github.com/openshift/api/config/v1"
9+
"github.com/openshift/microshift/pkg/assets"
10+
"github.com/openshift/microshift/pkg/config"
11+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
12+
"k8s.io/utils/ptr"
13+
)
14+
15+
func newTestConfig() *config.Config {
16+
return &config.Config{
17+
DNS: config.DNS{BaseDomain: "example.com"},
18+
Network: config.Network{
19+
ClusterNetwork: []string{"10.42.0.0/16"},
20+
ServiceNetwork: []string{"10.43.0.0/16"},
21+
},
22+
Ingress: config.IngressConfig{
23+
Ports: config.IngressPortsConfig{
24+
Http: ptr.To(80),
25+
Https: ptr.To(443),
26+
},
27+
TuningOptions: config.IngressControllerTuningOptions{
28+
HeaderBufferBytes: 32768,
29+
HeaderBufferMaxRewriteBytes: 8192,
30+
HealthCheckInterval: &metav1.Duration{Duration: 5 * time.Second},
31+
ClientTimeout: &metav1.Duration{Duration: 30 * time.Second},
32+
ClientFinTimeout: &metav1.Duration{Duration: 1 * time.Second},
33+
ServerTimeout: &metav1.Duration{Duration: 30 * time.Second},
34+
ServerFinTimeout: &metav1.Duration{Duration: 1 * time.Second},
35+
TunnelTimeout: &metav1.Duration{Duration: 1 * time.Hour},
36+
TLSInspectDelay: &metav1.Duration{Duration: 5 * time.Second},
37+
ThreadCount: 4,
38+
MaxConnections: 50000,
39+
},
40+
ForwardedHeaderPolicy: "Append",
41+
TLSSecurityProfile: &configv1.TLSSecurityProfile{
42+
Type: configv1.TLSProfileIntermediateType,
43+
},
44+
ServingCertificateSecret: "router-certs-default",
45+
DefaultHttpVersionPolicy: 1,
46+
LogEmptyRequests: "Log",
47+
HTTPEmptyRequestsPolicy: "Respond",
48+
AccessLogging: config.AccessLogging{
49+
Status: config.AccessLoggingDisabled,
50+
},
51+
},
52+
}
53+
}
54+
55+
func requireStringParam(t *testing.T, params assets.RenderParams, key string) string {
56+
t.Helper()
57+
v, ok := params[key]
58+
if !ok {
59+
t.Fatalf("missing param %q", key)
60+
}
61+
s, ok := v.(string)
62+
if !ok {
63+
t.Fatalf("param %q has type %T, want string", key, v)
64+
}
65+
return s
66+
}
67+
68+
func TestGenerateIngressParamsFIPSCiphers(t *testing.T) {
69+
t.Run("FIPS enabled filters non-FIPS TLS 1.3 ciphers", func(t *testing.T) {
70+
cfg := newTestConfig()
71+
params, err := generateIngressParams(cfg, true)
72+
if err != nil {
73+
t.Fatalf("unexpected error: %v", err)
74+
}
75+
76+
cipherSuites := requireStringParam(t, params, "RouterCiphersSuites")
77+
if strings.Contains(cipherSuites, "TLS_CHACHA20_POLY1305_SHA256") {
78+
t.Errorf("FIPS mode should filter out TLS_CHACHA20_POLY1305_SHA256, got: %s", cipherSuites)
79+
}
80+
if !strings.Contains(cipherSuites, "TLS_AES_128_GCM_SHA256") {
81+
t.Errorf("FIPS mode should keep TLS_AES_128_GCM_SHA256, got: %s", cipherSuites)
82+
}
83+
if !strings.Contains(cipherSuites, "TLS_AES_256_GCM_SHA384") {
84+
t.Errorf("FIPS mode should keep TLS_AES_256_GCM_SHA384, got: %s", cipherSuites)
85+
}
86+
})
87+
88+
t.Run("non-FIPS keeps all TLS 1.3 ciphers", func(t *testing.T) {
89+
cfg := newTestConfig()
90+
params, err := generateIngressParams(cfg, false)
91+
if err != nil {
92+
t.Fatalf("unexpected error: %v", err)
93+
}
94+
95+
cipherSuites := requireStringParam(t, params, "RouterCiphersSuites")
96+
if !strings.Contains(cipherSuites, "TLS_CHACHA20_POLY1305_SHA256") {
97+
t.Errorf("non-FIPS mode should keep TLS_CHACHA20_POLY1305_SHA256, got: %s", cipherSuites)
98+
}
99+
})
100+
}
101+
102+
func TestGenerateIngressParamsFIPSCurves(t *testing.T) {
103+
t.Run("FIPS enabled uses only NIST curves", func(t *testing.T) {
104+
cfg := newTestConfig()
105+
params, err := generateIngressParams(cfg, true)
106+
if err != nil {
107+
t.Fatalf("unexpected error: %v", err)
108+
}
109+
110+
curves := requireStringParam(t, params, "RouterTLSCurves")
111+
if strings.Contains(curves, "X25519MLKEM768") {
112+
t.Errorf("FIPS mode should exclude X25519MLKEM768, got: %s", curves)
113+
}
114+
for _, c := range strings.Split(curves, ":") {
115+
if c == "X25519" {
116+
t.Errorf("FIPS mode should exclude X25519, got: %s", curves)
117+
}
118+
}
119+
if curves != "P-256:P-384:P-521" {
120+
t.Errorf("FIPS mode curves should be P-256:P-384:P-521, got: %s", curves)
121+
}
122+
})
123+
124+
t.Run("non-FIPS includes PQC hybrid curve", func(t *testing.T) {
125+
cfg := newTestConfig()
126+
params, err := generateIngressParams(cfg, false)
127+
if err != nil {
128+
t.Fatalf("unexpected error: %v", err)
129+
}
130+
131+
curves := requireStringParam(t, params, "RouterTLSCurves")
132+
if !strings.Contains(curves, "X25519MLKEM768") {
133+
t.Errorf("non-FIPS mode should include X25519MLKEM768, got: %s", curves)
134+
}
135+
if curves != "X25519MLKEM768:X25519:P-256:P-384:P-521" {
136+
t.Errorf("non-FIPS mode curves should be X25519MLKEM768:X25519:P-256:P-384:P-521, got: %s", curves)
137+
}
138+
})
139+
}

test/suites/optional/tls-scanner.robot

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,10 @@ Verify ML-KEM Post Quantum Curve Negotiation
133133
... negotiates successfully via oc exec into the router pod, which
134134
... has OpenSSL 3.5+ (the host OpenSSL may be too old for ML-KEM).
135135
... Skipped on FIPS clusters where ML-KEM is not configured.
136+
${curves}= Oc Get JsonPath deployment openshift-ingress router-default
137+
... .spec.template.spec.containers[0].env[?(@.name=="ROUTER_CURVES")].value
138+
Skip If "X25519MLKEM768" not in """${curves}"""
139+
... ROUTER_CURVES does not include X25519MLKEM768 (FIPS mode); skipping ML-KEM test
136140
${router_ip}= Oc Get JsonPath svc openshift-ingress router-default
137141
... .spec.clusterIP
138142
${pod_name}= Oc Get JsonPath pod openshift-ingress ${EMPTY}

0 commit comments

Comments
 (0)