Skip to content

Allow non-SNI requests and fix 421 responses#4878

Merged
sjberman merged 2 commits into
mainfrom
fix/https-sni
Mar 11, 2026
Merged

Allow non-SNI requests and fix 421 responses#4878
sjberman merged 2 commits into
mainfrom
fix/https-sni

Conversation

@sjberman

@sjberman sjberman commented Mar 3, 2026

Copy link
Copy Markdown
Contributor

Problem: Some frontend load balancers strip out SNI info before the client request reaches the NGINX Gateway. With the current behavior, the request will hit our default_server and the handshake is rejected (handshake rejected while SSL handshaking). It will never be forwarded to the proper server block to handle that request and proxy to the backend.

In addition, we always return a 421 when the SNI and Host headers don't match, but the Gateway API indicates that this isn't always the case.

  • If another Listener has an exact match or more specific wildcard entry, the Gateway SHOULD return a 421.
    • If the current Listener (selected by SNI matching during ClientHello) oes not match the Host:
      • If another Listener does match the Host, the Gateway SHOULD return a 421.
      • If no other Listener matches the Host, the Gateway MUST return a 404.

Solution: If an HTTPS Listener without a hostname is specified, use the cert/key on that listener in the default server block to allow for non-SNI requests to have TLS termination, before sent to the proper server block based on the host header. Adjust the logic for returning 421 to match the conditions in the Gateway API spec.

Testing: Manually verified request without SNI are routed properly.

Please focus on (optional): If you any specific areas where you would like reviewers to focus their attention or provide
specific feedback, add them here.

Closes #4836
Closes #4838

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

Release notes

If this PR introduces a change that affects users and needs to be mentioned in the release notes,
please add a brief note that summarizes the change.

Client requests without SNI can now be routed properly as long as an HTTPS Listener without a hostname is defined on the Gateway.

Also aligned 421 Misdirected Requests behavior with the Gateway API specifications, adding flexibility to mismatched SNI and Host header values based on the Listener that these requests are intended for.

@sjberman sjberman requested a review from a team as a code owner March 3, 2026 15:39
@github-actions github-actions Bot added the bug Something isn't working label Mar 3, 2026
@sjberman sjberman requested a review from Copilot March 3, 2026 15:40
@codecov

codecov Bot commented Mar 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.30769% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.91%. Comparing base (e5fb5dd) to head (e718cc2).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
internal/controller/nginx/config/servers.go 63.15% 6 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4878      +/-   ##
==========================================
+ Coverage   85.89%   85.91%   +0.02%     
==========================================
  Files         143      143              
  Lines       16744    16813      +69     
  Branches       35       35              
==========================================
+ Hits        14382    14445      +63     
- Misses       2111     2116       +5     
- Partials      251      252       +1     

☔ View full report in Codecov by Sentry.
📢 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot AI 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.

Pull request overview

This PR updates NGINX Gateway Fabric’s HTTPS handling to support TLS termination for requests without SNI (when an HTTPS Listener has no hostname) and adjusts “misdirected request” (421) behavior to align with the Gateway API’s connection coalescing expectations.

Changes:

  • Populate the default SSL server with a cert/key derived from the empty-hostname HTTPS Listener, enabling non-SNI TLS handshakes.
  • Introduce per-port NGINX map variables to compare SNI-derived vs Host-derived listener selection for misdirected request detection.
  • Advertise SupportGatewayHTTPSListenerDetectMisdirectedRequests as a supported Gateway API feature and extend unit tests accordingly.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
internal/controller/status/gatewayclass.go Adds the misdirected-requests feature to the supported feature set.
internal/controller/status/gatewayclass_test.go Updates supported-feature tests to include the new feature.
internal/controller/state/dataplane/types.go Adds SSLListenerHostnames to the dataplane configuration model.
internal/controller/state/dataplane/configuration.go Builds SSLListenerHostnames and propagates default SSL cert config for the default server.
internal/controller/state/dataplane/configuration_test.go Updates expected configurations to include default SSL and listener hostname mappings.
internal/controller/nginx/config/http/config.go Adds MisdirectedRequestVars to the rendered server model.
internal/controller/nginx/config/servers.go Threads misdirected-request vars into SSL servers; supports default SSL server cert/key material.
internal/controller/nginx/config/servers_template.go Updates templates to optionally terminate TLS on the default server and to use the new misdirected-request vars.
internal/controller/nginx/config/servers_test.go Updates server generation tests for the new misdirected-request variable names.
internal/controller/nginx/config/maps.go Adds misdirected-request map generation driven by SSLListenerHostnames.
internal/controller/nginx/config/maps_test.go Extends map execution tests and adds focused tests for the new misdirected-request maps.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/controller/nginx/config/servers_template.go
Comment thread internal/controller/nginx/config/servers_template.go
Comment thread internal/controller/nginx/config/maps.go
Comment thread internal/controller/nginx/config/maps.go
@sjberman sjberman marked this pull request as draft March 3, 2026 16:12
@sjberman sjberman marked this pull request as ready for review March 3, 2026 16:27
Comment thread internal/controller/nginx/config/maps.go
Comment thread internal/controller/nginx/config/maps.go
Problem: Some frontend load balancers strip out SNI info before the client request reaches the NGINX Gateway. With the current behavior, the request will hit our default_server and the handshake is rejected (handshake rejected while SSL handshaking). It will never be forwarded to the proper server block to handle that request and proxy to the backend.

In addition, we always return a 421 when the SNI and Host headers don't match, but the Gateway API indicates that this isn't always the case.

* If another Listener has an exact match or more specific wildcard entry, the Gateway SHOULD return a 421.
  * If the current Listener (selected by SNI matching during ClientHello) oes not match the Host:
    * If another Listener does match the Host, the Gateway SHOULD return a 421.
    * If no other Listener matches the Host, the Gateway MUST return a 404.

Solution: If an HTTPS Listener without a hostname is specified, use the cert/key on that listener in the default server block to allow for non-SNI requests to have TLS termination, before sent to the proper server block based on the host header. Adjust the logic for returning 421 to match the conditions in the Gateway API spec.
@sjberman sjberman enabled auto-merge (squash) March 11, 2026 13:35
@sjberman sjberman merged commit 7d14fca into main Mar 11, 2026
101 of 103 checks passed
@sjberman sjberman deleted the fix/https-sni branch March 11, 2026 13:54
@github-project-automation github-project-automation Bot moved this from 🆕 New to ✅ Done in NGINX Gateway Fabric Mar 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working release-notes

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

Fix SNI/Host connection coalescing to meet API expectations for HTTPS Listeners Allow HTTPS requests without SNI to still be proxied

4 participants