* Re: [PATCH v2] fscrypt: support trusted keys
From: Jarkko Sakkinen @ 2021-08-10 18:06 UTC (permalink / raw)
To: Eric Biggers
Cc: Ahmad Fatoum, Theodore Y. Ts'o, Jaegeuk Kim, kernel,
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: <YRGVcaquAJiuc8bp@gmail.com>
On Mon, Aug 09, 2021 at 01:52:01PM -0700, Eric Biggers wrote:
> On Mon, Aug 09, 2021 at 12:44:08PM +0300, Jarkko Sakkinen wrote:
> > > @@ -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) {
> >
> > Why does fscrypt have own key type, and does not extend 'encrypted' with a
> > new format [*]?
>
> Are you referring to the "fscrypt-provisioning" key type? That is an existing
> feature (which in most cases isn't used, but there is a use case that requires
> it), not something being added by this patch. We just needed a key type where
> userspace can add a raw key to the kernel and not be able to read it back (so
> like the "logon" key type), but also have the kernel enforce that that key is
> only used for fscrypt with a particular KDF version, and not with other random
> kernel features. The "encrypted" key type wouldn't have worked for this at all;
> it's a totally different thing.
>
> > > + } 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 {
> >
> >
> > I don't think this is right, or at least it does not follow the pattern
> > in [*]. I.e. you should rather use trusted key to seal your fscrypt key.
>
> What's the benefit of the extra layer of indirection over just using a "trusted"
> key directly? The use case for "encrypted" keys is not at all clear to me.
Because it is more robust to be able to use small amount of trusted keys,
which are not entirely software based.
And since it's also a pattern on existing kernel features utilizing trusted
keys, the pressure here to explain why break the pattern, should be on the
side of the one who breaks it.
/Jarkko
^ permalink raw reply
* Re: [PATCH v2] fscrypt: support trusted keys
From: Jarkko Sakkinen @ 2021-08-10 18:02 UTC (permalink / raw)
To: Ahmad Fatoum
Cc: Theodore Y. Ts'o, Jaegeuk Kim, Eric Biggers, kernel,
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: <10dac5c6-4530-217c-e1ea-a7e2e3572f43@pengutronix.de>
On Mon, Aug 09, 2021 at 12:00:40PM +0200, Ahmad Fatoum wrote:
> Hello Jarkko,
>
> On 09.08.21 11:44, Jarkko Sakkinen wrote:
> > On Fri, Aug 06, 2021 at 05:09:28PM +0200, Ahmad Fatoum wrote:
> >> 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
> >> - if (key->type != &key_type_fscrypt_provisioning)
> >> - goto bad_key;
> >> - payload = key->payload.data[0];
> >> + if (key->type == &key_type_fscrypt_provisioning) {
> >
> > Why does fscrypt have own key type, and does not extend 'encrypted' with a
> > new format [*]?
>
> See the commit[1] adding it for more information. TL;DR:
>
> fscrypt maintainers would've preferred keys to be associated with
> a "domain". So an encrypted key generated for fscrypt use couldn't be reused
> for e.g. dm-crypt. They are wary of fscrypt users being more exposed if their
> keys can be used with weaker ciphers via other kernel functionality that could
> be used to extract information about the raw key material.
>
> Eric also mentioned dislike of the possibility of rooting encrypted keys to
> user keys. v2 is only restricted to v2, so we didn't discuss this further.
>
> Restricting the key to fscrypt-only precludes this reuse.
>
> My commit makes no attempts in changing that. It just adds a new way to pass
> raw key material into fscrypt. For more information, see the commit[1] adding
> that key type.
>
> > [*] https://www.kernel.org/doc/html/v5.13/security/keys/trusted-encrypted.html
>
> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=93edd392ca
OK, so why does the trusted key does not seal a fscrypt key, but instead
its key material is directly used?
> Cheers,
> Ahmad
/Jarkko
^ permalink raw reply
* Re: [PATCH v4 2/2] tpm: ibmvtpm: Rename tpm_process_cmd to tpm_status and define flag
From: Jarkko Sakkinen @ 2021-08-10 17:58 UTC (permalink / raw)
To: Stefan Berger
Cc: nasastry, linux-integrity, linux-security-module, linux-kernel,
Stefan Berger, Nayna Jain, George Wilson
In-Reply-To: <20210809192159.2176580-3-stefanb@linux.vnet.ibm.com>
On Mon, Aug 09, 2021 at 03:21:59PM -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.
>
> 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 | 15 ++++++++-------
> drivers/char/tpm/tpm_ibmvtpm.h | 3 ++-
> 2 files changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
> index 7a9eca5768f8..5d795866b483 100644
> --- a/drivers/char/tpm/tpm_ibmvtpm.c
> +++ b/drivers/char/tpm/tpm_ibmvtpm.c
> @@ -215,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;
> }
> @@ -232,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,
> @@ -250,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);
> @@ -266,7 +267,7 @@ static u8 tpm_ibmvtpm_status(struct tpm_chip *chip)
> {
> struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(&chip->dev);
>
> - return ibmvtpm->tpm_processing_cmd;
> + return ibmvtpm->tpm_status;
> }
>
> /**
> @@ -454,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 = true,
> + .req_complete_mask = TPM_STATUS_BUSY,
> .req_complete_val = 0,
> .req_canceled = tpm_ibmvtpm_req_canceled,
> };
> @@ -547,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:
> diff --git a/drivers/char/tpm/tpm_ibmvtpm.h b/drivers/char/tpm/tpm_ibmvtpm.h
> index 51198b137461..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;
> - u8 tpm_processing_cmd;
> + u8 tpm_status;
> +#define TPM_STATUS_BUSY (1 << 0) /* vtpm is processing a command */
Declare this already in the fix, and just leave the rename here.
> };
>
> #define CRQ_RES_BUF_SIZE PAGE_SIZE
> --
> 2.31.1
>
>
Otherwise, these look fine.
/Jarkko
^ permalink raw reply
* Re: [PATCH v2] fscrypt: support trusted keys
From: Eric Biggers @ 2021-08-10 17:35 UTC (permalink / raw)
To: Ahmad Fatoum
Cc: Theodore Y. Ts'o, Jaegeuk Kim, kernel, 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: <2bc19003-82a1-0d2d-4548-3315686d77b4@pengutronix.de>
On Tue, Aug 10, 2021 at 09:41:20AM +0200, Ahmad Fatoum wrote:
> Hello Eric,
>
> On 09.08.21 23:24, Eric Biggers wrote:
> > Hi Ahmad,
> >
> > This generally looks okay, but I have some comments below.
> >
> > On Fri, Aug 06, 2021 at 05:09:28PM +0200, Ahmad Fatoum wrote:
> >> 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.
> >
> > Please be explicit about where and how the keys get generated in this case.
>
> I intentionally avoided talking about this. You see, the trusted key documentation[1]
> phrases it as "all keys are created in the kernel", but you consider
> "'The key material is generated
> within the kernel' [a] misleading claim'. [2]
>
> Also, I hope patches to force kernel RNG and CAAM support (using kernel RNG as
> default) will soon be accepted, which would invalidate any further claims in the
> commit message without a means to correct them.
>
> I thus restricted my commit message to the necessary bit that are needed to
> understand the patch, which is: userspace knowledge of the key material is
> not required. If you disagree, could you provide me the text you'd prefer?
Just write that the trusted key subsystem is responsible for generating the
keys. And please fix the trusted keys documentation to properly document key
generation, or better yet just fix the trusted keys subsystem to generate the
keys properly.
> >> 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.
> >
> > As far as I can see, "Key unsealing in userspace" actually is the preferred way
> > to implement TPM-bound encryption. So it doesn't seem fair to call it a "work
> > around".
>
> In the context of *kernel trusted keys*, direct interaction with the TPM
> outside the kernel to decrypt a kernel-encrypted blob is surely not the
> preferred way.
>
> For TPM-bound encryption completely in userspace? Maybe. But that's not
> what this patch is about. It's about kernel trusted keys and offloading
> part of its functionality to userspace to _work around_ lack of kernel-side
> integration is exactly that: a _work around_.
As I said before, there's no need for kernel trusted keys at all in cases where
the TPM userspace tools can be used. This is existing, well-documented process,
e.g. see: https://wiki.archlinux.org/title/Trusted_Platform_Module. You are
starting with a solution ("I'm going to use kernel trusted keys") and not a
problem ("I want my fscrypt key(s) to be TPM-bound"). So please fix this patch
to explain the situation(s) in which it actually solves a problem that isn't
already solved.
- Eric
^ permalink raw reply
* Re: [PATCH 0/4] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
From: David Gstir @ 2021-08-10 11:28 UTC (permalink / raw)
To: Ahmad Fatoum
Cc: Jarkko Sakkinen, Horia Geantă, Mimi Zohar, Aymen Sghaier,
Herbert Xu, David S. Miller, James Bottomley, Jan Luebbe,
Udit Agarwal, Sumit Garg, 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: <8321cac9-350b-1325-4b7e-390f4f292070@pengutronix.de>
Hi Ahmad,
> On 09.08.2021, at 12:16, Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
[...]
> If it interests you, I described[2] my CAAM+ubifs+fscrypt use case in the
> discussion thread on my fscrypt-trusted-keys v1. Jan, a colleague of mine, held a
> talk[3] on the different solutions for authenticated and encrypted storage, which
> you may want to check out.
>
> I'd really appreciate feedback here on the the CAAM parts of this series, so this can
> eventually go mainline.
Since you mention the fscrypt trusted-keys use case:
I noticed that the key length for trusted-keys is limited to
256 - 1024bit keys. fscrypt does however also support keys
with e.g. 128bit keys (AES-128-CBC-ESSIV, AES-128-CTS-CBC).
AFAIK, CAAM and TEE key blobs would also support key lengths outside the 256 - 1024bit range.
Wouldn’t it make sense to align the supported key lengths?
I.e. extend the range of supported key lengths for trusted keys.
Or is there a specific reason why key lengths below 256bit are
not supported by trusted-keys?
Cheers,
David
^ permalink raw reply
* Re: [PATCH 3/4] crypto: caam - add in-kernel interface for blob generator
From: David Gstir @ 2021-08-10 11:29 UTC (permalink / raw)
To: Ahmad Fatoum
Cc: Horia Geantă, Aymen Sghaier, Herbert Xu, David S. Miller,
kernel, James Bottomley, Jarkko Sakkinen, Mimi Zohar,
David Howells, James Morris, Eric Biggers, Serge E. Hallyn,
Udit Agarwal, Jan Luebbe, Richard Weinberger, Franck LENORMAND,
Sumit Garg, linux-integrity, keyrings, linux-crypto, linux-kernel,
linux-security-module
In-Reply-To: <4078060ab2e44114af8204b4defea4f3d4b9e285.1626885907.git-series.a.fatoum@pengutronix.de>
Hi Ahmad,
> On 21.07.2021, at 18:48, Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
[...]
> diff --git a/drivers/crypto/caam/blob_gen.c b/drivers/crypto/caam/blob_gen.c
> new file mode 100644
> index 000000000000..513d3f90e438
> --- /dev/null
> +++ b/drivers/crypto/caam/blob_gen.c
> @@ -0,0 +1,230 @@
[...]
> +
> +int caam_encap_blob(struct caam_blob_priv *priv, const char *keymod,
> + void *input, void *output, size_t length)
> +{
> + u32 *desc;
> + struct device *jrdev = &priv->jrdev;
> + dma_addr_t dma_in, dma_out;
> + struct caam_blob_job_result testres;
> + size_t keymod_len = strlen(keymod);
> + int ret;
> +
> + if (length <= CAAM_BLOB_OVERHEAD || keymod_len > CAAM_BLOB_KEYMOD_LENGTH)
The docs for this function mention the length <= CAAM_BLOB_MAX_LEN
restriction. This is not checked here. Is this intended?
Since you already assert that MAX_BLOB_SIZE <= CAAM_BLOB_MAX_LEN
in security/keys/trusted-keys/trusted_caam.c, this will never
be an issue for CAAM-based trusted-keys though.
> + return -EINVAL;
> +
> + desc = caam_blob_alloc_desc(keymod_len);
> + if (!desc) {
> + dev_err(jrdev, "unable to allocate desc\n");
> + return -ENOMEM;
> + }
> +
[...]
> diff --git a/include/soc/fsl/caam-blob.h b/include/soc/fsl/caam-blob.h
> new file mode 100644
> index 000000000000..aebbc9335f64
> --- /dev/null
> +++ b/include/soc/fsl/caam-blob.h
> @@ -0,0 +1,56 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2020 Pengutronix, Ahmad Fatoum <kernel@pengutronix.de>
> + */
> +
> +#ifndef __CAAM_BLOB_GEN
> +#define __CAAM_BLOB_GEN
> +
> +#include <linux/types.h>
> +
> +#define CAAM_BLOB_KEYMOD_LENGTH 16
> +#define CAAM_BLOB_OVERHEAD (32 + 16)
> +#define CAAM_BLOB_MAX_LEN 4096
> +
> +struct caam_blob_priv;
> +
> +/** caam_blob_gen_init - initialize blob generation
> + *
> + * returns either pointer to new caam_blob_priv instance
> + * or error pointer
> + */
> +struct caam_blob_priv *caam_blob_gen_init(void);
> +
> +/** caam_blob_gen_init - free blob generation resources
s/init/exit/
> + *
> + * @priv: instance returned by caam_blob_gen_init
> + */
> +void caam_blob_gen_exit(struct caam_blob_priv *priv);
Except these minor things, I noticed no issues with this whole series:
Reviewed-by: David Gstir <david@sigma-star.at>
^ permalink raw reply
* Re: [PATCH v2] fscrypt: support trusted keys
From: Ahmad Fatoum @ 2021-08-10 7:41 UTC (permalink / raw)
To: Eric Biggers
Cc: Theodore Y. Ts'o, Jaegeuk Kim, kernel, 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: <YRGdBiJQ3xqZAT4w@gmail.com>
Hello Eric,
On 09.08.21 23:24, Eric Biggers wrote:
> Hi Ahmad,
>
> This generally looks okay, but I have some comments below.
>
> On Fri, Aug 06, 2021 at 05:09:28PM +0200, Ahmad Fatoum wrote:
>> 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.
>
> Please be explicit about where and how the keys get generated in this case.
I intentionally avoided talking about this. You see, the trusted key documentation[1]
phrases it as "all keys are created in the kernel", but you consider
"'The key material is generated
within the kernel' [a] misleading claim'. [2]
Also, I hope patches to force kernel RNG and CAAM support (using kernel RNG as
default) will soon be accepted, which would invalidate any further claims in the
commit message without a means to correct them.
I thus restricted my commit message to the necessary bit that are needed to
understand the patch, which is: userspace knowledge of the key material is
not required. If you disagree, could you provide me the text you'd prefer?
>> 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.
>
> As far as I can see, "Key unsealing in userspace" actually is the preferred way
> to implement TPM-bound encryption. So it doesn't seem fair to call it a "work
> around".
In the context of *kernel trusted keys*, direct interaction with the TPM
outside the kernel to decrypt a kernel-encrypted blob is surely not the
preferred way.
For TPM-bound encryption completely in userspace? Maybe. But that's not
what this patch is about. It's about kernel trusted keys and offloading
part of its functionality to userspace to _work around_ lack of kernel-side
integration is exactly that: a _work around_.
>> + 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.
>
> Again, please be explicit about key generation. Note that key generation is
> already discussed in a different section, "Master Keys". There should be a
> mention of trusted keys there. The above note about not reusing keys probably
> belongs there too. (The section you're editing here is
> "FS_IOC_ADD_ENCRYPTION_KEY", which is primarily intended to just document the
> ioctl, so it's not necessarily the best place for this type of information.)
Yes. The content of the note is more appropriate there.
>> @@ -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) {
>
> This function is getting long; it probably should be broken this up into several
> functions. E.g.:
Will do for v3.
> static int get_keyring_key(u32 key_id, u32 type,
> struct fscrypt_master_key_secret *secret)
> {
> key_ref_t ref;
> struct key *key;
> int err;
>
> 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) {
> err = fscrypt_get_provisioning_key(key, type, secret);
> } else if (IS_REACHABLE(CONFIG_TRUSTED_KEYS) &&
> key->type == &key_type_trusted) {
> err = fscrypt_get_trusted_key(key, secret);
> } else {
> err = -EKEYREJECTED;
> }
> key_ref_put(ref);
> return err;
> }
>
>> + /* Don't allow fscrypt v1 keys to be used as v2 keys and vice versa. */
>
> Please avoid overly-long lines.
For v3 with helper functions, there will be one indentation level less,
so this will be less 79 again instead of 87.
>
>> + 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;
>> + }
>
> What does the !tkp case mean? For "user" and "logon" keys it means "key
> revoked", but the "trusted" key type doesn't implement revoke. Is this included
> just to be safe?
Oh, good point. I think I cargo-culted it off encrypted key support for
eCryptfs and dm-crypt. Encrypted keys don't support revoke either..
That might be reasonable, but perhaps the error code in that
> case (but not the invalid length cases) should be -EKEYREVOKED instead?
Yes. It was like this for v1, but I missed it when dropping the
dependency on the key_extract_material patch. Will fix for v3.
[1]: https://www.kernel.org/doc/html/v5.14-rc5/security/keys/trusted-encrypted.html
[2]: https://lore.kernel.org/linux-fscrypt/YQLzOwnPF1434kUk@gmail.com/
Cheers and thanks for the review,
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: [PATCH v4 1/2] tpm: ibmvtpm: Avoid error message when process gets signal while waiting
From: Nageswara Sastry @ 2021-08-10 5:58 UTC (permalink / raw)
To: Stefan Berger, jarkko
Cc: nasastry, linux-integrity, linux-security-module, linux-kernel,
Stefan Berger, Nayna Jain, George Wilson
In-Reply-To: <20210809192159.2176580-2-stefanb@linux.vnet.ibm.com>
On 10/08/21 12:51 am, 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
> 'true' for as long as the vTPM is indicated as being busy in
> tpm_processing_cmd. 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>
Tested-by: Nageswara R Sastry <rnsastry@linux.ibm.com>
Tested with /dev/hwrng and /dev/tpm0 and not seen any tpm errors from
the kernel.
> ---
> drivers/char/tpm/tpm_ibmvtpm.c | 20 ++++++++++++--------
> drivers/char/tpm/tpm_ibmvtpm.h | 2 +-
> 2 files changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
> index 903604769de9..7a9eca5768f8 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) {
> @@ -269,7 +264,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_processing_cmd;
> }
>
> /**
> @@ -457,7 +454,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 = true,
> .req_complete_val = 0,
> .req_canceled = tpm_ibmvtpm_req_canceled,
> };
> @@ -688,8 +685,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..51198b137461 100644
> --- a/drivers/char/tpm/tpm_ibmvtpm.h
> +++ b/drivers/char/tpm/tpm_ibmvtpm.h
> @@ -41,7 +41,7 @@ struct ibmvtpm_dev {
> wait_queue_head_t wq;
> u16 res_len;
> u32 vtpm_version;
> - bool tpm_processing_cmd;
> + u8 tpm_processing_cmd;
> };
>
> #define CRQ_RES_BUF_SIZE PAGE_SIZE
>
--
Thanks and Regards
R.Nageswara Sastry
^ permalink raw reply
* Re: [PATCH 2/4] KEYS: trusted: allow trust sources to use kernel RNG for key material
From: Sumit Garg @ 2021-08-10 5:24 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Ahmad Fatoum, James Bottomley, 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: <20210809095647.7xcxjeot5gyvmlpj@kernel.org>
On Mon, 9 Aug 2021 at 15:26, Jarkko Sakkinen <jarkko@kernel.org> wrote:
>
> On Mon, Aug 09, 2021 at 09:52:20AM +0200, Ahmad Fatoum wrote:
> > 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.
>
> Ternary operators are pain to read, unless a super trivial case.
>
> I'd stick to what you did.
Fair enough, I am fine with the current patch.
-Sumit
>
> /Jarkko
^ permalink raw reply
* Re: [PATCH 1/1] NAX LSM: Add initial support support
From: J Freyensee @ 2021-08-10 4:52 UTC (permalink / raw)
To: THOBY Simon, Igor Zhbanov, linux-integrity, linux-security-module
Cc: Igor Zhbanov, Mimi Zohar
In-Reply-To: <d97d7fdb-1676-9670-6cf5-2427f780ec6f@viveris.fr>
snip...
>> +#define pr_fmt(fmt) "NAX: " fmt
>> +
>> +#include <linux/capability.h>
>> +#include <linux/cred.h>
>> +#include <linux/ctype.h>
>> +#include <linux/lsm_hooks.h>
>> +#include <linux/mman.h>
>> +#include <linux/sched.h>
>> +#include <linux/securebits.h>
>> +#include <linux/security.h>
>> +#include <linux/sysctl.h>
>> +#include <linux/uidgid.h>
>> +
>> +#define NAX_MODE_PERMISSIVE 0 /* Log only */
>> +#define NAX_MODE_ENFORCING 1 /* Enforce and log */
>> +#define NAX_MODE_KILL 2 /* Kill process and log */
>> +
>> +static int mode = CONFIG_SECURITY_NAX_MODE,
>> + quiet = IS_ENABLED(CONFIG_SECURITY_NAX_QUIET),
>> + locked = IS_ENABLED(CONFIG_SECURITY_NAX_LOCKED);
>> +
>> +#define ALLOWED_CAPS_HEX_LEN (_KERNEL_CAPABILITY_U32S * 8)
>> +
>> +static char allowed_caps_hex[ALLOWED_CAPS_HEX_LEN + 1];
>> +static kernel_cap_t allowed_caps = CAP_EMPTY_SET;
>> +
>> +static int
>> +is_privileged_process(void)
>> +{
>> + const struct cred *cred;
>> + kuid_t root_uid;
>> +
>> + cred = current_cred();
>> + root_uid = make_kuid(cred->user_ns, 0);
>> + /* We count a process as privileged if it any of its IDs is zero
>> + * or it has any unsafe capability (even in a user namespace) */
>> + if ((!issecure(SECURE_NOROOT) && (uid_eq(cred->uid, root_uid) ||
>> + uid_eq(cred->euid, root_uid) ||
>> + uid_eq(cred->suid, root_uid) ||
>> + uid_eq(cred->fsuid, root_uid))) ||
>> + (!cap_issubset(cred->cap_effective, allowed_caps)) ||
>> + (!cap_issubset(cred->cap_permitted, allowed_caps)))
>> + return 1;
>> +
>> + return 0;
>> +}
>> +
>> +static void
>> +log_warn(const char *reason)
>> +{
>> + if (quiet)
>> + return;
>> +
>> + pr_warn_ratelimited("%s: pid=%d, uid=%u, comm=\"%s\"\n",
>> + reason, current->pid,
>> + from_kuid(&init_user_ns, current_cred()->uid),
>> + current->comm);
> Have you considered writing to the audit log instead of the kernel messages directly?
> (not saying that this is necessarily better, but is there a reasoning to prefer one or
> the other here? Audit logs are often consumed by automated tools and it may be more pratical
> for people to detect and treat violations if the messages were pushed to the audit log
> - but conversely, that requires defining and maintaining a stable log format for consumers)
It's a good idea to writing to the audit log, HOWEVER I'd want to know
what all the rest of the LSMs are doing in a case like this. If all of
them just write kernel messages, I'd want this module to also write just
kernel messages for consistency sake for use with say, log harvesters
for a SIEM/XDR system solution.
Just in general I like the thought of this LSM. I used to work for a
security company in which their cloud "watched" situations where
mmap()/mprotect() would use anonymous executable pages for possible
"dodgy" behavior.
Jay
^ permalink raw reply
* Re: [PATCH v2] fscrypt: support trusted keys
From: Eric Biggers @ 2021-08-09 21:24 UTC (permalink / raw)
To: Ahmad Fatoum
Cc: Theodore Y. Ts'o, Jaegeuk Kim, kernel, 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: <20210806150928.27857-1-a.fatoum@pengutronix.de>
Hi Ahmad,
This generally looks okay, but I have some comments below.
On Fri, Aug 06, 2021 at 05:09:28PM +0200, Ahmad Fatoum wrote:
> 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.
Please be explicit about where and how the keys get generated in this case.
> 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.
As far as I can see, "Key unsealing in userspace" actually is the preferred way
to implement TPM-bound encryption. So it doesn't seem fair to call it a "work
around".
> + 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.
Again, please be explicit about key generation. Note that key generation is
already discussed in a different section, "Master Keys". There should be a
mention of trusted keys there. The above note about not reusing keys probably
belongs there too. (The section you're editing here is
"FS_IOC_ADD_ENCRYPTION_KEY", which is primarily intended to just document the
ioctl, so it's not necessarily the best place for this type of information.)
> @@ -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) {
This function is getting long; it probably should be broken this up into several
functions. E.g.:
static int get_keyring_key(u32 key_id, u32 type,
struct fscrypt_master_key_secret *secret)
{
key_ref_t ref;
struct key *key;
int err;
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) {
err = fscrypt_get_provisioning_key(key, type, secret);
} else if (IS_REACHABLE(CONFIG_TRUSTED_KEYS) &&
key->type == &key_type_trusted) {
err = fscrypt_get_trusted_key(key, secret);
} else {
err = -EKEYREJECTED;
}
key_ref_put(ref);
return err;
}
> + /* Don't allow fscrypt v1 keys to be used as v2 keys and vice versa. */
Please avoid overly-long lines.
> + 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;
> + }
What does the !tkp case mean? For "user" and "logon" keys it means "key
revoked", but the "trusted" key type doesn't implement revoke. Is this included
just to be safe? That might be reasonable, but perhaps the error code in that
case (but not the invalid length cases) should be -EKEYREVOKED instead?
- Eric
^ permalink raw reply
* Re: [PATCH v2] fscrypt: support trusted keys
From: Eric Biggers @ 2021-08-09 20:52 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Ahmad Fatoum, Theodore Y. Ts'o, Jaegeuk Kim, kernel,
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: <20210809094408.4iqwsx77u64usfx6@kernel.org>
On Mon, Aug 09, 2021 at 12:44:08PM +0300, Jarkko Sakkinen wrote:
> > @@ -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) {
>
> Why does fscrypt have own key type, and does not extend 'encrypted' with a
> new format [*]?
Are you referring to the "fscrypt-provisioning" key type? That is an existing
feature (which in most cases isn't used, but there is a use case that requires
it), not something being added by this patch. We just needed a key type where
userspace can add a raw key to the kernel and not be able to read it back (so
like the "logon" key type), but also have the kernel enforce that that key is
only used for fscrypt with a particular KDF version, and not with other random
kernel features. The "encrypted" key type wouldn't have worked for this at all;
it's a totally different thing.
> > + } 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 {
>
>
> I don't think this is right, or at least it does not follow the pattern
> in [*]. I.e. you should rather use trusted key to seal your fscrypt key.
What's the benefit of the extra layer of indirection over just using a "trusted"
key directly? The use case for "encrypted" keys is not at all clear to me.
- Eric
^ permalink raw reply
* [PATCH v4 1/2] tpm: ibmvtpm: Avoid error message when process gets signal while waiting
From: Stefan Berger @ 2021-08-09 19:21 UTC (permalink / raw)
To: jarkko
Cc: nasastry, linux-integrity, linux-security-module, linux-kernel,
Stefan Berger, Nayna Jain, George Wilson, Nageswara R Sastry
In-Reply-To: <20210809192159.2176580-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_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
'true' for as long as the vTPM is indicated as being busy in
tpm_processing_cmd. 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>
---
drivers/char/tpm/tpm_ibmvtpm.c | 20 ++++++++++++--------
drivers/char/tpm/tpm_ibmvtpm.h | 2 +-
2 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
index 903604769de9..7a9eca5768f8 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) {
@@ -269,7 +264,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_processing_cmd;
}
/**
@@ -457,7 +454,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 = true,
.req_complete_val = 0,
.req_canceled = tpm_ibmvtpm_req_canceled,
};
@@ -688,8 +685,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..51198b137461 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.h
+++ b/drivers/char/tpm/tpm_ibmvtpm.h
@@ -41,7 +41,7 @@ struct ibmvtpm_dev {
wait_queue_head_t wq;
u16 res_len;
u32 vtpm_version;
- bool tpm_processing_cmd;
+ u8 tpm_processing_cmd;
};
#define CRQ_RES_BUF_SIZE PAGE_SIZE
--
2.31.1
^ permalink raw reply related
* [PATCH v4 2/2] tpm: ibmvtpm: Rename tpm_process_cmd to tpm_status and define flag
From: Stefan Berger @ 2021-08-09 19:21 UTC (permalink / raw)
To: jarkko
Cc: nasastry, linux-integrity, linux-security-module, linux-kernel,
Stefan Berger, Nayna Jain, George Wilson
In-Reply-To: <20210809192159.2176580-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.
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 | 15 ++++++++-------
drivers/char/tpm/tpm_ibmvtpm.h | 3 ++-
2 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
index 7a9eca5768f8..5d795866b483 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.c
+++ b/drivers/char/tpm/tpm_ibmvtpm.c
@@ -215,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;
}
@@ -232,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,
@@ -250,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);
@@ -266,7 +267,7 @@ static u8 tpm_ibmvtpm_status(struct tpm_chip *chip)
{
struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(&chip->dev);
- return ibmvtpm->tpm_processing_cmd;
+ return ibmvtpm->tpm_status;
}
/**
@@ -454,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 = true,
+ .req_complete_mask = TPM_STATUS_BUSY,
.req_complete_val = 0,
.req_canceled = tpm_ibmvtpm_req_canceled,
};
@@ -547,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:
diff --git a/drivers/char/tpm/tpm_ibmvtpm.h b/drivers/char/tpm/tpm_ibmvtpm.h
index 51198b137461..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;
- u8 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
* [PATCH v4 0/2] ibmvtpm: Avoid error message when process gets signal while waiting
From: Stefan Berger @ 2021-08-09 19:21 UTC (permalink / raw)
To: jarkko
Cc: nasastry, 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
v4:
- Reverted order of patches
v3:
- Split into two patches
Stefan Berger (2):
tpm: ibmvtpm: Avoid error message when process gets signal while
waiting
tpm: ibmvtpm: Rename tpm_process_cmd to tpm_status and define flag
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 1/3] efi/libstub: Copy confidential computing secret area
From: Dov Murik @ 2021-08-09 19:01 UTC (permalink / raw)
To: linux-efi
Cc: Dov Murik, Borislav Petkov, Ashish Kalra, Brijesh Singh,
Tom Lendacky, Ard Biesheuvel, James Morris, Serge E. Hallyn,
Andi Kleen, Dr. David Alan Gilbert, James Bottomley,
Tobin Feldman-Fitzthum, Jim Cadden, linux-coco,
linux-security-module, linux-kernel
In-Reply-To: <20210809190157.279332-1-dovmurik@linux.ibm.com>
Confidential computing (coco) hardware such as AMD SEV (Secure Encrypted
Virtualization) allows a guest owner to inject secrets into the VMs
memory without the host/hypervisor being able to read them.
Firmware support for secret injection is available in OVMF, which
reserves a memory area for secret injection and includes a pointer to it
the in EFI config table entry LINUX_EFI_COCO_SECRET_TABLE_GUID.
However, OVMF doesn't force the guest OS to keep this memory area
reserved.
If EFI exposes such a table entry, efi/libstub will copy this area to a
reserved memory for future use inside the kernel.
A pointer to the new copy is kept in the EFI table under
LINUX_EFI_COCO_SECRET_AREA_GUID.
Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
---
drivers/firmware/efi/libstub/Makefile | 2 +-
drivers/firmware/efi/libstub/coco.c | 68 +++++++++++++++++++++++++
drivers/firmware/efi/libstub/efi-stub.c | 2 +
drivers/firmware/efi/libstub/efistub.h | 2 +
drivers/firmware/efi/libstub/x86-stub.c | 2 +
include/linux/efi.h | 6 +++
6 files changed, 81 insertions(+), 1 deletion(-)
create mode 100644 drivers/firmware/efi/libstub/coco.c
diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index d0537573501e..d77690b7dfb9 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -55,7 +55,7 @@ KCOV_INSTRUMENT := n
lib-y := efi-stub-helper.o gop.o secureboot.o tpm.o \
file.o mem.o random.o randomalloc.o pci.o \
skip_spaces.o lib-cmdline.o lib-ctype.o \
- alignedmem.o relocate.o vsprintf.o
+ alignedmem.o relocate.o vsprintf.o coco.o
# include the stub's generic dependencies from lib/ when building for ARM/arm64
efi-deps-y := fdt_rw.c fdt_ro.c fdt_wip.c fdt.c fdt_empty_tree.c fdt_sw.c
diff --git a/drivers/firmware/efi/libstub/coco.c b/drivers/firmware/efi/libstub/coco.c
new file mode 100644
index 000000000000..bf546b6a3f72
--- /dev/null
+++ b/drivers/firmware/efi/libstub/coco.c
@@ -0,0 +1,68 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Confidential computing (coco) secret area handling
+ *
+ * Copyright (C) 2021 IBM Corporation
+ * Author: Dov Murik <dovmurik@linux.ibm.com>
+ */
+
+#include <linux/efi.h>
+#include <linux/sizes.h>
+#include <asm/efi.h>
+
+#include "efistub.h"
+
+#define LINUX_EFI_COCO_SECRET_TABLE_GUID \
+ EFI_GUID(0xadf956ad, 0xe98c, 0x484c, 0xae, 0x11, 0xb5, 0x1c, 0x7d, 0x33, 0x64, 0x47)
+
+/**
+ * struct efi_coco_secret_table - EFI config table that points to the
+ * confidential computing secret area. The guid
+ * LINUX_EFI_COCO_SECRET_TABLE_GUID holds this table.
+ * @base: Physical address of the EFI secret area
+ * @size: Size (in bytes) of the EFI secret area
+ */
+struct efi_coco_secret_table {
+ u64 base;
+ u64 size;
+} __attribute((packed));
+
+/*
+ * Create a copy of EFI's confidential computing secret area (if available) so
+ * that the secrets are accessible in the kernel after ExitBootServices.
+ */
+void efi_copy_coco_secret_area(void)
+{
+ efi_guid_t linux_secret_area_guid = LINUX_EFI_COCO_SECRET_AREA_GUID;
+ efi_status_t status;
+ struct efi_coco_secret_table *secret_table;
+ struct linux_efi_coco_secret_area *secret_area;
+
+ secret_table = get_efi_config_table(LINUX_EFI_COCO_SECRET_TABLE_GUID);
+ if (!secret_table)
+ return;
+
+ if (secret_table->size == 0 || secret_table->size >= SZ_4G)
+ return;
+
+ /* Allocate space for the secret area and copy it */
+ status = efi_bs_call(allocate_pool, EFI_LOADER_DATA,
+ sizeof(*secret_area) + secret_table->size, (void **)&secret_area);
+
+ if (status != EFI_SUCCESS) {
+ efi_err("Unable to allocate memory for confidential computing secret area copy\n");
+ return;
+ }
+
+ secret_area->size = secret_table->size;
+ memcpy(secret_area->area, (void *)(unsigned long)secret_table->base, secret_table->size);
+
+ status = efi_bs_call(install_configuration_table, &linux_secret_area_guid, secret_area);
+ if (status != EFI_SUCCESS)
+ goto err_free;
+
+ return;
+
+err_free:
+ efi_bs_call(free_pool, secret_area);
+}
diff --git a/drivers/firmware/efi/libstub/efi-stub.c b/drivers/firmware/efi/libstub/efi-stub.c
index 26e69788f27a..18b3acd15c85 100644
--- a/drivers/firmware/efi/libstub/efi-stub.c
+++ b/drivers/firmware/efi/libstub/efi-stub.c
@@ -205,6 +205,8 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
efi_retrieve_tpm2_eventlog();
+ efi_copy_coco_secret_area();
+
/* Ask the firmware to clear memory on unclean shutdown */
efi_enable_reset_attack_mitigation();
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index cde0a2ef507d..d604c6744cef 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -858,4 +858,6 @@ efi_enable_reset_attack_mitigation(void) { }
void efi_retrieve_tpm2_eventlog(void);
+void efi_copy_coco_secret_area(void);
+
#endif
diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index f14c4ff5839f..4ad85e1b6191 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -793,6 +793,8 @@ unsigned long efi_main(efi_handle_t handle,
efi_retrieve_tpm2_eventlog();
+ efi_copy_coco_secret_area();
+
setup_graphics(boot_params);
setup_efi_pci(boot_params);
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 6b5d36babfcc..9021dd521302 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -359,6 +359,7 @@ void efi_native_runtime_setup(void);
#define LINUX_EFI_MEMRESERVE_TABLE_GUID EFI_GUID(0x888eb0c6, 0x8ede, 0x4ff5, 0xa8, 0xf0, 0x9a, 0xee, 0x5c, 0xb9, 0x77, 0xc2)
#define LINUX_EFI_INITRD_MEDIA_GUID EFI_GUID(0x5568e427, 0x68fc, 0x4f3d, 0xac, 0x74, 0xca, 0x55, 0x52, 0x31, 0xcc, 0x68)
#define LINUX_EFI_MOK_VARIABLE_TABLE_GUID EFI_GUID(0xc451ed2b, 0x9694, 0x45d3, 0xba, 0xba, 0xed, 0x9f, 0x89, 0x88, 0xa3, 0x89)
+#define LINUX_EFI_COCO_SECRET_AREA_GUID EFI_GUID(0x940ed1e9, 0xd3da, 0x408b, 0xb3, 0x07, 0xe3, 0x2d, 0x25, 0x4a, 0x65, 0x16)
/* OEM GUIDs */
#define DELLEMC_EFI_RCI2_TABLE_GUID EFI_GUID(0x2d9f28a2, 0xa886, 0x456a, 0x97, 0xa8, 0xf1, 0x1e, 0xf2, 0x4f, 0xf4, 0x55)
@@ -1282,4 +1283,9 @@ static inline struct efi_mokvar_table_entry *efi_mokvar_entry_find(
}
#endif
+struct linux_efi_coco_secret_area {
+ u32 size;
+ u8 area[];
+};
+
#endif /* _LINUX_EFI_H */
--
2.25.1
^ permalink raw reply related
* [PATCH 3/3] virt: Add sev_secret module to expose confidential computing secrets
From: Dov Murik @ 2021-08-09 19:01 UTC (permalink / raw)
To: linux-efi
Cc: Dov Murik, Borislav Petkov, Ashish Kalra, Brijesh Singh,
Tom Lendacky, Ard Biesheuvel, James Morris, Serge E. Hallyn,
Andi Kleen, Dr. David Alan Gilbert, James Bottomley,
Tobin Feldman-Fitzthum, Jim Cadden, linux-coco,
linux-security-module, linux-kernel
In-Reply-To: <20210809190157.279332-1-dovmurik@linux.ibm.com>
The new sev_secret module exposes the confidential computing (coco)
secret area via securityfs interface.
When the module is loaded (and securityfs is mounted, typically under
/sys/kernel/security), a "coco/sev_secret" directory is created in
securityfs. In it, a file is created for each secret entry. The name
of each such file is the GUID of the secret entry, and its content is
the secret data.
This allows applications running in a confidential computing setting to
read secrets provided by the guest owner via a secure secret injection
mechanism (such as AMD SEV's LAUNCH_SECRET command).
Removing (unlinking) files in the "coco/sev_secret" directory will zero
out the secret in memory, and remove the filesystem entry. If the
module is removed and loaded again, that secret will not appear in the
filesystem.
Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
---
drivers/virt/Kconfig | 3 +
drivers/virt/Makefile | 1 +
drivers/virt/coco/sev_secret/Kconfig | 11 +
drivers/virt/coco/sev_secret/Makefile | 2 +
drivers/virt/coco/sev_secret/sev_secret.c | 313 ++++++++++++++++++++++
5 files changed, 330 insertions(+)
create mode 100644 drivers/virt/coco/sev_secret/Kconfig
create mode 100644 drivers/virt/coco/sev_secret/Makefile
create mode 100644 drivers/virt/coco/sev_secret/sev_secret.c
diff --git a/drivers/virt/Kconfig b/drivers/virt/Kconfig
index 8061e8ef449f..6f73672f593f 100644
--- a/drivers/virt/Kconfig
+++ b/drivers/virt/Kconfig
@@ -36,4 +36,7 @@ source "drivers/virt/vboxguest/Kconfig"
source "drivers/virt/nitro_enclaves/Kconfig"
source "drivers/virt/acrn/Kconfig"
+
+source "drivers/virt/coco/sev_secret/Kconfig"
+
endif
diff --git a/drivers/virt/Makefile b/drivers/virt/Makefile
index 3e272ea60cd9..2a7d472478bd 100644
--- a/drivers/virt/Makefile
+++ b/drivers/virt/Makefile
@@ -8,3 +8,4 @@ obj-y += vboxguest/
obj-$(CONFIG_NITRO_ENCLAVES) += nitro_enclaves/
obj-$(CONFIG_ACRN_HSM) += acrn/
+obj-$(CONFIG_AMD_SEV_SECRET) += coco/sev_secret/
diff --git a/drivers/virt/coco/sev_secret/Kconfig b/drivers/virt/coco/sev_secret/Kconfig
new file mode 100644
index 000000000000..76cfb4f405e0
--- /dev/null
+++ b/drivers/virt/coco/sev_secret/Kconfig
@@ -0,0 +1,11 @@
+# SPDX-License-Identifier: GPL-2.0-only
+config AMD_SEV_SECRET
+ tristate "AMD SEV secret area securityfs support"
+ depends on AMD_MEM_ENCRYPT && EFI
+ select SECURITYFS
+ help
+ This is a driver for accessing the AMD SEV secret area via
+ securityfs.
+
+ To compile this driver as a module, choose M here.
+ The module will be called sev_secret.
diff --git a/drivers/virt/coco/sev_secret/Makefile b/drivers/virt/coco/sev_secret/Makefile
new file mode 100644
index 000000000000..dca0ed3f8f94
--- /dev/null
+++ b/drivers/virt/coco/sev_secret/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0-only
+obj-$(CONFIG_AMD_SEV_SECRET) += sev_secret.o
diff --git a/drivers/virt/coco/sev_secret/sev_secret.c b/drivers/virt/coco/sev_secret/sev_secret.c
new file mode 100644
index 000000000000..d9a60166b142
--- /dev/null
+++ b/drivers/virt/coco/sev_secret/sev_secret.c
@@ -0,0 +1,313 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * sev_secret module
+ *
+ * Copyright (C) 2021 IBM Corporation
+ * Author: Dov Murik <dovmurik@linux.ibm.com>
+ */
+
+/**
+ * DOC: sev_secret: Allow reading confidential computing (coco) secret area via
+ * securityfs interface.
+ *
+ * When the module is loaded (and securityfs is mounted, typically under
+ * /sys/kernel/security), a "coco/sev_secret" directory is created in
+ * securityfs. In it, a file is created for each secret entry. The name of
+ * each such file is the GUID of the secret entry, and its content is the
+ * secret data.
+ */
+
+#include <linux/seq_file.h>
+#include <linux/fs.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/security.h>
+#include <linux/efi.h>
+
+#define SEV_SECRET_NUM_FILES 64
+
+#define EFI_SEVSECRET_TABLE_HEADER_GUID \
+ EFI_GUID(0x1e74f542, 0x71dd, 0x4d66, 0x96, 0x3e, 0xef, 0x42, 0x87, 0xff, 0x17, 0x3b)
+
+struct sev_secret {
+ struct dentry *coco_dir;
+ struct dentry *fs_dir;
+ struct dentry *fs_files[SEV_SECRET_NUM_FILES];
+ struct linux_efi_coco_secret_area *secret_area;
+};
+
+/*
+ * Structure of the SEV secret area
+ *
+ * Offset Length
+ * (bytes) (bytes) Usage
+ * ------- ------- -----
+ * 0 16 Secret table header GUID (must be 1e74f542-71dd-4d66-963e-ef4287ff173b)
+ * 16 4 Length of bytes of the entire secret area
+ *
+ * 20 16 First secret entry's GUID
+ * 36 4 First secret entry's length in bytes (= 16 + 4 + x)
+ * 40 x First secret entry's data
+ *
+ * 40+x 16 Second secret entry's GUID
+ * 56+x 4 Second secret entry's length in bytes (= 16 + 4 + y)
+ * 60+x y Second secret entry's data
+ *
+ * (... and so on for additional entries)
+ *
+ * The GUID of each secret entry designates the usage of the secret data.
+ */
+
+/**
+ * struct secret_header - Header of entire secret area; this should be followed
+ * by instances of struct secret_entry.
+ * @guid: Must be EFI_SEVSECRET_TABLE_HEADER_GUID
+ * @len: Length in bytes of entire secret area, including header
+ */
+struct secret_header {
+ efi_guid_t guid;
+ u32 len;
+} __attribute((packed));
+
+/**
+ * struct secret_entry - Holds one secret entry
+ * @guid: Secret-specific GUID (or NULL_GUID if this secret entry was deleted)
+ * @len: Length of secret entry, including its guid and len fields
+ * @data: The secret data (full of zeros if this secret entry was deleted)
+ */
+struct secret_entry {
+ efi_guid_t guid;
+ u32 len;
+ u8 data[];
+} __attribute((packed));
+
+static size_t secret_entry_data_len(struct secret_entry *e)
+{
+ return e->len - sizeof(*e);
+}
+
+static struct sev_secret the_sev_secret;
+
+static inline struct sev_secret *sev_secret_get(void)
+{
+ return &the_sev_secret;
+}
+
+static int sev_secret_bin_file_show(struct seq_file *file, void *data)
+{
+ struct secret_entry *e = file->private;
+
+ if (e)
+ seq_write(file, e->data, secret_entry_data_len(e));
+
+ return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(sev_secret_bin_file);
+
+static int sev_secret_unlink(struct inode *dir, struct dentry *dentry)
+{
+ struct sev_secret *s = sev_secret_get();
+ struct inode *inode = d_inode(dentry);
+ struct secret_entry *e = (struct secret_entry *)inode->i_private;
+ int i;
+
+ if (e) {
+ /* Zero out the secret data */
+ memzero_explicit(e->data, secret_entry_data_len(e));
+ e->guid = NULL_GUID;
+ }
+
+ inode->i_private = NULL;
+
+ for (i = 0; i < SEV_SECRET_NUM_FILES; i++)
+ if (s->fs_files[i] == dentry)
+ s->fs_files[i] = NULL;
+
+ /*
+ * securityfs_remove tries to lock the directory's inode, but we reach
+ * the unlink callback when it's already locked
+ */
+ inode_unlock(dir);
+ securityfs_remove(dentry);
+ inode_lock(dir);
+
+ return 0;
+}
+
+static const struct inode_operations sev_secret_dir_inode_operations = {
+ .lookup = simple_lookup,
+ .unlink = sev_secret_unlink,
+};
+
+static int sev_secret_map_area(void)
+{
+ struct sev_secret *s = sev_secret_get();
+ struct linux_efi_coco_secret_area *secret_area;
+ u32 secret_area_size;
+
+ if (efi.coco_secret == EFI_INVALID_TABLE_ADDR) {
+ pr_err("Secret area address is not available\n");
+ return -EINVAL;
+ }
+
+ secret_area = memremap(efi.coco_secret, sizeof(*secret_area), MEMREMAP_WB);
+ if (secret_area == NULL) {
+ pr_err("Could not map secret area header\n");
+ return -ENOMEM;
+ }
+
+ secret_area_size = sizeof(*secret_area) + secret_area->size;
+ memunmap(secret_area);
+
+ secret_area = memremap(efi.coco_secret, secret_area_size, MEMREMAP_WB);
+ if (secret_area == NULL) {
+ pr_err("Could not map secret area\n");
+ return -ENOMEM;
+ }
+
+ s->secret_area = secret_area;
+ return 0;
+}
+
+static void sev_secret_securityfs_teardown(void)
+{
+ struct sev_secret *s = sev_secret_get();
+ int i;
+
+ for (i = (SEV_SECRET_NUM_FILES - 1); i >= 0; i--) {
+ securityfs_remove(s->fs_files[i]);
+ s->fs_files[i] = NULL;
+ }
+
+ securityfs_remove(s->fs_dir);
+ s->fs_dir = NULL;
+
+ securityfs_remove(s->coco_dir);
+ s->coco_dir = NULL;
+
+ pr_debug("Removed sev_secret securityfs entries\n");
+}
+
+static int sev_secret_securityfs_setup(void)
+{
+ efi_guid_t tableheader_guid = EFI_SEVSECRET_TABLE_HEADER_GUID;
+ struct sev_secret *s = sev_secret_get();
+ int ret = 0, i = 0, bytes_left;
+ unsigned char *ptr;
+ struct secret_header *h;
+ struct secret_entry *e;
+ struct dentry *dent;
+ char guid_str[EFI_VARIABLE_GUID_LEN + 1];
+
+ s->coco_dir = NULL;
+ s->fs_dir = NULL;
+ memset(s->fs_files, 0, sizeof(s->fs_files));
+
+ dent = securityfs_create_dir("coco", NULL);
+ if (IS_ERR(dent)) {
+ pr_err("Error creating coco securityfs directory entry err=%ld\n", PTR_ERR(dent));
+ return PTR_ERR(dent);
+ }
+ s->coco_dir = dent;
+
+ dent = securityfs_create_dir("sev_secret", s->coco_dir);
+ if (IS_ERR(dent)) {
+ pr_err("Error creating SEV secret securityfs directory entry err=%ld\n",
+ PTR_ERR(dent));
+ return PTR_ERR(dent);
+ }
+ d_inode(dent)->i_op = &sev_secret_dir_inode_operations;
+ s->fs_dir = dent;
+
+ ptr = s->secret_area->area;
+ h = (struct secret_header *)ptr;
+ if (memcmp(&h->guid, &tableheader_guid, sizeof(h->guid))) {
+ pr_err("SEV secret area does not start with correct GUID\n");
+ ret = -EINVAL;
+ goto err_cleanup;
+ }
+ if (h->len < sizeof(*h)) {
+ pr_err("SEV secret area reported length is too small\n");
+ ret = -EINVAL;
+ goto err_cleanup;
+ }
+
+ bytes_left = h->len - sizeof(*h);
+ ptr += sizeof(*h);
+ while (bytes_left >= (int)sizeof(*e) && i < SEV_SECRET_NUM_FILES) {
+ e = (struct secret_entry *)ptr;
+ if (e->len < sizeof(*e) || e->len > (unsigned int)bytes_left) {
+ pr_err("SEV secret area is corrupted\n");
+ ret = -EINVAL;
+ goto err_cleanup;
+ }
+
+ /* Skip deleted entries (which will have NULL_GUID) */
+ if (efi_guidcmp(e->guid, NULL_GUID)) {
+ efi_guid_to_str(&e->guid, guid_str);
+
+ dent = securityfs_create_file(guid_str, 0440, s->fs_dir, (void *)e,
+ &sev_secret_bin_file_fops);
+ if (IS_ERR(dent)) {
+ pr_err("Error creating SEV secret securityfs entry\n");
+ ret = PTR_ERR(dent);
+ goto err_cleanup;
+ }
+
+ s->fs_files[i++] = dent;
+ }
+ ptr += e->len;
+ bytes_left -= e->len;
+ }
+
+ pr_debug("Created %d entries in sev_secret securityfs\n", i);
+ return 0;
+
+err_cleanup:
+ sev_secret_securityfs_teardown();
+ return ret;
+}
+
+static void sev_secret_unmap_area(void)
+{
+ struct sev_secret *s = sev_secret_get();
+
+ if (s->secret_area) {
+ memunmap(s->secret_area);
+ s->secret_area = NULL;
+ }
+}
+
+static int __init sev_secret_init(void)
+{
+ int ret;
+
+ ret = sev_secret_map_area();
+ if (ret)
+ return ret;
+
+ ret = sev_secret_securityfs_setup();
+ if (ret)
+ goto err_unmap;
+
+ return ret;
+
+err_unmap:
+ sev_secret_unmap_area();
+ return ret;
+}
+
+static void __exit sev_secret_exit(void)
+{
+ sev_secret_securityfs_teardown();
+ sev_secret_unmap_area();
+}
+
+module_init(sev_secret_init);
+module_exit(sev_secret_exit);
+
+MODULE_DESCRIPTION("AMD SEV confidential computing secret area access");
+MODULE_AUTHOR("IBM");
+MODULE_LICENSE("GPL");
--
2.25.1
^ permalink raw reply related
* [PATCH 2/3] efi: Reserve confidential computing secret area
From: Dov Murik @ 2021-08-09 19:01 UTC (permalink / raw)
To: linux-efi
Cc: Dov Murik, Borislav Petkov, Ashish Kalra, Brijesh Singh,
Tom Lendacky, Ard Biesheuvel, James Morris, Serge E. Hallyn,
Andi Kleen, Dr. David Alan Gilbert, James Bottomley,
Tobin Feldman-Fitzthum, Jim Cadden, linux-coco,
linux-security-module, linux-kernel
In-Reply-To: <20210809190157.279332-1-dovmurik@linux.ibm.com>
When efi-stub copies an EFI-provided confidential computing (coco)
secret area, reserve that memory block for future use within the kernel.
Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
---
arch/x86/platform/efi/efi.c | 1 +
drivers/firmware/efi/Makefile | 2 +-
drivers/firmware/efi/coco.c | 41 +++++++++++++++++++++++++++++++++++
drivers/firmware/efi/efi.c | 3 +++
include/linux/efi.h | 3 +++
5 files changed, 49 insertions(+), 1 deletion(-)
create mode 100644 drivers/firmware/efi/coco.c
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index 147c30a81f15..35e082e5f603 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -93,6 +93,7 @@ static const unsigned long * const efi_tables[] = {
#ifdef CONFIG_LOAD_UEFI_KEYS
&efi.mokvar_table,
#endif
+ &efi.coco_secret,
};
u64 efi_setup; /* efi setup_data physical address */
diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile
index 467e94259679..8703ffcca351 100644
--- a/drivers/firmware/efi/Makefile
+++ b/drivers/firmware/efi/Makefile
@@ -12,7 +12,7 @@ KASAN_SANITIZE_runtime-wrappers.o := n
obj-$(CONFIG_ACPI_BGRT) += efi-bgrt.o
obj-$(CONFIG_EFI) += efi.o vars.o reboot.o memattr.o tpm.o
-obj-$(CONFIG_EFI) += memmap.o
+obj-$(CONFIG_EFI) += memmap.o coco.o
ifneq ($(CONFIG_EFI_CAPSULE_LOADER),)
obj-$(CONFIG_EFI) += capsule.o
endif
diff --git a/drivers/firmware/efi/coco.c b/drivers/firmware/efi/coco.c
new file mode 100644
index 000000000000..42f477d6188c
--- /dev/null
+++ b/drivers/firmware/efi/coco.c
@@ -0,0 +1,41 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Confidential computing (coco) secret area handling
+ *
+ * Copyright (C) 2021 IBM Corporation
+ * Author: Dov Murik <dovmurik@linux.ibm.com>
+ */
+
+#define pr_fmt(fmt) "efi: " fmt
+
+#include <linux/efi.h>
+#include <linux/init.h>
+#include <linux/memblock.h>
+#include <asm/early_ioremap.h>
+
+/*
+ * Reserve the confidential computing secret area memory
+ */
+int __init efi_coco_secret_area_reserve(void)
+{
+ struct linux_efi_coco_secret_area *secret_area;
+ unsigned long secret_area_size;
+
+ if (efi.coco_secret == EFI_INVALID_TABLE_ADDR)
+ return 0;
+
+ secret_area = early_memremap(efi.coco_secret, sizeof(*secret_area));
+ if (!secret_area) {
+ pr_err("Failed to map confidential computing secret area\n");
+ efi.coco_secret = EFI_INVALID_TABLE_ADDR;
+ return -ENOMEM;
+ }
+
+ secret_area_size = sizeof(*secret_area) + secret_area->size;
+ memblock_reserve(efi.coco_secret, secret_area_size);
+
+ pr_info("Reserved memory of EFI-provided confidential computing secret area");
+
+ early_memunmap(secret_area, sizeof(*secret_area));
+ return 0;
+}
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 847f33ffc4ae..07e17ad225a6 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -526,6 +526,7 @@ static const efi_config_table_type_t common_tables[] __initconst = {
#ifdef CONFIG_LOAD_UEFI_KEYS
{LINUX_EFI_MOK_VARIABLE_TABLE_GUID, &efi.mokvar_table, "MOKvar" },
#endif
+ {LINUX_EFI_COCO_SECRET_AREA_GUID, &efi.coco_secret, "CocoSecret" },
{},
};
@@ -613,6 +614,8 @@ int __init efi_config_parse_tables(const efi_config_table_t *config_tables,
efi_tpm_eventlog_init();
+ efi_coco_secret_area_reserve();
+
if (mem_reserve != EFI_INVALID_TABLE_ADDR) {
unsigned long prsv = mem_reserve;
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 9021dd521302..e86600af5dfd 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -550,6 +550,7 @@ extern struct efi {
unsigned long tpm_log; /* TPM2 Event Log table */
unsigned long tpm_final_log; /* TPM2 Final Events Log table */
unsigned long mokvar_table; /* MOK variable config table */
+ unsigned long coco_secret; /* Confidential computing secret table */
efi_get_time_t *get_time;
efi_set_time_t *set_time;
@@ -1189,6 +1190,8 @@ extern int efi_tpm_final_log_size;
extern unsigned long rci2_table_phys;
+extern int efi_coco_secret_area_reserve(void);
+
/*
* efi_runtime_service() function identifiers.
* "NONE" is used by efi_recover_from_page_fault() to check if the page
--
2.25.1
^ permalink raw reply related
* [PATCH 0/3] Allow access to confidential computing secret area in SEV guests
From: Dov Murik @ 2021-08-09 19:01 UTC (permalink / raw)
To: linux-efi
Cc: Dov Murik, Borislav Petkov, Ashish Kalra, Brijesh Singh,
Tom Lendacky, Ard Biesheuvel, James Morris, Serge E. Hallyn,
Andi Kleen, Dr. David Alan Gilbert, James Bottomley,
Tobin Feldman-Fitzthum, Jim Cadden, linux-coco,
linux-security-module, linux-kernel
Confidential computing (coco) hardware such as AMD SEV (Secure Encrypted
Virtualization) allows guest owners to inject secrets into the VMs
memory without the host/hypervisor being able to read them. In SEV,
secret injection is performed early in the VM launch process, before the
guest starts running.
OVMF already reserves designated area for secret injection (in its
AmdSev package; see edk2 commit 01726b6d23d4 "OvmfPkg/AmdSev: Expose the
Sev Secret area using a configuration table" [1]), but the secrets were
not available in the guest kernel.
The patch series copies the secrets from the EFI-provided memory to
kernel reserved memory, and optionally exposes them to userspace via
securityfs using a new sev_secret kernel module.
The first patch in efi/libstub copies the secret area from the EFI
memory to specially allocated memory; the second patch reserves that
memory block; and the third patch introduces the new sev_secret module
that exposes the content of the secret entries as securityfs files, and
allows clearing out secrets with a file unlink interface.
As a usage example, consider a guest performing computations on
encrypted files. The Guest Owner provides the decryption key (= secret)
using the secret injection mechanism. The guest application reads the
secret from the sev_secret filesystem and proceeds to decrypt the files
into memory and then performs the needed computations on the content.
In this example, the host can't read the files from the disk image
because they are encrypted. Host can't read the decryption key because
it is passed using the secret injection mechanism (= secure channel).
Host can't read the decrypted content from memory because it's a
confidential (memory-encrypted) guest.
This has been tested with AMD SEV guests, but the kernel side of
handling the secret area has no SEV-specific dependencies, and therefore
might be usable (perhaps with minor changes) for any confidential
computing hardware that can publish the secret area via the standard EFI
config table entry.
Here is a simple example for usage of the sev_secret module in a guest
to which a secret are with 4 secrets was injected during launch:
# modprobe sev_secret
# ls -la /sys/kernel/security/coco/sev_secret
total 0
drwxr-xr-x 2 root root 0 Jun 28 11:54 .
drwxr-xr-x 3 root root 0 Jun 28 11:54 ..
-r--r----- 1 root root 0 Jun 28 11:54 736870e5-84f0-4973-92ec-06879ce3da0b
-r--r----- 1 root root 0 Jun 28 11:54 83c83f7f-1356-4975-8b7e-d3a0b54312c6
-r--r----- 1 root root 0 Jun 28 11:54 9553f55d-3da2-43ee-ab5d-ff17f78864d2
-r--r----- 1 root root 0 Jun 28 11:54 e6f5a162-d67f-4750-a67c-5d065f2a9910
# xxd /sys/kernel/security/coco/sev_secret/e6f5a162-d67f-4750-a67c-5d065f2a9910
00000000: 7468 6573 652d 6172 652d 7468 652d 6b61 these-are-the-ka
00000010: 7461 2d73 6563 7265 7473 0001 0203 0405 ta-secrets......
00000020: 0607 ..
# rm /sys/kernel/security/coco/sev_secret/e6f5a162-d67f-4750-a67c-5d065f2a9910
# ls -la /sys/kernel/security/coco/sev_secret
total 0
drwxr-xr-x 2 root root 0 Jun 28 11:55 .
drwxr-xr-x 3 root root 0 Jun 28 11:54 ..
-r--r----- 1 root root 0 Jun 28 11:54 736870e5-84f0-4973-92ec-06879ce3da0b
-r--r----- 1 root root 0 Jun 28 11:54 83c83f7f-1356-4975-8b7e-d3a0b54312c6
-r--r----- 1 root root 0 Jun 28 11:54 9553f55d-3da2-43ee-ab5d-ff17f78864d2
Previously sent as an RFC series [2].
[1] https://github.com/tianocore/edk2/commit/01726b6d23d4
[2] https://lore.kernel.org/linux-coco/20210628183431.953934-1-dovmurik@linux.ibm.com/
Dov Murik (3):
efi/libstub: Copy confidential computing secret area
efi: Reserve confidential computing secret area
virt: Add sev_secret module to expose confidential computing secrets
arch/x86/platform/efi/efi.c | 1 +
drivers/firmware/efi/Makefile | 2 +-
drivers/firmware/efi/coco.c | 41 +++
drivers/firmware/efi/efi.c | 3 +
drivers/firmware/efi/libstub/Makefile | 2 +-
drivers/firmware/efi/libstub/coco.c | 68 +++++
drivers/firmware/efi/libstub/efi-stub.c | 2 +
drivers/firmware/efi/libstub/efistub.h | 2 +
drivers/firmware/efi/libstub/x86-stub.c | 2 +
drivers/virt/Kconfig | 3 +
drivers/virt/Makefile | 1 +
drivers/virt/coco/sev_secret/Kconfig | 11 +
drivers/virt/coco/sev_secret/Makefile | 2 +
drivers/virt/coco/sev_secret/sev_secret.c | 313 ++++++++++++++++++++++
include/linux/efi.h | 9 +
15 files changed, 460 insertions(+), 2 deletions(-)
create mode 100644 drivers/firmware/efi/coco.c
create mode 100644 drivers/firmware/efi/libstub/coco.c
create mode 100644 drivers/virt/coco/sev_secret/Kconfig
create mode 100644 drivers/virt/coco/sev_secret/Makefile
create mode 100644 drivers/virt/coco/sev_secret/sev_secret.c
base-commit: 36a21d51725af2ce0700c6ebcb6b9594aac658a6
--
2.25.1
^ permalink raw reply
* Re: [PATCH 0/4] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys
From: Ahmad Fatoum @ 2021-08-09 10:16 UTC (permalink / raw)
To: Jarkko Sakkinen
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: <20210809093519.er32rmspuvkrww45@kernel.org>
On 09.08.21 11:35, Jarkko Sakkinen wrote:
> 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?
For supported file systems, fscrypt[1] allows you to encrypt at a directory level.
It has no trusted key integration yet, which is something I am trying to upstream
in parallel to this series, so I eventually can use fscrypt together with CAAM-backed
trusted keys on an unpatched kernel.
If it interests you, I described[2] my CAAM+ubifs+fscrypt use case in the
discussion thread on my fscrypt-trusted-keys v1. Jan, a colleague of mine, held a
talk[3] on the different solutions for authenticated and encrypted storage, which
you may want to check out.
I'd really appreciate feedback here on the the CAAM parts of this series, so this can
eventually go mainline.
Thanks,
Ahmad
[1]: https://www.kernel.org/doc/html/v5.13/filesystems/fscrypt.html
[2]: https://lore.kernel.org/linux-fscrypt/367ea5bb-76cf-6020-cb99-91b5ca82d679@pengutronix.de/
[3]: https://www.youtube.com/watch?v=z_y84v9076c
>
> /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] fscrypt: support trusted keys
From: Ahmad Fatoum @ 2021-08-09 10:02 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Sumit Garg, David Howells, Theodore Y. Ts'o,
linux-security-module, James Bottomley, James Morris, Mimi Zohar,
linux-kernel, Eric Biggers, linux-fscrypt, keyrings, linux-crypto,
kernel, Jaegeuk Kim, linux-integrity, Serge E. Hallyn
In-Reply-To: <10dac5c6-4530-217c-e1ea-a7e2e3572f43@pengutronix.de>
On 09.08.21 12:00, Ahmad Fatoum wrote:
> Hello Jarkko,
>
> On 09.08.21 11:44, Jarkko Sakkinen wrote:
>> On Fri, Aug 06, 2021 at 05:09:28PM +0200, Ahmad Fatoum wrote:
>>> 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
>>> - if (key->type != &key_type_fscrypt_provisioning)
>>> - goto bad_key;
>>> - payload = key->payload.data[0];
>>> + if (key->type == &key_type_fscrypt_provisioning) {
>>
>> Why does fscrypt have own key type, and does not extend 'encrypted' with a
>> new format [*]?
>
> See the commit[1] adding it for more information. TL;DR:
>
> fscrypt maintainers would've preferred keys to be associated with
> a "domain". So an encrypted key generated for fscrypt use couldn't be reused
> for e.g. dm-crypt. They are wary of fscrypt users being more exposed if their
> keys can be used with weaker ciphers via other kernel functionality that could
> be used to extract information about the raw key material.
>
> Eric also mentioned dislike of the possibility of rooting encrypted keys to
> user keys. v2 is only restricted to v2, so we didn't discuss this further.
Typo: v2 (of my series) is only restricted to s/v2/trusted keys/
>
> Restricting the key to fscrypt-only precludes this reuse.
>
> My commit makes no attempts in changing that. It just adds a new way to pass
> raw key material into fscrypt. For more information, see the commit[1] adding
> that key type.
>
>> [*] https://www.kernel.org/doc/html/v5.13/security/keys/trusted-encrypted.html
>
> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=93edd392ca
>
> Cheers,
> Ahmad
>
--
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] fscrypt: support trusted keys
From: Ahmad Fatoum @ 2021-08-09 10:00 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Theodore Y. Ts'o, Jaegeuk Kim, Eric Biggers, kernel,
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: <20210809094408.4iqwsx77u64usfx6@kernel.org>
Hello Jarkko,
On 09.08.21 11:44, Jarkko Sakkinen wrote:
> On Fri, Aug 06, 2021 at 05:09:28PM +0200, Ahmad Fatoum wrote:
>> 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
>> - if (key->type != &key_type_fscrypt_provisioning)
>> - goto bad_key;
>> - payload = key->payload.data[0];
>> + if (key->type == &key_type_fscrypt_provisioning) {
>
> Why does fscrypt have own key type, and does not extend 'encrypted' with a
> new format [*]?
See the commit[1] adding it for more information. TL;DR:
fscrypt maintainers would've preferred keys to be associated with
a "domain". So an encrypted key generated for fscrypt use couldn't be reused
for e.g. dm-crypt. They are wary of fscrypt users being more exposed if their
keys can be used with weaker ciphers via other kernel functionality that could
be used to extract information about the raw key material.
Eric also mentioned dislike of the possibility of rooting encrypted keys to
user keys. v2 is only restricted to v2, so we didn't discuss this further.
Restricting the key to fscrypt-only precludes this reuse.
My commit makes no attempts in changing that. It just adds a new way to pass
raw key material into fscrypt. For more information, see the commit[1] adding
that key type.
> [*] https://www.kernel.org/doc/html/v5.13/security/keys/trusted-encrypted.html
[1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=93edd392ca
Cheers,
Ahmad
--
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 2/4] KEYS: trusted: allow trust sources to use kernel RNG for key material
From: Jarkko Sakkinen @ 2021-08-09 9:56 UTC (permalink / raw)
To: Ahmad Fatoum
Cc: Sumit Garg, James Bottomley, 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: <7537c853-3641-a6d3-91d8-70fea9f01a89@pengutronix.de>
On Mon, Aug 09, 2021 at 09:52:20AM +0200, Ahmad Fatoum wrote:
> 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.
Ternary operators are pain to read, unless a super trivial case.
I'd stick to what you did.
/Jarkko
^ permalink raw reply
* Re: [PATCH v2] fscrypt: support trusted keys
From: Jarkko Sakkinen @ 2021-08-09 9:44 UTC (permalink / raw)
To: Ahmad Fatoum
Cc: Theodore Y. Ts'o, Jaegeuk Kim, Eric Biggers, kernel,
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: <20210806150928.27857-1-a.fatoum@pengutronix.de>
On Fri, Aug 06, 2021 at 05:09:28PM +0200, Ahmad Fatoum wrote:
> 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) {
Why does fscrypt have own key type, and does not extend 'encrypted' with a
new format [*]?
> + 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 {
I don't think this is right, or at least it does not follow the pattern
in [*]. I.e. you should rather use trusted key to seal your fscrypt key.
> + err = -EKEYREJECTED;
> + }
>
> -bad_key:
> - err = -EKEYREJECTED;
> out_put:
> key_ref_put(ref);
> return err;
> --
> 2.30.2
>
>
[*] https://www.kernel.org/doc/html/v5.13/security/keys/trusted-encrypted.html
/Jarkko
^ permalink raw reply
* 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
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