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

ITRenew SHIM 15.7 amd64 and i386 #323

Closed
8 tasks done
keithdlopresto opened this issue Mar 1, 2023 · 35 comments
Closed
8 tasks done

ITRenew SHIM 15.7 amd64 and i386 #323

keithdlopresto opened this issue Mar 1, 2023 · 35 comments
Labels
accepted Submission is ready for sysdev new vendor This is a new vendor

Comments

@keithdlopresto
Copy link

keithdlopresto commented Mar 1, 2023

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/ITRenew/shim-review/tree/ITRenew-shim-amd64_i386-20240118


What is the SHA256 hash of your final SHIM binary?


092498ad40bf7da13f72a4649e1c41abf45a76659a8a825e23e1594df7030425  shimx64.efi

150eda5b87d49c41267b75d5932a18a1c72e736bd009e00a6ca37ffe9e3f8e55  shimia32.efi

What is the link to your previous shim review request (if any, otherwise N/A)?


N/A

@frozencemetery frozencemetery added new vendor This is a new vendor incomplete This submission is missing required bits contact verification needed Contact verification is needed for this review labels Mar 1, 2023
@keithdlopresto
Copy link
Author

Our issue has been tagged with the 'incomplete' label.
Have we missed something in either the review submission or this issue template? This is our first attempt at this.
Please advise.
Thanks!

@frozencemetery
Copy link
Member

At the time that tag was applied, you had not filled out the issue template completely.

@frozencemetery frozencemetery removed the incomplete This submission is missing required bits label Mar 6, 2023
@keithdlopresto
Copy link
Author

Please advise on next steps in this process. Do the verification emails need to occur before the review is started?
Is there anything needed from our side at this time?
Thanks, Keith

@keithdlopresto
Copy link
Author

Is there any update on the status of our request?  Are we missing any information for this review to proceed?
I am getting pressure from my management on possible approval timeline.
Since we are new to this process I am just trying to get an idea so I can set expectations.
Thanks in advance.

@dennis-tseng99
Copy link
Collaborator

dennis-tseng99 commented Mar 30, 2023

  • I'm not an authorized reviewer. I just want to help.
  • reproducible is okay by using command buildah bud, rather than docker build --no-cache --tag=shim-review-itrenew . specified in README.md
objcopy -D -j .text -j .sdata -j .data -j .data.ident \
        -j .dynamic -j .rodata -j .rel* \
        -j .rela* -j .dyn -j .reloc -j .eh_frame \
        -j .vendor_cert -j .sbat -j .sbatlevel \
        --target efi-app-ia32 shimia32.so shimia32.efi
./post-process-pe -vv shimia32.efi
/shim-review# find / -name shimia32.efi -print 
/var/lib/containers/storage/overlay/bea9f1e61d7a3303b91d581c16c9037e65271d2089589994632ad126b3711e94/diff/build/shim/build-ia32/shimia32.efi
/var/lib/containers/storage/overlay/bea9f1e61d7a3303b91d581c16c9037e65271d2089589994632ad126b3711e94/diff/build/target/shimia32.efi
/var/lib/containers/storage/overlay/bea9f1e61d7a3303b91d581c16c9037e65271d2089589994632ad126b3711e94/diff/shim-review/shimia32.efi
/var/lib/containers/storage/overlay/435bf8dd5bd4dc95938e22846e61ad21f38649ec3d673329e038a45be2221e00/diff/build/shim/build-ia32/shimia32.efi
/var/lib/containers/storage/overlay/435bf8dd5bd4dc95938e22846e61ad21f38649ec3d673329e038a45be2221e00/diff/build/target/shimia32.efi
/var/lib/containers/storage/overlay/435bf8dd5bd4dc95938e22846e61ad21f38649ec3d673329e038a45be2221e00/diff/shim-review/shimia32.efi
/shim-review/shimia32.efi

root@GoogleDNS:/shim-review# find / -name shimx64.efi -print
/var/lib/containers/storage/overlay/bea9f1e61d7a3303b91d581c16c9037e65271d2089589994632ad126b3711e94/diff/build/shim/build-x86_64/shimx64.efi
/var/lib/containers/storage/overlay/bea9f1e61d7a3303b91d581c16c9037e65271d2089589994632ad126b3711e94/diff/build/target/shimx64.efi
/var/lib/containers/storage/overlay/bea9f1e61d7a3303b91d581c16c9037e65271d2089589994632ad126b3711e94/diff/shim-review/shimx64.efi
/var/lib/containers/storage/overlay/435bf8dd5bd4dc95938e22846e61ad21f38649ec3d673329e038a45be2221e00/diff/build/shim/build-x86_64/shimx64.efi
/var/lib/containers/storage/overlay/435bf8dd5bd4dc95938e22846e61ad21f38649ec3d673329e038a45be2221e00/diff/build/target/shimx64.efi
/var/lib/containers/storage/overlay/435bf8dd5bd4dc95938e22846e61ad21f38649ec3d673329e038a45be2221e00/diff/shim-review/shimx64.efi
/shim-review/shimx64.efi
  • hash values are matched for shimx64.efi and shimia32.efi respectively.
STEP 24: RUN sha256sum /build/target/shimx64.efi /shim-review/shimx64.efi
a227d70c0f8273ee399025e42c9276243d76d1dc2493042a6cfe7ade763b5b0f  /build/target/shimx64.efi
a227d70c0f8273ee399025e42c9276243d76d1dc2493042a6cfe7ade763b5b0f  /shim-review/shimx64.efi
STEP 25: RUN sha256sum /build/target/shimia32.efi /shim-review/shimia32.efi
56359f37e20b75f23911e890dc167e4278538e7a3a008b15e07b96a74182d857  /build/target/shimia32.efi
56359f37e20b75f23911e890dc167e4278538e7a3a008b15e07b96a74182d857  /shim-review/shimia32.efi
  • sbat seems okay
.sbat section:
 d2000 73626174 2c312c53 42415420 56657273  sbat,1,SBAT Vers
 d2010 696f6e2c 73626174 2c312c68 74747073  ion,sbat,1,https
 d2020 3a2f2f67 69746875 622e636f 6d2f7268  ://github.com/rh
 d2030 626f6f74 2f736869 6d2f626c 6f622f6d  boot/shim/blob/m
 d2040 61696e2f 53424154 2e6d640a 7368696d  ain/SBAT.md.shim
 d2050 2c332c55 45464920 7368696d 2c736869  ,3,UEFI shim,shi
 d2060 6d2c312c 68747470 733a2f2f 67697468  m,1,https://gith
 d2070 75622e63 6f6d2f72 68626f6f 742f7368  ub.com/rhboot/sh
 d2080 696d0a73 68696d2e 69747265 6e65772c  im.shim.itrenew,
 d2090 312c4954 72656e65 7720496e 632e2c73  1,ITrenew Inc.,s
 d20a0 68696d2c 31352e37 2c6d6169 6c3a7365  him,15.7,mail:se
 d20b0 63757269 74794069 7472656e 65772e63  curity@itrenew.c
 d20c0 6f6d                                 om
  • CA validity seems okay
Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number:
            96:8f:66:83:2e:46:4b:bb:24:76:c4:17:99:52:56:8e
        Signature Algorithm: sha256WithRSAEncryption
        Issuer: C = GB, O = Sectigo Limited, CN = Sectigo Public Code Signing CA EV R36
        Validity
            Not Before: Jan 22 00:00:00 2023 GMT
            Not After : Jan 21 23:59:59 2026 GMT

@keithdlopresto
Copy link
Author

Thank you Dennis.

@keithdlopresto
Copy link
Author

Is there anything we can/'need to' do from our side to move this process along?
We have a product release waiting on this. Without this shim our customers will have to disable secure boot.
Thanks in advance.

@aronowski
Copy link
Collaborator

aronowski commented Apr 20, 2023

The review is done very well and it surprised me positively how you implemented NX support - using the native functionality rather than porting a patch.

Although there's one error I noticed. Take a look:

$ objdump -s -j .sbatlevel shimx64.efi 

shimx64.efi:     file format pei-x86-64

Contents of section .sbatlevel:
 85000 00000000 08000000 26000000 73626174  ........&...sbat
 85010 2c00312c 00323032 32303532 34303000  ,.1,.2022052400.
 85020 0a006772 75622c32 0a007362 61742c00  ..grub,2..sbat,.
 85030 312c0032 30323231 31313530 30000a00  1,.2022111500...
 85040 7368696d 2c320a67 7275622c 330a00    shim,2.grub,3.. 

There are some NUL characters, which shouldn't be here. You'll need to port this patch and once that's done, update the shims' checksums.


Also, I interpret the question URL for a repo that contains the exact code which was built to get this binary as providing the URL to rhboot's shim repository or their appropriate tag.


Is there anything we can/'need to' do from our side to move this process along?

Regarding the update status of your request I heard somewhere the official reviewers are already overworked with their jobs unrelated to reviewing. And Robbie said this too:

shim-review is meant to be distros reviewing each other and right now it's very much not.
[...]
Submitters quickly become frustrated when they don't get feedback. For reasons we can only speculate about, they tend to complain and try to "escalate" rather than reviewing other submissions.

I think we all may review other reviews to make it easier for the official reviewers. I pointed out something in yours, you may review the one I posted as a token of appreciation. ;)

@keithdlopresto
Copy link
Author

@aronowski
Thank you for the review and feedback.
I wish I could take credit for the 'very well done' review, but it was another engineer who did this work.
We have applied the mentioned patch and I have updated our issue with the new shim repository tag and updated checksums.
One of our engineers will be providing updated .sbat and .sbatlevel data to confirm this.

@vden-irm
Copy link

@aronowski
Thank you very much for the review and for finding the problem in our submission.
And indeed the .sbatlevel section was not properly generated by the binutils.
We use Debian 11 as a build system and there is the binutils version 2.35.2.

We have applied this patch and rebuilt EFI binaries.
Then we have updated submission template (README.md) with a new information about patch applied and updated EFI binaries checksums.
The new tag was created and this issue has been updated accordingly.

Currently .sbat and .sbatlevel sections look like:

$ objdump -s -j .sbatlevel shimx64.efi 

shimx64.efi:     file format pei-x86-64

Contents of section .sbatlevel:
 85000 00000000 08000000 22000000 73626174  ........"...sbat
 85010 2c312c32 30323230 35323430 300a6772  ,1,2022052400.gr
 85020 75622c32 0a007362 61742c31 2c323032  ub,2..sbat,1,202
 85030 32313131 3530300a 7368696d 2c320a67  2111500.shim,2.g
 85040 7275622c 330a00                      rub,3..   
$ objdump -s -j .sbatlevel shimia32.efi

shimia32.efi:     file format pei-i386

Contents of section .sbatlevel:
 6e000 00000000 08000000 22000000 73626174  ........"...sbat
 6e010 2c312c32 30323230 35323430 300a6772  ,1,2022052400.gr
 6e020 75622c32 0a007362 61742c31 2c323032  ub,2..sbat,1,202
 6e030 32313131 3530300a 7368696d 2c320a67  2111500.shim,2.g
 6e040 7275622c 330a00                      rub,3..    
$ objdump -s -j .sbat shimx64.efi 

shimx64.efi:     file format pei-x86-64

Contents of section .sbat:
 d2000 73626174 2c312c53 42415420 56657273  sbat,1,SBAT Vers
 d2010 696f6e2c 73626174 2c312c68 74747073  ion,sbat,1,https
 d2020 3a2f2f67 69746875 622e636f 6d2f7268  ://github.com/rh
 d2030 626f6f74 2f736869 6d2f626c 6f622f6d  boot/shim/blob/m
 d2040 61696e2f 53424154 2e6d640a 7368696d  ain/SBAT.md.shim
 d2050 2c332c55 45464920 7368696d 2c736869  ,3,UEFI shim,shi
 d2060 6d2c312c 68747470 733a2f2f 67697468  m,1,https://gith
 d2070 75622e63 6f6d2f72 68626f6f 742f7368  ub.com/rhboot/sh
 d2080 696d0a73 68696d2e 69747265 6e65772c  im.shim.itrenew,
 d2090 312c4954 72656e65 7720496e 632e2c73  1,ITrenew Inc.,s
 d20a0 68696d2c 31352e37 2c6d6169 6c3a7365  him,15.7,mail:se
 d20b0 63757269 74794069 7472656e 65772e63  curity@itrenew.c
 d20c0 6f6d                                 om    
$ objdump -s -j .sbat shimia32.efi

shimia32.efi:     file format pei-i386

Contents of section .sbat:
 a0000 73626174 2c312c53 42415420 56657273  sbat,1,SBAT Vers
 a0010 696f6e2c 73626174 2c312c68 74747073  ion,sbat,1,https
 a0020 3a2f2f67 69746875 622e636f 6d2f7268  ://github.com/rh
 a0030 626f6f74 2f736869 6d2f626c 6f622f6d  boot/shim/blob/m
 a0040 61696e2f 53424154 2e6d640a 7368696d  ain/SBAT.md.shim
 a0050 2c332c55 45464920 7368696d 2c736869  ,3,UEFI shim,shi
 a0060 6d2c312c 68747470 733a2f2f 67697468  m,1,https://gith
 a0070 75622e63 6f6d2f72 68626f6f 742f7368  ub.com/rhboot/sh
 a0080 696d0a73 68696d2e 69747265 6e65772c  im.shim.itrenew,
 a0090 312c4954 72656e65 7720496e 632e2c73  1,ITrenew Inc.,s
 a00a0 68696d2c 31352e37 2c6d6169 6c3a7365  him,15.7,mail:se
 a00b0 63757269 74794069 7472656e 65772e63  curity@itrenew.c
 a00c0 6f6d                                 om 

Checking for NX Support:

$ objdump -p shimx64.efi | grep DllCharacteristics
DllCharacteristics	00000100
$ objdump -p shimia32.efi | grep DllCharacteristics
DllCharacteristics	00000100

Confirmation of checksums:

$ sha256sum shimx64.efi 
13fd404bea0193501b2878d85377529a2d1b2a01d5e9b831749ecf732110ee09  shimx64.efi
$ sha256sum shimia32.efi 
50e655bbce496836545c6dd4ccf9fa36caba3e66193987e09b440aea0531703d  shimia32.efi

And compare them with checksums from the Dockerfile's build.log:

#31 [27/30] RUN sha256sum /build/target/shimx64.efi /shim-review/shimx64.efi
#31 0.489 13fd404bea0193501b2878d85377529a2d1b2a01d5e9b831749ecf732110ee09  /build/target/shimx64.efi
#31 0.494 13fd404bea0193501b2878d85377529a2d1b2a01d5e9b831749ecf732110ee09  /shim-review/shimx64.efi
#31 DONE 0.5s

#32 [28/30] RUN sha256sum /build/target/shimia32.efi /shim-review/shimia32.efi
#32 0.463 50e655bbce496836545c6dd4ccf9fa36caba3e66193987e09b440aea0531703d  /build/target/shimia32.efi
#32 0.468 50e655bbce496836545c6dd4ccf9fa36caba3e66193987e09b440aea0531703d  /shim-review/shimia32.efi
#32 DONE 0.5s

Checking if a code section is read-only:

$ objdump -h shimx64.efi | grep -A1 .text
  1 .text         0005e839  0000000000023000  0000000000023000  0001de00  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
$ objdump -h shimia32.efi | grep -A1 .text
  0 .text         0006595c  00005000  00005000  00000400  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, CODE

Unfortunately, I could not find information on how to initiate the process of Contacts Verification specified in the template.
We cannot send ourselves a set of words for verification, because in this case there is no arbitration of the third party.
Can any reviewer may do verification of contacts or only an authorized one?

Thank you.

@aronowski
Copy link
Collaborator

I can see the updates. Reproduced the artifacts as well and everything looks good to me.

Great job!


Regarding the verification of contacts, I believe only the ones by authorized reviewers count. The same when it comes to labeling issues and, as far as I understand, letting Microsoft know on the updates.

I can perform this process unofficially for you to make you happy but I don't think it will count. I believe the best we can do is to, as mentioned in my previous comment, help with the peer review so the official reviewers have less work to do manually - especially if there are some hard-to-notice errors, which can be fixed in the meantime.

For instance I can see you reviewed the review I initiated and I'm grateful for that. That's a way of learning, how all this works. You can review other reviews too, confirm that others' builds are reproducible, look for common errors, etc. even if they already have been reviewed by someone else, like this one.

I hope this goes well and wish you all the best!
If there's something I can help with even more, please ping me or email me.

@keithdlopresto
Copy link
Author

Is there any way to expedite this process?
Ours, along with many other open issues, appear to be stagnant. Is there any third party providers who can help with the approval process?

@THS-on
Copy link
Collaborator

THS-on commented Sep 30, 2023

Review of ITRenew-shim-amd64_i386-20230421

  • It's the first submission for ITRenew
  • They require a custom shim because they include custom storage driver patches and the product is used on devices with Secure Boot.
  • Security contacts still need to be verified
  • Shim is reproducible with Dockerfile

HASHES

#31 [27/30] RUN sha256sum /build/target/shimx64.efi /shim-review/shimx64.efi
#31 0.388 13fd404bea0193501b2878d85377529a2d1b2a01d5e9b831749ecf732110ee09  /build/target/shimx64.efi
#31 0.393 13fd404bea0193501b2878d85377529a2d1b2a01d5e9b831749ecf732110ee09  /shim-review/shimx64.efi
#31 DONE 0.5s

#32 [28/30] RUN sha256sum /build/target/shimia32.efi /shim-review/shimia32.efi
#32 0.399 50e655bbce496836545c6dd4ccf9fa36caba3e66193987e09b440aea0531703d  /build/target/shimia32.efi
#32 0.404 50e655bbce496836545c6dd4ccf9fa36caba3e66193987e09b440aea0531703d  /shim-review/shimia32.efi
#32 DONE 0.5s

SBAT

sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md
shim,3,UEFI shim,shim,1,https://github.com/rhboot/shim
shim.itrenew,1,ITrenew Inc.,shim,15.7,mail:security@itrenew.com
  • Shim version is 15.7 with NX flag manually set
    • sbat_var.S.patch matches rhboot/shim@657b248
    • NX patch is not applied, but the flag is set manually after the build
  • The SBAT entry has no newline at the end, I don't know if that causes issues with some parsers
  • Certificate matches the organization
    • Subject: serialNumber = 2418938, jurisdictionC = US, jurisdictionST = California, businessCategory = Private Organization, C = US, ST = California, O = "ITrenew, Inc.", CN = "ITrenew, Inc."
    • Serial: 96:8f:66:83:2e:46:4b:bb:24:76:c4:17:99:52:56:8e
    • Valid till: Jan 21 23:59:59 2026 GMT (3 years)
    • Is an EV certificate signed by Sectigo Limited, extended key usage for code signing is set
  • Key is stored on a FIPS-140-2 token with restricted access
  • Shim launches only GRUB2
    • Upstream GRUB is used, therefore shim_lock is used
    • SBAT entry for GRUB2 looks good
    • List of GRUB2 modules looks good: all_video boot chain configfile echo ext2 exfat fat font gfxmenu gfxterm gfxterm_background gzio halt iso9660 jpeg linux loadenv loopback lvm memdisk normal ntfs part_gpt part_msdos png probe reboot search search_fs_file search_fs_uuid search_label test tpm true video xfs
  • Kernel is 5.15.70
    • is build with lockdown support and CONFIG_MODULE_SIG_FORCE set to y

Questions

  • How does ITRenew and Iron Mountain relate to each other?
  • Can you update your SBAT entry to include a newline at the end?
  • You mentioned that all Linux modules are signed with your EV certificate. We recently added a question on how you ensure that a different kernel build cannot load modules from another kernel (1f85d85). The easiest way is to use an ephemeral key for signing, but there are also other ways. Do you have a mechanism for that in place?

@THS-on THS-on added bug Problem with the review that must be fixed before it will be accepted question Reviewer(s) waiting on response labels Sep 30, 2023
@keithdlopresto
Copy link
Author

Hi Thore,

Thank you for your review.
I'll have to let one of our engineers handle the technical question. ;)

How does ITRenew and Iron Mountain relate to each other?

ITRenew was acquired by IronMountain. We were in the process of purchasing the certificate and it was decided to continue with the ITRenew name at that time. Our ITRenew emails will route to our IronMountain inboxes.
At a future date, renewal(?), we may purchase certificates that reflect IronMountain.

Hope that helps. Please revert back if you have additional questions on this.

@vden-irm
Copy link

vden-irm commented Oct 4, 2023

Hi Thore,

Thank you very much for the review.
I updated SBAT and added the newline at the end. Rebuilt shimx64.efi and shimia32.efi both:

$ objdump -s -j .sbat shimx64.efi 

shimx64.efi:     file format pei-x86-64

Contents of section .sbat:
 d2000 73626174 2c312c53 42415420 56657273  sbat,1,SBAT Vers
 d2010 696f6e2c 73626174 2c312c68 74747073  ion,sbat,1,https
 d2020 3a2f2f67 69746875 622e636f 6d2f7268  ://github.com/rh
 d2030 626f6f74 2f736869 6d2f626c 6f622f6d  boot/shim/blob/m
 d2040 61696e2f 53424154 2e6d640a 7368696d  ain/SBAT.md.shim
 d2050 2c332c55 45464920 7368696d 2c736869  ,3,UEFI shim,shi
 d2060 6d2c312c 68747470 733a2f2f 67697468  m,1,https://gith
 d2070 75622e63 6f6d2f72 68626f6f 742f7368  ub.com/rhboot/sh
 d2080 696d0a73 68696d2e 69747265 6e65772c  im.shim.itrenew,
 d2090 312c4954 72656e65 7720496e 632e2c73  1,ITrenew Inc.,s
 d20a0 68696d2c 31352e37 2c6d6169 6c3a7365  him,15.7,mail:se
 d20b0 63757269 74794069 7472656e 65772e63  curity@itrenew.c
 d20c0 6f6d0a                               om.        
$ objdump -s -j .sbat shimia32.efi 

shimia32.efi:     file format pei-i386

Contents of section .sbat:
 a0000 73626174 2c312c53 42415420 56657273  sbat,1,SBAT Vers
 a0010 696f6e2c 73626174 2c312c68 74747073  ion,sbat,1,https
 a0020 3a2f2f67 69746875 622e636f 6d2f7268  ://github.com/rh
 a0030 626f6f74 2f736869 6d2f626c 6f622f6d  boot/shim/blob/m
 a0040 61696e2f 53424154 2e6d640a 7368696d  ain/SBAT.md.shim
 a0050 2c332c55 45464920 7368696d 2c736869  ,3,UEFI shim,shi
 a0060 6d2c312c 68747470 733a2f2f 67697468  m,1,https://gith
 a0070 75622e63 6f6d2f72 68626f6f 742f7368  ub.com/rhboot/sh
 a0080 696d0a73 68696d2e 69747265 6e65772c  im.shim.itrenew,
 a0090 312c4954 72656e65 7720496e 632e2c73  1,ITrenew Inc.,s
 a00a0 68696d2c 31352e37 2c6d6169 6c3a7365  him,15.7,mail:se
 a00b0 63757269 74794069 7472656e 65772e63  curity@itrenew.c
 a00c0 6f6d0a                               om. 

New hashes:

$ sha256sum shimx64.efi 
0609f3c1557699c5d22d6c9b3275c1d548f9eb15418bee2e0b7e660e794801ea  shimx64.efi
$ sha256sum shimia32.efi 
d286d48415259e769671b96fb807a10e3a1e918b2f8757bf871ff474c8ee39e8  shimia32.efi

I also updated the README.md template with these new hashes the have created a new tag:
https://github.com/ITRenew/shim-review/tree/ITRenew-shim-amd64_i386-20231004

Unfortunately I don't have permission to edit this issue to reflect hashes and tag changes in it as well.
@keithdlopresto Could you please edit this issue and update two hashes and tag link?

You mentioned that all Linux modules are signed with your EV certificate. We recently added a question on how you ensure that a different kernel build cannot load modules from another kernel (1f85d85). The easiest way is to use an ephemeral key for signing, but there are also other ways. Do you have a mechanism for that in place?

No, we do not use ephemeral key for signing modules for our current kernel.
To prevent loading modules between different builds we use following:

  1. Add a suffix to the version by build system. 5.15.70-xxxx. Each build has unique version number.
  2. Module versioning support is disabled in the kernel config: CONFIG_MODVERSIONS is not set.

In the next iteration we plan update the kernel in our platform to the next longterm support kernel 6.1.x. And we definitely will use ephemeral key module signing for it.

Thanks.

@THS-on
Copy link
Collaborator

THS-on commented Oct 4, 2023

SBAT entries are now fixed and shims also reproducible:

#31 [27/30] RUN sha256sum /build/target/shimx64.efi /shim-review/shimx64.efi
#31 0.617 0609f3c1557699c5d22d6c9b3275c1d548f9eb15418bee2e0b7e660e794801ea  /build/target/shimx64.efi
#31 0.623 0609f3c1557699c5d22d6c9b3275c1d548f9eb15418bee2e0b7e660e794801ea  /shim-review/shimx64.efi
#31 DONE 0.6s

#32 [28/30] RUN sha256sum /build/target/shimia32.efi /shim-review/shimia32.efi
#32 0.421 d286d48415259e769671b96fb807a10e3a1e918b2f8757bf871ff474c8ee39e8  /build/target/shimia32.efi
#32 0.428 d286d48415259e769671b96fb807a10e3a1e918b2f8757bf871ff474c8ee39e8  /shim-review/shimia32.efi
#32 DONE 0.4s

No, we do not use ephemeral key for signing modules for our current kernel.
To prevent loading modules between different builds we use following:

  1. Add a suffix to the version by build system. 5.15.70-xxxx. Each build has unique version number.
  2. Module versioning support is disabled in the kernel config: CONFIG_MODVERSIONS is not set.

In the next iteration we plan update the kernel in our platform to the next longterm support kernel 6.1.x. And we definitely will use ephemeral key module signing for it.

According the the kernel documentation changing the local version every time is a valid strategy (https://www.kernel.org/doc/html/latest/admin-guide/module-signing.html#administering-protecting-the-private-key).

I'll remove the bug label, once the top issue comment is updated to the new information.

@THS-on THS-on removed the question Reviewer(s) waiting on response label Oct 4, 2023
@keithdlopresto
Copy link
Author

keithdlopresto commented Oct 4, 2023

@THS-on
Issue details (first post) updated with new Tag link and Hash values.

@THS-on THS-on removed the bug Problem with the review that must be fixed before it will be accepted label Oct 5, 2023
@THS-on
Copy link
Collaborator

THS-on commented Oct 5, 2023

While trying to sending out the contact verification messages, I noticed that the PGP key for David Friedman only includes the email address dfriedman@ironmountain.com not david.friedman@ironmountain.com (https://keyserver.ubuntu.com/pks/lookup?search=B2C6438FFDE5A3AAFB8CE3BEA3CD579F25D41BC3&fingerprint=on&op=index). Can you update that?

@keithdlopresto
Copy link
Author

keithdlopresto commented Oct 17, 2023

@THS-on
Sorry for the delay. We had problems updating the key.
The key has been updated to include "david.friedman@ironmountain.com" email address.
Key have been updated on the keyserver.

Thank you for reviewing!

@keithdlopresto
Copy link
Author

@THS-on

The following words were received by Keith:

sauce
champion
find
lightweight
prefix
deejays
copybook
regards's
fanciest
superstitions

@irmdfriedman
Copy link

@THS-on

The following words were received by David

midterms
shareable
footprints
whoops
contradicts
enjoying
software's
shamelessness
treetops
spell

@THS-on
Copy link
Collaborator

THS-on commented Oct 20, 2023

Contact verification is complete the phrases are correct. I would like that another reviewer again take a look at it, because it is a new vendor. @dennis-tseng99 @aronowski can one of you take another look at it?

Note that we are currently discussing what are the acceptable strategies for kernel module singing, besides ephemeral key singing. So we are likely only marking the submission as accepted once we agreed on that and it matches your approach.

@THS-on THS-on added extra review wanted Initial review(s) look good, another review desired and removed contact verification needed Contact verification is needed for this review labels Oct 20, 2023
@aronowski
Copy link
Collaborator

@THS-on, I'll perform an additional review soon, most likely the next Monday. Assigning this one to myself.

If there are no major errors, I'm then marking this application as accepted, since the people behind it do have broad knowledge on the subject.

@aronowski aronowski self-assigned this Oct 21, 2023
@aronowski
Copy link
Collaborator

I've checked the changes and the build reproducibility. Everything looks alright.

  • The build does reproduce and considering how much of a solid, stable platform Debian is, they should still do in the far future.
  • There's an additional 0A (LF '\n' (new line)) both in the sbat.csv file, as well as in the binaries.

On April 24 I wrote:

I hope this goes well and wish you all the best!

And it did! We can celebrate! ;-)

@aronowski aronowski added accepted Submission is ready for sysdev and removed extra review wanted Initial review(s) look good, another review desired labels Oct 23, 2023
@aronowski aronowski removed their assignment Oct 23, 2023
@vden-irm
Copy link

Kamil, Thore, Dennis and everyone,
Thank you guys for helping with our submission, for multiple reviews and finding problems there.

@keithdlopresto
Copy link
Author

Yes, thank you to all for your time on this matter. It is greatly appreciated.

@aronowski aronowski mentioned this issue Dec 18, 2023
8 tasks
@vden-irm
Copy link

vden-irm commented Jan 18, 2024

Hello colleagues,

Two months ago, when we were preparing Cabinet file with included shim images to submit an application to Microsoft, we had a hardware failure with the eToken FIPS device where our company's EV Code Sign certificate is stored. It stopped working and we were unable to sign the Cabinet file.

It took some time for the certificate authority to help us and the issuer confirmed that it is not possible to restore the eToken FIPS and the certificate on it is lost, so we need to issue a new certificate.

We received a replacement of a new hardware eToken FIPS with a new certificate issued.
Since the SHIM from our previous request was never signed and released, there is no need for it to be revoked.

I updated this shim-review request with a new issued vendor certificate for approval.
SHIM binaries (shimx64.efi and shimia32.efi) were rebuilt again using this new certificate.
Also the NX bit setting has been disabled during the build process as we do not use kernel 6.7 yet.

Change list:

  1. Certificate replacement
  2. Disable NX bit setting
  3. Updated README.md template with a new hashes.

@keithdlopresto Could you please edit this issue and update two hashes and tag link?

092498ad40bf7da13f72a4649e1c41abf45a76659a8a825e23e1594df7030425  shimx64.efi
150eda5b87d49c41267b75d5932a18a1c72e736bd009e00a6ca37ffe9e3f8e55  shimia32.efi

https://github.com/ITRenew/shim-review/tree/ITRenew-shim-amd64_i386-20240118

Thank you!

@aronowski aronowski self-assigned this Jan 18, 2024
@keithdlopresto
Copy link
Author

Issue details updated with new Tag link and Hash values with Vlad's latest update.

@aronowski
Copy link
Collaborator

The build reproduces, checksums match, NX support properly disabled until the whole chain gets NX support. Certificate is valid.

Great job! Leaving the accepted label as it was. ;-)

@aronowski aronowski removed their assignment Jan 19, 2024
@vden-irm
Copy link

Thank you very much for your help and review!

We'll start preparing for the Microsoft submission.
Will keep you informed about the result.

Thanks.

@keithdlopresto
Copy link
Author

keithdlopresto commented Jan 19, 2024 via email

@THS-on
Copy link
Collaborator

THS-on commented Feb 5, 2024

What is the status of this? Did you get a signed shim back or are you creating a new submission for 15.8?

@keithdlopresto
Copy link
Author

As of last Friday we were stuck in the 'scanning' state on the MS submission site. I will update this issue with any change of status.

@keithdlopresto
Copy link
Author

Our submission to MS is lingering in the 'scanning' state. I suspect we will have to go through the process with the v15.8 shim.

@THS-on
Copy link
Collaborator

THS-on commented Feb 8, 2024

Ok then I'll close this one, because there is already a new submission.

@THS-on THS-on closed this as not planned Won't fix, can't repro, duplicate, stale Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted Submission is ready for sysdev new vendor This is a new vendor
Projects
None yet
Development

No branches or pull requests

7 participants