search

rhboot / shim-review

Reviews of shim
67 stars 131 forks source link

shim-15.8 for Isoo (20240323) #390

Closed haobinnan closed 3 months ago

haobinnan commented 7 months ago

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

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)?

https://github.com/rhboot/shim-review/issues/338

haobinnan commented 7 months ago

@Dennis-Tseng99 Can the submission be reviewed

MuthuvelKuppusamy commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

The current builds include the grub,3 fixes

SherifNagy commented 7 months ago

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

Either way of those, you will need to rebuild shim

dennis-tseng99 commented 7 months ago

@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.

haobinnan commented 7 months ago

@dennis-tseng99 Can the submission be reviewed

haobinnan commented 7 months ago

https://github.com/haobinnan/shim-review/tree/isoo-shim-20240304

SherifNagy commented 7 months ago

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

steve-mcintyre commented 7 months ago

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

haobinnan commented 7 months ago

@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 commented 7 months ago

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

haobinnan commented 7 months ago

https://github.com/haobinnan/shim-review/tree/isoo-shim-20240313

SherifNagy commented 7 months ago

@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:

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 commented 7 months ago

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 commented 7 months ago

@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 commented 7 months ago

@SherifNagy

https://github.com/haobinnan/shim-review/tree/isoo-shim-20240314

dennis-tseng99 commented 7 months ago

@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 ===

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

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/

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

haobinnan commented 7 months ago

@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 haobinnan@gmail.com when I submit next time?

ignore-print.patch: This patch ignores some warning messages from print output, for example: https://github.com/rhboot/shim/issues/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 commented 7 months ago

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 commented 7 months ago

@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 commented 7 months ago

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 commented 7 months ago

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

haobinnan commented 7 months ago

The grub2 version I am using now is 2.06, not 2.12. In this case, is it possible not to add grub.peimage? I found that grub.peimag is only available in 2.12.

kukrimate commented 7 months ago

The review seems to have said:

The signed bootloaders are derived from grub 2.12 with all of the relevant patches.

julian-klode commented 7 months ago

Lots of issues here:

haobinnan commented 7 months ago

@kukrimate

I made an omission here, I probably forgot that. Thank you very much for pointing it out! It should be 2.06.

https://github.com/haobinnan/shim-review/tree/isoo-shim-20240321

haobinnan commented 7 months ago

@julian-klode

The version I used before was import/2.06-14, but this one was too old and had bugs, so I changed to 2.06-2ubuntu17.2.

The 2.06-2ubuntu17.2 fixes many bugs and includes ntfs bug patches. That's why I use 2.06-2ubuntu17.2

https://github.com/haobinnan/shim-review/tree/isoo-shim-20240321

kukrimate commented 7 months ago

The currently maintained branch of grub 2.06 in ubuntu is 2.06-2ubuntu14.5, not 2.06-2ubuntu17.2.

haobinnan commented 7 months ago

@kukrimate I downloaded the patch code from here: https://launchpad.net/ubuntu/+source/grub2-unsigned/2.06-2ubuntu17.2 May I know if it is possible to tell me a URL that has the code library to fix the ntfs bug?

haobinnan commented 7 months ago

image

kukrimate commented 7 months ago

@haobinnan

Please look at grub2-unsigned (2.06-2ubuntu17.2) lunar; urgency=high, it says lunar in this line which refers to a now unsupported interim release of Ubuntu.

If you look on this page: https://launchpad.net/ubuntu/+source/grub2-unsigned/ The latest supported version of GRUB 2.06 is contained in Jammy Jellyfish and is 2.06-2ubuntu14.4, and there is a pending upload for the same series 2.06-2ubuntu14.5

haobinnan commented 7 months ago

@kukrimate https://github.com/haobinnan/shim-review/tree/isoo-shim-20240323

haobinnan commented 7 months ago

https://github.com/haobinnan/shim-review/tree/isoo-shim-20240323

haobinnan commented 7 months ago

Can the submission be reviewed

haobinnan commented 6 months ago

Can the submission be reviewed

SherifNagy commented 6 months ago

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

haobinnan commented 6 months ago

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 commented 6 months ago

Can the submission be reviewed

steve-mcintyre commented 6 months ago

@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:

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 commented 6 months ago

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: https://github.com/rhboot/shim-review/issues/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 commented 5 months ago

Can the submission be reviewed

haobinnan commented 5 months ago

Can the submission be reviewed @steve-mcintyre

haobinnan commented 5 months ago

Can the submission be reviewed

steve-mcintyre commented 4 months ago

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.

haobinnan commented 4 months ago

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 commented 4 months ago

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.

liusirjiayou@126.com

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

steve-mcintyre commented 4 months ago

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

haobinnan commented 4 months ago

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

https://github.com/rhboot/shim-review/issues/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

https://github.com/rhboot/shim-review/issues/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

haobinnan commented 4 months ago

@steve-mcintyre Can it pass the review?