search

rhboot / shim-review

Reviews of shim
66 stars 130 forks source link

Shim 15.8 for ECOS Technology GmbH #423

Closed richterger closed 1 month ago

richterger commented 6 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/ecos-platypus/shim-review/tree/ECOS_Technology_GmbH-shim-x64-20240528

What is the SHA256 hash of your final SHIM binary?

cfb155df60992a5cee2dff99d75089ee03a578d2d01e7d30b7cf5fc1c67da3b0

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

https://github.com/ecos-platypus/shim-review/tree/ECOS_Technology_GmbH-shim-x64-20220628

If no security contacts have changed since verification, what is the link to your request, where they've been verified (if any, otherwise N/A)?

christoph.stolz@ecos.de is a new contact gerald.richter@ecos.de was already verified in #243 https://github.com/ecos-platypus/shim-review/blob/ECOS_Technology_GmbH-shim-x64-20220628/pgp/gerald.richter.asc

tSU-RooT commented 6 months ago

Disclaimer: I'm not authorized reviewer. I am quite reffering to previous accepted review. https://github.com/rhboot/shim-review/issues/243

Shim

Grub2

New Certificate

steve-mcintyre commented 6 months ago

christoph.stolz@ecos.de is a new contact gerald.richter@ecos.de was already verified in https://github.com/rhboot/shim-review/issues/243

Sending verification mail to Christoph now...

stolzchr commented 5 months ago

Contact verification: moisture moors shunt customs camellias sustains pane oyster Hanna maintainer

richterger commented 3 months ago

Hi, is there anything we can do to support the review process of our shim? Thanks & Regards Gerald

THS-on commented 3 months ago

@richterger we are working through the queue. It helps us, if you can look at other submissions and review them, as this process should be a community effort and most of us are doing this in our spare time. Looking at the submission with the "easy to review" or "extra reviewer wanted" tag is a good starting point.

es-fabricemarie commented 3 months ago

Questions:

richterger commented 3 months ago

The modifications to shim/grub we made are the excatly the same just adjusted for the newer shim and grub2. No changes at all in the way it works.

richterger commented 3 months ago

@richterger we are working through the queue. It helps us, if you can look at other submissions and review them, as this process should be a community effort and most of us are doing this in our spare time.

I understand the problem of "doing this in our spare time". I know this from other projects I am doing as well.

It took me some time to look thru the docs for reviewers and other reviews (I also have to do it my spare time), but now I was able to do a few reviews (#430, #435, #436). I try to do some more. If you have any hints what can be done better or should be done different or were to have a deeper look, let me know, so I can keep it in mind for the next reviews I am doing.

BTW: In #345 @steve-mcintyre mentioned @ecos-platypus as one who done a lot reviews. He is the one, togerther with myself, who designed and implemented the patches for shim and grub, that were approved in our last shim review (and included unmodified in the current review as well). Unfortunately he left our company, so the call for helping with reviews, never reached him. His github account he used for ecos was retired and I only use his repository to be able to provide the history for this review.

aronowski commented 2 months ago

Thank you for the patience. I did take a look, and, good for me, the patches have already been reviewed as part of the earlier application. I still have some things on my mind, though.

The whole chain should be loaded only as part of an image carried on that stick. Then there should be no possibility of booting anything else anyway. I'm wondering if it's possible to load an older, vulnerable kernel module from an already booted system. It would be awesome if there was already an ephemeral key usage implemented.

I appreciate the help with reviewing other applications. I'll ask around during the next call for additional support. Please, ping me if no answer arrives by the end of next Monday.

richterger commented 2 months ago

Regarding the question about loading an older, vulnerable kernel module. That is not possible, because module and kernel are build with modversions and vermagic . From the modprobe manpage:

"Every module contains a small string containing important information, such as the kernel and compiler versions. If a module fails to load and the kernel complains that the "version magic" doesn't match, ..."

Every kernel we build has a unique version number, which is part of the module vermagic. The vermagic in a module looks like this:

vermagic=6.2.16-BB5000-sbs64-R17 SMP preempt mod_unload modversions

So if we build a new kernel, either the kernel version changes or we change the R number. Without enforceing signed modules it would be still possible to try to load the module if the vermagic doesn't match the runing kernel, by using the --force option, but with signed modules and signing enforced this isn't possible. See kernel/module/signing.c in the kernel source, in function module_sig_check you find:

bool mangled_module = flags & (MODULE_INIT_IGNORE_MODVERSIONS |
                       MODULE_INIT_IGNORE_VERMAGIC);
    /*
     * Do not allow mangled modules as a module with version information
     * removed is no longer the module that was signed.
     */
    if (!mangled_module &&
    ...

This and ths following code enforces that, if module signing is enforced (which is the case in our kernels), the function will return EKEYREJECTED in case one of the flags MODULE_INIT_IGNORE_MODVERSIONS or MODULE_INIT_IGNORE_VERMAGIC are set. So there is no possibility to load a module, if vermagic doesn't match.

By just giving it a try, it shows that this is true.

Trying to load a module from a different kernel version, with --force:

# strace modprobe --force -S 6.1.43-BB5000-sbs64-R17 sha3_generic
...
finit_module(3, "", MODULE_INIT_IGNORE_MODVERSIONS|MODULE_INIT_IGNORE_VERMAGIC) = -1 EKEYREJECTED (Key was rejected by service)

Trying to load a module from a different kernel without --force:

# strace modprobe  -S 6.1.43-BB5000-sbs64-R17 sha3_generic
...
finit_module(3, "", 0)                  = -1 ENOEXEC (Exec format error)

Also an ephemeral key would add an extra security, in case of any bugs in the kernel, in the way the above shown code works, it is already now only possible to load modules, that exactly match the running kernel and no other (vulnerable) kernel modules can be loaded, reagardless if they are signed by us in the past or not.

Also there is no access to the linux system in neither way for the user of our stick. He/She just has a GUI to use some predefined options. In addition there are a lot of other security measures that makes sure that it is not possible, no software (neither kernel module or other binary) can be executed, that is not part of our image and signed by us.

richterger commented 2 months ago

@aronowski wrote: Please, ping me if no answer arrives by the end of next Monday.

Any news here?

aronowski commented 2 months ago

@richterger, looking good! Accepting.

richterger commented 2 months ago

@aronowski Thank you very much for spending your time doing the reviews.

richterger commented 1 month ago

@aronowski Thank you very much for spending your time doing the reviews.