* Re: [PATCH 0/4] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
From: Jarkko Sakkinen @ 2021-08-09 9:35 UTC (permalink / raw)
To: Ahmad Fatoum
Cc: Horia Geantă, Mimi Zohar, Aymen Sghaier, Herbert Xu,
David S. Miller, James Bottomley, Jan Luebbe, Udit Agarwal,
Sumit Garg, David Gstir, Eric Biggers, Franck LENORMAND,
Richard Weinberger, James Morris, linux-kernel, David Howells,
linux-security-module, keyrings, linux-crypto, kernel,
linux-integrity, Steffen Trumtrar, Serge E. Hallyn
In-Reply-To: <b9e44f8e-84a0-90be-6cfc-d3a0bde12178@pengutronix.de>
On Fri, Aug 06, 2021 at 05:12:19PM +0200, Ahmad Fatoum wrote:
> Dear trusted key maintainers,
>
> On 21.07.21 18:48, Ahmad Fatoum wrote:
> > Series applies on top of
> > https://lore.kernel.org/linux-integrity/20210721160258.7024-1-a.fatoum@pengutronix.de/T/#u
> >
> > v2 -> v3:
> > - Split off first Kconfig preparation patch. It fixes a regression,
> > so sent that out, so it can be applied separately (Sumit)
> > - Split off second key import patch. I'll send that out separately
> > as it's a development aid and not required within the CAAM series
> > - add MAINTAINERS entry
>
> Gentle ping. I'd appreciate feedback on this series.
Simple question: what is fscrypt?
/Jarkko
^ permalink raw reply
* Re: [PATCH 2/4] KEYS: trusted: allow trust sources to use kernel RNG for key material
From: Ahmad Fatoum @ 2021-08-09 7:52 UTC (permalink / raw)
To: Sumit Garg
Cc: James Bottomley, Jarkko Sakkinen, Mimi Zohar, David Howells,
kernel, James Morris, Serge E. Hallyn, Horia Geantă,
Aymen Sghaier, Herbert Xu, David S. Miller, Udit Agarwal,
Eric Biggers, Jan Luebbe, David Gstir, Richard Weinberger,
Franck LENORMAND, open list:ASYMMETRIC KEYS,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE, linux-integrity,
Linux Kernel Mailing List, open list:SECURITY SUBSYSTEM
In-Reply-To: <CAFA6WYOskwZNe5Wb5PTtnSHQBonSXZ48eEex0w9jQ+JW4vG=+w@mail.gmail.com>
Hello Sumit,
On 22.07.21 08:31, Sumit Garg wrote:
> On Wed, 21 Jul 2021 at 22:19, Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>>
>> The two existing trusted key sources don't make use of the kernel RNG,
>> but instead let the hardware that does the sealing/unsealing also
>> generate the random key material. While a previous change offers users
>> the choice to use the kernel RNG instead for both, new trust sources
>> may want to unconditionally use the kernel RNG for generating key
>> material, like it's done elsewhere in the kernel.
>>
>> This is especially prudent for hardware that has proven-in-production
>> HWRNG drivers implemented, as otherwise code would have to be duplicated
>> only to arrive at a possibly worse result.
>>
>> Make this possible by turning struct trusted_key_ops::get_random
>> into an optional member. If a driver leaves it NULL, kernel RNG
>> will be used instead.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>> To: James Bottomley <jejb@linux.ibm.com>
>> To: Jarkko Sakkinen <jarkko@kernel.org>
>> To: Mimi Zohar <zohar@linux.ibm.com>
>> To: David Howells <dhowells@redhat.com>
>> Cc: James Morris <jmorris@namei.org>
>> Cc: "Serge E. Hallyn" <serge@hallyn.com>
>> Cc: "Horia Geantă" <horia.geanta@nxp.com>
>> Cc: Aymen Sghaier <aymen.sghaier@nxp.com>
>> Cc: Herbert Xu <herbert@gondor.apana.org.au>
>> Cc: "David S. Miller" <davem@davemloft.net>
>> Cc: Udit Agarwal <udit.agarwal@nxp.com>
>> Cc: Eric Biggers <ebiggers@kernel.org>
>> Cc: Jan Luebbe <j.luebbe@pengutronix.de>
>> Cc: David Gstir <david@sigma-star.at>
>> Cc: Richard Weinberger <richard@nod.at>
>> Cc: Franck LENORMAND <franck.lenormand@nxp.com>
>> Cc: Sumit Garg <sumit.garg@linaro.org>
>> Cc: keyrings@vger.kernel.org
>> Cc: linux-crypto@vger.kernel.org
>> Cc: linux-integrity@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> Cc: linux-security-module@vger.kernel.org
>> ---
>> include/keys/trusted-type.h | 2 +-
>> security/keys/trusted-keys/trusted_core.c | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
>> index d89fa2579ac0..4eb64548a74f 100644
>> --- a/include/keys/trusted-type.h
>> +++ b/include/keys/trusted-type.h
>> @@ -64,7 +64,7 @@ struct trusted_key_ops {
>> /* Unseal a key. */
>> int (*unseal)(struct trusted_key_payload *p, char *datablob);
>>
>> - /* Get a randomized key. */
>> + /* Optional: Get a randomized key. */
>> int (*get_random)(unsigned char *key, size_t key_len);
>>
>> /* Exit key interface. */
>> diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c
>> index 569af9af8df0..d2b7626cde8b 100644
>> --- a/security/keys/trusted-keys/trusted_core.c
>> +++ b/security/keys/trusted-keys/trusted_core.c
>> @@ -334,7 +334,7 @@ static int __init init_trusted(void)
>> continue;
>>
>> get_random = trusted_key_sources[i].ops->get_random;
>> - if (trusted_kernel_rng)
>> + if (trusted_kernel_rng || !get_random)
>> get_random = kernel_get_random;
>>
>
> For ease of understanding, I would prefer to write it as:
>
> get_random = trusted_key_sources[i].ops->get_random ?:
> kernel_get_random;
> if (trusted_kernel_rng)
> get_random = kernel_get_random;
>
> With that:
>
> Acked-by: Sumit Garg <sumit.garg@linaro.org>
I don't think it improves readability to split up the conditional.
At least I need to take a second pass over the code to understand
the second conditional.
Cheers,
Ahmad
>
> -Sumit
>
>> static_call_update(trusted_key_init,
>> --
>> git-series 0.9.1
>
--
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
* Re: [PATCH v3 1/2] tpm: ibmvtpm: Rename tpm_process_cmd to tpm_status and define flag
From: Jarkko Sakkinen @ 2021-08-09 4:27 UTC (permalink / raw)
To: Stefan Berger
Cc: Stefan Berger, peterhuewe, jgg, linux-integrity,
linux-security-module, linux-kernel, Nayna Jain, George Wilson
In-Reply-To: <cddf0b42-c69f-c110-9543-e16d30c9927a@linux.ibm.com>
On Fri, Aug 06, 2021 at 08:08:27AM -0400, Stefan Berger wrote:
>
> On 8/6/21 7:25 AM, Jarkko Sakkinen wrote:
> > On Thu, Aug 05, 2021 at 05:52:55PM -0400, Stefan Berger wrote:
> > > From: Stefan Berger <stefanb@linux.ibm.com>
> > >
> > > Rename the field tpm_processing_cmd to tpm_status in ibmvtpm_dev and set
> > > the TPM_STATUS_BUSY flag while the vTPM is busy processing a command.
> > >
> > > Fixes: 6674ff145eef ("tpm_ibmvtpm: properly handle interrupted packet receptions")
> > > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> > > Cc: Nayna Jain <nayna@linux.ibm.com>
> > > Cc: George Wilson <gcwilson@linux.ibm.com>
> > Please put the bug fix first because otherwise it will be dependent of this
> > patch, which is bad thing when it comes to backporting.
>
> Yes, and that's why I have this one here also with a Fix tag. I basically
> don't want to logically '&' with the 'true' flag but want this
> TPM_STATUS_BUSY flag first.
>
> Stefan
You can then just change the type to 'u8'.
/Jarkko
^ permalink raw reply
* Documenting the requirement of CAP_SETFCAP to map UID 0
From: Michael Kerrisk (man-pages) @ 2021-08-08 9:09 UTC (permalink / raw)
To: Serge E. Hallyn
Cc: mtk.manpages, linux-security-module, lkml, Alejandro Colomar,
Kir Kolyshkin, linux-man
Hello Serge,
Your commit:
[[
commit db2e718a47984b9d71ed890eb2ea36ecf150de18
Author: Serge E. Hallyn <serge@hallyn.com>
Date: Tue Apr 20 08:43:34 2021 -0500
capabilities: require CAP_SETFCAP to map uid 0
]]
added a new requirement when updating a UID map a user namespace
with a value of '0 0 *'.
Kir sent a patch to briefly document this change, but I think much more
should be written. I've attempted to do so. Could you tell me whether the
following text (to be added in user_namespaces(7)) is accurate please:
[[
In order for a process to write to the /proc/[pid]/uid_map
(/proc/[pid]/gid_map) file, all of the following requirements must
be met:
[...]
4. If updating /proc/[pid]/uid_map to create a mapping that maps
UID 0 in the parent namespace, then one of the following must
be true:
* if writing process is in the parent user namespace, then it
must have the CAP_SETFCAP capability in that user namespace;
or
* if the writing process is in the child user namespace, then
the process that created the user namespace must have had
the CAP_SETFCAP capability when the namespace was created.
This rule has been in place since Linux 5.12. It eliminates an
earlier security bug whereby a UID 0 process that lacks the
CAP_SETFCAP capability, which is needed to create a binary with
namespaced file capabilities (as described in capabilities(7)),
could nevertheless create such a binary, by the following
steps:
* Create a new user namespace with the identity mapping (i.e.,
UID 0 in the new user namespace maps to UID 0 in the parent
namespace), so that UID 0 in both namespaces is equivalent
to the same root user ID.
* Since the child process has the CAP_SETFCAP capability, it
could create a binary with namespaced file capabilities that
would then be effective in the parent user namespace (be‐
cause the root user IDs are the same in the two namespaces).
[...]
]]
Thanks,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
^ permalink raw reply
* Re: [PATCH RFC v2 10/12] KEYS: link system_trusted_keys to mok_trusted_keys
From: Eric Snowberg @ 2021-08-06 21:20 UTC (permalink / raw)
To: Mimi Zohar
Cc: keyrings, linux-integrity, David Howells, David Woodhouse,
Herbert Xu, David S . Miller, Jarkko Sakkinen, James Morris,
Serge E . Hallyn, keescook, gregkh, torvalds, scott.branden,
weiyongjun1, nayna, ebiggers, ardb, Lakshmi Ramasubramanian,
lszubowi, linux-kernel, linux-crypto, linux-security-module,
James Bottomley, pjones, glin, konrad.wilk@oracle.com
In-Reply-To: <dc76d9463bb5e081d10154e909321b0d75391846.camel@linux.ibm.com>
> On Aug 6, 2021, at 9:18 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Fri, 2021-08-06 at 09:00 -0600, Eric Snowberg wrote:
>>> On Aug 5, 2021, at 9:19 PM, Mimi Zohar <zohar@linux.ibm.com> wrote:
>>>
>>> On Thu, 2021-08-05 at 19:29 -0600, Eric Snowberg wrote:
>>>
>>>>> From the thread discussion on 00/12:
>>>>>
>>>>> Only the builtin keys should ever be on the builtin keyring. The
>>>>> builtin keyring would need to be linked to the mok keyring. But in the
>>>>> secondary keyring case, the mok keyring would be linked to the
>>>>> secondary keyring, similar to how the builtin keyring is linked to the
>>>>> secondary keyring.
>>>>>
>>>>> if (key_link(secondary_trusted_keys, builtin_trusted_keys) < 0)
>>>>> panic("Can't link trusted keyrings\n");
>>>>
>>>>
>>>> This part is confusing me though.
>>>>
>>>> Here are some of the tests I’m performing with the current series:
>>>>
>>>> Initial setup:
>>>> Create and enroll my own key into the MOK.
>>>> Sign a kernel, kernel module and IMA key with my new CA key.
>>>> Boot with lockdown enabled (to enforce sig validation).
>>>>
>>>> Kernel built with CONFIG_SECONDARY_TRUSTED_KEYRING=y
>>>>
>>>> $ keyctl show %:.secondary_trusted_keys
>>>> Keyring
>>>> 530463486 ---lswrv 0 0 keyring: .secondary_trusted_keys
>>>> 411466727 ---lswrv 0 0 \_ keyring: .builtin_trusted_keys
>>>> 979167715 ---lswrv 0 0 | \_ asymmetric: Build time autogenerated kernel key: 07a56e29cfa1e21379aff2c522efff7d1963202a
>>>> 534573591 ---lswrv 0 0 | \_ asymmetric: Oracle-CA: Oracle certificate signing key: aeefb4bfde095cacaabff81dd266974b1b4e23b8
>>>> 968109018 ---lswrv 0 0 \_ keyring: .mok
>>>> 857795115 ---lswrv 0 0 \_ asymmetric: Erics-CA: UEK signing key: 9bfa6860483aa46bd83f7fa1289d9fc35799e93b
>>>>
>>>> With this setup I can:
>>>> * load a kernel module signed with my CA key
>>>> * run "kexec -ls" with the kernel signed with my CA key
>>>> * run "kexec -ls" with a kernel signed by a key in the platform keyring
>>>> * load another key into the secondary trusted keyring that is signed by my CA key
>>>> * load a key into the ima keyring, signed by my CA key
>>>>
>>>> Kernel built without CONFIG_SECONDARY_TRUSTED_KEYRING defined
>>>>
>>>> $ keyctl show %:.builtin_trusted_keys
>>>> Keyring
>>>> 812785375 ---lswrv 0 0 keyring: .builtin_trusted_keys
>>>> 455418681 ---lswrv 0 0 \_ keyring: .mok
>>>> 910809006 ---lswrv 0 0 | \_ asymmetric: Erics-CA: UEK signing key: 9bfa6860483aa46bd83f7fa1289d9fc35799e93b
>>>> 115345009 ---lswrv 0 0 \_ asymmetric: Oracle-CA: Oracle certificate signing key: aeefb4bfde095cacaabff81dd266974b1b4e23b8
>>>> 513131506 ---lswrv 0 0 \_ asymmetric: Build time autogenerated kernel key: 22353509f203b55b84f15d0aadeddc134b646185
>>>>
>>>> With this setup I can:
>>>> * load a kernel module signed with my CA key
>>>> * run "kexec -ls" with the kernel signed with my CA key
>>>> * run "kexec -ls" with a kernel signed by a key in the platform keyring
>>>> * load a key into the ima keyring, signed by my CA key
>>>>
>>>> So why would the linking need to be switched? Is there a test I’m
>>>> missing? Thanks.
>>>
>>> It's a question of semantics. The builtin keyring name is self
>>> describing. It should only contain the keys compiled into the kernel
>>> or inserted post build into the reserved memory. Not only the kernel
>>> uses the builtin keyring, but userspace may as well[1]. Other users of
>>> the builtin keyring might not want to trust the mok keyring as well.
>>
>> Should this feature only work with kernels built with
>> CONFIG_SECONDARY_TRUSTED_KEYRING defined? If so, I could drop support in
>> the next version for kernels built without it.
>
> Support for loading the CA keys stored in the MOK db onto the mok
> keyring, only if the secondary keyring is configured would really
> simplify the code. Support for using the mok keyring without the
> secondary keyring being configured, could always be added later. As
> long as the other distros agree, I'm all for it.
Agreed, it will simplify the series and there is nothing preventing the
dropped code from being added in the future if a different distro finds
it necessary. I’ll work on this in the next version along with the other
changes you identified. Thanks for your review.
^ permalink raw reply
* Re: lsm-stacking: fix broken lsm audit
From: Casey Schaufler @ 2021-08-06 20:01 UTC (permalink / raw)
To: Dmitry Mastykin, linux-security-module; +Cc: akovalenko, Casey Schaufler
In-Reply-To: <20210806070245.26338-1-dmastykin@astralinux.ru>
On 8/6/2021 12:02 AM, Dmitry Mastykin wrote:
> Hello,
> These patches address the problem of not processing LSM audit rules.
> Problem was introduced in lsm stacking series.
Thank you. I will incorporate these changes in v29.
> These patches are for cschaufler/lsm-stacking repository branch stack-5.10-rc4-v23
> Some UBUNTU distributions have also this problem.
>
> Kind regards,
> Dmitry Mastykin
^ permalink raw reply
* Re: [PATCH] LSM: add NULL check for kcalloc()
From: Ken Goldman @ 2021-08-06 18:33 UTC (permalink / raw)
Cc: linux-security-module, Linux Kernel Mailing List, kernel-team
In-Reply-To: <CADLLry6RmSDuB4nmVKDEiqxXmEU0xrhMn2wieuuVTypMWqc4cQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1257 bytes --]
On 7/14/2021 5:44 PM, Austin Kim wrote:
> 2021년 7월 15일 (목) 오전 4:12, James Morris <jmorris@namei.org>님이 작성:
>>
>> On Tue, 13 Jul 2021, Austin Kim wrote:
>>
>>> From: Austin Kim <austin.kim@lge.com>
>>>
>>> kcalloc() may return NULL when memory allocation fails.
>>> So it is necessary to add NULL check after the call to kcalloc() is made.
>>>
>>> Signed-off-by: Austin Kim <austin.kim@lge.com>
>>> ---
>>> security/security.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/security/security.c b/security/security.c
>>> index 09533cbb7221..f885c9e9bc35 100644
>>> --- a/security/security.c
>>> +++ b/security/security.c
>>> @@ -321,6 +321,8 @@ static void __init ordered_lsm_init(void)
>>>
>>> ordered_lsms = kcalloc(LSM_COUNT + 1, sizeof(*ordered_lsms),
>>> GFP_KERNEL);
>>> + if (ordered_lsms)
>>> + return;
>>
>> Your logic is reversed here.
>
> I feel very sorry for my terrible mistake.
> 'if (ordered_lsms)' should have been 'if (!ordered_lsms)'.
>
I know it's a bit more typing, but
if (ordered_lsms == NULL)
compiles down to the same binary and avoids there errors that
try to treat a pointer as a boolean.
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4490 bytes --]
^ permalink raw reply
* Re: [PATCH RFC v2 10/12] KEYS: link system_trusted_keys to mok_trusted_keys
From: Mimi Zohar @ 2021-08-06 15:18 UTC (permalink / raw)
To: Eric Snowberg
Cc: keyrings, linux-integrity, David Howells, David Woodhouse,
Herbert Xu, David S . Miller, Jarkko Sakkinen, James Morris,
Serge E . Hallyn, keescook, gregkh, torvalds, scott.branden,
weiyongjun1, nayna, ebiggers, ardb, Lakshmi Ramasubramanian,
lszubowi, linux-kernel, linux-crypto, linux-security-module,
James.Bottomley, pjones, glin, konrad.wilk@oracle.com
In-Reply-To: <21CB8F51-9066-4095-9C6E-428FF9E86443@oracle.com>
On Fri, 2021-08-06 at 09:00 -0600, Eric Snowberg wrote:
> > On Aug 5, 2021, at 9:19 PM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> >
> > On Thu, 2021-08-05 at 19:29 -0600, Eric Snowberg wrote:
> >
> >>> From the thread discussion on 00/12:
> >>>
> >>> Only the builtin keys should ever be on the builtin keyring. The
> >>> builtin keyring would need to be linked to the mok keyring. But in the
> >>> secondary keyring case, the mok keyring would be linked to the
> >>> secondary keyring, similar to how the builtin keyring is linked to the
> >>> secondary keyring.
> >>>
> >>> if (key_link(secondary_trusted_keys, builtin_trusted_keys) < 0)
> >>> panic("Can't link trusted keyrings\n");
> >>
> >>
> >> This part is confusing me though.
> >>
> >> Here are some of the tests I’m performing with the current series:
> >>
> >> Initial setup:
> >> Create and enroll my own key into the MOK.
> >> Sign a kernel, kernel module and IMA key with my new CA key.
> >> Boot with lockdown enabled (to enforce sig validation).
> >>
> >> Kernel built with CONFIG_SECONDARY_TRUSTED_KEYRING=y
> >>
> >> $ keyctl show %:.secondary_trusted_keys
> >> Keyring
> >> 530463486 ---lswrv 0 0 keyring: .secondary_trusted_keys
> >> 411466727 ---lswrv 0 0 \_ keyring: .builtin_trusted_keys
> >> 979167715 ---lswrv 0 0 | \_ asymmetric: Build time autogenerated kernel key: 07a56e29cfa1e21379aff2c522efff7d1963202a
> >> 534573591 ---lswrv 0 0 | \_ asymmetric: Oracle-CA: Oracle certificate signing key: aeefb4bfde095cacaabff81dd266974b1b4e23b8
> >> 968109018 ---lswrv 0 0 \_ keyring: .mok
> >> 857795115 ---lswrv 0 0 \_ asymmetric: Erics-CA: UEK signing key: 9bfa6860483aa46bd83f7fa1289d9fc35799e93b
> >>
> >> With this setup I can:
> >> * load a kernel module signed with my CA key
> >> * run "kexec -ls" with the kernel signed with my CA key
> >> * run "kexec -ls" with a kernel signed by a key in the platform keyring
> >> * load another key into the secondary trusted keyring that is signed by my CA key
> >> * load a key into the ima keyring, signed by my CA key
> >>
> >> Kernel built without CONFIG_SECONDARY_TRUSTED_KEYRING defined
> >>
> >> $ keyctl show %:.builtin_trusted_keys
> >> Keyring
> >> 812785375 ---lswrv 0 0 keyring: .builtin_trusted_keys
> >> 455418681 ---lswrv 0 0 \_ keyring: .mok
> >> 910809006 ---lswrv 0 0 | \_ asymmetric: Erics-CA: UEK signing key: 9bfa6860483aa46bd83f7fa1289d9fc35799e93b
> >> 115345009 ---lswrv 0 0 \_ asymmetric: Oracle-CA: Oracle certificate signing key: aeefb4bfde095cacaabff81dd266974b1b4e23b8
> >> 513131506 ---lswrv 0 0 \_ asymmetric: Build time autogenerated kernel key: 22353509f203b55b84f15d0aadeddc134b646185
> >>
> >> With this setup I can:
> >> * load a kernel module signed with my CA key
> >> * run "kexec -ls" with the kernel signed with my CA key
> >> * run "kexec -ls" with a kernel signed by a key in the platform keyring
> >> * load a key into the ima keyring, signed by my CA key
> >>
> >> So why would the linking need to be switched? Is there a test I’m
> >> missing? Thanks.
> >
> > It's a question of semantics. The builtin keyring name is self
> > describing. It should only contain the keys compiled into the kernel
> > or inserted post build into the reserved memory. Not only the kernel
> > uses the builtin keyring, but userspace may as well[1]. Other users of
> > the builtin keyring might not want to trust the mok keyring as well.
>
> Should this feature only work with kernels built with
> CONFIG_SECONDARY_TRUSTED_KEYRING defined? If so, I could drop support in
> the next version for kernels built without it.
Support for loading the CA keys stored in the MOK db onto the mok
keyring, only if the secondary keyring is configured would really
simplify the code. Support for using the mok keyring without the
secondary keyring being configured, could always be added later. As
long as the other distros agree, I'm all for it.
thanks,
Mimi
^ permalink raw reply
* Re: [PATCH 0/4] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
From: Ahmad Fatoum @ 2021-08-06 15:12 UTC (permalink / raw)
To: Jarkko Sakkinen, Horia Geantă, Mimi Zohar, Aymen Sghaier,
Herbert Xu, David S. Miller, James Bottomley
Cc: Jan Luebbe, Udit Agarwal, Sumit Garg, David Gstir, Eric Biggers,
Franck LENORMAND, Richard Weinberger, James Morris, linux-kernel,
David Howells, linux-security-module, keyrings, linux-crypto,
kernel, linux-integrity, Steffen Trumtrar, Serge E. Hallyn
In-Reply-To: <cover.9fc9298fd9d63553491871d043a18affc2dbc8a8.1626885907.git-series.a.fatoum@pengutronix.de>
Dear trusted key maintainers,
On 21.07.21 18:48, Ahmad Fatoum wrote:
> Series applies on top of
> https://lore.kernel.org/linux-integrity/20210721160258.7024-1-a.fatoum@pengutronix.de/T/#u
>
> v2 -> v3:
> - Split off first Kconfig preparation patch. It fixes a regression,
> so sent that out, so it can be applied separately (Sumit)
> - Split off second key import patch. I'll send that out separately
> as it's a development aid and not required within the CAAM series
> - add MAINTAINERS entry
Gentle ping. I'd appreciate feedback on this series.
Cheers,
Ahmad
>
> v1 -> v2:
> - Added new commit to make trusted key Kconfig option independent
> of TPM and added new Kconfig file for trusted keys
> - Add new commit for importing existing key material
> - Allow users to force use of kernel RNG (Jarkko)
> - Enforce maximum keymod size (Horia)
> - Use append_seq_(in|out)_ptr_intlen instead of append_seq_(in|out)_ptr
> (Horia)
> - Make blobifier handle private to CAAM glue code file (Horia)
> - Extend trusted keys documentation for CAAM
> - Rebased and updated original cover letter:
>
> The Cryptographic Acceleration and Assurance Module (CAAM) is an IP core
> built into many newer i.MX and QorIQ SoCs by NXP.
>
> Its blob mechanism can AES encrypt/decrypt user data using a unique
> never-disclosed device-specific key.
>
> There has been multiple discussions on how to represent this within the kernel:
>
> The Cryptographic Acceleration and Assurance Module (CAAM) is an IP core
> built into many newer i.MX and QorIQ SoCs by NXP.
>
> Its blob mechanism can AES encrypt/decrypt user data using a unique
> never-disclosed device-specific key. There has been multiple
> discussions on how to represent this within the kernel:
>
> - [RFC] crypto: caam - add red blobifier
> Steffen implemented[1] a PoC sysfs driver to start a discussion on how to
> best integrate the blob mechanism.
> Mimi suggested that it could be used to implement trusted keys.
> Trusted keys back then were a TPM-only feature.
>
> - security/keys/secure_key: Adds the secure key support based on CAAM.
> Udit added[2] a new "secure" key type with the CAAM as backend. The key
> material stays within the kernel only.
> Mimi and James agreed that this needs a generic interface, not specific
> to CAAM. Mimi suggested trusted keys. Jan noted that this could serve as
> basis for TEE-backed keys.
>
> - [RFC] drivers: crypto: caam: key: Add caam_tk key type
> Franck added[3] a new "caam_tk" key type based on Udit's work. This time
> it uses CAAM "black blobs" instead of "red blobs", so key material stays
> within the CAAM and isn't exposed to kernel in plaintext.
> James voiced the opinion that there should be just one user-facing generic
> wrap/unwrap key type with multiple possible handlers.
> David suggested trusted keys.
>
> - Introduce TEE based Trusted Keys support
> Sumit reworked[4] trusted keys to support multiple possible backends with
> one chosen at boot time and added a new TEE backend along with TPM.
> This now sits in Jarkko's master branch to be sent out for v5.13
>
> This patch series builds on top of Sumit's rework to have the CAAM as yet another
> trusted key backend.
>
> The CAAM bits are based on Steffen's initial patch from 2015. His work had been
> used in the field for some years now, so I preferred not to deviate too much from it.
>
> This series has been tested with dmcrypt[5] on an i.MX6DL.
>
> Looking forward to your feedback.
>
> Cheers,
> Ahmad
>
> [1]: https://lore.kernel.org/linux-crypto/1447082306-19946-2-git-send-email-s.trumtrar@pengutronix.de/
> [2]: https://lore.kernel.org/linux-integrity/20180723111432.26830-1-udit.agarwal@nxp.com/
> [3]: https://lore.kernel.org/lkml/1551456599-10603-2-git-send-email-franck.lenormand@nxp.com/
> [4]: https://lore.kernel.org/lkml/1604419306-26105-1-git-send-email-sumit.garg@linaro.org/
> [5]: https://lore.kernel.org/linux-integrity/20210122084321.24012-2-a.fatoum@pengutronix.de/
>
> ---
> To: Jarkko Sakkinen <jarkko@kernel.org>
> To: "Horia Geantă" <horia.geanta@nxp.com>
> To: Mimi Zohar <zohar@linux.ibm.com>
> To: Aymen Sghaier <aymen.sghaier@nxp.com>
> To: Herbert Xu <herbert@gondor.apana.org.au>
> To: "David S. Miller" <davem@davemloft.net>
> To: James Bottomley <jejb@linux.ibm.com>
> Cc: David Howells <dhowells@redhat.com>
> Cc: James Morris <jmorris@namei.org>
> Cc: "Serge E. Hallyn" <serge@hallyn.com>
> Cc: Steffen Trumtrar <s.trumtrar@pengutronix.de>
> Cc: Udit Agarwal <udit.agarwal@nxp.com>
> Cc: Jan Luebbe <j.luebbe@pengutronix.de>
> Cc: David Gstir <david@sigma-star.at>
> Cc: Eric Biggers <ebiggers@kernel.org>
> Cc: Richard Weinberger <richard@nod.at>
> Cc: Franck LENORMAND <franck.lenormand@nxp.com>
> Cc: Sumit Garg <sumit.garg@linaro.org>
> Cc: linux-integrity@vger.kernel.org
> Cc: keyrings@vger.kernel.org
> Cc: linux-crypto@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-security-module@vger.kernel.org
>
> Ahmad Fatoum (4):
> KEYS: trusted: allow users to use kernel RNG for key material
> KEYS: trusted: allow trust sources to use kernel RNG for key material
> crypto: caam - add in-kernel interface for blob generator
> KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
>
> Documentation/admin-guide/kernel-parameters.txt | 8 +-
> Documentation/security/keys/trusted-encrypted.rst | 60 +++-
> MAINTAINERS | 9 +-
> drivers/crypto/caam/Kconfig | 3 +-
> drivers/crypto/caam/Makefile | 1 +-
> drivers/crypto/caam/blob_gen.c | 230 +++++++++++++++-
> include/keys/trusted-type.h | 2 +-
> include/keys/trusted_caam.h | 11 +-
> include/soc/fsl/caam-blob.h | 56 ++++-
> security/keys/trusted-keys/Kconfig | 11 +-
> security/keys/trusted-keys/Makefile | 2 +-
> security/keys/trusted-keys/trusted_caam.c | 74 +++++-
> security/keys/trusted-keys/trusted_core.c | 23 +-
> 13 files changed, 477 insertions(+), 13 deletions(-)
> create mode 100644 drivers/crypto/caam/blob_gen.c
> create mode 100644 include/keys/trusted_caam.h
> create mode 100644 include/soc/fsl/caam-blob.h
> create mode 100644 security/keys/trusted-keys/trusted_caam.c
>
> base-commit: 97408d81ed533b953326c580ff2c3f1948b3fcee
>
--
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
* [PATCH v2] fscrypt: support trusted keys
From: Ahmad Fatoum @ 2021-08-06 15:09 UTC (permalink / raw)
To: Theodore Y. Ts'o, Jaegeuk Kim, Eric Biggers
Cc: kernel, Ahmad Fatoum, Jarkko Sakkinen, James Morris,
Serge E. Hallyn, James Bottomley, Mimi Zohar, Sumit Garg,
David Howells, linux-fscrypt, linux-crypto, linux-integrity,
linux-security-module, keyrings, linux-kernel
Kernel trusted keys don't require userspace knowledge of the raw key
material and instead export a sealed blob, which can be persisted to
unencrypted storage. Userspace can then load this blob into the kernel,
where it's unsealed and from there on usable for kernel crypto.
This is incompatible with fscrypt, where userspace is supposed to supply
the raw key material. For TPMs, a work around is to do key unsealing in
userspace, but this may not be feasible for other trusted key backends.
Make it possible to benefit from both fscrypt and trusted key sealing
by extending fscrypt_add_key_arg::key_id to hold either the ID of a
fscrypt-provisioning or a trusted key.
A non fscrypt-provisioning key_id was so far prohibited, so additionally
allowing trusted keys won't break backwards compatibility.
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
Tested with:
https://github.com/google/fscryptctl/pull/23
v1 here:
https://lore.kernel.org/linux-fscrypt/20210727144349.11215-1-a.fatoum@pengutronix.de/T/#u
v1 -> v2:
- Drop encrypted key support and key_extract_material
- Use key_id instead of repurposing raw (Eric)
- Shift focus to trusted key sealing for non-TPM as a rationale
why this integration is worthwhile (Eric)
- Extend documentation with rationale on why one would
use trusted keys and warn about trusted key reuse
To: "Theodore Y. Ts'o" <tytso@mit.edu>
To: Jaegeuk Kim <jaegeuk@kernel.org>
To: Eric Biggers <ebiggers@kernel.org>
Cc: Jarkko Sakkinen <jarkko@kernel.org>
Cc: James Morris <jmorris@namei.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Mimi Zohar <zohar@linux.ibm.com>
Cc: Sumit Garg <sumit.garg@linaro.org>
Cc: David Howells <dhowells@redhat.com>
Cc: linux-fscrypt@vger.kernel.org
Cc: linux-crypto@vger.kernel.org
Cc: linux-integrity@vger.kernel.org
Cc: linux-security-module@vger.kernel.org
Cc: keyrings@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
Documentation/filesystems/fscrypt.rst | 31 ++++++++++++++-----
fs/crypto/keyring.c | 43 +++++++++++++++++++--------
2 files changed, 54 insertions(+), 20 deletions(-)
diff --git a/Documentation/filesystems/fscrypt.rst b/Documentation/filesystems/fscrypt.rst
index 44b67ebd6e40..c1811fa4285a 100644
--- a/Documentation/filesystems/fscrypt.rst
+++ b/Documentation/filesystems/fscrypt.rst
@@ -734,23 +734,40 @@ as follows:
- ``key_id`` is 0 if the raw key is given directly in the ``raw``
field. Otherwise ``key_id`` is the ID of a Linux keyring key of
- type "fscrypt-provisioning" whose payload is
+ type "fscrypt-provisioning" or "trusted":
+ "fscrypt-provisioning" keys have a payload of
struct fscrypt_provisioning_key_payload whose ``raw`` field contains
the raw key and whose ``type`` field matches ``key_spec.type``.
Since ``raw`` is variable-length, the total size of this key's
payload must be ``sizeof(struct fscrypt_provisioning_key_payload)``
- plus the raw key size. The process must have Search permission on
- this key.
-
- Most users should leave this 0 and specify the raw key directly.
- The support for specifying a Linux keyring key is intended mainly to
- allow re-adding keys after a filesystem is unmounted and re-mounted,
+ plus the raw key size.
+ For "trusted" keys, the payload is directly taken as the raw key.
+
+ The process must have Search permission on this key.
+
+ Most users leave this 0 and specify the raw key directly.
+ "trusted" keys are useful to leverage kernel support for sealing
+ and unsealing key material. Sealed keys can be persisted to
+ unencrypted storage and later be used to decrypt the file system
+ without requiring userspace to have knowledge of the raw key
+ material.
+ "fscrypt-provisioning" key support is intended mainly to allow
+ re-adding keys after a filesystem is unmounted and re-mounted,
without having to store the raw keys in userspace memory.
- ``raw`` is a variable-length field which must contain the actual
key, ``raw_size`` bytes long. Alternatively, if ``key_id`` is
nonzero, then this field is unused.
+.. note::
+
+ Users should take care not to reuse the fscrypt key material with
+ different ciphers or in multiple contexts as this may make it
+ easier to deduce the key.
+ This also applies when the key material is supplied indirectly
+ via a kernel trusted key. In this case, the trusted key should
+ perferably be used only in a single context.
+
For v2 policy keys, the kernel keeps track of which user (identified
by effective user ID) added the key, and only allows the key to be
removed by that user --- or by "root", if they use
diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c
index 0b3ffbb4faf4..721f5da51416 100644
--- a/fs/crypto/keyring.c
+++ b/fs/crypto/keyring.c
@@ -20,6 +20,7 @@
#include <crypto/skcipher.h>
#include <linux/key-type.h>
+#include <keys/trusted-type.h>
#include <linux/random.h>
#include <linux/seq_file.h>
@@ -577,28 +578,44 @@ static int get_keyring_key(u32 key_id, u32 type,
key_ref_t ref;
struct key *key;
const struct fscrypt_provisioning_key_payload *payload;
- int err;
+ int err = 0;
ref = lookup_user_key(key_id, 0, KEY_NEED_SEARCH);
if (IS_ERR(ref))
return PTR_ERR(ref);
key = key_ref_to_ptr(ref);
- if (key->type != &key_type_fscrypt_provisioning)
- goto bad_key;
- payload = key->payload.data[0];
+ if (key->type == &key_type_fscrypt_provisioning) {
+ payload = key->payload.data[0];
- /* Don't allow fscrypt v1 keys to be used as v2 keys and vice versa. */
- if (payload->type != type)
- goto bad_key;
+ /* Don't allow fscrypt v1 keys to be used as v2 keys and vice versa. */
+ if (payload->type != type) {
+ err = -EKEYREJECTED;
+ goto out_put;
+ }
- secret->size = key->datalen - sizeof(*payload);
- memcpy(secret->raw, payload->raw, secret->size);
- err = 0;
- goto out_put;
+ secret->size = key->datalen - sizeof(*payload);
+ memcpy(secret->raw, payload->raw, secret->size);
+ } else if (IS_REACHABLE(CONFIG_TRUSTED_KEYS) && key->type == &key_type_trusted) {
+ struct trusted_key_payload *tkp;
+
+ /* avoid reseal changing payload while we memcpy key */
+ down_read(&key->sem);
+ tkp = key->payload.data[0];
+ if (!tkp || tkp->key_len < FSCRYPT_MIN_KEY_SIZE ||
+ tkp->key_len > FSCRYPT_MAX_KEY_SIZE) {
+ up_read(&key->sem);
+ err = -EINVAL;
+ goto out_put;
+ }
+
+ secret->size = tkp->key_len;
+ memcpy(secret->raw, tkp->key, secret->size);
+ up_read(&key->sem);
+ } else {
+ err = -EKEYREJECTED;
+ }
-bad_key:
- err = -EKEYREJECTED;
out_put:
key_ref_put(ref);
return err;
--
2.30.2
^ permalink raw reply related
* Re: [PATCH RFC v2 10/12] KEYS: link system_trusted_keys to mok_trusted_keys
From: Eric Snowberg @ 2021-08-06 15:00 UTC (permalink / raw)
To: Mimi Zohar
Cc: keyrings, linux-integrity, David Howells, David Woodhouse,
Herbert Xu, David S . Miller, Jarkko Sakkinen, James Morris,
Serge E . Hallyn, keescook, gregkh, torvalds, scott.branden,
weiyongjun1, nayna, ebiggers, ardb, Lakshmi Ramasubramanian,
lszubowi, linux-kernel, linux-crypto, linux-security-module,
James.Bottomley, pjones, glin, konrad.wilk@oracle.com
In-Reply-To: <d85bfe88bb4abd06e47a36743f53d0610da0a259.camel@linux.ibm.com>
> On Aug 5, 2021, at 9:19 PM, Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Thu, 2021-08-05 at 19:29 -0600, Eric Snowberg wrote:
>
>>> From the thread discussion on 00/12:
>>>
>>> Only the builtin keys should ever be on the builtin keyring. The
>>> builtin keyring would need to be linked to the mok keyring. But in the
>>> secondary keyring case, the mok keyring would be linked to the
>>> secondary keyring, similar to how the builtin keyring is linked to the
>>> secondary keyring.
>>>
>>> if (key_link(secondary_trusted_keys, builtin_trusted_keys) < 0)
>>> panic("Can't link trusted keyrings\n");
>>
>>
>> This part is confusing me though.
>>
>> Here are some of the tests I’m performing with the current series:
>>
>> Initial setup:
>> Create and enroll my own key into the MOK.
>> Sign a kernel, kernel module and IMA key with my new CA key.
>> Boot with lockdown enabled (to enforce sig validation).
>>
>> Kernel built with CONFIG_SECONDARY_TRUSTED_KEYRING=y
>>
>> $ keyctl show %:.secondary_trusted_keys
>> Keyring
>> 530463486 ---lswrv 0 0 keyring: .secondary_trusted_keys
>> 411466727 ---lswrv 0 0 \_ keyring: .builtin_trusted_keys
>> 979167715 ---lswrv 0 0 | \_ asymmetric: Build time autogenerated kernel key: 07a56e29cfa1e21379aff2c522efff7d1963202a
>> 534573591 ---lswrv 0 0 | \_ asymmetric: Oracle-CA: Oracle certificate signing key: aeefb4bfde095cacaabff81dd266974b1b4e23b8
>> 968109018 ---lswrv 0 0 \_ keyring: .mok
>> 857795115 ---lswrv 0 0 \_ asymmetric: Erics-CA: UEK signing key: 9bfa6860483aa46bd83f7fa1289d9fc35799e93b
>>
>> With this setup I can:
>> * load a kernel module signed with my CA key
>> * run "kexec -ls" with the kernel signed with my CA key
>> * run "kexec -ls" with a kernel signed by a key in the platform keyring
>> * load another key into the secondary trusted keyring that is signed by my CA key
>> * load a key into the ima keyring, signed by my CA key
>>
>> Kernel built without CONFIG_SECONDARY_TRUSTED_KEYRING defined
>>
>> $ keyctl show %:.builtin_trusted_keys
>> Keyring
>> 812785375 ---lswrv 0 0 keyring: .builtin_trusted_keys
>> 455418681 ---lswrv 0 0 \_ keyring: .mok
>> 910809006 ---lswrv 0 0 | \_ asymmetric: Erics-CA: UEK signing key: 9bfa6860483aa46bd83f7fa1289d9fc35799e93b
>> 115345009 ---lswrv 0 0 \_ asymmetric: Oracle-CA: Oracle certificate signing key: aeefb4bfde095cacaabff81dd266974b1b4e23b8
>> 513131506 ---lswrv 0 0 \_ asymmetric: Build time autogenerated kernel key: 22353509f203b55b84f15d0aadeddc134b646185
>>
>> With this setup I can:
>> * load a kernel module signed with my CA key
>> * run "kexec -ls" with the kernel signed with my CA key
>> * run "kexec -ls" with a kernel signed by a key in the platform keyring
>> * load a key into the ima keyring, signed by my CA key
>>
>> So why would the linking need to be switched? Is there a test I’m
>> missing? Thanks.
>
> It's a question of semantics. The builtin keyring name is self
> describing. It should only contain the keys compiled into the kernel
> or inserted post build into the reserved memory. Not only the kernel
> uses the builtin keyring, but userspace may as well[1]. Other users of
> the builtin keyring might not want to trust the mok keyring as well.
Should this feature only work with kernels built with
CONFIG_SECONDARY_TRUSTED_KEYRING defined? If so, I could drop support in
the next version for kernels built without it.
^ permalink raw reply
* Re: [PATCH v3 1/2] tpm: ibmvtpm: Rename tpm_process_cmd to tpm_status and define flag
From: Stefan Berger @ 2021-08-06 12:08 UTC (permalink / raw)
To: Jarkko Sakkinen, Stefan Berger
Cc: peterhuewe, jgg, linux-integrity, linux-security-module,
linux-kernel, Nayna Jain, George Wilson
In-Reply-To: <20210806112557.y7q2av6pk7r4xorm@kernel.org>
On 8/6/21 7:25 AM, Jarkko Sakkinen wrote:
> On Thu, Aug 05, 2021 at 05:52:55PM -0400, Stefan Berger wrote:
>> From: Stefan Berger <stefanb@linux.ibm.com>
>>
>> Rename the field tpm_processing_cmd to tpm_status in ibmvtpm_dev and set
>> the TPM_STATUS_BUSY flag while the vTPM is busy processing a command.
>>
>> Fixes: 6674ff145eef ("tpm_ibmvtpm: properly handle interrupted packet receptions")
>> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
>> Cc: Nayna Jain <nayna@linux.ibm.com>
>> Cc: George Wilson <gcwilson@linux.ibm.com>
> Please put the bug fix first because otherwise it will be dependent of this
> patch, which is bad thing when it comes to backporting.
Yes, and that's why I have this one here also with a Fix tag. I
basically don't want to logically '&' with the 'true' flag but want this
TPM_STATUS_BUSY flag first.
Stefan
>
> /Jarkko
^ permalink raw reply
* Re: [PATCH v3 1/2] tpm: ibmvtpm: Rename tpm_process_cmd to tpm_status and define flag
From: Jarkko Sakkinen @ 2021-08-06 11:25 UTC (permalink / raw)
To: Stefan Berger
Cc: peterhuewe, jgg, linux-integrity, linux-security-module,
linux-kernel, Stefan Berger, Nayna Jain, George Wilson
In-Reply-To: <20210805215256.1293987-2-stefanb@linux.vnet.ibm.com>
On Thu, Aug 05, 2021 at 05:52:55PM -0400, Stefan Berger wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>
>
> Rename the field tpm_processing_cmd to tpm_status in ibmvtpm_dev and set
> the TPM_STATUS_BUSY flag while the vTPM is busy processing a command.
>
> Fixes: 6674ff145eef ("tpm_ibmvtpm: properly handle interrupted packet receptions")
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
> Cc: Nayna Jain <nayna@linux.ibm.com>
> Cc: George Wilson <gcwilson@linux.ibm.com>
Please put the bug fix first because otherwise it will be dependent of this
patch, which is bad thing when it comes to backporting.
/Jarkko
^ permalink raw reply
* Re: [RFC PATCH v1 0/4] keys: introduce key_extract_material helper
From: Ahmad Fatoum @ 2021-08-06 10:53 UTC (permalink / raw)
To: David Howells, Jarkko Sakkinen, James Morris, Serge E. Hallyn,
Alasdair Kergon, Mike Snitzer, dm-devel, Song Liu,
Richard Weinberger
Cc: linux-kernel, linux-raid, linux-security-module, keyrings,
linux-mtd, kernel, linux-integrity
In-Reply-To: <cover.b2fdd70b830d12853b12a12e32ceb0c8162c1346.1626945419.git-series.a.fatoum@pengutronix.de>
Hello everyone,
On 22.07.21 11:17, Ahmad Fatoum wrote:
> While keys of differing type have a common struct key definition, there is
> no common scheme to the payload and key material extraction differs.
>
> For kernel functionality that supports different key types,
> this means duplicated code for key material extraction and because key type
> is discriminated by a pointer to a global, users need to replicate
> reachability checks as well, so builtin code doesn't depend on a key
> type symbol offered by a module.
>
> Make this easier by adding a common helper with initial support for
> user, logon, encrypted and trusted keys.
>
> This series contains two example of its use: dm-crypt uses it to reduce
> boilerplate and ubifs authentication uses it to gain support for trusted
> and encrypted keys alongside the already supported logon keys.
>
> Looking forward to your feedback,
@Mike, Aliasdair: Do you think of key_extract_material as an improvement?
Does someone share the opinion that the helper is useful or should I drop
it and just send out the ubifs auth patch seperately?
Cheers,
Ahmad
> Ahmad
>
> ---
> To: David Howells <dhowells@redhat.com>
> To: Jarkko Sakkinen <jarkko@kernel.org>
> To: James Morris <jmorris@namei.org>
> To: "Serge E. Hallyn" <serge@hallyn.com>
> To: Alasdair Kergon <agk@redhat.com>
> To: Mike Snitzer <snitzer@redhat.com>
> To: dm-devel@redhat.com
> To: Song Liu <song@kernel.org>
> To: Richard Weinberger <richard@nod.at>
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-raid@vger.kernel.org
> Cc: linux-integrity@vger.kernel.org
> Cc: keyrings@vger.kernel.org
> Cc: linux-mtd@lists.infradead.org
> Cc: linux-security-module@vger.kernel.org
>
> Ahmad Fatoum (4):
> keys: introduce key_extract_material helper
> dm: crypt: use new key_extract_material helper
> ubifs: auth: remove never hit key type error check
> ubifs: auth: consult encrypted and trusted keys if no logon key was found
>
> Documentation/filesystems/ubifs.rst | 2 +-
> drivers/md/dm-crypt.c | 65 ++++--------------------------
> fs/ubifs/auth.c | 25 +++++-------
> include/linux/key.h | 45 +++++++++++++++++++++-
> security/keys/key.c | 40 ++++++++++++++++++-
> 5 files changed, 107 insertions(+), 70 deletions(-)
>
> base-commit: 2734d6c1b1a089fb593ef6a23d4b70903526fe0c
>
--
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
* [PATCH 2/3] security/security: get rid of a duplicated condition
From: Dmitry Mastykin @ 2021-08-06 7:02 UTC (permalink / raw)
To: casey, linux-security-module; +Cc: dmastykin, akovalenko
In-Reply-To: <20210806070245.26338-1-dmastykin@astralinux.ru>
Just remove a typo: the same if() was duplicated.
Signed-off-by: Dmitry Mastykin <dmastykin@astralinux.ru>
---
security/security.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/security/security.c b/security/security.c
index fd14064e9106..1ab6f56a93b6 100644
--- a/security/security.c
+++ b/security/security.c
@@ -2930,8 +2930,6 @@ int security_audit_rule_match(struct lsmblob *blob, u32 field, u32 op,
continue;
if (lsmrule[hp->lsmid->slot] == NULL)
continue;
- if (lsmrule[hp->lsmid->slot] == NULL)
- continue;
rc = hp->hook.audit_rule_match(blob->secid[hp->lsmid->slot],
field, op,
lsmrule[hp->lsmid->slot]);
--
2.11.0
^ permalink raw reply related
* lsm-stacking: fix broken lsm audit
From: Dmitry Mastykin @ 2021-08-06 7:02 UTC (permalink / raw)
To: casey, linux-security-module; +Cc: dmastykin, akovalenko
Hello,
These patches address the problem of not processing LSM audit rules.
Problem was introduced in lsm stacking series.
These patches are for cschaufler/lsm-stacking repository branch stack-5.10-rc4-v23
Some UBUNTU distributions have also this problem.
Kind regards,
Dmitry Mastykin
^ permalink raw reply
* [PATCH 3/3] kernel/auditsc: use correct blob for files in security_audit_rule_match call
From: Dmitry Mastykin @ 2021-08-06 7:02 UTC (permalink / raw)
To: casey, linux-security-module; +Cc: dmastykin, akovalenko
In-Reply-To: <20210806070245.26338-1-dmastykin@astralinux.ru>
File audit didn't work. Uninitialized local structure was passed
to security_audit_rule_match instead of audit_names oblob.
Signed-off-by: Dmitry Mastykin <dmastykin@astralinux.ru>
---
kernel/auditsc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index c314533dd220..3b1afdb5cda4 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -689,14 +689,14 @@ static int audit_filter_rules(struct task_struct *tsk,
/* Find files that match */
if (name) {
result = security_audit_rule_match(
- &blob,
+ &name->oblob,
f->type,
f->op,
f->lsm_rules);
} else if (ctx) {
list_for_each_entry(n, &ctx->names_list, list) {
if (security_audit_rule_match(
- &blob,
+ &n->oblob,
f->type,
f->op,
f->lsm_rules)) {
--
2.11.0
^ permalink raw reply related
* [PATCH 1/3] security/security: remove extra address-of in hook.audit_rule_match call
From: Dmitry Mastykin @ 2021-08-06 7:02 UTC (permalink / raw)
To: casey, linux-security-module; +Cc: dmastykin, akovalenko
In-Reply-To: <20210806070245.26338-1-dmastykin@astralinux.ru>
Wrong address was passed to audit_rule_match hooks instead of
rule's address.
Signed-off-by: Dmitry Mastykin <dmastykin@astralinux.ru>
---
security/security.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/security/security.c b/security/security.c
index e33c8ccc06a0..fd14064e9106 100644
--- a/security/security.c
+++ b/security/security.c
@@ -2934,7 +2934,7 @@ int security_audit_rule_match(struct lsmblob *blob, u32 field, u32 op,
continue;
rc = hp->hook.audit_rule_match(blob->secid[hp->lsmid->slot],
field, op,
- &lsmrule[hp->lsmid->slot]);
+ lsmrule[hp->lsmid->slot]);
if (rc)
return rc;
}
--
2.11.0
^ permalink raw reply related
* Re: [PATCH RFC v2 10/12] KEYS: link system_trusted_keys to mok_trusted_keys
From: Mimi Zohar @ 2021-08-06 3:19 UTC (permalink / raw)
To: Eric Snowberg
Cc: keyrings, linux-integrity, David Howells, David Woodhouse,
Herbert Xu, David S . Miller, Jarkko Sakkinen, James Morris,
Serge E . Hallyn, keescook, gregkh, torvalds, scott.branden,
weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
linux-kernel, linux-crypto, linux-security-module,
James.Bottomley, pjones, glin, konrad.wilk@oracle.com
In-Reply-To: <44ADB68B-4310-462B-96A8-2F69759BA2D8@oracle.com>
On Thu, 2021-08-05 at 19:29 -0600, Eric Snowberg wrote:
> > From the thread discussion on 00/12:
> >
> > Only the builtin keys should ever be on the builtin keyring. The
> > builtin keyring would need to be linked to the mok keyring. But in the
> > secondary keyring case, the mok keyring would be linked to the
> > secondary keyring, similar to how the builtin keyring is linked to the
> > secondary keyring.
> >
> > if (key_link(secondary_trusted_keys, builtin_trusted_keys) < 0)
> > panic("Can't link trusted keyrings\n");
>
>
> This part is confusing me though.
>
> Here are some of the tests I’m performing with the current series:
>
> Initial setup:
> Create and enroll my own key into the MOK.
> Sign a kernel, kernel module and IMA key with my new CA key.
> Boot with lockdown enabled (to enforce sig validation).
>
> Kernel built with CONFIG_SECONDARY_TRUSTED_KEYRING=y
>
> $ keyctl show %:.secondary_trusted_keys
> Keyring
> 530463486 ---lswrv 0 0 keyring: .secondary_trusted_keys
> 411466727 ---lswrv 0 0 \_ keyring: .builtin_trusted_keys
> 979167715 ---lswrv 0 0 | \_ asymmetric: Build time autogenerated kernel key: 07a56e29cfa1e21379aff2c522efff7d1963202a
> 534573591 ---lswrv 0 0 | \_ asymmetric: Oracle-CA: Oracle certificate signing key: aeefb4bfde095cacaabff81dd266974b1b4e23b8
> 968109018 ---lswrv 0 0 \_ keyring: .mok
> 857795115 ---lswrv 0 0 \_ asymmetric: Erics-CA: UEK signing key: 9bfa6860483aa46bd83f7fa1289d9fc35799e93b
>
> With this setup I can:
> * load a kernel module signed with my CA key
> * run "kexec -ls" with the kernel signed with my CA key
> * run "kexec -ls" with a kernel signed by a key in the platform keyring
> * load another key into the secondary trusted keyring that is signed by my CA key
> * load a key into the ima keyring, signed by my CA key
>
> Kernel built without CONFIG_SECONDARY_TRUSTED_KEYRING defined
>
> $ keyctl show %:.builtin_trusted_keys
> Keyring
> 812785375 ---lswrv 0 0 keyring: .builtin_trusted_keys
> 455418681 ---lswrv 0 0 \_ keyring: .mok
> 910809006 ---lswrv 0 0 | \_ asymmetric: Erics-CA: UEK signing key: 9bfa6860483aa46bd83f7fa1289d9fc35799e93b
> 115345009 ---lswrv 0 0 \_ asymmetric: Oracle-CA: Oracle certificate signing key: aeefb4bfde095cacaabff81dd266974b1b4e23b8
> 513131506 ---lswrv 0 0 \_ asymmetric: Build time autogenerated kernel key: 22353509f203b55b84f15d0aadeddc134b646185
>
> With this setup I can:
> * load a kernel module signed with my CA key
> * run "kexec -ls" with the kernel signed with my CA key
> * run "kexec -ls" with a kernel signed by a key in the platform keyring
> * load a key into the ima keyring, signed by my CA key
>
> So why would the linking need to be switched? Is there a test I’m
> missing? Thanks.
It's a question of semantics. The builtin keyring name is self
describing. It should only contain the keys compiled into the kernel
or inserted post build into the reserved memory. Not only the kernel
uses the builtin keyring, but userspace may as well[1]. Other users of
the builtin keyring might not want to trust the mok keyring as well.
thanks,
Mimi
[1] Refer to Mat Martineau's LSS 2019 talk titled "Using and
Implementing Keyring Restrictions in Userspace".
^ permalink raw reply
* Re: [PATCH RFC v2 10/12] KEYS: link system_trusted_keys to mok_trusted_keys
From: Eric Snowberg @ 2021-08-06 1:29 UTC (permalink / raw)
To: Mimi Zohar
Cc: keyrings, linux-integrity, David Howells, David Woodhouse,
Herbert Xu, David S . Miller, Jarkko Sakkinen, James Morris,
Serge E . Hallyn, keescook, gregkh, torvalds, scott.branden,
weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
linux-kernel, linux-crypto, linux-security-module,
James.Bottomley, pjones, glin, konrad.wilk@oracle.com
In-Reply-To: <6c751dadf4ce7385d0391ea26f1c7e4e910219e0.camel@linux.ibm.com>
> On Aug 5, 2021, at 7:58 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Mon, 2021-07-26 at 13:13 -0400, Eric Snowberg wrote:
>
>> diff --git a/certs/system_keyring.c b/certs/system_keyring.c
>> index dcaf74102ab2..b27ae30eaadc 100644
>> --- a/certs/system_keyring.c
>> +++ b/certs/system_keyring.c
>> @@ -45,6 +45,15 @@ int restrict_link_by_builtin_trusted(struct key *dest_keyring,
>> const union key_payload *payload,
>> struct key *restriction_key)
>> {
>> + /* If the secondary trusted keyring is not enabled, we may link
>> + * through to the mok keyring and the search may follow that link.
>> + */
>
> Refer to section "8) Commenting" of Documentation/process/coding-
> style.rst for the format of multi line comments.
Sure, I’ll fix this in the next version.
>> + if (mok_trusted_keys && type == &key_type_keyring &&
>> + dest_keyring == builtin_trusted_keys &&
>> + payload == &mok_trusted_keys->payload)
>> + /* Allow the mok keyring to be added to the builtin */
>> + return 0;
>> +
>
> Unless you're changing the meaning of the restriction, then a new
> restriction needs to be defined. In this case, please don't change the
> meaning of restrict_link_by_builtin_trusted(). Instead define a new
> restriction named restrict_link_by_builtin_and_ca_trusted().
Along with this
>> return restrict_link_by_signature(dest_keyring, type, payload,
>> builtin_trusted_keys);
>> }
>> @@ -91,6 +100,15 @@ int restrict_link_by_builtin_and_secondary_trusted(
>> /* Allow the builtin keyring to be added to the secondary */
>> return 0;
>>
>> + /* If we have a secondary trusted keyring, it may contain a link
>> + * through to the mok keyring and the search may follow that link.
>> + */
>> + if (mok_trusted_keys && type == &key_type_keyring &&
>> + dest_keyring == secondary_trusted_keys &&
>> + payload == &mok_trusted_keys->payload)
>> + /* Allow the mok keyring to be added to the secondary */
>> + return 0;
>> +
>
> Similarly here, please define a new restriction maybe named
> restrict_link_by_builtin_secondary_and_ca_trusted(). To avoid code
> duplication, the new restriction could be a wrapper around the existing
> function.
and this too.
>
>> return restrict_link_by_signature(dest_keyring, type, payload,
>> secondary_trusted_keys);
>> }
>> @@ -321,5 +339,8 @@ void __init set_platform_trusted_keys(struct key *keyring)
>> void __init set_mok_trusted_keys(struct key *keyring)
>> {
>> mok_trusted_keys = keyring;
>> +
>> + if (key_link(system_trusted_keys, mok_trusted_keys) < 0)
>> + panic("Can't link (mok) trusted keyrings\n");
>> }
>
> From the thread discussion on 00/12:
>
> Only the builtin keys should ever be on the builtin keyring. The
> builtin keyring would need to be linked to the mok keyring. But in the
> secondary keyring case, the mok keyring would be linked to the
> secondary keyring, similar to how the builtin keyring is linked to the
> secondary keyring.
>
> if (key_link(secondary_trusted_keys, builtin_trusted_keys) < 0)
> panic("Can't link trusted keyrings\n");
This part is confusing me though.
Here are some of the tests I’m performing with the current series:
Initial setup:
Create and enroll my own key into the MOK.
Sign a kernel, kernel module and IMA key with my new CA key.
Boot with lockdown enabled (to enforce sig validation).
Kernel built with CONFIG_SECONDARY_TRUSTED_KEYRING=y
$ keyctl show %:.secondary_trusted_keys
Keyring
530463486 ---lswrv 0 0 keyring: .secondary_trusted_keys
411466727 ---lswrv 0 0 \_ keyring: .builtin_trusted_keys
979167715 ---lswrv 0 0 | \_ asymmetric: Build time autogenerated kernel key: 07a56e29cfa1e21379aff2c522efff7d1963202a
534573591 ---lswrv 0 0 | \_ asymmetric: Oracle-CA: Oracle certificate signing key: aeefb4bfde095cacaabff81dd266974b1b4e23b8
968109018 ---lswrv 0 0 \_ keyring: .mok
857795115 ---lswrv 0 0 \_ asymmetric: Erics-CA: UEK signing key: 9bfa6860483aa46bd83f7fa1289d9fc35799e93b
With this setup I can:
* load a kernel module signed with my CA key
* run "kexec -ls" with the kernel signed with my CA key
* run "kexec -ls" with a kernel signed by a key in the platform keyring
* load another key into the secondary trusted keyring that is signed by my CA key
* load a key into the ima keyring, signed by my CA key
Kernel built without CONFIG_SECONDARY_TRUSTED_KEYRING defined
$ keyctl show %:.builtin_trusted_keys
Keyring
812785375 ---lswrv 0 0 keyring: .builtin_trusted_keys
455418681 ---lswrv 0 0 \_ keyring: .mok
910809006 ---lswrv 0 0 | \_ asymmetric: Erics-CA: UEK signing key: 9bfa6860483aa46bd83f7fa1289d9fc35799e93b
115345009 ---lswrv 0 0 \_ asymmetric: Oracle-CA: Oracle certificate signing key: aeefb4bfde095cacaabff81dd266974b1b4e23b8
513131506 ---lswrv 0 0 \_ asymmetric: Build time autogenerated kernel key: 22353509f203b55b84f15d0aadeddc134b646185
With this setup I can:
* load a kernel module signed with my CA key
* run "kexec -ls" with the kernel signed with my CA key
* run "kexec -ls" with a kernel signed by a key in the platform keyring
* load a key into the ima keyring, signed by my CA key
So why would the linking need to be switched? Is there a test I’m
missing? Thanks.
^ permalink raw reply
* [PATCH v3 2/2] tpm: ibmvtpm: Avoid error message when process gets signal while waiting
From: Stefan Berger @ 2021-08-05 21:52 UTC (permalink / raw)
To: jarkko
Cc: peterhuewe, jgg, linux-integrity, linux-security-module,
linux-kernel, Stefan Berger, Nayna Jain, George Wilson,
Nageswara R Sastry
In-Reply-To: <20210805215256.1293987-1-stefanb@linux.vnet.ibm.com>
From: Stefan Berger <stefanb@linux.ibm.com>
When rngd is run as root then lots of these types of message will appear
in the kernel log if the TPM has been configured to provide random bytes:
[ 7406.275163] tpm tpm0: tpm_transmit: tpm_recv: error -4
The issue is caused by the following call that is interrupted while
waiting for the TPM's response.
sig = wait_event_interruptible(ibmvtpm->wq,
(ibmvtpm->tpm_status & TPM_STATUS_BUSY) == 0);
Rather than waiting for the response in the low level driver, have it use
the polling loop in tpm_try_transmit() that uses a command's duration to
poll until a result has been returned by the TPM, thus ending when the
timeout has occurred but not responding to signals and ctrl-c anymore. To
stay in this polling loop extend tpm_ibmvtpm_status() to return
TPM_STATUS_BUSY for as long as the vTPM is busy. Since the loop requires
the TPM's timeouts, get them now using tpm_get_timeouts() after setting
the TPM2 version flag on the chip.
To recreat the resolved issue start rngd like this:
sudo rngd -r /dev/hwrng -t
sudo rngd -r /dev/tpm0 -t
Link: https://bugzilla.redhat.com/show_bug.cgi?id=1981473
Fixes: 6674ff145eef ("tpm_ibmvtpm: properly handle interrupted packet receptions")
Cc: Nayna Jain <nayna@linux.ibm.com>
Cc: George Wilson <gcwilson@linux.ibm.com>
Reported-by: Nageswara R Sastry <rnsastry@linux.ibm.com>
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
v3L
- split for renaming of tpm_processing_cmd
v2:
- reworded commit text
---
drivers/char/tpm/tpm_ibmvtpm.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
index cd6457061a2e..5d795866b483 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.c
+++ b/drivers/char/tpm/tpm_ibmvtpm.c
@@ -106,18 +106,12 @@ static int tpm_ibmvtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count)
{
struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(&chip->dev);
u16 len;
- int sig;
if (!ibmvtpm->rtce_buf) {
dev_err(ibmvtpm->dev, "ibmvtpm device is not ready\n");
return 0;
}
- sig = wait_event_interruptible(ibmvtpm->wq,
- (ibmvtpm->tpm_status & TPM_STATUS_BUSY) == 0);
- if (sig)
- return -EINTR;
-
len = ibmvtpm->res_len;
if (count < len) {
@@ -271,7 +265,9 @@ static void tpm_ibmvtpm_cancel(struct tpm_chip *chip)
static u8 tpm_ibmvtpm_status(struct tpm_chip *chip)
{
- return 0;
+ struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(&chip->dev);
+
+ return ibmvtpm->tpm_status;
}
/**
@@ -459,7 +455,7 @@ static const struct tpm_class_ops tpm_ibmvtpm = {
.send = tpm_ibmvtpm_send,
.cancel = tpm_ibmvtpm_cancel,
.status = tpm_ibmvtpm_status,
- .req_complete_mask = 0,
+ .req_complete_mask = TPM_STATUS_BUSY,
.req_complete_val = 0,
.req_canceled = tpm_ibmvtpm_req_canceled,
};
@@ -690,8 +686,15 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
goto init_irq_cleanup;
}
- if (!strcmp(id->compat, "IBM,vtpm20")) {
+
+ if (!strcmp(id->compat, "IBM,vtpm20"))
chip->flags |= TPM_CHIP_FLAG_TPM2;
+
+ rc = tpm_get_timeouts(chip);
+ if (rc)
+ goto init_irq_cleanup;
+
+ if (chip->flags & TPM_CHIP_FLAG_TPM2) {
rc = tpm2_get_cc_attrs_tbl(chip);
if (rc)
goto init_irq_cleanup;
--
2.31.1
^ permalink raw reply related
* [PATCH v3 0/2] ibmvtpm: Avoid error message when process gets signal while waiting
From: Stefan Berger @ 2021-08-05 21:52 UTC (permalink / raw)
To: jarkko
Cc: peterhuewe, jgg, linux-integrity, linux-security-module,
linux-kernel, Stefan Berger
From: Stefan Berger <stefanb@linux.ibm.com>
This series of patches fixes an issue related to the ibmvtpm driver causing
unnecessary kernel log messages when a process is interrupted while waiting
for the TPM to respond. The aborted wait causes the core TPM driver to emit
the log message. The solution is to convert the driver to use the normal
polling loop to wait for TPM responses.
Stefan
v3:
- Split into two patches
Stefan Berger (2):
tpm: ibmvtpm: Rename tpm_process_cmd to tpm_status and define flag
tpm: ibmvtpm: Avoid error message when process gets signal while
waiting
drivers/char/tpm/tpm_ibmvtpm.c | 31 ++++++++++++++++++-------------
drivers/char/tpm/tpm_ibmvtpm.h | 3 ++-
2 files changed, 20 insertions(+), 14 deletions(-)
--
2.31.1
^ permalink raw reply
* [PATCH v3 1/2] tpm: ibmvtpm: Rename tpm_process_cmd to tpm_status and define flag
From: Stefan Berger @ 2021-08-05 21:52 UTC (permalink / raw)
To: jarkko
Cc: peterhuewe, jgg, linux-integrity, linux-security-module,
linux-kernel, Stefan Berger, Nayna Jain, George Wilson
In-Reply-To: <20210805215256.1293987-1-stefanb@linux.vnet.ibm.com>
From: Stefan Berger <stefanb@linux.ibm.com>
Rename the field tpm_processing_cmd to tpm_status in ibmvtpm_dev and set
the TPM_STATUS_BUSY flag while the vTPM is busy processing a command.
Fixes: 6674ff145eef ("tpm_ibmvtpm: properly handle interrupted packet receptions")
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Cc: Nayna Jain <nayna@linux.ibm.com>
Cc: George Wilson <gcwilson@linux.ibm.com>
---
drivers/char/tpm/tpm_ibmvtpm.c | 14 ++++++++------
drivers/char/tpm/tpm_ibmvtpm.h | 3 ++-
2 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
index 903604769de9..cd6457061a2e 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.c
+++ b/drivers/char/tpm/tpm_ibmvtpm.c
@@ -113,7 +113,8 @@ static int tpm_ibmvtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count)
return 0;
}
- sig = wait_event_interruptible(ibmvtpm->wq, !ibmvtpm->tpm_processing_cmd);
+ sig = wait_event_interruptible(ibmvtpm->wq,
+ (ibmvtpm->tpm_status & TPM_STATUS_BUSY) == 0);
if (sig)
return -EINTR;
@@ -220,11 +221,12 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
return -EIO;
}
- if (ibmvtpm->tpm_processing_cmd) {
+ if ((ibmvtpm->tpm_status & TPM_STATUS_BUSY)) {
dev_info(ibmvtpm->dev,
"Need to wait for TPM to finish\n");
/* wait for previous command to finish */
- sig = wait_event_interruptible(ibmvtpm->wq, !ibmvtpm->tpm_processing_cmd);
+ sig = wait_event_interruptible(ibmvtpm->wq,
+ (ibmvtpm->tpm_status & TPM_STATUS_BUSY) == 0);
if (sig)
return -EINTR;
}
@@ -237,7 +239,7 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
* set the processing flag before the Hcall, since we may get the
* result (interrupt) before even being able to check rc.
*/
- ibmvtpm->tpm_processing_cmd = true;
+ ibmvtpm->tpm_status |= TPM_STATUS_BUSY;
again:
rc = ibmvtpm_send_crq(ibmvtpm->vdev,
@@ -255,7 +257,7 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
goto again;
}
dev_err(ibmvtpm->dev, "tpm_ibmvtpm_send failed rc=%d\n", rc);
- ibmvtpm->tpm_processing_cmd = false;
+ ibmvtpm->tpm_status &= ~TPM_STATUS_BUSY;
}
spin_unlock(&ibmvtpm->rtce_lock);
@@ -550,7 +552,7 @@ static void ibmvtpm_crq_process(struct ibmvtpm_crq *crq,
case VTPM_TPM_COMMAND_RES:
/* len of the data in rtce buffer */
ibmvtpm->res_len = be16_to_cpu(crq->len);
- ibmvtpm->tpm_processing_cmd = false;
+ ibmvtpm->tpm_status &= ~TPM_STATUS_BUSY;
wake_up_interruptible(&ibmvtpm->wq);
return;
default:
diff --git a/drivers/char/tpm/tpm_ibmvtpm.h b/drivers/char/tpm/tpm_ibmvtpm.h
index b92aa7d3e93e..252f1cccdfc5 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.h
+++ b/drivers/char/tpm/tpm_ibmvtpm.h
@@ -41,7 +41,8 @@ struct ibmvtpm_dev {
wait_queue_head_t wq;
u16 res_len;
u32 vtpm_version;
- bool tpm_processing_cmd;
+ u8 tpm_status;
+#define TPM_STATUS_BUSY (1 << 0) /* vtpm is processing a command */
};
#define CRQ_RES_BUF_SIZE PAGE_SIZE
--
2.31.1
^ permalink raw reply related
* Re: [PATCH v2] tpm: ibmvtpm: Avoid error message when process gets signal while waiting
From: Jarkko Sakkinen @ 2021-08-05 20:44 UTC (permalink / raw)
To: Stefan Berger
Cc: peterhuewe, jgg, linux-integrity, linux-security-module,
linux-kernel, Stefan Berger, Nayna Jain, George Wilson,
Nageswara R Sastry
In-Reply-To: <20210803202622.1537040-1-stefanb@linux.vnet.ibm.com>
On Tue, Aug 03, 2021 at 04:26:22PM -0400, Stefan Berger wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>
>
> When rngd is run as root then lots of these types of message will appear
> in the kernel log if the TPM has been configured to provide random bytes:
>
> [ 7406.275163] tpm tpm0: tpm_transmit: tpm_recv: error -4
>
> The issue is caused by the following call that is interrupted while
> waiting for the TPM's response.
>
> sig = wait_event_interruptible(ibmvtpm->wq,
> !ibmvtpm->tpm_processing_cmd);
>
> Rather than waiting for the response in the low level driver, have it use
> the polling loop in tpm_try_transmit() that uses a command's duration to
> poll until a result has been returned by the TPM, thus ending when the
> timeout has occurred but not responding to signals and ctrl-c anymore. To
> stay in this polling loop extend tpm_ibmvtpm_status() to return
> TPM_STATUS_BUSY for as long as the vTPM is busy. Since the loop requires
> the TPM's timeouts, get them now using tpm_get_timeouts() after setting
> the TPM2 version flag on the chip.
>
> Rename the tpm_processing_cmd to tpm_status in ibmvtpm_dev and set the
> TPM_STATUS_BUSY flag while the vTPM is busy processing a command.
>
> To recreat the resolved issue start rngd like this:
>
> sudo rngd -r /dev/hwrng -t
> sudo rngd -r /dev/tpm0 -t
>
> Link: https://bugzilla.redhat.com/show_bug.cgi?id=1981473
> Fixes: 6674ff145eef ("tpm_ibmvtpm: properly handle interrupted packet receptions")
> Cc: Nayna Jain <nayna@linux.ibm.com>
> Cc: George Wilson <gcwilson@linux.ibm.com>
> Reported-by: Nageswara R Sastry <rnsastry@linux.ibm.com>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
>
> ---
>
> v2:
> - reworded commit text
> ---
> drivers/char/tpm/tpm_ibmvtpm.c | 31 ++++++++++++++++++-------------
> drivers/char/tpm/tpm_ibmvtpm.h | 3 ++-
> 2 files changed, 20 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
> index 903604769de9..5d795866b483 100644
> --- a/drivers/char/tpm/tpm_ibmvtpm.c
> +++ b/drivers/char/tpm/tpm_ibmvtpm.c
> @@ -106,17 +106,12 @@ static int tpm_ibmvtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count)
> {
> struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(&chip->dev);
> u16 len;
> - int sig;
>
> if (!ibmvtpm->rtce_buf) {
> dev_err(ibmvtpm->dev, "ibmvtpm device is not ready\n");
> return 0;
> }
>
> - sig = wait_event_interruptible(ibmvtpm->wq, !ibmvtpm->tpm_processing_cmd);
> - if (sig)
> - return -EINTR;
> -
> len = ibmvtpm->res_len;
>
> if (count < len) {
> @@ -220,11 +215,12 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
> return -EIO;
> }
>
> - if (ibmvtpm->tpm_processing_cmd) {
> + if ((ibmvtpm->tpm_status & TPM_STATUS_BUSY)) {
> dev_info(ibmvtpm->dev,
> "Need to wait for TPM to finish\n");
> /* wait for previous command to finish */
> - sig = wait_event_interruptible(ibmvtpm->wq, !ibmvtpm->tpm_processing_cmd);
> + sig = wait_event_interruptible(ibmvtpm->wq,
> + (ibmvtpm->tpm_status & TPM_STATUS_BUSY) == 0);
> if (sig)
> return -EINTR;
> }
> @@ -237,7 +233,7 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
> * set the processing flag before the Hcall, since we may get the
> * result (interrupt) before even being able to check rc.
> */
> - ibmvtpm->tpm_processing_cmd = true;
> + ibmvtpm->tpm_status |= TPM_STATUS_BUSY;
>
> again:
> rc = ibmvtpm_send_crq(ibmvtpm->vdev,
> @@ -255,7 +251,7 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
> goto again;
> }
> dev_err(ibmvtpm->dev, "tpm_ibmvtpm_send failed rc=%d\n", rc);
> - ibmvtpm->tpm_processing_cmd = false;
> + ibmvtpm->tpm_status &= ~TPM_STATUS_BUSY;
> }
>
> spin_unlock(&ibmvtpm->rtce_lock);
> @@ -269,7 +265,9 @@ static void tpm_ibmvtpm_cancel(struct tpm_chip *chip)
>
> static u8 tpm_ibmvtpm_status(struct tpm_chip *chip)
> {
> - return 0;
> + struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(&chip->dev);
> +
> + return ibmvtpm->tpm_status;
> }
>
> /**
> @@ -457,7 +455,7 @@ static const struct tpm_class_ops tpm_ibmvtpm = {
> .send = tpm_ibmvtpm_send,
> .cancel = tpm_ibmvtpm_cancel,
> .status = tpm_ibmvtpm_status,
> - .req_complete_mask = 0,
> + .req_complete_mask = TPM_STATUS_BUSY,
> .req_complete_val = 0,
> .req_canceled = tpm_ibmvtpm_req_canceled,
> };
> @@ -550,7 +548,7 @@ static void ibmvtpm_crq_process(struct ibmvtpm_crq *crq,
> case VTPM_TPM_COMMAND_RES:
> /* len of the data in rtce buffer */
> ibmvtpm->res_len = be16_to_cpu(crq->len);
> - ibmvtpm->tpm_processing_cmd = false;
> + ibmvtpm->tpm_status &= ~TPM_STATUS_BUSY;
> wake_up_interruptible(&ibmvtpm->wq);
> return;
> default:
> @@ -688,8 +686,15 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
> goto init_irq_cleanup;
> }
>
> - if (!strcmp(id->compat, "IBM,vtpm20")) {
> +
> + if (!strcmp(id->compat, "IBM,vtpm20"))
> chip->flags |= TPM_CHIP_FLAG_TPM2;
> +
> + rc = tpm_get_timeouts(chip);
> + if (rc)
> + goto init_irq_cleanup;
> +
> + if (chip->flags & TPM_CHIP_FLAG_TPM2) {
> rc = tpm2_get_cc_attrs_tbl(chip);
> if (rc)
> goto init_irq_cleanup;
> diff --git a/drivers/char/tpm/tpm_ibmvtpm.h b/drivers/char/tpm/tpm_ibmvtpm.h
> index b92aa7d3e93e..252f1cccdfc5 100644
> --- a/drivers/char/tpm/tpm_ibmvtpm.h
> +++ b/drivers/char/tpm/tpm_ibmvtpm.h
> @@ -41,7 +41,8 @@ struct ibmvtpm_dev {
> wait_queue_head_t wq;
> u16 res_len;
> u32 vtpm_version;
> - bool tpm_processing_cmd;
> + u8 tpm_status;
> +#define TPM_STATUS_BUSY (1 << 0) /* vtpm is processing a command */
> };
>
> #define CRQ_RES_BUF_SIZE PAGE_SIZE
> --
> 2.31.1
>
>
Please do not do the rename in the bug fix. If you really want to rename,
then this must be split into two commits.
/Jarkko
^ permalink raw reply
* Re: [GIT PULL] SELinux fixes for v5.14 (#1)
From: pr-tracker-bot @ 2021-08-05 19:26 UTC (permalink / raw)
To: Paul Moore; +Cc: Linus Torvalds, selinux, linux-security-module, linux-kernel
In-Reply-To: <CAHC9VhSW0zVR7wB9dxR-AkQAMK_H_fKQ75tTbMLomkBQzfzciw@mail.gmail.com>
The pull request you sent on Thu, 5 Aug 2021 14:29:27 -0400:
> git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git tags/selinux
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/0b53abfc5f66449d42fb1738c1c191e29e3be2e4
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox