Merge pull request #39526 (improve dhparams)

This introduces an option that allows us to turn off stateful generation
of Diffie-Hellman parameters, which in some way is still "stateful" as
the generated DH params file is non-deterministic.

However what we can avoid with this is to have an increased surface for
failures during system startup, because generation of the parameters is
done during build-time.

Aside from adding a NixOS VM test it also restructures the type of the
security.dhparams.params option, so that it's a submodule.

A new defaultBitSize option is also there to allow users to set a
system-wide default.

I added a release notes entry that described what has changed and also
included a few notes for module developers using this module, as the
first usage already popped up in NixOS/nixpkgs#39507.

Thanks to @Ekleog and @abbradar for reviewing.

aszlig 78b4b90d ec198337

Changed files
+341 -78
nixos
doc
manual
release-notes
modules
security
tests
+52
nixos/doc/manual/release-notes/rl-1809.xml
···
for further reference.
</para>
</listitem>
+
<listitem>
+
<para>
+
The module for <option>security.dhparams</option> has two new options now:
+
</para>
+
+
<variablelist>
+
<varlistentry>
+
<term><option>security.dhparams.stateless</option></term>
+
<listitem><para>
+
Puts the generated Diffie-Hellman parameters into the Nix store instead
+
of managing them in a stateful manner in
+
<filename class="directory">/var/lib/dhparams</filename>.
+
</para></listitem>
+
</varlistentry>
+
<varlistentry>
+
<term><option>security.dhparams.defaultBitSize</option></term>
+
<listitem><para>
+
The default bit size to use for the generated Diffie-Hellman parameters.
+
</para></listitem>
+
</varlistentry>
+
</variablelist>
+
+
<note><para>
+
The path to the actual generated parameter files should now be queried
+
using
+
<literal>config.security.dhparams.params.<replaceable>name</replaceable>.path</literal>
+
because it might be either in the Nix store or in a directory configured
+
by <option>security.dhparams.path</option>.
+
</para></note>
+
+
<note>
+
<title>For developers:</title>
+
<para>
+
Module implementers should not set a specific bit size in order to let
+
users configure it by themselves if they want to have a different bit
+
size than the default (2048).
+
</para>
+
<para>
+
An example usage of this would be:
+
<programlisting>
+
{ config, ... }:
+
+
{
+
security.dhparams.params.myservice = {};
+
environment.etc."myservice.conf".text = ''
+
dhparams = ${config.security.dhparams.params.myservice.path}
+
'';
+
}
+
</programlisting>
+
</para>
+
</note>
+
</listitem>
</itemizedlist>
</section>
</section>
+144 -78
nixos/modules/security/dhparams.nix
···
{ config, lib, pkgs, ... }:
-
with lib;
let
+
inherit (lib) mkOption types;
cfg = config.security.dhparams;
-
in
-
{
+
+
bitType = types.addCheck types.int (b: b >= 16) // {
+
name = "bits";
+
description = "integer of at least 16 bits";
+
};
+
+
paramsSubmodule = { name, config, ... }: {
+
options.bits = mkOption {
+
type = bitType;
+
default = cfg.defaultBitSize;
+
description = ''
+
The bit size for the prime that is used during a Diffie-Hellman
+
key exchange.
+
'';
+
};
+
+
options.path = mkOption {
+
type = types.path;
+
readOnly = true;
+
description = ''
+
The resulting path of the generated Diffie-Hellman parameters
+
file for other services to reference. This could be either a
+
store path or a file inside the directory specified by
+
<option>security.dhparams.path</option>.
+
'';
+
};
+
+
config.path = let
+
generated = pkgs.runCommand "dhparams-${name}.pem" {
+
nativeBuildInputs = [ pkgs.openssl ];
+
} "openssl dhparam -out \"$out\" ${toString config.bits}";
+
in if cfg.stateful then "${cfg.path}/${name}.pem" else generated;
+
};
+
+
in {
options = {
security.dhparams = {
+
enable = mkOption {
+
type = types.bool;
+
default = false;
+
description = ''
+
Whether to generate new DH params and clean up old DH params.
+
'';
+
};
+
params = mkOption {
-
description =
-
''
-
Diffie-Hellman parameters to generate.
+
type = with types; let
+
coerce = bits: { inherit bits; };
+
in attrsOf (coercedTo int coerce (submodule paramsSubmodule));
+
default = {};
+
example = lib.literalExample "{ nginx.bits = 3072; }";
+
description = ''
+
Diffie-Hellman parameters to generate.
+
+
The value is the size (in bits) of the DH params to generate. The
+
generated DH params path can be found in
+
<literal>config.security.dhparams.params.<replaceable>name</replaceable>.path</literal>.
-
The value is the size (in bits) of the DH params to generate. The
-
generated DH params path can be found in
-
<filename><replaceable>security.dhparams.path</replaceable>/<replaceable>name</replaceable>.pem</filename>.
+
<note><para>The name of the DH params is taken as being the name of
+
the service it serves and the params will be generated before the
+
said service is started.</para></note>
-
Note: The name of the DH params is taken as being the name of the
-
service it serves: the params will be generated before the said
-
service is started.
+
<warning><para>If you are removing all dhparams from this list, you
+
have to leave <option>security.dhparams.enable</option> for at
+
least one activation in order to have them be cleaned up. This also
+
means if you rollback to a version without any dhparams the
+
existing ones won't be cleaned up. Of course this only applies if
+
<option>security.dhparams.stateful</option> is
+
<literal>true</literal>.</para></warning>
+
+
<note><title>For module implementers:</title><para>It's recommended
+
to not set a specific bit size here, so that users can easily
+
override this by setting
+
<option>security.dhparams.defaultBitSize</option>.</para></note>
+
'';
+
};
+
+
stateful = mkOption {
+
type = types.bool;
+
default = true;
+
description = ''
+
Whether generation of Diffie-Hellman parameters should be stateful or
+
not. If this is enabled, PEM-encoded files for Diffie-Hellman
+
parameters are placed in the directory specified by
+
<option>security.dhparams.path</option>. Otherwise the files are
+
created within the Nix store.
-
Warning: If you are removing all dhparams from this list, you have
-
to leave security.dhparams.enable for at least one activation in
-
order to have them be cleaned up. This also means if you rollback to
-
a version without any dhparams the existing ones won't be cleaned
-
up.
-
'';
-
type = with types; attrsOf int;
-
default = {};
-
example = { nginx = 3072; };
+
<note><para>If this is <literal>false</literal> the resulting store
+
path will be non-deterministic and will be rebuilt every time the
+
<package>openssl</package> package changes.</para></note>
+
'';
+
};
+
+
defaultBitSize = mkOption {
+
type = bitType;
+
default = 2048;
+
description = ''
+
This allows to override the default bit size for all of the
+
Diffie-Hellman parameters set in
+
<option>security.dhparams.params</option>.
+
'';
};
path = mkOption {
-
description =
-
''
-
Path to the directory in which Diffie-Hellman parameters will be
-
stored.
-
'';
type = types.str;
default = "/var/lib/dhparams";
-
};
-
-
enable = mkOption {
-
description =
-
''
-
Whether to generate new DH params and clean up old DH params.
-
'';
-
default = false;
-
type = types.bool;
+
description = ''
+
Path to the directory in which Diffie-Hellman parameters will be
+
stored. This only is relevant if
+
<option>security.dhparams.stateful</option> is
+
<literal>true</literal>.
+
'';
};
};
};
-
config = mkIf cfg.enable {
+
config = lib.mkIf (cfg.enable && cfg.stateful) {
systemd.services = {
dhparams-init = {
-
description = "Cleanup old Diffie-Hellman parameters";
-
wantedBy = [ "multi-user.target" ]; # Clean up even when no DH params is set
+
description = "Clean Up Old Diffie-Hellman Parameters";
+
+
# Clean up even when no DH params is set
+
wantedBy = [ "multi-user.target" ];
+
+
serviceConfig.RemainAfterExit = true;
serviceConfig.Type = "oneshot";
-
script =
-
# Create directory
-
''
-
if [ ! -d ${cfg.path} ]; then
-
mkdir -p ${cfg.path}
+
+
script = ''
+
if [ ! -d ${cfg.path} ]; then
+
mkdir -p ${cfg.path}
+
fi
+
+
# Remove old dhparams
+
for file in ${cfg.path}/*; do
+
if [ ! -f "$file" ]; then
+
continue
fi
-
'' +
-
# Remove old dhparams
-
''
-
for file in ${cfg.path}/*; do
-
if [ ! -f "$file" ]; then
+
${lib.concatStrings (lib.mapAttrsToList (name: { bits, path, ... }: ''
+
if [ "$file" = ${lib.escapeShellArg path} ] && \
+
${pkgs.openssl}/bin/openssl dhparam -in "$file" -text \
+
| head -n 1 | grep "(${toString bits} bit)" > /dev/null; then
continue
fi
-
'' + concatStrings (mapAttrsToList (name: value:
-
''
-
if [ "$file" == "${cfg.path}/${name}.pem" ] && \
-
${pkgs.openssl}/bin/openssl dhparam -in "$file" -text | head -n 1 | grep "(${toString value} bit)" > /dev/null; then
-
continue
-
fi
-
''
-
) cfg.params) +
-
''
-
rm $file
-
done
+
'') cfg.params)}
+
rm $file
+
done
-
# TODO: Ideally this would be removing the *former* cfg.path, though this
-
# does not seem really important as changes to it are quite unlikely
-
rmdir --ignore-fail-on-non-empty ${cfg.path}
-
'';
+
# TODO: Ideally this would be removing the *former* cfg.path, though
+
# this does not seem really important as changes to it are quite
+
# unlikely
+
rmdir --ignore-fail-on-non-empty ${cfg.path}
+
'';
};
-
} //
-
mapAttrs' (name: value: nameValuePair "dhparams-gen-${name}" {
-
description = "Generate Diffie-Hellman parameters for ${name} if they don't exist yet";
-
after = [ "dhparams-init.service" ];
-
before = [ "${name}.service" ];
-
wantedBy = [ "multi-user.target" ];
-
serviceConfig.Type = "oneshot";
-
script =
-
''
-
mkdir -p ${cfg.path}
-
if [ ! -f ${cfg.path}/${name}.pem ]; then
-
${pkgs.openssl}/bin/openssl dhparam -out ${cfg.path}/${name}.pem ${toString value}
-
fi
-
'';
-
}) cfg.params;
+
} // lib.mapAttrs' (name: { bits, path, ... }: lib.nameValuePair "dhparams-gen-${name}" {
+
description = "Generate Diffie-Hellman Parameters for ${name}";
+
after = [ "dhparams-init.service" ];
+
before = [ "${name}.service" ];
+
wantedBy = [ "multi-user.target" ];
+
unitConfig.ConditionPathExists = "!${path}";
+
serviceConfig.Type = "oneshot";
+
script = ''
+
mkdir -p ${lib.escapeShellArg cfg.path}
+
${pkgs.openssl}/bin/openssl dhparam -out ${lib.escapeShellArg path} \
+
${toString bits}
+
'';
+
}) cfg.params;
};
}
+1
nixos/release.nix
···
tests.containers-macvlans = callTest tests/containers-macvlans.nix {};
tests.couchdb = callTest tests/couchdb.nix {};
tests.deluge = callTest tests/deluge.nix {};
+
tests.dhparams = callTest tests/dhparams.nix {};
tests.docker = callTestOnMatchingSystems ["x86_64-linux"] tests/docker.nix {};
tests.docker-tools = callTestOnMatchingSystems ["x86_64-linux"] tests/docker-tools.nix {};
tests.docker-tools-overlay = callTestOnMatchingSystems ["x86_64-linux"] tests/docker-tools-overlay.nix {};
+144
nixos/tests/dhparams.nix
···
+
let
+
common = { pkgs, ... }: {
+
security.dhparams.enable = true;
+
environment.systemPackages = [ pkgs.openssl ];
+
};
+
+
in import ./make-test.nix {
+
name = "dhparams";
+
+
nodes.generation1 = { pkgs, config, ... }: {
+
imports = [ common ];
+
security.dhparams.params = {
+
# Use low values here because we don't want the test to run for ages.
+
foo.bits = 16;
+
# Also use the old format to make sure the type is coerced in the right
+
# way.
+
bar = 17;
+
};
+
+
systemd.services.foo = {
+
description = "Check systemd Ordering";
+
wantedBy = [ "multi-user.target" ];
+
unitConfig = {
+
# This is to make sure that the dhparams generation of foo occurs
+
# before this service so we need this service to start as early as
+
# possible to provoke a race condition.
+
DefaultDependencies = false;
+
+
# We check later whether the service has been started or not.
+
ConditionPathExists = config.security.dhparams.params.foo.path;
+
};
+
serviceConfig.Type = "oneshot";
+
serviceConfig.RemainAfterExit = true;
+
# The reason we only provide an ExecStop here is to ensure that we don't
+
# accidentally trigger an error because a file system is not yet ready
+
# during very early startup (we might not even have the Nix store
+
# available, for example if future changes in NixOS use systemd mount
+
# units to do early file system initialisation).
+
serviceConfig.ExecStop = "${pkgs.coreutils}/bin/true";
+
};
+
};
+
+
nodes.generation2 = {
+
imports = [ common ];
+
security.dhparams.params.foo.bits = 18;
+
};
+
+
nodes.generation3 = common;
+
+
nodes.generation4 = {
+
imports = [ common ];
+
security.dhparams.stateful = false;
+
security.dhparams.params.foo2.bits = 18;
+
security.dhparams.params.bar2.bits = 19;
+
};
+
+
nodes.generation5 = {
+
imports = [ common ];
+
security.dhparams.defaultBitSize = 30;
+
security.dhparams.params.foo3 = {};
+
security.dhparams.params.bar3 = {};
+
};
+
+
testScript = { nodes, ... }: let
+
getParamPath = gen: name: let
+
node = "generation${toString gen}";
+
in nodes.${node}.config.security.dhparams.params.${name}.path;
+
+
assertParamBits = gen: name: bits: let
+
path = getParamPath gen name;
+
in ''
+
$machine->nest('check bit size of ${path}', sub {
+
my $out = $machine->succeed('openssl dhparam -in ${path} -text');
+
$out =~ /^\s*DH Parameters:\s+\((\d+)\s+bit\)\s*$/m;
+
die "bit size should be ${toString bits} but it is $1 instead."
+
if $1 != ${toString bits};
+
});
+
'';
+
+
switchToGeneration = gen: let
+
node = "generation${toString gen}";
+
inherit (nodes.${node}.config.system.build) toplevel;
+
switchCmd = "${toplevel}/bin/switch-to-configuration test";
+
in ''
+
$machine->nest('switch to generation ${toString gen}', sub {
+
$machine->succeed('${switchCmd}');
+
$main::machine = ''$${node};
+
});
+
'';
+
+
in ''
+
my $machine = $generation1;
+
+
$machine->waitForUnit('multi-user.target');
+
+
subtest "verify startup order", sub {
+
$machine->succeed('systemctl is-active foo.service');
+
};
+
+
subtest "check bit sizes of dhparam files", sub {
+
${assertParamBits 1 "foo" 16}
+
${assertParamBits 1 "bar" 17}
+
};
+
+
${switchToGeneration 2}
+
+
subtest "check whether bit size has changed", sub {
+
${assertParamBits 2 "foo" 18}
+
};
+
+
subtest "ensure that dhparams file for 'bar' was deleted", sub {
+
$machine->fail('test -e ${getParamPath 1 "bar"}');
+
};
+
+
${switchToGeneration 3}
+
+
subtest "ensure that 'security.dhparams.path' has been deleted", sub {
+
$machine->fail(
+
'test -e ${nodes.generation3.config.security.dhparams.path}'
+
);
+
};
+
+
${switchToGeneration 4}
+
+
subtest "check bit sizes dhparam files", sub {
+
${assertParamBits 4 "foo2" 18}
+
${assertParamBits 4 "bar2" 19}
+
};
+
+
subtest "check whether dhparam files are in the Nix store", sub {
+
$machine->succeed(
+
'expr match ${getParamPath 4 "foo2"} ${builtins.storeDir}',
+
'expr match ${getParamPath 4 "bar2"} ${builtins.storeDir}',
+
);
+
};
+
+
${switchToGeneration 5}
+
+
subtest "check whether defaultBitSize works as intended", sub {
+
${assertParamBits 5 "foo3" 30}
+
${assertParamBits 5 "bar3" 30}
+
};
+
'';
+
}