Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: LJIv8flpL0DOywwYbtQu5Hl8LrWdqdaH6V2Dqx7GplVt/wXlIzbf9ciy7uAtbQqyzVWBVVfBTK
 b9kdr44se+dw==
IronPort-SDR: FWtAd1kDyXb7Jwvx402XLN432nPl1pZL/T1Abu+ZisNKd8aDkusbQdrCa19Ey81VCSABByadA/
 uJBokpct78oQ==
Message-ID: <2dad6366d2fceb0a9e36f284a8ed5a8ed86d8756.camel@linux.intel.com>
Subject: Re: [GRUB PATCH RFC 00/18] i386: Intel TXT secure launcher
From:   Lukasz Hawrylko <lukasz.hawrylko@linux.intel.com>
To:     Daniel Kiper <daniel.kiper@oracle.com>, grub-devel@gnu.org,
        linux-kernel@vger.kernel.org, trenchboot-devel@googlegroups.com,
        x86@kernel.org
Cc:     alexander.burmashev@oracle.com, andrew.cooper3@citrix.com,
        ard.biesheuvel@linaro.org, dpsmith@apertussolutions.com,
        eric.snowberg@oracle.com, javierm@redhat.com,
        kanth.ghatraju@oracle.com, konrad.wilk@oracle.com,
        krystian.hebel@3mdeb.com, michal.zygowski@3mdeb.com,
        mjg59@google.com, phcoder@gmail.com, pirot.krol@3mdeb.com,
        pjones@redhat.com, ross.philipson@oracle.com
Date:   Tue, 05 May 2020 16:38:02 +0200
In-Reply-To: <20200504232132.23570-1-daniel.kiper@oracle.com>
References: <20200504232132.23570-1-daniel.kiper@oracle.com>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.34.4 (3.34.4-1.fc31) 
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, 2020-05-05 at 01:21 +0200, Daniel Kiper wrote:
> Hi,
> 
> This is an RFC patchset for the GRUB introducing the Intel TXT secure launcher.
> This is a part of larger work known as the TrenchBoot. Patchset can be split
> into two distinct parts:
>   - 01-12: preparatory patches,
>   - 13-18: the Intel TXT secure launcher itself.
> 
> The initial implementation of the Intel TXT secure launcher works. However,
> there are still some missing bits and pieces, e.g.:
>   - SINIT ACM auto loader,
>   - lack of RMRR support,
>   - lack of support for MLEs larger than 1 GiB,
>   - lack of TPM 1.2 support.
>   - various fixes and cleanups.
> 
> Commands introduced by this patchset: tpm_type, slaunch, slaunch_module (not
> required on server platforms) and slaunch_state (useful for checking platform
> configuration and state; based on tboot's txt-stat).
> 
> Daniel
> 

Hi Daniel

Your patch looks promising, however I have few concerns.

In OS-MLE table there is a buffer for TPM event log, however I see that
you are not using it, but instead allocate space somewhere in the
memory. I am just wondering if, from security perspective, it will be
better to use memory from TXT heap for event log, like we do in TBOOT.

There is a function that verifies if platform is TXT capable
-grub_txt_verify_platform(), it only checks SMX and GETSEC features.
Although BIOS should enforce both VMX and VT-d enabled when enabling
TXT, I think that adding these check here as redundancy may be a good
idea. The same situation is with TPM presence.

I suggest to add possibility to skip TXT launch when last boot ended
with TXT error. This option can avoid boot loops when something goes
wrong.

How will you read LCP from storage? I see that there is slaunch_module
command that currently you are using only for loading SINIT. In the
future it can be expanded to support LCP file too, what do you think?

Do not forget to apply changes required by latest Intel's platforms, you
should check following commits in TBOOT's repository: 2f03b57ffdba,
fe2dddd742dc.

Lukasz