All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Lutomirski <luto@kernel.org>
To: "Daniel P. Smith" <dpsmith@apertussolutions.com>
Cc: Daniel Kiper <daniel.kiper@oracle.com>,
	Lukasz Hawrylko <lukasz.hawrylko@linux.intel.com>,
	grub-devel@gnu.org, LKML <linux-kernel@vger.kernel.org>,
	trenchboot-devel@googlegroups.com, X86 ML <x86@kernel.org>,
	alexander.burmashev@oracle.com,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	eric.snowberg@oracle.com, javierm@redhat.com,
	kanth.ghatraju@oracle.com,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	krystian.hebel@3mdeb.com, michal.zygowski@3mdeb.com,
	Matthew Garrett <mjg59@google.com>,
	phcoder@gmail.com, piotr.krol@3mdeb.com,
	Peter Jones <pjones@redhat.com>,
	Ross Philipson <ross.philipson@oracle.com>
Subject: Re: [GRUB PATCH RFC 00/18] i386: Intel TXT secure launcher
Date: Mon, 1 Jun 2020 09:51:23 -0700	[thread overview]
Message-ID: <CALCETrVPK=m3F84NtiU59SLyDrBNxi1ONyhH1GuOhx4aGU=_fQ@mail.gmail.com> (raw)
In-Reply-To: <d1b55f25-e54c-b259-8836-d834a572de3b@apertussolutions.com>

On Mon, Jun 1, 2020 at 8:33 AM Daniel P. Smith
<dpsmith@apertussolutions.com> wrote:
>
> On 5/7/20 7:06 AM, Daniel Kiper wrote:
> > Hi Łukasz,
> >
> > On Tue, May 05, 2020 at 04:38:02PM +0200, Lukasz Hawrylko wrote:
> >> On Tue, 2020-05-05 at 01:21 +0200, Daniel Kiper wrote:
>
> ...
>
> >> 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
> >
> > I think that this part requires more discussion. In my opinion we should
> > have this region dynamically allocated because it gives us more flexibility.
> > Of course there is a question about the size of this buffer too. I am
> > not sure about that because I have not checked yet how many log entries
> > are created by the SINIT ACM. Though probably it should not be large...
> >
> >> 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.
> >
> > Appendix F, TPM Event Log, has following sentence: There are no
> > requirements for event log to be in DMA protected memory – SINIT will
> > not enforce it.
> >
> > I was thinking about it and it seems to me that the TPM event log does
> > not require any special protections. Any changes in it can be quickly
> > detected by comparing hashes with the TPM PCRs. Does not it?
> >
>
> I think it would be beneficial to consider the following in deciding
> where the log is placed. There are two areas of attack/manipulation that
> need to be considered. The first area is the log contents itself, which
> as Daniel has pointed out, the log contents do not really need to be
> protected from tampering as that would/should be detected during
> verification by the attestor. The second area that we need to consider
> is the log descriptors themselves. If these are in an area that can be
> manipulated, it is an opportunity for an attacker to attempt to
> influence the ACM's execution. For a little perspective, the ACM
> executes from CRAM to take the most possible precaution to ensure that
> it cannot be tampered with during execution. This is very important
> since it runs a CPU mode (ACM Mode) that I would consider to be higher
> (or lower depending on how you view it) than SMM. As a result, the txt
> heap is also included in what is mapped into CRAM. If the event log is
> place in the heap, this ensures that the ACM is not using memory outside
> of CRAM during execution. Now as Daniel has pointed out, the down side
> to this is that it greatly restricts the log size and can only be
> managed by a combination of limiting the number of events and
> restricting what content is carried in the event data field.

Can you clarify what the actual flow of control is?  If I had to guess, it's:

GRUB (or other bootloader) writes log.

GRUB transfers control to the ACM.  At this point, GRUB is done
running and GRUB code will not run again.

ACM validates system configuration and updates TPM state using magic
privileged TPM access.

ACM transfers control to the shiny new Linux secure launch entry point

Maybe this is right, and maybe this is wrong.  But I have some
questions about this whole setup.  Is the ACM code going to inspect
this log at all?  If so, why?  Who supplies the ACM code?  If the ACM
can be attacked by putting its inputs (e.g. this log) in the wrong
place in memory, why should this be considered anything other than a
bug in the ACM?

If GRUB is indeed done by the time anyone consumes the log, why does
GRUB care where the log ends up?

And finally, the log presumably has nonzero size, and it would be nice
not to pin some physical memory forever for the log.  Could the kernel
copy it into tmpfs during boot so it's at least swappable and then
allow userspace to delete it when userspace is done with it?

  reply	other threads:[~2020-06-01 16:51 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-04 23:21 [GRUB PATCH RFC 00/18] i386: Intel TXT secure launcher Daniel Kiper
2020-05-04 23:21 ` [GRUB PATCH RFC 01/18] i386/msr: Merge rdmsr.h and wrmsr.h into msr.h Daniel Kiper
2020-05-04 23:21 ` [GRUB PATCH RFC 02/18] i386/msr: Rename grub_msr_read() and grub_msr_write() Daniel Kiper
2020-05-04 23:21 ` [GRUB PATCH RFC 03/18] i386/msr: Extract and improve MSR support detection code Daniel Kiper
2020-05-04 23:21 ` [GRUB PATCH RFC 04/18] i386/memory: Rename PAGE_SHIFT to GRUB_PAGE_SHIFT Daniel Kiper
2020-05-04 23:21 ` [GRUB PATCH RFC 05/18] i386/memory: Rename PAGE_SIZE to GRUB_PAGE_SIZE and make it global Daniel Kiper
2020-05-04 23:21 ` [GRUB PATCH RFC 06/18] mmap: Add grub_mmap_get_lowest() and grub_mmap_get_highest() Daniel Kiper
2020-05-04 23:21 ` [GRUB PATCH RFC 07/18] i386/tpm: Rename tpm module to tpm_verifier Daniel Kiper
2020-05-04 23:21 ` [GRUB PATCH RFC 08/18] i386/tpm: Add TPM TIS and CRB driver Daniel Kiper
2020-05-04 23:21 ` [GRUB PATCH RFC 09/18] efi: Make shim_lock GUID and protocol type public Daniel Kiper
2020-05-04 23:21 ` [GRUB PATCH RFC 10/18] efi: Return grub_efi_status_t from grub_efi_get_variable() Daniel Kiper
2020-05-04 23:21 ` [GRUB PATCH RFC 11/18] efi: Add a function to read EFI variables with attributes Daniel Kiper
2020-05-04 23:21 ` [GRUB PATCH RFC 12/18] i386/efi: Report UEFI Secure Boot status to the Linux kernel Daniel Kiper
2020-05-05 17:29   ` Matthew Garrett
2020-05-06 13:33     ` Daniel Kiper
2020-05-06 18:36       ` Matthew Garrett
2020-05-07 10:46         ` Daniel Kiper
2020-05-04 23:21 ` [GRUB PATCH RFC 13/18] i386/slaunch: Add basic platform support for secure launch Daniel Kiper
2020-05-04 23:21 ` [GRUB PATCH RFC 14/18] i386/txt: Add Intel TXT definitions header file Daniel Kiper
2020-05-04 23:21 ` [GRUB PATCH RFC 15/18] i386/txt: Add Intel TXT core implementation Daniel Kiper
2020-05-22 13:24   ` Krystian Hebel
2020-06-01 14:16     ` Ross Philipson
2020-05-04 23:21 ` [GRUB PATCH RFC 16/18] i386/txt: Add Intel TXT ACM module support Daniel Kiper
2020-05-04 23:21 ` [GRUB PATCH RFC 17/18] i386/txt: Add Intel TXT verification routines Daniel Kiper
2020-05-04 23:21 ` [GRUB PATCH RFC 18/18] i386/slaunch: Add secure launch framework and commands Daniel Kiper
2020-05-05 14:38 ` [GRUB PATCH RFC 00/18] i386: Intel TXT secure launcher Lukasz Hawrylko
2020-05-07 11:06   ` Daniel Kiper
2020-05-13 13:47     ` Lukasz Hawrylko
2020-06-01 15:32     ` Daniel P. Smith
2020-06-01 16:51       ` Andy Lutomirski [this message]
2020-06-01 17:56         ` Daniel P. Smith
2020-06-01 18:03           ` Ross Philipson
2020-06-01 19:39           ` Andy Lutomirski
2020-06-02  0:13             ` Daniel P. Smith
2020-06-02  0:49               ` Andy Lutomirski
2020-06-02  1:29                 ` Daniel P. Smith

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CALCETrVPK=m3F84NtiU59SLyDrBNxi1ONyhH1GuOhx4aGU=_fQ@mail.gmail.com' \
    --to=luto@kernel.org \
    --cc=alexander.burmashev@oracle.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=ard.biesheuvel@linaro.org \
    --cc=daniel.kiper@oracle.com \
    --cc=dpsmith@apertussolutions.com \
    --cc=eric.snowberg@oracle.com \
    --cc=grub-devel@gnu.org \
    --cc=javierm@redhat.com \
    --cc=kanth.ghatraju@oracle.com \
    --cc=konrad.wilk@oracle.com \
    --cc=krystian.hebel@3mdeb.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lukasz.hawrylko@linux.intel.com \
    --cc=michal.zygowski@3mdeb.com \
    --cc=mjg59@google.com \
    --cc=phcoder@gmail.com \
    --cc=piotr.krol@3mdeb.com \
    --cc=pjones@redhat.com \
    --cc=ross.philipson@oracle.com \
    --cc=trenchboot-devel@googlegroups.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.