Proper Secure Boot support

[mariobalanica]

No description provided.

[pbatard]

In case you haven't seen how we do it on the Pi, this is relatively easy to add during the EDK2 build process.

Basically you want to first get all the needed Secure Boot certificates and dbx, most of which can be downloaded directly:
https://github.com/pftf/RPi4/blob/master/.github/workflows/linux_edk2.yml#L50-L58

Note that, because we sure don't want any third party (including ourselves) to have control over somebody else's machine when it comes to Secure Boot, we always generate a new PK as part of the build process and then discard the private key altogether.

Then, at EDK2 build time, you just need to feed the -D SECURE_BOOT_ENABLE=TRUE option along with something like -D DEFAULT_KEYS=TRUE -D PK_DEFAULT_FILE=$WORKSPACE/keys/pk.cer -D KEK_DEFAULT_FILE1=$WORKSPACE/keys/ms_kek.cer -D DB_DEFAULT_FILE1=$WORKSPACE/keys/ms_db1.cer -D DB_DEFAULT_FILE2=$WORKSPACE/keys/ms_db2.cer -D DBX_DEFAULT_FILE1=$WORKSPACE/keys/arm64_dbx.bin:
https://github.com/pftf/RPi4/blob/master/.github/workflows/linux_edk2.yml#L64-L65

And with this, you should have a UEFI firmware that both Windows and Linux are happy with when it comes to Secure Boot.

[mariobalanica]

Thanks for input, Pete!

I've actually put this on hold because we don't have a secure var store, and there are more steps involved to ensure a trusted boot chain that I'm not sure many would bother with.

But yeah, some formal support in EDK2 to make Windows happy and satisfy one of the requirements is better than nothing.

[pbatard]

Oh yeah, you're never going to get to a proper trusted boot chain on SoC systems without a lot of extra work, and we are well aware that, at least for the Pi, the Secure Boot feature we provide is more a showcase than anything that will actually help "secure" the system. It was indeed mostly added so that people could get Secure Boot status: On when running WoR...

[mariobalanica]

Added initial support in: 8b6b3a6

Will keep this open to track further progress.

One obvious thing that needs to be fixed is disabling the recently added FDT override support when SecureBoot is on, since those files aren't signed. But it may be desirable to leave it this way until work on mainline Linux and DT settles, so folks won't have to rely on overrides as much.

[hongkongkiwi]

@pbatard Does the above method work without code modifications just passing the extra arguments? how can I sign a linux image I want to securely boot?

[pbatard]

The above method is for adding the Secure Boot certificates to a UEFI firmware that you build yourself. It has nothing to do with signing a linux image to work with an existing UEFI firmware that you did not build. If you do build your own UEFI firmware from scratch you should however be able to install your own certificate by adding an extra -D DB_DEFAULT_FILE3=my_cert.cer, and then whatever you sign with the private key that matches the one from this public cert should be validated under Secure Boot.

OR, and I assume this is really what you actually want to accomplish, if you are not building a UEFI firmware from scratch, but just want to have something you sign work with an existing UEFI firmware, you should follow this extensive guide to add your own certs using the existing UEFI interfaces.

[hongkongkiwi]

Makes sense, thanks for the link. That clears up signing downstream.

But for upstream, I am interested to use the builtin eFuse on Rockchip to make sure that only my custom signed bootloader can be used.

I've seen this done successfully with U-Boot and the rockchip signing tools (they sign the idbloader.img file). In our case, is the idblock.bin an equivalent of that?

[pbatard]

I'm afraid you're going to have to ask somebody else about this, as I know nothing about idblock.bin and I am not involved in RK3588 development.

[DualTachyon]

@hongkongkiwi you want this I guess: https://github.com/DualTachyon/rk3588-secure-boot

[hongkongkiwi]

Very helpful! Thanks @DualTachyon

[bexcran]

@pbatard I just noticed there are two arm64 dbx files: the one listed on uefi.org is https://uefi.org/sites/default/files/resources/arm64_DBXUpdate.bin while the RPi4 script is fetching https://uefi.org/sites/default/files/resources/dbxupdate_arm64.bin - and they're different (though I haven't looked into how/why).

Also, I'm seeing an error trying to add the dbxDefault variable: the code expects it to be in a X509 format, not the raw data suitable for adding directly via SetVariable:

SecureBootFetchData: Invalid key format: 0
Content for dbxDefault not found

[pbatard]

@bexcran, the file to use is always the one linked on https://uefi.org/revocationlistfile, therefore, currently that would be https://uefi.org/sites/default/files/resources/arm64_DBXUpdate.bin. At the time when we added the files for Pi 4, that link pointed to https://uefi.org/sites/default/files/resources/dbxupdate_arm64.bin (and we expected that the UEFI forum would not change URLs when they updated the DBXs, but it appears that they do), hence the discrepancy.

AFAIK, the DBX links have always been in the format needed for adding with SetVariable(), because it would make no sense to provide it otherwise (since it is meant to be used by the OS directly with SetVariable()), so your error must come from something else.

[bexcran]

@pbatard RPi4 is also using SecureBootDefaultKeysDxe and SecureBootVariableProvisionLib so it's going to have the same problem I ran into: keys/arm64_dbx.bin is in a format for SetVariable while the other certificates are in DER format. SecureBootDefaultKeysEntryPoint tries to initialize PKDefault, KEKDefault, dbDefault, dbtDefault and dbxDefault.

SecureBootInitKEKDefault, SecureBootInitDbDefault and SecureBootInitDbxDefault all have the same pattern of code:

  Status = SecureBootFetchData (&gDefaultdbFileGuid, &SigListsSize, &EfiSig);
  if (EFI_ERROR (Status)) {
    return Status;
  }

  Status = gRT->SetVariable (
                  EFI_DB_DEFAULT_VARIABLE_NAME,
                  &gEfiGlobalVariableGuid,
                  EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_BOOTSERVICE_ACCESS,
                  SigListsSize,
                  (VOID *)EfiSig
                  );

SecureBootFetchData does the same thing in all cases:

  while (1) {
    if (NewCertInfo == NULL) {
      Status = EFI_OUT_OF_RESOURCES;
      break;
    } else {
      CertInfo = NewCertInfo;
    }

    Status = GetSectionFromAnyFv (
               KeyFileGuid,
               EFI_SECTION_RAW,
               KeyIndex,
               &Buffer,
               &Size
               );

    if (Status == EFI_SUCCESS) {
      RsaPubKey = NULL;
      if (RsaGetPublicKeyFromX509 (Buffer, Size, &RsaPubKey) == FALSE) {
        DEBUG ((DEBUG_ERROR, "%a: Invalid key format: %d\n", __func__, KeyIndex));
        if (EfiSig != NULL) {
          FreePool (EfiSig);
        }

        FreePool (Buffer);
        Status = EFI_INVALID_PARAMETER;
        break;
      }

      CertInfo[KeyIndex].Data     = Buffer;
      CertInfo[KeyIndex].DataSize = Size;
      KeyIndex++;
      NewCertInfo = ReallocatePool (
                      sizeof (SECURE_BOOT_CERTIFICATE_INFO) * KeyIndex,
                      sizeof (SECURE_BOOT_CERTIFICATE_INFO) * (KeyIndex + 1),
                      CertInfo
                      );
    }

    if (Status == EFI_NOT_FOUND) {
      Status = EFI_SUCCESS;
      break;
    }
  }

So, since the DBX is in a different format than the KEK and DB it's going to cause a problem. I only noticed because I created a RELEASE build with DEBUG_ERROR and DEBUG_WARN levels enabled so any errors being logged were easy to notice.

[pbatard]

There's no DER format for DBX.

DBX is a proprietary format for a list of revoked hashes or certificates.

You seem to be equating DBX to a certificate, which it is not. It's a list of things, and it may not include any certificate at all.

Please see the UEFI specs. There's simply no way for DBX to be anything else than a custom list of element. It's always going to have to be in a different format than KEK or DB (well, as far as we're talking about adding individual certs to KEK and DB) because by nature we're dealing with completely different entities.

[bexcran]

Please see the UEFI specs. There's simply no way for DBX to be anything else than a custom list of element. It's always going to have to be in a different format than KEK or DB (well, as far as we're talking about adding individual certs to KEK and DB) because by nature we're dealing with completely different entities.

Then the code in EDK2 is wrong. It's assuming it's a list of certificates.

[pbatard]

In that case you need to work with the EDK2 to fix that code.

Please bear in mind that Secure Boot support for Pi was provided as a courtesy to avoid the Windows 11 annoyance of requiring Secure Boot, because the Pi platform cannot actually deliver proper Secure Boot, and our expectation is that if you pass a blob to the EDK2 as DBX, and EDK2 doesn't say "This blob is not in the right format" then the EDK2 thinks that the blob is in a format it can handle and will use it as expected.

We have no tested DBX revocation or anything outside of Windows not complaining that it wants to see Secure Boot, and, since I am no longer directly involved with the Pi EDK2 platform, I am not planning to invest any time on this.

So, if you think you have identified EDK2 code that does not perform what it's advertising, and not producing the error it should produce, then I will expect you to work with the EDK2 to fix it (and then that fix will naturally find its way for Pi and other platforms that use unmodified EDK2 code).