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.per...@citrix.com> > Cc: Ard Biesheuvel <ard.biesheu...@linaro.org> > Cc: Jordan Justen <jordan.l.jus...@intel.com> > Cc: Julien Grall <julien.gr...@arm.com> > Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1747 > Signed-off-by: Laszlo Ersek <ler...@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 <phi...@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/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-