* Re: [RFC PATCH v1] fscrypt: support encrypted and trusted keys
From: Ahmad Fatoum @ 2021-07-28 8:50 UTC (permalink / raw)
To: Eric Biggers
Cc: Theodore Y. Ts'o, Jaegeuk Kim, 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, git, Omar Sandoval,
Pengutronix Kernel Team
In-Reply-To: <YQA2fHPwH6EsH9BR@sol.localdomain>
Hello Eric,
On 27.07.21 18:38, Eric Biggers wrote:
> On Tue, Jul 27, 2021 at 04:43:49PM +0200, Ahmad Fatoum wrote:
>> For both v1 and v2 key setup mechanisms, userspace supplies the raw key
>> material to the kernel after which it is never again disclosed to
>> userspace.
>>
>> Use of encrypted and trusted keys offers stronger guarantees:
>> The key material is generated within the kernel and is never disclosed to
>> userspace in clear text and, in the case of trusted keys, can be
>> directly rooted to a trust source like a TPM chip.
>
> Please include a proper justification for this feature
I've patches pending for extending trusted keys to wrap the key sealing
functionality of the CAAM IP on NXP SoCs[1]. I want the kernel to
generate key material in the factory, have the CAAM encrypt it using its
undisclosed unique key and pass it to userspace as encrypted blob that is
persisted to an unencrypted volume. The intention is to thwart offline
decryption of an encrypted file system in an embedded system, where a
passphrase can't be supplied by an end user.
Employing TPM and TEE trusted keys with this is already possible with
dm-crypt, but I'd like this to be possible out-of-the-box with
ubifs + fscrypt as well.
> and update the relevant
> sections of Documentation/filesystems/fscrypt.rst to explain why someone would
> want to use this feature and what it accomplishes.
How about:
- 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.
+ 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 should leave this 0 and specify the raw key directly.
+ Most users 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,
+ "trusted" keys are useful to leverage kernel support for sealing and
+ unsealing key material. Sealed keys can be persisted to unencrypted
+ storage and later used to decrypt the file system without requiring
+ userspace to know the raw key material.
+ "fscrypt-provisioning" key support is intended mainly to allow
+ re-adding keys after a filesystem is unmounted and re-mounted,
> As-is, this feature doesn't seem to have a very strong justification. Please
> also see previous threads where this feature was discussed/requested:
> https://lkml.kernel.org/linux-fscrypt/20180110124418.24385-1-git@andred.net/T/#u,
> https://lkml.kernel.org/linux-fscrypt/20180118131359.8365-1-git@andred.net/T/#u,
> https://lkml.kernel.org/linux-fscrypt/20200116193228.GA266386@vader/T/#u
Thanks. I wasn't aware of the last one. I (re-)read them now. I hope
this mail manages to address the concerns.
(Also added original authors of these mail threads to CC)
> Note that there are several design flaws with the encrypted and trusted key
> types:
>
> - By default, trusted keys are generated using the TPM's RNG rather than the
> kernel's RNG, which places all trust in an unauditable black box.
Patch to fix that awaits feedback on linux-integrity[2].
> - trusted and encrypted keys aren't restricted to specific uses in the kernel
> (like the fscrypt-provisioning key type is) but rather are general-purpose.
> Hence, it may be possible to leak their contents to userspace by requesting
> their use for certain algorithms/features, e.g. to encrypt a dm-crypt target
> using a weak cipher that is vulnerable to key recovery attacks.
The footgun is already there by allowing users to specify their own
raw key. Users can already use $keyid for dm-crypt and then do
$ keyctl pipe $keyid | fscryptctl add_key /mnt
The responsibility to not reuse key material already lies with the users,
regardless if they handle the raw key material directly or indirectly via
a trusted key description/ID.
> - "encrypted" keys that use a master key of type "user" are supported, despite
> these being easily obtainable in the clear by userspace providing their own
> master key. This violates one of the main design goals of "encrypted" keys.
I care for trusted keys foremost, so I've no problems dropping the encrypted
key support.
> Also, using the "trusted" key type isn't necessary to achieve TPM-bound
> encryption, as TPM binding can be handled in userspace instead.
Trusted keys support TEE and hopefully CAAM soon as well. I don't want my
userspace directly poking a DMA master.
> So I really would like to see a proper justification for this feature, and have
> it be properly documented.
In light of the extended justification above, do you want me to respin with
the proposed changes?
> One comment on the UAPI below.
> Why not just allow the key_id field to specify a "trusted" or "encrypted" key?
> Why is it necessary for FS_IOC_ADD_ENCRYPTION_KEY to support two different ways
> of looking up keyring keys -- by ID and by description? Looking up by ID works
> fine for "fscrypt-provisioning" keys; why are "trusted" and "encrypted" keys
> different in this regard?
Mixture of reading emails predating key_id and misunderstanding the API.
key_id would be much cleaner indeed. I can change this for v2.
Thanks for your review.
[1]: https://lore.kernel.org/linux-integrity/655aab117f922320e2123815afb5bf3daeb7b8b3.1626885907.git-series.a.fatoum@pengutronix.de/
[2]: https://lore.kernel.org/linux-integrity/cover.9fc9298fd9d63553491871d043a18affc2dbc8a8.1626885907.git-series.a.fatoum@pengutronix.de/T/#meaefcdc9ac091944ddadaebe0410c2325af0032e
Cheers,
Ahmad
>
> - Eric
>
--
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: [RFC][PATCH v2 02/12] diglim: Basic definitions
From: Roberto Sassu @ 2021-07-28 6:59 UTC (permalink / raw)
To: Greg KH
Cc: zohar@linux.ibm.com, mchehab+huawei@kernel.org,
linux-integrity@vger.kernel.org,
linux-security-module@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <YQAwqGOEkmDzZ9MJ@kroah.com>
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Tuesday, July 27, 2021 6:13 PM
> On Tue, Jul 27, 2021 at 04:09:37PM +0000, Roberto Sassu wrote:
> > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > Sent: Tuesday, July 27, 2021 5:44 PM
> > > On Tue, Jul 27, 2021 at 03:35:16PM +0000, Roberto Sassu wrote:
> > > > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > > > Sent: Tuesday, July 27, 2021 4:44 PM
> > > > > On Mon, Jul 26, 2021 at 06:36:50PM +0200, Roberto Sassu wrote:
> > > > > > --- /dev/null
> > > > > > +++ b/include/uapi/linux/diglim.h
> > > > > > @@ -0,0 +1,51 @@
> > > > > > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> > > > > > +/*
> > > > > > + * Copyright (C) 2017-2021 Huawei Technologies Duesseldorf GmbH
> > > > > > + *
> > > > > > + * Author: Roberto Sassu <roberto.sassu@huawei.com>
> > > > > > + *
> > > > > > + * DIGLIM definitions exported to user space, useful for generating
> > > digest
> > > > > > + * lists.
> > > > > > + */
> > > > > > +
> > > > > > +#ifndef _UAPI__LINUX_DIGLIM_H
> > > > > > +#define _UAPI__LINUX_DIGLIM_H
> > > > > > +
> > > > > > +#include <linux/types.h>
> > > > > > +#include <linux/hash_info.h>
> > > > > > +
> > > > > > +enum compact_types { COMPACT_KEY, COMPACT_PARSER,
> > > > > COMPACT_FILE,
> > > > > > + COMPACT_METADATA, COMPACT_DIGEST_LIST,
> > > > > COMPACT__LAST };
> > > > > > +
> > > > > > +enum compact_modifiers { COMPACT_MOD_IMMUTABLE,
> > > > > COMPACT_MOD__LAST };
> > > > > > +
> > > > > > +enum compact_actions { COMPACT_ACTION_IMA_MEASURED,
> > > > > > + COMPACT_ACTION_IMA_APPRAISED,
> > > > > > + COMPACT_ACTION_IMA_APPRAISED_DIGSIG,
> > > > > > + COMPACT_ACTION__LAST };
> > > > > > +
> > > > > > +enum ops { DIGEST_LIST_ADD, DIGEST_LIST_DEL,
> > > DIGEST_LIST_OP__LAST };
> > > > > > +
> > > > > > +/**
> > > > > > + * struct compact_list_hdr - header of the following concatenated
> > > digests
> > > > > > + * @version: version of the digest list
> > > > > > + * @_reserved: field reserved for future use
> > > > > > + * @type: type of digest list among enum compact_types
> > > > > > + * @modifiers: additional attributes among (1 << enum
> > > compact_modifiers)
> > > > >
> > > > > I do not understand this description, what does it mean?
> > > >
> > > > Hi Greg
> > > >
> > > > yes, it is not very clear.
> > > >
> > > > @modifiers is a bitmask where each bit corresponds to a different
> > > > attribute. enum compact_modifiers defines which bit position is
> > > > assigned to each attribute.
> > >
> > > Watch out with endian issues and bitmasks... Anyway, please document
> > > this.
> > >
> > > >
> > > > > > + * @algo: digest algorithm
> > > > >
> > > > > Is this also a #define or an enum? Where is the list of them?
> > > >
> > > > @algo is an enum defined in include/uapi/linux/hash_info.h.
> > >
> > > Please say that.
> > >
> > > > > > + * @count: number of digests
> > > > > > + * @datalen: length of concatenated digests
> > > > >
> > > > > Where does this count and length come into play as nothing else is in
> > > > > this structure?
> > > >
> > > > Each digest list must begin with this structure. From it, the parser knows
> > > > how much data it should expect afterwards. After the data, there could
> be
> > > > another or more blocks of this structure and following data.
> > >
> > > Ah, that was not obvious at all :)
> > >
> > > Why do you not have a __u8 data[]; type field as the last one here for
> > > that memory so you can access it easier?
> >
> > After the digest list is parsed, I'm accessing the digest with the offset from
> > the beginning of the digest list. If the offset was relative to the header, it
> could
> > have been useful. I could add the new field, but I'm afraid of the
> incompatibility
> > with existing tools that we have.
>
> What tools? This isn't a feature in the kernel yet, so we have no
> legacy to support, right?
Yes, right. We shouldn't be limited by previously written code.
Thanks
Roberto
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Li Jian, Shi Yanli
> thanks,
>
> greg k-h
^ permalink raw reply
* Re: [PATCH RESEND] net: cipso: fix warnings in netlbl_cipsov4_add_std
From: patchwork-bot+netdevbpf @ 2021-07-27 20:00 UTC (permalink / raw)
To: Pavel Skripkin
Cc: paul, davem, kuba, netdev, linux-security-module, linux-kernel,
syzbot+cdd51ee2e6b0b2e18c0d
In-Reply-To: <20210727163530.3057-1-paskripkin@gmail.com>
Hello:
This patch was applied to netdev/net-next.git (refs/heads/master):
On Tue, 27 Jul 2021 19:35:30 +0300 you wrote:
> Syzbot reported warning in netlbl_cipsov4_add(). The
> problem was in too big doi_def->map.std->lvl.local_size
> passed to kcalloc(). Since this value comes from userpace there is
> no need to warn if value is not correct.
>
> The same problem may occur with other kcalloc() calls in
> this function, so, I've added __GFP_NOWARN flag to all
> kcalloc() calls there.
>
> [...]
Here is the summary with links:
- [RESEND] net: cipso: fix warnings in netlbl_cipsov4_add_std
https://git.kernel.org/netdev/net-next/c/8ca34a13f7f9
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [RFC PATCH v1] fscrypt: support encrypted and trusted keys
From: Eric Biggers @ 2021-07-27 16:38 UTC (permalink / raw)
To: Ahmad Fatoum
Cc: Theodore Y. Ts'o, Jaegeuk Kim, 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
In-Reply-To: <20210727144349.11215-1-a.fatoum@pengutronix.de>
On Tue, Jul 27, 2021 at 04:43:49PM +0200, Ahmad Fatoum wrote:
> For both v1 and v2 key setup mechanisms, userspace supplies the raw key
> material to the kernel after which it is never again disclosed to
> userspace.
>
> Use of encrypted and trusted keys offers stronger guarantees:
> The key material is generated within the kernel and is never disclosed to
> userspace in clear text and, in the case of trusted keys, can be
> directly rooted to a trust source like a TPM chip.
Please include a proper justification for this feature and update the relevant
sections of Documentation/filesystems/fscrypt.rst to explain why someone would
want to use this feature and what it accomplishes.
As-is, this feature doesn't seem to have a very strong justification. Please
also see previous threads where this feature was discussed/requested:
https://lkml.kernel.org/linux-fscrypt/20180110124418.24385-1-git@andred.net/T/#u,
https://lkml.kernel.org/linux-fscrypt/20180118131359.8365-1-git@andred.net/T/#u,
https://lkml.kernel.org/linux-fscrypt/20200116193228.GA266386@vader/T/#u
Note that there are several design flaws with the encrypted and trusted key
types:
- By default, trusted keys are generated using the TPM's RNG rather than the
kernel's RNG, which places all trust in an unauditable black box.
- trusted and encrypted keys aren't restricted to specific uses in the kernel
(like the fscrypt-provisioning key type is) but rather are general-purpose.
Hence, it may be possible to leak their contents to userspace by requesting
their use for certain algorithms/features, e.g. to encrypt a dm-crypt target
using a weak cipher that is vulnerable to key recovery attacks.
- "encrypted" keys that use a master key of type "user" are supported, despite
these being easily obtainable in the clear by userspace providing their own
master key. This violates one of the main design goals of "encrypted" keys.
Also, using the "trusted" key type isn't necessary to achieve TPM-bound
encryption, as TPM binding can be handled in userspace instead.
So I really would like to see a proper justification for this feature, and have
it be properly documented.
One comment on the UAPI below.
>
> Add support for trusted and encrypted keys by repurposing
> fscrypt_add_key_arg::raw to hold the key description when the new
> FSCRYPT_KEY_ARG_TYPE_DESC flag is supplied. The location of the flag
> was previously reserved and enforced by ioctl code to be zero, so this
> change won't break backwards compatibility.
>
> Corresponding userspace patches are available for fscryptctl:
> https://github.com/google/fscryptctl/pull/23
>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> key_extract_material used by this patch is added in
> <cover.b2fdd70b830d12853b12a12e32ceb0c8162c1346.1626945419.git-series.a.fatoum@pengutronix.de>
> which still awaits feedback.
>
> Sending this RFC out anyway to get some feedback from the fscrypt
> developers whether this is the correct way to go about it.
>
> 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 | 24 ++++++++---
> fs/crypto/keyring.c | 59 ++++++++++++++++++++++++---
> include/uapi/linux/fscrypt.h | 16 +++++++-
> 3 files changed, 87 insertions(+), 12 deletions(-)
>
> diff --git a/Documentation/filesystems/fscrypt.rst b/Documentation/filesystems/fscrypt.rst
> index 44b67ebd6e40..83738af2afa3 100644
> --- a/Documentation/filesystems/fscrypt.rst
> +++ b/Documentation/filesystems/fscrypt.rst
> @@ -681,11 +681,15 @@ It can be executed on any file or directory on the target filesystem,
> but using the filesystem's root directory is recommended. It takes in
> a pointer to struct fscrypt_add_key_arg, defined as follows::
>
> + #define FSCRYPT_KEY_ADD_RAW_ASIS 0
> + #define FSCRYPT_KEY_ADD_RAW_DESC 1
> +
> struct fscrypt_add_key_arg {
> struct fscrypt_key_specifier key_spec;
> __u32 raw_size;
> __u32 key_id;
> - __u32 __reserved[8];
> + __u32 raw_flags; /* one of FSCRYPT_KEY_ADD_RAW_* */
> + __u32 __reserved[7];
> __u8 raw[];
> };
>
> @@ -732,8 +736,11 @@ as follows:
> Alternatively, if ``key_id`` is nonzero, this field must be 0, since
> in that case the size is implied by the specified Linux keyring key.
>
> -- ``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
> +- If ``key_id`` is 0, the raw key is given directly in the ``raw``
> + field if ``raw_flags == FSCRYPT_KEY_ADD_RAW_ASIS``. With
> + ``raw_flags == FSCRYPT_KEY_ADD_RAW_DESC``, ``raw`` is instead
> + interpreted as the description of an encrypted or trusted key.
> + Otherwise ``key_id`` is the ID of a Linux keyring key of
> type "fscrypt-provisioning" whose payload is
> struct fscrypt_provisioning_key_payload whose ``raw`` field contains
> the raw key and whose ``type`` field matches ``key_spec.type``.
> @@ -748,8 +755,15 @@ as follows:
> without having to store the raw keys in userspace memory.
Why not just allow the key_id field to specify a "trusted" or "encrypted" key?
Why is it necessary for FS_IOC_ADD_ENCRYPTION_KEY to support two different ways
of looking up keyring keys -- by ID and by description? Looking up by ID works
fine for "fscrypt-provisioning" keys; why are "trusted" and "encrypted" keys
different in this regard?
- Eric
^ permalink raw reply
* [PATCH RESEND] net: cipso: fix warnings in netlbl_cipsov4_add_std
From: Pavel Skripkin @ 2021-07-27 16:35 UTC (permalink / raw)
To: paul, davem, kuba
Cc: netdev, linux-security-module, linux-kernel, Pavel Skripkin,
syzbot+cdd51ee2e6b0b2e18c0d
Syzbot reported warning in netlbl_cipsov4_add(). The
problem was in too big doi_def->map.std->lvl.local_size
passed to kcalloc(). Since this value comes from userpace there is
no need to warn if value is not correct.
The same problem may occur with other kcalloc() calls in
this function, so, I've added __GFP_NOWARN flag to all
kcalloc() calls there.
Reported-and-tested-by: syzbot+cdd51ee2e6b0b2e18c0d@syzkaller.appspotmail.com
Fixes: 96cb8e3313c7 ("[NetLabel]: CIPSOv4 and Unlabeled packet integration")
Acked-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
---
net/netlabel/netlabel_cipso_v4.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/netlabel/netlabel_cipso_v4.c b/net/netlabel/netlabel_cipso_v4.c
index baf235721c43..000bb3da4f77 100644
--- a/net/netlabel/netlabel_cipso_v4.c
+++ b/net/netlabel/netlabel_cipso_v4.c
@@ -187,14 +187,14 @@ static int netlbl_cipsov4_add_std(struct genl_info *info,
}
doi_def->map.std->lvl.local = kcalloc(doi_def->map.std->lvl.local_size,
sizeof(u32),
- GFP_KERNEL);
+ GFP_KERNEL | __GFP_NOWARN);
if (doi_def->map.std->lvl.local == NULL) {
ret_val = -ENOMEM;
goto add_std_failure;
}
doi_def->map.std->lvl.cipso = kcalloc(doi_def->map.std->lvl.cipso_size,
sizeof(u32),
- GFP_KERNEL);
+ GFP_KERNEL | __GFP_NOWARN);
if (doi_def->map.std->lvl.cipso == NULL) {
ret_val = -ENOMEM;
goto add_std_failure;
@@ -263,7 +263,7 @@ static int netlbl_cipsov4_add_std(struct genl_info *info,
doi_def->map.std->cat.local = kcalloc(
doi_def->map.std->cat.local_size,
sizeof(u32),
- GFP_KERNEL);
+ GFP_KERNEL | __GFP_NOWARN);
if (doi_def->map.std->cat.local == NULL) {
ret_val = -ENOMEM;
goto add_std_failure;
@@ -271,7 +271,7 @@ static int netlbl_cipsov4_add_std(struct genl_info *info,
doi_def->map.std->cat.cipso = kcalloc(
doi_def->map.std->cat.cipso_size,
sizeof(u32),
- GFP_KERNEL);
+ GFP_KERNEL | __GFP_NOWARN);
if (doi_def->map.std->cat.cipso == NULL) {
ret_val = -ENOMEM;
goto add_std_failure;
--
2.32.0
^ permalink raw reply related
* Re: [RFC][PATCH v2 02/12] diglim: Basic definitions
From: Greg KH @ 2021-07-27 16:13 UTC (permalink / raw)
To: Roberto Sassu
Cc: zohar@linux.ibm.com, mchehab+huawei@kernel.org,
linux-integrity@vger.kernel.org,
linux-security-module@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <4746947088404edaa31594fb095a6e46@huawei.com>
On Tue, Jul 27, 2021 at 04:09:37PM +0000, Roberto Sassu wrote:
> > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > Sent: Tuesday, July 27, 2021 5:44 PM
> > On Tue, Jul 27, 2021 at 03:35:16PM +0000, Roberto Sassu wrote:
> > > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > > Sent: Tuesday, July 27, 2021 4:44 PM
> > > > On Mon, Jul 26, 2021 at 06:36:50PM +0200, Roberto Sassu wrote:
> > > > > --- /dev/null
> > > > > +++ b/include/uapi/linux/diglim.h
> > > > > @@ -0,0 +1,51 @@
> > > > > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> > > > > +/*
> > > > > + * Copyright (C) 2017-2021 Huawei Technologies Duesseldorf GmbH
> > > > > + *
> > > > > + * Author: Roberto Sassu <roberto.sassu@huawei.com>
> > > > > + *
> > > > > + * DIGLIM definitions exported to user space, useful for generating
> > digest
> > > > > + * lists.
> > > > > + */
> > > > > +
> > > > > +#ifndef _UAPI__LINUX_DIGLIM_H
> > > > > +#define _UAPI__LINUX_DIGLIM_H
> > > > > +
> > > > > +#include <linux/types.h>
> > > > > +#include <linux/hash_info.h>
> > > > > +
> > > > > +enum compact_types { COMPACT_KEY, COMPACT_PARSER,
> > > > COMPACT_FILE,
> > > > > + COMPACT_METADATA, COMPACT_DIGEST_LIST,
> > > > COMPACT__LAST };
> > > > > +
> > > > > +enum compact_modifiers { COMPACT_MOD_IMMUTABLE,
> > > > COMPACT_MOD__LAST };
> > > > > +
> > > > > +enum compact_actions { COMPACT_ACTION_IMA_MEASURED,
> > > > > + COMPACT_ACTION_IMA_APPRAISED,
> > > > > + COMPACT_ACTION_IMA_APPRAISED_DIGSIG,
> > > > > + COMPACT_ACTION__LAST };
> > > > > +
> > > > > +enum ops { DIGEST_LIST_ADD, DIGEST_LIST_DEL,
> > DIGEST_LIST_OP__LAST };
> > > > > +
> > > > > +/**
> > > > > + * struct compact_list_hdr - header of the following concatenated
> > digests
> > > > > + * @version: version of the digest list
> > > > > + * @_reserved: field reserved for future use
> > > > > + * @type: type of digest list among enum compact_types
> > > > > + * @modifiers: additional attributes among (1 << enum
> > compact_modifiers)
> > > >
> > > > I do not understand this description, what does it mean?
> > >
> > > Hi Greg
> > >
> > > yes, it is not very clear.
> > >
> > > @modifiers is a bitmask where each bit corresponds to a different
> > > attribute. enum compact_modifiers defines which bit position is
> > > assigned to each attribute.
> >
> > Watch out with endian issues and bitmasks... Anyway, please document
> > this.
> >
> > >
> > > > > + * @algo: digest algorithm
> > > >
> > > > Is this also a #define or an enum? Where is the list of them?
> > >
> > > @algo is an enum defined in include/uapi/linux/hash_info.h.
> >
> > Please say that.
> >
> > > > > + * @count: number of digests
> > > > > + * @datalen: length of concatenated digests
> > > >
> > > > Where does this count and length come into play as nothing else is in
> > > > this structure?
> > >
> > > Each digest list must begin with this structure. From it, the parser knows
> > > how much data it should expect afterwards. After the data, there could be
> > > another or more blocks of this structure and following data.
> >
> > Ah, that was not obvious at all :)
> >
> > Why do you not have a __u8 data[]; type field as the last one here for
> > that memory so you can access it easier?
>
> After the digest list is parsed, I'm accessing the digest with the offset from
> the beginning of the digest list. If the offset was relative to the header, it could
> have been useful. I could add the new field, but I'm afraid of the incompatibility
> with existing tools that we have.
What tools? This isn't a feature in the kernel yet, so we have no
legacy to support, right?
thanks,
greg k-h
^ permalink raw reply
* RE: [RFC][PATCH v2 02/12] diglim: Basic definitions
From: Roberto Sassu @ 2021-07-27 16:09 UTC (permalink / raw)
To: Greg KH
Cc: zohar@linux.ibm.com, mchehab+huawei@kernel.org,
linux-integrity@vger.kernel.org,
linux-security-module@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <YQApyqP7J/8GpItS@kroah.com>
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Tuesday, July 27, 2021 5:44 PM
> On Tue, Jul 27, 2021 at 03:35:16PM +0000, Roberto Sassu wrote:
> > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > Sent: Tuesday, July 27, 2021 4:44 PM
> > > On Mon, Jul 26, 2021 at 06:36:50PM +0200, Roberto Sassu wrote:
> > > > --- /dev/null
> > > > +++ b/include/uapi/linux/diglim.h
> > > > @@ -0,0 +1,51 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> > > > +/*
> > > > + * Copyright (C) 2017-2021 Huawei Technologies Duesseldorf GmbH
> > > > + *
> > > > + * Author: Roberto Sassu <roberto.sassu@huawei.com>
> > > > + *
> > > > + * DIGLIM definitions exported to user space, useful for generating
> digest
> > > > + * lists.
> > > > + */
> > > > +
> > > > +#ifndef _UAPI__LINUX_DIGLIM_H
> > > > +#define _UAPI__LINUX_DIGLIM_H
> > > > +
> > > > +#include <linux/types.h>
> > > > +#include <linux/hash_info.h>
> > > > +
> > > > +enum compact_types { COMPACT_KEY, COMPACT_PARSER,
> > > COMPACT_FILE,
> > > > + COMPACT_METADATA, COMPACT_DIGEST_LIST,
> > > COMPACT__LAST };
> > > > +
> > > > +enum compact_modifiers { COMPACT_MOD_IMMUTABLE,
> > > COMPACT_MOD__LAST };
> > > > +
> > > > +enum compact_actions { COMPACT_ACTION_IMA_MEASURED,
> > > > + COMPACT_ACTION_IMA_APPRAISED,
> > > > + COMPACT_ACTION_IMA_APPRAISED_DIGSIG,
> > > > + COMPACT_ACTION__LAST };
> > > > +
> > > > +enum ops { DIGEST_LIST_ADD, DIGEST_LIST_DEL,
> DIGEST_LIST_OP__LAST };
> > > > +
> > > > +/**
> > > > + * struct compact_list_hdr - header of the following concatenated
> digests
> > > > + * @version: version of the digest list
> > > > + * @_reserved: field reserved for future use
> > > > + * @type: type of digest list among enum compact_types
> > > > + * @modifiers: additional attributes among (1 << enum
> compact_modifiers)
> > >
> > > I do not understand this description, what does it mean?
> >
> > Hi Greg
> >
> > yes, it is not very clear.
> >
> > @modifiers is a bitmask where each bit corresponds to a different
> > attribute. enum compact_modifiers defines which bit position is
> > assigned to each attribute.
>
> Watch out with endian issues and bitmasks... Anyway, please document
> this.
>
> >
> > > > + * @algo: digest algorithm
> > >
> > > Is this also a #define or an enum? Where is the list of them?
> >
> > @algo is an enum defined in include/uapi/linux/hash_info.h.
>
> Please say that.
>
> > > > + * @count: number of digests
> > > > + * @datalen: length of concatenated digests
> > >
> > > Where does this count and length come into play as nothing else is in
> > > this structure?
> >
> > Each digest list must begin with this structure. From it, the parser knows
> > how much data it should expect afterwards. After the data, there could be
> > another or more blocks of this structure and following data.
>
> Ah, that was not obvious at all :)
>
> Why do you not have a __u8 data[]; type field as the last one here for
> that memory so you can access it easier?
After the digest list is parsed, I'm accessing the digest with the offset from
the beginning of the digest list. If the offset was relative to the header, it could
have been useful. I could add the new field, but I'm afraid of the incompatibility
with existing tools that we have.
Thanks
Roberto
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Li Jian, Shi Yanli
> thanks,
>
> greg k-h
^ permalink raw reply
* Re: [RFC][PATCH v2 02/12] diglim: Basic definitions
From: Greg KH @ 2021-07-27 15:44 UTC (permalink / raw)
To: Roberto Sassu
Cc: zohar@linux.ibm.com, mchehab+huawei@kernel.org,
linux-integrity@vger.kernel.org,
linux-security-module@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <e87ba6f452254067a5eb6d58937d65d1@huawei.com>
On Tue, Jul 27, 2021 at 03:35:16PM +0000, Roberto Sassu wrote:
> > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > Sent: Tuesday, July 27, 2021 4:44 PM
> > On Mon, Jul 26, 2021 at 06:36:50PM +0200, Roberto Sassu wrote:
> > > --- /dev/null
> > > +++ b/include/uapi/linux/diglim.h
> > > @@ -0,0 +1,51 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> > > +/*
> > > + * Copyright (C) 2017-2021 Huawei Technologies Duesseldorf GmbH
> > > + *
> > > + * Author: Roberto Sassu <roberto.sassu@huawei.com>
> > > + *
> > > + * DIGLIM definitions exported to user space, useful for generating digest
> > > + * lists.
> > > + */
> > > +
> > > +#ifndef _UAPI__LINUX_DIGLIM_H
> > > +#define _UAPI__LINUX_DIGLIM_H
> > > +
> > > +#include <linux/types.h>
> > > +#include <linux/hash_info.h>
> > > +
> > > +enum compact_types { COMPACT_KEY, COMPACT_PARSER,
> > COMPACT_FILE,
> > > + COMPACT_METADATA, COMPACT_DIGEST_LIST,
> > COMPACT__LAST };
> > > +
> > > +enum compact_modifiers { COMPACT_MOD_IMMUTABLE,
> > COMPACT_MOD__LAST };
> > > +
> > > +enum compact_actions { COMPACT_ACTION_IMA_MEASURED,
> > > + COMPACT_ACTION_IMA_APPRAISED,
> > > + COMPACT_ACTION_IMA_APPRAISED_DIGSIG,
> > > + COMPACT_ACTION__LAST };
> > > +
> > > +enum ops { DIGEST_LIST_ADD, DIGEST_LIST_DEL, DIGEST_LIST_OP__LAST };
> > > +
> > > +/**
> > > + * struct compact_list_hdr - header of the following concatenated digests
> > > + * @version: version of the digest list
> > > + * @_reserved: field reserved for future use
> > > + * @type: type of digest list among enum compact_types
> > > + * @modifiers: additional attributes among (1 << enum compact_modifiers)
> >
> > I do not understand this description, what does it mean?
>
> Hi Greg
>
> yes, it is not very clear.
>
> @modifiers is a bitmask where each bit corresponds to a different
> attribute. enum compact_modifiers defines which bit position is
> assigned to each attribute.
Watch out with endian issues and bitmasks... Anyway, please document
this.
>
> > > + * @algo: digest algorithm
> >
> > Is this also a #define or an enum? Where is the list of them?
>
> @algo is an enum defined in include/uapi/linux/hash_info.h.
Please say that.
> > > + * @count: number of digests
> > > + * @datalen: length of concatenated digests
> >
> > Where does this count and length come into play as nothing else is in
> > this structure?
>
> Each digest list must begin with this structure. From it, the parser knows
> how much data it should expect afterwards. After the data, there could be
> another or more blocks of this structure and following data.
Ah, that was not obvious at all :)
Why do you not have a __u8 data[]; type field as the last one here for
that memory so you can access it easier?
thanks,
greg k-h
^ permalink raw reply
* RE: [RFC][PATCH v2 02/12] diglim: Basic definitions
From: Roberto Sassu @ 2021-07-27 15:35 UTC (permalink / raw)
To: Greg KH
Cc: zohar@linux.ibm.com, mchehab+huawei@kernel.org,
linux-integrity@vger.kernel.org,
linux-security-module@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <YQAblc+UuMq68jxu@kroah.com>
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Tuesday, July 27, 2021 4:44 PM
> On Mon, Jul 26, 2021 at 06:36:50PM +0200, Roberto Sassu wrote:
> > --- /dev/null
> > +++ b/include/uapi/linux/diglim.h
> > @@ -0,0 +1,51 @@
> > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> > +/*
> > + * Copyright (C) 2017-2021 Huawei Technologies Duesseldorf GmbH
> > + *
> > + * Author: Roberto Sassu <roberto.sassu@huawei.com>
> > + *
> > + * DIGLIM definitions exported to user space, useful for generating digest
> > + * lists.
> > + */
> > +
> > +#ifndef _UAPI__LINUX_DIGLIM_H
> > +#define _UAPI__LINUX_DIGLIM_H
> > +
> > +#include <linux/types.h>
> > +#include <linux/hash_info.h>
> > +
> > +enum compact_types { COMPACT_KEY, COMPACT_PARSER,
> COMPACT_FILE,
> > + COMPACT_METADATA, COMPACT_DIGEST_LIST,
> COMPACT__LAST };
> > +
> > +enum compact_modifiers { COMPACT_MOD_IMMUTABLE,
> COMPACT_MOD__LAST };
> > +
> > +enum compact_actions { COMPACT_ACTION_IMA_MEASURED,
> > + COMPACT_ACTION_IMA_APPRAISED,
> > + COMPACT_ACTION_IMA_APPRAISED_DIGSIG,
> > + COMPACT_ACTION__LAST };
> > +
> > +enum ops { DIGEST_LIST_ADD, DIGEST_LIST_DEL, DIGEST_LIST_OP__LAST };
> > +
> > +/**
> > + * struct compact_list_hdr - header of the following concatenated digests
> > + * @version: version of the digest list
> > + * @_reserved: field reserved for future use
> > + * @type: type of digest list among enum compact_types
> > + * @modifiers: additional attributes among (1 << enum compact_modifiers)
>
> I do not understand this description, what does it mean?
Hi Greg
yes, it is not very clear.
@modifiers is a bitmask where each bit corresponds to a different
attribute. enum compact_modifiers defines which bit position is
assigned to each attribute.
> > + * @algo: digest algorithm
>
> Is this also a #define or an enum? Where is the list of them?
@algo is an enum defined in include/uapi/linux/hash_info.h.
> > + * @count: number of digests
> > + * @datalen: length of concatenated digests
>
> Where does this count and length come into play as nothing else is in
> this structure?
Each digest list must begin with this structure. From it, the parser knows
how much data it should expect afterwards. After the data, there could be
another or more blocks of this structure and following data.
There is an example in the 'Compact Digest List Example' subsection,
in Documentation/security/diglim/implementation.rst.
> > + *
> > + * A digest list is a set of blocks composed by struct compact_list_hdr and
> > + * the following concatenated digests.
> > + */
> > +struct compact_list_hdr {
> > + __u8 version;
> > + __u8 _reserved;
>
> You MUST check this for 0 today, and document it above. If not, you can
> never use it in the future.
Ok, yes. I will add it.
Thanks
Roberto
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Li Jian, Shi Yanli
> > + __le16 type;
> > + __le16 modifiers;
> > + __le16 algo;
> > + __le32 count;
> > + __le32 datalen;
> > +} __packed;
> > +#endif /*_UAPI__LINUX_DIGLIM_H*/
> > --
> > 2.25.1
> >
^ permalink raw reply
* [RFC PATCH v1] fscrypt: support encrypted and trusted keys
From: Ahmad Fatoum @ 2021-07-27 14:43 UTC (permalink / raw)
To: Theodore Y. Ts'o, Jaegeuk Kim, Eric Biggers
Cc: 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
For both v1 and v2 key setup mechanisms, userspace supplies the raw key
material to the kernel after which it is never again disclosed to
userspace.
Use of encrypted and trusted keys offers stronger guarantees:
The key material is generated within the kernel and is never disclosed to
userspace in clear text and, in the case of trusted keys, can be
directly rooted to a trust source like a TPM chip.
Add support for trusted and encrypted keys by repurposing
fscrypt_add_key_arg::raw to hold the key description when the new
FSCRYPT_KEY_ARG_TYPE_DESC flag is supplied. The location of the flag
was previously reserved and enforced by ioctl code to be zero, so this
change won't break backwards compatibility.
Corresponding userspace patches are available for fscryptctl:
https://github.com/google/fscryptctl/pull/23
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
key_extract_material used by this patch is added in
<cover.b2fdd70b830d12853b12a12e32ceb0c8162c1346.1626945419.git-series.a.fatoum@pengutronix.de>
which still awaits feedback.
Sending this RFC out anyway to get some feedback from the fscrypt
developers whether this is the correct way to go about it.
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 | 24 ++++++++---
fs/crypto/keyring.c | 59 ++++++++++++++++++++++++---
include/uapi/linux/fscrypt.h | 16 +++++++-
3 files changed, 87 insertions(+), 12 deletions(-)
diff --git a/Documentation/filesystems/fscrypt.rst b/Documentation/filesystems/fscrypt.rst
index 44b67ebd6e40..83738af2afa3 100644
--- a/Documentation/filesystems/fscrypt.rst
+++ b/Documentation/filesystems/fscrypt.rst
@@ -681,11 +681,15 @@ It can be executed on any file or directory on the target filesystem,
but using the filesystem's root directory is recommended. It takes in
a pointer to struct fscrypt_add_key_arg, defined as follows::
+ #define FSCRYPT_KEY_ADD_RAW_ASIS 0
+ #define FSCRYPT_KEY_ADD_RAW_DESC 1
+
struct fscrypt_add_key_arg {
struct fscrypt_key_specifier key_spec;
__u32 raw_size;
__u32 key_id;
- __u32 __reserved[8];
+ __u32 raw_flags; /* one of FSCRYPT_KEY_ADD_RAW_* */
+ __u32 __reserved[7];
__u8 raw[];
};
@@ -732,8 +736,11 @@ as follows:
Alternatively, if ``key_id`` is nonzero, this field must be 0, since
in that case the size is implied by the specified Linux keyring key.
-- ``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
+- If ``key_id`` is 0, the raw key is given directly in the ``raw``
+ field if ``raw_flags == FSCRYPT_KEY_ADD_RAW_ASIS``. With
+ ``raw_flags == FSCRYPT_KEY_ADD_RAW_DESC``, ``raw`` is instead
+ interpreted as the description of an encrypted or trusted key.
+ Otherwise ``key_id`` is the ID of a Linux keyring key of
type "fscrypt-provisioning" whose payload is
struct fscrypt_provisioning_key_payload whose ``raw`` field contains
the raw key and whose ``type`` field matches ``key_spec.type``.
@@ -748,8 +755,15 @@ as follows:
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.
+ key when ``raw_flags == FSCRYPT_KEY_ADD_RAW_ASIS``,
+ ``raw_size`` bytes long. Alternatively, if
+ ``raw_flags == FSCRYPT_KEY_ADD_RAW_DESC``, ``raw`` is interpreted
+ as the key description of an encrypted or trusted key, in that order.
+ The material of this key will be used as if it were a raw key
+ supplied by userspace.
+
+ In both cases, the buffer is ``raw_size`` bytes long. If ````key_id``
+ is nonzero, then this field is unused.
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
diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c
index 0b3ffbb4faf4..484f7c883b17 100644
--- a/fs/crypto/keyring.c
+++ b/fs/crypto/keyring.c
@@ -20,6 +20,9 @@
#include <crypto/skcipher.h>
#include <linux/key-type.h>
+#include <linux/key-type.h>
+#include <keys/encrypted-type.h>
+#include <keys/trusted-type.h>
#include <linux/random.h>
#include <linux/seq_file.h>
@@ -662,13 +665,57 @@ int fscrypt_ioctl_add_key(struct file *filp, void __user *_uarg)
if (err)
goto out_wipe_secret;
} else {
- if (arg.raw_size < FSCRYPT_MIN_KEY_SIZE ||
- arg.raw_size > FSCRYPT_MAX_KEY_SIZE)
+ struct key *keyring_key = ERR_PTR(-EINVAL);
+ const void *key_material;
+ const char *desc;
+
+ switch (arg.raw_flags) {
+ case FSCRYPT_KEY_ADD_RAW_ASIS:
+ if (arg.raw_size < FSCRYPT_MIN_KEY_SIZE ||
+ arg.raw_size > FSCRYPT_MAX_KEY_SIZE)
+ return -EINVAL;
+ secret.size = arg.raw_size;
+ err = -EFAULT;
+ if (copy_from_user(secret.raw, uarg->raw, secret.size))
+ goto out_wipe_secret;
+ break;
+ case FSCRYPT_KEY_ADD_RAW_DESC:
+ if (arg.raw_size > 4096)
+ return -EINVAL;
+ desc = memdup_user_nul(uarg->raw, arg.raw_size);
+ if (IS_ERR(desc))
+ return PTR_ERR(desc);
+
+ if (IS_REACHABLE(CONFIG_ENCRYPTED_KEYS))
+ keyring_key = request_key(&key_type_encrypted, desc, NULL);
+ if (IS_REACHABLE(CONFIG_TRUSTED_KEYS) && IS_ERR(keyring_key))
+ keyring_key = request_key(&key_type_trusted, desc, NULL);
+
+ kfree(desc);
+
+ if (IS_ERR(keyring_key))
+ return PTR_ERR(keyring_key);
+
+ down_read(&keyring_key->sem);
+
+ key_material = key_extract_material(keyring_key, &secret.size);
+ if (!IS_ERR(key_material) && (secret.size < FSCRYPT_MIN_KEY_SIZE ||
+ secret.size > FSCRYPT_MAX_KEY_SIZE))
+ key_material = ERR_PTR(-EINVAL);
+ if (IS_ERR(key_material)) {
+ up_read(&keyring_key->sem);
+ key_put(keyring_key);
+ return PTR_ERR(key_material);
+ }
+
+ memcpy(secret.raw, key_material, secret.size);
+
+ up_read(&keyring_key->sem);
+ key_put(keyring_key);
+ break;
+ default:
return -EINVAL;
- secret.size = arg.raw_size;
- err = -EFAULT;
- if (copy_from_user(secret.raw, uarg->raw, secret.size))
- goto out_wipe_secret;
+ }
}
err = add_master_key(sb, &secret, &arg.key_spec);
diff --git a/include/uapi/linux/fscrypt.h b/include/uapi/linux/fscrypt.h
index 9f4428be3e36..bd498a188cf5 100644
--- a/include/uapi/linux/fscrypt.h
+++ b/include/uapi/linux/fscrypt.h
@@ -119,12 +119,26 @@ struct fscrypt_provisioning_key_payload {
__u8 raw[];
};
+/*
+ * fscrypt_add_key_arg::raw contains the raw key material directly
+ * if key_id == 0
+ */
+#define FSCRYPT_KEY_ADD_RAW_ASIS 0
+
+/*
+ * fscrypt_add_key_arg::raw is a key descriptor for an already
+ * existing kernel encrypted or trusted key if key_id == 0.
+ * The kernel key's material will be used as input for fscrypt.
+ */
+#define FSCRYPT_KEY_ADD_RAW_DESC 1
+
/* Struct passed to FS_IOC_ADD_ENCRYPTION_KEY */
struct fscrypt_add_key_arg {
struct fscrypt_key_specifier key_spec;
__u32 raw_size;
__u32 key_id;
- __u32 __reserved[8];
+ __u32 raw_flags; /* one of FSCRYPT_KEY_ADD_RAW_* */
+ __u32 __reserved[7];
__u8 raw[];
};
--
2.30.2
^ permalink raw reply related
* Re: [RFC][PATCH v2 02/12] diglim: Basic definitions
From: Greg KH @ 2021-07-27 14:43 UTC (permalink / raw)
To: Roberto Sassu
Cc: zohar, mchehab+huawei, linux-integrity, linux-security-module,
linux-doc, linux-kselftest, linux-kernel
In-Reply-To: <20210726163700.2092768-3-roberto.sassu@huawei.com>
On Mon, Jul 26, 2021 at 06:36:50PM +0200, Roberto Sassu wrote:
> --- /dev/null
> +++ b/include/uapi/linux/diglim.h
> @@ -0,0 +1,51 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/*
> + * Copyright (C) 2017-2021 Huawei Technologies Duesseldorf GmbH
> + *
> + * Author: Roberto Sassu <roberto.sassu@huawei.com>
> + *
> + * DIGLIM definitions exported to user space, useful for generating digest
> + * lists.
> + */
> +
> +#ifndef _UAPI__LINUX_DIGLIM_H
> +#define _UAPI__LINUX_DIGLIM_H
> +
> +#include <linux/types.h>
> +#include <linux/hash_info.h>
> +
> +enum compact_types { COMPACT_KEY, COMPACT_PARSER, COMPACT_FILE,
> + COMPACT_METADATA, COMPACT_DIGEST_LIST, COMPACT__LAST };
> +
> +enum compact_modifiers { COMPACT_MOD_IMMUTABLE, COMPACT_MOD__LAST };
> +
> +enum compact_actions { COMPACT_ACTION_IMA_MEASURED,
> + COMPACT_ACTION_IMA_APPRAISED,
> + COMPACT_ACTION_IMA_APPRAISED_DIGSIG,
> + COMPACT_ACTION__LAST };
> +
> +enum ops { DIGEST_LIST_ADD, DIGEST_LIST_DEL, DIGEST_LIST_OP__LAST };
> +
> +/**
> + * struct compact_list_hdr - header of the following concatenated digests
> + * @version: version of the digest list
> + * @_reserved: field reserved for future use
> + * @type: type of digest list among enum compact_types
> + * @modifiers: additional attributes among (1 << enum compact_modifiers)
I do not understand this description, what does it mean?
> + * @algo: digest algorithm
Is this also a #define or an enum? Where is the list of them?
> + * @count: number of digests
> + * @datalen: length of concatenated digests
Where does this count and length come into play as nothing else is in
this structure?
> + *
> + * A digest list is a set of blocks composed by struct compact_list_hdr and
> + * the following concatenated digests.
> + */
> +struct compact_list_hdr {
> + __u8 version;
> + __u8 _reserved;
You MUST check this for 0 today, and document it above. If not, you can
never use it in the future.
> + __le16 type;
> + __le16 modifiers;
> + __le16 algo;
> + __le32 count;
> + __le32 datalen;
> +} __packed;
> +#endif /*_UAPI__LINUX_DIGLIM_H*/
> --
> 2.25.1
>
^ permalink raw reply
* Re: [PATCH v2] KEYS: trusted: fix use as module when CONFIG_TCG_TPM=m
From: Ahmad Fatoum @ 2021-07-27 4:24 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: James Morris, Serge E. Hallyn, James Bottomley, Mimi Zohar,
Sumit Garg, David Howells, Herbert Xu, David S. Miller, kernel,
Andreas Rammhold, David Gstir, Richard Weinberger, keyrings,
linux-crypto, linux-kernel, linux-security-module,
linux-integrity
In-Reply-To: <20210727030433.3dwod2elwtdkhwsc@kernel.org>
On 27.07.21 05:04, Jarkko Sakkinen wrote:
>> Reported-by: Andreas Rammhold <andreas@rammhold.de>
>> Fixes: 5d0682be3189 ("KEYS: trusted: Add generic trusted keys framework")
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>
> Is it absolutely need to do all this *just* to fix the bug?
>
> For a pure bug fix the most essential thing is to be able the backport
> it to stable kernels.
Not much happened in-between, so a backport should be trivial.
I can provide these if needed.
> I don't really care at all about extra niceties ("it's now possible
> stuff).
>
> This looks like a bug fix and improvements bundle into a single patch.
You can replace the #ifdefs with #if IS_REACHABLE in trusted_core.c to fix the
intermediate breakage and then throw that away again to fix the remaining
dependency of trusted keys on TCG_TPM.
If you prefer that, Andreas perhaps could respin his series with
s/IS_ENABLED/IS_REACHABLE/?
Cheers,
Ahmad
>
> /Jarkko
>
--
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][V2] security: keys: trusted: Fix memory leaks on allocated blob
From: Jarkko Sakkinen @ 2021-07-27 3:13 UTC (permalink / raw)
To: Colin King
Cc: James Bottomley, Mimi Zohar, David Howells, James Morris,
Serge E . Hallyn, linux-integrity, keyrings,
linux-security-module, kernel-janitors, linux-kernel
In-Reply-To: <20210726114431.18042-1-colin.king@canonical.com>
On Mon, Jul 26, 2021 at 12:44:31PM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> There are several error return paths that don't kfree the allocated
> blob, leading to memory leaks. Ensure blob is initialized to null as
> some of the error return paths in function tpm2_key_decode do not
> change blob. Add an error return path to kfree blob and use this on
> the current leaky returns.
>
> Addresses-Coverity: ("Resource leak")
> Fixes: f2219745250f ("security: keys: trusted: use ASN.1 TPM2 key format for the blobs")
> Acked-by: Sumit Garg <sumit.garg@linaro.org>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>
> ---
>
> V2: Add a couple more leaky return path fixes as noted by Sumit Garg
> Add the if (blob != payload->blob) check on the kfree as
> noted by Dan Carpenter
>
> ---
> security/keys/trusted-keys/trusted_tpm2.c | 39 ++++++++++++++++-------
> 1 file changed, 27 insertions(+), 12 deletions(-)
>
> diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c
> index 0165da386289..a2cfdfdf17fa 100644
> --- a/security/keys/trusted-keys/trusted_tpm2.c
> +++ b/security/keys/trusted-keys/trusted_tpm2.c
> @@ -366,7 +366,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
> unsigned int private_len;
> unsigned int public_len;
> unsigned int blob_len;
> - u8 *blob, *pub;
> + u8 *blob = NULL, *pub;
> int rc;
> u32 attrs;
>
> @@ -378,22 +378,30 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
> }
>
> /* new format carries keyhandle but old format doesn't */
> - if (!options->keyhandle)
> - return -EINVAL;
> + if (!options->keyhandle) {
> + rc = -EINVAL;
> + goto err;
> + }
>
> /* must be big enough for at least the two be16 size counts */
> - if (payload->blob_len < 4)
> - return -EINVAL;
> + if (payload->blob_len < 4) {
> + rc = -EINVAL;
> + goto err;
> + }
>
> private_len = get_unaligned_be16(blob);
>
> /* must be big enough for following public_len */
> - if (private_len + 2 + 2 > (payload->blob_len))
> - return -E2BIG;
> + if (private_len + 2 + 2 > (payload->blob_len)) {
> + rc = -E2BIG;
> + goto err;
> + }
>
> public_len = get_unaligned_be16(blob + 2 + private_len);
> - if (private_len + 2 + public_len + 2 > payload->blob_len)
> - return -E2BIG;
> + if (private_len + 2 + public_len + 2 > payload->blob_len) {
> + rc = -E2BIG;
> + goto err;
> + }
>
> pub = blob + 2 + private_len + 2;
> /* key attributes are always at offset 4 */
> @@ -406,12 +414,14 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
> payload->migratable = 1;
>
> blob_len = private_len + public_len + 4;
> - if (blob_len > payload->blob_len)
> - return -E2BIG;
> + if (blob_len > payload->blob_len) {
> + rc = -E2BIG;
> + goto err;
> + }
>
> rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_LOAD);
> if (rc)
> - return rc;
> + goto err;
>
> tpm_buf_append_u32(&buf, options->keyhandle);
> tpm2_buf_append_auth(&buf, TPM2_RS_PW,
> @@ -441,6 +451,11 @@ static int tpm2_load_cmd(struct tpm_chip *chip,
> rc = -EPERM;
>
> return rc;
> +
> +err:
> + if (blob != payload->blob)
> + kfree(blob);
> + return rc;
> }
>
> /**
> --
> 2.31.1
>
>
Just denoting that I saw this, so just response to my other email,
and I'll use this one.
/Jarkko
^ permalink raw reply
* Re: [PATCH] security: keys: trusted: Fix memory leaks on allocated blob
From: Jarkko Sakkinen @ 2021-07-27 3:05 UTC (permalink / raw)
To: Colin King
Cc: James Bottomley, Mimi Zohar, David Howells, James Morris,
Serge E . Hallyn, linux-integrity, keyrings,
linux-security-module, kernel-janitors, linux-kernel
In-Reply-To: <20210723172121.156687-1-colin.king@canonical.com>
On Fri, Jul 23, 2021 at 06:21:21PM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> There are several error return paths that don't kfree the allocated
> blob, leading to memory leaks. Ensure blob is initialized to null as
> some of the error return paths in function tpm2_key_decode do not
> change blob. Add an error return path to kfree blob and use this on
> the current leaky returns.
>
> Addresses-Coverity: ("Resource leak")
> Fixes: f2219745250f ("security: keys: trusted: use ASN.1 TPM2 key format for the blobs")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
Probably makes sense (for me) to add also
Cc: stable@vger.kernel.org
?
/Jarkko
^ permalink raw reply
* Re: [PATCH v2] KEYS: trusted: fix use as module when CONFIG_TCG_TPM=m
From: Jarkko Sakkinen @ 2021-07-27 3:04 UTC (permalink / raw)
To: Ahmad Fatoum
Cc: James Morris, Serge E. Hallyn, James Bottomley, Mimi Zohar,
Sumit Garg, David Howells, Herbert Xu, David S. Miller, kernel,
Andreas Rammhold, David Gstir, Richard Weinberger, keyrings,
linux-crypto, linux-kernel, linux-security-module,
linux-integrity
In-Reply-To: <20210721160258.7024-1-a.fatoum@pengutronix.de>
On Wed, Jul 21, 2021 at 06:02:59PM +0200, Ahmad Fatoum wrote:
> Since commit 5d0682be3189 ("KEYS: trusted: Add generic trusted keys
> framework"), trusted.ko built with CONFIG_TCG_TPM=CONFIG_TRUSTED_KEYS=m
> will not register the TPM trusted key type at runtime.
>
> This is because, after that rework, CONFIG_DEPENDENCY of the TPM
> and TEE backends were checked with #ifdef, but that's only true
> when they're built-in.
>
> Fix this by introducing two new boolean Kconfig symbols:
> TRUSTED_KEYS_TPM and TRUSTED_KEYS_TEE with the appropriate
> dependencies and use them to check which backends are available.
>
> This also has a positive effect on user experience:
>
> - It's now possible to use TEE trusted keys without CONFIG_TCG_TPM
> - It's now possible to enable CONFIG_TCG_TPM, but exclude TPM from
> available trust sources
> - TEE=m && TRUSTED_KEYS=y no longer leads to TEE support
> being silently dropped
>
> Any code depending on the TPM trusted key backend or symbols exported
> by it will now need to explicitly state that it
>
> depends on TRUSTED_KEYS && TRUSTED_KEYS_TPM
>
> The latter to ensure the dependency is built and the former to ensure
> it's reachable for module builds. This currently only affects
> CONFIG_ASYMMETRIC_TPM_KEY_SUBTYPE, so it's fixed up here as well.
>
> Reported-by: Andreas Rammhold <andreas@rammhold.de>
> Fixes: 5d0682be3189 ("KEYS: trusted: Add generic trusted keys framework")
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Is it absolutely need to do all this *just* to fix the bug?
For a pure bug fix the most essential thing is to be able the backport
it to stable kernels.
I don't really care at all about extra niceties ("it's now possible
stuff).
This looks like a bug fix and improvements bundle into a single patch.
/Jarkko
^ permalink raw reply
* Re: [PATCH] tpm: ibmvtpm: Avoid error message when process gets signal while waiting
From: Stefan Berger @ 2021-07-27 3:00 UTC (permalink / raw)
To: Jarkko Sakkinen, Stefan Berger
Cc: peterhuewe, jgg, linux-integrity, linux-security-module,
linux-kernel, Nayna Jain, George Wilson, Nageswara R Sastry
In-Reply-To: <20210727024225.swqy5ypcytsngpd6@kernel.org>
On 7/26/21 10:42 PM, Jarkko Sakkinen wrote:
> On Mon, Jul 12, 2021 at 12:25:05PM -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 configure 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);
>>
>> The solution is to use wait_event() instead.
> Why?
So it becomes uninterruptible and these error messages go away.
Stefan
>
> /Jarkko
^ permalink raw reply
* Re: [PATCH] KEYS: trusted: Fix trusted key backends when building as module
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
In-Reply-To: <0a684d56-66d0-184e-4853-9faafa2d243d@pengutronix.de>
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
* Re: [PATCH] KEYS: trusted: Fix trusted key backends when building as module
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
In-Reply-To: <20210716081722.4130161-1-andreas@rammhold.de>
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
* Re: [PATCH RFC 0/9] sk_buff: optimize layout for GRO
From: Paul Moore @ 2021-07-27 2:51 UTC (permalink / raw)
To: Casey Schaufler
Cc: Florian Westphal, Paolo Abeni, netdev, David S. Miller,
Jakub Kicinski, Eric Dumazet, linux-security-module, selinux
In-Reply-To: <d0186e8f-41f8-7d4d-5c2c-706bfe3c30cc@schaufler-ca.com>
On Mon, Jul 26, 2021 at 11:13 AM Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 7/25/2021 3:52 PM, Florian Westphal wrote:
> > Casey Schaufler <casey@schaufler-ca.com> wrote:
> >> RedHat and android use SELinux and will want this. Ubuntu doesn't
> >> yet, but netfilter in in the AppArmor task list. Tizen definitely
> >> uses it with Smack. The notion that security modules are only used
> >> in fringe cases is antiquated.
> > I was not talking about LSM in general, I was referring to the
> > extended info that Paul mentioned.
> >
> > If thats indeed going to be used on every distro then skb extensions
> > are not suitable for this, it would result in extr akmalloc for every
> > skb.
>
> I am explicitly talking about the use of secmarks. All my
> references are uses of secmarks.
I'm talking about a void* which would contain LSM specific data; as I
said earlier, think of inodes. This LSM specific data would include
the existing secmark data as well as network peer security information
which would finally (!!!) allow us to handle forwarded traffic and
enable a number of other fixes and performance improvements.
(The details are a bit beyond this discussion but it basically
revolves around us not having to investigate the import the packet
headers every time we want to determine the network peer security
attributes, we could store the resolved LSM information in the
sk_buff.security blob.)
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [PATCH] tpm: ibmvtpm: Avoid error message when process gets signal while waiting
From: Jarkko Sakkinen @ 2021-07-27 2:42 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: <20210712162505.205943-1-stefanb@linux.vnet.ibm.com>
On Mon, Jul 12, 2021 at 12:25:05PM -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 configure 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);
>
> The solution is to use wait_event() instead.
Why?
/Jarkko
^ permalink raw reply
* Re: [PATCH 1/2] net: cipso: fix warnings in netlbl_cipsov4_add_std
From: Paul Moore @ 2021-07-27 2:40 UTC (permalink / raw)
To: Pavel Skripkin
Cc: davem, yoshfuji, dsahern, kuba, netdev, linux-security-module,
linux-kernel, syzbot+cdd51ee2e6b0b2e18c0d
In-Reply-To: <20210726141140.24e8db78@gmail.com>
On Mon, Jul 26, 2021 at 7:11 AM Pavel Skripkin <paskripkin@gmail.com> wrote:
> On Sat, 10 Jul 2021 10:03:13 +0300
> Pavel Skripkin <paskripkin@gmail.com> wrote:
>
> > Syzbot reported warning in netlbl_cipsov4_add(). The
> > problem was in too big doi_def->map.std->lvl.local_size
> > passed to kcalloc(). Since this value comes from userpace there is
> > no need to warn if value is not correct.
> >
> > The same problem may occur with other kcalloc() calls in
> > this function, so, I've added __GFP_NOWARN flag to all
> > kcalloc() calls there.
> >
> > Reported-and-tested-by:
> > syzbot+cdd51ee2e6b0b2e18c0d@syzkaller.appspotmail.com Fixes:
> > 96cb8e3313c7 ("[NetLabel]: CIPSOv4 and Unlabeled packet integration")
> > Signed-off-by: Pavel Skripkin <paskripkin@gmail.com> ---
> > net/netlabel/netlabel_cipso_v4.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
>
> Hi, net developers!
>
> Is this patch merged somewhere? I've checked net tree and Paul Moore
> tree on https://git.kernel.org/, but didn't find it. Did I miss it
> somewhere? If not, it's just a gentle ping :)
>
> Btw: maybe I should send it as separete patch, since 2/2 in this
> series is invalid as already in-tree?
I'm not sure why this hasn't been picked up yet, but I suppose
resubmitting just this patch couldn't hurt. Don't forget to include
my ACK if you do.
--
paul moore
www.paul-moore.com
^ permalink raw reply
* [PATCH RFC v2 06/12] integrity: accessor function to get trust_moklist
From: Eric Snowberg @ 2021-07-26 17:13 UTC (permalink / raw)
To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert, davem,
jarkko, jmorris, serge
Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
linux-kernel, linux-crypto, linux-security-module,
James.Bottomley, pjones, glin, konrad.wilk
In-Reply-To: <20210726171319.3133879-1-eric.snowberg@oracle.com>
Add an accessor function to see if the mok list should be trusted.
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v1: Initial version
v2: Added trust_moklist function
---
security/integrity/integrity.h | 5 +++++
security/integrity/platform_certs/mok_keyring.c | 16 ++++++++++++++++
2 files changed, 21 insertions(+)
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 60d5c7ba05b2..1fcefceb0da1 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -279,6 +279,7 @@ integrity_audit_log_start(struct audit_context *ctx, gfp_t gfp_mask, int type)
void __init add_to_platform_keyring(const char *source, const void *data,
size_t len);
void __init add_to_mok_keyring(const char *source, const void *data, size_t len);
+bool __init trust_moklist(void);
#else
static inline void __init add_to_platform_keyring(const char *source,
const void *data, size_t len)
@@ -287,4 +288,8 @@ static inline void __init add_to_platform_keyring(const char *source,
void __init add_to_mok_keyring(const char *source, const void *data, size_t len)
{
}
+static inline bool __init trust_moklist(void)
+{
+ return false;
+}
#endif
diff --git a/security/integrity/platform_certs/mok_keyring.c b/security/integrity/platform_certs/mok_keyring.c
index f260edac0863..c7820d9136f3 100644
--- a/security/integrity/platform_certs/mok_keyring.c
+++ b/security/integrity/platform_certs/mok_keyring.c
@@ -8,6 +8,8 @@
#include <linux/efi.h>
#include "../integrity.h"
+bool trust_mok;
+
static __init int mok_keyring_init(void)
{
int rc;
@@ -67,3 +69,17 @@ static __init bool uefi_check_trust_mok_keys(void)
*/
return (status == EFI_SUCCESS && (!(attr & EFI_VARIABLE_NON_VOLATILE)));
}
+
+bool __init trust_moklist(void)
+{
+ static bool initialized;
+
+ if (!initialized) {
+ initialized = true;
+
+ if (uefi_check_trust_mok_keys())
+ trust_mok = true;
+ }
+
+ return trust_mok;
+}
--
2.18.4
^ permalink raw reply related
* [PATCH RFC v2 08/12] integrity: Suppress error message for keys added to the mok keyring
From: Eric Snowberg @ 2021-07-26 17:13 UTC (permalink / raw)
To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert, davem,
jarkko, jmorris, serge
Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
linux-kernel, linux-crypto, linux-security-module,
James.Bottomley, pjones, glin, konrad.wilk
In-Reply-To: <20210726171319.3133879-1-eric.snowberg@oracle.com>
Suppress the error message for keys added to the mok keyring. If an
error occurs, the key will be added to the platform keyring instead.
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v1: Initial version
v2: Unmodified from v1
---
security/integrity/digsig.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index 2f6898c89f60..be4860c596b9 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -165,7 +165,8 @@ static int __init integrity_add_key(const unsigned int id, const void *data,
KEY_ALLOC_NOT_IN_QUOTA);
if (IS_ERR(key)) {
rc = PTR_ERR(key);
- pr_err("Problem loading X.509 certificate %d\n", rc);
+ if (id != INTEGRITY_KEYRING_MOK)
+ pr_err("Problem loading X.509 certificate %d\n", rc);
} else {
pr_notice("Loaded X.509 cert '%s'\n",
key_ref_to_ptr(key)->description);
--
2.18.4
^ permalink raw reply related
* [PATCH RFC v2 09/12] KEYS: add a reference to mok keyring
From: Eric Snowberg @ 2021-07-26 17:13 UTC (permalink / raw)
To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert, davem,
jarkko, jmorris, serge
Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
linux-kernel, linux-crypto, linux-security-module,
James.Bottomley, pjones, glin, konrad.wilk
In-Reply-To: <20210726171319.3133879-1-eric.snowberg@oracle.com>
Expose the .mok keyring created in integrity code by adding
a reference. This makes the mok keyring accessible for keyring
restrictions in the future.
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v2: Initial version
---
certs/system_keyring.c | 5 +++++
include/keys/system_keyring.h | 4 ++++
2 files changed, 9 insertions(+)
diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index 0a7b16c28a72..dcaf74102ab2 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -27,6 +27,7 @@ static struct key *secondary_trusted_keys;
#endif
#ifdef CONFIG_INTEGRITY_PLATFORM_KEYRING
static struct key *platform_trusted_keys;
+static struct key *mok_trusted_keys;
#endif
extern __initconst const u8 system_certificate_list[];
@@ -317,4 +318,8 @@ void __init set_platform_trusted_keys(struct key *keyring)
{
platform_trusted_keys = keyring;
}
+void __init set_mok_trusted_keys(struct key *keyring)
+{
+ mok_trusted_keys = keyring;
+}
#endif
diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
index 2041254d74f4..1adf78ddc035 100644
--- a/include/keys/system_keyring.h
+++ b/include/keys/system_keyring.h
@@ -94,10 +94,14 @@ static inline struct key *get_ima_blacklist_keyring(void)
#if defined(CONFIG_INTEGRITY_PLATFORM_KEYRING) && \
defined(CONFIG_SYSTEM_TRUSTED_KEYRING)
extern void __init set_platform_trusted_keys(struct key *keyring);
+extern void __init set_mok_trusted_keys(struct key *keyring);
#else
static inline void set_platform_trusted_keys(struct key *keyring)
{
}
+static void __init set_mok_trusted_keys(struct key *keyring)
+{
+}
#endif
#endif /* _KEYS_SYSTEM_KEYRING_H */
--
2.18.4
^ permalink raw reply related
* [PATCH RFC v2 02/12] KEYS: CA link restriction
From: Eric Snowberg @ 2021-07-26 17:13 UTC (permalink / raw)
To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert, davem,
jarkko, jmorris, serge
Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
linux-kernel, linux-crypto, linux-security-module,
James.Bottomley, pjones, glin, konrad.wilk
In-Reply-To: <20210726171319.3133879-1-eric.snowberg@oracle.com>
Add a new link restriction. Restrict the addition of keys in a keyring
based on the key to be added being a CA (self-signed) or by being
vouched for by a key in either the built-in or the secondary trusted
keyrings.
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v1: Initial version
v2: Removed secondary keyring references
---
certs/system_keyring.c | 21 +++++++++++
crypto/asymmetric_keys/restrict.c | 60 +++++++++++++++++++++++++++++++
include/crypto/public_key.h | 5 +++
include/keys/system_keyring.h | 6 ++++
4 files changed, 92 insertions(+)
diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index 692365dee2bd..0a7b16c28a72 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -21,6 +21,9 @@
static struct key *builtin_trusted_keys;
#ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
static struct key *secondary_trusted_keys;
+#define system_trusted_keys secondary_trusted_keys
+#else
+#define system_trusted_keys builtin_trusted_keys
#endif
#ifdef CONFIG_INTEGRITY_PLATFORM_KEYRING
static struct key *platform_trusted_keys;
@@ -45,6 +48,24 @@ int restrict_link_by_builtin_trusted(struct key *dest_keyring,
builtin_trusted_keys);
}
+/**
+ * restrict_link_by_system_trusted_or_ca - Restrict keyring
+ * addition by being a CA or vouched by the system trusted keyrings.
+ *
+ * Restrict the addition of keys in a keyring based on the key-to-be-added
+ * being a CA (self signed) or by being vouched for by a key in either
+ * the built-in or the secondary system keyrings.
+ */
+int restrict_link_by_system_trusted_or_ca(
+ struct key *dest_keyring,
+ const struct key_type *type,
+ const union key_payload *payload,
+ struct key *restrict_key)
+{
+ return restrict_link_by_ca(dest_keyring, type, payload,
+ system_trusted_keys);
+}
+
#ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
/**
* restrict_link_by_builtin_and_secondary_trusted - Restrict keyring
diff --git a/crypto/asymmetric_keys/restrict.c b/crypto/asymmetric_keys/restrict.c
index 84cefe3b3585..75e4379226e8 100644
--- a/crypto/asymmetric_keys/restrict.c
+++ b/crypto/asymmetric_keys/restrict.c
@@ -108,6 +108,66 @@ int restrict_link_by_signature(struct key *dest_keyring,
return ret;
}
+/**
+ * restrict_link_by_ca - Restrict additions to a ring of public keys
+ * based on it being a CA
+ * @dest_keyring: Keyring being linked to.
+ * @type: The type of key being added.
+ * @payload: The payload of the new key.
+ * @trusted: A key or ring of keys that can be used to vouch for the new cert.
+ *
+ * Check if the new certificate is a CA or if they key can be vouched for
+ * by keys already linked in the destination keyring or the trusted
+ * keyring. If one of those is the signing key or it is self signed, then
+ * mark the new certificate as being ok to link.
+ *
+ * Returns 0 if the new certificate was accepted, -ENOKEY if we could not find
+ * a matching parent certificate in the trusted list. -ENOPKG if the signature
+ * uses unsupported crypto, or some other error if there is a matching
+ * certificate but the signature check cannot be performed.
+ */
+int restrict_link_by_ca(struct key *dest_keyring,
+ const struct key_type *type,
+ const union key_payload *payload,
+ struct key *trust_keyring)
+{
+ const struct public_key_signature *sig;
+ const struct public_key *pkey;
+ struct key *key;
+ int ret;
+
+ if (type != &key_type_asymmetric)
+ return -EOPNOTSUPP;
+
+ sig = payload->data[asym_auth];
+ if (!sig)
+ return -ENOPKG;
+
+ if (!sig->auth_ids[0] && !sig->auth_ids[1])
+ return -ENOKEY;
+
+ pkey = payload->data[asym_crypto];
+ if (!pkey)
+ return -ENOPKG;
+
+ ret = public_key_verify_signature(pkey, sig);
+ if (!ret)
+ return 0;
+
+ if (!trust_keyring)
+ return -ENOKEY;
+
+ key = find_asymmetric_key(trust_keyring,
+ sig->auth_ids[0], sig->auth_ids[1],
+ false);
+ if (IS_ERR(key))
+ return -ENOKEY;
+
+ ret = verify_signature(key, sig);
+ key_put(key);
+ return ret;
+}
+
static bool match_either_id(const struct asymmetric_key_ids *pair,
const struct asymmetric_key_id *single)
{
diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
index 47accec68cb0..545af1ea57de 100644
--- a/include/crypto/public_key.h
+++ b/include/crypto/public_key.h
@@ -71,6 +71,11 @@ extern int restrict_link_by_key_or_keyring_chain(struct key *trust_keyring,
const union key_payload *payload,
struct key *trusted);
+extern int restrict_link_by_ca(struct key *dest_keyring,
+ const struct key_type *type,
+ const union key_payload *payload,
+ struct key *trust_keyring);
+
extern int query_asymmetric_key(const struct kernel_pkey_params *,
struct kernel_pkey_query *);
diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
index 6acd3cf13a18..2041254d74f4 100644
--- a/include/keys/system_keyring.h
+++ b/include/keys/system_keyring.h
@@ -28,6 +28,12 @@ static inline __init int load_module_cert(struct key *keyring)
#endif
+extern int restrict_link_by_system_trusted_or_ca(
+ struct key *dest_keyring,
+ const struct key_type *type,
+ const union key_payload *payload,
+ struct key *restrict_key);
+
#ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
extern int restrict_link_by_builtin_and_secondary_trusted(
struct key *keyring,
--
2.18.4
^ permalink raw reply related
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