Skip to content

[release-4.14] OCPBUGS-86711: Strip X-SSL-* headers for plain HTTP#807

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

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

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

Summary by CodeRabbit

  • Security Improvements
    • Incoming requests now have mutual-TLS identity headers removed at the edge when header filtering is enabled, reducing the risk of spoofed client identity data.
    • This protection applies across both plain HTTP and TLS termination entrypoints.

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

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

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 config template adds conditional http-request del-header rules for X-SSL and X-SSL-Client-* headers across all three frontends (plain HTTP, fe_sni, fe_no_sni), guarded by ROUTER_MUTUAL_TLS_HEADER_FILTER (default "true").

Changes

mTLS Header Stripping in HAProxy Frontends

Layer / File(s) Summary
X-SSL* header deletion across all frontends
images/router/haproxy/conf/haproxy-config.template
Adds identical conditional blocks to the plain HTTP frontend (line 245), fe_sni (line 376), and fe_no_sni (line 506) that delete X-SSL and all X-SSL-Client-* headers when ROUTER_MUTUAL_TLS_HEADER_FILTER is enabled.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 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
Container-Privileges ❌ Error deploy/router.yaml adds hostNetwork: true in the pod spec, which the check explicitly flags in Kubernetes manifests. Remove hostNetwork: true or document/justify the privilege requirement and ensure no other privileged settings are introduced.
✅ 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 matches the main security fix of stripping X-SSL-* headers, though it only mentions plain HTTP and omits the TLS frontend changes.
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 a HAProxy config template changed; it contains no Ginkgo test titles or dynamic test-name strings.
Test Structure And Quality ✅ Passed No Ginkgo tests were changed; the only keyword hit is plain testing in template_helper_test.go, with no Describe/It/Eventually blocks.
Microshift Test Compatibility ✅ Passed No new Ginkgo e2e tests were added; the PR only changes router config and unit tests, with no MicroShift-unsupported APIs in new test code.
Single Node Openshift (Sno) Test Compatibility ✅ Passed No new Ginkgo e2e tests were added; the PR only changes the router HAProxy template, so SNO test compatibility is not applicable.
Topology-Aware Scheduling Compatibility ✅ Passed The PR only changes HAProxy header filtering in a config template; no Deployment/controller scheduling fields or topology assumptions were added.
Ote Binary Stdout Contract ✅ Passed Only the HAProxy template changed; no main/TestMain/suite setup code or stdout logging was introduced.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed No Ginkgo e2e tests were added; scans found no Describe/It/When/Context usage or e2e test paths, and the change is only the HAProxy template.
No-Weak-Crypto ✅ Passed The patch only strips existing X-SSL-Client-SHA1/mTLS headers; no new weak crypto, custom crypto, or secret comparison logic was added.
No-Sensitive-Data-In-Logs ✅ Passed Only X-SSL header-stripping rules were added in three frontends; no new logging or log-format changes expose sensitive data.
✨ 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 ironcladlou June 29, 2026 18:02
@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 alebedev87 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.

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

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

Optional: the identical 12-line strip block is repeated in all three frontends.

This security-critical list now lives in three places (here, 380-391, 510-521) and can silently drift out of sync with the set-header list. A single {{- define }}/{{- template }} block would keep them aligned. Given this is a backport of #806, only do this if it stays consistent with upstream; otherwise prefer matching upstream verbatim.

🤖 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 247 - 260,
The repeated mutual TLS header stripping block in haproxy-config.template is
duplicated across the frontend sections and can drift from the set-header list.
Extract the 12-line delete sequence into a single reusable template with a
unique name using {{- define }} and render it with {{- template }} in each
frontend, keeping the existing ROUTER_MUTUAL_TLS_HEADER_FILTER gate and matching
upstream behavior if this backport needs to stay verbatim.
🤖 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 247-260: The repeated mutual TLS header stripping block in
haproxy-config.template is duplicated across the frontend sections and can drift
from the set-header list. Extract the 12-line delete sequence into a single
reusable template with a unique name using {{- define }} and render it with {{-
template }} in each frontend, keeping the existing
ROUTER_MUTUAL_TLS_HEADER_FILTER gate and matching upstream behavior if this
backport needs to stay verbatim.

ℹ️ Review info
⚙️ Run configuration

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

Review profile: CHILL

Plan: Enterprise

Run ID: 4dfe00a5-018d-452e-9faa-cdc6852db232

📥 Commits

Reviewing files that changed from the base of the PR and between acad61a and f61fa0a.

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

@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