Skip to content

feat(tektonconfig): add ML-KEM group to nginx for PQC readiness#3711

Closed
jkhelil wants to merge 3 commits into
tektoncd:mainfrom
jkhelil:feat/pqc-nginx-mlkem-ssl-ecdh-curve
Closed

feat(tektonconfig): add ML-KEM group to nginx for PQC readiness#3711
jkhelil wants to merge 3 commits into
tektoncd:mainfrom
jkhelil:feat/pqc-nginx-mlkem-ssl-ecdh-curve

Conversation

@jkhelil

@jkhelil jkhelil commented Jul 13, 2026

Copy link
Copy Markdown
Member

Summary

  • The OpenShift TLS FAQ mandates TLS 1.3 servers negotiate ML-KEM if the client supports it (pqc_capable=true is a mandatory requirement)
  • nginx/OpenSSL requires an explicit ssl_ecdh_curve directive to advertise post-quantum groups; Go-based components get this for free via Go 1.24+ but nginx does not
  • Added tlsECDHGroups() that always emits ssl_ecdh_curve, with a FIPS gate to prevent nginx crashes

What changes

console_plugin_reconciler.go:

  • tlsECDHGroups() — returns X25519MLKEM768:X25519:secp256r1:secp384r1 on non-FIPS clusters, X25519:secp256r1:secp384r1 on FIPS clusters
  • isFIPSEnabled() — reads /proc/sys/crypto/fips_enabled (kernel file, readable from containers without privileges)
  • fipsEnabledPath — package-level var so tests can override it with a temp file

Why FIPS gating is needed:
X25519MLKEM768 is not FIPS-approved. OpenSSL's FIPS provider rejects it with a fatal error that crashes nginx:

nginx: [emerg] SSL_CONF_cmd("Groups","X25519MLKEM768:X25519") failed

Per Davide's openshift/api announcement: "components running in FIPS mode must ignore [X25519MLKEM768]".

Why hardcoded (not from APIServer):
library-go's ObserveTLSSecurityProfile does not yet expose the groups field from the APIServer TLS profile — confirmed by @ddonati in the TLS crypto Slack channel (#C098FU5MRAB). The tracking PR is openshift/library-go#2347 (open). Once merged, the hardcoded list will be replaced by dynamic propagation from the APIServer profile.

Test plan

  • TestIsFIPSEnabled — verifies all three states: missing file, 0, 1
  • TestTLSECDHGroups — verifies ML-KEM included on non-FIPS, excluded on FIPS (using temp file override)
  • TestBuildNginxTLSDirectives — all cases assert ssl_ecdh_curve X25519MLKEM768:X25519:secp256r1:secp384r1
  • TestGenerateNginxConfWithTLS — end-to-end nginx.conf generation
  • TestNginxTLSIntegration — full reconciler integration
  • Manual: deploy on non-FIPS cluster, run TLS scanner → expect pqc_capable=true
  • Manual: deploy on FIPS cluster → expect no crash, pqc_capable=false (acceptable per crypto team guidance)

Made with Cursor

jkhelil and others added 3 commits July 13, 2026 16:01
Ship fixed NetworkPolicies for the operator's own namespace
(tekton-operator / openshift-operators) as static manifests
alongside the Deployment/RBAC, scoping the main operator and CR
admission webhook pods for ingress/egress. No CR watches the
operator's own namespace, so these cannot be reconciled and ship
statically instead. Each podSelector targets this operator's own
pod labels only, never an empty/namespace-wide selector, so the
bundle never affects other pods sharing openshift-operators.

Related to SRVKP-12050.

Signed-off-by: Jawed khelil <jkhelil@redhat.com>
Assisted-by: Claude Sonnet 5 (via Cursor)
Co-authored-by: Cursor <cursoragent@cursor.com>
Reuse the NetworkPolicy foundation built for TektonTrigger to scope
ingress/egress for the proxy-webhook workload TektonPipeline
deploys into the operand namespace. Adds spec.networkPolicy to
TektonPipeline, propagated from TektonConfig, and reconciles it via
a new custom InstallerSet, mirroring the TektonTrigger pattern.

Related to SRVKP-12050.

Signed-off-by: Jawed khelil <jkhelil@redhat.com>
Assisted-by: Claude Sonnet 5 (via Cursor)
Co-authored-by: Cursor <cursoragent@cursor.com>
The OpenShift TLS FAQ mandates that every TLS 1.3 server negotiate
ML-KEM if the client supports it. nginx/OpenSSL requires an explicit
ssl_ecdh_curve directive to advertise post-quantum groups; without
it the TLS scanner reports pqc_capable=false.

Add tlsECDHGroups() that emits:
  X25519MLKEM768:X25519:secp256r1:secp384r1  (non-FIPS)
  X25519:secp256r1:secp384r1                 (FIPS)

X25519MLKEM768 is not FIPS-approved: OpenSSL's FIPS provider rejects
it with a fatal error crashing nginx. isFIPSEnabled() detects FIPS
mode by reading /proc/sys/crypto/fips_enabled, which is accessible
from inside containers via the shared host kernel.

The group list is hardcoded until library-go's
ObserveTLSSecurityProfile exposes the groups field from the APIServer
TLS profile (openshift/library-go#2347, currently open). At that
point the hardcoded list is replaced by dynamic propagation.

Signed-off-by: Jawed khelil <jkhelil@redhat.com>
Assisted-by: Claude Sonnet 4.6 (via Cursor)
Co-authored-by: Cursor <cursoragent@cursor.com>
@tekton-robot

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please ask for approval from jkhelil after the PR has been reviewed.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@tekton-robot tekton-robot added do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 13, 2026
@jkhelil

jkhelil commented Jul 13, 2026

Copy link
Copy Markdown
Member Author

/release-note-none

@tekton-robot tekton-robot added release-note-none Denotes a PR that doesnt merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jul 13, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 25.48%. Comparing base (9f33b03) to head (6e13655).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3711      +/-   ##
==========================================
- Coverage   25.51%   25.48%   -0.03%     
==========================================
  Files         448      449       +1     
  Lines       23309    23370      +61     
==========================================
+ Hits         5948     5957       +9     
- Misses      16675    16726      +51     
- Partials      686      687       +1     
Flag Coverage Δ
unit-tests 25.48% <ø> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jkhelil jkhelil closed this Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-note-none Denotes a PR that doesnt merit a release note. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants