linux-integrity.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] KEYS: trusted: Fix trusted key backends when building as module
@ 2021-07-16  8:17 Andreas Rammhold
  2021-07-19  6:26 ` Sumit Garg
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Andreas Rammhold @ 2021-07-16  8:17 UTC (permalink / raw)
  To: James Bottomley, Jarkko Sakkinen, Mimi Zohar, David Howells,
	James Morris, Serge E . Hallyn, linux-integrity, keyrings,
	linux-security-module, linux-kernel
  Cc: Andreas Rammhold

Before this commit the kernel could end up with no trusted key sources
even thought both of the currently supported backends (tpm & tee) were
compoiled as modules. This manifested in the trusted key type not being
registered at all.

When checking if a CONFIG_… preprocessor variable is defined we only
test for the builtin (=y) case and not the module (=m) case. By using
the IS_ENABLE(…) macro we to test for both cases.

Signed-off-by: Andreas Rammhold <andreas@rammhold.de>
---
 security/keys/trusted-keys/trusted_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c
index d5c891d8d353..fd640614b168 100644
--- a/security/keys/trusted-keys/trusted_core.c
+++ b/security/keys/trusted-keys/trusted_core.c
@@ -27,10 +27,10 @@ module_param_named(source, trusted_key_source, charp, 0);
 MODULE_PARM_DESC(source, "Select trusted keys source (tpm or tee)");
 
 static const struct trusted_key_source trusted_key_sources[] = {
-#if defined(CONFIG_TCG_TPM)
+#if IS_ENABLED(CONFIG_TCG_TPM)
 	{ "tpm", &trusted_key_tpm_ops },
 #endif
-#if defined(CONFIG_TEE)
+#if IS_ENABLED(CONFIG_TEE)
 	{ "tee", &trusted_key_tee_ops },
 #endif
 };
-- 
2.32.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] KEYS: trusted: Fix trusted key backends when building as module
  2021-07-16  8:17 [PATCH] KEYS: trusted: Fix trusted key backends when building as module Andreas Rammhold
@ 2021-07-19  6:26 ` Sumit Garg
  2021-07-19  7:10 ` Ahmad Fatoum
  2021-07-27  2:55 ` Jarkko Sakkinen
  2 siblings, 0 replies; 7+ messages in thread
From: Sumit Garg @ 2021-07-19  6:26 UTC (permalink / raw)
  To: Andreas Rammhold
  Cc: James Bottomley, Jarkko Sakkinen, Mimi Zohar, David Howells,
	James Morris, Serge E . Hallyn, linux-integrity,
	open list:ASYMMETRIC KEYS, open list:SECURITY SUBSYSTEM,
	Linux Kernel Mailing List

On Fri, 16 Jul 2021 at 13:54, Andreas Rammhold <andreas@rammhold.de> wrote:
>
> Before this commit the kernel could end up with no trusted key sources
> even thought both of the currently supported backends (tpm & tee) were

s/thought/though/

> compoiled as modules. This manifested in the trusted key type not being

s/compoiled/compiled/

> registered at all.
>
> When checking if a CONFIG_… preprocessor variable is defined we only
> test for the builtin (=y) case and not the module (=m) case. By using
> the IS_ENABLE(…) macro we to test for both cases.
>

s/to/do/

> Signed-off-by: Andreas Rammhold <andreas@rammhold.de>
> ---
>  security/keys/trusted-keys/trusted_core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>

Apart from minor nits above, add a corresponding fixes tag. With that:

Reviewed-by: Sumit Garg <sumit.garg@linaro.org>

-Sumit

> diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c
> index d5c891d8d353..fd640614b168 100644
> --- a/security/keys/trusted-keys/trusted_core.c
> +++ b/security/keys/trusted-keys/trusted_core.c
> @@ -27,10 +27,10 @@ module_param_named(source, trusted_key_source, charp, 0);
>  MODULE_PARM_DESC(source, "Select trusted keys source (tpm or tee)");
>
>  static const struct trusted_key_source trusted_key_sources[] = {
> -#if defined(CONFIG_TCG_TPM)
> +#if IS_ENABLED(CONFIG_TCG_TPM)
>         { "tpm", &trusted_key_tpm_ops },
>  #endif
> -#if defined(CONFIG_TEE)
> +#if IS_ENABLED(CONFIG_TEE)
>         { "tee", &trusted_key_tee_ops },
>  #endif
>  };
> --
> 2.32.0
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] KEYS: trusted: Fix trusted key backends when building as module
  2021-07-16  8:17 [PATCH] KEYS: trusted: Fix trusted key backends when building as module Andreas Rammhold
  2021-07-19  6:26 ` Sumit Garg
@ 2021-07-19  7:10 ` Ahmad Fatoum
  2021-07-19  8:06   ` Sumit Garg
  2021-07-27  2:57   ` Jarkko Sakkinen
  2021-07-27  2:55 ` Jarkko Sakkinen
  2 siblings, 2 replies; 7+ messages in thread
From: Ahmad Fatoum @ 2021-07-19  7:10 UTC (permalink / raw)
  To: Andreas Rammhold, James Bottomley, Jarkko Sakkinen, Mimi Zohar,
	David Howells, James Morris, Serge E . Hallyn, linux-integrity,
	keyrings, linux-security-module, linux-kernel,
	Pengutronix Kernel Team

Hello Andreas,

On 16.07.21 10:17, Andreas Rammhold wrote:
> Before this commit the kernel could end up with no trusted key sources
> even thought both of the currently supported backends (tpm & tee) were
> compoiled as modules. This manifested in the trusted key type not being
> registered at all.

I assume (TPM) trusted key module use worked before the TEE rework? If so,

an appropriate Fixes: Tag would then be in order.

> When checking if a CONFIG_… preprocessor variable is defined we only
> test for the builtin (=y) case and not the module (=m) case. By using
> the IS_ENABLE(…) macro we to test for both cases.

It looks to me like you could now provoke a link error if TEE is a module
and built-in trusted key core tries to link against trusted_key_tee_ops.

One solution for that IS_REACHABLE(). Another is to address the root cause,
which is the inflexible trusted keys Kconfig description:

- Trusted keys despite TEE support can still only be built when TCG_TPM is enabled
- There is no support to have TEE or TPM enabled without using those for
  enabled trusted keys as well
- As you noticed, module build of the backend has issues

I addressed these three issues in a patch[1], a month ago, but have yet to
receive feedback.

[1]: https://lore.kernel.org/linux-integrity/f8285eb0135ba30c9d846cf9dd395d1f5f8b1efc.1624364386.git-series.a.fatoum@pengutronix.de/

Cheers,
Ahmad

> Signed-off-by: Andreas Rammhold <andreas@rammhold.de>
> ---
>  security/keys/trusted-keys/trusted_core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c
> index d5c891d8d353..fd640614b168 100644
> --- a/security/keys/trusted-keys/trusted_core.c
> +++ b/security/keys/trusted-keys/trusted_core.c
> @@ -27,10 +27,10 @@ module_param_named(source, trusted_key_source, charp, 0);
>  MODULE_PARM_DESC(source, "Select trusted keys source (tpm or tee)");
>  
>  static const struct trusted_key_source trusted_key_sources[] = {
> -#if defined(CONFIG_TCG_TPM)
> +#if IS_ENABLED(CONFIG_TCG_TPM)
>  	{ "tpm", &trusted_key_tpm_ops },
>  #endif
> -#if defined(CONFIG_TEE)
> +#if IS_ENABLED(CONFIG_TEE)
>  	{ "tee", &trusted_key_tee_ops },
>  #endif
>  };
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] KEYS: trusted: Fix trusted key backends when building as module
  2021-07-19  7:10 ` Ahmad Fatoum
@ 2021-07-19  8:06   ` Sumit Garg
  2021-07-19  9:13     ` andreas
  2021-07-27  2:57   ` Jarkko Sakkinen
  1 sibling, 1 reply; 7+ messages in thread
From: Sumit Garg @ 2021-07-19  8:06 UTC (permalink / raw)
  To: Ahmad Fatoum
  Cc: Andreas Rammhold, James Bottomley, Jarkko Sakkinen, Mimi Zohar,
	David Howells, James Morris, Serge E . Hallyn, linux-integrity,
	open list:ASYMMETRIC KEYS, open list:SECURITY SUBSYSTEM,
	Linux Kernel Mailing List, Pengutronix Kernel Team

On Mon, 19 Jul 2021 at 12:40, Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>
> Hello Andreas,
>
> On 16.07.21 10:17, Andreas Rammhold wrote:
> > Before this commit the kernel could end up with no trusted key sources
> > even thought both of the currently supported backends (tpm & tee) were
> > compoiled as modules. This manifested in the trusted key type not being
> > registered at all.
>
> I assume (TPM) trusted key module use worked before the TEE rework? If so,
>
> an appropriate Fixes: Tag would then be in order.
>
> > When checking if a CONFIG_… preprocessor variable is defined we only
> > test for the builtin (=y) case and not the module (=m) case. By using
> > the IS_ENABLE(…) macro we to test for both cases.
>
> It looks to me like you could now provoke a link error if TEE is a module
> and built-in trusted key core tries to link against trusted_key_tee_ops.
>

That's true.

> One solution for that IS_REACHABLE(). Another is to address the root cause,
> which is the inflexible trusted keys Kconfig description:
>
> - Trusted keys despite TEE support can still only be built when TCG_TPM is enabled
> - There is no support to have TEE or TPM enabled without using those for
>   enabled trusted keys as well
> - As you noticed, module build of the backend has issues
>
> I addressed these three issues in a patch[1], a month ago, but have yet to
> receive feedback.

That's an oversight on my part since this patch was part of the new
CAAM trust source patch-set. Although I do admit that it was on my
TODO list. So I have provided some feedback on that patch. Can you
post the next version as an independent fix patch?

-Sumit

>
> [1]: https://lore.kernel.org/linux-integrity/f8285eb0135ba30c9d846cf9dd395d1f5f8b1efc.1624364386.git-series.a.fatoum@pengutronix.de/
>
> Cheers,
> Ahmad
>
> > Signed-off-by: Andreas Rammhold <andreas@rammhold.de>
> > ---
> >  security/keys/trusted-keys/trusted_core.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c
> > index d5c891d8d353..fd640614b168 100644
> > --- a/security/keys/trusted-keys/trusted_core.c
> > +++ b/security/keys/trusted-keys/trusted_core.c
> > @@ -27,10 +27,10 @@ module_param_named(source, trusted_key_source, charp, 0);
> >  MODULE_PARM_DESC(source, "Select trusted keys source (tpm or tee)");
> >
> >  static const struct trusted_key_source trusted_key_sources[] = {
> > -#if defined(CONFIG_TCG_TPM)
> > +#if IS_ENABLED(CONFIG_TCG_TPM)
> >       { "tpm", &trusted_key_tpm_ops },
> >  #endif
> > -#if defined(CONFIG_TEE)
> > +#if IS_ENABLED(CONFIG_TEE)
> >       { "tee", &trusted_key_tee_ops },
> >  #endif
> >  };
> >
>
>
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] KEYS: trusted: Fix trusted key backends when building as module
  2021-07-19  8:06   ` Sumit Garg
@ 2021-07-19  9:13     ` andreas
  0 siblings, 0 replies; 7+ messages in thread
From: andreas @ 2021-07-19  9:13 UTC (permalink / raw)
  To: Sumit Garg, Ahmad Fatoum
  Cc: James Bottomley, Jarkko Sakkinen, Mimi Zohar, David Howells,
	James Morris, Serge E . Hallyn, linux-integrity,
	open list:ASYMMETRIC KEYS, open list:SECURITY SUBSYSTEM,
	Linux Kernel Mailing List, Pengutronix Kernel Team

On 13:36 19.07.21, Sumit Garg wrote:
> On Mon, 19 Jul 2021 at 12:40, Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> >
> > Hello Andreas,
> >
> > On 16.07.21 10:17, Andreas Rammhold wrote:
> > > Before this commit the kernel could end up with no trusted key sources
> > > even thought both of the currently supported backends (tpm & tee) were
> > > compoiled as modules. This manifested in the trusted key type not being
> > > registered at all.
> >
> > I assume (TPM) trusted key module use worked before the TEE rework? If so,
> >
> > an appropriate Fixes: Tag would then be in order.
> >
> > > When checking if a CONFIG_… preprocessor variable is defined we only
> > > test for the builtin (=y) case and not the module (=m) case. By using
> > > the IS_ENABLE(…) macro we to test for both cases.
> >
> > It looks to me like you could now provoke a link error if TEE is a module
> > and built-in trusted key core tries to link against trusted_key_tee_ops.
> >
> 
> That's true.
> 
> > One solution for that IS_REACHABLE(). Another is to address the root cause,
> > which is the inflexible trusted keys Kconfig description:
> >
> > - Trusted keys despite TEE support can still only be built when TCG_TPM is enabled
> > - There is no support to have TEE or TPM enabled without using those for
> >   enabled trusted keys as well
> > - As you noticed, module build of the backend has issues
> >
> > I addressed these three issues in a patch[1], a month ago, but have yet to
> > receive feedback.
> 
> That's an oversight on my part since this patch was part of the new
> CAAM trust source patch-set. Although I do admit that it was on my
> TODO list. So I have provided some feedback on that patch. Can you
> post the next version as an independent fix patch?

Thank you both for the feedback. In light of thes feedback and the
patchset that Ahmad posted I'll not address the issue and not send a v2
of this.

I'll try to squeeze in some time to test the other patch and provide
feedback.

Andi

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] KEYS: trusted: Fix trusted key backends when building as module
  2021-07-16  8:17 [PATCH] KEYS: trusted: Fix trusted key backends when building as module Andreas Rammhold
  2021-07-19  6:26 ` Sumit Garg
  2021-07-19  7:10 ` Ahmad Fatoum
@ 2021-07-27  2:55 ` Jarkko Sakkinen
  2 siblings, 0 replies; 7+ messages in thread
From: Jarkko Sakkinen @ 2021-07-27  2:55 UTC (permalink / raw)
  To: Andreas Rammhold
  Cc: James Bottomley, Mimi Zohar, David Howells, James Morris,
	Serge E . Hallyn, linux-integrity, keyrings,
	linux-security-module, linux-kernel

On Fri, Jul 16, 2021 at 10:17:22AM +0200, Andreas Rammhold wrote:
> Before this commit the kernel could end up with no trusted key sources
> even thought both of the currently supported backends (tpm & tee) were

Nit: "TPM and TEE" instead of "tpm & tee"

> compoiled as modules. This manifested in the trusted key type not being
> registered at all.

Do you have a commit ID for the failing commit?

> When checking if a CONFIG_… preprocessor variable is defined we only
> test for the builtin (=y) case and not the module (=m) case. By using
> the IS_ENABLE(…) macro we to test for both cases.

Nit: IS_ENABLED() (without dots inside, missing 'D').

> 
> Signed-off-by: Andreas Rammhold <andreas@rammhold.de>
> ---
>  security/keys/trusted-keys/trusted_core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c
> index d5c891d8d353..fd640614b168 100644
> --- a/security/keys/trusted-keys/trusted_core.c
> +++ b/security/keys/trusted-keys/trusted_core.c
> @@ -27,10 +27,10 @@ module_param_named(source, trusted_key_source, charp, 0);
>  MODULE_PARM_DESC(source, "Select trusted keys source (tpm or tee)");
>  
>  static const struct trusted_key_source trusted_key_sources[] = {
> -#if defined(CONFIG_TCG_TPM)
> +#if IS_ENABLED(CONFIG_TCG_TPM)
>  	{ "tpm", &trusted_key_tpm_ops },
>  #endif
> -#if defined(CONFIG_TEE)
> +#if IS_ENABLED(CONFIG_TEE)
>  	{ "tee", &trusted_key_tee_ops },
>  #endif
>  };
> -- 
> 2.32.0
> 
> 

/Jarkko

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] KEYS: trusted: Fix trusted key backends when building as module
  2021-07-19  7:10 ` Ahmad Fatoum
  2021-07-19  8:06   ` Sumit Garg
@ 2021-07-27  2:57   ` Jarkko Sakkinen
  1 sibling, 0 replies; 7+ messages in thread
From: Jarkko Sakkinen @ 2021-07-27  2:57 UTC (permalink / raw)
  To: Ahmad Fatoum
  Cc: Andreas Rammhold, James Bottomley, Mimi Zohar, David Howells,
	James Morris, Serge E . Hallyn, linux-integrity, keyrings,
	linux-security-module, linux-kernel, Pengutronix Kernel Team

On Mon, Jul 19, 2021 at 09:10:01AM +0200, Ahmad Fatoum wrote:
> Hello Andreas,
> 
> On 16.07.21 10:17, Andreas Rammhold wrote:
> > Before this commit the kernel could end up with no trusted key sources
> > even thought both of the currently supported backends (tpm & tee) were
> > compoiled as modules. This manifested in the trusted key type not being
> > registered at all.
> 
> I assume (TPM) trusted key module use worked before the TEE rework? If so,
> 
> an appropriate Fixes: Tag would then be in order.
> 
> > When checking if a CONFIG_… preprocessor variable is defined we only
> > test for the builtin (=y) case and not the module (=m) case. By using
> > the IS_ENABLE(…) macro we to test for both cases.
> 
> It looks to me like you could now provoke a link error if TEE is a module
> and built-in trusted key core tries to link against trusted_key_tee_ops.
> 
> One solution for that IS_REACHABLE(). Another is to address the root cause,
> which is the inflexible trusted keys Kconfig description:
> 
> - Trusted keys despite TEE support can still only be built when TCG_TPM is enabled
> - There is no support to have TEE or TPM enabled without using those for
>   enabled trusted keys as well
> - As you noticed, module build of the backend has issues
> 
> I addressed these three issues in a patch[1], a month ago, but have yet to
> receive feedback.

Which of the patches is the bug fix?

/Jarkko

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-07-27  2:57 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-16  8:17 [PATCH] KEYS: trusted: Fix trusted key backends when building as module Andreas Rammhold
2021-07-19  6:26 ` Sumit Garg
2021-07-19  7:10 ` Ahmad Fatoum
2021-07-19  8:06   ` Sumit Garg
2021-07-19  9:13     ` andreas
2021-07-27  2:57   ` Jarkko Sakkinen
2021-07-27  2:55 ` Jarkko Sakkinen

This is a public inbox, see mirroring instructions
on how to clone and mirror all data and code used for this inbox