Even if errors occurred, always try to measure all of our Mok entries.
This way we won't fail on e.g. MokList not being set.
Signed-off-by: Peter Jones <pjones@redhat.com>
We're currently measuring the raw second stage loader into PCR 9, but
we're closer to spec if we measure the semi-parsed PE into PCR 4. The
hash that's logged is the same as the hash used for the Authenticode
validation, so refactor shim.c a little to separate out the hash
generation.
It's desirable to be able to use PCR 7 for all TPM policy on Secure Boot
systems, but right now Shim doesn't record any information about its
configuration or the signature used to launch the second stage loader. Add
support for that.
Rob Clark noticed while, implementing a UEFI like backend on u-boot,
that if a File Handle actually returns a meaningful device path from
DevicePathFromHandle(), we wind up with a horribly wrong device path in
the boot variable. He's right, normal UEFI doesn't return that, which
means FileDevicePath() in our code currently does nothing at all.
Instead of all that, pass in the device's handle, and it'll do what
we're doing after the fact there.
Here's the log from a current run:
FS0:\> \efi\BOOT\BOOTX64.EFI
System BootOrder not found. Initializing defaults.
find_boot_options:778:Found directory named "fedora"
try_boot_csv:532:Found file "\EFI\fedora\BOOT.CSV"
try_boot_csv:544:File looks like:
?shim.efi,Fedora,,This is the boot entry for Fedora
populate_stanza:495:CSV data: "shim.efi,Fedora,,This is the boot entry for Fedora"
populate_stanza:501:filename: "shim.efi"
populate_stanza:508:label: "Fedora"
populate_stanza:514:arguments: ""
add_to_boot_list:430:file DP: PciRoot(0)/Pci(0x1F,0x2)/Sata(0x0,0x0,0x0)/HD(Part1,Sig6584272A-D7B9-442A-B8A4-19B5EC4566F4)/\EFI\fedora\shim.efi
FindSubDevicePath:78:input device path: "PciRoot(0)/Pci(0x1F,0x2)/Sata(0x0,0x0,0x0)/HD(Part1,Sig6584272A-D7B9-442A-B8A4-19B5EC4566F4)/\EFI\fedora\shim.efi"
FindSubDevicePath:86:sub-path (4,1): "HD(Part1,Sig6584272A-D7B9-442A-B8A4-19B5EC4566F4)/\EFI\fedora\shim.efi"
add_to_boot_list:452:04 01 2A 00 01 00 00 00 00 08 00 00 00 00 00 00
add_to_boot_list:452:00 40 06 00 00 00 00 00 2A 27 84 65 B9 D7 2A 44
add_to_boot_list:452:B8 A4 19 B5 EC 45 66 F4 02 02 04 04 2E 00 5C 00
add_to_boot_list:452:45 00 46 00 49 00 5C 00 66 00 65 00 64 00 6F 00
add_to_boot_list:452:72 00 61 00 5C 00 73 00 68 00 69 00 6D 00 2E 00
add_to_boot_list:452:65 00 66 00 69 00 00 00 7F FF 04 00
add_to_boot_list:459:device path: "HD(Part1,Sig6584272A-D7B9-442A-B8A4-19B5EC4566F4)/\EFI\fedora\shim.efi"
Creating boot entry "Boot0000" with label "Fedora" for file "\EFI\fedora\shim.efi"
AddOption - Boot0000, then CurrentCount = 0x00000008
update_boot_order:390:nbootorder: 7
BootOrder: 0000 0002 0001 0003 0005 0006 0004
Signed-off-by: Peter Jones <pjones@redhat.com>
This lets you do:
mkdir build-x64 build-ia32
cd build-x64
make TOPDIR=.. -f ../Makefile
cd ../build-ia32
setarch i686 -B make ARCH=ia32 TOPDIR=.. -f ../Makefile
And not worry about generated sources and headers mixing and matching.
Signed-off-by: Peter Jones <pjones@redhat.com>
BOOT.CSV should be placed in fedora directory in order to locate the base
directory of files recorded in $FILENAME column.
Signed-off-by: Lans Zhang <jia.zhang@windriver.com>
When dir->Read() says bs=0, we shouldn't try to allocate a buffer and
read into it. On edk2 this works because there's an implicit (possibly
accidental) minimum size of one pool list entry that can be allocated,
so you wind up getting (I think) 8 bytes.
When Rob Clark tried to run this under uboot's emulated UEFI
environment, dir->Read() returned 0 and when we passed that to
AllocateZeroPool() less good things happened.
So just check for that case and exit appropriately.
Signed-off-by: Peter Jones <pjones@redhat.com>
The TCG EFI Protocol Specification for family "2.0" mentions that not all
TPM2 chips may support the EFI_TCG2_EVENT_LOG_FORMAT_TCG_2 (crypto agile)
log format. So instead of always use this log format, the GetCapability()
function should be used to determine which format is supported by the TPM.
For example, the Intel PTT firmware based TPM found in Lenovo Thinkapd X1
Carbon (4th gen), only supports SHA-1 (EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2)
log format. So a call to GetEventLog() using the crypto agile format was
returning EFI_INVALID_PARAMETER, making tpm_log_event() function to fail.
This was preventing shim to correctly measure the second stage bootloader:
$ tpm2_listpcrs -L 0x04:9
Bank/Algorithm: TPM_ALG_SHA1(0x0004)
PCR_09: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
After passing a supported log format to GetEventLog(), it succeeds and so
shim is able to call the HashLogExtendEvent() EFI function correctly:
$ tpm2_listpcrs -L 0x04:9
Bank/Algorithm: TPM_ALG_SHA1(0x0004)
PCR_09: 07 5a 7e d3 75 64 ad 91 1a 34 17 17 c2 34 10 2b 58 5b de b7
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
The EFI_TCG2_PROTOCOL.GetCapability() function is used to learn if a TPM2
chip is present. But the protocol capability information is also needed
for other reasons, for example to determine what event log formats are
supported by the firmware.
Take out the GetCapability() call from the tpm2_present() logic and reduce
that function to just checking if a TPM2 chip is available or not, so the
capabilities can later be used to determine the supported TPM log formats.
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
When measuring data into the TPM and generating events logs, the event
type is set to EV_IPL (0xd), and for TPM1.2 the algorithm will always
be set to SHA-1 (0x4).
So, add some macro-defined constants for these instead of having them
as magic numbers to make the code more readable.
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
EFI_NOT_FOUND will be returned when creating MokListRT if vendor cert is
empty. This is harmless, meaningless and skippable.
Signed-off-by: Lans Zhang <jia.zhang@windriver.com>
Since 87060b2fc effectively means signing with signtool.exe simply does
not work correctly, and that's sort of the biggest goal for shim, make
this version 12.
Signed-off-by: Peter Jones <pjones@redhat.com>
When I added 4990d3f I inadvertantly made .data.ident and .rela.got
sections appear in the top-level section headers at file offsets not
aligned with PE->OptionalHeader.FileAlignment. This results in a
section table that looks like:
Sections:
Idx Name Size VMA LMA File off Algn
0 .eh_frame 00018648 0000000000005000 0000000000005000 00000400 2**3
CONTENTS, ALLOC, LOAD, READONLY, DATA
1 .text 00093f45 000000000001e000 000000000001e000 00018c00 2**4
CONTENTS, ALLOC, LOAD, READONLY, CODE
2 .reloc 0000000a 00000000000b2000 00000000000b2000 000acc00 2**0
CONTENTS, ALLOC, LOAD, READONLY, DATA
3 .data.ident 000000e4 00000000000b3040 00000000000b3040 000ace40 2**5
CONTENTS, ALLOC, LOAD, DATA
4 .data 000291e8 00000000000b4000 00000000000b4000 000ad200 2**5
CONTENTS, ALLOC, LOAD, DATA
5 .vendor_cert 000003e2 00000000000de000 00000000000de000 000d6400 2**0
CONTENTS, ALLOC, LOAD, READONLY, DATA
6 .dynamic 000000f0 00000000000df000 00000000000df000 000d6800 2**3
CONTENTS, ALLOC, LOAD, DATA
7 .rela 0001aef8 00000000000e0000 00000000000e0000 000d6a00 2**3
CONTENTS, ALLOC, LOAD, READONLY, DATA
8 .rela.got 00000060 00000000000faef8 00000000000faef8 000f1af8 2**3
CONTENTS, ALLOC, LOAD, READONLY, DATA
9 .dynsym 0000ecd0 00000000000fb000 00000000000fb000 000f1e00 2**3
CONTENTS, ALLOC, LOAD, READONLY, DATA
rather than:
Sections:
Idx Name Size VMA LMA File off Algn
0 .eh_frame 00018118 0000000000005000 0000000000005000 00000400 2**3
CONTENTS, ALLOC, LOAD, READONLY, DATA
1 .text 00091898 000000000001e000 000000000001e000 00018600 2**4
CONTENTS, ALLOC, LOAD, READONLY, CODE
2 .reloc 0000000a 00000000000b0000 00000000000b0000 000aa000 2**0
CONTENTS, ALLOC, LOAD, READONLY, DATA
3 .data 00028848 00000000000b1000 00000000000b1000 000aa200 2**5
CONTENTS, ALLOC, LOAD, DATA
4 .vendor_cert 00000449 00000000000da000 00000000000da000 000d2c00 2**0
CONTENTS, ALLOC, LOAD, READONLY, DATA
5 .dynamic 00000100 00000000000db000 00000000000db000 000d3200 2**3
CONTENTS, ALLOC, LOAD, DATA
6 .rela 0001ae50 00000000000dc000 00000000000dc000 000d3400 2**3
CONTENTS, ALLOC, LOAD, READONLY, DATA
7 .dynsym 0000ea78 00000000000f7000 00000000000f7000 000ee400 2**3
CONTENTS, ALLOC, LOAD, READONLY, DATA
(Note "File off" on sections #3 and #8 on the top one.)
This seems to work fine with edk2's loader and shim's loader, as well as
their Authenticode implementation, and pesign's as well.
While PE loaders seem to be fine with sections with alignments smaller
than PE->OptionalHeader.FileAlignment, MS's signtool.exe does ...
something else with them. I'm not sure what. What it definitely does
*not* do is extend the digest based on their file offset and size.
So just don't allow anything that small, and don't allow anything
smaller than SectionAlignment either, just to be on the safe side.
Since most of our stuff gets stripped into the debuginfo anyway, and
shim has relatively few sections, this should not be a very large
burden.
So just to be clear:
If you have a binary with a section that's not aligned on
PE->OptionalHeader.FileAlignment:
- pesign hashes it to A
- tiano hashes it to A
- shim hashes it to A
- signtool.exe hashes it to B
Because that makes sense.
This patch works around the bug in signtool.exe .
Signed-off-by: Peter Jones <pjones@redhat.com>
CryptPem only provides one function: RsaGetPrivateKeyFromPem(). Since we
don't need to retrieve any private key, it's safe to disable the
function.
Signed-off-by: Gary Lin <glin@suse.com>
Disable DES completely since it's already old and insecure.
This makes MokManager not support the DES based password hash but
probably no one is using it.
Signed-off-by: Gary Lin <glin@suse.com>
strcmp() and strcasecmp() are widely used in openssl. Implement those
two functions to eliminate the gcc warnings and the potential crash.
Signed-off-by: Gary Lin <glin@suse.com>
- Declare some functions in the proper headers
+ We missed them for a long time...
- Cast offsetof to UINTN
+ The original casting triggers the gcc warning since int can not
present the offset for the 64bit machines.
- Cast the "char" array to "CHAR8 *" to avoid the gcc warnings
- Implement atoi correctly
Signed-off-by: Gary Lin <glin@suse.com>
The changes in the openssl headers cause the inclusion of
CrtLibSupport.h eariler than the inclusion of stddef.h, so "offsetof"
was defined twice and this caused the followling build error:
In file included from Cryptlib/Include/openssl/buffer.h:23:0,
from Cryptlib/Include/openssl/x509.h:22,
from shim.c:56:
/usr/lib64/gcc/x86_64-suse-linux/6/include/stddef.h:417:0: error: "offsetof" redefined [-Werror]
#define offsetof(TYPE, MEMBER) __builtin_offsetof (TYPE, MEMBER)
In file included from Cryptlib/Include/limits.h:15:0,
from Cryptlib/Include/openssl/ossl_typ.h:13,
from Cryptlib/Include/openssl/x509.h:20,
from shim.c:56:
Cryptlib/Include/CrtLibSupport.h:192:0: note: this is the location of the previous definition
#define offsetof(type, member) ( (int) & ((type*)0) -> member )
We can lower the priority of the gcc include path or just remove the
path, but this might cause problem since the path was introduced on
purpose(*). Instead, including stddef.h first is more feasible.
(*) d51739a416
Signed-off-by: Gary Lin <glin@suse.com>
- Delete the old openssl files and use the script to copy the new files
- Add "-DNO_SYSLOG" to CFLAGS and add crypto/include to the include path
Signed-off-by: Gary Lin <glin@suse.com>
- Update update.sh to copy the openssl 1.1.0 source files
- Refresh the supplemental patch to reflect the change
Signed-off-by: Gary Lin <glin@suse.com>
- Update to edk2 commit 7c410b3d4180087020c7734bf67cdc4ad9fdb136
CryptoPkg/BaseCryptLib: Adding NULL checking in time() wrapper.
- Update headers in Cryptlib/Include/openssl/ to 1.1.0e
+ Also copy the openssl internal headers
Signed-off-by: Gary Lin <glin@suse.com>
- Remove the openssl version from update.sh since edk2 doesn't use the
version number in the directory name anymore.
- Refresh Cryptlib.diff to reflect the change
Signed-off-by: Gary Lin <glin@suse.com>
Edk2 renamed OpenSslSupport.h, so we have to follow the change.
Also merge some changes from edk2 CrtLibSupport.h
Signed-off-by: Gary Lin <glin@suse.com>
The commit 03b9f800 introduces an issue in case the gap between
SumOfBytesHashed and context->SecDir->VirtualAddress exists.
This would be a typo because a formal PE image always meet
SumOfBytesHashed + hashsize == context->SecDir->VirtualAddress either
the gap exists or not.
Signed-off-by: Lans Zhang <jia.zhang@windriver.com>
Update to edk2 commit 6e4489d8129d233ef0fe85eeb6eebfecafe9ea6e
(CryptoPkg: Refine type cast for pointer subtraction)
Also replaced CryptAes.c, CryptArc4.c, CryptTdes.c, CryptMd4.c,
CryptHmacMd5.c, and CryptHmacSha1.c with the Null version since
we don't really need those functions.
Signed-off-by: Gary Lin <glin@suse.com>
Under a strict memory protection policy, UEFI may give out EfiLoaderData
memory with the XN attribute set. So use EfiLoaderCode explicitly.
At the same time, use a page based allocation rather than a pool
allocation, which is more appropriate when loading PE/COFF images.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
This makes it so two builds of the same .deb on different hosts won't
have wildly different file offsets.
Signed-off-by: Peter Jones <pjones@redhat.com>