Re: [PATCH] ecryptfs: add mount option for specifying cipher driver.

[Tyler Hicks <code@tyhicks.com>]

On 2020-02-18 17:42:18, Brian Kubisiak wrote:
> Hi Tyler,
> 
> > Can you elaborate some on the use case you have?
> 
> On many modern embedded SoCs, there are multiple implementations of the same
> crypto algorithm---usually a CPU-based implementation using ISA extensions and a
> "security engine" implementation that implements crypto primitives on dedicated
> silicon. There are a few tradeoffs involved (performance, CPU overhead,
> resistance to side-channels attacks, etc), so it is not always clear which
> implementation is best.
> 
> An SoC that I've been working on has both the CPU implementation and the
> security engine implementation of the cbc(aes) algorithm at the same priority,
> so the one picked to perform the encryption for a given ecryptfs mount is
> somewhat random.

Have you looked into the possibility of increasing the priority of the
implementation that you prefer on your SoC?

If that's not feasible, have you considered blacklisting the module
providing the implementation that you don't prefer?

> My intention with this patch is to be able to select which
> implementation gets used for the mount using the
> ecryptfs_cipher_driver option instead of having the kernel pick.

I don't think allowing users to specify a cipher driver is a good idea.
eCryptfs has always assumed that the crypto subsystem knows best about
the ideal implementation of "cbc(aes)" and I believe that this is how
the crypto subsystem expects eCryptfs to make use of their API.

In addition to the design objection above, I'm worried about users
shooting themselves in the foot with this mount option. For example,
"ecryptfs_cipher_driver=ecb_aes_aesni" and
"ecryptfs_cipher_driver=xts_aes_aesni" are accepted. eCryptfs is only
implemented to operated in a (modified) CBC mode and letting users force
their way into using anything else is dangerous/insecure.

Lets see if we can address your problem in the crypto subsystem (or with
the module blacklist) rather than creating this amount of flexibility in
eCryptfs.

Tyler

> The fact that this would also effectively override the ecryptfs_cipher option is
> an unintended side effect, since the crypto_alloc_skcipher() function will take
> either the algorithm name or the driver name. Maybe there is a better way to do
> this?
> 
> Thanks,
> Brian