Support ssl_reject_handshake in Ingress and VS #1500
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pleshakov
added
enhancement
To handle missing or invalid TLS Secrets in Ingress and VirtualServer resources, previously the Ingress Controller would generate the following configuration: ssl_certificate /etc/nginx/secrets/default; ssl_certificate_key /etc/nginx/secrets/default; ssl_ciphers NULL; The configuration will break any attempts for clients to establish TLS connections for the affected server in NGINX. The configuration has the following limitations: - It requires a TLS cert and key (we used the default server Secret as it was always present on the file system) - It doesn't work if clients and NGINX use TLS v1.3: NGINX will terminate TLS connection. This commit introduces the new ssl_reject_handshake directive, which configures NGINX to break any attempt to establish a TLS connection: ssl_reject_handshake on; The directive addresses the mentioned limitations: it doesn't require a TLS cert and key and works with TLS v1.3.
pleshakov
force-pushed
the
use-ssl-reject-handshake
branch
from
5dc2a4a
to
2ae6e26
Compare
soneillf5
reviewed
Apr 6, 2021
| @@ -313,6 +313,7 @@ func addSSLConfig(server *version1.Server, owner runtime.Object, host string, in | |||
| } | |||
|
|
|||
| var pemFile string | |||
| var rejectHandshake bool | |||
There was a problem hiding this comment.
Consider assigning rejectHandshake as 'true' and handle the single case it's false? If I'm reading it right.
There was a problem hiding this comment.
it's 2 cases for false vs 3 cases for true
soneillf5
approved these changes
Apr 6, 2021
lucacome
approved these changes
Apr 7, 2021
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed changes
To handle missing or invalid TLS Secrets in Ingress and VirtualServer
resources, previously the Ingress Controller would generate the
following configuration:
The configuration will break any attempts for clients to establish
TLS connections for the affected server in NGINX.
The configuration has the following limitations:
it was always present on the file system)
TLS connection.
This commit introduces the new ssl_reject_handshake directive, which
configures NGINX to break any attempt to establish a TLS connection:
The directive addresses the mentioned limitations: it doesn't require
a TLS cert and key and works with TLS v1.3.
Changes for Changelog
ssl_ciphers NULL;in the NGINX configuration. The method didn't work for TLS v1.3. Now the Ingress Controller usesssl_reject_handshake on;, which works for TLS v1.3.