Merge pull request #131118 from etu/sanoid-syncoid-improvements

nixos/{syncoid,sanoid}: Improve ZFS permission delegation
This commit is contained in:
Elis Hirwing 2021-07-26 11:40:51 +02:00 committed by GitHub
commit 699ea65439
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 363 additions and 299 deletions

View file

@ -702,6 +702,19 @@
option.
</para>
</listitem>
<listitem>
<para>
The
<link xlink:href="options.html#opt-services.syncoid.enable">services.syncoid.enable</link>
module now properly drops ZFS permissions after usage. Before
it delegated permissions to whole pools instead of datasets
and didnt clean up after execution. You can manually look
this up for your pools by running
<literal>zfs allow your-pool-name</literal> and use
<literal>zfs unallow syncoid your-pool-name</literal> to clean
this up.
</para>
</listitem>
</itemizedlist>
</section>
</section>

View file

@ -183,3 +183,5 @@ pt-services.clipcat.enable).
- NSS modules which should come after `dns` should use mkAfter.
- The [networking.wireless.iwd](options.html#opt-networking.wireless.iwd.enable) module has a new [networking.wireless.iwd.settings](options.html#opt-networking.wireless.iwd.settings) option.
- The [services.syncoid.enable](options.html#opt-services.syncoid.enable) module now properly drops ZFS permissions after usage. Before it delegated permissions to whole pools instead of datasets and didn't clean up after execution. You can manually look this up for your pools by running `zfs allow your-pool-name` and use `zfs unallow syncoid your-pool-name` to clean this up.

View file

@ -52,7 +52,7 @@ let
use_template = mkOption {
description = "Names of the templates to use for this dataset.";
type = types.listOf (types.enum (attrNames cfg.templates));
default = [];
default = [ ];
};
useTemplate = use_template;
@ -70,21 +70,36 @@ let
processChildrenOnly = process_children_only;
};
# Extract pool names from configured datasets
pools = unique (map (d: head (builtins.match "([^/]+).*" d)) (attrNames cfg.datasets));
# Extract unique dataset names
datasets = unique (attrNames cfg.datasets);
configFile = let
# Function to build "zfs allow" and "zfs unallow" commands for the
# filesystems we've delegated permissions to.
buildAllowCommand = zfsAction: permissions: dataset: lib.escapeShellArgs [
# Here we explicitly use the booted system to guarantee the stable API needed by ZFS
"-+/run/booted-system/sw/bin/zfs"
zfsAction
"sanoid"
(concatStringsSep "," permissions)
dataset
];
configFile =
let
mkValueString = v:
if builtins.isList v then concatStringsSep "," v
else generators.mkValueStringDefault {} v;
else generators.mkValueStringDefault { } v;
mkKeyValue = k: v: if v == null then ""
mkKeyValue = k: v:
if v == null then ""
else if k == "processChildrenOnly" then ""
else if k == "useTemplate" then ""
else generators.mkKeyValueDefault { inherit mkValueString; } "=" k v;
in generators.toINI { inherit mkKeyValue; } cfg.settings;
in
generators.toINI { inherit mkKeyValue; } cfg.settings;
in {
in
{
# Interface
@ -105,13 +120,13 @@ in {
};
datasets = mkOption {
type = types.attrsOf (types.submodule ({config, options, ...}: {
type = types.attrsOf (types.submodule ({ config, options, ... }: {
freeformType = datasetSettingsType;
options = commonOptions // datasetOptions;
config.use_template = mkAliasDefinitions (mkDefault options.useTemplate or {});
config.process_children_only = mkAliasDefinitions (mkDefault options.processChildrenOnly or {});
config.use_template = mkAliasDefinitions (mkDefault options.useTemplate or { });
config.process_children_only = mkAliasDefinitions (mkDefault options.processChildrenOnly or { });
}));
default = {};
default = { };
description = "Datasets to snapshot.";
};
@ -120,7 +135,7 @@ in {
freeformType = datasetSettingsType;
options = commonOptions;
});
default = {};
default = { };
description = "Templates for datasets.";
};
@ -135,7 +150,7 @@ in {
extraArgs = mkOption {
type = types.listOf types.str;
default = [];
default = [ ];
example = [ "--verbose" "--readonly" "--debug" ];
description = ''
Extra arguments to pass to sanoid. See
@ -156,18 +171,14 @@ in {
systemd.services.sanoid = {
description = "Sanoid snapshot service";
serviceConfig = {
ExecStartPre = map (pool: lib.escapeShellArgs [
"+/run/booted-system/sw/bin/zfs" "allow"
"sanoid" "snapshot,mount,destroy" pool
]) pools;
ExecStartPre = (map (buildAllowCommand "allow" [ "snapshot" "mount" "destroy" ]) datasets);
ExecStopPost = (map (buildAllowCommand "unallow" [ "snapshot" "mount" "destroy" ]) datasets);
ExecStart = lib.escapeShellArgs ([
"${pkgs.sanoid}/bin/sanoid"
"--cron"
"--configdir" (pkgs.writeTextDir "sanoid.conf" configFile)
"--configdir"
(pkgs.writeTextDir "sanoid.conf" configFile)
] ++ cfg.extraArgs);
ExecStopPost = map (pool: lib.escapeShellArgs [
"+/run/booted-system/sw/bin/zfs" "unallow" "sanoid" pool
]) pools;
User = "sanoid";
Group = "sanoid";
DynamicUser = true;
@ -182,4 +193,4 @@ in {
};
meta.maintainers = with maintainers; [ lopsided98 ];
}
}

View file

@ -5,16 +5,29 @@ with lib;
let
cfg = config.services.syncoid;
# Extract the pool name of a local dataset (any dataset not containing "@")
localPoolName = d: optionals (d != null) (
let m = builtins.match "([^/@]+)[^@]*" d; in
optionals (m != null) m);
# Extract local dasaset names (so no datasets containing "@")
localDatasetName = d: optionals (d != null) (
let m = builtins.match "([^/@]+[^@]*)" d; in
optionals (m != null) m
);
# Escape as required by: https://www.freedesktop.org/software/systemd/man/systemd.unit.html
escapeUnitName = name:
lib.concatMapStrings (s: if lib.isList s then "-" else s)
(builtins.split "[^a-zA-Z0-9_.\\-]+" name);
in {
# Function to build "zfs allow" and "zfs unallow" commands for the
# filesystems we've delegated permissions to.
buildAllowCommand = zfsAction: permissions: dataset: lib.escapeShellArgs [
# Here we explicitly use the booted system to guarantee the stable API needed by ZFS
"-+/run/booted-system/sw/bin/zfs"
zfsAction
cfg.user
(concatStringsSep "," permissions)
dataset
];
in
{
# Interface
@ -68,7 +81,7 @@ in {
commonArgs = mkOption {
type = types.listOf types.str;
default = [];
default = [ ];
example = [ "--no-sync-snap" ];
description = ''
Arguments to add to every syncoid command, unless disabled for that
@ -80,7 +93,7 @@ in {
service = mkOption {
type = types.attrs;
default = {};
default = { };
description = ''
Systemd configuration common to all syncoid services.
'';
@ -150,7 +163,7 @@ in {
service = mkOption {
type = types.attrs;
default = {};
default = { };
description = ''
Systemd configuration specific to this syncoid service.
'';
@ -158,7 +171,7 @@ in {
extraArgs = mkOption {
type = types.listOf types.str;
default = [];
default = [ ];
example = [ "--sshport 2222" ];
description = "Extra syncoid arguments for this command.";
};
@ -168,7 +181,7 @@ in {
sshKey = mkDefault cfg.sshKey;
};
}));
default = {};
default = { };
example = literalExample ''
{
"pool/test".target = "root@target:pool/test";
@ -193,37 +206,41 @@ in {
};
};
groups = mkIf (cfg.group == "syncoid") {
syncoid = {};
syncoid = { };
};
};
systemd.services = mapAttrs' (name: c:
systemd.services = mapAttrs'
(name: c:
nameValuePair "syncoid-${escapeUnitName name}" (mkMerge [
{ description = "Syncoid ZFS synchronization from ${c.source} to ${c.target}";
{
description = "Syncoid ZFS synchronization from ${c.source} to ${c.target}";
after = [ "zfs.target" ];
startAt = cfg.interval;
# syncoid may need zpool to get feature@extensible_dataset
path = [ "/run/booted-system/sw/bin/" ];
serviceConfig = {
ExecStartPre =
map (pool: lib.escapeShellArgs [
"+/run/booted-system/sw/bin/zfs" "allow"
cfg.user "bookmark,hold,send,snapshot,destroy" pool
# Permissions snapshot and destroy are in case --no-sync-snap is not used
]) (localPoolName c.source) ++
map (pool: lib.escapeShellArgs [
"+/run/booted-system/sw/bin/zfs" "allow"
cfg.user "create,mount,receive,rollback" pool
]) (localPoolName c.target);
(map (buildAllowCommand "allow" [ "bookmark" "hold" "send" "snapshot" "destroy" ]) (localDatasetName c.source)) ++
(map (buildAllowCommand "allow" [ "create" "mount" "receive" "rollback" ]) (localDatasetName c.target));
ExecStopPost =
# Permissions snapshot and destroy are in case --no-sync-snap is not used
(map (buildAllowCommand "unallow" [ "bookmark" "hold" "send" "snapshot" "destroy" ]) (localDatasetName c.source)) ++
(map (buildAllowCommand "unallow" [ "create" "mount" "receive" "rollback" ]) (localDatasetName c.target));
ExecStart = lib.escapeShellArgs ([ "${pkgs.sanoid}/bin/syncoid" ]
++ optionals c.useCommonArgs cfg.commonArgs
++ optional c.recursive "-r"
++ optionals (c.sshKey != null) [ "--sshkey" c.sshKey ]
++ c.extraArgs
++ [ "--sendoptions" c.sendOptions
"--recvoptions" c.recvOptions
++ [
"--sendoptions"
c.sendOptions
"--recvoptions"
c.recvOptions
"--no-privilege-elevation"
c.source c.target
c.source
c.target
]);
User = cfg.user;
Group = cfg.group;
@ -240,7 +257,7 @@ in {
# systemd-analyze security | grep syncoid-'*'
AmbientCapabilities = "";
CapabilityBoundingSet = "";
DeviceAllow = ["/dev/zfs"];
DeviceAllow = [ "/dev/zfs" ];
LockPersonality = true;
MemoryDenyWriteExecute = true;
NoNewPrivileges = true;
@ -266,7 +283,7 @@ in {
BindPaths = [ "/dev/zfs" ];
BindReadOnlyPaths = [ builtins.storeDir "/etc" "/run" "/bin/sh" ];
# Avoid useless mounting of RootDirectory= in the own RootDirectory= of ExecStart='s mount namespace.
InaccessiblePaths = ["-+/run/syncoid/${escapeUnitName name}"];
InaccessiblePaths = [ "-+/run/syncoid/${escapeUnitName name}" ];
MountAPIVFS = true;
# Create RootDirectory= in the host's mount namespace.
RuntimeDirectory = [ "syncoid/${escapeUnitName name}" ];
@ -277,8 +294,14 @@ in {
# perf stat -x, 2>perf.log -e 'syscalls:sys_enter_*' syncoid …
# awk >perf.syscalls -F "," '$1 > 0 {sub("syscalls:sys_enter_","",$3); print $3}' perf.log
# systemd-analyze syscall-filter | grep -v -e '#' | sed -e ':loop; /^[^ ]/N; s/\n //; t loop' | grep $(printf ' -e \\<%s\\>' $(cat perf.syscalls)) | cut -f 1 -d ' '
"~@aio" "~@chown" "~@keyring" "~@memlock" "~@privileged"
"~@resources" "~@setuid" "~@sync" "~@timer"
"~@aio"
"~@chown"
"~@keyring"
"~@memlock"
"~@privileged"
"~@resources"
"~@setuid"
"~@timer"
];
SystemCallArchitectures = "native";
# This is for BindPaths= and BindReadOnlyPaths=
@ -288,8 +311,9 @@ in {
}
cfg.service
c.service
])) cfg.commands;
]))
cfg.commands;
};
meta.maintainers = with maintainers; [ julm lopsided98 ];
}
}

View file

@ -85,10 +85,18 @@ in {
"chown -R syncoid:syncoid /var/lib/syncoid/",
)
assert len(source.succeed("zfs allow pool")) == 0, "Pool shouldn't have delegated permissions set before snapshotting"
assert len(source.succeed("zfs allow pool/sanoid")) == 0, "Sanoid dataset shouldn't have delegated permissions set before snapshotting"
assert len(source.succeed("zfs allow pool/syncoid")) == 0, "Syncoid dataset shouldn't have delegated permissions set before snapshotting"
# Take snapshot with sanoid
source.succeed("touch /mnt/pool/sanoid/test.txt")
source.systemctl("start --wait sanoid.service")
assert len(source.succeed("zfs allow pool")) == 0, "Pool shouldn't have delegated permissions set after snapshotting"
assert len(source.succeed("zfs allow pool/sanoid")) == 0, "Sanoid dataset shouldn't have delegated permissions set after snapshotting"
assert len(source.succeed("zfs allow pool/syncoid")) == 0, "Syncoid dataset shouldn't have delegated permissions set after snapshotting"
# Sync snapshots
target.wait_for_open_port(22)
source.succeed("touch /mnt/pool/syncoid/test.txt")
@ -96,5 +104,9 @@ in {
target.succeed("cat /mnt/pool/sanoid/test.txt")
source.systemctl("start --wait syncoid-pool-syncoid.service")
target.succeed("cat /mnt/pool/syncoid/test.txt")
assert len(source.succeed("zfs allow pool")) == 0, "Pool shouldn't have delegated permissions set after syncing snapshots"
assert len(source.succeed("zfs allow pool/sanoid")) == 0, "Sanoid dataset shouldn't have delegated permissions set after syncing snapshots"
assert len(source.succeed("zfs allow pool/syncoid")) == 0, "Syncoid dataset shouldn't have delegated permissions set after syncing snapshots"
'';
})

View file

@ -1,4 +1,4 @@
{ lib, stdenv, fetchFromGitHub, makeWrapper, zfs
{ lib, stdenv, fetchFromGitHub, nixosTests, makeWrapper, zfs
, perlPackages, procps, which, openssh, mbuffer, pv, lzop, gzip, pigz }:
with lib;
@ -17,6 +17,8 @@ stdenv.mkDerivation rec {
nativeBuildInputs = [ makeWrapper ];
buildInputs = with perlPackages; [ perl ConfigIniFiles CaptureTiny ];
passthru.tests = nixosTests.sanoid;
installPhase = ''
runHook preInstall