Re: [PATCH v3 0/3] Cryptomount detached headers

[Daniel Kiper <dkiper@net-space.pl>]

On Wed, Jun 08, 2022 at 10:34:01AM -0500, Glenn Washburn wrote:
> Updates since v2:
>  * Address uneeded ret variable pointed out by Patrick
>  * Rebased onto latest master with keyfile and security changes. I don't think
>    this actually changed these patches though.
>
> Conceptually the approach is different than the previous detach header
> series because this one uses the disk objects read hook to hook reads to
> the disk in scan() and recover_key() such that if there is an associated
> header file, the hook will cause the read to return data from the header
> file instead of from the source disk.
>
> For this the read hook implementation needed to be upgaded because prior
> it didn't get the read buffer sent from the read caller and so could not
> modify its contents. Patch #1 updates the hook accordingly and all instances
> of its use, but doesn't functionally change how GRUB operates.
>
> The second patch adds the header option to cryptomount and the read hook
> to the source disk during the scan() and recover_key() stages.
> The benefit of this approach is its simpler/less code and does not require
> the modification of backends, except to potentially cause a failure in
> scan() if the backend doesn't support the current implementation of detached
> headers, as with the GELI backend. This implementation requires that the
> crypto volume header reside at the beginning of the volume. GELI has its
> header at the end, which is why it can not currently be supported. In
> theory, GELI could be supported if extra assumptions about its source
> access pattern during scan() and recovery_key() were made. I don't use GELI,
> no one seems to be asking for GELI detached headers, and I don't think that
> GELI even support detached headers with the official tools. So for me, not
> supporting crypto volumes with headers at the end of the disk is not a big
> deal. With the patch series each backend gets the header file through cargs,
> so it can implement detached headers by solely operating on that file instead
> of the hooked source disk. In the the future, a flag can be added to the
> cryptodisk_dev_t that backends can sent when registering to enabled/disable
> the use of the read hook, if the backend needs to read from both the detached
> header file and the source disk.
>
> Glenn
>
> Glenn Washburn (3):
>   disk: Allow read hook callback to take read buffer to potentially
>     modify it
>   cryptodisk: Add support for using detached header files
>   docs: Add documentation on detached header option to cryptomount

For all the patches Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>...

Big thank you for all working on this!

Daniel