fusion: init at 0.8.9 #353616
fusion: init at 0.8.9 #353616
Conversation
github-actions
bot
added
6.topic: nixos
ofborg
bot
added
8.has: package (new)
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-ready-for-review/3032/4802 |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-ready-for-review/3032/4819 |
|
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-already-reviewed/2617/2109 |
wegank
added
the
12.approvals: 1
h7x4
added
8.has: module (new)
| }; | ||
|
|
||
| script = '' | ||
| export PASSWORD=$(cat $CREDENTIALS_DIRECTORY/fusion) |
There was a problem hiding this comment.
Isn't this exposing the password in ps?
There was a problem hiding this comment.
I don't think so? This is run in a run script that's only run by the systemd user, and $CREDENTIALS_DIRECTORY isn't visible anywhere else (see systemd docs.
It's not optimal, but you can argue that a program that's designed to only source its password from an unencrypted environmental variable is already unsafe anyway if you have security concerns 🤷🏼♀️
There was a problem hiding this comment.
This is used as an example in the upstream docs:
In order to reference the path a credential may be read from within a ExecStart= command line use "${CREDENTIALS_DIRECTORY}/mycred", e.g. "ExecStart=cat ${CREDENTIALS_DIRECTORY}/mycred"
But next they also give an example of
In order to reference the path a credential may be read from within a Environment= line use "%d/mycred", e.g. "Environment=MYCREDPATH=%d/mycred".
That may be better to apply here rather than doing it in the script -- assuming it works
There was a problem hiding this comment.
The thing is that fusion doesn't expect a path to the credential, but the whole plain password itself. This is why I mentioned that honestly wanting any amount of actual security on this thing is somewhat futile since it only works with plain passwords 100% visible and readable as an environment variable
See 0x2E/fusion#32
There was a problem hiding this comment.
This is why I mentioned that honestly wanting any amount of actual security on this thing is somewhat futile since it only works with plain passwords 100% visible and readable as an environment variable
I wouldn't say it's that bad, as this environment variable is at least scoped to the service script (compared to ps info being available globally on many machines)
I don't think this would actually make the secrets contents appear in ps though, only the directory -- which doesn't matter as it should have restricted perms. If we really want to avoid cat though, we could use this instead
PASSWORD="$(< "$CREDENTIALS_DIRECTORY"/fusion)"
export PASSWORD| tls = lib.mkOption { | ||
| type = lib.types.nullOr ( | ||
| lib.types.submodule { | ||
| options = { | ||
| cert = lib.mkOption { | ||
| type = lib.types.path; | ||
| description = "Path to TLS certificate"; | ||
| }; | ||
| key = lib.mkOption { | ||
| type = lib.types.path; | ||
| description = "Path to TLS key"; | ||
| }; | ||
| }; | ||
| } | ||
| ); | ||
| default = null; | ||
| description = '' | ||
| The paths to the TLS certificate and key files for Fusion. | ||
| If these options are set, then Fusion can only be accessed through a secure | ||
| TLS connection. If you are using a reverse proxy like Nginx to handle HTTPS, | ||
| please leave these unset. | ||
| ''; | ||
| }; |
There was a problem hiding this comment.
Not sure if we just want to use nginx here all the time or wire this into acme directly.
There was a problem hiding this comment.
We're not using Nginx? I'm just saying that if you are using a reverse proxy, for example with Nginx, then you need to leave these unset following upstream instructions
There was a problem hiding this comment.
I believe this may have more been of a recommendation to use Nginx over this, as most modules will do that over exposing TLS cert options like this and accessing services directly
I think it could be a good addition here as well, but not a blocker by any means
There was a problem hiding this comment.
I'm not exactly familiar with Nginx 🤦🏼♀️ Is there an example I can look at?
There was a problem hiding this comment.
pixelfed uses this well. A minimal example would be
{
options.myService = {
# ...
domain = lib.mkOption {
type = lib.types.str;
description = "FQDN for myService";
example = "my-service.example.org";
};
nginx = lib.mkOption {
type = lib.types.nullOr (
lib.types.submodule (import ../web-servers/nginx/vhost-options.nix { inherit config lib; })
);
default = { };
example = lib.literalExpression ''
{
enableACME = true;
forceSSL = true;
}
'';
};
};
config = lib.mkIf cfg.enable {
# ...
services.nginx = lib.mkIf (cfg.nginx != null) {
enable = lib.mkDefault true;
virtualHosts.${cfg.domain} = lib.mkMerge [
{
some = "nice";
defaults = [
"if"
"any"
];
}
cfg.nginx
];
};
};
}
| }; | ||
|
|
||
| script = '' | ||
| export PASSWORD=$(cat $CREDENTIALS_DIRECTORY/fusion) |
There was a problem hiding this comment.
This is used as an example in the upstream docs:
In order to reference the path a credential may be read from within a ExecStart= command line use "${CREDENTIALS_DIRECTORY}/mycred", e.g. "ExecStart=cat ${CREDENTIALS_DIRECTORY}/mycred"
But next they also give an example of
In order to reference the path a credential may be read from within a Environment= line use "%d/mycred", e.g. "Environment=MYCREDPATH=%d/mycred".
That may be better to apply here rather than doing it in the script -- assuming it works
| tls = lib.mkOption { | ||
| type = lib.types.nullOr ( | ||
| lib.types.submodule { | ||
| options = { | ||
| cert = lib.mkOption { | ||
| type = lib.types.path; | ||
| description = "Path to TLS certificate"; | ||
| }; | ||
| key = lib.mkOption { | ||
| type = lib.types.path; | ||
| description = "Path to TLS key"; | ||
| }; | ||
| }; | ||
| } | ||
| ); | ||
| default = null; | ||
| description = '' | ||
| The paths to the TLS certificate and key files for Fusion. | ||
| If these options are set, then Fusion can only be accessed through a secure | ||
| TLS connection. If you are using a reverse proxy like Nginx to handle HTTPS, | ||
| please leave these unset. | ||
| ''; | ||
| }; |
There was a problem hiding this comment.
I believe this may have more been of a recommendation to use Nginx over this, as most modules will do that over exposing TLS cert options like this and accessing services directly
I think it could be a good addition here as well, but not a blocker by any means
wegank
removed
the
12.approvals: 1
| ++ lib.optionals (cfg.tls != null) [ | ||
| cfg.tls.cert | ||
| cfg.tls.key | ||
| ]; |
There was a problem hiding this comment.
Thanks for packaging this, @pluiedev!
I tested this on my system, and the systemd hardening seems to prevent fusion from making outbound HTTP connections to RSS feeds.
I was able to fix it by adding this:
BindReadOnlyPaths = [
builtins.storeDir
"/etc/ssl/certs"
"/etc/resolv.conf"
"/etc/nsswitch.conf"
"/etc/hosts"
"${pkgs.fusion}"
"${pkgs.cacert}/etc/ssl/certs/ca-bundle.crt"
] ++ lib.optionals (cfg.tls != null) [
cfg.tls.cert
cfg.tls.key
];
Environment = [
"SSL_CERT_FILE=${pkgs.cacert}/etc/ssl/certs/ca-bundle.crt"
"NIX_SSL_CERT_FILE=${pkgs.cacert}/etc/ssl/certs/ca-bundle.crt"
];
| nativeBuildInputs = prev.nativeBuildInputs ++ [ mockgen ]; | ||
|
|
||
| preBuild = '' | ||
| go generate ./... |
There was a problem hiding this comment.
As of 0.8.11, this is no longer needed (see 0x2E/fusion@d6194e3)
Fixes #353330
Would probably be nice to add a NixOS module tooDoneThings done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.