Skip to content

[release-4.16] OCPBUGS-86713: Strip X-SSL-* headers for plain HTTP#805

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

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

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 : #804

Summary by CodeRabbit

  • Bug Fixes
    • Improved protection against header spoofing by removing X-SSL and related client identity headers on supported traffic paths unless explicitly allowed.
    • Applied the same header filtering consistently for plain HTTP and TLS connections.
    • Existing request-safety protections remain in place, including blocking empty Content-Length requests and removing the Proxy header.

@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-86713, which is invalid:

  • expected dependent Jira Issue OCPBUGS-86714 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 : #804

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.

@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown

Walkthrough

The HAProxy configuration template adds a conditional block to all three frontends (frontend public, fe_sni, fe_no_sni) that deletes X-SSL and X-SSL-Client-* request headers when ROUTER_MUTUAL_TLS_HEADER_FILTER is set to true (default).

Changes

X-SSL Header Spoofing Prevention

Layer / File(s) Summary
X-SSL header stripping on all frontends
images/router/haproxy/conf/haproxy-config.template
Adds a ROUTER_MUTUAL_TLS_HEADER_FILTER-guarded http-request del-header block for X-SSL and X-SSL-Client-* headers in the plain HTTP frontend and both TLS frontends (fe_sni, fe_no_sni).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error)

Check name Status Explanation Resolution
No-Weak-Crypto ❌ Error The added HAProxy template includes weak cipher suites like DES-CBC3-SHA/3DES and DES/RC4/MD5 references, which violates the no-weak-crypto check. Remove weak cipher-suite entries and keep only modern, approved TLS ciphers in the template defaults.
✅ Passed checks (14 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main security fix: stripping X-SSL-* headers for plain HTTP requests.
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 PR only changes a HAProxy template; no Ginkgo test titles were added or modified, so the stability check is not applicable.
Test Structure And Quality ✅ Passed PR only changes the HAProxy template; no Ginkgo test code was modified, so this test-structure checklist is not applicable.
Microshift Test Compatibility ✅ Passed No new Ginkgo e2e tests were added; the patch only changes HAProxy config and unit tests, with no MicroShift-sensitive APIs or guards to review.
Single Node Openshift (Sno) Test Compatibility ✅ Passed No new Ginkgo e2e tests were added; the PR only changes a HAProxy config template, so SNO test compatibility is not applicable.
Topology-Aware Scheduling Compatibility ✅ Passed Only HAProxy header-stripping template logic changed; no manifests/controllers or topology-aware scheduling constraints were added.
Ote Binary Stdout Contract ✅ Passed Only images/router/haproxy/conf/haproxy-config.template changed; no process-level Go code (main/TestMain/init) was modified, so no stdout contract violation.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed The PR only changes the HAProxy template; no new Ginkgo e2e tests or network-test code were added.
Container-Privileges ✅ Passed PASS: The PR only changes haproxy-config.template; no container/K8s manifests or securityContext/privilege fields were added or modified.
No-Sensitive-Data-In-Logs ✅ Passed The PR only adds header-stripping rules and an env gate; it does not add logging or capture of sensitive values.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@openshift-ci openshift-ci Bot requested review from alebedev87 and candita June 29, 2026 17:53
@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 ironcladlou 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

@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.

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@images/router/haproxy/conf/haproxy-config.template`:
- Around line 244-259: The HAProxy header-scrubbing blocks in
haproxy-config.template omit X-SSL-Client-Cert, leaving it spoofable even though
downstream code relies on it. Update every mutual-TLS header filter block in the
template to also delete X-SSL-Client-Cert alongside the other X-SSL-Client-*
headers, so the router’s mTLS identity hardening is consistent across all
listeners.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

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

Review profile: CHILL

Plan: Enterprise

Run ID: 2bb05804-187f-4b8d-a5ef-89999e386aba

📥 Commits

Reviewing files that changed from the base of the PR and between cc38043 and 22d9c63.

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

Comment on lines +244 to +259
# 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 }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

X-SSL-Client-Cert is still spoofable.

These blocks claim to strip X-SSL-Client-* headers, but they omit X-SSL-Client-Cert. That header is already used downstream (pkg/router/router_test.go, Line 479 onward), so a client can still inject it and bypass the intended mTLS identity hardening for backends that trust it.

Suggested fix
   http-request del-header X-SSL-Client-CN
+  http-request del-header X-SSL-Client-Cert
   http-request del-header X-SSL-Client-DER

Also applies to: 378-394, 511-527

🤖 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 - 259,
The HAProxy header-scrubbing blocks in haproxy-config.template omit
X-SSL-Client-Cert, leaving it spoofable even though downstream code relies on
it. Update every mutual-TLS header filter block in the template to also delete
X-SSL-Client-Cert alongside the other X-SSL-Client-* headers, so the router’s
mTLS identity hardening is consistent across all listeners.

@openshift-ci

openshift-ci Bot commented Jun 29, 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.

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants