Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate a default SSL server for select HTTPS listener hostname #173

Merged
merged 2 commits into from
Aug 10, 2022

Conversation

kate-osborn
Copy link
Contributor

This change fixes the following problems:

(1) If a valid HTTPS listener is configured but has no attached routes, a request to the listener's hostname will result in an SSL handshake error.

(2) If a valid HTTPS listener with a catch-all hostname is configured, a request to any hostname that does not match an existing route will result in an SSL handshake error.

After discussion, we decided that it doesn't make sense to reject the SSL handshake when there's a valid HTTPS listener configured for the hostname. The desired behavior is to return a 404.

Solution: Generate an SSL server block for each listener described above that terminates the TLS connection and returns a 404.

@kate-osborn kate-osborn requested a review from pleshakov August 5, 2022 19:59
Copy link
Contributor

@pleshakov pleshakov left a comment

Choose a reason for hiding this comment

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

Hi @kate-osborn Please see my review

internal/state/configuration.go Outdated Show resolved Hide resolved
internal/state/configuration.go Outdated Show resolved Hide resolved
internal/nginx/config/generator.go Outdated Show resolved Hide resolved
Problems:

(1) If a valid HTTPS listener is configured but has no attached routes, a
request to the listener's hostname will result in an SSL handshake error.

(2) If a valid HTTPS listener with a catch-all hostname is configured, a request
to any hostname that does not match an existing route will result in an SSL
handshake error.

After discussion, we decided that it doesn't make sense to reject the SSL
handshake when there's a valid HTTPS listener configured for the hostname. The
desired behavior is to return a 404.

Solution: Generate an SSL server block for each listener described above that
terminates the TLS connection and returns a 404.
@kate-osborn kate-osborn force-pushed the fix/listener-no-routes-404 branch from 48accd4 to afb4e45 Compare August 10, 2022 16:23
@kate-osborn kate-osborn requested a review from pleshakov August 10, 2022 16:25
Copy link
Contributor

@pleshakov pleshakov left a comment

Choose a reason for hiding this comment

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

👍

@kate-osborn kate-osborn merged commit f9bf557 into main Aug 10, 2022
@kate-osborn kate-osborn deleted the fix/listener-no-routes-404 branch August 10, 2022 17:31
@lucacome lucacome added the bug Something isn't working label Aug 19, 2022
miledxz added a commit to miledxz/nginx-gateway-fabric that referenced this pull request Jan 14, 2025
…nx#173)

Generate a default SSL server for select listener hostnames

Problems:

(1) If a valid HTTPS listener is configured but has no attached routes, a
request to the listener's hostname will result in an SSL handshake error.

(2) If a valid HTTPS listener with a catch-all hostname is configured, a request
to any hostname that does not match an existing route will result in an SSL
handshake error.

After discussion, we decided that it doesn't make sense to reject the SSL
handshake when there's a valid HTTPS listener configured for the hostname. The
desired behavior is to return a 404.

Solution: Generate an SSL server block for each listener described above that
terminates the TLS connection and returns a 404.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants