On Tue, 2022-12-20 at 09:13 -0500, Mimi Zohar wrote: > On Tue, 2022-12-20 at 09:03 -0500, James Bottomley wrote: > > On Tue, 2022-12-20 at 08:54 -0500, Mimi Zohar wrote: > > > On Tue, 2022-12-20 at 07:50 -0500, James Bottomley wrote: > > > > On Tue, 2022-12-20 at 12:03 +0530, Sughosh Ganu wrote: > > [...] > > > > > I was able to load the key after clearing the keyring. Thanks > > > > > James and Mimi for your pointers. > > > > > > > > Actually, I think this is a bug in trusted keys. Add on > > > > existing key is supposed to go through the update path. If the > > > > path doesn't exist it returns -EEXIST. Trusted keys have an > > > > update path but they return - EINVAL if the trusted key command > > > > is anything but update (which is used to reseal a key). > > > > Obviously this is incorrect and the code should be returning - > > > > EEXIST for a key we refuse to update to match every other key > > > > type. > > > > > > Re-loading an existing key was previously permitted. Obviously > > > this changed at some point. Any "fixes" should point out when > > > it changed. > > > > Git history doesn't think so. It thinks when you added trusted > > keys with d00a1c72f7f4661212299e6cb132dfa58030bcdb the update path > > already had the -EINVAL return, so reload has always failed this > > way unless we were doing a reseal update. We could certainly > > permit overwriting an existing key with load, but that would be a > > more extensive change. > > Replacing existing keys/keyrings was part of the infrastructure. I > don't think this change has anything to do with trusted type keys. > The ability of replacing keys/keyrings was one of the main reasons > for trusted keyrings (dot prefixed keyrings). Keys can only be replaced by calling the ->update() helper for the key type. If that doesn't exist keyctl add will return -EEXIST (behaviour for at least the last 12 years). Most key type update routines do unconditionally update, so the error they return is the same error they would have returned for an add of a non existent key (EINVAL if the payload is too large, for instance). The trusted keys ->update() helper (trusted_update()) only allows update if the trusted operation is update, so they've always failed a load with EINVAL going back to the original commit I quoted. At no time that I can find has there ever been a modification to this supporting updating trusted keys with anything other than an update trusted operation. So they've supported changing the sealing parameters (PCR values) but not changing the payload. This contrasts with user keys where add of a new payload on an existing key changes the payload. James
