linux-kernel - Re: [RFC PATCH] platform/x86: Add sysfs interface for Intel TXT status

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 9 Mar 2022 17:55:10 +0000
From:   Jonathan McDowell <noodles@...com>
To:     Matthew Garrett <mjg59@...f.ucam.org>
CC:     Dmitrii Okunev <xaionaro@...com>,
        Hans de Goede <hdegoede@...hat.com>,
        Mark Gross <markgross@...nel.org>,
        Qiaowei Ren <qiaowei.ren@...el.com>,
        Xiaoyan Zhang <xiaoyan.zhang@...el.com>,
        Pavel Machek <pavel@...x.de>,
        Greg Kroah-Hartman <greg@...ah.com>,
        "x86@...nel.org" <x86@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "platform-driver-x86@...r.kernel.org" 
        <platform-driver-x86@...r.kernel.org>
Subject: Re: [RFC PATCH] platform/x86: Add sysfs interface for Intel TXT
 status

On Wed, Mar 09, 2022 at 10:53:43AM +0000, Matthew Garrett wrote:
> On Wed, Mar 09, 2022 at 10:40:03AM +0000, Jonathan McDowell wrote:
> 
> > This module provides read-only access to the Intel TXT (Trusted
> > Execution Technology) status registers, allowing userspace to determine
> > the status of measured boot and whether the dynamic root of trust for
> > measurement (DRTM) has been fully enabled.
> 
> So there's the obvious issue that in the event that the system has been 
> compromised this information is no longer trustworthy - is this expected 
> to just be informative for diagnostic purposes rather than forming any 
> part of security policy?

This is purely for diagnostic purposes when the system ends up in an
unexpected state, to aid automated remediation. In particular dealing
with unexpected PCR0 values that have failed attestation; the state of
TXT helps with trying to diagnose how we got to the unexpected value.

> > +	  These registers provide details about the status of the platform's
> > +	  measured launch and execution environment, allowing userspace to
> > +	  make trust based decisions. See tboot
> 
> Mm. This makes it sound like it's expected that userspace make decisions 
> based on this, which sounds like a bad plan?

I should clean up the description to be clearer for v2.

> > +/* Shows if TXT has been enabled */
> > +static int txt_enabled_show(struct seq_file *m, void *v)
> > +{
> > +	/* If the BIOS has enabled TXT then the heap base will be set */
> 
> Sorry it's not that I want to say "Wait are you trusting that the BIOS 
> will do the right thing here" but wait are you trusting that the BIOS 
> will do the right thing here? Does setting the heap base guarantee that 
> TXT was enabled (and, conversely, are there any scenarios where TXT was 
> enabled and the BIOS could have cleared the heap base after a 
> measurement event?)

If the BIOS hasn't enabled the heap then it won't have been able to pass
any data to the system for a managed launch environment, so I think it's
fairly safe to assume if it's never been set we're not in a situation
where anything has tried to initialise TXT. If we're in a situation
where we've tried to initialise it and then something has gone wrong and
the BIOS has managed to clear it that's probably a firmware bug and I'd
expect to see some status register oddities too. We're not trusting the
BIOS here, we're just using the fact the heap is actually setup to
indicate that we've tried to do TXT in terms of diagnosis. (And, in
fact, one of the failure modes we've seen is where TXT is supposed to be
configured in the BIOS but it doesn't actually get setup at all and we
correctly see a 0 entry here.)

> > +/* Shows the 256 bit hash of the public key */
> > +static int txt_key_show(struct seq_file *m, void *v)
> > +{
> > +	seq_printf(m, "%016llx%016llx%016llx%016llx\n",
> > +		cpu_to_be64(*(u64 *)(txt_pub_regs + TXT_CR_PUBLIC_KEY)),
> > +		cpu_to_be64(*(u64 *)(txt_pub_regs + TXT_CR_PUBLIC_KEY + 8)),
> > +		cpu_to_be64(*(u64 *)(txt_pub_regs + TXT_CR_PUBLIC_KEY + 16)),
> > +		cpu_to_be64(*(u64 *)(txt_pub_regs + TXT_CR_PUBLIC_KEY + 24)));
> 
> What's the expected consumer of this, and what are they expected to do
> with it?

I added it purely because txt-stat is already showing it, and it differs
between platforms. I note 315168-017 says newer platforms use CPU MSRs
0x20::0x23 instead. So it's informational at this point for older
platforms.

J.