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

Blancco Shim 15.7 for x64 & ia32 #290

Closed
8 tasks done
dpedigo opened this issue Nov 1, 2022 · 30 comments
Closed
8 tasks done

Blancco Shim 15.7 for x64 & ia32 #290

dpedigo opened this issue Nov 1, 2022 · 30 comments
Labels
accepted Submission is ready for sysdev

Comments

@dpedigo
Copy link

dpedigo commented Nov 1, 2022

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/dpedigo/shim-review/tree/blancco-shim15.7-x64-ia32-20231113


What is the SHA256 hash of your final SHIM binary?


4e77679d52d358d3ad441a37adf9584fdc6cd0b699ed77026f5af2f9b457b20a  shimia32.efi
28b4d0c57f95b0b3d6d27d97a31a20e634ff7175012f732f26232f0b71190749  shimx64.efi

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


#9

@dpedigo
Copy link
Author

dpedigo commented Nov 11, 2022

Tagged a new release, we needed to replace the embedded certificate.

@frozencemetery
Copy link
Member

You appear to be using Red Hat's grub2 + shim (which is of course fine). But I don't see from your submission why you need to rebuild them - could you not just ship Red Hat's signed shim+grub2+kernel directly?

@frozencemetery frozencemetery added the question Reviewer(s) waiting on response label Nov 18, 2022
@dpedigo
Copy link
Author

dpedigo commented Nov 18, 2022

I'd have to compare the Red Hat Kconfig with ours to find all of the differences but end of the day the biggest issue is that we have a variety of custom kernel patches (mainly to storage drivers) to work around a variety of quirks. These have not been upstreamed.

@dpedigo dpedigo changed the title Blancco Shim 15.6 for x64 & ia32 Blancco Shim 15.7 for x64 & ia32 Dec 2, 2022
@dpedigo
Copy link
Author

dpedigo commented Dec 2, 2022

Updated the shim request to version 15.7. Need any additional information from our end?

@dpedigo
Copy link
Author

dpedigo commented Jan 3, 2023

Updated the shim submission with the NX compatibility flags set.

@don-blancco
Copy link

@frozencemetery @vathpela Checking to see if there is something we need to do on our end. Our customers are having to turn off secure boot to use our products.
Thanks, Don

@frozencemetery frozencemetery removed the question Reviewer(s) waiting on response label Feb 16, 2023
@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
@dpedigo
Copy link
Author

dpedigo commented Feb 16, 2023

@frozencemetery We're already setting the NX flag, as can be seen in the Dockerfile. I can switch to providing a patch, but it seems unnecessary given the changes. Is there some other issue holding this up?

[...]
RUN ./build-x64/post-process-pe -vvv -n build-x64/shimx64.efi
RUN ./build-ia32/post-process-pe -vvv -n build-ia32/shimia32.efi
[...]

@frozencemetery frozencemetery removed the bug Problem with the review that must be fixed before it will be accepted label Feb 16, 2023
@don-blancco
Copy link

@frozencemetery
From the history it appears, to me, that our submission has been complete and correct since Jan 3.
Is there anything else we need to do to gain approval?

@don-blancco
Copy link

@frozencemetery
Is this process just totally broken?
What can we do to get this moving?

@aronowski
Copy link
Collaborator

@don-blancco, answering your question:

What can we do to get this moving?

As far as I can see, the best people can do to speed up the reviewing process is to help review other issues - the point of this project is that issues should be peer-reviewed:

shim-review is meant to be distros reviewing each other and right now it's very much not.

@steve-mcintyre
Copy link
Collaborator

Working on this now, apologies for the delay.

Identification mails sent - please follow the instructions there.

@steve-mcintyre
Copy link
Collaborator

steve-mcintyre commented Sep 5, 2023

Review of Blancco Shim 15.7 for x64 & ia32 blancco-shim15.7-x64-ia32-20230103

OK

  • Shim build reproduces fine for ia32
  • Shim from upstream, with no patches applied
  • Includes a CA cert with ~9 years left, ok
  • Hardware token for key management, good
  • Previous signed shim has been revoked, new clean trust chain here
  • SBAT data looks fine
  • Grub looks ok, borrowed from RH
  • List of grub modules looks OK for now
  • Kernel lockdown sounds ok

Issues / queries / outstanding

  • Shim build does not reproduce for x64. You claim
    92dac8731ba43b206c3f6f3b331748ff26f93a3dc10e62ae3be35e40c4753452 shimx64.efi
    but I get from my build locally:
    28b4d0c57f95b0b3d6d27d97a31a20e634ff7175012f732f26232f0b71190749 /build/output/shimx64.efi
    Hexdump and comparison suggests minor shuffling of code. Looks like
    the compiler has moved on (you used gcc 11.3, I'm seeing 11.4 in
    jammy now). Could you please update?
  • Contact verification required - mails sent
  • You mention custom kernel patches - could you expand on that please?

@evilteq
Copy link

evilteq commented Sep 6, 2023

Thanks for reviewing, @steve-mcintyre !
I also get that hash now, the review is updated accordingly in today's tag (here).

About the kernel patches, we sometimes patch the kernel for hardware support issues, from patches that have not been accepted into mainline yet, or backporting to the version we are using at that moment.
We also add kernel messages and sysfs properties to get hardware info.

@daniel-blancco
Copy link

@steve-mcintyre decriminalize crest redoubtable nodular inkwells nonconductor canisters programmed rankness turmoil

@steve-mcintyre
Copy link
Collaborator

Thanks for reviewing, @steve-mcintyre ! I also get that hash now, the review is updated accordingly in today's tag (here).

Could you also please update the copy of shimx64.efi in git as well?

@steve-mcintyre
Copy link
Collaborator

About the kernel patches, we sometimes patch the kernel for hardware support issues, from patches that have not been accepted into mainline yet, or backporting to the version we are using at that moment. We also add kernel messages and sysfs properties to get hardware info.

Hmmm, ok.

@evilteq
Copy link

evilteq commented Sep 8, 2023

Could you also please update the copy of shimx64.efi in git as well?

Updated on today's tag.

@steve-mcintyre
Copy link
Collaborator

Still waiting on contact verification from @evilteq

@evilteq
Copy link

evilteq commented Sep 13, 2023

Still waiting on contact verification from @evilteq

I'm me, I swear ;)
Mail probably got lost in the spam filters or something, I've mailed you directly to see if it helps.

@evilteq
Copy link

evilteq commented Sep 14, 2023

Validating identity: midland wrests nasturtiums galleys optimizer investiture swigging Han booklets bologna

@THS-on
Copy link
Collaborator

THS-on commented Nov 1, 2023

Review of blancco-shim15.7-x64-ia32-20230908

  • It is not the first shim submission for Blancco Technology Group 2017-11-02 Blancco #9 (accepted)
  • Needs a shim because their solution runs on off the shelf hardware and they patch some kernel drivers
  • Security contacts (@steve-mcintyre I assume the validation was sucessful?)
  • Shim is reproducible using Dockerfile:

HASHES

#23 0.568 4e77679d52d358d3ad441a37adf9584fdc6cd0b699ed77026f5af2f9b457b20a  /build/output/shimia32.efi
#23 0.577 28b4d0c57f95b0b3d6d27d97a31a20e634ff7175012f732f26232f0b71190749  /build/output/shimx64.efi

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.blancco,1,Blancco Technology Group,shim,15.7,mail:security@blancco.com
  • Note SBAT entry has no newline at the end. I don't know if that is an issue

  • SBAT suffix .blancco is fine

  • Shim is upstream 17.5 with NX flag manually set

    • Is not affected by buggy binutils issue
  • Certificate matches organization

    • Subject: = Blancco Secure Boot CA 2022, OU = Secure Boot, O = Blancco Technology Group IP Oy, C = FI
    • Valid till: Nov 8 12:06:43 2032 GM (10 years)
    • Is CA certificate and has Code Signing and Digital Signature set
  • Keys are stored in an HSM

  • GRUB2

    • Based on Fedora downstream
    • Because you are using the downstream Fedora version of GRUB2 please also keep their SBAT entry i.e. grub.rh,2,Red Hat,grub2,@@VERSION_RELEASE@@,mailto:secalert@redhat.com (https://src.fedoraproject.org/rpms/grub2/blob/rawhide/f/sbat.csv.in)
    • There were recently some issues with the ntfs module that also increased the GRUB2 SBAT version. Can you update to a snapshot that has those issues fixed and set the upstream GRUB2 version to 4?
    • Module list looks fine: all_video boot chain configfile echo fat font gfxmenu gfxterm gfxterm_background halt iso9660 http jpeg linux loopback memdisk normal ntfs part_apple part_gpt part_msdos png reboot search test tftp true video efinet tpm
  • Kernel

    • 6.2.16 with lockdown patches

Questions/Issues

  • Old tag and hashes need to be replaced in the top comment of the issue
  • Because you are using the downstream Fedora version of GRUB2 please also keep their SBAT entry i.e. grub.rh,2,Red Hat,grub2,@@VERSION_RELEASE@@,mailto:secalert@redhat.com (https://src.fedoraproject.org/rpms/grub2/blob/rawhide/f/sbat.csv.in)
  • There were recently some issues with the ntfs module that also increased the GRUB2 SBAT version. Can you update to a snapshot that has those issues fixed and set the upstream GRUB2 version to 4?
  • We recently added a question how kernel modules are signed (Add question about how kernel modules are signed #337). Can you add this to your submission?

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

evilteq commented Nov 1, 2023

Thanks for reviewing!

  • Original post was updated.
  • That grub was made a year ago when this review was made. We will take the current one when this moves on, and update the SBAT accordingly.
  • We use ephemeral keys generated on each kernel build. The fork review was updated to add that.

@THS-on
Copy link
Collaborator

THS-on commented Nov 1, 2023

That grub was made a year ago when this review was made. We will take the current one when this moves on, and update the SBAT accordingly.

Can you make an update to GRUB2 and include it with the SBAT in the review just to document it?

Otherwise this LGTM! If @steve-mcintyre confirms the contact verification and finishes the review, this should be ready to go.

@THS-on THS-on added extra review wanted Initial review(s) look good, another review desired and removed bug Problem with the review that must be fixed before it will be accepted labels Nov 1, 2023
@steve-mcintyre
Copy link
Collaborator

Yes, contact verification looks fine. I can't find any mention of the labels for that now which is odd :-(

Happy to mark this approved if we can see a new GRUB2 build.

@evilteq
Copy link

evilteq commented Nov 3, 2023

NTFS patches are still not applied in their repository. Just for a quick test, I've applied them manually and everything seems fine.
We will append our line to the sbat after theirs.
Meanwhile we would like to start QA testing ASAP and we still don't know how long its going to take for the MS signing itself. The shim itself is not going to change anyway.

@THS-on
Copy link
Collaborator

THS-on commented Nov 8, 2023

NTFS patches are still not applied in their repository. Just for a quick test, I've applied them manually and everything seems fine.

Yes indeed. Just apply them once they are in their repo or applied as patches in Fedora.

@evilteq can you add the SBAT of a current GRUB2 build with upstream line appended to the review, for documentation
purposes? Once done, I can mark it as ready because as you mentioned the shim itself does not change.

@THS-on THS-on removed the extra review wanted Initial review(s) look good, another review desired label Nov 8, 2023
@evilteq
Copy link

evilteq commented Nov 13, 2023

We just updated the review repo with today's tag with the SBAT of a test build of fedora + ntfs cve patches.

@THS-on
Copy link
Collaborator

THS-on commented Nov 14, 2023

LGTM, marking as accepted. Just make sure that when you are building a GRUB2 with SBAT level 4 to include ntfs patches, because as mentioned the git branch currently does not include them.

@THS-on THS-on added accepted Submission is ready for sysdev and removed question Reviewer(s) waiting on response labels Nov 14, 2023
@dpedigo
Copy link
Author

dpedigo commented Dec 7, 2023

Thank you very much everyone, we've received the signed shims from Microsoft.

@dpedigo dpedigo closed this as completed Dec 7, 2023
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
Projects
None yet
Development

No branches or pull requests

8 participants