On Fri, 21 Jun 2024 at 18:39, Eric Biggers <ebiggers@xxxxxxxxxx> wrote: > > On Fri, Jun 21, 2024 at 06:16:37PM +0300, Dmitry Baryshkov wrote: > > On Fri, 21 Jun 2024 at 07:47, Eric Biggers <ebiggers@xxxxxxxxxx> wrote: > > > > > > On Thu, Jun 20, 2024 at 02:57:40PM +0300, Dmitry Baryshkov wrote: > > > > > > > > > > > > > > > Is it possible to use both kind of keys when working on standard mode? > > > > > > > > If not, it should be the user who selects what type of keys to be used. > > > > > > > > Enforcing this via DT is not a way to go. > > > > > > > > > > > > > > > > > > > > > > Unfortunately, that support is not there yet. When you say user, do > > > > > > > you mean to have it as a filesystem mount option? > > > > > > > > > > > > During cryptsetup time. When running e.g. cryptsetup I, as a user, would like > > > > > > to be able to use either a hardware-wrapped key or a standard key. > > > > > > > > > > > > > > > > What we are looking for with these patches is for per-file/folder encryption using fscrypt policies. > > > > > Cryptsetup to my understanding supports only full-disk , and does not support FBE (File-Based) > > > > > > > > I must admit, I mostly used dm-crypt beforehand, so I had to look at > > > > fscrypt now. Some of my previous comments might not be fully > > > > applicable. > > > > > > > > > Hence the idea here is that we mount an unencrypted device (with the inlinecrypt option that indicates inline encryption is supported) > > > > > And specify policies (links to keys) for different folders. > > > > > > > > > > > > The way the UFS/EMMC crypto layer is designed currently is that, this > > > > > > > information is needed when the modules are loaded. > > > > > > > > > > > > > > https://lore.kernel.org/all/20231104211259.17448-2-ebiggers@xxxxxxxxxx > > > > > > > /#Z31drivers:ufs:core:ufshcd-crypto.c > > > > > > > > > > > > I see that the driver lists capabilities here. E.g. that it supports HW-wrapped > > > > > > keys. But the line doesn't specify that standard keys are not supported. > > > > > > > > > > > > > > > > Those are capabilities that are read from the storage controller. However, wrapped keys > > > > > Are not a standard in the ICE JEDEC specification, and in most cases, is a value add coming > > > > > from the SoC. > > > > > > > > > > QCOM SOC and firmware currently does not support both kinds of keys in the HWKM mode. > > > > > That is something we are internally working on, but not available yet. > > > > > > > > I'd say this is a significant obstacle, at least from my point of > > > > view. I understand that the default might be to use hw-wrapped keys, > > > > but it should be possible for the user to select non-HW keys if the > > > > ability to recover the data is considered to be important. Note, I'm > > > > really pointing to the user here, not to the system integrator. So > > > > using DT property or specifying kernel arguments to switch between > > > > these modes is not really an option. > > > > > > > > But I'd really love to hear some feedback from linux-security and/or > > > > linux-fscrypt here. > > > > > > > > In my humble opinion the user should be able to specify that the key > > > > is wrapped using the hardware KMK. Then if the hardware has already > > > > started using the other kind of keys, it should be able to respond > > > > with -EINVAL / whatever else. Then the user can evict previously > > > > programmed key and program a desired one. > > > > > > > > > > Also, I'd have expected that hw-wrapped keys are handled using trusted > > > > > > keys mechanism (see security/keys/trusted-keys/). Could you please point > > > > > > out why that's not the case? > > > > > > > > > > > > > > > > I will evaluate this. > > > > > But my initial response is that we currently cannot communicate to our TPM directly from HLOS, but > > > > > goes through QTEE, and I don't think our qtee currently interfaces with the open source tee > > > > > driver. The interface is through QCOM SCM driver. > > > > > > > > Note, this is just an API interface, see how it is implemented for the > > > > CAAM hardware. > > > > > > > > > > The problem is that this patchset was sent out without the patches that add the > > > block and filesystem-level framework for hardware-wrapped inline encryption > > > keys, which it depends on. So it's lacking context. The proposed framework can > > > be found at > > > https://lore.kernel.org/linux-block/20231104211259.17448-1-ebiggers@xxxxxxxxxx/T/#u > > > > Thank you. I have quickly skimmed through the patches, but I didn't > > review them thoroughly. Maybe the patchset already implements the > > interfaces that I'm thinking about. In such a case please excuse me. I > > will give it a more thorough look later today. > > > > > As for why "trusted keys" aren't used, they just aren't helpful here. "Trusted > > > keys" are based around a model where the kernel can request that keys be sealed > > > and unsealed using a trust source, and the kernel gets access to the raw > > > unsealed keys. Hardware-wrapped inline encryption keys use a different model > > > where the kernel never gets access to the raw keys. They also have the concept > > > of ephemeral wrapping which does not exist in "trusted keys". And they need to > > > be properly integrated with the inline encryption framework in the block layer. > > > > Then what exactly does qcom_scm_derive_sw_secret() do? Does it rewrap > > the key under some other key? > > It derives a secret for functionality such as filenames encryption that can't > use inline encryption. > > > I had the feeling that there are two separate pieces of functionality > > being stuffed into a single patchset and into a single solution. > > > > First one is handling the keys. I keep on thinking that there should > > be a separate software interface to unseal the key and rewrap it under > > an ephemeral key. > > There is. That's what the BLKCRYPTOPREPAREKEY ioctl is for. > > > Some hardware might permit importing raw keys. > > That's what BLKCRYPTOIMPORTKEY is for. > > > Other hardware might insist on generating the keys on-chip so that raw keys > > can never be used. > > And that's what BLKCRYPTOGENERATEKEY is for. Again, this might be answered somewhere, but why can't we use keyctl for handling the keys and then use a single IOCTL to point the block device to the key in the keyring? > > > Second part is the actual block interface. Gaurav wrote about > > targeting fscrypt, but there should be no actual difference between > > crypto targets. FDE or having a single partition encrypted should > > probably work in the same way. Convert the key into blk_crypto_key > > (including the cookie for the ephemeral key), program the key into the > > slot, use the slot to en/decrypt hardware blocks. > > > > My main point is that the decision on the key type should be coming > > from the user. > > That's exactly how it works. There is a block interface for specifying an > inline encryption key along with each bio. The submitter of the bio can specify > either a standard key or a HW-wrapped key. Not in this patchset. The ICE driver decides whether it can support HW-wrapped keys or not and then fails to support other type of keys. > > Again, take a look at the patchset > https://lore.kernel.org/linux-block/20231104211259.17448-1-ebiggers@xxxxxxxxxx/T/#u. > That's where all this stuff is. I was mostly looking at the hardware-specific implementation. -- With best wishes Dmitry
