Skip to content

fix: SslHandler retrieval for HTTP/1.1 client certificate auth - #22509

Open
jasstionzyf wants to merge 1 commit into
opensearch-project:mainfrom
jasstionzyf:fix/22504-http1-client-cert-auth
Open

fix: SslHandler retrieval for HTTP/1.1 client certificate auth#22509
jasstionzyf wants to merge 1 commit into
opensearch-project:mainfrom
jasstionzyf:fix/22504-http1-client-cert-auth

Conversation

@jasstionzyf

Copy link
Copy Markdown

Summary

Fixes #22504 — SslHandler retrieval in ReactorNetty4BaseHttpChannel.get() prioritizes the parent channel over the current channel, breaking client certificate authentication on HTTP/1.1.

Root cause

For HTTP/1.1 connections, the connection channel has a parent server channel, but its SslHandler is on the current channel. The original code checked parent() first, which returned null — so the Security plugin could not retrieve the SSLEngine or peer certificate.

HTTP/2 stream channels correctly inherit the TLS handler from the parent, so HTTP/2 was not affected.

Fix

Check the current channel first, and fall back to the parent only when no SslHandler is found:

channels[0] = channel.pipeline().get(NettyPipeline.SslHandler);
if (channels[0] == null && channel.parent() != null) {
    channels[0] = channel.parent().pipeline().get(NettyPipeline.SslHandler);
}

Verified matrix (by reporter)

Protocol Auth Result
HTTP/1.1 valid cert 200 ✓
HTTP/2 valid cert 200 ✓
HTTP/1.1 no auth 401 ✓
HTTP/2 no auth 401 ✓
HTTP/1.1 basic auth 200 ✓
HTTP/2 basic auth 200 ✓

Issues resolved

Closes #22504

Signed-off-by: jasstionzyf jasstionzyf@gmail.com

…earch-project#22504)

The SslHandler retrieval in ReactorNetty4BaseHttpChannel.get() prioritizes
the parent channel's pipeline over the current channel's. For HTTP/1.1, the
connection channel has a parent server channel, but its SslHandler is on the
current channel — causing the parent lookup to return null and breaking
client certificate authentication.

Fix: check the current channel's pipeline first, and fall back to the parent
only when no SslHandler is found on the current channel.

Signed-off-by: jasstionzyf <jasstionzyf@gmail.com>
@jasstionzyf
jasstionzyf requested a review from a team as a code owner July 20, 2026 02:12
@github-actions

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 No relevant tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ No major issues detected

@github-actions

Copy link
Copy Markdown
Contributor

✅ Gradle check result for 4155b2b: SUCCESS

@codecov

codecov Bot commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.55%. Comparing base (c93e9af) to head (4155b2b).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...p/reactor/netty4/ReactorNetty4BaseHttpChannel.java 0.00% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #22509      +/-   ##
============================================
+ Coverage     73.43%   73.55%   +0.11%     
- Complexity    76472    76538      +66     
============================================
  Files          6104     6104              
  Lines        346573   346587      +14     
  Branches      49886    49888       +2     
============================================
+ Hits         254514   254916     +402     
+ Misses        71798    71441     -357     
+ Partials      20261    20230      -31     

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

@reta

reta commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

This is duplicate of #22506

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] transport-reactor-netty4 client certificate auth fails on HTTP/1.1

2 participants