Encrypted datasets cannot have unencrypted children

[rlaager]

System information

Type	Version/Name
Distribution Name	Ubuntu
Distribution Version	18.04
Linux Kernel	4.15.0-15-generic
Architecture	amd64
ZFS Version	0.8.0~rc4-1~rlaager1.18.04.1
SPL Version	0.8.0~rc4-1~rlaager1.18.04.1

Describe the problem you're observing

I cannot create an unencrypted child of an encrypted dataset.

This is really annoying if one wants to encrypt the root pool, but create some unencrypted exceptions, which I had planned to do. This user has the same issue: https://www.reddit.com/r/zfs/comments/bnv8d1/openzfs_080_encryption_dont_encrypt_the_pool_root/

This seems to be intentional, as lib/libzfs/libzfs_crypto.c has this:

        /* Check for encryption being explicitly truned off */
        if (crypt == ZIO_CRYPT_OFF && pcrypt != ZIO_CRYPT_OFF) {
                ret = EINVAL;
                zfs_error_aux(hdl, dgettext(TEXT_DOMAIN,
                    "Invalid encryption value. Dataset must be encrypted."));
                goto out;
        }

@tcaputi, what is the point of this restriction? If this is purely artificial, I think we should remove it.

I realize it's possible to work around this by creating an extra dataset layer for encryption, but that's a pain for root pool setups. I'd have to have rpool/encrypted/ROOT/ubuntu and rpool/encrypted/var/log, etc. if I want to allow for the possibility to have something unencrypted. This artificial difference in dataset structure between encrypted and unencrypted setups is problematic for tooling. For example, Ubuntu is investigating root-on-ZFS. Their tooling would have to account for this. Or, we'd have to have some extra dataset level all the time, for everything, which seems unnecessary and runs counter to current root-on-ZFS conventions.

Describe how to reproduce the problem

$ zfs get encryption rpool
NAME   PROPERTY    VALUE        SOURCE
rpool  encryption  aes-256-gcm  -
$ zfs create -o encryption=off rpool/test
cannot create 'rpool/test': Invalid encryption value. Dataset must be encrypted.

Include any warning/errors/backtraces from the system logs

[tcaputi]

It is artificial, but there is a security reason for it. The primary concern here was that an attacker with root access can still rename / move datasets because the encryption code really only works on a per-dataset level. Pool metadata is not encrypted. The thought was that an attacker (or an accident as @kpande mentioned) could move an unencrypted dataset underneath an encrypted one and trick an application to write its data there where it isn't safe.

We have talked about this issue a bit, however. It has been brought to my attention that ZFS on Illumos would like the ability to do this, since their crash dumping mechanism basically just writes the dump to preallocated space in zfs , without doing any transforms (compression / encryption) or checksumming. Therefore they can't have crash dumps on a pool that is entirely encrypted. We haven't really talked about this in a couple months though.

Perhaps @behlendorf will have some thoughts on what we want to do when he gets back.

[kithrup]

An unencrypted child of an encrypted dataset allows for one administrative trick which may be useful: it ensures that the unencrypted dataset cannot be mounted without the keys being loaded.

I don't know that this trick is worth the risks.

[tcaputi]

I believe we can get around that either way with zfs set mountpoint

[jasonbking]

That's correct -- for illumos to be able to dump to a volume, it must be unencrypted as zio's are not used to write the blocks (a simple polled I/O method for the underlying HBA is used). Any encryption of the dump volume has to happen at a higher layer due to that. In the Joyent zfs-crypto branch, we have a simple patch that disables the check for zvols.

I had contemplated perhaps instead having a create-time property that controls the ability to create unencrypted children, but not sure how clunky or confusing that might be

[rlaager]

The thought was that an attacker (or an accident as @kpande mentioned) could move an unencrypted dataset underneath an encrypted one and trick an application to write its data there where it isn't safe.

This restriction seems very incomplete, as the dataset hierarchy is not necessarily 1:1 with the filesystem hierarchy anyway. In other words, what is different between these two:

This presumably works (assuming unencrypted tank):

zfs create -o encryption=on ... -o mountpoint=/foo     tank/foo
zfs create -o encryption=off    -o mountpoint=/foo/bar tank/bar

This does not work (but I want it to):

zfs create -o encryption=on ... -o mountpoint=/foo     tank/foo
zfs create -o encryption=off    -o mountpoint=/foo/bar tank/foo/bar

In either case, writing to /foo/bar is unencrypted.

[numinit]

As the other user in question (that post was deleted and re-created to fix a typo in the title, BTW), I think unencrypted children of encrypted datasets is more confusing and could cause more security problems, and actually agree with the original behavior. I also thought that storing files on a pool root wasn't a best practice anyway, since it rules out using zfs destroy if you decide that the dataset should be deleted in the future.

Whatever you decide, just make sure it's documented so users don't cause problems for themselves. :-)

[rlaager]

@numinit I'm not suggesting storing files in the root dataset.

My actual use case is as follows: 1) I want everything encrypted by default, to protect my personal data. 2) But, a large amount of data on the system (~75% by size) is completely non-sensitive (legal!) media files, which do not need encryption, so I'd rather those be unencrypted.

A counter-point would be: Is it really a problem to encrypt everything? Is there an actual performance impact?

But I don't see why the system should prevent me from implementing a perfectly reasonable setup in exchange for partially (but not anywhere near completely) protecting people from shooting themselves in the foot if they make unfounded assumptions (i.e. that everything is encrypted).

[mtippmann]

A counter-point would be: Is it really a problem to encrypt everything? Is there an actual performance impact?

If you have AES-NI or the AMD equivalent it should be completly transparent with maybe a very small CPU overhead - without AES-NI you are probably bottlenecked depending on the algorithm ~50-80MByte/s. Even on NVMe with a modern CPU you should be able to reach speeds >1GByte/s.

Every CPU since 2010? supports AES-NI.

[ideologysec]

Though, since the 5.0 kernel broke the use of kernel_fpu, the performance hit is quite severe...

https://www.phoronix.com/scan.php?page=news_item&px=NixOS-Linux-5.0-ZFS-FPU-Drop

[rlaager]

My CPU does not support AES-NI.

[tcaputi]

The conversation here has gotten a bit off topic. But to give a few quick answers:

A counter-point would be: Is it really a problem to encrypt everything?

It can be a problem. Some people don't really want to deal with key management that comes with encrypting everything. It can also be a configuration problem if you're booting off of your zpool.

Is there an actual performance impact?

If you have AES-NI or the AMD equivalent it should be completly transparent with maybe a very small CPU overhead - without AES-NI you are probably bottlenecked depending on the algorithm ~50-80MByte/s. Even on NVMe with a modern CPU you should be able to reach speeds >1GByte/s. Every CPU since 2010? supports AES-NI.

There can be a performance impact even on newer CPUs. Although many CPUs are capable of encrypting a ton of data very quickly, that is almost never their only job. Just within ZFS the CPU is also responsible for raidz, checksumming, and compression, not to mention the overhead of running ZFS, the OS, and any applications as well. All of these tasks must be shared by the CPU, which can cause it to become the performance bottleneck in some instances.

All of that being said, this getting a bit off-topic. There are definitely valid use cases for both encrypted and unencrypted datasets. So lets keep talk here about how those should be handled. Any other questions or concerns regarding encryption can be emailed to me or posted as separate issues.

[rlaager]

@tcaputi Does this safety check fire at import and/or mount time? If it's only firing at dataset creation time, then it does nothing to stop the hypothetical attacker with offline access to my disk anyway.

[tcaputi]

@rlaager There should be checks at mount time as well (but I'd need to check). After hearing the concerns about crash dumps we might very well decide to scrap this mechanism in favor of something else anyway. I will discuss it with @behlendorf when he gets back from vacation.

[zviratko]

@tcaputi User with administrative access can still create an unencrypted dataset in a different tree and set its mountpoint beneath the encrypted one. Or he can simply skip ZFS, and create an ext4 mount backed by a file or something to that effect. This would be more obvious but I don't think disallowing unencrypted datasets nested in encrypted ones solves anything, really...
(Thinking about it - there seems to be no mechanism for an application to make sure that data is written encrypted, something like O_ENCRYPT would be very useful generally)

[behlendorf]

Given the useful discussion and very reasonable use cases outlined above I think we should consider providing a way to relax this artificial restriction when appropriate.