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

Remove buffering from nginx config, make config secure by default #451

Merged
merged 1 commit into from
Nov 2, 2022

Conversation

SuperSandro2000
Copy link
Contributor

Turning off proxy buffering is not recommend by upstream https://www.nginx.com/blog/avoiding-top-10-nginx-configuration-mistakes/#proxy_buffering-off by default. And making configuration more secure by removing TLSv1 TLSv1.1 and redirecting to https all the time to never leak credentials.

PS: https is not annoying and curl can follow redirects with -L.

Turning off proxy buffering is not recommend by upstream https://www.nginx.com/blog/avoiding-top-10-nginx-configuration-mistakes/#proxy_buffering-off by default. And making configuration more secure by removing TLSv1 TLSv1.1 and redirecting to https all the time to never leak credentials.

PS: https is not annoying and curl can follow redirects with -L.
@binwiederhier
Copy link
Owner

My apologies for not getting to this earlier, but I'm not sure I 100% agree with the PR.

  • I do agree with the TLS cipher suite changes, those should be in the example
  • As I said before, I do want people to be able to use curl without typing the https:// part, and I want the sample setup to reflect that. So maybe we can just have two examples like that? Let me try to see what I can whip up.

@binwiederhier binwiederhier merged commit 93fe19b into binwiederhier:main Nov 2, 2022
@binwiederhier
Copy link
Owner

Added two nginx configs: One called "convienient" and one called "more secure" (yours). https://github.com/binwiederhier/ntfy/blob/main/docs/config.md?plain=1#L437

Also added some explanations. I don't have the time to try the output buffering stuff, but I swear I had issues with that.

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.

None yet

2 participants