Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sun, 13 Nov 2022 12:46:40 -0800
From:   Eric Biggers <ebiggers@kernel.org>
To:     Evan Green <evgreen@chromium.org>
Cc:     linux-kernel@vger.kernel.org, corbet@lwn.net,
        linux-integrity@vger.kernel.org, gwendal@chromium.org,
        dianders@chromium.org, apronin@chromium.org,
        Pavel Machek <pavel@ucw.cz>, Ben Boeckel <me@benboeckel.net>,
        rjw@rjwysocki.net, jejb@linux.ibm.com,
        Kees Cook <keescook@chromium.org>, dlunev@google.com,
        zohar@linux.ibm.com, Matthew Garrett <mgarrett@aurora.tech>,
        jarkko@kernel.org, linux-pm@vger.kernel.org,
        Matthew Garrett <mjg59@google.com>,
        Jason Gunthorpe <jgg@ziepe.ca>, Peter Huewe <peterhuewe@gmx.de>
Subject: Re: [PATCH v5 03/11] tpm: Allow PCR 23 to be restricted to
 kernel-only use
Message-ID: <Y3FXsPbBeZjAKv1/@sol.localdomain>
References: <20221111231636.3748636-1-evgreen@chromium.org>
 <20221111151451.v5.3.I9ded8c8caad27403e9284dfc78ad6cbd845bc98d@changeid>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20221111151451.v5.3.I9ded8c8caad27403e9284dfc78ad6cbd845bc98d@changeid>
Precedence: bulk

On Fri, Nov 11, 2022 at 03:16:28PM -0800, Evan Green wrote:
> Introduce a new Kconfig, TCG_TPM_RESTRICT_PCR, which if enabled

TCG_TPM_RESTRICT_PCR => TCG_TPM2_RESTRICT_PCR

> For systems with TPM1 devices, having this Kconfig enabled completely
> restricts usermode's access to the TPM.

This doesn't appear to actually be the case.

> +config TCG_TPM2_RESTRICT_PCR
> +	bool "Restrict userland access to PCR 23 on TPM2 devices"
> +	depends on TCG_TPM

I assume you also considered making this a once-settable sysctl, or similar?
I guess this kconfig is fine for now, but IMO it does violate the concept of
"kernel provides mechanism, not policy".

> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 303ce2ea02a4b0..3bc5546fddc792 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -778,3 +778,25 @@ int tpm2_find_cc(struct tpm_chip *chip, u32 cc)
>  
>  	return -1;
>  }
> +
> +#ifdef CONFIG_TCG_TPM2_RESTRICT_PCR
> +int tpm2_cmd_restricted(struct tpm_chip *chip, u8 *buffer, size_t size)
> +{
> +	int cc = tpm2_find_and_validate_cc(chip, NULL, buffer, size);
> +	__be32 *handle;
> +
> +	switch (cc) {
> +	case TPM2_CC_PCR_EXTEND:
> +	case TPM2_CC_PCR_RESET:
> +		if (size < (TPM_HEADER_SIZE + sizeof(u32)))
> +			return -EINVAL;
> +
> +		handle = (__be32 *)&buffer[TPM_HEADER_SIZE];
> +		if (be32_to_cpu(*handle) == TPM_RESTRICTED_PCR)
> +			return -EPERM;

get_unaligned_be32((__be32 *)&buffer[TPM_HEADER_SIZE]),
to avoid an unaligned memory access.

> +		break;
> +	}
> +
> +	return 0;

So, if tpm2_find_and_validate_cc() returns an error code, the command is *not*
restricted, even if it uses one of the forbidden command codes.  Are you sure
there are no loopholes here?

- Eric