oracle-shim-15-2.0.3.el8-x86_64

[AlexeyPetrenkoOracle]

Make sure you have provided the following information:

• link to your code branch cloned from rhboot/shim-review in the form user/repo@tag
• completed README.md file with the necessary information
• shim.efi to be signed
• public portion of your certificate embedded in shim (the file passed to VENDOR_CERT_FILE)
• any extra patches to shim via your own git tree or as files
• any extra patches to grub via your own git tree or as files
• build logs

What organization or people are asking to have this signed:

Oracle Corporation

What product or service is this for:

Oracle Linux
https://www.oracle.com/linux/index.html

What is the origin and full version number of your shim?

We used upstream shim version 15
https://github.com/rhboot/shim/releases/tag/15

What's the justification that this really does need to be signed for the whole world to be able to boot it:

Oracle Linux is a popular enterprise Linux distribution with Secure Boot support.

How do you manage and protect the keys used in your SHIM?

The key is installed in the server with restricted physical and system access

Do you use EV certificates as embedded certificates in the SHIM?

Yes

What is the origin and full version number of your bootloader (GRUB or other)?

grub2 - upstream plus number of patches from RedHat and Oracle

If your SHIM launches any other components, please provide further details on what is launched

[your text here]

How do the launched components prevent execution of unauthenticated code?

grub verifies signatures on booted kernels

Does your SHIM load any loaders that support loading unsigned kernels (e.g. GRUB)?

No

What kernel are you using? Which patches does it includes to enforce Secure Boot?

4.18 based kernel with lockdown patches

What changes were made since your SHIM was last signed?

This is the first shim build for Oracle Linux 8

What is the hash of your final SHIM binary?

a9aa90daa9da19332910442b739fa6f2ab6564f0afdfc2eedc167ce219d59f98 shimia32.efi
affdd0fe76153b0623e0cd7e044997f073b7232754cf808a068fd245762b3714 shimx64.efi

Public certificate:
-----BEGIN CERTIFICATE-----
MIIF2zCCBMOgAwIBAgIQA+YFGHHEE60dWTdlqxCjpDANBgkqhkiG9w0BAQsFADBs
MQswCQYDVQQGEwJVUzEVMBMGA1UEChMMRGlnaUNlcnQgSW5jMRkwFwYDVQQLExB3
d3cuZGlnaWNlcnQuY29tMSswKQYDVQQDEyJEaWdpQ2VydCBFViBDb2RlIFNpZ25p
bmcgQ0EgKFNIQTIpMB4XDTE4MTAwODAwMDAwMFoXDTIxMTIyMjEyMDAwMFowgfEx
EzARBgsrBgEEAYI3PAIBAxMCVVMxGTAXBgsrBgEEAYI3PAIBAhMIRGVsYXdhcmUx
HTAbBgNVBA8MFFByaXZhdGUgT3JnYW5pemF0aW9uMRAwDgYDVQQFEwc0MDI4MTI1
MQswCQYDVQQGEwJVUzETMBEGA1UECBMKQ2FsaWZvcm5pYTEVMBMGA1UEBxMMUmVk
d29vZCBDaXR5MRswGQYDVQQKExJPcmFjbGUgQ29ycG9yYXRpb24xGzAZBgNVBAsT
Ek9yYWNsZSBDb3Jwb3JhdGlvbjEbMBkGA1UEAxMST3JhY2xlIENvcnBvcmF0aW9u
MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAkvcd/qAEUeN9JpqzSGuh
7Azuy9FN2+ZwOZ0/WqWGBOxnDbVAKDIAxMT/XdezrtRGzGHwEDrKU3Wo1SOsZeNa
DXCy4mWxiXwYj4rvoDbwpq+uqFBs3fies1/YpHkx8SsOhuDGFdmLaXriYh8QyNHy
gqYUeaYNAcN4h7Cpxzthp3ER6Xx9giI597Ee9UArXzgmPH54UbJs2+8xflz9M7n4
GUhf9cTMmd1StSM0gJ6JmyTtqjl/QjCTFho6lN1MeWxKPBcUBP3ThQi3cEqvGixc
SwQ2ILbg3a+qXGa3EozK7FlZFf+bnRVu8wBECBh6oR+3P0TTeKWh2hZ9Tmjbxe1G
nwIDAQABo4IB8TCCAe0wHwYDVR0jBBgwFoAUj+h+8G0yagAFI8dwl2o6kP9r6tQw
HQYDVR0OBBYEFH9Vm46dngUYNICnFJ5aHeC8JlraMC4GA1UdEQQnMCWgIwYIKwYB
BQUHCAOgFzAVDBNVUy1ERUxBV0FSRS00MDI4MTI1MA4GA1UdDwEB/wQEAwIHgDAT
BgNVHSUEDDAKBggrBgEFBQcDAzB7BgNVHR8EdDByMDegNaAzhjFodHRwOi8vY3Js
My5kaWdpY2VydC5jb20vRVZDb2RlU2lnbmluZ1NIQTItZzEuY3JsMDegNaAzhjFo
dHRwOi8vY3JsNC5kaWdpY2VydC5jb20vRVZDb2RlU2lnbmluZ1NIQTItZzEuY3Js
MEsGA1UdIAREMEIwNwYJYIZIAYb9bAMCMCowKAYIKwYBBQUHAgEWHGh0dHBzOi8v
d3d3LmRpZ2ljZXJ0LmNvbS9DUFMwBwYFZ4EMAQMwfgYIKwYBBQUHAQEEcjBwMCQG
CCsGAQUFBzABhhhodHRwOi8vb2NzcC5kaWdpY2VydC5jb20wSAYIKwYBBQUHMAKG
PGh0dHA6Ly9jYWNlcnRzLmRpZ2ljZXJ0LmNvbS9EaWdpQ2VydEVWQ29kZVNpZ25p
bmdDQS1TSEEyLmNydDAMBgNVHRMBAf8EAjAAMA0GCSqGSIb3DQEBCwUAA4IBAQAv
yMvkUcvctEwi+tJSviZ9TpvhI/pQvNpRoMyPkI3zlNWb6b+luJ0KLr3vgJRIk7un
u37gYJ6AfBnPrXmGyJykp13+qE5Xr/LBkvnpwIxxwjELLttRIi4u5aG7N/shIRcM
ASORa02c8TO1SeoxwjXcuTveGDJE4hmCMD19FMUKMf5bZJNFLyhl+XavG+mAAXZ2
YIp69dO8Q1CCIXzg6MJ45j7nSaXf6akAX2bm0IfJpXhUwfHM0rUGCyQI2TQwpp+/
8eg8hkganUOOPhZA1V5MVg8jFME9WnL08Zyrf7fH7M9rTOR+Y88JOtogmb698P3g
aE3h/+nD3ZP72Yj5AXhK
-----END CERTIFICATE-----

Link to shim tag: https://github.com/AlexeyPetrenkoOracle/shim-review/releases/tag/oracle-shim-15-2.0.3.el8-x86_64-20191112

[AlexeyPetrenkoOracle]

Is there any ETA for this review?

[cyphermox]

Sorry, I will start building a system with Oracle Linux to reproduce the build.

[AlexeyPetrenkoOracle]

It's over three months now.
Is there any ETA?

[cyphermox]

I have no interest in creating yet one more account somewhere to download an ISO. Is there any other way we can reproduce the build? Using Docker perhaps?

[AlexeyPetrenkoOracle]

Yes, OL8 images are available on docker hub.

[AlexeyPetrenkoOracle]

It's four months since I've filed this review request and the shim still was not reviewed.
It doesn't make any sense to wait for the review of the original shim any longer. It's already outdated.

I've updated this request with the new shim version for OL8.

This process is clearly dysfunctional.

[cyphermox]

So, how do I go about reproducing this build? I'm not very familiar with RPM-based distros. Any steps you can take to make this easier is obviously going to help having things reviewed faster. Could you please provide a Dockerfile like other distros have done that wraps around the right calls to mimick what you do to build your shim on your infrastructure?

[AlexeyPetrenkoOracle]

You can use the following Dockerfile:

FROM oraclelinux:8

RUN dnf -y update
RUN dnf -y install wget rpm-build gcc gcc-c++ make
RUN wget https://oss.oracle.com/ol8/shim/15-2.0.2.el8/shim-unsigned-x64-15-2.0.2.el8.src.rpm
RUN rpm -ivh shim-unsigned-x64-15-2.0.2.el8.src.rpm
RUN dnf -y --enablerepo="*" builddep /root/rpmbuild/SPECS/shim-unsigned-x64.spec
RUN rpmbuild -bb /root/rpmbuild/SPECS/shim-unsigned-x64.spec

[AlexeyPetrenkoOracle]

It's five months since this review request was submitted without any progress.
This process is clearly dysfunctional and puts all Linux distributions at a serious security risk.

[cyphermox]

The binaries built are vastly different, in .text sections and elsewhere. Are you sure the provided rules, and the available build environment for an oraclelinux:8 container are indeed what was used to build the binary?

I've used the Dockerfile you provided, supplemented it with additional command to validate the files:

FROM oraclelinux:8
  
RUN dnf -y update
RUN dnf -y install wget rpm-build gcc gcc-c++ make
RUN wget https://oss.oracle.com/ol8/shim/15-2.0.2.el8/shim-unsigned-x64-15-2.0.2.el8.src.rpm
RUN rpm -ivh shim-unsigned-x64-15-2.0.2.el8.src.rpm
RUN dnf -y --enablerepo="*" builddep /root/rpmbuild/SPECS/shim-unsigned-x64.spec
RUN rpmbuild -bb /root/rpmbuild/SPECS/shim-unsigned-x64.spec
COPY shimia32.efi /
COPY shimx64.efi /
RUN ls -l /
RUN ls -l /root/rpmbuild/RPMS/x86_64
RUN rpm2cpio /root/rpmbuild/RPMS/x86_64/shim-unsigned-ia32-15-2.0.2.el8.x86_64.rpm | cpio -idmv
RUN rpm2cpio /root/rpmbuild/RPMS/x86_64/shim-unsigned-x64-15-2.0.2.el8.x86_64.rpm | cpio -idmv
RUN hexdump -Cv ./usr/share/shim/15-2.0.2.el8/x64/shimx64.efi > built-x64.hex
RUN hexdump -Cv ./usr/share/shim/15-2.0.2.el8/ia32/shimia32.efi > built-x32.hex
RUN hexdump -Cv /shimia32.efi > orig-x32.hex
RUN hexdump -Cv /shimx64.efi > orig-x64.hex
RUN sha256sum /usr/share/shim/15-2.0.2.el8/x64/shimx64.efi /shimx64.efi
RUN sha256sum /usr/share/shim/15-2.0.2.el8/ia32/shimia32.efi /shimia32.efi
#RUN diff -u orig-x32.hex built-x32.hex
#RUN diff -u orig-x64.hex built-x64.hex
RUN objdump -h /usr/share/shim/15-2.0.2.el8/x64/shimx64.efi
RUN objdump -h /usr/share/shim/15-2.0.2.el8/ia32/shimia32.efi

I can't continue; we need to find out where the build environment (or the build process in general) differs to generate such different binaries.

[AlexeyPetrenkoOracle]

We had to update our build system to satisfy this new requirement of the build being reproduced in Docker container and the new shim binary is available here: https://github.com/AlexeyPetrenkoOracle/shim-review/releases/tag/oracle-shim-15-2.0.3.el8-x86_64-20191112

[AlexeyPetrenkoOracle]

Hi. Any updates or ETA?
This request is open for over half a year now!

[YustasSwamp]

Hi @AlexeyPetrenkoOracle ,
I'm happy to cross-review it. But the build seems not reproducible.
Here is my advise. Try to avoid installing latest available toolchain instead of exact version. For example, dnf -y install gcc-7.3.0-1.el8 looks more reliable to get reproducible build over the time.
Link to the container/VM image with preinstalled toolchains is ideal.
Thanks.

[AlexeyPetrenkoOracle]

This request is open for eight (!) months already. And we already made a bunch of adjustments and provided a bunch of additional information.
And there is still no review from maintainers.

We have provided a fully reproducible build over two months ago. And there is still no review from maintainers.

There is no surprise that during these two months we've released a number of package updates and this build is not reproducible anymore on the latest version. Sure, we'll update the docker file with the specific version to reproduce this build.

But this process is completely dysfunctional. It doesn't make any sense to review security-critical components for over eight months.

[jsegitz]

yes, it's problematic, as discussed in #84
@YustasSwamp started to cross-review and I'm also jumping in. So if you update your build instructions it should be done this week

[AlexeyPetrenkoOracle]

Hi.
Here is the updated Docker file and dependency list, which allows this docker file to build precisely the shim in the review.

Thanks.

Docker_upd.tar.gz

[jsegitz]

I can confirm that the shim binaries result from shim-15 sources plus
0001-Make-sure-that-MOK-variables-always-get-mirrored.patch
0002-mok-fix-the-mirroring-of-RT-variables.patch
0003-mok-consolidate-mirroring-code-in-a-helper-instead-o.patch
0004-Make-VLogError-behave-as-expected.patch
0005-MokListRT-Fatal.patch
1006-Once-again-try-even-harder-to-get-binaries-without-t.patch
1007-Remove-call-to-TPM2-get_event_log.patch
1008-Add-grub2-PCR-usage-to_README_TPM.patch
1009-shim-Properly-generate-absolute-paths-from-relative-.patch
The patches look okay to me and the build results in the binaries as specified:
a9aa90daa9da19332910442b739fa6f2ab6564f0afdfc2eedc167ce219d59f98 shimia32.efi
affdd0fe76153b0623e0cd7e044997f073b7232754cf808a068fd245762b3714 shimx64.efi

A note on your review request: For the questions

• What is the origin and full version number of your bootloader (GRUB or other)?
• What kernel are you using? Which patches does it includes to enforce Secure Boot?

please list the patches you applied. Not relevant for me, since I only verify that the binary in fact results from the official sources + a (hopefully small) collection of patches, but it might be relevant for Microsoft when deciding if they'll sign your shim

[iokomin]

@cyphermox , reviewers - is there anything else needed on this review request from our side to get "accepted" label?
Cross review passed, please let me know if anything else is required to provide.
Note: This request provides same amount of information which was enough for successful #74 signing by MSFT.

[xnox]

@jsegitz @iokomin

Hi, I'm not a shim-reviewer.

When I compare el7 #74 submission with this el8 submission, I see that this one drops a patch included in the el7 submission.

This el8 submission, however has the bugfix for the relative paths.

It kind of means that el7 has ARM discardable sections bugfix, and el8 has relative path bugfix. But neither have both.

Diff of the patched el7 tree vs patched el8 tree

$ diff -ru el7/shim-15/shim.c el8/shim-15/shim.c 
--- el7/shim-15/shim.c	2020-05-29 16:21:12.955092506 +0100
+++ el8/shim-15/shim.c	2020-05-29 16:21:45.499402201 +0100
@@ -1341,11 +1341,6 @@
 	 */
 	Section = context.FirstSection;
 	for (i = 0; i < context.NumberOfSections; i++, Section++) {
-		/* Don't try to copy discardable sections with zero size */
-		if ((Section->Characteristics & EFI_IMAGE_SCN_MEM_DISCARDABLE) &&
-		    !Section->Misc.VirtualSize)
-			continue;
-
 		base = ImageAddress (buffer, context.ImageSize,
 				     Section->VirtualAddress);
 		end = ImageAddress (buffer, context.ImageSize,
@@ -1609,9 +1604,11 @@
 		bootpath[j] = '\0';
 	}
 
-	while (*ImagePath == '\\')
-		ImagePath++;
+	for (i = 0, last = 0; i < StrLen(ImagePath); i++)
+		if (ImagePath[i] == '\\')
+			last = i + 1;
 
+	ImagePath = ImagePath + last;
 	*PathName = AllocatePool(StrSize(bootpath) + StrSize(ImagePath));
 
 	if (!*PathName) {

Is the above intentional?

[iokomin]

@xnox thanks for your comment!

ol7 shim review request was submitted after ol8 one, thus it is expected ol7 submission have more fixes comparing to ol8.
Speaking of "ARM discardable sections bugfix" - it will be added in the next ol8 shim update. Absence of this patch is not considered to be critical and shouldn't affect #69 review request.

[julian-klode]

This request seems to have been superseded by a private review in keybase during the boothole review process.

If you still need a newer shim signed, please submit a new issue with the new templates. This should include a Dockerfile that can build your shim binaries so that review does not require manual effort for the building part, see examples/Dockerfile.ubuntu for an example.

(this is a semi-automatic message copied to multiple bugs).