-
-
Notifications
You must be signed in to change notification settings - Fork 14.2k
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
Nextcloud: add openFirewall setting #50256
Conversation
...similarly to sshd module.
fix spelling hostName vs hostname. Mention nextcloud blocking if hostName wrongly set.
Success on aarch64-linux (full log) Attempted: nextcloud Partial log (click to expand)
|
Success on x86_64-linux (full log) Attempted: nextcloud Partial log (click to expand)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There appear to be two schools regarding addition of settings like openFirewall
.
Basically, the drawback such options have is that they slow down all evaluations, even if the module is never used. Ideally this wouldn't be the case and such options could be accepted without thinking twice about it, but… It may be a better idea to just document the required allowedTCPPorts
in services.nextcloud.enable
.
@@ -50,6 +53,13 @@ in { | |||
default = false; | |||
description = "Enable if there is a TLS terminating proxy in front of nextcloud."; | |||
}; | |||
openFirewall = mkOption { | |||
type = types.bool; | |||
default = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm pretty sure we don't want openFirewall
to default to true
: sshd is the exception to the rule, and while it could make sense to provide the openFirewall
option, having it defaulting to opening the firewall would be unexpected and potentially a source of security issues (because people wouldn't expect their nextcloud to be opened to the whole internet).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh my bad I definitely wanted to set it to false but forgot.
@@ -50,6 +53,13 @@ in { | |||
default = false; | |||
description = "Enable if there is a TLS terminating proxy in front of nextcloud."; | |||
}; | |||
openFirewall = mkOption { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this may be better placed in services.nginx
, as it would open the firewall for all of nginx
, and not only for opensmtpd
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO it would make sense th have the setting for both modules (with nextcloud passing it to nginx if nginx is enabled). Now if that slows evaluation, that looks bad xD
Is there any documentation so that I can better understand the problem ? I had hoped that lazy evaluation would get rid of these. Is there any hope for the situation to change, for instance with a new nix module format like the one presented at nixcon ? Hypothetically, assuming this is not a problem, coupling the firewall configuration with the module enabling seems fitting to the nix philosophy (since the firewall can be seen as a dependency). |
There are ways towards not evaluating all modules all the time that are in discussion, eg. NixOS/rfcs#22 … hopefully, some day :) |
Sorry, but I don't think this approach is useful. Enabling the nextcloud module does not produce a listening TCP socket by default at all. Even if php-fpm would be configure to open a TCP socket, you would talk FastCGI to it so the HTTP ports don't make any sense in this module. You would put an HTTP server like nginx in front of it which would serve the HTTP ports 80 and 443 and that service should rather have the option to open the firewall than the nextcloud module. |
Motivation for this change
adding openFirewall setting as is done for sshd so that the firewall ports get coupled with nextcloud (they get removed along with nextcloud => more secure).
#48881
Things done
sandbox
innix.conf
on non-NixOS)nix-shell -p nox --run "nox-review wip"
./result/bin/
)nix path-info -S
before and after)