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 Oracle Solaris 11.4 #447

Open
8 tasks done
daveminer1 opened this issue Oct 10, 2024 · 17 comments
Open
8 tasks done

shim 15.8 for Oracle Solaris 11.4 #447

daveminer1 opened this issue Oct 10, 2024 · 17 comments
Labels
1 review needed Needs 1 (additional) successful review before being accepted Accredited review needed Needs a successful review by an accredited reviewer contacts verified OK Contact verification is complete here (or in an earlier submission) question Reviewer(s) waiting on response

Comments

@daveminer1
Copy link

daveminer1 commented Oct 10, 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/daveminer1/shim-review/tree/oraclesolaris-shim-x86_64-20241010


What is the SHA256 hash of your final SHIM binary?


b098fb90bff86509aacff0e5bc197583e7e77968cc64da4d41d310fb4eab3087 shimx64.efi


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


N/A


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


N/A

@es-fabricemarie
Copy link

  • build reproduces fine with dockerfile and docker build .
    b098fb90bff86509aacff0e5bc197583e7e77968cc64da4d41d310fb4eab3087  /usr/share/shim/15.8-1.0.3.el9/x64/shimx64.efi
    
  • shim built from 15.8 official tarball, unpatched
  • NX bit is not set
  • SBAT entries look alright
  • OpenPGP keys:
    • Alan Coopersmith's key found on Ubuntu OpenPGP key server:
      • https://keyserver.ubuntu.com/pks/lookup?search=4A193C06D35E7C670FA4EF0BA2FB9E081F2D130E&fingerprint=on&op=index
      • expires: 2028-04-19, key is cross-signed
    • Dave Miner's key not found on Ubuntu/MIT OpenPGP key servers, no key expiry. Key is cross-signed.
  • EV Code Signing cert used:
    • RSA 3072 bit
    • Issuer: C=US, O=DigiCert, Inc., CN=DigiCert Trusted G4 Code Signing RSA4096 SHA384 2021 CA1
    • Subject: jurisdictionC=US, jurisdictionST=Delaware, businessCategory=Private Organization, serialNumber=2101822, C=US, ST=California, L=Redwood City, O=Oracle America, Inc., CN=Oracle America, Inc.
    • Expires in April next year: Not After : Apr 2 23:59:59 2025 GMT

Issues:

  • DISABLE_EBS_PROTECTION is set so official reviewer will have to take a deeper look.
  • grub2 is patched (build glue and zfs among others). More scrutiny by core team member is advised.

@steve-mcintyre steve-mcintyre added the contact verification needed Contact verification is needed for this review label Oct 14, 2024
@jsetje
Copy link
Collaborator

jsetje commented Oct 15, 2024

Since I work for the same company as the submitters, I don't count as a reviewer in this case. However, I looked this over in some detail before it was submitted.

DISABLE_EBS_PROTECTION is expected, since with the custom ELF validation code nothing will call back into shim lock protocol to have shim validate anything additional.

@daveminer1
Copy link
Author

My key is present on pgp.mit.edu:

Search results for 'oracle miner dave com'

Type bits/keyID Date User ID

pub 256E/8C450938 2024-07-23 Dave Miner [email protected]

@steve-mcintyre
Copy link
Collaborator

Mail sent to @daveminer1 for verification.

Alan's key is an ancient 1024-bit DSA key. We can't use that - it's way past time he generated something newer that's less readily broken.

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

Confirming identity

leaders drearier picker sequins oatmeal torrential Portsmouth revert Lindsay strait

@alanc
Copy link

alanc commented Oct 21, 2024

Mail sent to @daveminer1 for verification.

Alan's key is an ancient 1024-bit DSA key. We can't use that - it's way past time he generated something newer that's less readily broken.

Is not the RSA4096 key shown on https://keyserver.ubuntu.com/pks/lookup?search=0xCFDF148828C642A7&fingerprint=on&op=index sufficient?

@jsetje
Copy link
Collaborator

jsetje commented Oct 21, 2024

The old DSA one shows up first, presumably because it's your oldest key endorsing the others? I don't think we should use it anymore, but removing it from the server doesn't seem right either.

@julian-klode
Copy link
Collaborator

Mail sent to @daveminer1 for verification.
Alan's key is an ancient 1024-bit DSA key. We can't use that - it's way past time he generated something newer that's less readily broken.

Is not the RSA4096 key shown on https://keyserver.ubuntu.com/pks/lookup?search=0xCFDF148828C642A7&fingerprint=on&op=index sufficient?

That is a subkey of a dsa1024 key, and as such the binding signature (the signature the primary key makes to certify the subkey as bound to it) is not strong enough, you need to properly rotate to a strong primary key.

As the primary key is the root of trust, when someone goes fetch the key it will fetch and trust all it's subkeys and encrypt to the latest encryption subkey.

An attacker with enough compute could be able to fake signatures binding arbitrary new suvkeys to the primary keys as dsa1024 is particularly weak.

@steve-mcintyre
Copy link
Collaborator

@julian-klode has described this exactly - just adding new stronger subkeys isn't sufficient

@alanc
Copy link

alanc commented Oct 22, 2024

Sorry, I thought what I had done years ago was doing that, but I guess I did not. Unfortunately that description, no matter how exact, does not help me understand what I need to do now to satisfy you while not breaking the verification of the hundreds of X.Org package releases I've signed with that key which distros use to verify. Can you point to instructions for what gpg commands you want me to use here?

@steve-mcintyre
Copy link
Collaborator

Hi Alan!

No worries! You don't have to kill the old key, you can just create another new key for this new purpose.

The guide we typically point people to in the Debian community is https://keyring.debian.org/creating-key.html ; the defaults in gpg tend to be reasonable out of the box these days, but it dosn't hurt to check things look sane.

@daveminer1
Copy link
Author

Alan has generated a new key, the review is updated with the new fingerprint and updated pub files for both

@steve-mcintyre
Copy link
Collaborator

Mail to Alan on the way now

@steve-mcintyre steve-mcintyre added contact verification pending Contact verification emails have been sent, waiting on response and removed contact verification needed Contact verification is needed for this review bug Problem with the review that must be fixed before it will be accepted labels Oct 24, 2024
@alanc
Copy link

alanc commented Oct 24, 2024

Confirming my identity:

primitive epigram tablelands raking drubs reactive teakettle layettes hoppers extrinsic

@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 Oct 24, 2024
@lorddoskias
Copy link

Review

  • Build reproduces:
    b098fb90bff86509aacff0e5bc197583e7e77968cc64da4d41d310fb4eab3087 /shimx64.efi

  • HSM is used to store the key

  • SBAT entries ok

  • Shim is built from the provided srpm, which includes the upstream 15.8 as verified by the sha256 csum:

       fisk:~/projects/shim-review/oracle/src$ sha256sum shim-15.8.tar.bz2 
           a79f0a9b89f3681ab384865b1a46ab3f79d88b11b4ca59aa040ab03fffae80a9  shim-15.8.tar.bz2
    
  • SBAT looks ok:

#23 0.185 sbat,1,SBAT Version,sbat,1,https://github.com/rhboot/shim/blob/main/SBAT.md
#23 0.185 shim,4,UEFI shim,shim,1,https://github.com/rhboot/shim
#23 0.185 shim.solaris11,3,UEFI shim,shim,15.8,mail:[email protected]
  • Certificate is OK

@steve-mcintyre steve-mcintyre added 1 review needed Needs 1 (additional) successful review before being accepted Accredited review needed Needs a successful review by an accredited reviewer labels Nov 13, 2024
@steve-mcintyre steve-mcintyre added new vendor This is a new vendor and removed new vendor This is a new vendor labels Nov 20, 2024
@steve-mcintyre
Copy link
Collaborator

steve-mcintyre commented Nov 21, 2024

I'm still looking at GRUB patches yet, but here's current state with some questions - see the end...

Shim 15.8 for Oracle Solaris 11.4

Tag oraclesolaris-shim-x86_64-20241010

Good

General

  • Jan from Oracle is often reviewing things - good!
  • Contact verification completed - OK
  • Key management using a hardware token - OK
  • Old builds revoked via DBX, now using a new EV cert and SBAT - OK

Shim

  • Previous shim was ancient - 2016, prior to current proccess. New
    certs, new CA now - OK
  • Builds from 15.8 upstream, with no patches - OK
  • Very similar to the existing Oracle Linux 9 shim (Shim 15.8 for OL9 (ol9-shim-x86_64-aarch64-20240212) #378), with 2
    changes - OK
    • change of EFIDIR (meh)
    • DISABLE_EBS_PROTECTION enabled as they can't use shim_lock with
      GRUB and Solaris
  • shim build reproduces here for x64 - OK
b098fb90bff86509aacff0e5bc197583e7e77968cc64da4d41d310fb4eab3087  /shimx64.efi
b098fb90bff86509aacff0e5bc197583e7e77968cc64da4d41d310fb4eab3087  /usr/share/shim/15.8-1.0.3.el9/x64/shimx64.efi

  • NX bits not set - OK
  • Emedded EV cert expires in April 2025, getting a bit short...
  Serial Number:
      04:e6:d1:e5:28:2a:74:d6:92:19:6d:bb:71:0f:22:d6
  Signature Algorithm: sha256WithRSAEncryption
  Issuer: C = US, O = "DigiCert, Inc.", CN = DigiCert Trusted G4 Code Signing RSA4096 SHA384 2021 CA1
  Validity
      Not Before: Apr  1 00:00:00 2022 GMT
      Not After : Apr  2 23:59:59 2025 GMT
  Subject: jurisdictionC = US, jurisdictionST = Delaware, businessCategory = Private Organization, serialNumber = 2101822, C = US, ST = California, L = Redwood City, O = "Oracle America, Inc.", CN = "Oracle America, Inc."
  Subject Public Key Info:
      Public Key Algorithm: rsaEncryption
          Public-Key: (3072 bit)
  • SBAT data looks OK for shim - OK

GRUB

Solaris

  • Claim to have equivalent lockdown to Linux - OK
    • Question below

Issues / queries

  • Minor nit in the Shim SBAT data: you're using solaris11,3 for
    the vendor SBAT. Why not just solaris11,1?
  • Why are you including iorw and memrw in your signed GRUB build?
    Can you explain how they're compatible with Secure Boot please?
  • If Secure Boot is enabled in firmware, does that enforce the use of
    Verified Boot?

@steve-mcintyre steve-mcintyre added the question Reviewer(s) waiting on response label Nov 21, 2024
@daveminer1
Copy link
Author

Thanks for the questions, Steve:

  • We could use 1 as the generation; I was aligning it with the OL generation number initially since we are aligning with/reusing their build and intend to stay that way but there's really no reason we need to do this.
  • iorw and memrw certainly aren't needed, that list was initially carried over from our 1.99 build and I hadn't considered them. They'll be dropped.
  • Yes, Secure Boot in firmware does enforce use of Verified Boot. In contrib/vboot/verify.c, the verify_get_boot_policy() function reads the EFI variable and forces the policy to VERIFY_ENFORCE if enabled. The policy is then enforced in verify_module() which is called from the verifier that's in verfy_efi.c.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 review needed Needs 1 (additional) successful review before being accepted Accredited review needed Needs a successful review by an accredited reviewer contacts verified OK Contact verification is complete here (or in an earlier submission) question Reviewer(s) waiting on response
Projects
None yet
Development

No branches or pull requests

7 participants