Skip to content

[release-4.17] OCPBUGS-86714: Strip X-SSL-* headers for plain HTTP#804

Open
MrSanketkumar wants to merge 1 commit into
openshift:release-4.17from
MrSanketkumar:CVE-2026-46579-4.17
Open

[release-4.17] OCPBUGS-86714: Strip X-SSL-* headers for plain HTTP#804
MrSanketkumar wants to merge 1 commit into
openshift:release-4.17from
MrSanketkumar:CVE-2026-46579-4.17

Conversation

@MrSanketkumar

@MrSanketkumar MrSanketkumar commented Jun 29, 2026

Copy link
Copy Markdown

Vulnerability: CVE-2026-46579 - mTLS client certificate spoofing via HTTP header injection

Fix: Prevents unauthenticated spoofing of mutual TLS client identities by stripping X-SSL-Client-* headers from HTTP requests before they reach backends.

Changes:

  • Adds `ROUTER_MUTUAL_TLS_HEADER_FILTER` environment variable (default: `true`)
  • Strips all 12 X-SSL headers in HTTP frontends: `public`, `fe_sni`, `fe_no_sni`
  • Secure by default - header stripping enabled unless explicitly disabled

Backport : #800

Summary by CodeRabbit

  • Bug Fixes
    • Improved request security by removing client certificate identity headers from incoming traffic in supported HTTPS and HTTP paths.
    • Added a configurable on/off switch for this header filtering, enabled by default.

@openshift-ci-robot openshift-ci-robot added jira/severity-important Referenced Jira bug's severity is important for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Jun 29, 2026
@openshift-ci-robot

Copy link
Copy Markdown
Contributor

@MrSanketkumar: This pull request references Jira Issue OCPBUGS-86714, which is invalid:

  • expected dependent Jira Issue OCPBUGS-86715 to be in one of the following states: VERIFIED, RELEASE PENDING, CLOSED (ERRATA), CLOSED (CURRENT RELEASE), CLOSED (DONE), CLOSED (DONE-ERRATA), but it is ASSIGNED instead

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

Details

In response to this:

Vulnerability: CVE-2026-46579 - mTLS client certificate spoofing via HTTP header injection

Fix: Prevents unauthenticated spoofing of mutual TLS client identities by stripping X-SSL-Client-* headers from HTTP requests before they reach backends.

Changes:

  • Adds `ROUTER_MUTUAL_TLS_HEADER_FILTER` environment variable (default: `true`)
  • Strips all 12 X-SSL headers in HTTP frontends: `public`, `fe_sni`, `fe_no_sni`
  • Secure by default - header stripping enabled unless explicitly disabled

Backport : #800

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci openshift-ci Bot requested review from candita and gcs278 June 29, 2026 17:49
@openshift-ci

openshift-ci Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign miciah for approval. For more information see the Code Review Process.

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

@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown

Walkthrough

The HAProxy configuration template adds conditional X-SSL* header deletion to all three frontends (public, fe_sni, fe_no_sni). The stripping is gated on ROUTER_MUTUAL_TLS_HEADER_FILTER (default: enabled) and removes X-SSL plus all X-SSL-Client-* fields to prevent spoofing of mTLS identity headers.

Changes

mTLS Header Spoof Prevention

Layer / File(s) Summary
X-SSL* header stripping across all frontends
images/router/haproxy/conf/haproxy-config.template
Identical conditional blocks, guarded by ROUTER_MUTUAL_TLS_HEADER_FILTER, delete X-SSL and all X-SSL-Client-* headers from incoming requests in the plain HTTP (frontend public), SNI (fe_sni), and non-SNI (fe_no_sni) frontends.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

🚥 Pre-merge checks | ✅ 15
✅ Passed checks (15 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title matches the main fix and mentions the X-SSL header stripping, though it understates that it applies beyond plain HTTP.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Stable And Deterministic Test Names ✅ Passed Only haproxy-config.template changed; no test files or Ginkgo titles were added or modified.
Test Structure And Quality ✅ Passed No Ginkgo-style tests were added or changed; the repo's tests use standard Go testing, so the check is not applicable.
Microshift Test Compatibility ✅ Passed The PR only changes an HAProxy config template; no Ginkgo e2e tests or MicroShift-incompatible APIs/features were added.
Single Node Openshift (Sno) Test Compatibility ✅ Passed PR only changes the HAProxy template; no new Ginkgo e2e tests or multi-node/SNO assumptions were added.
Topology-Aware Scheduling Compatibility ✅ Passed The only change is HAProxy request-header filtering in a config template; no manifests, controllers, pod affinity, selectors, or replica logic were added.
Ote Binary Stdout Contract ✅ Passed No new stdout writes were added in process-level entrypoints; the diff adds no main/TestMain/init stdout or logging lines.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed Only the HAProxy config template was changed; no new Ginkgo e2e tests or external connectivity code were added.
No-Weak-Crypto ✅ Passed The patch only adds X-SSL header stripping; no new MD5/SHA1/DES/RC4/3DES/Blowfish/ECB use or secret comparisons were introduced.
Container-Privileges ✅ Passed No privileged/hostNetwork/allowPrivilegeEscalation settings were added in the PR’s changed files; it only edits the HAProxy template.
No-Sensitive-Data-In-Logs ✅ Passed The diff only adds X-SSL* request-header stripping; it introduces no new log/capture directives or sensitive values in logs.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@MrSanketkumar

Copy link
Copy Markdown
Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
images/router/haproxy/conf/haproxy-config.template (1)

244-260: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consolidate the repeated X-SSL* deletions. Replace the 12 explicit del-header lines in each frontend with http-request del-header X-SSL -m beg to keep the behavior the same while removing the duplicated allowlist.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@images/router/haproxy/conf/haproxy-config.template` around lines 244 - 260,
Consolidate the repeated X-SSL header removals in the HAProxy template: the
current block in haproxy-config.template uses many explicit http-request
del-header directives for each X-SSL-* variant, but it should be replaced with a
single prefix-based delete so the behavior stays the same while removing the
duplicated allowlist. Update the header-filter block guarded by
ROUTER_MUTUAL_TLS_HEADER_FILTER in the template to use the broader X-SSL match,
and keep the surrounding conditional and comment intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@images/router/haproxy/conf/haproxy-config.template`:
- Around line 244-260: Consolidate the repeated X-SSL header removals in the
HAProxy template: the current block in haproxy-config.template uses many
explicit http-request del-header directives for each X-SSL-* variant, but it
should be replaced with a single prefix-based delete so the behavior stays the
same while removing the duplicated allowlist. Update the header-filter block
guarded by ROUTER_MUTUAL_TLS_HEADER_FILTER in the template to use the broader
X-SSL match, and keep the surrounding conditional and comment intact.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 75f46c42-ddb5-465b-967a-0c9d975be79b

📥 Commits

Reviewing files that changed from the base of the PR and between cbd6d04 and eba53ed.

📒 Files selected for processing (1)
  • images/router/haproxy/conf/haproxy-config.template

@MrSanketkumar

Copy link
Copy Markdown
Author

/test e2e-aws-serial

@openshift-ci

openshift-ci Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

@MrSanketkumar: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@UdayYendva

Copy link
Copy Markdown

I’ve created a cluster using the PR that includes the merged fix from PR #804. The corresponding ClusterVersion is: 4.17.0-0-2026-06-30-072506-test-ci-ln-3gbws4k-latest

oc get clusterversion
NAME      VERSION                                                AVAILABLE   PROGRESSING   SINCE   STATUS
version   4.17.0-0-2026-06-30-072506-test-ci-ln-3gbws4k-latest   True        False         40m     Cluster version is 4.17.0-0-2026-06-30-072506-test-ci-ln-3gbws4k-latest
oc project openshift-ingress
Now using project "openshift-ingress" on server "https://api.ci-ln-nx25ys2-76ef8.aws-4.ci.openshift.org:6443".
oc get pods
NAME                              READY   STATUS    RESTARTS      AGE
router-default-57988fd4df-2thfj   1/1     Running   0             50m
router-default-57988fd4df-p7htn   1/1     Running   2 (52m ago)   65m

After inspecting one of the pods, I confirmed that the latest changes from the PR are present in the Router , SNI and NO SNI code paths, Therefore succesfully indicating that the fix has been successfully deployed
These are the changes from the PR are correctly reflected in the running ingress/router pods.

oc rsh pods/router-default-57988fd4df-2thfj 
sh-5.1$ cat haproxy
haproxy-config.template  haproxy.config           
sh-5.1$ cat haproxy-config.template 

These are the same changes that were introduced in #804

  # Strip off X-SSL* headers for plain HTTP if not explicitly disabled.
  # This prevents unauthenticated spoofing of mutual TLS client identities.
  {{- if isTrue (env "ROUTER_MUTUAL_TLS_HEADER_FILTER" "true") }}
  http-request del-header X-SSL
  http-request del-header X-SSL-Client-CN
  http-request del-header X-SSL-Client-DER
  http-request del-header X-SSL-Client-DN
  http-request del-header X-SSL-Client-NotAfter
  http-request del-header X-SSL-Client-NotBefore
  http-request del-header X-SSL-Client-SHA1
  http-request del-header X-SSL-Client-Serial
  http-request del-header X-SSL-Client-Subject
  http-request del-header X-SSL-Client-Verify
  http-request del-header X-SSL-Client-Version
  http-request del-header X-SSL-Issuer
  {{- end }}

/Verified by ci

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Jun 30, 2026
@openshift-ci-robot

Copy link
Copy Markdown
Contributor

@UdayYendva: This PR has been marked as verified by ci.

Details

In response to this:

I’ve created a cluster using the PR that includes the merged fix from PR #804. The corresponding ClusterVersion is: 4.17.0-0-2026-06-30-072506-test-ci-ln-3gbws4k-latest

oc get clusterversion
NAME      VERSION                                                AVAILABLE   PROGRESSING   SINCE   STATUS
version   4.17.0-0-2026-06-30-072506-test-ci-ln-3gbws4k-latest   True        False         40m     Cluster version is 4.17.0-0-2026-06-30-072506-test-ci-ln-3gbws4k-latest
oc project openshift-ingress
Now using project "openshift-ingress" on server "https://api.ci-ln-nx25ys2-76ef8.aws-4.ci.openshift.org:6443".
oc get pods
NAME                              READY   STATUS    RESTARTS      AGE
router-default-57988fd4df-2thfj   1/1     Running   0             50m
router-default-57988fd4df-p7htn   1/1     Running   2 (52m ago)   65m

After inspecting one of the pods, I confirmed that the latest changes from the PR are present in the Router , SNI and NO SNI code paths, Therefore succesfully indicating that the fix has been successfully deployed
These are the changes from the PR are correctly reflected in the running ingress/router pods.

oc rsh pods/router-default-57988fd4df-2thfj 
sh-5.1$ cat haproxy
haproxy-config.template  haproxy.config           
sh-5.1$ cat haproxy-config.template 

These are the same changes that were introduced in #804

  # Strip off X-SSL* headers for plain HTTP if not explicitly disabled.
  # This prevents unauthenticated spoofing of mutual TLS client identities.
  {{- if isTrue (env "ROUTER_MUTUAL_TLS_HEADER_FILTER" "true") }}
  http-request del-header X-SSL
  http-request del-header X-SSL-Client-CN
  http-request del-header X-SSL-Client-DER
  http-request del-header X-SSL-Client-DN
  http-request del-header X-SSL-Client-NotAfter
  http-request del-header X-SSL-Client-NotBefore
  http-request del-header X-SSL-Client-SHA1
  http-request del-header X-SSL-Client-Serial
  http-request del-header X-SSL-Client-Subject
  http-request del-header X-SSL-Client-Verify
  http-request del-header X-SSL-Client-Version
  http-request del-header X-SSL-Issuer
  {{- end }}

/Verified by ci

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. jira/severity-important Referenced Jira bug's severity is important for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants