On Sat, Sep 21, 2024 at 12:49:39PM GMT, Eric Biggers wrote: > Hi Dmitry, > > On Fri, Sep 13, 2024 at 03:21:07PM +0300, Dmitry Baryshkov wrote: > > > > > > > > Once ICE has moved to a HWKM mode, the firmware key programming > > > > > > > currently does not support raw keys. > > > > > > > > This support is being added for the next Qualcomm chipset in Trustzone to > > > > > > > support both at he same time, but that will take another year or two to hit > > > > > > > the market. > > > > > > > > Until that time, due to TZ (firmware) limitations , the driver can only > > > > > > > support one or the other. > > > > > > > > > > > > > > > > We also cannot keep moving ICE modes, due to the HWKM enablement > > > > > > > being a one-time configurable value at boot. > > > > > > > > > > > > > > So the init of HWKM should be delayed until the point where the user tells if > > > > > > > HWKM or raw keys should be used. > > > > > > > > > > > > Ack. > > > > > > I'll work with Bartosz to look into moving to HWKM mode only during the first key program request > > > > > > > > > > > > > > > > That would mean the driver would have to initially advertise support for both > > > > > HW-wrapped keys and raw keys, and then it would revoke the support for one of > > > > > them later (due to the other one being used). However, runtime revocation of > > > > > crypto capabilities is not supported by the blk-crypto framework > > > > > (Documentation/block/inline-encryption.rst), and there is no clear path to > > > > > adding such support. Upper layers may have already checked the crypto > > > > > capabilities and decided to use them. It's too late to find out that the > > > > > support was revoked in the middle of an I/O request. Upper layer code > > > > > (blk-crypto, fscrypt, etc.) is not prepared for this. And even if it was, the > > > > > best it could do is cleanly fail the I/O, which is too late as e.g. it may > > > > > happen during background writeback and cause user data to be thrown away. > > > > > > > > Can we check crypto capabilities when the user sets the key? > > > > > > I think you mean when a key is programmed into a keyslot? That happens during > > > I/O, which is too late as I've explained above. > > > > > > > Compare this to the actual HSM used to secure communication or > > > > storage. It has certain capabilities, which can be enumerated, etc. > > > > But then at the time the user sets the key it is perfectly normal to > > > > return an error because HSM is out of resources. It might even have > > > > spare key slots, but it might be not enough to be able to program the > > > > required key (as a really crazy example, consider the HSM having at > > > > this time a single spare DES key slot, while the user wants to program > > > > 3DES key). > > > > > > That isn't how the kernel handles inline encryption keyslots. They are only > > > programmed as needed for I/O. If they are all in-use by pending I/O requests, > > > then the kernel waits for an I/O request to finish and reprograms the keyslot it > > > was using. There is never an error reported due to lack of keyslots. > > > > Does that mean that the I/O can be outstanding for the very long period > > of time? Or that if the ICE hardware has just a single keyslot, but > > there are two concurrent I/O processes using two different keys, the > > framework will be constantly swapping the keys programmed to the HW? > > Yes for both. Of course, system designers are supposed to put in enough > keyslots for this to not be much of a problem. > > So, the "wait for a keyslot" logic in the block layer is necessary in general so > that applications don't unnecessarily get I/O errors. But in a properly tuned > system this logic should be rarely executed. > > And in cases where the keyslots really are a bottleneck, users can of course > just use software encryption instead. > > Note that the number of keyslots is reported in sysfs. > > > I think it might be prefereable for the drivers and the framework to > > support "preprogramming" of the keys, when the key is programmed to the > > hardware when it is set by the user. > > This doesn't sound particularly useful. If there are always enough keyslots, > then keyslots never get evicted and there is no advantage to this. If there are > *not* always enough keyslots, then it's sometimes necessary to evict keyslots, > so it would not be desirable to have them permanently reserved. I'm still trying to propose solutions for the hwkm-or-raw keys problem, trying to find a way to return an error early enough. So it's not about the hints for frequently-used keys, but for returning an error if the user tries to program key type which became unusupported after a previous call. > It could make sense to have some sort of hints mechanism, where frequently-used > keys can be marked as high-priority to keep programmed in a keyslot. I don't > see much of a need for this though, given that the eviction policy is already > LRU, so it already prefers to keep frequently-used keys in a keyslot. > > > Another option might be to let the drivers validate the keys being set > > by userspace. This way in our case the driver might report that it > > supports both raw and wrapped keys, but start rejecting the keys once > > it gets notified that the user has programmed other kind of keys. This > > way key setup can fail, but the actual I/O can not. WDYT? > > Well, that has the same effect as the crypto capabilities check which is already > done. The problem is that your proposal effectively revokes a capability, and > that is racy. > > - Eric -- With best wishes Dmitry