| Message ID | 1565682784-10234-1-git-send-email-sumit.garg@linaro.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add generic trusted keys framework/subsystem | expand |
Hi Sumit, On Tue, 2019-08-13 at 13:22 +0530, Sumit Garg wrote: > This patch-set is an outcome of discussion here [1]. It has evolved very > much since v1 to create, consolidate and generalize trusted keys > subsystem. > > This framework has been tested with trusted keys support provided via TEE > but I wasn't able to test it with a TPM device as I don't possess one. It > would be really helpful if others could test this patch-set using a TPM > device. With the "CONFIG_HEADER_TEST" and "CONFIG_KERNEL_HEADER_TEST" config options enabled, which is required for linux-next, it fails to build. Mimi
Hi Mimi, On Wed, 14 Aug 2019 at 18:54, Mimi Zohar <zohar@kernel.org> wrote: > > Hi Sumit, > > On Tue, 2019-08-13 at 13:22 +0530, Sumit Garg wrote: > > This patch-set is an outcome of discussion here [1]. It has evolved very > > much since v1 to create, consolidate and generalize trusted keys > > subsystem. > > > > This framework has been tested with trusted keys support provided via TEE > > but I wasn't able to test it with a TPM device as I don't possess one. It > > would be really helpful if others could test this patch-set using a TPM > > device. > > With the "CONFIG_HEADER_TEST" and "CONFIG_KERNEL_HEADER_TEST" config > options enabled, which is required for linux-next, it fails to build. > TBH, I wasn't aware about this test feature for headers. It looks like the header which fails this test is "include/keys/trusted_tpm.h" which is basically a rename of "include/keys/trusted.h" plus changes in this patch-set. And "include/keys/trusted.h" header is already put under blacklist here: "include/Kbuild +68" as it fails to build. So its that rename due to which build failure is observed now. It seems to be an easy fix for this build failure via following changes: diff --git a/include/keys/trusted_tpm.h b/include/keys/trusted_tpm.h index 7b593447920b..ca1bec0ef65d 100644 --- a/include/keys/trusted_tpm.h +++ b/include/keys/trusted_tpm.h @@ -2,6 +2,9 @@ #ifndef __TRUSTED_TPM_H #define __TRUSTED_TPM_H +#include <keys/trusted-type.h> +#include <linux/tpm_command.h> + /* implementation specific TPM constants */ #define MAX_BUF_SIZE 1024 #define TPM_GETRANDOM_SIZE 14 So I will include above changes in this patch-set and also remove "include/keys/trusted.h" header from the blacklist. -Sumit > Mimi
On Thu, 2019-08-15 at 18:33 +0530, Sumit Garg wrote: > Hi Mimi, > > On Wed, 14 Aug 2019 at 18:54, Mimi Zohar <zohar@kernel.org> wrote: > > > > Hi Sumit, > > > > On Tue, 2019-08-13 at 13:22 +0530, Sumit Garg wrote: > > > This patch-set is an outcome of discussion here [1]. It has evolved very > > > much since v1 to create, consolidate and generalize trusted keys > > > subsystem. > > > > > > This framework has been tested with trusted keys support provided via TEE > > > but I wasn't able to test it with a TPM device as I don't possess one. It > > > would be really helpful if others could test this patch-set using a TPM > > > device. > > > > With the "CONFIG_HEADER_TEST" and "CONFIG_KERNEL_HEADER_TEST" config > > options enabled, which is required for linux-next, it fails to build. > > > > TBH, I wasn't aware about this test feature for headers. It's new to me too. > It looks like > the header which fails this test is "include/keys/trusted_tpm.h" which > is basically a rename of "include/keys/trusted.h" plus changes in this > patch-set. > > And "include/keys/trusted.h" header is already put under blacklist > here: "include/Kbuild +68" as it fails to build. So its that rename > due to which build failure is observed now. > > It seems to be an easy fix for this build failure via following changes: > > diff --git a/include/keys/trusted_tpm.h b/include/keys/trusted_tpm.h > index 7b593447920b..ca1bec0ef65d 100644 > --- a/include/keys/trusted_tpm.h > +++ b/include/keys/trusted_tpm.h > @@ -2,6 +2,9 @@ > #ifndef __TRUSTED_TPM_H > #define __TRUSTED_TPM_H > > +#include <keys/trusted-type.h> > +#include <linux/tpm_command.h> > + > /* implementation specific TPM constants */ > #define MAX_BUF_SIZE 1024 > #define TPM_GETRANDOM_SIZE 14 > > So I will include above changes in this patch-set and also remove > "include/keys/trusted.h" header from the blacklist. That works, thanks. With this patch set, at least the EVM trusted key is properly being decrypted by the encrypted key with both a TPM 1.2 and PTT TPM 2.0. My laptop still boots properly. Over the weekend I'll try to actually review the patches. Mimi
On Thu, 15 Aug 2019 at 20:36, Mimi Zohar <zohar@kernel.org> wrote: > > On Thu, 2019-08-15 at 18:33 +0530, Sumit Garg wrote: > > Hi Mimi, > > > > On Wed, 14 Aug 2019 at 18:54, Mimi Zohar <zohar@kernel.org> wrote: > > > > > > Hi Sumit, > > > > > > On Tue, 2019-08-13 at 13:22 +0530, Sumit Garg wrote: > > > > This patch-set is an outcome of discussion here [1]. It has evolved very > > > > much since v1 to create, consolidate and generalize trusted keys > > > > subsystem. > > > > > > > > This framework has been tested with trusted keys support provided via TEE > > > > but I wasn't able to test it with a TPM device as I don't possess one. It > > > > would be really helpful if others could test this patch-set using a TPM > > > > device. > > > > > > With the "CONFIG_HEADER_TEST" and "CONFIG_KERNEL_HEADER_TEST" config > > > options enabled, which is required for linux-next, it fails to build. > > > > > > > TBH, I wasn't aware about this test feature for headers. > > It's new to me too. > > > It looks like > > the header which fails this test is "include/keys/trusted_tpm.h" which > > is basically a rename of "include/keys/trusted.h" plus changes in this > > patch-set. > > > > And "include/keys/trusted.h" header is already put under blacklist > > here: "include/Kbuild +68" as it fails to build. So its that rename > > due to which build failure is observed now. > > > > It seems to be an easy fix for this build failure via following changes: > > > > diff --git a/include/keys/trusted_tpm.h b/include/keys/trusted_tpm.h > > index 7b593447920b..ca1bec0ef65d 100644 > > --- a/include/keys/trusted_tpm.h > > +++ b/include/keys/trusted_tpm.h > > @@ -2,6 +2,9 @@ > > #ifndef __TRUSTED_TPM_H > > #define __TRUSTED_TPM_H > > > > +#include <keys/trusted-type.h> > > +#include <linux/tpm_command.h> > > + > > /* implementation specific TPM constants */ > > #define MAX_BUF_SIZE 1024 > > #define TPM_GETRANDOM_SIZE 14 > > > > So I will include above changes in this patch-set and also remove > > "include/keys/trusted.h" header from the blacklist. > > That works, thanks. With this patch set, at least the EVM trusted key > is properly being decrypted by the encrypted key with both a TPM 1.2 > and PTT TPM 2.0. My laptop still boots properly. Over the weekend > I'll try to actually review the patches. > Thanks Mimi for testing this patch-set. -Sumit > Mimi
On Tue, Aug 13, 2019 at 01:22:59PM +0530, Sumit Garg wrote: > This patch-set is an outcome of discussion here [1]. It has evolved very > much since v1 to create, consolidate and generalize trusted keys > subsystem. > > This framework has been tested with trusted keys support provided via TEE > but I wasn't able to test it with a TPM device as I don't possess one. It > would be really helpful if others could test this patch-set using a TPM > device. I think 1/5-4/5 make up a non-RFC patch set that needs to reviewed, tested and merged as a separate entity. On the other hand 5/5 cannot be merged even if I fully agreed on the code change as without TEE patch it does not add any value for Linux. To straighten up thing I would suggest that the next patch set version would only consists of the first four patches and we meld them to the shape so that we can land them to the mainline. Then it should be way more easier to concentrate the actual problem you are trying to resolve. /Jarkko
On Mon, 19 Aug 2019 at 22:24, Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote: > > On Tue, Aug 13, 2019 at 01:22:59PM +0530, Sumit Garg wrote: > > This patch-set is an outcome of discussion here [1]. It has evolved very > > much since v1 to create, consolidate and generalize trusted keys > > subsystem. > > > > This framework has been tested with trusted keys support provided via TEE > > but I wasn't able to test it with a TPM device as I don't possess one. It > > would be really helpful if others could test this patch-set using a TPM > > device. > > I think 1/5-4/5 make up a non-RFC patch set that needs to reviewed, > tested and merged as a separate entity. > Okay. > On the other hand 5/5 cannot be merged even if I fully agreed on > the code change as without TEE patch it does not add any value for > Linux. > I agree here that 5/5 should go along with TEE patch-set. But if you look at initial v1 patch-set, the idea was to get feedback on trusted keys abstraction as a standalone patch along with testing using a TPM (1.x or 2.0). Since Mimi has tested this patch-set with TPM (1.x & 2.0), I am happy to merge 5/5 with TEE patch-set. But it would be nice if I could get feedback on 5/5 before I send next version of TEE patch-set. > To straighten up thing I would suggest that the next patch set > version would only consists of the first four patches and we meld > them to the shape so that we can land them to the mainline. Then > it should be way more easier to concentrate the actual problem you > are trying to resolve. > Okay will send next patch-set version with first four patches only. -Sumit > /Jarkko
On Tue, Aug 20, 2019 at 11:16:46AM +0530, Sumit Garg wrote: > I agree here that 5/5 should go along with TEE patch-set. But if you > look at initial v1 patch-set, the idea was to get feedback on trusted > keys abstraction as a standalone patch along with testing using a TPM > (1.x or 2.0). > > Since Mimi has tested this patch-set with TPM (1.x & 2.0), I am happy > to merge 5/5 with TEE patch-set. But it would be nice if I could get > feedback on 5/5 before I send next version of TEE patch-set. OK, that is understandable. I'll check it out tomorrow. /Jarkko
This patch-set is an outcome of discussion here [1]. It has evolved very much since v1 to create, consolidate and generalize trusted keys subsystem. This framework has been tested with trusted keys support provided via TEE but I wasn't able to test it with a TPM device as I don't possess one. It would be really helpful if others could test this patch-set using a TPM device. [1] https://www.mail-archive.com/linux-doc@vger.kernel.org/msg30591.html Changes in v4: 1. Separate patch for export of tpm_buf code to include/linux/tpm.h 2. Change TPM1.x trusted keys code to use common tpm_buf 3. Keep module name as trusted.ko only Changes in v3: Move TPM2 trusted keys code to trusted keys subsystem. Changes in v2: Split trusted keys abstraction patch for ease of review. Sumit Garg (5): tpm: move tpm_buf code to include/linux/ KEYS: trusted: use common tpm_buf for TPM1.x code KEYS: trusted: create trusted keys subsystem KEYS: trusted: move tpm2 trusted keys code KEYS: trusted: Add generic trusted keys framework crypto/asymmetric_keys/asym_tpm.c | 2 +- drivers/char/tpm/tpm-chip.c | 1 + drivers/char/tpm/tpm-interface.c | 56 --- drivers/char/tpm/tpm.h | 230 ----------- drivers/char/tpm/tpm2-cmd.c | 308 +-------------- include/keys/trusted-type.h | 45 +++ include/keys/{trusted.h => trusted_tpm.h} | 61 +-- include/linux/tpm.h | 270 ++++++++++++- security/keys/Makefile | 2 +- security/keys/trusted-keys/Makefile | 9 + security/keys/trusted-keys/trusted-common.c | 343 ++++++++++++++++ .../keys/{trusted.c => trusted-keys/trusted-tpm.c} | 437 +++++---------------- security/keys/trusted-keys/trusted-tpm2.c | 378 ++++++++++++++++++ 13 files changed, 1141 insertions(+), 1001 deletions(-) rename include/keys/{trusted.h => trusted_tpm.h} (64%) create mode 100644 security/keys/trusted-keys/Makefile create mode 100644 security/keys/trusted-keys/trusted-common.c rename security/keys/{trusted.c => trusted-keys/trusted-tpm.c} (72%) create mode 100644 security/keys/trusted-keys/trusted-tpm2.c