Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add raw flag when replicating encrypted datasets #26

Closed

Conversation

sevmonster
Copy link
Contributor

@sevmonster sevmonster commented Jul 26, 2021

Adds -w to zfs send when the dataset is encrypted. This is required to send encrypted datasets without having the key loaded and to keep that key set on the receiving side.
I do not see a reason to prevent the flag from being added, so I did not add an argument to zap to do so—having an encrypted dataset and then sending it unencrypted after they key was loaded sounds pretty weird. It is possible to do but I don't know why anyone would want to.

I still prefer zap for its simplicity over more robust solutions... With this PR, I can finally use zap in production on Linux 🎉

@Jehops
Copy link
Owner

Jehops commented Jul 26, 2021

Glad this is working for you.

I haven't done any testing, but there could be problem with systems that don't yet support encryption like perhaps some older but still supported Debian systems and FreeBSD 12.x. I think we could test whether the pool supports encryption with something like if [ "$(zpool get -H -o value feature@encryption zroot)" = 'enabled' ]; then.... In such a case, just supplying -w regardless whether the dataset has encryption on or not could work. ZFS-SEND(8) says "For unencrypted datasets, [the -w] flag will be equivalent to -Lec."

I'll push my work-in-progress branch (based on a patch David Samms emailed me). If you have the time and motivation any suggestions would be appreciated.

@sevmonster
Copy link
Contributor Author

sevmonster commented Jul 26, 2021

In such a case, just supplying -w regardless whether the dataset has encryption on or not could work. ZFS-SEND(8) says "For unencrypted datasets, [the -w] flag will be equivalent to -Lec."

I thought about just adding -w to all attempts, but then zap -C will no longer work for unencrypted datasets. Is there a problem trying to read the encryption value and just ignoring any errors if it does not exist? If it doesn't exist then the pool doesn't support it anyway.

@Jehops
Copy link
Owner

Jehops commented Jul 26, 2021

Compression could be a problem to deal with now that it's a separate argument. I have to do lots of testing to confirm everything, but $job is very busy right now and for the next month or two. I just pushed what I have.

@Jehops
Copy link
Owner

Jehops commented Jul 26, 2021

If you are able to test this on a system that doesn't support encryption (FreeBSD 12.2) and run it through https://www.shellcheck.net/, then I think we could merge.

@Jehops
Copy link
Owner

Jehops commented Jul 28, 2021

I just pushed a version to the encryption branch. It's similar to your version, but does a check at the pool level to see if encryption is supported. If you have a few minutes, could you confirm that it works for you with your setup?

Thanks,

Joe

@sevmonster
Copy link
Contributor Author

I like checking feature flags, but I just wanted to be sure it would not introduce issues to always use -w if encryption is active for the pool. Skimming the commit history for encryption and compression in OpenZFS, I don't think it will. I will test 0.8.3 in a little while, but I do not think it should cause any problems. My only concern is that both the sending and receiving pools should be checked to ensure there are no problems.

Since feature flags are being used for raw sends, why not also detect if compression is active and only add the -c option if it is? If you are too busy I'd like to open a PR for that.

@sevmonster sevmonster closed this Jul 29, 2021
@Jehops
Copy link
Owner

Jehops commented Jul 29, 2021

We could do that. If I'm recalling correctly, the -c/-C options were added based on emails I received from a NetBSD user and a Debian user whose systems still didn't support compression when replicating. I suspect (but am not sure) that compression is supported on those systems, just not when replicating. If we do end up setting the compression option automatically, we should test on those systems to be certain that we are doing the appropriate check. That is, does the result from zpool get -H -o value feature@lz4_compress zroot really tell us whether compression is supported with replication?

By the way, for raw sending, I ended up doing some tests myself and releasing 0.8.3 without waiting since I also wanted to get a fix in to a release for that syntax error that you reported.

If you do have time and motivation, a PR for issue #19 (or #9 or #11) would be welcomed. I won't have time to tackle any of those issues until the fall.

Thanks,

Joe

@sevmonster sevmonster deleted the feature/encrypted-raw-send branch February 20, 2024 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants