Encrypting the hash will be useless overhead for the most/intended usecases, as a receive-only folder will just ignore this hash. I'd rather handle the case of no blockshash such that the deficiency is on the unintended usecase. That should already work when just forwarding the nil blockshash in the encryptedModel, the model itself looks like it can already deal with no blockhash.
Does this make sense?

It does, and I'm not really sure why we do what we do here to be honest. There's also a thing about passing the original block offset as additional data in the encrypted hash which ... I'm not sure why we do. :D

So disregarding the thing about the offset that I haven't investigated why we do it, there's some (necessary, I think) asymmetry in how the requests are handled.

When a trusted device requests data from an untrusted device, they just pass the relevant offset/size and maybe fictitious hash, and the untrusted device reads the data and can do nothing to verify it. The trusted side verifies the data by way of decrypting it, but not (as far as I can see) that it matches the expected plaintext hash...

When an untrusted device requests data from a trusted device it gets the encrypted hash as part of the request, decrypts that hash and uses it to verify the data that is being read from disk. (Same as when we serve a normal request from a trusted peer.) It then obviously does no further verification of the received data.

I think it has some value to retain the second part, as the hash check when reading from disk is how we catch certain database-out-of-sync-with-disk conditions.

Agreed we should retain that, in untrusted to trusted conns. I think we should just drop it in the case of trusted to trusted but both en-/decrypting sent data, instead of alwasy sending an encrypted hash from a trusted to a untrusted device - which in most cases will be useless. So have a condition in encryptedModel.Request that it detects a missing hash and just handles it without a hash, instead of currently erroring as it can't decrypt it, and keep decrypting if there is something to decrypt.

Didn't remember the offset bit either, found this explanation:

		// The offset goes into the encrypted block hash as additional data,
		// essentially mixing in with the nonce. This means a block hash
		// remains stable for the same data at the same offset, but doesn't
		// reveal the existence of identical data blocks at other offsets.

Thanks for digging up the bit about the offset. I disagree about the uselessness however. We do a hash check of the data read from disk for requests from normal devices, and we do it for requests from untrusted devices, why not do it in this case? I think it's equally use{ful,less} as in the regular device case (except for the fact that the "both devices runs encryption" use case is dubious to begin with, but somewhat plausible corner cases have been produced for it...)

Am I missing something? Afaik it's useful for the case of trusted to trusted with encryption, but it doesn't do anything for trusted to untrusted: The trusted side encrypts the hash on request, the untrusted side ignores the hash (as it's not tied to the data content). So we either send the encrypted hash for the validation in the trusted to trusted with encryption case, doing useless encryption computations in the trusted to untrusted case. Or we don't get validation and no computation cost respectively. Given trusted to trusted with encryption is not recommended to do, I'd rather go with no computation cost.

Not sure if you're missing something, or I am, or we just disagree. :) To be sure we have the same world view I made a diagram of mine. This shows what happens with a response (assume a request just went in the other direction) -- whether the response data is verified when read from disk, verified before being written to disk, and where it's en/de-crypted.

We're talking about the to-be-or-not-to-be of the yellow highlighted verification in the bottom left, right? I think it nicely mirrors the one in the normal case at the top left, and provides "drift detection" of the data on disk which we think is valuable in the normal case.

(In all cases the verification-on-read on the left side is against the hash we got in the request, so we need to encrypt that hash into the request in order to be able to do the verification...)

Yep, ok we are in sync on the understanding :)

I'd just rather get rid of the overhead of encrypting that hash when it gets discarded in all expected use-cases (sent to untrusted), and in reality likely in most cases, and at the same time lose verification in the yellow part in the unexpected use-case. At the same time it's not like it matters very much either way IMO.