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 ECOS Technology GmbH #423

Closed
8 tasks done
richterger opened this issue May 28, 2024 · 14 comments
Closed
8 tasks done

Shim 15.8 for ECOS Technology GmbH #423

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

Comments

@richterger
Copy link

richterger commented May 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/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)?


[email protected] is a new contact
[email protected] 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
Copy link

Disclaimer: I'm not authorized reviewer.
I am quite reffering to previous accepted review.
#243

Shim

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

Grub2

New Certificate

  • Storing in FIPS-140-2 Token is OK.
  • 3-years certificate is OK as I know.
$ openssl x509 -inform der -in ECOS_Technology_GmbH_Code_Sign_24.cer -text -noout
Certificate:
    Data:
        Version: 3 (0x2)
        Serial Number:
            06:2f:83:49:c4:5c:13:d4:f2:a7:f6:e1
        Signature Algorithm: sha256WithRSAEncryption
        Issuer: C = BE, O = GlobalSign nv-sa, CN = GlobalSign GCC R45 EV CodeSigning CA 2020
        Validity
            Not Before: May  1 08:28:50 2024 GMT
            Not After : May  2 08:28:50 2027 GMT
[...]

@steve-mcintyre
Copy link
Collaborator

[email protected] is a new contact
[email protected] was already verified in #243

Sending verification mail to Christoph now...

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

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

@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 31, 2024
@richterger
Copy link
Author

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

@THS-on
Copy link
Collaborator

THS-on commented Jul 30, 2024

@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
Copy link

  • security contacts PGP keys are in keyserver, and do not expire.
  • NX bit not set for now.
  • shim EFI image build reproduce properly
    cfb155df60992a5cee2dff99d75089ee03a578d2d01e7d30b7cf5fc1c67da3b0  shimx64.efi
    
  • cert embedded is
    • an EV Code Signing cert from GlobalSign
    • valid until May 2027 (almost 3 years)
  • sbat entries looks ok to me

Questions:

  • Based on 15.8 but lots of patches in both shim and in grub2. More reviews needed
    • but shim was signed before. Changes listed are:
      • We updated shim from version 15.6 to the current release 15.8

      • We updated grub from version 2.06 to grub 2.12

      • We have a new EV certificate which is build into shim and used to sign grub

      • We use newer kernels

    • Does this mean that the shim/grub modifications you made are the same (just adjusted) for the newer shim and grub2? or you changed the way it works?

@richterger
Copy link
Author

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
Copy link
Author

richterger commented Aug 19, 2024

@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 aronowski self-assigned this Aug 28, 2024
@aronowski
Copy link
Collaborator

aronowski commented Aug 28, 2024

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.

@aronowski aronowski added the extra review wanted Initial review(s) look good, another review desired label Aug 28, 2024
@richterger
Copy link
Author

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
Copy link
Author

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

Any news here?

@aronowski
Copy link
Collaborator

@richterger, looking good! Accepting.

@aronowski aronowski added accepted Submission is ready for sysdev and removed extra review wanted Initial review(s) look good, another review desired labels Sep 4, 2024
@aronowski aronowski removed their assignment Sep 4, 2024
@richterger
Copy link
Author

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

1 similar comment
@richterger
Copy link
Author

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

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