Skip to content

Commit

Permalink
zfs: load keys for encrypted datasets during pool import
Browse files Browse the repository at this point in the history
If a user has set up their own zpools and given them to us to manage,
it's possible they've configured ZFS-native encryption. For the most
part, this works completely transparently to us. However, because we
manually do zpool-import and zpool-export during startup and shutdown of
Incus, ZFS datasets with keys will have their keys unloaded during
shutdown and then the keys are not automatically loaded on startup. This
results in containers being unable to start on startup because all IOs
are blocked indefinitely until the dataset keys are loaded manually by
the admin -- even if the admin has configured automatic key loading on
their system!

The simplest solution would be to pass -l to zfs-import (which causes
ZFS to auto-import all keys for all datasets in the pool). However, for
users that use keylocation=prompt, doing this naively would result in
errors when importing pools (because there is no stdin for the
zpool-import command). So we would have to silently ignore errors -- but
this would mean we would ignore regular import errors as well.

Instead, we do zfs-load-key manually after checking if there are any
non-file:// keylocation properties set within the dataset. If there are
then we are more forgiving with zfs-load-key errors and output some
warnings to tell users why their containers are not starting (previously
containers would just fail to start with no explanation). If all of the
keys are file:// then we make any load-key errors hard errors to help
admins realise their keylocation= configurations are wrong.

Signed-off-by: Aleksa Sarai <[email protected]>
  • Loading branch information
cyphar committed Nov 16, 2024
1 parent c120d62 commit b13e66e
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 0 deletions.
14 changes: 14 additions & 0 deletions internal/server/storage/drivers/driver_zfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,20 @@ func (d *zfs) importPool() (bool, error) {
return false, err
}

// We need to explicitly import the keys here so containers can start. This
// is always needed because even if the admin has set up auto-import of
// keys on the system, because incus manually imports and exports the pools
// the keys can get unloaded.
//
// We could do "zpool import -l" to request the keys during import, but we
// also should give warnings to users if they've configured their ZFS
// datasets to have encryption setups that are incompatible with how incus
// manages its ZFS pools.
if err := d.loadKeys(d.config["zfs.pool_name"]); err != nil {
_, _ = d.Unmount()
return false, err
}

if exists {
return true, nil
}
Expand Down
49 changes: 49 additions & 0 deletions internal/server/storage/drivers/driver_zfs_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/lxc/incus/v6/internal/server/migration"
"github.com/lxc/incus/v6/shared/api"
"github.com/lxc/incus/v6/shared/ioprogress"
"github.com/lxc/incus/v6/shared/logger"
"github.com/lxc/incus/v6/shared/subprocess"
"github.com/lxc/incus/v6/shared/units"
"github.com/lxc/incus/v6/shared/util"
Expand Down Expand Up @@ -409,6 +410,54 @@ func (d *zfs) receiveDataset(vol Volume, r io.Reader, tracker *ioprogress.Progre
return nil
}

// loadKeys loads any encryption keys for the pool if the keylocation is a
// regular file (and thus can be loaded non-interactively).
func (d *zfs) loadKeys(dataset string) error {
lines, err := subprocess.RunCommand("zfs", "get", "-Hr", "-o", "name,value", "keylocation", dataset)
if err != nil {
return err
}

var isEncrypted, hasInteractiveKey bool
for _, line := range strings.Split(lines, "\n") {
// "name\tkeylocation"
fields := strings.SplitN(line, "\t", 2)
subPath, keyLocation := fields[0], fields[1]

// We need to check that all of the keylocations are either no-ops
// (none, -) or are regular files (file://*). Assume any other values
// (like prompt) are interactive to avoid hanging.
switch keyLocation {
case "", "-", "none":
continue
}
isEncrypted = true

// Log a warning if we hit a keylocation= that is not a regular file.
// We still try to load all of the keys anyway, but we know that this
// would normally silently fail for keylocation=prompt.
if !strings.HasPrefix(keyLocation, "file://") {
hasInteractiveKey = true
logger.Warnf("Dataset %q key cannot be loaded from keylocation %q non-interactively -- containers might fail to start until manual 'zfs load-key'", subPath, keyLocation)
}
}
if !isEncrypted {
return nil
}
if _, err := subprocess.RunCommand("zfs", "load-key", "-r", dataset); err != nil {
// If all the keys are non-interactive then an error should fail the
// load, since this indicates that the keylocation= is misconfigured or
// the key itself is wrong.
if !hasInteractiveKey {
return err
}
logger.AddContext(logger.Ctx{
"err": err,
}).Warn(fmt.Sprintf("Dataset %q keys could not be loaded non-interactively", dataset))
}
return nil
}

// ValidateZfsBlocksize validates blocksize property value on the pool.
func ValidateZfsBlocksize(value string) error {
// Convert to bytes.
Expand Down

0 comments on commit b13e66e

Please sign in to comment.