tboot-devel — Developer support

[tboot-devel] [PATCH 1 of 3] Ignore modules that overlap with internal data structures

[Timo L. <tim...@ik...>]

# HG changeset patch
# User Timo Lindfors <tim...@ik...>
# Date 1647554321 -7200
#      Thu Mar 17 23:58:41 2022 +0200
# Node ID 94606b71c3dabaabee813971a223686257d65d52
# Parent  bcdf58c1d076bfe41c58ac93254914dc0f2fd449
Ignore modules that overlap with internal data structures
Without this patch the system can go to an infinite reboot loop as
corrupted module causes the system to reset.

Signed-off-by: Timo Lindfors <tim...@ik...>

diff -r bcdf58c1d076 -r 94606b71c3da include/config.h
--- a/include/config.h	Thu Mar 10 10:28:11 2022 +0200
+++ b/include/config.h	Thu Mar 17 23:58:41 2022 +0200
@@ -52,9 +52,10 @@
 
 /* these addrs must be in low memory so that they are mapped by the */
 /* kernel at startup */
+#define TBOOT_LOWMEM_START           0x60000
 
 /* address/size for memory-resident serial log (when enabled) */
-#define TBOOT_SERIAL_LOG_ADDR        0x60000
+#define TBOOT_SERIAL_LOG_ADDR        TBOOT_LOWMEM_START
 #define TBOOT_SERIAL_LOG_SIZE        0x08000
 
 /* address/size for modified e820 table */
@@ -72,6 +73,8 @@
                                       TBOOT_EFI_MEMMAP_COPY_SIZE)
 #define TBOOT_KERNEL_CMDLINE_SIZE    0x0400
 
+#define TBOOT_LOWMEM_END             (TBOOT_KERNEL_CMDLINE_ADDR + \
+                                      TBOOT_KERNEL_CMDLINE_SIZE)
 
 #ifndef NR_CPUS
 #define NR_CPUS     1024
diff -r bcdf58c1d076 -r 94606b71c3da tboot/common/loader.c
--- a/tboot/common/loader.c	Thu Mar 10 10:28:11 2022 +0200
+++ b/tboot/common/loader.c	Thu Mar 17 23:58:41 2022 +0200
@@ -1807,6 +1807,42 @@
 }
 
 /*
+ * Check if two memory regions overlap
+ */
+static bool
+regions_overlap(const void *base1, size_t size1, const void *base2, size_t size2) {
+    /*
+      11111
+        22222
+    */
+    if (base1 <= base2 && base2 < base1 + size1) {
+        return true;
+    }
+    /*
+        11111
+      22222
+    */
+    if (base2 <= base1 && base1 < base2 + size2) {
+        return true;
+    }
+    /*
+        1
+      22222
+    */
+    if (base2 <= base1 && base1 + size1 < base2 + size2) {
+        return true;
+    }
+    /*
+      11111
+        2
+    */
+    if (base1 <= base2 && base2 + size2 < base1 + size1) {
+        return true;
+    }
+    return false;
+}
+
+/*
  * will go through all modules to find an SINIT that matches the platform
  * (size can be NULL)
  */
@@ -1836,6 +1872,11 @@
 
         void *base2 = (void *)m->mod_start;
         uint32_t size2 = m->mod_end - (unsigned long)(base2);
+        if (regions_overlap(base2, size2,
+                            (void*)TBOOT_LOWMEM_START, TBOOT_LOWMEM_END - TBOOT_LOWMEM_START)) {
+            printk(TBOOT_DETA "Ignoring module as it overlaps with tboot's internal data structures\n");
+            continue;
+        }
         if ( is_sinit_acmod(base2, size2, false) &&
              does_acmod_match_platform((acm_hdr_t *)base2, NULL) ) {
             if ( base != NULL )

[tboot-devel] [PATCH 3 of 3] Allow selecting only SINIT modules that match platform

[Timo L. <tim...@ik...>]

# HG changeset patch
# User Timo Lindfors <tim...@ik...>
# Date 1647554330 -7200
#      Thu Mar 17 23:58:50 2022 +0200
# Node ID 538c14b1428d0625ebb3f9c3cae21656fd4c3b06
# Parent  e45ccbe6bf59ba534ad628f7be45e7c34629e19b
Allow selecting only SINIT modules that match platform
This introduces GRUB_TBOOT_SINIT_SELECT_MATCHING that defaults to
false.

Signed-off-by: Timo Lindfors <tim...@ik...>

diff -r e45ccbe6bf59 -r 538c14b1428d tboot/20_linux_tboot
--- a/tboot/20_linux_tboot	Thu Mar 17 23:58:45 2022 +0200
+++ b/tboot/20_linux_tboot	Thu Mar 17 23:58:50 2022 +0200
@@ -40,6 +40,7 @@
 [ -z "${GRUB_CMDLINE_LINUX_TBOOT}" ] && unset GRUB_CMDLINE_LINUX_TBOOT
 [ -z "${GRUB_TBOOT_POLICY_DATA}" ] && unset GRUB_TBOOT_POLICY_DATA
 [ -z "${GRUB_TBOOT_SINIT_LIST}" ] && unset GRUB_TBOOT_SINIT_LIST
+[ -z "${GRUB_TBOOT_SINIT_SELECT_MATCHING}" ] && unset GRUB_TBOOT_SINIT_SELECT_MATCHING
 # Command line for tboot itself
 : ${GRUB_CMDLINE_TBOOT='logging=serial,memory,vga'}
 # Linux kernel parameters to append for tboot
@@ -48,6 +49,8 @@
 : ${GRUB_TBOOT_POLICY_DATA=''}
 # List of SINIT modules to use, glob patterns are supported
 : ${GRUB_TBOOT_SINIT_LIST='/boot/*sinit* /boot/*SINIT*'}
+# Use only SINIT modules that match the current platform
+: ${GRUB_TBOOT_SINIT_SELECT_MATCHING='false'}
 
 export TEXTDOMAIN=grub
 export TEXTDOMAINDIR=${prefix}/share/locale
@@ -164,8 +167,16 @@
 tboot_list=`for i in /boot/tboot*.gz; do
         if grub_file_is_not_garbage "$i" ; then echo -n "$i " ; fi
       done`
+if [ "x${GRUB_TBOOT_SINIT_SELECT_MATCHING}" = "xtrue" ] && [ ! -e /dev/cpu/0/msr ]; then
+  modprobe msr
+fi
 sinit_list=`for i in ${GRUB_TBOOT_SINIT_LIST}; do
         basename=$(basename $i)
+        if [ "x${GRUB_TBOOT_SINIT_SELECT_MATCHING}" = "xtrue" ] \
+            && ! txt-acminfo "$i" | grep -qx "ACM matches platform"; then
+          # Skip SINIT that does not match
+          continue
+        fi
         if grub_file_is_not_garbage "$i" ; then echo -n "$basename " ; fi
       done`
 if [ -n "${GRUB_TBOOT_POLICY_DATA}" ]; then
diff -r e45ccbe6bf59 -r 538c14b1428d tboot/20_linux_xen_tboot
--- a/tboot/20_linux_xen_tboot	Thu Mar 17 23:58:45 2022 +0200
+++ b/tboot/20_linux_xen_tboot	Thu Mar 17 23:58:50 2022 +0200
@@ -41,6 +41,7 @@
 [ -z "${GRUB_CMDLINE_LINUX_XEN_TBOOT}" ] && unset GRUB_CMDLINE_LINUX_XEN_TBOOT
 [ -z "${GRUB_TBOOT_POLICY_DATA}" ] && unset GRUB_TBOOT_POLICY_DATA
 [ -z "${GRUB_TBOOT_SINIT_LIST}" ] && unset GRUB_TBOOT_SINIT_LIST
+[ -z "${GRUB_TBOOT_SINIT_SELECT_MATCHING}" ] && unset GRUB_TBOOT_SINIT_SELECT_MATCHING
 # Command line for tboot itself
 : ${GRUB_CMDLINE_TBOOT='logging=serial,memory,vga'}
 # Xen parameters to append for tboot
@@ -51,6 +52,8 @@
 : ${GRUB_TBOOT_POLICY_DATA=''}
 # List of SINIT modules to use, glob patterns are supported
 : ${GRUB_TBOOT_SINIT_LIST='/boot/*sinit* /boot/*SINIT*'}
+# Use only SINIT modules that match the current platform
+: ${GRUB_TBOOT_SINIT_SELECT_MATCHING='false'}
 
 export TEXTDOMAIN=grub
 export TEXTDOMAINDIR=${prefix}/share/locale
@@ -194,8 +197,16 @@
 tboot_list=`for i in /boot/tboot*.gz; do
         if grub_file_is_not_garbage "$i" ; then echo -n "$i " ; fi
       done`
+if [ "x${GRUB_TBOOT_SINIT_SELECT_MATCHING}" = "xtrue" ] && [ ! -e /dev/cpu/0/msr ]; then
+  modprobe msr
+fi
 sinit_list=`for i in ${GRUB_TBOOT_SINIT_LIST}; do
         basename=$(basename $i)
+        if [ "x${GRUB_TBOOT_SINIT_SELECT_MATCHING}" = "xtrue" ] \
+            && ! txt-acminfo "$i" | grep -qx "ACM matches platform"; then
+          # Skip SINIT that does not match
+          continue
+        fi
         if grub_file_is_not_garbage "$i" ; then echo -n "$basename " ; fi
       done`
 if [ -n "${GRUB_TBOOT_POLICY_DATA}" ]; then

[tboot-devel] [PATCH 2 of 3] Introduce GRUB_TBOOT_SINIT_LIST for selecting SINIT modules to use

[Timo L. <tim...@ik...>]

# HG changeset patch
# User Timo Lindfors <tim...@ik...>
# Date 1647554325 -7200
#      Thu Mar 17 23:58:45 2022 +0200
# Node ID e45ccbe6bf59ba534ad628f7be45e7c34629e19b
# Parent  94606b71c3dabaabee813971a223686257d65d52
Introduce GRUB_TBOOT_SINIT_LIST for selecting SINIT modules to use
Signed-off-by: Timo Lindfors <tim...@ik...>

diff -r 94606b71c3da -r e45ccbe6bf59 tboot/20_linux_tboot
--- a/tboot/20_linux_tboot	Thu Mar 17 23:58:41 2022 +0200
+++ b/tboot/20_linux_tboot	Thu Mar 17 23:58:45 2022 +0200
@@ -39,12 +39,15 @@
 [ -z "${GRUB_CMDLINE_TBOOT}" ] && unset GRUB_CMDLINE_TBOOT
 [ -z "${GRUB_CMDLINE_LINUX_TBOOT}" ] && unset GRUB_CMDLINE_LINUX_TBOOT
 [ -z "${GRUB_TBOOT_POLICY_DATA}" ] && unset GRUB_TBOOT_POLICY_DATA
+[ -z "${GRUB_TBOOT_SINIT_LIST}" ] && unset GRUB_TBOOT_SINIT_LIST
 # Command line for tboot itself
 : ${GRUB_CMDLINE_TBOOT='logging=serial,memory,vga'}
 # Linux kernel parameters to append for tboot
 : ${GRUB_CMDLINE_LINUX_TBOOT='intel_iommu=on'}
 # Base name of LCP policy data file for list policy
 : ${GRUB_TBOOT_POLICY_DATA=''}
+# List of SINIT modules to use, glob patterns are supported
+: ${GRUB_TBOOT_SINIT_LIST='/boot/*sinit* /boot/*SINIT*'}
 
 export TEXTDOMAIN=grub
 export TEXTDOMAINDIR=${prefix}/share/locale
@@ -161,7 +164,7 @@
 tboot_list=`for i in /boot/tboot*.gz; do
         if grub_file_is_not_garbage "$i" ; then echo -n "$i " ; fi
       done`
-sinit_list=`for i in /boot/*sinit* /boot/*SINIT*; do
+sinit_list=`for i in ${GRUB_TBOOT_SINIT_LIST}; do
         basename=$(basename $i)
         if grub_file_is_not_garbage "$i" ; then echo -n "$basename " ; fi
       done`
diff -r 94606b71c3da -r e45ccbe6bf59 tboot/20_linux_xen_tboot
--- a/tboot/20_linux_xen_tboot	Thu Mar 17 23:58:41 2022 +0200
+++ b/tboot/20_linux_xen_tboot	Thu Mar 17 23:58:45 2022 +0200
@@ -40,6 +40,7 @@
 [ -z "${GRUB_CMDLINE_XEN_TBOOT}" ] && unset GRUB_CMDLINE_XEN_TBOOT
 [ -z "${GRUB_CMDLINE_LINUX_XEN_TBOOT}" ] && unset GRUB_CMDLINE_LINUX_XEN_TBOOT
 [ -z "${GRUB_TBOOT_POLICY_DATA}" ] && unset GRUB_TBOOT_POLICY_DATA
+[ -z "${GRUB_TBOOT_SINIT_LIST}" ] && unset GRUB_TBOOT_SINIT_LIST
 # Command line for tboot itself
 : ${GRUB_CMDLINE_TBOOT='logging=serial,memory,vga'}
 # Xen parameters to append for tboot
@@ -48,6 +49,8 @@
 : ${GRUB_CMDLINE_LINUX_XEN_TBOOT=''}
 # Base name of LCP policy data file for list policy
 : ${GRUB_TBOOT_POLICY_DATA=''}
+# List of SINIT modules to use, glob patterns are supported
+: ${GRUB_TBOOT_SINIT_LIST='/boot/*sinit* /boot/*SINIT*'}
 
 export TEXTDOMAIN=grub
 export TEXTDOMAINDIR=${prefix}/share/locale
@@ -191,7 +194,7 @@
 tboot_list=`for i in /boot/tboot*.gz; do
         if grub_file_is_not_garbage "$i" ; then echo -n "$i " ; fi
       done`
-sinit_list=`for i in /boot/*sinit* /boot/*SINIT*; do
+sinit_list=`for i in ${GRUB_TBOOT_SINIT_LIST}; do
         basename=$(basename $i)
         if grub_file_is_not_garbage "$i" ; then echo -n "$basename " ; fi
       done`

Re: [tboot-devel] status of the grub patch to support multiple SINIT modules?

[Timo L. <tim...@ik...>]

On Fri, 11 Mar 2022, Łukasz Hawryłko wrote:
> I see that you have quite complex environment for testing tboot, if I
> find my old GRUB patch and prepare patch for tboot that combined should
> fix the issue, will you be able to run tests?

Yes, I'm happy to run tests :)

Re: [tboot-devel] status of the grub patch to support multiple SINIT modules?

[Łukasz H. <lu...@ha...>]

On Fri, 2022-03-11 at 11:23 +0200, Timo Lindfors wrote:
> Hi,
> 
> On Fri, 11 Mar 2022, Łukasz Hawryłko wrote:
> > In few words - when multiple SINITs is loaded, there is a chance that
> > one (or more) of them will be overwritten by some TBOOT data structures
> > that have hardcoded addresses. In most cases it is memory log, but this
> > is not a rule.
> 
> This sounds annoying indeed. Would it help if we could somehow embed 
> or append the SINIT modules to tboot.gz itself? I'm trying to make the 
> technology as easy to use and robust as possible. Being able to use e.g. 
> the same Live CD on all pieces of hardware would be a huge win.
> 

That could help, but I can't give you a definite answer. There is also
a risk that bigger tboot.gz will cause some other memory corruption
errors. Keep in mind that besides tboot.gz and SINITs, GRUB loads also
kernel image and initrd. The best way to fix all possible problems is
to instruct GRUB not to load anything in memory regions occupied by
tboot's hardcoded structures.

I see that you have quite complex environment for testing tboot, if I
find my old GRUB patch and prepare patch for tboot that combined should
fix the issue, will you be able to run tests?

Thanks,
Lukasz

Re: [tboot-devel] status of the grub patch to support multiple SINIT modules?

[Łukasz H. <lu...@ha...>]

Hi Timo

On Fri, 2022-03-11 at 09:09 +0200, Timo Lindfors wrote:
> Hi,
> 
> in https://sourceforge.net/p/tboot/mailman/message/37340469/ there was a 
> discussion about needing to get grub to accept a patch to reliably support 
> multiple SINIT modules. Any idea what's the status of this patch?
> 
> Using multiple SINIT modules is useful if you want to have a single image 
> that works on multiple devices. The intel-acm package in Debian non-free 
> provides these in /boot and it is very convenient that tboot can 
> choose the matching SINIT module at runtime.

As I left Intel and nobody has taken care about this patch, it has been
abandoned. As far as I remember, there were some minor changes
requested by GRUB maintainers, but overall idea has been accepted.

> 
> I was reminded of this issue since I hit it again on different hardware.
> 
> I've attached two serial console logs for tboot mercurial tip 
> (9c625ab2035b):
> 
> tboot_9c625ab2035b_2_SINIT_working.txt:
> - two SINIT ACMs are specified and the system boots correctly.
> 
> tboot_9c625ab2035b_26_SINIT_reboot.txt:
> - 26 SINIT ACMs are specified and the system enters an infinïte reboot 
> loop.
> 
> I do not see this problem on my BIOS system, only UEFI system, but it is 
> is difficult to say if this is actually related to the issue.
> 
> You can see more logs at https://lindi.iki.fi/lindi/tboot/smoketest/results.html
> The attached logs are all from test run 1646942019.
> 

In few words - when multiple SINITs is loaded, there is a chance that
one (or more) of them will be overwritten by some TBOOT data structures
that have hardcoded addresses. In most cases it is memory log, but this
is not a rule.

Everything depends on system memory map and where GRUB decided to put
SINITs. On some platforms you can load as many SINTIs as you want, on
other - only 2 or 3. So that's platform specific issue, fortunately, I
didn't come across a platform where this problem happens even with 1
SINIT loaded.

I will try to find some time to dig into grub-devel archive, find the
patch, fix it and resubmit it once again. However, it depends on OS
vendors when they will merge it to their distro.

> As a workaround, would you accept a patch that modifies 
> tboot/20_linux_tboot to use txt-acminfo to include only matching SINIT 
> modules in grub configuration? I could make this configurable in 
> /etc/default/grub-tboot. We could for example support the following three 
> options:
> 
> GRUB_TBOOT_SINIT=all
> - include all SINIT modules that are found, current behavior
> 
> GRUB_TBOOT_SINIT=detect
> - use txt-acminfo to find SINIT modules that match the current system.
> 
> GRUB_TBOOT_SINIT_LIST="module1 module2 module3"
> - use only the listed SINIT modules.
> 

That's sounds great to me. I am sure that Intel will accept this
change. It is much better to select proper SINIT during installation
that loads all possible ones every boot, only to always choose the same
right one.

Thanks,
Lukasz

Re: [tboot-devel] status of the grub patch to support multiple SINIT modules?

[Timo L. <tim...@ik...>]

Hi,

On Fri, 11 Mar 2022, Łukasz Hawryłko wrote:
> In few words - when multiple SINITs is loaded, there is a chance that
> one (or more) of them will be overwritten by some TBOOT data structures
> that have hardcoded addresses. In most cases it is memory log, but this
> is not a rule.

This sounds annoying indeed. Would it help if we could somehow embed 
or append the SINIT modules to tboot.gz itself? I'm trying to make the 
technology as easy to use and robust as possible. Being able to use e.g. 
the same Live CD on all pieces of hardware would be a huge win.

-Timo

[tboot-devel] status of the grub patch to support multiple SINIT modules?

[Timo L. <tim...@ik...>]

Hi,

in https://sourceforge.net/p/tboot/mailman/message/37340469/ there was a 
discussion about needing to get grub to accept a patch to reliably support 
multiple SINIT modules. Any idea what's the status of this patch?

Using multiple SINIT modules is useful if you want to have a single image 
that works on multiple devices. The intel-acm package in Debian non-free 
provides these in /boot and it is very convenient that tboot can 
choose the matching SINIT module at runtime.

I was reminded of this issue since I hit it again on different hardware.

I've attached two serial console logs for tboot mercurial tip 
(9c625ab2035b):

tboot_9c625ab2035b_2_SINIT_working.txt:
- two SINIT ACMs are specified and the system boots correctly.

tboot_9c625ab2035b_26_SINIT_reboot.txt:
- 26 SINIT ACMs are specified and the system enters an infinïte reboot 
loop.

I do not see this problem on my BIOS system, only UEFI system, but it is 
is difficult to say if this is actually related to the issue.

You can see more logs at https://lindi.iki.fi/lindi/tboot/smoketest/results.html
The attached logs are all from test run 1646942019.

As a workaround, would you accept a patch that modifies 
tboot/20_linux_tboot to use txt-acminfo to include only matching SINIT 
modules in grub configuration? I could make this configurable in 
/etc/default/grub-tboot. We could for example support the following three 
options:

GRUB_TBOOT_SINIT=all
- include all SINIT modules that are found, current behavior

GRUB_TBOOT_SINIT=detect
- use txt-acminfo to find SINIT modules that match the current system.

GRUB_TBOOT_SINIT_LIST="module1 module2 module3"
- use only the listed SINIT modules.

-Timo

Re: [tboot-devel] [PATCH] txt-acminfo: Map TXT heap using mmap

[Timo L. <tim...@ik...>]

On Wed, 9 Mar 2022, Timo Juhani Lindfors wrote:
> From: Timo Lindfors <tim...@ik...>
>
> Without this patch
>
> txt-acminfo 5th_gen_i5_i7_SINIT_79.BIN
>
> segfaults. This issue was introduced in

This is not the most beautiful patch I must say but I hope it at least 
helps somewhat and does not break anything. I tried adjusting the headers 
so that the duplicate typedef for txt_heap_t could be avoided. I also 
looked the possibility to have callers of does_acmod_match_platform call 
get_txt_heap. In both cases I hit all sorts of challenges with the way the 
include files are setup. So: feel free to rewrite this if you can come up 
with a cleaner way :)

-Timo

[tboot-devel] [PATCH] txt-acminfo: Map TXT heap using mmap

[Timo J. L. <tim...@ik...>]

From: Timo Lindfors <tim...@ik...>

Without this patch

txt-acminfo 5th_gen_i5_i7_SINIT_79.BIN

segfaults. This issue was introduced in

o  changeset:   627:d8a8e17f6d41
|  user:        Lukasz Hawrylko <lukas...@in...>
|  date:        Thu May 13 16:04:27 2021 +0200
|  summary:     Check for client/server match when selecting SINIT

Signed-off-by: Timo Lindfors <tim...@ik...>
---
 tboot/common/loader.c     |  4 ++--
 tboot/include/txt/acmod.h |  4 +++-
 tboot/txt/acmod.c         |  7 ++++---
 utils/txt-acminfo.c       | 26 +++++++++++++++++++++-----
 4 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/tboot/common/loader.c b/tboot/common/loader.c
index cd254d7..0b9a814 100644
--- a/tboot/common/loader.c
+++ b/tboot/common/loader.c
@@ -1792,7 +1792,7 @@ find_platform_racm(loader_ctx *lctx, void **base, uint32_t *size)
         void *base2 = (void *)m->mod_start;
         uint32_t size2 = m->mod_end - (unsigned long)(base2);
         if ( is_racm_acmod(base2, size2, false) &&
-             does_acmod_match_platform((acm_hdr_t *)base2) ) {
+             does_acmod_match_platform((acm_hdr_t *)base2, NULL) ) {
             if ( base != NULL )
                 *base = base2;
             if ( size != NULL )
@@ -1837,7 +1837,7 @@ find_platform_sinit_module(loader_ctx *lctx, void **base, uint32_t *size)
         void *base2 = (void *)m->mod_start;
         uint32_t size2 = m->mod_end - (unsigned long)(base2);
         if ( is_sinit_acmod(base2, size2, false) &&
-             does_acmod_match_platform((acm_hdr_t *)base2) ) {
+             does_acmod_match_platform((acm_hdr_t *)base2, NULL) ) {
             if ( base != NULL )
                 *base = base2;
             if ( size != NULL )
diff --git a/tboot/include/txt/acmod.h b/tboot/include/txt/acmod.h
index 23336c2..fc60d9d 100644
--- a/tboot/include/txt/acmod.h
+++ b/tboot/include/txt/acmod.h
@@ -37,6 +37,8 @@
 #ifndef __TXT_ACMOD_H__
 #define __TXT_ACMOD_H__
 
+typedef void   txt_heap_t;
+
 /*
  * authenticated code (AC) module header (ver 0.0)
  */
@@ -179,7 +181,7 @@ extern bool is_racm_acmod(const void *acmod_base, uint32_t acmod_size, bool quie
 extern acm_hdr_t *copy_racm(const acm_hdr_t *racm);
 extern bool verify_racm(const acm_hdr_t *acm_hdr);
 extern bool is_sinit_acmod(const void *acmod_base, uint32_t acmod_size, bool quiet);
-extern bool does_acmod_match_platform(const acm_hdr_t* hdr);
+extern bool does_acmod_match_platform(const acm_hdr_t* hdr, const txt_heap_t* txt_heap);
 extern acm_hdr_t *copy_sinit(const acm_hdr_t *sinit);
 extern bool verify_acmod(const acm_hdr_t *acm_hdr);
 extern uint32_t get_supported_os_sinit_data_ver(const acm_hdr_t* hdr);
diff --git a/tboot/txt/acmod.c b/tboot/txt/acmod.c
index 1e92efa..798093f 100644
--- a/tboot/txt/acmod.c
+++ b/tboot/txt/acmod.c
@@ -576,7 +576,7 @@ bool is_sinit_acmod(const void *acmod_base, uint32_t acmod_size, bool quiet)
     return true;
 }
 
-bool does_acmod_match_platform(const acm_hdr_t* hdr)
+bool does_acmod_match_platform(const acm_hdr_t* hdr, const txt_heap_t *txt_heap)
 {
     /* used to ensure we don't print chipset/proc info for each module */
     static bool printed_host_info;
@@ -587,7 +587,8 @@ bool does_acmod_match_platform(const acm_hdr_t* hdr)
         return false;
 
     /* verify client/server platform match */
-    txt_heap_t *txt_heap = get_txt_heap();
+    if (txt_heap == NULL)
+        txt_heap = get_txt_heap();
     bios_data_t *bios_data = get_bios_data_start(txt_heap);
     if (info_table->version >= 5 && bios_data->version >= 6) {
         uint32_t bios_type = bios_data->flags.bits.mle.platform_type;
@@ -713,7 +714,7 @@ acm_hdr_t *get_bios_sinit(const void *sinit_region_base)
 
     /* is it a valid SINIT module? */
     if ( !is_sinit_acmod(sinit_region_base, bios_data->bios_sinit_size, false) ||
-         !does_acmod_match_platform((acm_hdr_t *)sinit_region_base) )
+         !does_acmod_match_platform((acm_hdr_t *)sinit_region_base, NULL) )
         return NULL;
 
     return (acm_hdr_t *)sinit_region_base;
diff --git a/utils/txt-acminfo.c b/utils/txt-acminfo.c
index 06a5ee8..5635e27 100644
--- a/utils/txt-acminfo.c
+++ b/utils/txt-acminfo.c
@@ -203,15 +203,31 @@ static bool match_platform(acm_hdr_t *hdr)
         close(fd_mem);
         return false;
     }
-    else {
-        if ( does_acmod_match_platform(hdr) )
-            printf("ACM matches platform\n");
-        else
-            printf("ACM does not match platform\n");
 
+    uint64_t txt_heap_size = *(volatile uint64_t *)(pub_config_base + TXTCR_HEAP_SIZE);
+    if (txt_heap_size == 0) {
+        printf("ERROR: No TXT heap is available\n");
         munmap(pub_config_base, TXT_CONFIG_REGS_SIZE);
+        close(fd_mem);
+        return false;
+    }
+
+    uint64_t txt_heap_base = *(volatile uint64_t *)(pub_config_base + TXTCR_HEAP_BASE);
+    txt_heap_t *txt_heap = mmap(NULL, txt_heap_size, PROT_READ, MAP_PRIVATE,
+                                fd_mem, txt_heap_base);
+    if ( txt_heap == MAP_FAILED ) {
+        printf("ERROR: cannot map TXT heap by mmap()\n");
+        munmap(pub_config_base, TXT_CONFIG_REGS_SIZE);
+        close(fd_mem);
+        return false;
     }
+    if ( does_acmod_match_platform(hdr, txt_heap) )
+        printf("ACM matches platform\n");
+    else
+        printf("ACM does not match platform\n");
 
+    munmap(txt_heap, txt_heap_size);
+    munmap(pub_config_base, TXT_CONFIG_REGS_SIZE);
     close(fd_mem);
     return true;
 }
-- 
2.30.2

Re: [tboot-devel] typo in lcp2_crtpollist man page

[Randzio, P. <paw...@in...>]

> just a quick note: lcp2_crtpollist man page says "algorightm" but should probably say "algorithm". Thought about sending a patch but it's probably easier for you to just fix this directly :)
Fixed ;)

-Paweł
---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.
Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited.

[tboot-devel] typo in lcp2_crtpollist man page

[Timo L. <tim...@ik...>]

Hi,

just a quick note: lcp2_crtpollist man page says "algorightm" but should 
probably say "algorithm". Thought about sending a patch but it's probably 
easier for you to just fix this directly :)

-Timo

Re: [tboot-devel] new PGP for release tarballs?

[Randzio, P. <paw...@in...>]

> Good to know, can you release 1.10.5 so that I can upload it to Debian?
Done.

> Could you perhaps just put the public PGP key to version control if you cannot think of any other place? I don't see any way to ignore PGP validation in the workflow easily as only ".gpg" files are available and that's what the tooling tries to access.
Public key file uploaded to SF

-Paweł
---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.
Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited.

Re: [tboot-devel] new PGP for release tarballs?

[Timo L. <tim...@ik...>]

On Wed, 9 Mar 2022, Randzio, Pawel wrote:
> First of all - package 1.10.4 is already outdated (I mistakenly made a release with bugs), I forgot to add packages for 1.10.5.
> Thanks for the reminder :)
>
> And as for the key - older packages were signed by Łukasz, and I think I haven't been instructed on how to or where to provide the key I've generated for myself.

Good to know, can you release 1.10.5 so that I can upload it to Debian?

Could you perhaps just put the public PGP key to version control if you 
cannot think of any other place? I don't see any way to ignore PGP 
validation in the workflow easily as only ".gpg" files are available and 
that's what the tooling tries to access.

-Timo

Re: [tboot-devel] new PGP for release tarballs?

[Randzio, P. <paw...@in...>]

Hi Timo,

First of all - package 1.10.4 is already outdated (I mistakenly made a release with bugs), I forgot to add packages for 1.10.5.
Thanks for the reminder :)

And as for the key - older packages were signed by Łukasz, and I think I haven't been instructed on how to or where to provide the key I've generated for myself.

-Paweł

-----Original Message-----
From: Timo Lindfors <tim...@ik...> 
Sent: Wednesday, March 9, 2022 10:34 AM
To: tbo...@li...
Subject: [tboot-devel] new PGP for release tarballs?

Hi,

I noticed that the tboot-1.10.4.tar.gz.gpg release is signed using key A16A6B495B7ED435EF129F21FF6B78E7EB6D8A8B. Is this key available from somewhere? Previously key 5CECC9E12872F424009D0E0B6F2F48CC4E0B23EF was used.

-Timo



_______________________________________________
tboot-devel mailing list
tbo...@li...
https://lists.sourceforge.net/lists/listinfo/tboot-devel
---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.
Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited.

[tboot-devel] new PGP for release tarballs?

[Timo L. <tim...@ik...>]

Hi,

I noticed that the tboot-1.10.4.tar.gz.gpg release is signed using key 
A16A6B495B7ED435EF129F21FF6B78E7EB6D8A8B. Is this key available from 
somewhere? Previously key 5CECC9E12872F424009D0E0B6F2F48CC4E0B23EF was 
used.

-Timo

[tboot-devel] changeset 643:8635a40eab19 broke lcp2_mlehash

[Christopher B. <sal...@gm...>]

As of tboot 1.10.4, lcp2_mlehash has stopped working due to 
643:8635a40eab19 Touch ups for GCC12:


lcp2_mlehash --create --verbose /boot/tboot.gz
cmdline opt: mlefile:/boot/tboot.gz
[read_mle_file]
checking whether the file exists or not ... : existed!
trying to uncompress the file ... : succeeded!
creating a temporary file to uncompress ... : succeeded!
opening the decompressed file ... : succeeded!
testing decompression is ... : succeeded!
reading the decompressed file ... : succeeded!
[is_elf_image]
checking whether image is an elf image ... : failed! - Pointer is zero.

It looks like fix was designed to fix a use-after-free warning (which, 
due to -Werror is fatal), but the original code looks correct. I believe 
the gcc warning is bogus.

On a side note, could the -Werror flag be removed by default? Its useful 
for development but not in release versions: See 
https://flameeyes.blog/2009/02/25/future-proof-your-code-dont-use-werror/?utm_source=blogdoteu&utm_medium=referral&utm_campaign=permanent 

[tboot-devel] Patch: Add missing BE size_of_pcrs to hash buffer

[Christopher B. <sal...@gm...>]

First fix attempt was almost correct. System still doesn't boot with 
PCONF element. I forgot 1 important line though. Once added, the system 
now boots with the PCONF element. Tested with both 1 PCR and 2 PCRs.

# HG changeset patch
# User Christopher Byrne <sal...@gm...>
# Date 1645847383 21600
#      Fri Feb 25 21:49:43 2022 -0600
# Node ID ada51f373f12ee91feb2526220b6a94db92319ac
# Parent  6e831184f480ed34154e12eddc2d037d6a417a5f
lcptools-v2/pconf_legacy.c: Add missing BE size_of_pcrs to hash buffer

Signed-of-by: Christopher Byrne <sal...@gm...>

diff --git a/lcptools-v2/pconf_legacy.c b/lcptools-v2/pconf_legacy.c
--- a/lcptools-v2/pconf_legacy.c
+++ b/lcptools-v2/pconf_legacy.c
@@ -241,6 +241,7 @@
          ERROR("Error: failed to allocate buffer for composite digest.\n");
          return false;
      }
+    buff->size_of_pcrs = htonl(no_of_pcrs * SHA1_DIGEST_SIZE);
      memcpy_s(
          &buff->pcr_selection,
          sizeof buff->pcr_selection,

Re: [tboot-devel] suspend problem since kernel 5.15

[Derek D. <z2...@po...>]

On 2/1/22 5:25 AM, Łukasz Hawryłko wrote:
> Hi Derek
> 
> On Mon, 2022-01-31 at 17:26 +0000, Derek Dolney wrote:
>> I am using tboot 1.10.3 and all was working fine with Linux kernel
>> 5.10.88. However, I upgraded to kernel 5.15.16 and, while booting seems
>> to happen properly, suspend is broken. I am using a Lenovo T460p.
>> Usually when suspending the power button LED will blink 8 times and then
>> it goes into a sleep state. With the newer kernel I get power LED and
>> caps lock LED blinking, cpu fan runs fast, and can't get out of that
>> state. Need to hard powerdown.
>>
>> Attaching a txt-stat output. Any ideas what may be happening? Seems like
>> I maybe need to report to the kernel devs, but let me know if you have
>> some other suggestions. I could do a git bisect of the kernel source and
>> probably find the kernel code changes that broke suspend for me. Please
>> advise
> 
> Without serial console, I guess that as you have a laptop you don't
> have RS232 port, it will not be an easy task to debug the issue.
> 
> During S3 suspend Linux kernel is jumping to tboot's shutdown entry
> point to seal RAM content. You can try to comment out this behavior in
> Linux kernel to see if still you can see the issue. This experiment may
> tell us if hang is related to tboot's shutdown handler or not.
> 
> Lukasz
> 
Lukasz, thanks for the ideas. I tried commenting these 2 lines in
tboot_late_init():

// acpi_os_set_prepare_sleep(&tboot_sleep);
// acpi_os_set_prepare_extended_sleep(&tboot_extended_sleep);

Not entirely sure but my hope was that this would keep the kernel out of
tboot's shutdown handler. I don't know this code, please let me know if
you had something else in mind.

Anyway, it still hangs with these lines commented out. However, if I
boot this kernel (or the kernel with these lines not commented out)
without tboot, I can suspend and resume just fine.

Re: [tboot-devel] suspend problem since kernel 5.15

[Łukasz H. <lu...@ha...>]

Hi Derek

On Mon, 2022-01-31 at 17:26 +0000, Derek Dolney wrote:
> I am using tboot 1.10.3 and all was working fine with Linux kernel
> 5.10.88. However, I upgraded to kernel 5.15.16 and, while booting seems
> to happen properly, suspend is broken. I am using a Lenovo T460p.
> Usually when suspending the power button LED will blink 8 times and then
> it goes into a sleep state. With the newer kernel I get power LED and
> caps lock LED blinking, cpu fan runs fast, and can't get out of that
> state. Need to hard powerdown.
> 
> Attaching a txt-stat output. Any ideas what may be happening? Seems like
> I maybe need to report to the kernel devs, but let me know if you have
> some other suggestions. I could do a git bisect of the kernel source and
> probably find the kernel code changes that broke suspend for me. Please
> advise

Without serial console, I guess that as you have a laptop you don't
have RS232 port, it will not be an easy task to debug the issue.

During S3 suspend Linux kernel is jumping to tboot's shutdown entry
point to seal RAM content. You can try to comment out this behavior in
Linux kernel to see if still you can see the issue. This experiment may
tell us if hang is related to tboot's shutdown handler or not.

Lukasz

[tboot-devel] suspend problem since kernel 5.15

[Derek D. <z2...@po...>]

I am using tboot 1.10.3 and all was working fine with Linux kernel
5.10.88. However, I upgraded to kernel 5.15.16 and, while booting seems
to happen properly, suspend is broken. I am using a Lenovo T460p.
Usually when suspending the power button LED will blink 8 times and then
it goes into a sleep state. With the newer kernel I get power LED and
caps lock LED blinking, cpu fan runs fast, and can't get out of that
state. Need to hard powerdown.

Attaching a txt-stat output. Any ideas what may be happening? Seems like
I maybe need to report to the kernel devs, but let me know if you have
some other suggestions. I could do a git bisect of the kernel source and
probably find the kernel code changes that broke suspend for me. Please
advise

Thank you,
Derek

[tboot-devel] [PATCH] GCC update caused build error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow]

[Jun M. <jun...@in...>]

From: "Jun.Miao" <jun...@in...>

Add a CFLAGS "-Wo-stringop-overflow" to void build error in the fedora:36 by the
gcc -O2 optimization, which`s version: gcc (GCC) 12.0.1 20220118 (Red Hat 12.0.1-0).

build error log:
common/memlog.c: In function ‘memlog_init’:
common/memlog.c:53:20: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=]
   53 |        g_log->uuid = (uuid_t)TBOOT_LOG_UUID;
common/memlog.c:53:20: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=]
common/memlog.c:53:20: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=]
common/memlog.c:53:20: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=]
common/memlog.c:53:20: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=]
common/memlog.c:53:20: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=]
common/memlog.c:68:26: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=]
   68 |         g_log->zip_count = 0;
      |         ~~~~~~~~~~~~~~~~~^~~

Signed-off-by: Jun.Miao <jun...@in...>
---
 Config.mk | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Config.mk b/Config.mk
index ed0aef0..b9342bc 100644
--- a/Config.mk
+++ b/Config.mk
@@ -20,6 +20,7 @@ include $(ROOTDIR)/Config.mk
 CFLAGS		:= $(shell echo $(CFLAGS) | sed -e s/-m64/-m32/)
 CFLAGS		+= -march=i686
 CFLAGS		+= -nostdinc
+CFLAGS         += -Wno-stringop-overflow
 CFLAGS		+= -fno-builtin -fno-common -fno-strict-aliasing
 CFLAGS		+= -fomit-frame-pointer
 CFLAGS		+= -pipe
-- 
2.35.0.rc2

[tboot-devel] [Pull Request] tboot/Config.mk: Add a CFLAGS "-Wo-stringop-overflow" to void build error

[Jun M. <jun...@in...>]

Summary: gcc update caused build error: 
	 writing 1 byte into a region of size 0 [-Werror=stringop-overflow]


Hi, Pawel

	Could you help me review this patch to fix this build error in fedora 36?
	Branch：tboot-1.10.3.tar.gpg

Thanks 
Jun.Miao

[tboot-devel] [BUG Report] GCC error: writing 1 byte into a region of size 0

[Jun M. <jun...@in...>]

*build log:*

common/memlog.c: In function 'memlog_init':
common/memlog.c:53:20: error: writing 1 byte into a region of size 0 
[-Werror=stringop-overflow=] 53 | g_log->uuid = (uuid_t)TBOOT_LOG_UUID;
common/memlog.c:53:20: error: writing 1 byte into a region of size 0 
[-Werror=stringop-overflow=]
common/memlog.c:53:20: error: writing 1 byte into a region of size 0 
[-Werror=stringop-overflow=]
common/memlog.c:53:20: error: writing 1 byte into a region of size 0 
[-Werror=stringop-overflow=]
common/memlog.c:53:20: error: writing 1 byte into a region of size 0 
[-Werror=stringop-overflow=]
common/memlog.c:53:20: error: writing 1 byte into a region of size 0 
[-Werror=stringop-overflow=]
common/memlog.c:68:26: error: writing 1 byte into a region of size 0 
[-Werror=stringop-overflow=] 68 | g_log->zip_count = 0;


build error link: 
https://kojipkgs.fedoraproject.org/work/tasks/2525/81812525/build.log

*environment**:*

jmiao@fedora35:~$ cat /etc/os-release
NAME="Fedora Linux"
VERSION="36 (Workstation Edition Prerelease)"
ID=fedora
VERSION_ID=36
VERSION_CODENAME=""
PLATFORM_ID="platform:f36"
PRETTY_NAME="Fedora Linux 36 (Workstation Edition Prerelease)"
ANSI_COLOR="0;38;2;60;110;180"
LOGO=fedora-logo-icon
CPE_NAME="cpe:/o:fedoraproject:fedora:36"
HOME_URL="https://fedoraproject.org/"
DOCUMENTATION_URL="https://docs.fedoraproject.org/en-US/fedora/rawhide/system-administrators-guide/"
SUPPORT_URL="https://ask.fedoraproject.org/"
BUG_REPORT_URL="https://bugzilla.redhat.com/"
REDHAT_BUGZILLA_PRODUCT="Fedora"
REDHAT_BUGZILLA_PRODUCT_VERSION=rawhide
REDHAT_SUPPORT_PRODUCT="Fedora"
REDHAT_SUPPORT_PRODUCT_VERSION=rawhide
PRIVACY_POLICY_URL="https://fedoraproject.org/wiki/Legal:PrivacyPolicy"
VARIANT="Workstation Edition"
VARIANT_ID=workstation
jmiao@fedora35:~$ gcc --version
gcc (GCC) 12.0.1 20220118 (Red Hat 12.0.1-0)
Copyright (C) 2022 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

[tboot-devel] Fixes for legacy PCONF element

[Christopher B. <sal...@gm...>]

I have an TPM 1.2 and Ivy Bridge processor that supports TXT, however 
the new tools (lcptools-v2) do not produce the a legacy PCONF element 
that will boot like the old lcptools did (the other elements seems to 
work fine).

I created some patches, and with tboot finally starts:

# HG changeset patch
# User Christopher Byrne <sal...@gm...>
# Date 1641179189 21600
#      Sun Jan 02 21:06:29 2022 -0600
# Node ID 58b9fd5375215c662b7ba9db0155872c5196d2b4
# Parent  212289468e2d8ba2bf5f4d4b869f1a2f42262f18
Fix endianness of pcr_info->pcr_selection.size_of_select

This field is big endian, and the wrong endianness causes a 0xc0111c61
error (field wrong size)

Signed-off-by: Christopher Byrne <sal...@gm...>

diff -r 212289468e2d -r 58b9fd537521 lcptools-v2/pconf_legacy.c
--- a/lcptools-v2/pconf_legacy.c	Thu Dec 16 12:26:24 2021 +0100
+++ b/lcptools-v2/pconf_legacy.c	Sun Jan 02 21:06:29 2022 -0600
@@ -324,7 +324,7 @@
              return false;
          }
          pcr_info->locality_at_release = locality;
-        pcr_info->pcr_selection.size_of_select = 1;
+        pcr_info->pcr_selection.size_of_select = htons(1);
          pcr_info->pcr_selection.pcr_select = pcr_select;
          memcpy_s((void *)&pcr_info->digest_at_release, SHA1_DIGEST_SIZE,
                                  (const void *)&digest->sha1, 
SHA1_DIGEST_SIZE);
@@ -359,7 +359,7 @@
      for (int i = 0; i < pconf->num_pcr_infos; i++) {
          DISPLAY("%sPCRInfos[%d]\n", prefix, i);
          DISPLAY("%s%sTPM_PCR_SELECTION.sizeOfSelect: 0x%x\n", prefix,
-                                prefix, 
pcr_info->pcr_selection.size_of_select);
+                                prefix, 
ntohs(pcr_info->pcr_selection.size_of_select));
          DISPLAY("%s%sTPM_PCR_SELECTION.pcrSelect: 0x%x\n", prefix, prefix,
 
pcr_info->pcr_selection.pcr_select);
          DISPLAY("%s%s:PCR-0:PCR-1:PCR-2:PCR-3:PCR-4:PCR-5:PCR-6:PCR-7:\n",
# HG changeset patch
# User Christopher Byrne <sal...@gm...>
# Date 1641179723 21600
#      Sun Jan 02 21:15:23 2022 -0600
# Node ID 39ee48b3220fe67cf442c24c5db1397a6e7bc2b8
# Parent  58b9fd5375215c662b7ba9db0155872c5196d2b4
Don't ignore locality in PCR file

The locality was always set to 0x1f no matter what was in the PCR file
because the value read from the file was never written to to the global
variable, which seems to mostly unused anyway.

Signed-off-by: Christopher Byrne <sal...@gm...>

diff -r 58b9fd537521 -r 39ee48b3220f lcptools-v2/pconf_legacy.c
--- a/lcptools-v2/pconf_legacy.c	Sun Jan 02 21:06:29 2022 -0600
+++ b/lcptools-v2/pconf_legacy.c	Sun Jan 02 21:15:23 2022 -0600
@@ -68,7 +68,6 @@
  } pcr_data;

  //Global vars:
-uint8_t locality = DEFAULT_LOCALITY_SELECT;
  char pcr_info_files[MAX_FILES][MAX_PATH];
  uint8_t num_files = 0;
  int prevOpt = 'i';
@@ -169,6 +168,7 @@
          if (this_pcr.locality != 0xFF && this_pcr.num != 0xFF) {
              this_pcr.valid = true;
              pcrs[this_pcr.num] = this_pcr;
+	    pcrs[0].locality = locality;
          }
          else {
              ERROR("Error: failed to read PCR data. Check input file.\n");
@@ -323,7 +323,7 @@
              free(elt);
              return false;
          }
-        pcr_info->locality_at_release = locality;
+        pcr_info->locality_at_release = pcrs[0].locality;
          pcr_info->pcr_selection.size_of_select = htons(1);
          pcr_info->pcr_selection.pcr_select = pcr_select;
          memcpy_s((void *)&pcr_info->digest_at_release, SHA1_DIGEST_SIZE,
# HG changeset patch
# User Christopher Byrne <sal...@gm...>
# Date 1641194772 21600
#      Mon Jan 03 01:26:12 2022 -0600
# Node ID 78997bf8e6fb9ace37e40a188a47f8a0aaada5b0
# Parent  39ee48b3220fe67cf442c24c5db1397a6e7bc2b8
Fix composite hasing algorithm for pconf element to match lcptools-1

lcptools-v2 implements the pconf composite hashing algorithm described
in Intel® Trusted Execution Technology (Intel® TXT): Software Development
Guide (01/05/2021). The problem is doesn't work. TXT aborts wtih an error
"No match is found for Element." or "PCR info integrity failure".
lcptools (v1) did work, but hashed a different structure.

The structure to actually be hashed is:

typedef struct __packed {
    tpm_pcr_selection	   pcr_selection;
    uint32_t                size_of_pcrs; // Big endian
    unsigned char           pcrs[][SHA1_DIGEST_SIZE];
} pcr_composite_buffer;

Rather then just "pcrs" (which is what is described and what lcptools-v2
currently does.

Signed-off-by: Christopher Byrne <sal...@gm...>

diff -r 39ee48b3220f -r 78997bf8e6fb lcptools-v2/pconf_legacy.c
--- a/lcptools-v2/pconf_legacy.c	Sun Jan 02 21:15:23 2022 -0600
+++ b/lcptools-v2/pconf_legacy.c	Mon Jan 03 01:26:12 2022 -0600
@@ -67,6 +67,12 @@
      uint8_t digest[SHA1_DIGEST_SIZE];
  } pcr_data;

+typedef struct __packed {
+   tpm_pcr_selection	   pcr_selection;
+   uint32_t                size_of_pcrs; // Big endian
+   unsigned char           pcrs[][SHA1_DIGEST_SIZE];
+} pcr_composite_buffer;
+
  //Global vars:
  char pcr_info_files[MAX_FILES][MAX_PATH];
  uint8_t num_files = 0;
@@ -207,19 +213,20 @@
      }
  }

-static bool generate_composite_hash(pcr_data *pcrs, tb_hash_t *dest, 
uint8_t no_of_pcrs)
+static bool generate_composite_hash(tpm_pcr_selection *pcr_selection, 
pcr_data *pcrs, tb_hash_t *dest, uint8_t no_of_pcrs)
  {
      /*
      This function: concatenates pcr values to one blob and hashes it 
using sha1

-    in: array of pcrs, allocated destination buffer, no of pcrs
+    in: pcr selection, array of pcrs, allocated destination buffer, no 
of pcrs
      out: true/false on success/failure. Dest gets the composite hash

      */
      int count = 0;
      bool result;
-    size_t buff_size;
-    unsigned char *buff;
+    pcr_composite_buffer *buff;
+    size_t buff_size = 0;
+
      if (pcrs == NULL || dest == NULL) {
          ERROR("Error: pcrs or buffer for digest are not defined.\n");
          return false;
@@ -228,12 +235,20 @@
          ERROR("Error: at least 1 and at most 8 pcrs must be selected.\n");
          return false;
      }
-    buff_size = no_of_pcrs * SHA1_DIGEST_SIZE;
+    buff_size = no_of_pcrs * SHA1_DIGEST_SIZE + sizeof(buff) -
+		sizeof(buff->pcrs[0][0]);
      buff = calloc(1, buff_size);
      if (buff == NULL) {
          ERROR("Error: failed to allocate buffer for composite digest.\n");
          return false;
      }
+    memcpy_s(
+	&buff->pcr_selection,
+	sizeof buff->pcr_selection,
+	pcr_selection,
+	sizeof buff->pcr_selection
+	);
+    buff->size_of_pcrs = htonl(no_of_pcrs * SHA1_DIGEST_SIZE);
      for (int i = 0; i < MAX_PCRS; i++) {
          if (pcrs[i].valid) {
              if (verbose) {
@@ -241,8 +256,8 @@
                  print_hex("", (const void *) pcrs[i].digest, 
SHA1_DIGEST_SIZE);
              }
              memcpy_s(
-                buff + (count * SHA1_DIGEST_SIZE), //Dest
-                buff_size - (count * SHA1_DIGEST_SIZE), //Dest size
+                buff->pcrs[count], //Dest
+                SHA1_DIGEST_SIZE, //Dest size
                  (const void *) pcrs[i].digest, //Src
                  SHA1_DIGEST_SIZE //Src size
              );
@@ -251,7 +266,7 @@
          if (count == no_of_pcrs)
              break;
      }
-    result = hash_buffer(buff, buff_size, dest, LCP_POLHALG_SHA1);
+    result = hash_buffer((unsigned char *)buff, buff_size, dest, 
LCP_POLHALG_SHA1);
      if (verbose) {
          DISPLAY("Composite hash value: ");
          print_hex("", (const void *) dest, SHA1_DIGEST_SIZE);
@@ -316,16 +331,17 @@
              ERROR("Error: failed to allocate memory for digest 
buffer.\n");
              return NULL;
          }
-        result = generate_composite_hash(pcrs, digest, no_of_pcrs);
+        pcr_info->locality_at_release = pcrs[0].locality;
+        pcr_info->pcr_selection.size_of_select = htons(1);
+        pcr_info->pcr_selection.pcr_select = pcr_select;
+        result = generate_composite_hash(&pcr_info->pcr_selection, 
pcrs, digest, no_of_pcrs);
          if (!result) {
              ERROR("Error: failed to generate composite hash.\n");
              free(digest);
              free(elt);
              return false;
          }
-        pcr_info->locality_at_release = pcrs[0].locality;
-        pcr_info->pcr_selection.size_of_select = htons(1);
-        pcr_info->pcr_selection.pcr_select = pcr_select;
+
          memcpy_s((void *)&pcr_info->digest_at_release, SHA1_DIGEST_SIZE,
                                  (const void *)&digest->sha1, 
SHA1_DIGEST_SIZE);
          pcr_info++; //Move to next one