fix encrypt-on-receive with placeholders

fixes https://github.com/zrepl/zrepl/issues/504

Problem:
  plain send + recv with root_fs encrypted + placeholders causes plain recvs
  whereas user would expect encrypt-on-recv
Reason:
  We create placeholder filesytems with -o encryption=off.
  Thus, children received below those placeholders won't inherit
  encryption of root_fs.
Fix:
  We'll have three values for `recv.placeholders.encryption: unspecified (default) | off | inherit`.
  When we create a placeholder, we will fail the operation if  `recv.placeholders.encryption = unspecified`.
  The exception is if the placeholder filesystem is to encode the client identity ($root_fs/$client_identity) in a pull job.
  Those are created in `inherit` mode if the config field is `unspecified` so that users who don't need
  placeholders are not bothered by these details.

Future Work:
  Automatically warn existing users of encrypt-on-recv about the problem
  if they are affected.
  The problem that I hit during implementation of this is that the
  `encryption` prop's `source` doesn't quite behave like other props:
  `source` is `default` for `encryption=off` and `-` when `encryption=on`.
  Hence, we can't use `source` to distinguish the following 2x2 cases:
  (1) placeholder created with explicit -o encryption=off
  (2) placeholder created without specifying -o encryption
  with
  (A) an encrypted parent at creation time
  (B) an unencrypted parent at creation time

config/config.go

@@ -101,6 +101,8 @@ type RecvOptions struct {
 	Properties *PropertyRecvOptions `yaml:"properties,fromdefaults"`
 
 	BandwidthLimit *BandwidthLimit `yaml:"bandwidth_limit,optional,fromdefaults"`
+
+	Placeholder *PlaceholderRecvOptions `yaml:"placeholder,fromdefaults"`
 }
 
 var _ yaml.Unmarshaler = &datasizeunit.Bits{}
@@ -130,6 +132,10 @@ type PropertyRecvOptions struct {
 	Override map[zfsprop.Property]string `yaml:"override,optional"`
 }
 
+type PlaceholderRecvOptions struct {
+	Encryption string `yaml:"encryption,default=unspecified"`
+}
+
 type PushJob struct {
 	ActiveJob    `yaml:",inline"`
 	Snapshotting SnapshottingEnum  `yaml:"snapshotting"`

daemon/job/build_jobs_sendrecvoptions.go

@@ -72,6 +72,16 @@ func buildReceiverConfig(in ReceivingJobConfig, jobID endpoint.JobID) (rc endpoi
 		return rc, errors.Wrap(err, "cannot build bandwith limit config")
 	}
 
+	placeholderEncryption, err := endpoint.PlaceholderCreationEncryptionPropertyString(recvOpts.Placeholder.Encryption)
+	if err != nil {
+		options := []string{}
+		for _, v := range endpoint.PlaceholderCreationEncryptionPropertyValues() {
+			options = append(options, endpoint.PlaceholderCreationEncryptionProperty(v).String())
+		}
+		return rc, errors.Errorf("placeholder encryption value %q is invalid, must be one of %s",
+			recvOpts.Placeholder.Encryption, options)
+	}
+
 	rc = endpoint.ReceiverConfig{
 		JobID:                      jobID,
 		RootWithoutClientComponent: rootFs,
@@ -81,6 +91,8 @@ func buildReceiverConfig(in ReceivingJobConfig, jobID endpoint.JobID) (rc endpoi
 		OverrideProperties: recvOpts.Properties.Override,
 
 		BandwidthLimit: bwlim,
+
+		PlaceholderEncryption: placeholderEncryption,
 	}
 	if err := rc.Validate(); err != nil {
 		return rc, errors.Wrap(err, "cannot build receiver config")

docs/configuration/sendrecvoptions.rst

@@ -38,10 +38,10 @@ See the `upstream man page <https://openzfs.github.io/openzfs-docs/man/8/zfs-sen
       - Specific to zrepl, :ref:`see below <job-send-options-encrypted>`.
     * - ``bandwidth_limit``
       -
-      - Specific to zrepl, :ref:`see below <job-send-recv-options-bandwidth-limit>`.
+      - Specific to zrepl, :ref:`see below <job-send-recv-options--bandwidth-limit>`.
     * - ``raw``
       - ``-w``
-      - Use ``encrypted`` to only allow encrypted sends.
+      - Use ``encrypted`` to only allow encrypted sends. Mixed sends are not supported.
     * - ``send_properties``
       - ``-p``
       - **Be careful**, read the :ref:`note on property replication below <job-note-property-replication>`.
@@ -141,9 +141,16 @@ Recv Options
          override: {
            "org.openzfs.systemd:ignore": "on"
          }
-       bandwidth_limit: ... # see below
+       bandwidth_limit: ...
+       placeholder:
+         encryption: unspecified | off | inherit
      ...
 
+Jump to
+:ref:`properties <job-recv-options--inherit-and-override>` ,
+:ref:`bandwidth_limit <job-send-recv-options--bandwidth-limit>` , and
+:ref:`bandwidth_limit <job-recv-options--placeholder>`.
+
 .. _job-recv-options--inherit-and-override:
 
 ``properties``
@@ -217,10 +224,40 @@ and property replication is enabled, the receiver must :ref:`inherit the followi
 * ``keyformat``
 * ``encryption``
 
+.. _job-recv-options--placeholder:
+
+Placeholders
+------------
+
+During replication, zrepl :ref:`creates placeholder datasets <replication-placeholder-property>` on the receiving side if the sending side's ``filesystems`` filter creates gaps in the dataset hierarchy.
+This is generally fully transparent to the user.
+However, with OpenZFS Native Encryption, placeholders require zrepl user attention.
+Specifically, the problem is that, when zrepl attempts to create the placeholder dataset on the receiver, and that placeholder's parent dataset is encrypted, ZFS wants to inherit encryption to the placeholder.
+This is relevant to two use cases that zrepl supports:
+
+1. **encrypted-send-to-untrusted-receiver** In this use case, the sender sends an :ref:`encrypted send stream <job-send-options-encrypted>` and the receiver doesn't have the key loaded.
+2. **send-plain-encrypt-on-receive** The receive-side ``root_fs`` dataset is encrypted, and the senders are unencrypted.
+   The key of ``root_fs`` is loaded, and the goal is that the plain sends (e.g., from production) are encrypted on-the-fly during receive, with ``root_fs``'s key.
+
+For **encrypted-send-to-untrusted-receiver**, the placeholder datasets need to be created with ``-o encryption=off``.
+Without it, creation would fail with an error, indicating that the placeholder's parent dataset's key needs to be loaded.
+But we don't trust the receiver, so we can't expect that to ever happen.
+
+However, for **send-plain-encrypt-on-receive**, we cannot set ``-o encryption=off``.
+The reason is that if we did, any of the (non-placeholder) child datasets below the placeholder would inherit ``encryption=off``, thereby silently breaking our encrypt-on-receive use case.
+So, to cover this use case, we need to create placeholders without specifying ``-o encryption``.
+This will make ``zfs create`` inherit the encryption mode from the parent dataset, and thereby transitively from ``root_fs``.
+
+The zrepl config provides the `recv.placeholder.encryption` knob to control this behavior.
+In ``undefined`` mode (default), placeholder creation bails out and asks the user to configure a behavior.
+In ``off`` mode, the placeholder is created with ``encryption=off``, i.e., **encrypted-send-to-untrusted-rceiver** use case.
+In ``inherit`` mode, the placeholder is created without specifying ``-o encryption`` at all, i.e., the **send-plain-encrypt-on-receive** use case.
+
+
 Common Options
 ~~~~~~~~~~~~~~
 
-.. _job-send-recv-options-bandwidth-limit:
+.. _job-send-recv-options--bandwidth-limit:
 
 Bandwidth Limit (send & recv)
 -----------------------------

endpoint/endpoint.go

@@ -472,8 +472,20 @@ type ReceiverConfig struct {
 	OverrideProperties map[zfsprop.Property]string
 
 	BandwidthLimit bandwidthlimit.Config
+
+	PlaceholderEncryption PlaceholderCreationEncryptionProperty
 }
 
+//go:generate enumer -type=PlaceholderCreationEncryptionProperty -transform=kebab -trimprefix=PlaceholderCreationEncryptionProperty
+type PlaceholderCreationEncryptionProperty int
+
+// Note: the constant names, transformed through enumer, are part of the config format!
+const (
+	PlaceholderCreationEncryptionPropertyUnspecified PlaceholderCreationEncryptionProperty = 1 << iota
+	PlaceholderCreationEncryptionPropertyInherit
+	PlaceholderCreationEncryptionPropertyOff
+)
+
 func (c *ReceiverConfig) copyIn() {
 	c.RootWithoutClientComponent = c.RootWithoutClientComponent.Copy()
 
@@ -513,6 +525,10 @@ func (c *ReceiverConfig) Validate() error {
 		return errors.Wrap(err, "`BandwidthLimit` field invalid")
 	}
 
+	if !c.PlaceholderEncryption.IsAPlaceholderCreationEncryptionProperty() {
+		return errors.Errorf("`PlaceholderEncryption` field is invalid")
+	}
+
 	return nil
 }
 
@@ -525,6 +541,8 @@ type Receiver struct {
 	bwLimit bandwidthlimit.Wrapper
 
 	recvParentCreationMtx *chainlock.L
+
+	Test_OverrideClientIdentityFunc func() string // for use by platformtest
 }
 
 func NewReceiver(config ReceiverConfig) *Receiver {
@@ -562,9 +580,15 @@ func (s *Receiver) clientRootFromCtx(ctx context.Context) *zfs.DatasetPath {
 		return s.conf.RootWithoutClientComponent.Copy()
 	}
 
-	clientIdentity, ok := ctx.Value(ClientIdentityKey).(string)
+	var clientIdentity string
-	if !ok {
+	if s.Test_OverrideClientIdentityFunc != nil {
-		panic("ClientIdentityKey context value must be set")
+		clientIdentity = s.Test_OverrideClientIdentityFunc()
+	} else {
+		var ok bool
+		clientIdentity, ok = ctx.Value(ClientIdentityKey).(string) // no shadow
+		if !ok {
+			panic("ClientIdentityKey context value must be set")
+		}
 	}
 
 	clientRoot, err := clientRoot(s.conf.RootWithoutClientComponent, clientIdentity)
@@ -605,7 +629,8 @@ func (s *Receiver) ListFilesystems(ctx context.Context, req *pdu.ListFilesystemR
 	if rphs, err := zfs.ZFSGetFilesystemPlaceholderState(ctx, s.conf.RootWithoutClientComponent); err != nil {
 		return nil, errors.Wrap(err, "cannot determine whether root_fs exists")
 	} else if !rphs.FSExists {
-		return nil, errors.New("root_fs does not exist")
+		getLogger(ctx).WithField("root_fs", s.conf.RootWithoutClientComponent).Error("root_fs does not exist")
+		return nil, errors.Errorf("root_fs does not exist")
 	}
 
 	root := s.clientRootFromCtx(ctx)
@@ -714,6 +739,33 @@ func (s *Receiver) SendDry(ctx context.Context, r *pdu.SendReq) (*pdu.SendRes, e
 	return nil, fmt.Errorf("receiver does not implement SendDry()")
 }
 
+func (s *Receiver) receive_GetPlaceholderCreationEncryptionValue(client_root, path *zfs.DatasetPath) (zfs.FilesystemPlaceholderCreateEncryptionValue, error) {
+	if !s.conf.PlaceholderEncryption.IsAPlaceholderCreationEncryptionProperty() {
+		panic(s.conf.PlaceholderEncryption)
+	}
+
+	if client_root.Equal(path) && s.conf.PlaceholderEncryption == PlaceholderCreationEncryptionPropertyUnspecified {
+		// If our Receiver is configured to append a client component to s.conf.RootWithoutClientComponent
+		// then that dataset is always going to be a placeholder.
+		// We don't want to burden users with the concept of placeholders if their `filesystems` filter on the sender
+		// doesn't introduce any gaps.
+		// Since the dataset hierarchy up to and including that client component dataset is still fully controlled by us,
+		// using `inherit` is going to make it work in all expected use cases.
+		return zfs.FilesystemPlaceholderCreateEncryptionInherit, nil
+	}
+
+	switch s.conf.PlaceholderEncryption {
+	case PlaceholderCreationEncryptionPropertyUnspecified:
+		return 0, fmt.Errorf("placeholder filesystem encryption handling is unspecified in receiver config")
+	case PlaceholderCreationEncryptionPropertyInherit:
+		return zfs.FilesystemPlaceholderCreateEncryptionInherit, nil
+	case PlaceholderCreationEncryptionPropertyOff:
+		return zfs.FilesystemPlaceholderCreateEncryptionOff, nil
+	default:
+		panic(s.conf.PlaceholderEncryption)
+	}
+}
+
 func (s *Receiver) Receive(ctx context.Context, req *pdu.ReceiveReq, receive io.ReadCloser) (*pdu.ReceiveRes, error) {
 	defer trace.WithSpanFromStackUpdateCtx(&ctx)()
 
@@ -754,15 +806,18 @@ func (s *Receiver) Receive(ctx context.Context, req *pdu.ReceiveReq, receive io.
 			if v.Path.Equal(lp) {
 				return false
 			}
+
+			l := getLogger(ctx).
+				WithField("placeholder_fs", v.Path.ToString()).
+				WithField("receive_fs", lp.ToString())
+
 			ph, err := zfs.ZFSGetFilesystemPlaceholderState(ctx, v.Path)
-			getLogger(ctx).
+			l.WithField("placeholder_state", fmt.Sprintf("%#v", ph)).
-				WithField("fs", v.Path.ToString()).
-				WithField("placeholder_state", fmt.Sprintf("%#v", ph)).
 				WithField("err", fmt.Sprintf("%s", err)).
 				WithField("errType", fmt.Sprintf("%T", err)).
-				Debug("placeholder state for filesystem")
+				Debug("get placeholder state for filesystem")
 			if err != nil {
-				visitErr = err
+				visitErr = errors.Wrapf(err, "cannot get placeholder state of %s", v.Path.ToString())
 				return false
 			}
 
@@ -773,21 +828,34 @@ func (s *Receiver) Receive(ctx context.Context, req *pdu.ReceiveReq, receive io.
 					} else {
 						visitErr = fmt.Errorf("root_fs %q does not exist", s.conf.RootWithoutClientComponent.ToString())
 					}
-					getLogger(ctx).WithError(visitErr).Error("placeholders are only created automatically below root_fs")
+					l.WithError(visitErr).Error("placeholders are only created automatically below root_fs")
 					return false
 				}
-				l := getLogger(ctx).WithField("placeholder_fs", v.Path)
+
-				l.Debug("create placeholder filesystem")
+				// compute the value lazily so that users who don't rely on
-				err := zfs.ZFSCreatePlaceholderFilesystem(ctx, v.Path, v.Parent.Path)
+				// placeholders can use the default value PlaceholderCreationEncryptionPropertyUnspecified
+				placeholderEncryption, err := s.receive_GetPlaceholderCreationEncryptionValue(root, v.Path)
 				if err != nil {
-					l.WithError(err).Error("cannot create placeholder filesystem")
+					l.WithError(err).Error("cannot create placeholder filesystem") // logger already contains path
-					visitErr = err
+					visitErr = errors.Wrapf(err, "cannot create placeholder filesystem %s", v.Path.ToString())
 					return false
 				}
+
+				l := l.WithField("encryption", placeholderEncryption)
+
+				l.Debug("creating placeholder filesystem")
+				err = zfs.ZFSCreatePlaceholderFilesystem(ctx, v.Path, v.Parent.Path, placeholderEncryption)
+				if err != nil {
+					l.WithError(err).Error("cannot create placeholder filesystem") // logger already contains path
+					visitErr = errors.Wrapf(err, "cannot create placeholder filesystem %s", v.Path.ToString())
+					return false
+				}
+				l.Info("created placeholder filesystem")
 				return true
+			} else {
+				l.Debug("filesystem exists")
+				return true // leave this fs as is
 			}
-			getLogger(ctx).WithField("filesystem", v.Path.ToString()).Debug("exists")
-			return true // leave this fs as is
 		})
 	}()
 	getLogger(ctx).WithField("visitErr", visitErr).Debug("complete tree-walk")

endpoint/placeholdercreationencryptionproperty_enumer.go

@@ -0,0 +1,62 @@
+// Code generated by "enumer -type=PlaceholderCreationEncryptionProperty -transform=kebab -trimprefix=PlaceholderCreationEncryptionProperty"; DO NOT EDIT.
+
+//
+package endpoint
+
+import (
+	"fmt"
+)
+
+const (
+	_PlaceholderCreationEncryptionPropertyName_0 = "unspecifiedinherit"
+	_PlaceholderCreationEncryptionPropertyName_1 = "off"
+)
+
+var (
+	_PlaceholderCreationEncryptionPropertyIndex_0 = [...]uint8{0, 11, 18}
+	_PlaceholderCreationEncryptionPropertyIndex_1 = [...]uint8{0, 3}
+)
+
+func (i PlaceholderCreationEncryptionProperty) String() string {
+	switch {
+	case 1 <= i && i <= 2:
+		i -= 1
+		return _PlaceholderCreationEncryptionPropertyName_0[_PlaceholderCreationEncryptionPropertyIndex_0[i]:_PlaceholderCreationEncryptionPropertyIndex_0[i+1]]
+	case i == 4:
+		return _PlaceholderCreationEncryptionPropertyName_1
+	default:
+		return fmt.Sprintf("PlaceholderCreationEncryptionProperty(%d)", i)
+	}
+}
+
+var _PlaceholderCreationEncryptionPropertyValues = []PlaceholderCreationEncryptionProperty{1, 2, 4}
+
+var _PlaceholderCreationEncryptionPropertyNameToValueMap = map[string]PlaceholderCreationEncryptionProperty{
+	_PlaceholderCreationEncryptionPropertyName_0[0:11]:  1,
+	_PlaceholderCreationEncryptionPropertyName_0[11:18]: 2,
+	_PlaceholderCreationEncryptionPropertyName_1[0:3]:   4,
+}
+
+// PlaceholderCreationEncryptionPropertyString retrieves an enum value from the enum constants string name.
+// Throws an error if the param is not part of the enum.
+func PlaceholderCreationEncryptionPropertyString(s string) (PlaceholderCreationEncryptionProperty, error) {
+	if val, ok := _PlaceholderCreationEncryptionPropertyNameToValueMap[s]; ok {
+		return val, nil
+	}
+	return 0, fmt.Errorf("%s does not belong to PlaceholderCreationEncryptionProperty values", s)
+}
+
+// PlaceholderCreationEncryptionPropertyValues returns all values of the enum
+func PlaceholderCreationEncryptionPropertyValues() []PlaceholderCreationEncryptionProperty {
+	return _PlaceholderCreationEncryptionPropertyValues
+}
+
+// IsAPlaceholderCreationEncryptionProperty returns "true" if the value is listed in the enum definition. "false" otherwise
+func (i PlaceholderCreationEncryptionProperty) IsAPlaceholderCreationEncryptionProperty() bool {
+	for _, v := range _PlaceholderCreationEncryptionPropertyValues {
+		if i == v {
+			return true
+		}
+	}
+	return false
+}

platformtest/tests/generated_cases.go

@@ -24,6 +24,9 @@ var Cases = []Case{BatchDestroy,
 	ReplicationIsResumableFullSend__both_GuaranteeResumability,
 	ReplicationIsResumableFullSend__initial_GuaranteeIncrementalReplication_incremental_GuaranteeIncrementalReplication,
 	ReplicationIsResumableFullSend__initial_GuaranteeResumability_incremental_GuaranteeIncrementalReplication,
+	ReplicationPlaceholderEncryption__EncryptOnReceiverUseCase__WorksIfConfiguredWithInherit,
+	ReplicationPlaceholderEncryption__UnspecifiedIsOkForClientIdentityPlaceholder,
+	ReplicationPlaceholderEncryption__UnspecifiedLeadsToFailureAtRuntimeWhenCreatingPlaceholders,
 	ReplicationPropertyReplicationWorks,
 	ReplicationReceiverErrorWhileStillSending,
 	ReplicationStepCompletedLostBehavior__GuaranteeIncrementalReplication,

platformtest/tests/helpers.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/stretchr/testify/require"
 
+	"github.com/zrepl/zrepl/daemon/filters"
 	"github.com/zrepl/zrepl/platformtest"
 	"github.com/zrepl/zrepl/util/limitio"
 	"github.com/zrepl/zrepl/zfs"
@@ -174,3 +175,8 @@ func datasetToStringSortedTrimPrefix(prefix *zfs.DatasetPath, paths []*zfs.Datas
 	sort.Strings(pstrs)
 	return pstrs
 }
+
+func mustAddToSFilter(ctx *platformtest.Context, f *filters.DatasetMapFilter, fs string) {
+	err := f.Add(fs, "ok")
+	require.NoError(ctx, err)
+}

platformtest/tests/replication.go

@@ -79,6 +79,7 @@ func (i replicationInvocation) Do(ctx *platformtest.Context) *report.Report {
 		AppendClientIdentity:       false,
 		RootWithoutClientComponent: mustDatasetPath(i.rfsRoot),
 		BandwidthLimit:             bandwidthlimit.NoLimitConfig(),
+		PlaceholderEncryption:      endpoint.PlaceholderCreationEncryptionPropertyUnspecified,
 	}
 	if i.receiverConfigHook != nil {
 		i.receiverConfigHook(&receiverConfig)
@@ -903,13 +904,9 @@ func ReplicationFailingInitialParentProhibitsChildReplication(ctx *platformtest.
 	fsAA := ctx.RootDataset + "/sender/aa"
 
 	sfilter := filters.NewDatasetMapFilter(3, true)
-	mustAddToSFilter := func(fs string) {
+	mustAddToSFilter(ctx, sfilter, fsA)
-		err := sfilter.Add(fs, "ok")
+	mustAddToSFilter(ctx, sfilter, fsAChild)
-		require.NoError(ctx, err)
+	mustAddToSFilter(ctx, sfilter, fsAA)
-	}
-	mustAddToSFilter(fsA)
-	mustAddToSFilter(fsAChild)
-	mustAddToSFilter(fsAA)
 	rfsRoot := ctx.RootDataset + "/receiver"
 
 	mockRecvErr := fmt.Errorf("yifae4ohPhaquaes0hohghiep9oufie4roo7quoWooluaj2ee8")
@@ -965,6 +962,7 @@ func ReplicationPropertyReplicationWorks(ctx *platformtest.Context) {
 		+  "sender/a/child"
 		+  "sender/a/child@1"
 		+  "receiver"
+		R  zfs create -p "${ROOTDS}/receiver/${ROOTDS}/sender"
 	`)
 
 	sjid := endpoint.MustMakeJobID("sender-job")
@@ -974,12 +972,8 @@ func ReplicationPropertyReplicationWorks(ctx *platformtest.Context) {
 	fsAChild := ctx.RootDataset + "/sender/a/child"
 
 	sfilter := filters.NewDatasetMapFilter(2, true)
-	mustAddToSFilter := func(fs string) {
+	mustAddToSFilter(ctx, sfilter, fsA)
-		err := sfilter.Add(fs, "ok")
+	mustAddToSFilter(ctx, sfilter, fsAChild)
-		require.NoError(ctx, err)
-	}
-	mustAddToSFilter(fsA)
-	mustAddToSFilter(fsAChild)
 	rfsRoot := ctx.RootDataset + "/receiver"
 
 	type testPropExpectation struct {
@@ -1107,3 +1101,208 @@ func ReplicationPropertyReplicationWorks(ctx *platformtest.Context) {
 		}
 	}
 }
+
+func ReplicationPlaceholderEncryption__UnspecifiedLeadsToFailureAtRuntimeWhenCreatingPlaceholders(ctx *platformtest.Context) {
+
+	platformtest.Run(ctx, platformtest.PanicErr, ctx.RootDataset, `
+		CREATEROOT
+		+  "sender"
+		+  "sender/a"
+		+  "sender/a/child"
+		+  "receiver"
+		R  zfs snapshot -r ${ROOTDS}/sender@initial
+	`)
+
+	sjid := endpoint.MustMakeJobID("sender-job")
+	rjid := endpoint.MustMakeJobID("receiver-job")
+
+	childfs := ctx.RootDataset + "/sender/a/child"
+
+	sfilter := filters.NewDatasetMapFilter(3, true)
+
+	mustAddToSFilter(ctx, sfilter, childfs)
+	rfsRoot := ctx.RootDataset + "/receiver"
+
+	rep := replicationInvocation{
+		sjid:      sjid,
+		rjid:      rjid,
+		sfilter:   sfilter,
+		rfsRoot:   rfsRoot,
+		guarantee: pdu.ReplicationConfigProtectionWithKind(pdu.ReplicationGuaranteeKind_GuaranteeResumability),
+		receiverConfigHook: func(rc *endpoint.ReceiverConfig) {
+			rc.PlaceholderEncryption = endpoint.PlaceholderCreationEncryptionPropertyUnspecified
+		},
+	}
+
+	r := rep.Do(ctx)
+	ctx.Logf("\n%s", pretty.Sprint(r))
+
+	require.Len(ctx, r.Attempts, 1)
+	attempt := r.Attempts[0]
+	require.Nil(ctx, attempt.PlanError)
+	require.Len(ctx, attempt.Filesystems, 1)
+
+	afs := attempt.Filesystems[0]
+	require.Equal(ctx, childfs, afs.Info.Name)
+
+	require.Equal(ctx, 1, len(afs.Steps))
+	require.Equal(ctx, 0, afs.CurrentStep)
+
+	require.Equal(ctx, report.FilesystemSteppingErrored, afs.State)
+
+	childfsFirstComponent := strings.Split(childfs, "/")[0]
+	require.Contains(ctx, afs.StepError.Err, "cannot create placeholder filesystem "+rfsRoot+"/"+childfsFirstComponent+": placeholder filesystem encryption handling is unspecified in receiver config")
+}
+
+type ClientIdentityReceiver struct {
+	clientIdentity string
+	*endpoint.Receiver
+}
+
+func (r *ClientIdentityReceiver) Receive(ctx context.Context, req *pdu.ReceiveReq, stream io.ReadCloser) (*pdu.ReceiveRes, error) {
+	ctx = context.WithValue(ctx, endpoint.ClientIdentityKey, r.clientIdentity)
+	return r.Receiver.Receive(ctx, req, stream)
+}
+
+func ReplicationPlaceholderEncryption__UnspecifiedIsOkForClientIdentityPlaceholder(ctx *platformtest.Context) {
+	platformtest.Run(ctx, platformtest.PanicErr, ctx.RootDataset, `
+		CREATEROOT
+		+  "receiver"
+	`)
+
+	sjid := endpoint.MustMakeJobID("sender-job")
+	rjid := endpoint.MustMakeJobID("receiver-job")
+
+	sfilter := filters.NewDatasetMapFilter(1, true)
+
+	// hacky...
+	comps := strings.Split(ctx.RootDataset, "/")
+	require.GreaterOrEqual(ctx, len(comps), 2)
+	pool := comps[0]
+	require.Contains(ctx, pool, "zreplplatformtest", "don't want to cause accidents")
+	poolchild := pool + "/" + comps[1]
+
+	err := zfs.ZFSSnapshot(ctx, mustDatasetPath(pool), "testsnap", false)
+	require.NoError(ctx, err)
+
+	err = zfs.ZFSSnapshot(ctx, mustDatasetPath(poolchild), "testsnap", false)
+	require.NoError(ctx, err)
+
+	mustAddToSFilter(ctx, sfilter, pool)
+	mustAddToSFilter(ctx, sfilter, poolchild)
+
+	clientIdentity := "testclientid"
+
+	rfsRoot := ctx.RootDataset + "/receiver"
+
+	rep := replicationInvocation{
+		sjid:      sjid,
+		rjid:      rjid,
+		sfilter:   sfilter,
+		rfsRoot:   rfsRoot,
+		guarantee: pdu.ReplicationConfigProtectionWithKind(pdu.ReplicationGuaranteeKind_GuaranteeResumability),
+		receiverConfigHook: func(rc *endpoint.ReceiverConfig) {
+			rc.PlaceholderEncryption = endpoint.PlaceholderCreationEncryptionPropertyUnspecified
+			rc.AppendClientIdentity = true
+		},
+		interceptReceiver: func(r *endpoint.Receiver) logic.Receiver {
+			r.Test_OverrideClientIdentityFunc = func() string { return clientIdentity }
+			return r
+		},
+	}
+
+	r := rep.Do(ctx)
+	ctx.Logf("\n%s", pretty.Sprint(r))
+
+	require.Len(ctx, r.Attempts, 1)
+	attempt := r.Attempts[0]
+	require.Nil(ctx, attempt.PlanError)
+	require.Len(ctx, attempt.Filesystems, 2)
+
+	filesystemsByName := make(map[string]*report.FilesystemReport)
+	for _, fs := range attempt.Filesystems {
+		filesystemsByName[fs.Info.Name] = fs
+	}
+	require.Len(ctx, filesystemsByName, len(attempt.Filesystems))
+
+	afs, ok := filesystemsByName[pool]
+	require.True(ctx, ok)
+	require.Nil(ctx, afs.PlanError)
+	require.Nil(ctx, afs.StepError)
+	require.Equal(ctx, report.FilesystemDone, afs.State)
+
+	afs, ok = filesystemsByName[poolchild]
+	require.True(ctx, ok)
+	require.Nil(ctx, afs.PlanError)
+	require.Nil(ctx, afs.StepError)
+	require.Equal(ctx, report.FilesystemDone, afs.State)
+
+	mustGetFilesystemVersion(ctx, rfsRoot+"/"+clientIdentity+"/"+pool+"@testsnap")
+	mustGetFilesystemVersion(ctx, rfsRoot+"/"+clientIdentity+"/"+poolchild+"@testsnap")
+
+}
+
+func replicationPlaceholderEncryption__EncryptOnReceiverUseCase__impl(ctx *platformtest.Context, placeholderEncryption endpoint.PlaceholderCreationEncryptionProperty) {
+
+	platformtest.Run(ctx, platformtest.PanicErr, ctx.RootDataset, `
+		CREATEROOT
+		+  "sender"
+		+  "sender/a"
+		+  "sender/a/child"
+		+  "receiver" encrypted
+		R  zfs snapshot -r ${ROOTDS}/sender@initial
+	`)
+
+	sjid := endpoint.MustMakeJobID("sender-job")
+	rjid := endpoint.MustMakeJobID("receiver-job")
+
+	childfs := ctx.RootDataset + "/sender/a/child"
+
+	sfilter := filters.NewDatasetMapFilter(3, true)
+
+	mustAddToSFilter(ctx, sfilter, childfs)
+	rfsRoot := ctx.RootDataset + "/receiver"
+
+	rep := replicationInvocation{
+		sjid:      sjid,
+		rjid:      rjid,
+		sfilter:   sfilter,
+		rfsRoot:   rfsRoot,
+		guarantee: pdu.ReplicationConfigProtectionWithKind(pdu.ReplicationGuaranteeKind_GuaranteeResumability),
+		receiverConfigHook: func(rc *endpoint.ReceiverConfig) {
+			rc.PlaceholderEncryption = placeholderEncryption
+			rc.AppendClientIdentity = false
+		},
+	}
+
+	r := rep.Do(ctx)
+	ctx.Logf("\n%s", pretty.Sprint(r))
+
+	require.Len(ctx, r.Attempts, 1)
+	attempt := r.Attempts[0]
+	require.Equal(ctx, report.AttemptDone, attempt.State)
+	require.Len(ctx, attempt.Filesystems, 1)
+	afs := attempt.Filesystems[0]
+	require.Equal(ctx, childfs, afs.Info.Name)
+
+	require.Equal(ctx, 1, len(afs.Steps))
+
+	rfs := mustDatasetPath(rfsRoot + "/" + childfs)
+	mustGetFilesystemVersion(ctx, rfs.ToString()+"@initial")
+}
+
+func ReplicationPlaceholderEncryption__EncryptOnReceiverUseCase__WorksIfConfiguredWithInherit(ctx *platformtest.Context) {
+	placeholderEncryption := endpoint.PlaceholderCreationEncryptionPropertyInherit
+
+	replicationPlaceholderEncryption__EncryptOnReceiverUseCase__impl(ctx, placeholderEncryption)
+	childfs := ctx.RootDataset + "/sender/a/child"
+	rfsRoot := ctx.RootDataset + "/receiver"
+	rfs := mustDatasetPath(rfsRoot + "/" + childfs)
+
+	// The leaf child dataset should be inhering from rfsRoot.
+	// If we had replicated with PlaceholderCreationEncryptionPropertyOff
+	// then it would be unencrypted and inherit from the placeholder.
+	props, err := zfs.ZFSGet(ctx, rfs, []string{"encryptionroot"})
+	require.NoError(ctx, err)
+	require.Equal(ctx, rfsRoot, props.Get("encryptionroot"))
+}

zfs/filesystemplaceholdercreateencryptionvalue_enumer.go

@@ -0,0 +1,51 @@
+// Code generated by "enumer -type=FilesystemPlaceholderCreateEncryptionValue -trimprefix=FilesystemPlaceholderCreateEncryption"; DO NOT EDIT.
+
+//
+package zfs
+
+import (
+	"fmt"
+)
+
+const _FilesystemPlaceholderCreateEncryptionValueName = "InheritOff"
+
+var _FilesystemPlaceholderCreateEncryptionValueIndex = [...]uint8{0, 7, 10}
+
+func (i FilesystemPlaceholderCreateEncryptionValue) String() string {
+	i -= 1
+	if i < 0 || i >= FilesystemPlaceholderCreateEncryptionValue(len(_FilesystemPlaceholderCreateEncryptionValueIndex)-1) {
+		return fmt.Sprintf("FilesystemPlaceholderCreateEncryptionValue(%d)", i+1)
+	}
+	return _FilesystemPlaceholderCreateEncryptionValueName[_FilesystemPlaceholderCreateEncryptionValueIndex[i]:_FilesystemPlaceholderCreateEncryptionValueIndex[i+1]]
+}
+
+var _FilesystemPlaceholderCreateEncryptionValueValues = []FilesystemPlaceholderCreateEncryptionValue{1, 2}
+
+var _FilesystemPlaceholderCreateEncryptionValueNameToValueMap = map[string]FilesystemPlaceholderCreateEncryptionValue{
+	_FilesystemPlaceholderCreateEncryptionValueName[0:7]:  1,
+	_FilesystemPlaceholderCreateEncryptionValueName[7:10]: 2,
+}
+
+// FilesystemPlaceholderCreateEncryptionValueString retrieves an enum value from the enum constants string name.
+// Throws an error if the param is not part of the enum.
+func FilesystemPlaceholderCreateEncryptionValueString(s string) (FilesystemPlaceholderCreateEncryptionValue, error) {
+	if val, ok := _FilesystemPlaceholderCreateEncryptionValueNameToValueMap[s]; ok {
+		return val, nil
+	}
+	return 0, fmt.Errorf("%s does not belong to FilesystemPlaceholderCreateEncryptionValue values", s)
+}
+
+// FilesystemPlaceholderCreateEncryptionValueValues returns all values of the enum
+func FilesystemPlaceholderCreateEncryptionValueValues() []FilesystemPlaceholderCreateEncryptionValue {
+	return _FilesystemPlaceholderCreateEncryptionValueValues
+}
+
+// IsAFilesystemPlaceholderCreateEncryptionValue returns "true" if the value is listed in the enum definition. "false" otherwise
+func (i FilesystemPlaceholderCreateEncryptionValue) IsAFilesystemPlaceholderCreateEncryptionValue() bool {
+	for _, v := range _FilesystemPlaceholderCreateEncryptionValueValues {
+		if i == v {
+			return true
+		}
+	}
+	return false
+}

zfs/placeholder.go

@@ -80,7 +80,15 @@ func ZFSGetFilesystemPlaceholderState(ctx context.Context, p *DatasetPath) (stat
 	return state, nil
 }
 
-func ZFSCreatePlaceholderFilesystem(ctx context.Context, fs *DatasetPath, parent *DatasetPath) (err error) {
+//go:generate enumer -type=FilesystemPlaceholderCreateEncryptionValue -trimprefix=FilesystemPlaceholderCreateEncryption
+type FilesystemPlaceholderCreateEncryptionValue int
+
+const (
+	FilesystemPlaceholderCreateEncryptionInherit FilesystemPlaceholderCreateEncryptionValue = 1 << iota
+	FilesystemPlaceholderCreateEncryptionOff
+)
+
+func ZFSCreatePlaceholderFilesystem(ctx context.Context, fs *DatasetPath, parent *DatasetPath, encryption FilesystemPlaceholderCreateEncryptionValue) (err error) {
 	if fs.Length() == 1 {
 		return fmt.Errorf("cannot create %q: pools cannot be created with zfs create", fs.ToString())
 	}
@@ -90,11 +98,19 @@ func ZFSCreatePlaceholderFilesystem(ctx context.Context, fs *DatasetPath, parent
 		"-o", fmt.Sprintf("%s=%s", PlaceholderPropertyName, placeholderPropertyOn),
 		"-o", "mountpoint=none",
 	}
-	if parentEncrypted, err := ZFSGetEncryptionEnabled(ctx, parent.ToString()); err != nil {
+
-		return errors.Wrap(err, "cannot determine encryption support")
+	if !encryption.IsAFilesystemPlaceholderCreateEncryptionValue() {
-	} else if parentEncrypted {
+		panic(encryption)
-		cmdline = append(cmdline, "-o", "encryption=off")
 	}
+	switch encryption {
+	case FilesystemPlaceholderCreateEncryptionInherit:
+		// no-op
+	case FilesystemPlaceholderCreateEncryptionOff:
+		cmdline = append(cmdline, "-o", "encryption=off")
+	default:
+		panic(encryption)
+	}
+
 	cmdline = append(cmdline, fs.ToString())
 	cmd := zfscmd.CommandContext(ctx, ZFS_BINARY, cmdline...)
 
@@ -148,3 +164,34 @@ func ZFSMigrateHashBasedPlaceholderToCurrent(ctx context.Context, fs *DatasetPat
 	}
 	return &report, nil
 }
+
+func ZFSListPlaceholderFilesystemsWithAdditionalProps(ctx context.Context, root string, additionalProps []string) (map[string]*ZFSProperties, error) {
+
+	props := []string{PlaceholderPropertyName}
+	if len(additionalProps) > 0 {
+		props = append(props, additionalProps...)
+	}
+
+	propsByFS, err := zfsGetRecursive(ctx, root, -1, []string{"filesystem", "volume"}, props, SourceAny)
+	if err != nil {
+		return nil, errors.Wrapf(err, "cannot get placeholder filesystems under %q", root)
+	}
+
+	filtered := make(map[string]*ZFSProperties)
+	for fs, props := range propsByFS {
+		details := props.GetDetails(PlaceholderPropertyName)
+		if details.Source != SourceLocal {
+			continue
+		}
+		fsp, err := NewDatasetPath(fs)
+		if err != nil {
+			return nil, errors.Wrapf(err, "zfs get returned invalid dataset path %q", fs)
+		}
+		if !isLocalPlaceholderPropertyValuePlaceholder(fsp, details.Value) {
+			continue
+		}
+		filtered[fs] = props
+	}
+
+	return filtered, nil
+}

zfs/zfs.go

@@ -1567,8 +1567,18 @@ func (s PropertySource) zfsGetSourceFieldPrefixes() []string {
 	return prefixes
 }
 
-func zfsGet(ctx context.Context, path string, props []string, allowedSources PropertySource) (*ZFSProperties, error) {
+func zfsGetRecursive(ctx context.Context, path string, depth int, dstypes []string, props []string, allowedSources PropertySource) (map[string]*ZFSProperties, error) {
-	args := []string{"get", "-Hp", "-o", "property,value,source", strings.Join(props, ","), path}
+	args := []string{"get", "-Hp", "-o", "name,property,value,source"}
+	if depth != 0 {
+		args = append(args, "-r")
+		if depth != -1 {
+			args = append(args, "-d", fmt.Sprintf("%d", depth))
+		}
+	}
+	if len(dstypes) > 0 {
+		args = append(args, "-t", strings.Join(dstypes, ","))
+	}
+	args = append(args, strings.Join(props, ","), path)
 	cmd := zfscmd.CommandContext(ctx, ZFS_BINARY, args...)
 	stdout, err := cmd.Output()
 	if err != nil {
@@ -1588,36 +1598,67 @@ func zfsGet(ctx context.Context, path string, props []string, allowedSources Pro
 	}
 	o := string(stdout)
 	lines := strings.Split(o, "\n")
-	if len(lines) < 1 || // account for newlines
+	propsByFS := make(map[string]*ZFSProperties)
-		len(lines)-1 != len(props) {
-		return nil, fmt.Errorf("zfs get did not return the number of expected property values")
-	}
-	res := &ZFSProperties{
-		make(map[string]PropertyValue, len(lines)),
-	}
 	allowedPrefixes := allowedSources.zfsGetSourceFieldPrefixes()
-	for _, line := range lines[:len(lines)-1] {
+	for _, line := range lines[:len(lines)-1] { // last line is an empty line due to how strings.Split works
 		fields := strings.FieldsFunc(line, func(r rune) bool {
 			return r == '\t'
 		})
-		if len(fields) != 3 {
+		if len(fields) != 4 {
-			return nil, fmt.Errorf("zfs get did not return property,value,source tuples")
+			return nil, fmt.Errorf("zfs get did not return name,property,value,source tuples")
 		}
 		for _, p := range allowedPrefixes {
 			// prefix-match so that SourceAny (= "") works
-			if strings.HasPrefix(fields[2], p) {
+			if strings.HasPrefix(fields[3], p) {
-				source, err := parsePropertySource(fields[2])
+				source, err := parsePropertySource(fields[3])
 				if err != nil {
 					return nil, errors.Wrap(err, "parse property source")
 				}
-				res.m[fields[0]] = PropertyValue{
+				fsProps, ok := propsByFS[fields[0]]
-					Value:  fields[1],
+				if !ok {
+					fsProps = &ZFSProperties{
+						make(map[string]PropertyValue),
+					}
+				}
+				if _, ok := fsProps.m[fields[1]]; ok {
+					return nil, errors.Errorf("duplicate property %q for dataset %q", fields[1], fields[0])
+				}
+				fsProps.m[fields[1]] = PropertyValue{
+					Value:  fields[2],
 					Source: source,
 				}
+				propsByFS[fields[0]] = fsProps
 				break
 			}
 		}
 	}
+	// validate we got expected output
+	for fs, fsProps := range propsByFS {
+		if len(fsProps.m) != len(props) {
+			return nil, errors.Errorf("zfs get did not return all requested values for dataset %q\noutput was:\n%s", fs, o)
+		}
+	}
+	return propsByFS, nil
+}
+
+func zfsGet(ctx context.Context, path string, props []string, allowedSources PropertySource) (*ZFSProperties, error) {
+	propMap, err := zfsGetRecursive(ctx, path, 0, nil, props, allowedSources)
+	if err != nil {
+		return nil, err
+	}
+	if len(propMap) == 0 {
+		// XXX callers expect to always get a result here
+		// They will observe props.Get("propname") == ""
+		// We should change .Get to return a tuple, or an error, or whatever.
+		return &ZFSProperties{make(map[string]PropertyValue)}, nil
+	}
+	if len(propMap) != 1 {
+		return nil, errors.Errorf("zfs get unexpectedly returned properties for multiple datasets")
+	}
+	res, ok := propMap[path]
+	if !ok {
+		return nil, errors.Errorf("zfs get returned properties for a different dataset that requested")
+	}
 	return res, nil
 }