diff --git a/nixos/doc/manual/release-notes/rl-2411.section.md b/nixos/doc/manual/release-notes/rl-2411.section.md index a3042b88bf5f..a3a1e2ebed33 100644 --- a/nixos/doc/manual/release-notes/rl-2411.section.md +++ b/nixos/doc/manual/release-notes/rl-2411.section.md @@ -396,6 +396,12 @@ - The `services.trust-dns` module has been renamed to `services.hickory-dns`. +- The option `services.prometheus.exporters.pgbouncer.connectionStringFile` has been removed since + it leaked the connection string (and thus potentially the DB password) into the cmdline + of process making it effectively world-readable. + + Use [`services.prometheus.exporters.pgbouncer.connectionEnvFile`](#opt-services.prometheus.exporters.pgbouncer.connectionEnvFile) instead. + - The `lsh` package and the `services.lshd` module have been removed as they had no maintainer in Nixpkgs and hadn’t seen an upstream release in over a decade. It is recommended to migrate to `openssh` and `services.openssh`. - `opencv2` and `opencv3` have been removed, as they are obsolete and diff --git a/nixos/modules/services/monitoring/prometheus/exporters.nix b/nixos/modules/services/monitoring/prometheus/exporters.nix index 1471f97f1c24..c698c9005aaf 100644 --- a/nixos/modules/services/monitoring/prometheus/exporters.nix +++ b/nixos/modules/services/monitoring/prometheus/exporters.nix @@ -3,7 +3,7 @@ let inherit (lib) concatStrings foldl foldl' genAttrs literalExpression maintainers mapAttrs mapAttrsToList mkDefault mkEnableOption mkIf mkMerge mkOption - optional types mkOptionDefault flip attrNames; + optional types mkOptionDefault flip attrNames xor; cfg = config.services.prometheus.exporters; @@ -230,6 +230,7 @@ let in mkIf conf.enable { warnings = conf.warnings or []; + assertions = conf.assertions or []; users.users."${name}-exporter" = (mkIf (conf.user == "${name}-exporter" && !enableDynamicUser) { description = "Prometheus ${name} exporter service user"; isSystemUser = true; @@ -359,13 +360,6 @@ in Please specify either 'services.prometheus.exporters.nextcloud.passwordFile' or 'services.prometheus.exporters.nextcloud.tokenFile' ''; - } { - assertion = cfg.pgbouncer.enable -> ( - (cfg.pgbouncer.connectionStringFile != null || cfg.pgbouncer.connectionString != "") - ); - message = '' - PgBouncer exporter needs either connectionStringFile or connectionString configured" - ''; } { assertion = cfg.sql.enable -> ( (cfg.sql.configFile == null) != (cfg.sql.configuration == null) @@ -405,7 +399,15 @@ in Please ensure you have either `services.prometheus.exporters.deluge.delugePassword' or `services.prometheus.exporters.deluge.delugePasswordFile' set! ''; - } ] ++ (flip map (attrNames exporterOpts) (exporter: { + } { + assertion = cfg.pgbouncer.enable -> ( + xor (cfg.pgbouncer.connectionEnvFile == null) (cfg.pgbouncer.connectionString == null) + ); + message = '' + Options `services.prometheus.exporters.pgbouncer.connectionEnvFile` and + `services.prometheus.exporters.pgbouncer.connectionString` are mutually exclusive! + ''; + }] ++ (flip map (attrNames exporterOpts) (exporter: { assertion = cfg.${exporter}.firewallFilter != null -> cfg.${exporter}.openFirewall; message = '' The `firewallFilter'-option of exporter ${exporter} doesn't have any effect unless @@ -419,11 +421,6 @@ in Consider using `services.prometheus.exporters.idrac.configuration` instead. '' ) - (mkIf - (cfg.pgbouncer.enable && cfg.pgbouncer.connectionString != "") '' - config.services.prometheus.exporters.pgbouncer.connectionString is insecure. Use connectionStringFile instead. - '' - ) ] ++ config.services.prometheus.exporters.warnings; }] ++ [(mkIf config.services.prometheus.exporters.rtl_433.enable { hardware.rtl-sdr.enable = mkDefault true; diff --git a/nixos/modules/services/monitoring/prometheus/exporters/pgbouncer.nix b/nixos/modules/services/monitoring/prometheus/exporters/pgbouncer.nix index 9cd261099a95..5c111687c1d7 100644 --- a/nixos/modules/services/monitoring/prometheus/exporters/pgbouncer.nix +++ b/nixos/modules/services/monitoring/prometheus/exporters/pgbouncer.nix @@ -7,11 +7,8 @@ let mkPackageOption types optionals - optionalString getExe - getExe' escapeShellArg - escapeShellArgs concatStringsSep ; in @@ -29,8 +26,8 @@ in }; connectionString = mkOption { - type = types.str; - default = ""; + type = types.nullOr types.str; + default = null; example = "postgres://admin:@localhost:6432/pgbouncer?sslmode=require"; description = '' Connection string for accessing pgBouncer. @@ -43,26 +40,28 @@ in auth_file if auth_type other than "any" is used. WARNING: this secret is stored in the world-readable Nix store! - Use {option}`connectionStringFile` instead. + Use [](#opt-services.prometheus.exporters.pgbouncer.connectionEnvFile) if the + URL contains a secret. ''; }; - connectionStringFile = mkOption { - type = types.nullOr types.path; + connectionEnvFile = mkOption { + type = types.nullOr types.str; default = null; - example = "/run/keys/pgBouncer-connection-string"; description = '' - File that contains pgBouncer connection string in format: - postgres://admin:@localhost:6432/pgbouncer?sslmode=require + File that must contain the environment variable + `PGBOUNCER_EXPORTER_CONNECTION_STRING` which is set to the connection + string used by pgbouncer. I.e. the format is supposed to look like this: - NOTE: You MUST keep pgbouncer as database name (special internal db)!!! + ``` + PGBOUNCER_EXPORTER_CONNECTION_STRING="postgres://admin@localhost:6432/pgbouncer?sslmode=require" + ``` - NOTE: ignore_startup_parameters MUST contain "extra_float_digits". + NOTE: You MUST keep pgbouncer as database name (special internal db)! + NOTE: `services.pgbouncer.settings.pgbouncer.ignore_startup_parameters` + MUST contain "extra_float_digits". - NOTE: Admin user (with password or passwordless) MUST exist in the - auth_file if auth_type other than "any" is used. - - {option}`connectionStringFile` takes precedence over {option}`connectionString` + Mutually exclusive with [](#opt-services.prometheus.exporters.pgbouncer.connectionString). ''; }; @@ -126,16 +125,11 @@ in serviceOpts = { after = [ "pgbouncer.service" ]; - script = optionalString (cfg.connectionStringFile != null) '' - connectionString=$(${escapeShellArgs [ - (getExe' pkgs.coreutils "cat") "--" cfg.connectionStringFile - ]}) - '' + concatStringsSep " " ([ + script = concatStringsSep " " ([ "exec -- ${escapeShellArg (getExe cfg.package)}" "--web.listen-address ${cfg.listenAddress}:${toString cfg.port}" - "--pgBouncer.connectionString ${if cfg.connectionStringFile != null - then "\"$connectionString\"" - else "${escapeShellArg cfg.connectionString}"}" + ] ++ optionals (cfg.connectionString != null) [ + "--pgBouncer.connectionString ${escapeShellArg cfg.connectionString}" ] ++ optionals (cfg.telemetryPath != null) [ "--web.telemetry-path ${escapeShellArg cfg.telemetryPath}" ] ++ optionals (cfg.pidFile != null) [ @@ -151,5 +145,21 @@ in ] ++ cfg.extraFlags); serviceConfig.RestrictAddressFamilies = [ "AF_INET" "AF_INET6" "AF_UNIX" ]; + serviceConfig.EnvironmentFile = lib.mkIf (cfg.connectionEnvFile != null) [ + cfg.connectionEnvFile + ]; }; + + imports = [ + (lib.mkRemovedOptionModule [ "connectionStringFile" ] '' + As replacement, the option `services.prometheus.exporters.pgbouncer.connectionEnvFile` + has been added. In contrast to `connectionStringFile` it must be an environment file + with the connection string being set to `PGBOUNCER_EXPORTER_CONNECTION_STRING`. + + The change was necessary since the former option wrote the contents of the file + into the cmdline of the exporter making the connection string effectively + world-readable. + '') + ({ options.warnings = options.warnings; options.assertions = options.assertions; }) + ]; } diff --git a/nixos/tests/prometheus-exporters.nix b/nixos/tests/prometheus-exporters.nix index 3ce296c0cacf..71eef72df6f3 100644 --- a/nixos/tests/prometheus-exporters.nix +++ b/nixos/tests/prometheus-exporters.nix @@ -482,7 +482,6 @@ let json = { exporterConfig = { enable = true; - url = "http://localhost"; configFile = pkgs.writeText "json-exporter-conf.json" (builtins.toJSON { modules = { default = { @@ -932,7 +931,9 @@ let pgbouncer = { exporterConfig = { enable = true; - connectionStringFile = pkgs.writeText "connection.conf" "postgres://admin:@localhost:6432/pgbouncer?sslmode=disable"; + connectionEnvFile = "${pkgs.writeText "connstr-env" '' + PGBOUNCER_EXPORTER_CONNECTION_STRING=postgres://admin@localhost:6432/pgbouncer?sslmode=disable + ''}"; }; metricProvider = { diff --git a/pkgs/servers/monitoring/prometheus/pgbouncer-exporter.nix b/pkgs/servers/monitoring/prometheus/pgbouncer-exporter.nix index 970d63ec0f2e..90ffdb48f0d1 100644 --- a/pkgs/servers/monitoring/prometheus/pgbouncer-exporter.nix +++ b/pkgs/servers/monitoring/prometheus/pgbouncer-exporter.nix @@ -2,16 +2,16 @@ buildGoModule rec { pname = "pgbouncer-exporter"; - version = "0.8.0"; + version = "0.9.0"; src = fetchFromGitHub { owner = "prometheus-community"; repo = "pgbouncer_exporter"; rev = "v${version}"; - hash = "sha256-QnA9H4qedCPZKqJQ1I2OJO42mCWcWqYxLmeF3+JXzTw="; + hash = "sha256-fKoyRHYLwVefsZ014eazVCD5B9eV8/CUkuHE4mbUqVo="; }; - vendorHash = "sha256-NYiVW+CNrxFrEUl1nsTeNNgy7SmTYgqs1d50rCvyBcw="; + vendorHash = "sha256-IxmxfF9WsF0Hbym4G0UecyW8hAvucoaCFUE1kXUljJs="; meta = with lib; { description = "Prometheus exporter for PgBouncer";