Project

General

Profile

Actions
Copy link

Bug #13697

closed

zfs change-key does not follow clones, data loss ensues

Added by Alex Wilson about 2 years ago. Updated about 2 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Category:
-
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:
1405
External Bug:

Description

Currently zfs change-key does not properly cross clone boundaries:

root@omniosce:~# zfs create -o encryption=aes-256-gcm -o keyformat=passphrase rpool/enctest
Enter passphrase:
Re-enter passphrase:
root@omniosce:~# zfs create rpool/enctest/empty
root@omniosce:~# zfs snapshot rpool/enctest/empty@final
root@omniosce:~# zfs clone rpool/enctest/empty@final rpool/foobar
root@omniosce:~# zfs create rpool/foobar/baz
root@omniosce:~# echo hi > /rpool/foobar/baz/test.txt
root@omniosce:~# zfs umount rpool/foobar/baz
root@omniosce:~# zfs umount rpool/foobar    
root@omniosce:~# zfs change-key rpool/enctest
Enter new passphrase for 'rpool/enctest':
Re-enter new passphrase for 'rpool/enctest':
root@omniosce:~# zfs mount rpool/foobar
root@omniosce:~# zfs mount rpool/foobar/baz
cannot mount 'rpool/foobar/baz': Permission denied

This EACCES is being triggered by the MAC on the wrapped key failing because we never updated it. Once we get here, the rpool/foobar/baz dataset and all data on it is irretrievable.

Bug was fixed in 2019 in OpenZFS/ZoL: https://github.com/openzfs/zfs/pull/9294

Commit on their side was https://github.com/openzfs/zfs/commit/637f0c6019a67b7cc3f548ec254c5c55e2d1ef1e

My quick-and-dirty cherry-pick: https://github.com/arekinath/illumos-joyent/commit/a430005b58d95a3ab4f74f950abb7fe16e2b9c98 (haven't checked the tests yet)

Actions
Copy link
 #1

Updated by Jason King about 2 years ago

Just to copy over the ZFS commit info:

Fix clone handling with encryption roots

    Currently, spa_keystore_change_key_sync_impl() does not recurse
    into clones when updating encryption roots for either a call to
    'zfs promote' or 'zfs change-key'. This can cause children of
    these clones to end up in a state where they point to the wrong
    dataset as the encryption root. It can also trigger ASSERTs in
    some cases where the code checks reference counts on wrapping
    keys. This patch fixes this issue by ensuring that this function
    properly recurses into clones during processing.

    Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
    Reviewed-by: Alek Pinchuk <apinchuk@datto.com>
    Signed-off-by: Tom Caputi <tcaputi@datto.com>
    Closes #9267
    Closes #9294
Actions
Copy link
 #2

Updated by Jason King about 2 years ago

The ZFS test suite passes -- no new failures, only the expected failures (with existing tickets).

Actions
Copy link
 #3

Updated by Electric Monk about 2 years ago

  • Gerrit CR set to 1405
Actions
Copy link
 #4

Updated by Electric Monk about 2 years ago

  • Status changed from New to Closed
  • % Done changed from 0 to 100

git commit 11326df80789c71d3ac24d5ff3da2c1c0617961a

commit  11326df80789c71d3ac24d5ff3da2c1c0617961a
Author: Tom Caputi <tcaputi@datto.com>
Date:   2021-04-07T19:01:09.000Z

    13697 zfs change-key does not follow clones, data loss ensues
    Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
    Reviewed by: Alek Pinchuk <apinchuk@datto.com>
    Reviewed by: Andy Fiddaman <andy@omnios.org>
    Reviewed by: Vitaliy Gusev <gusev.vitaliy@icloud.com>
    Portions contributed by: Alex Wilson <alex@cooperi.net>
    Portions contributed by: Jason King <jason.brian.king@gmail.com>
    Approved by: Dan McDonald <danmcd@joyent.com>

Actions
Copy link

Also available in: Atom PDF