[edk2-devel] [PATCH 13/16] OvmfPkg/EnrollDefaultKeys: document the steps of the entry point function

Laszlo Ersek posted 16 patches 6 years, 9 months ago
[edk2-devel] [PATCH 13/16] OvmfPkg/EnrollDefaultKeys: document the steps of the entry point function
Posted by Laszlo Ersek 6 years, 9 months ago
The entry point function of EnrollDefaultKeys finishes with a sanity
check, verifying the values of the Secure Boot-related "control"
variables. Add a diagram to explain why we expect the values we do.

While at it, write comments on the rest of the entry point function.

Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Julien Grall <julien.grall@arm.com>
Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1747
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c | 54 ++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c b/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c
index 07297c631f38..9c4a0f06fb4d 100644
--- a/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c
+++ b/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c
@@ -356,92 +356,146 @@ EFIAPI
 ShellAppMain (
   IN UINTN  Argc,
   IN CHAR16 **Argv
   )
 {
   EFI_STATUS Status;
   SETTINGS   Settings;
 
+  //
+  // If we're not in Setup Mode, we can't do anything.
+  //
   Status = GetSettings (&Settings);
   if (EFI_ERROR (Status)) {
     return 1;
   }
   PrintSettings (&Settings);
 
   if (Settings.SetupMode != 1) {
     AsciiPrint ("error: already in User Mode\n");
     return 1;
   }
 
+  //
+  // Enter Custom Mode so we can enroll PK, KEK, db, and dbx without signature
+  // checks on those variable writes.
+  //
   if (Settings.CustomMode != CUSTOM_SECURE_BOOT_MODE) {
     Settings.CustomMode = CUSTOM_SECURE_BOOT_MODE;
     Status = gRT->SetVariable (EFI_CUSTOM_MODE_NAME, &gEfiCustomModeEnableGuid,
                     (EFI_VARIABLE_NON_VOLATILE |
                      EFI_VARIABLE_BOOTSERVICE_ACCESS),
                     sizeof Settings.CustomMode, &Settings.CustomMode);
     if (EFI_ERROR (Status)) {
       AsciiPrint ("error: SetVariable(\"%s\", %g): %r\n", EFI_CUSTOM_MODE_NAME,
         &gEfiCustomModeEnableGuid, Status);
       return 1;
     }
   }
 
+  //
+  // Enroll db.
+  //
   Status = EnrollListOfCerts (
              EFI_IMAGE_SECURITY_DATABASE,
              &gEfiImageSecurityDatabaseGuid,
              &gEfiCertX509Guid,
              mMicrosoftPca,    mSizeOfMicrosoftPca,    &gMicrosoftVendorGuid,
              mMicrosoftUefiCa, mSizeOfMicrosoftUefiCa, &gMicrosoftVendorGuid,
              NULL);
   if (EFI_ERROR (Status)) {
     return 1;
   }
 
+  //
+  // Enroll dbx.
+  //
   Status = EnrollListOfCerts (
              EFI_IMAGE_SECURITY_DATABASE1,
              &gEfiImageSecurityDatabaseGuid,
              &gEfiCertSha256Guid,
              mSha256OfDevNull, mSizeOfSha256OfDevNull, &gEfiCallerIdGuid,
              NULL);
   if (EFI_ERROR (Status)) {
     return 1;
   }
 
+  //
+  // Enroll KEK.
+  //
   Status = EnrollListOfCerts (
              EFI_KEY_EXCHANGE_KEY_NAME,
              &gEfiGlobalVariableGuid,
              &gEfiCertX509Guid,
              mRedHatPkKek1, mSizeOfRedHatPkKek1, &gEfiCallerIdGuid,
              mMicrosoftKek, mSizeOfMicrosoftKek, &gMicrosoftVendorGuid,
              NULL);
   if (EFI_ERROR (Status)) {
     return 1;
   }
 
+  //
+  // Enroll PK, leaving Setup Mode (entering User Mode) at once.
+  //
   Status = EnrollListOfCerts (
              EFI_PLATFORM_KEY_NAME,
              &gEfiGlobalVariableGuid,
              &gEfiCertX509Guid,
              mRedHatPkKek1, mSizeOfRedHatPkKek1, &gEfiGlobalVariableGuid,
              NULL);
   if (EFI_ERROR (Status)) {
     return 1;
   }
 
+  //
+  // Leave Custom Mode, so that updates to PK, KEK, db, and dbx require valid
+  // signatures.
+  //
   Settings.CustomMode = STANDARD_SECURE_BOOT_MODE;
   Status = gRT->SetVariable (EFI_CUSTOM_MODE_NAME, &gEfiCustomModeEnableGuid,
                   EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS,
                   sizeof Settings.CustomMode, &Settings.CustomMode);
   if (EFI_ERROR (Status)) {
     AsciiPrint ("error: SetVariable(\"%s\", %g): %r\n", EFI_CUSTOM_MODE_NAME,
       &gEfiCustomModeEnableGuid, Status);
     return 1;
   }
 
+  //
+  // Final sanity check:
+  //
+  //                                 [SetupMode]
+  //                        (read-only, standardized by UEFI)
+  //                                /                \_
+  //                               0               1, default
+  //                              /                    \_
+  //                      PK enrolled                   no PK enrolled yet,
+  //              (this is called "User Mode")          PK enrollment possible
+  //                             |
+  //                             |
+  //                     [SecureBootEnable]
+  //         (read-write, edk2-specific, boot service only)
+  //                /                           \_
+  //               0                         1, default
+  //              /                               \_
+  //       [SecureBoot]=0                     [SecureBoot]=1
+  // (read-only, standardized by UEFI)  (read-only, standardized by UEFI)
+  //     images are not verified         images are verified, platform is
+  //                                      operating in Secure Boot mode
+  //                                                 |
+  //                                                 |
+  //                                           [CustomMode]
+  //                          (read-write, edk2-specific, boot service only)
+  //                                /                           \_
+  //                          0, default                         1
+  //                              /                               \_
+  //                      PK, KEK, db, dbx                PK, KEK, db, dbx
+  //                    updates are verified          updates are not verified
+  //
   Status = GetSettings (&Settings);
   if (EFI_ERROR (Status)) {
     return 1;
   }
   PrintSettings (&Settings);
 
   if (Settings.SetupMode != 0 || Settings.SecureBoot != 1 ||
       Settings.SecureBootEnable != 1 || Settings.CustomMode != 0 ||
-- 
2.19.1.3.g30247aa5d201



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#39688): https://edk2.groups.io/g/devel/message/39688
Mute This Topic: https://groups.io/mt/31359385/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 13/16] OvmfPkg/EnrollDefaultKeys: document the steps of the entry point function
Posted by Philippe Mathieu-Daudé 6 years, 9 months ago
On 4/27/19 2:53 AM, Laszlo Ersek wrote:
> The entry point function of EnrollDefaultKeys finishes with a sanity
> check, verifying the values of the Secure Boot-related "control"
> variables. Add a diagram to explain why we expect the values we do.
> 
> While at it, write comments on the rest of the entry point function.
> 
> Cc: Anthony Perard <anthony.perard@citrix.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Julien Grall <julien.grall@arm.com>
> Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1747
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c | 54 ++++++++++++++++++++
>  1 file changed, 54 insertions(+)
> 
> diff --git a/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c b/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c
> index 07297c631f38..9c4a0f06fb4d 100644
> --- a/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c
> +++ b/OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.c
> @@ -356,92 +356,146 @@ EFIAPI
>  ShellAppMain (
>    IN UINTN  Argc,
>    IN CHAR16 **Argv
>    )
>  {
>    EFI_STATUS Status;
>    SETTINGS   Settings;
>  
> +  //
> +  // If we're not in Setup Mode, we can't do anything.
> +  //
>    Status = GetSettings (&Settings);
>    if (EFI_ERROR (Status)) {
>      return 1;
>    }
>    PrintSettings (&Settings);
>  
>    if (Settings.SetupMode != 1) {
>      AsciiPrint ("error: already in User Mode\n");
>      return 1;
>    }
>  
> +  //
> +  // Enter Custom Mode so we can enroll PK, KEK, db, and dbx without signature
> +  // checks on those variable writes.
> +  //
>    if (Settings.CustomMode != CUSTOM_SECURE_BOOT_MODE) {
>      Settings.CustomMode = CUSTOM_SECURE_BOOT_MODE;
>      Status = gRT->SetVariable (EFI_CUSTOM_MODE_NAME, &gEfiCustomModeEnableGuid,
>                      (EFI_VARIABLE_NON_VOLATILE |
>                       EFI_VARIABLE_BOOTSERVICE_ACCESS),
>                      sizeof Settings.CustomMode, &Settings.CustomMode);
>      if (EFI_ERROR (Status)) {
>        AsciiPrint ("error: SetVariable(\"%s\", %g): %r\n", EFI_CUSTOM_MODE_NAME,
>          &gEfiCustomModeEnableGuid, Status);
>        return 1;
>      }
>    }
>  
> +  //
> +  // Enroll db.
> +  //
>    Status = EnrollListOfCerts (
>               EFI_IMAGE_SECURITY_DATABASE,
>               &gEfiImageSecurityDatabaseGuid,
>               &gEfiCertX509Guid,
>               mMicrosoftPca,    mSizeOfMicrosoftPca,    &gMicrosoftVendorGuid,
>               mMicrosoftUefiCa, mSizeOfMicrosoftUefiCa, &gMicrosoftVendorGuid,
>               NULL);
>    if (EFI_ERROR (Status)) {
>      return 1;
>    }
>  
> +  //
> +  // Enroll dbx.
> +  //
>    Status = EnrollListOfCerts (
>               EFI_IMAGE_SECURITY_DATABASE1,
>               &gEfiImageSecurityDatabaseGuid,
>               &gEfiCertSha256Guid,
>               mSha256OfDevNull, mSizeOfSha256OfDevNull, &gEfiCallerIdGuid,
>               NULL);
>    if (EFI_ERROR (Status)) {
>      return 1;
>    }
>  
> +  //
> +  // Enroll KEK.
> +  //
>    Status = EnrollListOfCerts (
>               EFI_KEY_EXCHANGE_KEY_NAME,
>               &gEfiGlobalVariableGuid,
>               &gEfiCertX509Guid,
>               mRedHatPkKek1, mSizeOfRedHatPkKek1, &gEfiCallerIdGuid,
>               mMicrosoftKek, mSizeOfMicrosoftKek, &gMicrosoftVendorGuid,
>               NULL);
>    if (EFI_ERROR (Status)) {
>      return 1;
>    }
>  
> +  //
> +  // Enroll PK, leaving Setup Mode (entering User Mode) at once.
> +  //
>    Status = EnrollListOfCerts (
>               EFI_PLATFORM_KEY_NAME,
>               &gEfiGlobalVariableGuid,
>               &gEfiCertX509Guid,
>               mRedHatPkKek1, mSizeOfRedHatPkKek1, &gEfiGlobalVariableGuid,
>               NULL);
>    if (EFI_ERROR (Status)) {
>      return 1;
>    }
>  
> +  //
> +  // Leave Custom Mode, so that updates to PK, KEK, db, and dbx require valid
> +  // signatures.
> +  //
>    Settings.CustomMode = STANDARD_SECURE_BOOT_MODE;
>    Status = gRT->SetVariable (EFI_CUSTOM_MODE_NAME, &gEfiCustomModeEnableGuid,
>                    EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS,
>                    sizeof Settings.CustomMode, &Settings.CustomMode);
>    if (EFI_ERROR (Status)) {
>      AsciiPrint ("error: SetVariable(\"%s\", %g): %r\n", EFI_CUSTOM_MODE_NAME,
>        &gEfiCustomModeEnableGuid, Status);
>      return 1;
>    }
>  
> +  //
> +  // Final sanity check:
> +  //
> +  //                                 [SetupMode]
> +  //                        (read-only, standardized by UEFI)
> +  //                                /                \_
> +  //                               0               1, default
> +  //                              /                    \_
> +  //                      PK enrolled                   no PK enrolled yet,
> +  //              (this is called "User Mode")          PK enrollment possible
> +  //                             |
> +  //                             |
> +  //                     [SecureBootEnable]
> +  //         (read-write, edk2-specific, boot service only)
> +  //                /                           \_
> +  //               0                         1, default
> +  //              /                               \_
> +  //       [SecureBoot]=0                     [SecureBoot]=1
> +  // (read-only, standardized by UEFI)  (read-only, standardized by UEFI)
> +  //     images are not verified         images are verified, platform is
> +  //                                      operating in Secure Boot mode
> +  //                                                 |
> +  //                                                 |
> +  //                                           [CustomMode]
> +  //                          (read-write, edk2-specific, boot service only)
> +  //                                /                           \_
> +  //                          0, default                         1
> +  //                              /                               \_
> +  //                      PK, KEK, db, dbx                PK, KEK, db, dbx
> +  //                    updates are verified          updates are not verified
> +  //

Very helpful.

Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>

>    Status = GetSettings (&Settings);
>    if (EFI_ERROR (Status)) {
>      return 1;
>    }
>    PrintSettings (&Settings);
>  
>    if (Settings.SetupMode != 0 || Settings.SecureBoot != 1 ||
>        Settings.SecureBootEnable != 1 || Settings.CustomMode != 0 ||
> 

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#39769): https://edk2.groups.io/g/devel/message/39769
Mute This Topic: https://groups.io/mt/31359385/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-