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.8 for Isoo (20240323) #390

Closed
8 tasks done
haobinnan opened this issue Feb 28, 2024 · 56 comments
Closed
8 tasks done

shim-15.8 for Isoo (20240323) #390

haobinnan opened this issue Feb 28, 2024 · 56 comments
Assignees
Labels
accepted Submission is ready for sysdev contacts verified OK Contact verification is complete here (or in an earlier submission)

Comments

@haobinnan
Copy link

haobinnan commented Feb 28, 2024

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/haobinnan/shim-review/tree/isoo-shim-20240323


What is the SHA256 hash of your final SHIM binary?


shimia32.efi.sha256sum: a0241fc871a04202815b54a54fefb7943b1e284ded07e8327d85a1948ba50c79
shimx64.efi.sha256sum: fadcacd698dd6d6828e576228e3be6e0845c0f80b1de0a961c6fdbf6a1a63ec4


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


#338

@haobinnan
Copy link
Author

@dennis-tseng99
Can the submission be reviewed

@dennis-tseng99 dennis-tseng99 self-assigned this Feb 28, 2024
@MuthuvelKuppusamy
Copy link

SBAT for grub2:
sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md
grub,3,Free Software Foundation,grub,2.12,https://www.gnu.org/software/grub/
grub.ubuntu,1,Ubuntu,grub2,2.12-1ubuntu3,https://www.ubuntu.com/
grub.isoo,1,Isoo,grub2,2.12-isoo,https://www.isoo.com/

image

sbat for grub is 3 (or) 4 ?

@haobinnan
Copy link
Author

The grub2 I used to use was https://git.launchpad.net/ubuntu/+source/grub2/tree/?h=import/2.06-14

This time it has updated to https://git.launchpad.net/ubuntu/+source/grub2/tree/?h=import/2.12-1ubuntu3

The 2.06-14 is 4

The 2.12-1ubuntu3 is 3 for the moment

May I know if this make any difference?

@haobinnan
Copy link
Author

The current builds include the grub,3 fixes

@SherifNagy
Copy link
Collaborator

I am not an official reviewer, and haven't looked very deeply into the review yet, but here are couple of things:

  • If you ship NTFS modules for grub, which seems like you do, your grub needs to be patches for NTFS CVEs
  • When those CVEs patches apply, your group SBAT entry must have grub,4
  • You must revoke older grub2 binaries effected by the NTFS CVEs and there are few ways to do that:
    • add the grub2 hashes into dbx and build shim with this dbx
    • patch shim to revoke grub,4 since current shim 15.8 only revoke grub,3

Either way of those, you will need to rebuild shim

@dennis-tseng99
Copy link
Collaborator

dennis-tseng99 commented Mar 3, 2024

@haobinnan Hi, I saw you only a patch file. That is good. Instead of using /* ... */ to avoid debug msg from perror(), may I suggest that you could make use of :

in_protocol = 1;
........
........
in_protocol = 0;

Just like other shim-15.8 functions ? e.g. shim_hash()

BTW, generation number of grub is another question.
Thanks.

@dennis-tseng99 dennis-tseng99 removed their assignment Mar 4, 2024
@dennis-tseng99 dennis-tseng99 added the question Reviewer(s) waiting on response label Mar 4, 2024
@haobinnan haobinnan changed the title shim-15.8 for Isoo (20240228) shim-15.8 for Isoo (20240304) Mar 4, 2024
@haobinnan
Copy link
Author

@dennis-tseng99
Can the submission be reviewed

@haobinnan
Copy link
Author

@SherifNagy
Copy link
Collaborator

@haobinnan how are you revoking older grub2 with NTFS CVEs?

@steve-mcintyre
Copy link
Collaborator

@haobinnan still need to know what you're doing with older grub2.

@steve-mcintyre steve-mcintyre added the bug Problem with the review that must be fixed before it will be accepted label Mar 11, 2024
@haobinnan
Copy link
Author

@haobinnan still need to know what you're doing with older grub2.

I'm not using older grub2.

I was using older grubs(https://git.launchpad.net/ubuntu/+source/grub2/tag/?h=import/2.06-14), but now in the shim here I'm using the latest grubs which is https://git.launchpad.net/ubuntu/+source/grub2/tag/?h=import/2.12-1ubuntu5

If import/2.12-1ubuntu5 does not comply with safety regulations, I can change it to import/2.06-14,as the previously reviewed shim(https://github.com/rhboot/shim-review/issues/338)used import/2.06-14

@haobinnan haobinnan changed the title shim-15.8 for Isoo (20240304) shim-15.8 for Isoo (20240313) Mar 13, 2024
@haobinnan
Copy link
Author

@dennis-tseng99 @steve-mcintyre
Can the submission be reviewed

@haobinnan
Copy link
Author

@SherifNagy
Copy link
Collaborator

@haobinnan the question is about the "current" grub in your release / distro that is in the market with the NTFS CVEs, how are you revoking those.

Based on this: #246 you already had shim signed, and grub2 with NTFS modules signed as well, which means, you have grub2 out there in the market that has the NTFS CVEs and you need to make sure that your new SHIM "15.8" will revoke those binaries with the CVEs.

The way I see it, you have one of the following options:

  • Patch shim 15.8 to prevents grub,3 from loading and keep your grub at grub,4
  • Add all your already released grub2 hashes which has the NTFS CVE into dbx and compile shim with this, I already saw you have DBX in place, so maybe that option would make more sense

Also now Ubuntu includes grub.peimage in their sbat for grub, and since you are downstream from ubuntu, I think you need to retain this record as well in your sbat

@haobinnan
Copy link
Author

The current content of my grub2 sbat is as follows:
sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md
grub,4,Free Software Foundation,grub,2.06,https://www.gnu.org/software/grub/
grub.ubuntu,1,Ubuntu,grub2,2.06-2ubuntu17.2,https://www.ubuntu.com/
grub.isoo,1,Isoo,grub2,2.06-isoo,https://www.isoo.com/

If patches are added, will my grub2 sbat content meet the requirements?
Patch shim 15.8 to prevent grub,3 from loading and keep your grub at grub,4
Are there some cases for this method? I feel that this solution is better and safer.

May I know if my shim can be accepted if I perfect these operations?

@SherifNagy

@SherifNagy
Copy link
Collaborator

SherifNagy commented Mar 14, 2024

@haobinnan the folks from ALT Linux do have a patch for shim to do that and that is okay with the reviewers

And check https://github.com/canonical/shim-review/tree/ubuntu-shim-amd64+arm64-20240202 for their current sbat or the actual source code of the grub you are building

@haobinnan haobinnan changed the title shim-15.8 for Isoo (20240313) shim-15.8 for Isoo (20240314) Mar 14, 2024
@haobinnan
Copy link
Author

@dennis-tseng99
Copy link
Collaborator

@haobinnan Although grub2 is still controversial, let's check others.

Patch shim 15.8 to prevent grub,3 from loading and keep your grub at grub,4

Please let me list part of sbat codes for your reference:

verify_single_entry(......)
{
                ...........
                sbat_gen = atoi((const char *)entry->component_generation);
		sbat_var_gen = atoi((const char *)sbat_var_entry->component_generation);

		if (sbat_gen < sbat_var_gen) {
			....
                       return EFI_SECURITY_VIOLATION;
		}
	}
	return EFI_SUCCESS;
}

=== Review for Isoo (20240228) #390 ===

  • Binaries are producible based on tag isoo-shim-20240314 (ok)
  • Hash values are matched (ok)
  • NX flag is disable: (ok)

objdump -x shimx64.efi | grep -E 'SectionAlignment|DllCharacteristics'
SectionAlignment 00001000
DllCharacteristics 00000000

objdump -x shimia32.efi | grep -E 'SectionAlignment|DllCharacteristics'
SectionAlignment 00001000
DllCharacteristics 00000000

  • There are 2 patches but no any description. Please state "why" on your README.

What patches are being applied and why:
ignore-print.patch
shim-15.8-alt-Bump-grub-SBAT-revocation-to-4.patch

  • shim sbat (ok)

objcopy --only-section .sbat -O binary ./shim_orig/shimx64.efi /dev/stdout
sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md
shim,4,UEFI shim,shim,1,https://github.com/rhboot/shim
shim.isoo,1,Isoo,shim,15.8,https://www.isoo.com/

objcopy --only-section .sbat -O binary shimia32.efi /dev/stdout
sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md
shim,4,UEFI shim,shim,1,https://github.com/rhboot/shim
shim.isoo,1,Isoo,shim,15.8,https://www.isoo.com/

  • Certificate Validity: (ok)

openssl x509 -in CertFile.cer -inform der -noout -text
Validity
Not Before: Jun 13 06:09:33 2022 GMT
Not After : Jun 12 06:09:33 2052 GMT

30 years seems too long, but is acceptable.
Its size is 4096 bits (ok)

  • suggestion:
    For the next submission, please specify more details about the contact information for your product when you specify vendor_url field, or just use email instead. For example, you might change to:
    shim.isoo,1,Isoo,shim,15.8,https://www.isoo.com/packages/shim

  • need more experts to help this release

@haobinnan
Copy link
Author

@dennis-tseng99
shim.isoo,1,Isoo,shim,15.8,https://www.isoo.com/
grub.isoo,1,Isoo,grub2,2.06-isoo,https://www.isoo.com/
May I know if it is okay if I change https://www.isoo.com/ to [email protected] when I submit next time?

ignore-print.patch:
This patch ignores some warning messages from print output, for example: rhboot/shim#506

shim-15.8-alt-Bump-grub-SBAT-revocation-to-4.patch:
This patch is mainly used to prevent grub with sbat less than 4 from running.

@SherifNagy
Copy link
Collaborator

We are looking int the shim-15.8-alt-Bump-grub-SBAT-revocation-to-4.patch , there might be a better way to do this, we will get back to you as soon as we figure it out.

In the mean time, we see you don't have the grub.peimage entry in your grub SBAT, I will request input from @julian-klode and / or @kukrimate to look at the submission as well

@haobinnan
Copy link
Author

@SherifNagy
Thank you very much for your quick response! May I know what you mean by “grub.peimage entry in your grub SBAT”? Would it be possible to tell more details or some samples so that I can fix the issue? Thank you!

@julian-klode
Copy link
Collaborator

julian-klode commented Mar 19, 2024

The following SBAT entry needs to be preserved:

grub.peimage,1,Canonical,grub2,@DEB_VERSION@,https://salsa.debian.org/grub-team/grub/-/blob/master/debian/patches/secure-boot/efi-use-peimage-shim.patch

This was added in 2.12~rc1-1 to be able to revoke the new loader component shared by Debian and Ubuntu in case of vulnerabilities.

if grub binaries from the 2.12~rc1 or later, derived from Debian or Ubuntu, have been signed already as grub.isoo, you will also need to do a revocation for grub.isoo as they can't be revoked using grub.peimage if there is a vulnerability in the peimage component - which is the shared loader code between Debian and Ubuntu starting in 2.12~rc1.

@kukrimate
Copy link
Contributor

If you use a GRUB containing the peimage module, you need the peimage sbat entry in the GRUB.

@SherifNagy
Copy link
Collaborator

@haobinnan we will get to it in turn, we are volunteers and we are going through all the reviews, please be patient.

@haobinnan
Copy link
Author

May I know if the BUG tag can be removed? Would it be possible to review my shim? It's been a little long time. Thank you very much!

@haobinnan
Copy link
Author

Can the submission be reviewed

@steve-mcintyre
Copy link
Collaborator

@haobinnan other things look OK, but it's still not clear to me exactly what versions of GRUB2 you're using now and what you have shipped in the past. Could you please give us a list of the following data for all the signed versions of GRUB2 you have released:

  • GRUB2 version
    • SBAT data (for GNU upstream, for any parent distro and from Isoo)
    • CVE fixes applied

This list should include any development versions that anybody might have been able to download (e.g. 2.12-based builds with peimage etc.).

What do you expected your next planned signed GRUB2 build is going to look like?

Apologies if this seems like a lot of work - we have to be thorough here.

@haobinnan
Copy link
Author

haobinnan commented Apr 23, 2024

The GRUB2 version I'm using now is grub2.06: https://git.launchpad.net/ubuntu/+source/grub2/tag/?h=import/2.06-14
I also add this patch: https://launchpad.net/ubuntu/+archive/primary/+sourcefiles/grub2-unsigned/2.06-2ubuntu14.4/grub2-unsigned_2.06-2ubuntu14.4.debian.tar.xz, as this patch fixes the NTFS security vulnerability.

Here are the history of signed shims I've released, and I hope you can find what you want:
#17
https://github.com/haobinnan/shim-review/tree/isoo-shim-20210423-2
https://github.com/haobinnan/shim-review/tree/isoo-shim-20210809
https://github.com/haobinnan/shim-review/tree/isoo-shim-20220802
https://github.com/haobinnan/shim-review/tree/isoo-shim-20231226

For the next planned signed GRUB2 build, I might use grub2.12

Let me know if you need more informaiton please.

May I know the suggested version of grub2 here? I can change to the suggested version so that the review process can be accepted as soon as possible. Thank you very much!

@steve-mcintyre

@haobinnan
Copy link
Author

Can the submission be reviewed

@haobinnan
Copy link
Author

Can the submission be reviewed
@steve-mcintyre

@haobinnan
Copy link
Author

Can the submission be reviewed

@steve-mcintyre
Copy link
Collaborator

Picking this up again - sorry for the delay, volunteer effort can take a while.

Although we've accepted previous submissions, I don't think we've ever done a formal contact verification step yet. Let's fix that. I've just sent mails to both contacts.

@steve-mcintyre steve-mcintyre added the contact verification pending Contact verification emails have been sent, waiting on response label May 29, 2024
@haobinnan
Copy link
Author

Picking this up again - sorry for the delay, volunteer effort can take a while.

Although we've accepted previous submissions, I don't think we've ever done a formal contact verification step yet. Let's fix that. I've just sent mails to both contacts.

Every time you use Tcl, God kills a kitten.

@haobinnan
Copy link
Author

Picking this up again - sorry for the delay, volunteer effort can take a while.

Although we've accepted previous submissions, I don't think we've ever done a formal contact verification step yet. Let's fix that. I've just sent mails to both contacts.

[email protected]

I suspect most samba developers are already technically insane... Of
course, since many of them are Australians, you can't tell.

@steve-mcintyre
Copy link
Collaborator

I'd like to see the random words too, not my .sig lines

@haobinnan
Copy link
Author

I'd like to see the random words too, not my .sig lines

Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hi!

Please quote the following words in

#390

to confirm your identity:

tiredness beast stuffiest Townsend bushed wannest flatfoot lighten decontam=
inates Matilda

--=20
Steve McIntyre, Cambridge, UK. steve@einval.=
com
"I suspect most samba developers are already technically insane... Of
course, since many of them are Australians, you can't tell." -- Linus Torv=
alds


Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hi!

Please quote the following words in

#390

to confirm your identity:

phrenology sweat Bethany prognosticates Tarkington osmosis masculine Goldma=
n blooming Edgardo

--=20
Steve McIntyre, Cambridge, UK. steve@einval.=
com
"Every time you use Tcl, God kills a kitten." -- Malcolm Ray

@steve-mcintyre steve-mcintyre added contacts verified OK Contact verification is complete here (or in an earlier submission) and removed contact verification pending Contact verification emails have been sent, waiting on response labels May 29, 2024
@haobinnan
Copy link
Author

@steve-mcintyre
Can it pass the review?

@steve-mcintyre
Copy link
Collaborator

Please don't pester - there's more work for me to do yet

@steve-mcintyre
Copy link
Collaborator

Review of shim-15.8 for Isoo (20240323)

OK

  • Contact verification done - OK
  • shim builds reproduce here for x64 and ia32 - OK
  • key management using an HSM - OK
  • Kernel modules signed with ephemeral key - OK
  • NX bit not set - OK
  • Builds from 15.8 upstream - OK
    • with 2 patches applied:
      • ignore-print.patch
        Simply disables some error message printing in shim.c. Clearly
        no effect on security
      • shim-15.8-alt-Bump-grub-SBAT-revocation-to-4.patch
        Simple patch to bump revocations for old grub binaries, fine
  • Includes a CA key expiring in 2052 - very long, but OK
  • List of grub modules looks OK.
    • Includes ntfs, so needs SBAT for GRUB bumped to 4
    • Checking grub some more
  • kernel 6.1.80 with lockdown patches - OK
  • SBAT bumped for shim and GRUB to revoke old binaries, and older
    signing certs are blocked by switching to a new CA in shim - OK.
  • Shim loads gruband Linux only - OK
  • SBAT data looks OK for shim - OK

Issues / queries

I can't find enough information about your GRUB packages to be
completely sure that things are OK. I've spent a lot of time and dug through a
lot of history around your shim review submissions and still can't
find complete information about versions of GRUB that have been signed
and released by you. Asking more directly:

In version isoo-shim-20240304 of your submission you mentioned a
GRUB 2.12 build based on Ubuntu's 2.12-1ubuntu4 package. Your SBAT
there did not include a grub.peimage SBAT line that would be
required. The version information for that GRUB build is just given as
2.12-isoo, which give us very little information. Was that specific
version of GRUB ever released by you?

In submission version isoo-shim-20240313, you moved back to a
version of GRUB based on 2.06, using Ubuntu's 2.06-2ubuntu17 (from
their Lunar release). In isoo-shim-20240314, based on feedback from
@kukrimate, you switched to 2.06-2ubuntu14.4. The version that you
say you're using in both cases is just listed as grub2,2.06-isoo in
SBAT, which makes it difficult to verify what's happened.

We can't see your GRUB sources to take a look directly here, hence I'm
asking more questions. When we review shim submissions, we're much
more used to people shipping a Linux distribution than Linux-based
tools.

In the common Linux distribution model, the distribution will build
and ship multiple incremental versions of each signed package (GRUB,
kernel, etc.). End users can download many versions of these packages,
and this is why we are very careful to check that different package
versions are tracked and revoked via SBAT where necessary.

From conversation so far, I get the impression that this is very
different to the release model that Isoo is following for Linux-based
tools. Could you please explain a little how your normal release
process looks to help here? Do you ship different packages that end
users may download and use? Or is each release a complete image that
that people download and use as-is? With each generation of your
product, do you release beta versions (etc.) that might include
differing binaries of shim, GRUB and the Linux kernel?

Basically, I'm trying to get a sense of exactly what GRUB binaries you
have shipped in any way that might need revocation. Apologies if
this might sound too rigid, but right now I can't be sure - hence all
the extra questions.

@haobinnan
Copy link
Author

Issue:
In version isoo-shim-20240304 of your submission you mentioned a
GRUB 2.12 build based on Ubuntu's 2.12-1ubuntu4 package. Your SBAT
there did not include a grub.peimage SBAT line that would be
required. The version information for that GRUB build is just given as
2.12-isoo, which give us very little information. Was that specific
version of GRUB ever released by you?

Response:
Never officially released


Issue:
In submission version isoo-shim-20240313, you moved back to a
version of GRUB based on 2.06, using Ubuntu's 2.06-2ubuntu17 (from
their Lunar release). In isoo-shim-20240314, based on feedback from
@kukrimate, you switched to 2.06-2ubuntu14.4. The version that you
say you're using in both cases is just listed as grub2,2.06-isoo in
SBAT, which makes it difficult to verify what's happened.

Response:
I only used the upstream GRUB (Ubuntu) code without making any modifications. Below is the complete address for the GRUB2 code.

GRUB2 version: 2.06-2ubuntu14.4
https://git.launchpad.net/ubuntu/+source/grub2/tag/?h=import/2.06-14
2.06-2ubuntu14.4 -- https://launchpad.net/ubuntu/+archive/primary/+sourcefiles/grub2-unsigned/2.06-2ubuntu14.4/grub2-unsigned_2.06-2ubuntu14.4.debian.tar.xz
No extra patches


Issue:
From conversation so far, I get the impression that this is very
different to the release model that Isoo is following for Linux-based
tools. Could you please explain a little how your normal release
process looks to help here? Do you ship different packages that end
users may download and use? Or is each release a complete image that
that people download and use as-is? With each generation of your
product, do you release beta versions (etc.) that might include
differing binaries of shim, GRUB and the Linux kernel?

Response:
Each release is a complete image, and we do not release beta versions of shim, GRUB, or the Linux kernel.

,,,,,,,,,,,,,,,,,,,,

I understand that this has added to your workload, and I deeply apologize for any inconvenience this has caused.

@steve-mcintyre

@steve-mcintyre
Copy link
Collaborator

steve-mcintyre commented Jun 10, 2024

Thanks for clarifying!

To make things easier in future, it would be helpful if you could share more details about what you're doing in a few places. Giving your GRUB builds distinct versions (e.g. 2.06-isoo2, 2.12-isoo7) would be nice, for example. Please share info on your product releases so we don't have to spend as much time digging, e.g. something like:

Isoo release shim version grub version linux version
1.0 15.8 2.06-isoo (from ubuntu 2.06-XXX.1) 6.1.80
1.1 15.9 2.12-isoo (from ubuntu 2.12-XXX.2) 6.9.ZZ

Finally: as you go forwards to grub 2.12, be sure to pick up the grub.peimage SBAT entry too if you're basing on either Debian or Ubuntu.

@steve-mcintyre steve-mcintyre removed bug Problem with the review that must be fixed before it will be accepted question Reviewer(s) waiting on response labels Jun 10, 2024
@steve-mcintyre
Copy link
Collaborator

For now, I think we've had enough review on this submission. Accepted.

@steve-mcintyre steve-mcintyre added the accepted Submission is ready for sysdev label Jun 10, 2024
@haobinnan
Copy link
Author

Thank you for helping me pass the review. I'll be sure to pay more attention to details in the future to facilitate the review process for reviewers. Thanks again!

@steve-mcintyre

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 contacts verified OK Contact verification is complete here (or in an earlier submission)
Projects
None yet
Development

No branches or pull requests

7 participants