Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shim 15.6 for SUSE Euler Linux 2.0 #260

Closed
8 tasks done
parheliamm opened this issue Jul 4, 2022 · 25 comments
Closed
8 tasks done

Shim 15.6 for SUSE Euler Linux 2.0 #260

parheliamm opened this issue Jul 4, 2022 · 25 comments
Labels
bug Problem with the review that must be fixed before it will be accepted new vendor This is a new vendor

Comments

@parheliamm
Copy link

Confirm the following are included in your repo, checking each box:

  • completed README.md file with the necessary information
  • shim.efi to be signed
  • public portion of your certificate(s) embedded in shim (the file passed to VENDOR_CERT_FILE)
  • binaries, for which hashes are added to vendor_db ( if you use vendor_db and have hashes allow-listed )
  • 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
  • a Dockerfile to reproduce the build of the provided shim EFI binaries

What is the link to your tag in a repo cloned from rhboot/shim-review?


`https://github.com/parheliamm/shim-review/tree/sel-2.0-shim-20220704


What is the SHA256 hash of your final SHIM binary?


aarch64:
pesign --hash --padding --in usr/share/efi/aarch64/shim-sel.efi
hash: bde31b7ef3c81f7eccbce167057b6675160113fbaaca313108da1848b9556cfd

sha256sum ./shim-sel_aarch64.efi
220af16cb67ea54e34263bfc8d50275a21d66e80036b4cb5516d145c8ffd5809 shim-sel_aarch64.efi

x86_64:
pesign --hash --padding --in=./shim-sel_x86_64.efi
hash: a87485be25d3f27b5dfa17390491537b979cefc8483c0fb08da7dfdc81f04bf8

sha256sum ./shim-sel_x86_64.efi
4dae5b2f24eb0e5d7a96194694fc5a62c1dfb6809f3d1162c86bfe900cc65308 shim-sel_x86_64.efi

@parheliamm
Copy link
Author

Hi reviewers:

Would you please feedback some comments for this request?
Any feedback would be appreciated.

Chenxi

@tSU-RooT
Copy link

tSU-RooT commented Aug 5, 2022

Disclaimer: I am not an authorized reviewer

  • What was being reviewed?
    sel-2.0-shim-20220704 currently points at the following commit:
Author: Chenxi Mao <chenxi.mao@suse.com>
Date:   Tue Jun 28 14:08:41 2022 +0800

    SUSE Euler Linux submission
    
    Signed-off-by: Chenxi Mao <chenxi.mao@suse.com>


  • Reproducibility
    I couldn't get same x86_64 binary from container rebuild.
    Part of log is here.
$ podman build . --no-cache --build-arg ARCHITECTURE="x86_64"
STEP 1/11: FROM docker.io/chenximao/suseeuler-shim-review:2.0-shimreview
STEP 2/11: ARG ARCHITECTURE
--> 9774a04e60e
STEP 3/11: ENV ARCHITECTURE=${ARCHITECTURE}
--> 863fd2db7f2

[snip]

STEP 9/11: RUN cd /shim/ && unrpm /root/rpmbuild/RPMS/$ARCHITECTURE/shim-*.$ARCHITECTURE.rpm
/root/rpmbuild/RPMS/x86_64/shim-15.6-1.se2.x86_64.rpm:	7403 blocks
/root/rpmbuild/RPMS/x86_64/shim-debuginfo-15.6-1.se2.x86_64.rpm:	12082 blocks
/root/rpmbuild/RPMS/x86_64/shim-debugsource-15.6-1.se2.x86_64.rpm:	17251 blocks
--> ba56b4fdb0b
STEP 10/11: RUN pesign --hash --padding --in=/shim/usr/share/efi/$ARCHITECTURE/shim-sel.efi
/shim/usr/share/efi/x86_64/shim-sel.efi a87485be25d3f27b5dfa17390491537b979cefc8483c0fb08da7dfdc81f04bf8
--> ddf2ef34abf
STEP 11/11: RUN sha256sum /shim/usr/share/efi/$ARCHITECTURE/shim-sel.efi
2625283003d4462d85e927b50f686416b86617670e873ea7f73558e7b61d5de3  /shim/usr/share/efi/x86_64/shim-sel.efi
COMMIT
--> 63f027da71f
63f027da71f62a442cd4ff0317cc6a632a4b145841b74940256212c5b7b46c73

Version of packages

gcc: 10.3.1-11.se2
binutils: 2.37-7.se2

In addition, operations of pre-rebuild(copying shim source, install compiler, etc...) stage is hidden this method.
I couldn't debug of mismatch reason.

  • Content of certificate file
Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number: 1 (0x1)
        Signature Algorithm: sha256WithRSAEncryption
        Issuer: CN = Yunche Secure Boot CA, C = CN, ST = Guangdong, L = Shenzhen, O = Yunche Ltd., OU = Build Team, emailAddress = build@yuncheos.net
        Validity
            Not Before: Jun  5 01:57:30 2022 GMT
            Not After : Jun  4 01:57:30 2052 GMT
        Subject: C = CN, ST = Guangdong, O = Yunche Ltd., OU = Secure Boot, CN = Yunche Secure Boot Signing, emailAddress = YCL@bsen
[snip]
        X509v3 extensions:
            X509v3 Basic Constraints: critical
                CA:FALSE
            X509v3 Key Usage: critical
                Digital Signature, Certificate Sign
            X509v3 Extended Key Usage: critical
                Code Signing, 1.3.6.1.4.1.311.10.3.6
            X509v3 Subject Key Identifier: 
                F2:95:A3:56:0E:3B:8F:9C:36:07:4B:A8:E8:D7:ED:81:C0:F5:7E:A3
            X509v3 Authority Key Identifier: 
                C9:6F:E4:D6:63:F3:30:6E:42:42:19:0C:34:5A:CE:1A:BA:50:FE:26
[snip]

This non-CA certificate has 30 years lifetime.
According to reviewer-guidelines,
1 to 5 years is sensible to non-CA certificate.
Probably you should renew cert files.

  • Private key management

The key is installed in a machine with restricted physical and system access. Shim binaries do not include private portions of the key.

If SUSE Euler Linux's management is same quality as SLES(SUSE Linux Enterprise Server), I think OK to pass.

  • SBAT
    Please reconfirm mismatches between your text and actual file.

https://github.com/parheliamm/shim-review/blob/sel-2.0-shim-20220704/README.md?plain=1#L213

On shim, we have:

sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md
shim,1,UEFI shim,shim,1,https://github.com/rhboot/shim
shim.sel,1,SUSE Euler Linux,shim,15.6-1.se2,mailto:euler-security@suse.com

Actual .sbat section of your SHIM is

$ sha256sum shim-sel_x86_64.efi 
4dae5b2f24eb0e5d7a96194694fc5a62c1dfb6809f3d1162c86bfe900cc65308  shim-sel_x86_64.efi
$ objcopy -O binary  --only-section=.sbat ./shim-sel_x86_64.efi /tmp/sbat.txt && cat /tmp/sbat.txt
sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md
shim,2,UEFI shim,shim,1,https://github.com/rhboot/shim
shim.sel,1,SUSE Euler Linux,shim,15.6-1.se2,mailto:euler-security@suse.com

FYI: shim's component_generation had increased at 15.6 release.
rhboot/shim@9a09faf
grub2 is also in a similar situation.

  • Patches for kernel and grub
    Sorry, I'm not familiar with SUSE Euler Linux.
    So I didn't find source of grub2-2.06-11.se2 and kernel patches in something public repository(like a launchpad or salsa).
    Attaching SRPM or sources is makes me easy to review.

@parheliamm
Copy link
Author

Hi @tSU-RooT:

Here is my feedback, please review and comment on it.
Thanks in advance.

  • Reproducibility
    I couldn't get same x86_64 binary from container rebuild.
    Part of log is here.

  • [Chenxi]: According to docker rebuild, the pesign hash data are the same.

    • The reason why sha256sum is different is below:

      1. shim-sel_x86_64.efi is built via OBS automatically, this shim-sel_x86_64.efi will be attached SUSE Euler Cert by default.

      2. To submit shim file to shim-review, we strip SUSE Euler Cert via below command:

        pesign -r -i "$infile" -o "$outfile"
        
      3. pesign will align data with 0 after strip cert, there is no data or information changed.

    • Proof of concept test:

      • Docker shim file size:

        • -rwxr-xr-x 1 root root 937289 Aug  9 09:04 /shim/usr/share/efi/x86_64/shim-sel.efi
      • shim-sel_x86_64.efi

        • -rwxr-xr-x   1 chenxi chenxi  937296  7月  4 14:47 shim-sel_x86_64.efi
      • Copy 937289 bytes from shim-sel_x86_64.efi then calculate the sha256sum

        • dd if=shim-sel_x86_64.efi of=test.efi bs=1 count=937289
          sha256sum test.efi
          ==> 2625283003d4462d85e927b50f686416b86617670e873ea7f73558e7b61d5de3  test.efi
      • The calculation is the same as the docker build. The pesign hash data is the same as well.

        • pesign --hash --padding --in=./test.efi
          hash: a87485be25d3f27b5dfa17390491537b979cefc8483c0fb08da7dfdc81f04bf8
      • Align data are all 0:

        • dd if=shim-sel_x86_64.efi of=aligndata bs=1 skip=937289 count=7
          hexdump -C test.efi 
          00000000  00 00 00 00 00 00 00                              |.......|
          00000007

  • This non-CA certificate has 30 years lifetime.

    According to [reviewer-guidelines](https://github.com/rhboot/shim/wiki/reviewer-guidelines),

    1 to 5 years is sensible to non-CA certificate.

    Probably you should renew cert files.

  • [Chenxi]: The new cert validity is below:

        Validity
            Not Before: Aug  8 11:23:15 2022 GMT
            Not After : Aug  7 11:23:15 2027 GMT

​ For subject information, I would like to request your comments about the email address, is "YCL@bsen" acceptable?

        Subject: C=CN, ST=Guangdong, O=Yunche Ltd., OU=Secure Boot, CN=Yunche Secure Boot Signing/emailAddress=YCL@bsen
  • SBAT:

    shim will change sbat info as below:

    sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md
    shim,2,UEFI shim,shim,1,https://github.com/rhboot/shim
    shim.sel,1,SUSE Euler Linux,shim,15.6-1.se2,mailto:euler-security@suse.com
    

    grub2: do we need to change it to 2 as below:

    sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md
    grub,2,Free Software Foundation,grub,2.06,https://www.gnu.org/software/grub/
    grub.sel,1,SUSE Euler Linux,grub2,2.06-11.se2,mailto:euler-security@suse.com
    
  • grub2 & kernel source code

    grub2 source code enclosed in shim-review package:

    https://github.com/parheliamm/shim-review/blob/sel-2.0-shim-20220704/grub2.tar.gz

    kernel source code available at:

    https://github.com/suseEuler/kernel

@tSU-RooT
Copy link

tSU-RooT commented Aug 9, 2022

@parheliamm

  • Reproducibility

pesign hash data are the same

That's right.

The reason why sha256sum is different

I will check.

  • Certificate

​ For subject information, I would like to request your comments about the email address, is "YCL@bsen" acceptable?

IMHO, actual email address is better.

  • kernel patches
    Looks OK to me.
    Base version is 5.10.

1957a85b0032a81e6482ca4aab883643b8dae06e: had merged to 5.4
75b0cea7bf307f362057cc778efe89af4c615354: had merged to 5.8
eadb2f47a3ced5c64b23b90fd2a3463f63726066: fixed by this patch https://github.com/suseEuler/kernel/blob/SEL-2.0/patches.stable/v5.10.119-0001-lockdown-also-lock-down-previous-kgdb-use.patch

  • grub2 patches
    Base version is 2.06(latest upstream version)

Almost CVEs(July 2020,the March 2021 and the June 7th 2022 grub2 CVE list) are fixed.

Question: I found 'CVE-2022-28737' is not fixed(i.e. Not patched). Miss?

@parheliamm
Copy link
Author

Hi @tSU-RooT:

Thanks for your feedback, here are my comments below, please review and check:

Please review our new shim/grub2 based on your comments. shim/grub2 bumps version to 2

sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md
shim,2,UEFI shim,shim,1,https://github.com/rhboot/shim
shim.sel,1,SUSE Euler Linux,shim,15.6-1.se2,mailto:euler-security@suse.com

grub2:

sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md
grub,2,Free Software Foundation,grub,2.06,https://www.gnu.org/software/grub/
grub.sel,1,SUSE Euler Linux,grub2,2.06-11.se2,mailto:euler-security@suse.com

@tSU-RooT
Copy link

@parheliamm

  • Patches

CVE-2022-28737 affects shim only, grub2 didn't need to apply patches.

Oh, sorry I was wrong.
All necessary patches are seems to be included.

  • SBAT
    Bumped component_generation to 2.
    Looks to OK to me.

@parheliamm
Copy link
Author

@tSU-RooT :
Thanks for your confirmation for sbat definition, I will update shim/grub2 sbat section under your suggestions.

On the other hand, how about the binary checksum, is that still a problem?

@frozencemetery frozencemetery added new vendor This is a new vendor contact verification needed Contact verification is needed for this review labels Aug 15, 2022
@frozencemetery
Copy link
Member

I am going to email you some words for contact verification. Please post them here when you receive them.

@kailiu42
Copy link

I am going to email you some words for contact verification. Please post them here when you receive them.

Here are mine:

nøkleskilter
bibliotekbilen
meiningsfullheter
depigmentasjoner
pretensjonens
sendeskrivet
diettutgifter
partiblokk
henholding
organisasjonsevnens
vass-slipes
Brandsfjord
frelstest

The second letter of the first word seems is not ASCII but some other western language that I don't understand. I just copied & pasted from mutt display on the terminal, not sure if it actually is correct.

@parheliamm
Copy link
Author

I am going to email you some words for contact verification. Please post them here when you receive them.

chenxi.mao@yuncheos.com got below words:

matkroken
ratepensjon
telefonregistra
menneskerettene
ventecellen
virksomhetsområdene
anbudsevalueringen
slentring
tekstdokument
sjølaksen
sommerbeitets
prekentema
personellsituasjonens

@frozencemetery
Copy link
Member

Both verified, thanks!

The second letter of the first word seems is not ASCII but some other western language that I don't understand. I just copied & pasted from mutt display on the terminal, not sure if it actually is correct.

It's Norwegian (Bokmål). Julian uses German, which gave me the idea... it's more entertaining to me this way, and also I get to see how widespread working UTF-8 actually is :)

@frozencemetery frozencemetery removed the contact verification needed Contact verification is needed for this review label Aug 16, 2022
@kailiu42
Copy link

It's Norwegian (Bokmål). Julian uses German, which gave me the idea... it's more entertaining to me this way, and also I get to see how widespread working UTF-8 actually is :)

Nice. UTF-8 indeed works on my system as I'm using Chinese.

@tSU-RooT
Copy link

@parheliamm
Hi,

On the other hand, how about the binary checksum, is that still a problem?

Alright, I think the reason of mismatching is well explained.
I confirmed difference is '0' data align.

--- orig-x64.hex	2022-08-22 19:13:26.000000000 +0800
+++ built-x64.hex	2022-08-22 19:13:25.000000000 +0800
@@ -58578,5 +58578,5 @@
 000e4d10  55 49 44 00 6c 6f 61 64  5f 6f 70 74 69 6f 6e 73  |UID.load_options|
 000e4d20  00 50 4b 45 59 5f 55 53  41 47 45 5f 50 45 52 49  |.PKEY_USAGE_PERI|
 000e4d30  4f 44 5f 69 74 00 58 35  30 39 5f 4e 41 4d 45 5f  |OD_it.X509_NAME_|
-000e4d40  45 4e 54 52 59 5f 69 74  00 00 00 00 00 00 00 00  |ENTRY_it........|
-000e4d50
+000e4d40  45 4e 54 52 59 5f 69 74  00                       |ENTRY_it.|
+000e4d49

Additinally, I've checked by appending below commands.

COPY shim-sel_x86_64.efi /
RUN dd if=/shim-sel_x86_64.efi of=test.efi bs=1 count=937289
RUN hexdump -Cv /shim/usr/share/efi/$ARCHITECTURE/shim-sel.efi > /built-x64.hex
RUN hexdump -Cv /test.efi > /orig-x64.hex
RUN ls -la /shim/usr/share/efi/$ARCHITECTURE/shim-sel.efi /test.efi
RUN sha256sum test.efi
RUN diff -u orig-x64.hex built-x64.hex

@parheliamm
Copy link
Author

Hi @tSU-RooT and @frozencemetery:

We updated the new version based on your previous comments and suggestion, please review:

https://github.com/parheliamm/shim-review/tree/sel-2.0-shim-20220909

Here are 3 changes:
1. New cert file with 5 years validity period
2. Update grub2 cert file and sbat info
3. Update new efi files checksum

Any comments and suggestion would be appreciated.

Chenxi

@parheliamm
Copy link
Author

Hi @tSU-RooT and @frozencemetery :

Would you please review our new submission?
Any feedback would be appreciated.

Chenxi

@tSU-RooT
Copy link

Hi, sorry for my late response.

  • Cert file
$ openssl x509 -text -noout -in shim-sel.der
Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number: 3 (0x3)
        Signature Algorithm: sha256WithRSAEncryption
        Issuer: CN = Yunche Secure Boot CA, C = CN, ST = Guangdong, L = Shenzhen, O = Yunche Ltd., OU = Build Team, emailAddress = build@yuncheos.net
        Validity
            Not Before: Sep  6 10:39:11 2022 GMT
            Not After : Sep  5 10:39:11 2027 GMT
[...]

5 years expiry time, I think OK.

  • New sha256 checksum
    Matched OK.

  • Reproducibility from docker(=podman) build
    Differences are only 0-allign data.

@@ -58578,5 +58578,5 @@
 000e4d10  55 49 44 00 6c 6f 61 64  5f 6f 70 74 69 6f 6e 73  |UID.load_options|
 000e4d20  00 50 4b 45 59 5f 55 53  41 47 45 5f 50 45 52 49  |.PKEY_USAGE_PERI|
 000e4d30  4f 44 5f 69 74 00 58 35  30 39 5f 4e 41 4d 45 5f  |OD_it.X509_NAME_|
-000e4d40  45 4e 54 52 59 5f 69 74  00 00 00 00 00 00 00 00  |ENTRY_it........|
-000e4d50
+000e4d40  45 4e 54 52 59 5f 69 74  00                       |ENTRY_it.|
+000e4d49

  • SBAT
sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md
shim,2,UEFI shim,shim,1,https://github.com/rhboot/shim
shim.sel,1,SUSE Euler Linux,shim,15.6-3.se2,mailto:euler-security@suse.com

Looks OK.
I confirmed that build shim has above sbat section.

I hope someone authorized reviewer confirms OK to pass.

@parheliamm
Copy link
Author

@tSU-RooT :

Thanks for your review and comments.
Unfortunately, We have to update shim binary in coming days because we found a P1 level bug on aarch64 platform which lead to MokManager cannot be launched in some scenarios.
We need to apply patches to binutils to fix that bug which lead to pesign hash data changed on aarch64 platform.
After verification, we will update shim binary later.

Sorry for inconvenience.

Chenxi

@parheliamm
Copy link
Author

Hi @tSU-RooT and @frozencemetery:

We updated the new version based on your previous comments and suggestion, please review:

https://github.com/parheliamm/shim-review/releases/tag/sel-2.0-shim-20221012

Changes:

  1. Update shim binary because of binutils bug fixes
  2. Shim spec update to fix dependency package installation issue.

Any comments and suggestion would be appreciated.

Chenxi

@tSU-RooT
Copy link

Hi, I see some points of new commit.

shim-sel_x86_64.efi: little change from previous version in binary level, reproducibility is also good, so no problem I think.
shim-sel_aarch64.efi: There are a lot of changes, seems to affected from build environment changes(probably binutils update you know)
I will try to check it on aarch64 build.

However, seems to be OK in source level I think.

@tSU-RooT
Copy link

Seem to affected from enhancement binutils-AArch64-EFI.patch (that backported from upstream commit ) in binutils-2.37-10.se2
This patch affects aarch64 EFI(PE) binary build, but looks not problem.

@parheliamm
Copy link
Author

parheliamm commented Oct 14, 2022

Seem to affected from enhancement binutils-AArch64-EFI.patch (that backported from upstream commit ) in binutils-2.37-10.se2 This patch affects aarch64 EFI(PE) binary build, but looks not problem.

binutils-AArch64-EFI.patch is not enough to support AArch64 EFI feature.
Some bug fixes patches need to be backported as well.
Bugs fixes patches has been contributed to openEuler community as below:

https://gitee.com/src-openeuler/binutils/commit/c2c2e8be79dca138196b1cd71d0e1304d3c894f3

Chenxi

@parheliamm
Copy link
Author

Hi @frozencemetery:

Would you please review our new submission?

Looking forward your reply and thanks in advance.

Chenxi

@parheliamm
Copy link
Author

Hi Reviewers:
@frozencemetery @steve-mcintyre:
Would you please review our submission?

Thanks in advance.

Chenxi

@frozencemetery
Copy link
Member

Please note #307

@frozencemetery frozencemetery added the bug Problem with the review that must be fixed before it will be accepted label Feb 16, 2023
@parheliamm
Copy link
Author

Please note #307

The shim of new product (SUSE Euler Linux 2.1) is ready for review(#322), so the old product submission will be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Problem with the review that must be fixed before it will be accepted new vendor This is a new vendor
Projects
None yet
Development

No branches or pull requests

4 participants