-
Notifications
You must be signed in to change notification settings - Fork 232
USHIFT-6925 USHIFT-6851: Introduce Post-Quantum Curves to Ingress defaults and FIPs detection #6622
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
1f8b3c0
54d29bd
71470b3
afaf023
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,6 +27,9 @@ const ( | |
| haproxyMaxTimeoutMilliseconds = 2147483647 * time.Millisecond | ||
| ) | ||
|
|
||
| // isFIPSEnabled reports whether the cluster has FIPS enabled. | ||
| var isFIPSEnabled = detectFIPS() | ||
|
|
||
| var ( | ||
| tlsVersion13Ciphers = sets.NewString( | ||
| "TLS_AES_128_GCM_SHA256", | ||
|
|
@@ -35,8 +38,41 @@ var ( | |
| "TLS_AES_128_CCM_SHA256", | ||
| "TLS_AES_128_CCM_8_SHA256", | ||
| ) | ||
|
|
||
| fipsApprovedTLS13Ciphers = sets.NewString( | ||
| "TLS_AES_128_GCM_SHA256", | ||
| "TLS_AES_256_GCM_SHA384", | ||
| ) | ||
| ) | ||
|
|
||
| // detectFIPS reports whether the cluster is operating in FIPS | ||
| // mode by checking the FIPS_ENABLED environment variable if set or | ||
| // the /proc/sys/crypto/fips_enabled file otherwise. | ||
| func detectFIPS() bool { | ||
| if v, ok := os.LookupEnv("FIPS_ENABLED"); ok { | ||
| if result, err := strconv.ParseBool(v); err != nil { | ||
| klog.Warningf("Failed to parse FIPS_ENABLED environment variable: %v; falling back to procfs", err) | ||
| } else { | ||
| klog.Infof("Found FIPS_ENABLED environment variable: value=%s, result=%v", v, result) | ||
| return result | ||
| } | ||
| } | ||
|
|
||
| result := false | ||
| data, err := os.ReadFile("/proc/sys/crypto/fips_enabled") | ||
| if err != nil { | ||
| klog.Warningf("Failed to read /proc/sys/crypto/fips_enabled: %v; assuming FIPS is not enabled", err) | ||
| return result | ||
| } | ||
| if len(data) == 0 { | ||
| klog.Warningf("Got empty /proc/sys/crypto/fips_enabled; assuming FIPS is not enabled") | ||
| return result | ||
| } | ||
| result = strings.TrimSpace(string(data)) == "1" | ||
| klog.Infof("Read /proc/sys/crypto/fips_enabled: data=%s, result=%v", string(data), result) | ||
| return result | ||
| } | ||
|
|
||
| func startServiceCAController(ctx context.Context, cfg *config.Config, kubeconfigPath string) error { | ||
| var ( | ||
| //TODO: fix the rolebinding and sa | ||
|
|
@@ -230,7 +266,7 @@ func startIngressController(ctx context.Context, cfg *config.Config, kubeconfigP | |
| return err | ||
| } | ||
|
|
||
| extraParams, err := generateIngressParams(cfg) | ||
| extraParams, err := generateIngressParams(cfg, isFIPSEnabled) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now wondering if we should just call the detectFIPS() here, when starting the actual ingress controller manifests. Having it in a variable at the top means that importing this package will be using I/O immediately without using a function. |
||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
@@ -451,7 +487,7 @@ func validateClientTLS(patterns []string) error { | |
| return nil | ||
| } | ||
|
|
||
| func generateIngressParams(cfg *config.Config) (assets.RenderParams, error) { | ||
| func generateIngressParams(cfg *config.Config, fipsEnabled bool) (assets.RenderParams, error) { | ||
| routerMode := "v4" | ||
| if cfg.IsIPv6() { | ||
| routerMode = "v4v6" | ||
|
|
@@ -492,12 +528,34 @@ func generateIngressParams(cfg *config.Config) (assets.RenderParams, error) { | |
| } | ||
| } | ||
|
|
||
| // On FIPS-enabled clusters, remove non-FIPS-compliant TLS 1.3 cipher | ||
| // suites (e.g. TLS_CHACHA20_POLY1305_SHA256). HAProxy would fail TLS | ||
| // handshakes when a client offers a non-FIPS cipher first if that cipher | ||
| // is listed in ssl-default-bind-ciphersuites but excluded by the OS FIPS policy. | ||
| if fipsEnabled { | ||
| fipsCiphers := tls13Ciphers[:0] | ||
| for _, c := range tls13Ciphers { | ||
| if fipsApprovedTLS13Ciphers.Has(c) { | ||
| fipsCiphers = append(fipsCiphers, c) | ||
| } | ||
| } | ||
| tls13Ciphers = fipsCiphers | ||
| } | ||
|
|
||
| RouterCiphers := strings.Join(otherCiphers, ":") | ||
| RouterCiphersSuites := "" | ||
| if len(tls13Ciphers) != 0 { | ||
| RouterCiphersSuites = strings.Join(tls13Ciphers, ":") | ||
| } | ||
|
|
||
| // Default TLS supportedGroups (curves) include X25519MLKEM768 for | ||
| // post-quantum readiness. In FIPS mode, ML-KEM and X25519 are not | ||
| // supported by OpenSSL FIPS 140-3. | ||
| tlsCurves := "X25519MLKEM768:X25519:P-256:P-384:P-521" | ||
| if fipsEnabled { | ||
| tlsCurves = "P-256:P-384:P-521" | ||
| } | ||
|
|
||
| var RouterSSLMinVersion string | ||
| switch tlsProfileSpec.MinTLSVersion { | ||
| // TLS 1.0 is not supported, convert to TLS 1.1. | ||
|
|
@@ -589,6 +647,7 @@ func generateIngressParams(cfg *config.Config) (assets.RenderParams, error) { | |
| "RouterCiphers": RouterCiphers, | ||
| "RouterCiphersSuites": RouterCiphersSuites, | ||
| "RouterSSLMinVersion": RouterSSLMinVersion, | ||
| "RouterTLSCurves": tlsCurves, | ||
| "RouterAllowWildcardRoutes": RouterAllowWildcardRoutes, | ||
| "ClientCAMapName": clientCAMapName, | ||
| "ClientAuthPolicy": clientAuthPolicy, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,139 @@ | ||
| package components | ||
|
|
||
| import ( | ||
| "strings" | ||
| "testing" | ||
| "time" | ||
|
|
||
| configv1 "github.com/openshift/api/config/v1" | ||
| "github.com/openshift/microshift/pkg/assets" | ||
| "github.com/openshift/microshift/pkg/config" | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| "k8s.io/utils/ptr" | ||
| ) | ||
|
|
||
| func newTestConfig() *config.Config { | ||
| return &config.Config{ | ||
| DNS: config.DNS{BaseDomain: "example.com"}, | ||
| Network: config.Network{ | ||
| ClusterNetwork: []string{"10.42.0.0/16"}, | ||
| ServiceNetwork: []string{"10.43.0.0/16"}, | ||
| }, | ||
| Ingress: config.IngressConfig{ | ||
| Ports: config.IngressPortsConfig{ | ||
| Http: ptr.To(80), | ||
| Https: ptr.To(443), | ||
| }, | ||
| TuningOptions: config.IngressControllerTuningOptions{ | ||
| HeaderBufferBytes: 32768, | ||
| HeaderBufferMaxRewriteBytes: 8192, | ||
| HealthCheckInterval: &metav1.Duration{Duration: 5 * time.Second}, | ||
| ClientTimeout: &metav1.Duration{Duration: 30 * time.Second}, | ||
| ClientFinTimeout: &metav1.Duration{Duration: 1 * time.Second}, | ||
| ServerTimeout: &metav1.Duration{Duration: 30 * time.Second}, | ||
| ServerFinTimeout: &metav1.Duration{Duration: 1 * time.Second}, | ||
| TunnelTimeout: &metav1.Duration{Duration: 1 * time.Hour}, | ||
| TLSInspectDelay: &metav1.Duration{Duration: 5 * time.Second}, | ||
| ThreadCount: 4, | ||
| MaxConnections: 50000, | ||
| }, | ||
| ForwardedHeaderPolicy: "Append", | ||
| TLSSecurityProfile: &configv1.TLSSecurityProfile{ | ||
| Type: configv1.TLSProfileIntermediateType, | ||
| }, | ||
| ServingCertificateSecret: "router-certs-default", | ||
| DefaultHttpVersionPolicy: 1, | ||
| LogEmptyRequests: "Log", | ||
| HTTPEmptyRequestsPolicy: "Respond", | ||
| AccessLogging: config.AccessLogging{ | ||
| Status: config.AccessLoggingDisabled, | ||
| }, | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| func requireStringParam(t *testing.T, params assets.RenderParams, key string) string { | ||
| t.Helper() | ||
| v, ok := params[key] | ||
| if !ok { | ||
| t.Fatalf("missing param %q", key) | ||
| } | ||
| s, ok := v.(string) | ||
| if !ok { | ||
| t.Fatalf("param %q has type %T, want string", key, v) | ||
| } | ||
| return s | ||
| } | ||
|
|
||
| func TestGenerateIngressParamsFIPSCiphers(t *testing.T) { | ||
| t.Run("FIPS enabled filters non-FIPS TLS 1.3 ciphers", func(t *testing.T) { | ||
| cfg := newTestConfig() | ||
| params, err := generateIngressParams(cfg, true) | ||
| if err != nil { | ||
| t.Fatalf("unexpected error: %v", err) | ||
| } | ||
|
|
||
| cipherSuites := requireStringParam(t, params, "RouterCiphersSuites") | ||
| if strings.Contains(cipherSuites, "TLS_CHACHA20_POLY1305_SHA256") { | ||
| t.Errorf("FIPS mode should filter out TLS_CHACHA20_POLY1305_SHA256, got: %s", cipherSuites) | ||
| } | ||
| if !strings.Contains(cipherSuites, "TLS_AES_128_GCM_SHA256") { | ||
| t.Errorf("FIPS mode should keep TLS_AES_128_GCM_SHA256, got: %s", cipherSuites) | ||
| } | ||
| if !strings.Contains(cipherSuites, "TLS_AES_256_GCM_SHA384") { | ||
| t.Errorf("FIPS mode should keep TLS_AES_256_GCM_SHA384, got: %s", cipherSuites) | ||
| } | ||
| }) | ||
|
|
||
| t.Run("non-FIPS keeps all TLS 1.3 ciphers", func(t *testing.T) { | ||
| cfg := newTestConfig() | ||
| params, err := generateIngressParams(cfg, false) | ||
| if err != nil { | ||
| t.Fatalf("unexpected error: %v", err) | ||
| } | ||
|
|
||
| cipherSuites := requireStringParam(t, params, "RouterCiphersSuites") | ||
| if !strings.Contains(cipherSuites, "TLS_CHACHA20_POLY1305_SHA256") { | ||
| t.Errorf("non-FIPS mode should keep TLS_CHACHA20_POLY1305_SHA256, got: %s", cipherSuites) | ||
| } | ||
| }) | ||
| } | ||
|
|
||
| func TestGenerateIngressParamsFIPSCurves(t *testing.T) { | ||
| t.Run("FIPS enabled uses only NIST curves", func(t *testing.T) { | ||
| cfg := newTestConfig() | ||
| params, err := generateIngressParams(cfg, true) | ||
| if err != nil { | ||
| t.Fatalf("unexpected error: %v", err) | ||
| } | ||
|
|
||
| curves := requireStringParam(t, params, "RouterTLSCurves") | ||
| if strings.Contains(curves, "X25519MLKEM768") { | ||
| t.Errorf("FIPS mode should exclude X25519MLKEM768, got: %s", curves) | ||
| } | ||
| for _, c := range strings.Split(curves, ":") { | ||
| if c == "X25519" { | ||
| t.Errorf("FIPS mode should exclude X25519, got: %s", curves) | ||
| } | ||
| } | ||
| if curves != "P-256:P-384:P-521" { | ||
| t.Errorf("FIPS mode curves should be P-256:P-384:P-521, got: %s", curves) | ||
| } | ||
| }) | ||
|
|
||
| t.Run("non-FIPS includes PQC hybrid curve", func(t *testing.T) { | ||
| cfg := newTestConfig() | ||
| params, err := generateIngressParams(cfg, false) | ||
| if err != nil { | ||
| t.Fatalf("unexpected error: %v", err) | ||
| } | ||
|
|
||
| curves := requireStringParam(t, params, "RouterTLSCurves") | ||
| if !strings.Contains(curves, "X25519MLKEM768") { | ||
| t.Errorf("non-FIPS mode should include X25519MLKEM768, got: %s", curves) | ||
| } | ||
| if curves != "X25519MLKEM768:X25519:P-256:P-384:P-521" { | ||
| t.Errorf("non-FIPS mode curves should be X25519MLKEM768:X25519:P-256:P-384:P-521, got: %s", curves) | ||
| } | ||
| }) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -51,6 +51,10 @@ TLS Scanner Host Scan Completes And Produces Artifacts | |
| ... Cleanup TLS Scanner Job | ||
| ... Ensure Cluster Reader Role Deleted | ||
|
|
||
| Ingress Router TLS Curves supports ML-KEM Post Quantum Curves | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now wondering if the router part fits in the router.robot file? |
||
| [Documentation] Verify TLS curve negotiation with openssl from inside the router pod. | ||
| Verify ML-KEM Post Quantum Curve Negotiation | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
|
|
||
| *** Keywords *** | ||
| Setup | ||
|
|
@@ -137,3 +141,23 @@ Cleanup TLS Scanner Job | |
| IF '${TLS_SCANNER_DIR}' != '' | ||
| Run Keyword And Ignore Error Remove Directory ${TLS_SCANNER_DIR} recursive=True | ||
| END | ||
|
|
||
| Verify ML-KEM Post Quantum Curve Negotiation | ||
| [Documentation] Verify X25519MLKEM768 post-quantum hybrid key exchange | ||
| ... negotiates successfully via oc exec into the router pod, which | ||
| ... has OpenSSL 3.5+ (the host OpenSSL may be too old for ML-KEM). | ||
| ... Skipped on FIPS clusters where ML-KEM is not configured. | ||
|
eslutsky marked this conversation as resolved.
|
||
| ${curves}= Oc Get JsonPath deployment openshift-ingress router-default | ||
| ... .spec.template.spec.containers[0].env[?(@.name=="ROUTER_CURVES")].value | ||
| Skip If "X25519MLKEM768" not in """${curves}""" | ||
| ... ROUTER_CURVES does not include X25519MLKEM768 (FIPS mode); skipping ML-KEM test | ||
| ${router_ip}= Oc Get JsonPath svc openshift-ingress router-default | ||
| ... .spec.clusterIP | ||
| ${pod_name}= Oc Get JsonPath pod openshift-ingress ${EMPTY} | ||
| ... .items[0].metadata.name | ||
| ${output}= Oc Exec ${pod_name} | ||
| ... echo Q | openssl s_client -connect ${router_ip}:443 -groups X25519MLKEM768 2>&1 || true | ||
| ... ns=openshift-ingress | ||
| Should Contain ${output} Negotiated TLS1.3 group: X25519MLKEM768 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So if it doesnt exist, what is the message? Is there any way to make this a bit more loose on the string matching? In the end this is like a log line, which could change anytime? |
||
| ... msg=ML-KEM post-quantum curve X25519MLKEM768 negotiation failed | ||
| Log Post-quantum ML-KEM negotiation verified: OK | ||
Uh oh!
There was an error while loading. Please reload this page.