[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

Looks like we managed to pass all tests finally. I plan to squash everything into one single commit unless there is some other number of commits people would prefer..? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub:

Re: [developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

On 12/10/2017 12:47, Joerg Schilling wrote: Jorgen Lundman wrote: I prefer `print` myself, but there were no uses of `print` in existing zfs-tester test files, whereas there was precedent in using `tr -d` in `zfs_create_014_pos`. "print" is non standard and tus

Re: [developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

Jorgen Lundman wrote: > I prefer `print` myself, but there were no uses of `print` in existing > zfs-tester test files, whereas there was precedent in using `tr -d` in > `zfs_create_014_pos`. "print" is non standard and tus non-portable! "print" is a ksh-ism that

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

@lundman pushed 2 commits. 53406a2 More corrections to testers bf0d5b0 Disable dump on encrypted zvol -- You are receiving this because you are subscribed to this thread. View it on GitHub:

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

I think you missed this from ZOL in the latest update: ```diff diff --git a/usr/src/uts/common/fs/zfs/bptree.c b/usr/src/uts/common/fs/zfs/bptree.c index c74d072..25c08f6 100644 --- a/usr/src/uts/common/fs/zfs/bptree.c +++ b/usr/src/uts/common/fs/zfs/bptree.c @@ -211,7 +211,8 @@

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

@lundman pushed 1 commit. 599316e OpenZFS repo runs delphix.run in tests, so update it to match -- You are receiving this because you are subscribed to this thread. View it on GitHub:

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

sorry to report that send/receive (see Aug. 18th/19th) still crashes when testing with more than 10GB of data: panic[cpu1]/thread=ff000faf4c40: BAD TRAP: type=e (#pf Page fault) rp=ff000faf4870 addr=68 occurred in module "zfs" due to a NULL pointer dereference sched: #pf Page fault Bad

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

This PR has been updated with latest commits up and including Oct 2nd "060e4cf33002c4". -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/openzfs/openzfs/pull/124#issuecomment-334077937

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

Still crashing with send/receive. Not related to RAIDZ, also happens with single disk pool. btw dmu_send.c, patch does not apply cleanly here: struct drr_object *drro = >rrd->header.drr_u.drr_object; - uint32_t size = P2ROUNDUP(drro->drr_bonuslen, 8); +

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

@lundman pushed 1 commit. 807e3db Minor code style changes -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/openzfs/openzfs/pull/124/files/0b4ee95274efe6e3103402a8d390e9206cca2669..807e3dbdbda5090243d8d534c010ee6ccb34187a

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

@lundman pushed 1 commit. 0b4ee95 prevent panic when non-raidz checksum error detected -- You are receiving this because you are subscribed to this thread. View it on GitHub:

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

pcd1193182 requested changes on this pull request. I just looked through the zfs send related code, but that part of it seemed pretty good to me. > @@ -1332,9 +1533,17 @@ recv_begin_check_existing_impl(dmu_recv_begin_arg_t > *drba, dsl_dataset_t *ds, /* if full, then must be

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

pcd1193182 commented on this pull request. > @@ -1332,9 +1533,17 @@ recv_begin_check_existing_impl(dmu_recv_begin_arg_t > *drba, dsl_dataset_t *ds, /* if full, then must be forced */ if (!drba->drba_cookie->drc_force) return

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

Now contains the send/recv fixes from ZOL. Please test. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/openzfs/openzfs/pull/124#issuecomment-324526413 --

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

@lundman pushed 1 commit. 57291b0 de-linting -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/openzfs/openzfs/pull/124/files/8e2c3b855a7eb155744586ec610d642c2c46a5c9..57291b074f93cd588106df1a5c0147778c600c6e

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

I can confirm that a `send --raw plain` into encrypted panics, which shouldn't be allowed. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/openzfs/openzfs/pull/124#issuecomment-323559993

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

exactly. But you might need more than 10GB data to transfer. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/openzfs/openzfs/pull/124#issuecomment-323513670 --

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

Ah volumes, so something like ``` # zpool create -f BOOM c2t3d0 # zfs create BOOM/plain # zfs create -o encryption=on -o keyformat=passphrase BOOM/ccm Enter passphrase: Re-enter passphrase: # zfs create -V 1G BOOM/vol # zfs snapshot BOOM/vol@send # zfs send BOOM/vol@send | zfs recv BOOM/ccm/evol

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

I am sending a unencrypted zvol to an encrypted pool. Now the crash is different: panic[cpu0]/thread=ff000fb50c40: BAD TRAP: type=e (#pf Page fault) rp=ff000fb50540 addr=68 occurred in module "zfs" due to a NULL pointer dereference zpool-zones: #pf Page fault Bad kernel fault at

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

@zettabot go -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/openzfs/openzfs/pull/124#issuecomment-323239573 -- openzfs-developer Archives:

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

@GernotS are these the commands used? I've tried it with latest commit: ``` # zpool create -f BOOM c2t3d0 # zfs create -o encryption=aes-256-ccm -o keyformat=passphrase BOOM/ccm # zfs create BOOM/plain # zfs snapshot BOOM/plain@send # zfs send BOOM/plain@send | zfs recv BOOM/ccm/new # zfs get

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

still crashing when trying to zfs send/receive from an unencrypted to an encrypted dataset: panic[cpu1]/thread=ff000feecc40: assertion failed: spa_do_crypt_abd(B_TRUE, spa, zio->io_bookmark.zb_objset, bp, zio->io_txg, psize, zio->io_abd, eabd, iv, mac, salt, _crypt) == 0 (0x5 == 0x0), file:

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

@lundman pushed 1 commit. e418fcb Small bug fixes -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/openzfs/openzfs/pull/124/files/c26140e60da6f8a885be38fd339e9c5e2781007c..e418fcbb7204fafc5f0b77881739fa0289175352

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

@arekinath The panic should be fixed in this PR. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/openzfs/openzfs/pull/124#issuecomment-321363891 -- openzfs-developer

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

@ahrens Ok. That works for me. Is the change to make that an error going to go in this repo and this PR? Or be a separate but we file and integrate just in illumos when we merge this up (seeing as it's illumos specific)? -- You are receiving this because you are subscribed to this thread.

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

@arekinath Yes, it should be an error to dumpify (i.e. disable checksums/encryption on) a zvol that has encryption on. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub:

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

I totally understand wanting to have encrypted dumps. However, the current designs of dump and encryption are incompatible. Specifically: when dumping we do not modify any ZFS data structures; instead we just overwrite pre-allocated space. Therefore dump doesn't have checksums, doesn't

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

Actually I'm gonna correct myself there a bit, I'm pretty sure after more reading that the thing that was too complex to have in dump code was not so much computing Fletcher4 itself, as updating the actual pool structure and committing a new txg (but hopefully someone who knows more about it

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

> Are you serious? I would definitely not want to expose plaintext dumps of > kernel memory if I'm otherwise encrypting the entire pool. (I don't know if > encryption keys can be dumped, but if so, that's obviously a huge problem) I am, unfortunately, serious - there's a big tension here

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

Because dumps are useful for post-mortem debugging of production failures as well; OmniOS for example creates a dump device by default. Even if not, it's not nice to have a "trap" like that where the user thinks they're encrypting the entire disk but crash dumps happen to be special. -- You

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

@arekinath > And I think the dump zvol should not be encrypted Are you serious? I would definitely not want to expose plaintext dumps of kernel memory if I'm otherwise encrypting the entire pool. -- You are receiving this because you are subscribed to this thread. Reply to this email directly

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

I debugged the crash that I got a bit further, in case it's helpful: When we died we're trying to encrypt a dnode, and `zio_crypt_init_uios_dnode` is setting `total_len = 0` (as well as `*no_crypt = B_TRUE`). In `zio_do_crypt_data` we carry on to call `zio_do_crypt_uio` after this returns,

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

@ahrens care to weigh in on how to handle this at all? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/openzfs/openzfs/pull/124#issuecomment-321138143 --

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

> Could you create pool/dump, pool/swap and pool/encrypted? Is there a reason > this data shouldn't be encrypted in the first place (even though it should > only be used by the system)? Yeah, we probably can do that, there's just a disturbing number of scripts that assume things are in the

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

So, when I do: ``` # zfs create -o encryption=on -o checksum=off -o keyformat=passphrase -V 4G tank/testvol ``` I get this panic: ``` > ::status debugging crash dump ./vmcore.9 (64-bit) from 00-0c-29-5d-4c-f7 operating system: 5.11 joyent_20170807T220431Z (i86pc) image uuid: (not set) panic

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

@ahrens I tried an earlier version of this patch, which crashed in debug builds. The tests ran fine on a non-debug build though. See https://github.com/openzfs/openzfs/pull/124#issuecomment-288516088 ff and https://github.com/openzfs/openzfs/pull/124#issuecomment-285545199 ff I'm planing to

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

@ilovezfs I'd like to see the encryption changes merged to ZoL first, since that's where they originated and they've gotten the most testing and code review on Linux. https://github.com/zfsonlinux/zfs/pull/5769 We also need to figure out the failing tests (which might or might not be the

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

@ahrens what are the blockers to getting this merged? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/openzfs/openzfs/pull/124#issuecomment-320549971 --

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

A few of the tests fail that might not be related, which tests do I need to look at wrt to this PR ? -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/openzfs/openzfs/pull/124#issuecomment-316238036

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

@lundman pushed 1 commit. ed02f22 bug fixes -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/openzfs/openzfs/pull/124/files/2992551a9e789513fdd9a5c7868771d78c12b8bf..ed02f22933e7c5c9ae31cbd3cbaf1322c302f299

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

tcaputi commented on this pull request. > @@ -1200,6 +1210,18 @@ transactions. Not all damaged pools can be recovered by using this option. If successful, the data from the discarded transactions is irretrievably lost. This option is ignored if the pool is importable or already imported.

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

@lundman pushed 1 commit. a19f544 zfs-tests corrections -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/openzfs/openzfs/pull/124/files/1045b9c5762c2f0bad1110fe1303f461a1b96aff..a19f54495262141f5a551d525c9d6179276c066e

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

Ok I went through the FAILs I got locally ``` functional/cli_root/zpool_get/zpool_get_002_pos (run as root) [00:00] [FAIL] ``` It seems `zpool_get.cfg` is missing the `bootsize` property, which is outside of the scope of this PR, so I will ignore it. ``` 14:17:01.36 Assertion failed:

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

found another assert when doing a simple send/recv from an unencrypted to an encrypted dataset: zfs create -o canmount=noauto -o encryption=on -o keyformat=passphrase -o keylocation=prompt backup/test zfs send zones/testdata@1 |zfs recv backup/test/testdata

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

@lundman pushed 1 commit. 1045b9c Remove rngd from tests -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/openzfs/openzfs/pull/124/files/7e91e1af19bceb66dcddfc8776e281e7a72d685e..1045b9c5762c2f0bad1110fe1303f461a1b96aff

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

`rngd` is a Linux helper for random, it can just be `echo` or similar on Illumos. I'll have a look at the tests in the morning. -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub:

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

doing a zfs destroy on an encrypted dataset without loading the key causes a panic: panic[cpu0]/thread=ff00116d0c40: BAD TRAP: type=e (#pf Page fault) rp=ff00116cff50 addr=18 occurred in module "zfs" due to a NULL pointer dereference sched: #pf Page fault Bad kernel fault at addr=0x18

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

@lundman pushed 1 commit. 9170e97 Correct code in raidz_checksum_error() -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/openzfs/openzfs/pull/124/files/84fbf78cb5c75fe5027fb3edcaeacb496e25400c..9170e977ffe1f483173d85dece954b08e2ff

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

The test is the one immediate after ``` /opt/zfs-tests/tests/functional/cli_root/zpool_clear/setup ``` ie ``` zpool_clear_001_pos.ksh ``` -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub:

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

@prakashsurya can you identify test what produce panic? and we can run only this test or just couple of tests in this are for reproduce it? can we identify minimal steps with scenario for reproduce steps for this panic ? -- You are receiving this because you are subscribed to this thread. Reply

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

It was my guess that all the zdb cores (there are many of them) made the tests run slow, and ended up in a timeout.. I will take a look at the zdb assert and see if it is something obvious. Cheers for your feedback -- You are receiving this because you are subscribed to this thread. Reply to

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

re: ``` Cannot contact i-09a7ea9f1f400ab9b: java.lang.InterruptedException Could not connect to i-09a7ea9f1f400ab9b to send interrupt signal to process ``` we have an 8 hour time limit set on the zfstest runs, so if any run takes longer than that, it'll be killed by the Jenkins framework, and the

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

@lundman pushed 2 commits. bbf9bd3 Fixes and improvements after 5th round of review 439cd9c Fixes after rebase and more review -- You are receiving this because you are subscribed to this thread. View it on GitHub:

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

@lundman pushed 1 commit. f68ee3c Attempt to update the manpages -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/openzfs/openzfs/pull/124/files/79798fac7fde50d17bd90222d13ee0ac3d806be3..f68ee3ce27b2a9758ae41e0addf24b511fcd1cb4

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

@lundman pushed 2 commits. 27bc76c ABD released too early in abd_hmac check 785086e Corrections in zio_checksum for crypto -- You are receiving this because you are subscribed to this thread. View it on GitHub:

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

ahrens commented on this pull request. > @@ -444,7 +448,7 @@ blkptr(uintptr_t addr, uint_t flags, int argc, const > mdb_arg_t *argv) } SNPRINTF_BLKPTR(mdb_snprintf, '\n', buf, sizeof (buf), bp, type, - checksum, compress); + checksum, 0, compress); 1.

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

@lundman pushed 1 commit. dc4e629 Fixes -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/openzfs/openzfs/pull/124/files/6c89a04a59661b4bedc7e4b892c0a100204695da..dc4e629f2d708e7241963ddaa8b94996863075d9

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

@lundman pushed 1 commit. 6c89a04 Delinting -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/openzfs/openzfs/pull/124/files/ea53ac7930549f31a8d6480ab697c404ffbc05f4..6c89a04a59661b4bedc7e4b892c0a100204695da

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

@lundman looks like there are some lint errors. If you click through to "Details" of the test failure you'll see them: ``` "../../../common/modules/zfs/zfs.c", line 451: warning: suspicious comparison of unsigned with 0: op "<=" (E_SUSPICIOUS_COMPARISON) "../../../common/modules/zfs/zfs.c",

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

@lundman pushed 1 commit. ea53ac7 Compile fixes -- You are receiving this because you are subscribed to this thread. View it on GitHub: https://github.com/openzfs/openzfs/pull/124/files/0f52c029de5953d706bec7b3d288b5cc6d6f0072..ea53ac7930549f31a8d6480ab697c404ffbc05f4

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

Pushed the latest commits, I did not do the manpage changes, specifically those in https://github.com/zfsonlinux/zfs/pull/5769/commits/705a2bfc5eabb45facf1e0f7a38a3ee5a69a54ea#diff-42027d93bce823d185f48bd265de4ac0

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

@lundman pushed 2 commits. 24aa9ff Various fixes and improvements 0f52c02 Fixes and improvements after 4th round of review -- You are receiving this because you are subscribed to this thread. View it on GitHub:

[developer] Re: [openzfs/openzfs] Native data and metadata encryption for zfs (#124)

The PR is just 3 weeks away from its first birthday, yaaay \o/ -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/openzfs/openzfs/pull/124#issuecomment-302028603 --