-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Added support for proxying with TSLv1.3 encryption. #2045
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
Open
moritzbeck13
wants to merge
2
commits into
NginxProxyManager:develop
Choose a base branch
from
moritzbeck13:proxy-TLSv1.3
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Added support for proxying with TSLv1.3 encryption. #2045
moritzbeck13
wants to merge
2
commits into
NginxProxyManager:develop
from
moritzbeck13:proxy-TLSv1.3
Conversation
This file contains hidden or 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
Very cool Idea! It would be interesting if this would be a setting in the Web-UI to change it if needed by the user. With TLSv1.3 as default would be good for the future. |
PR is now considered stale. If you want to keep it open, please comment 👍 |
CI Error:
|
PR is now considered stale. If you want to keep it open, please comment 👍 |
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.
After Portainer dropped support for everything but TLSv1.3 in 2.13, it was noticed (portainer/portainer#6902 and portainer/portainer#6900) that NPM seemed to only offer TLSv1.3 for serving, but not for proxying.
This makes sense, since the ssl_protocols flag is manually set to support TLSv1.2 and TSLv1.3 here, but the respective proxy_ssl_protocols flag is unset, meaning it is relied on the default NGINX setting, which enables support for TLSv1, TLSv1.1 and TLSv1.2, but not TLSv1.3.
Please note that I was not able to test this and just wanted to get the fix out as fast as possible. Since I included all the older versions, that are also in the default settings, unlike the ssl_protocols flag, which theoretically drops support for some older version, the functionality should only be enhanced and there should be no compatibility issues. With regards to the age of some of these older protocols, feel free to change this to exclude support for those, though.
I am not 100% sure about the location for this flag, but I think there is no better place for it. The ssl_protocols is only imported for HTTPS connections, but since you can also proxy an HTTP server over an HTTPS connection, I think it should be included in the main config.