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
