On Fri, Jan 7, 2022 at 8:32 AM Sumit Garg <sumit.garg@xxxxxxxxxx> wrote: > > On Fri, 7 Jan 2022 at 18:23, Yael Tiomkin <yaelt@xxxxxxxxxx> wrote: > > > > Hi Sumit, > > > > On Fri, Jan 7, 2022 at 12:15 AM Sumit Garg <sumit.garg@xxxxxxxxxx> wrote: > > > > > > Hi Yael, > > > > > > On Thu, 6 Jan 2022 at 01:48, Yael Tiomkin <yaelt@xxxxxxxxxx> wrote: > > > > > > > > Hi Sumit, > > > > > > > > On Mon, Jan 3, 2022 at 1:51 AM Sumit Garg <sumit.garg@xxxxxxxxxx> wrote: > > > > > > > > > > Hi Mimi, > > > > > > > > > > Apologies for the delayed reply as I was on leave for a long new year weekend. > > > > > > > > > > On Thu, 30 Dec 2021 at 18:59, Mimi Zohar <zohar@xxxxxxxxxxxxx> wrote: > > > > > > > > > > > > Hi Sumit, > > > > > > > > > > > > On Thu, 2021-12-30 at 15:37 +0530, Sumit Garg wrote: > > > > > > > + Jan, Ahmad > > > > > > > > > > > > > > On Thu, 30 Dec 2021 at 03:24, Yael Tiomkin <yaelt@xxxxxxxxxx> wrote: > > > > > > > > > > > > > > > > The encrypted.c class supports instantiation of encrypted keys with > > > > > > > > either an already-encrypted key material, or by generating new key > > > > > > > > material based on random numbers. This patch defines a new datablob > > > > > > > > format: [<format>] <master-key name> <decrypted data length> > > > > > > > > <decrypted data> that allows to instantiate encrypted keys using > > > > > > > > user-provided decrypted data, and therefore allows to perform key > > > > > > > > encryption from userspace. The decrypted key material will be > > > > > > > > inaccessible from userspace. > > > > > > > > > > > > > > This type of user-space key import feature has already been discussed > > > > > > > at large in the context of trusted keys here [1]. So what makes it > > > > > > > special in case of encrypted keys such that it isn't a "UNSAFE_IMPORT" > > > > > > > or "DEBUGGING_IMPORT" or "DEVELOPMENT_IMPORT", ...? > > > > > > > > > > > > > > [1] https://lore.kernel.org/linux-integrity/74830d4f-5a76-8ba8-aad0-0d79f7c01af9@xxxxxxxxxxxxxx/ > > > > > > > > > > > > > > -Sumit > > > > > > > > > > > > > > > > > > > > > > > Reviewed-by: Mimi Zohar <zohar@xxxxxxxxxxxxx> > > > > > > > > Signed-off-by: Yael Tiomkin <yaelt@xxxxxxxxxx> > > > > > > > > > > > > There is a difference between trusted and encrypted keys. > > > > > > > > > > Yeah I understand the implementation differences. > > > > > > > > > > > So in > > > > > > addition to pointing to the rather long discussion thread, please > > > > > > summarize the conclusion and, assuming you agree, include why in once > > > > > > case it was acceptable and in the other it wasn't to provide userspace > > > > > > key data. > > > > > > > > > > My major concern with importing user-space key data in *plain* format > > > > > is that if import is *not* done in a safe (manufacturing or > > > > > production) environment then the plain key data is susceptible to > > > > > user-space compromises when the device is in the field. > > > > > > > > I agree this can happen. Key distribution in any scenario needs to be > > > > secure and this could also potentially be an issue if the key is first > > > > encrypted and then imported. > > > > > > Currently its not the case with encrypted keys. These are random keys > > > generated within the kernel and encrypted with master key within the > > > kernel and then exposed to user-space as encrypted blob only. > > > > > > There are two different ways to create encrypted keys. One is to have > > them generated within the kernel using random numbers, and the other > > is by importing them in their encrypted form from user-space. > > I was referring to the latter in my previous statement. > > > > So, from a key distribution security point of view, encrypted key > user-space import is **not equal to** plain key user-space import. > That's why we need to have a separate compile time option as every > device may come with its own threat model and may choose to enable or > disable this user-space plain key import feature. > > -Sumit > > > > > > > > We can make sure the documentation > > > > highlights the safety requirement. > > > > > > > > > > IMO, you should enable this feature as a compile time option. The help > > > text for that config option should highlight the use-case along with a > > > safety warning. > > > > > > -Sumit > > > > > > > > > > > > > And it sounds like we are diverting from basic definition [1] of encrypted keys: > > > > > > > > > > "Trusted and Encrypted Keys are two new key types added to the > > > > > existing kernel key ring service. Both of these new types are variable > > > > > length symmetric keys, and in both cases all keys are created in the > > > > > kernel, and **user space sees, stores, and loads** only encrypted > > > > > blobs." > > > > > > > > > > Also, as Jarrko mentioned earlier the use-case is still not clear to > > > > > me as well. Isn't user logon keys an alternative option for > > > > > non-readable user-space keys? > > > > > > > > The goal in this change is to allow key encryption from userspace, > > > > using user-provided decrypted data. This cannot be achieved in logon > > > > keys, which as you mentioned, are simply non-readable user type keys. > > > > > > > > > > > > > > > > > > [1] https://www.kernel.org/doc/html/v4.13/security/keys/trusted-encrypted.html > > > > > > > > > > -Sumit > > > > > > > > > > > > > > > > > thanks, > > > > > > > > > > > > Mimi > > > > > > > > > > > > > > Yael Hi Sumit, The encrypted-keys module is already controlled by the CONFIG_ENCRYPTED_KEYS option, which I think might give sufficient granularity to control the behavior. Do you still think a feature dedicated option is needed? Yael
