[release-4.13] OCPBUGS-86710: Strip X-SSL-* headers for plain HTTP#808
[release-4.13] OCPBUGS-86710: Strip X-SSL-* headers for plain HTTP#808MrSanketkumar wants to merge 1 commit into
Conversation
|
@MrSanketkumar: This pull request references Jira Issue OCPBUGS-86710, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
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. |
WalkthroughThe HAProxy config template adds ChangesmTLS Header Stripping
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
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 228-243: The mutual-TLS header stripping in the
haproxy-config.template blocks is only enabled when isTrue(env(..., "true"))
evaluates true, which can silently disable the mitigation for malformed
non-empty values. Update the ROUTER_MUTUAL_TLS_HEADER_FILTER check so the filter
stays enabled by default unless the variable is explicitly set to a recognized
false value, and reuse the same logic in the other matching header-filter
sections to keep behavior consistent.
🪄 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: 8a255777-f969-4646-a1b0-fe7515d3114b
📒 Files selected for processing (1)
images/router/haproxy/conf/haproxy-config.template
| # 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 }} |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Keep the filter enabled on malformed env values.
isTrue(env(..., "true")) is only secure-by-default when the variable is unset. If ROUTER_MUTUAL_TLS_HEADER_FILTER is set to any non-empty typo, env() returns that value and isTrue() falls back to false, which silently disables the CVE mitigation without an explicit opt-out.
Suggested fix
- {{- if isTrue (env "ROUTER_MUTUAL_TLS_HEADER_FILTER" "true") }}
+ {{- $mtlsHeaderFilter := firstMatch "(?i:true|false)" (env "ROUTER_MUTUAL_TLS_HEADER_FILTER") "true" }}
+ {{- if isTrue $mtlsHeaderFilter }}Also applies to: 341-357, 453-469
🤖 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 228 - 243,
The mutual-TLS header stripping in the haproxy-config.template blocks is only
enabled when isTrue(env(..., "true")) evaluates true, which can silently disable
the mitigation for malformed non-empty values. Update the
ROUTER_MUTUAL_TLS_HEADER_FILTER check so the filter stays enabled by default
unless the variable is explicitly set to a recognized false value, and reuse the
same logic in the other matching header-filter sections to keep behavior
consistent.
|
/test e2e-aws-serial |
1 similar comment
|
/test e2e-aws-serial |
|
@MrSanketkumar: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
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:
Backport of : #807
Summary by CodeRabbit