From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.85.128.65, mailfrom: philmd@redhat.com) Received: from mail-wm1-f65.google.com (mail-wm1-f65.google.com [209.85.128.65]) by groups.io with SMTP; Mon, 29 Apr 2019 05:36:19 -0700 Received: by mail-wm1-f65.google.com with SMTP id p21so12061299wmc.0 for ; Mon, 29 Apr 2019 05:36:18 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:openpgp:message-id :date:user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=1QdNQuHZ2ZrGLPZBVmxmxfNy6X5IfPsZrmTcN1615bw=; b=MAn2FfDgMsWzoAkRBqNcW9BLxDk20Yax+CPrevH4/cH59aOn+hufN6U4VgjSPJeFOp +JfW6rysoPNhl1XipTi72FfRYDoR2obwgrhe0f4lgCpYvhfSsooOsi2cqVodFWC8LJmN CoKv/gS4IACdPheEUswvsK0k/160zLHZc8LOTAYc/7aXJvnGOsb8iGl4wPcmrmN6JXqU rzV4IEBHqrn9gRt686k/APwfO74+03gr3D20n2NdWSnWhPsbWmvsXThpgaW+w/l6YImA 0cSiWHMmcSzMKPO+hjpew9/Ff9/qmpvoqFtgwPaFQJE4ccXnhYhXI8tFhCeLRY/Nr0HN 4kEw== X-Gm-Message-State: APjAAAVpfP0U4caZ/pnny9KsyJtm6skqGRoczm7dORFWNjQyjLnxzvtg rlF0GvJ0AepuVl2pGNuVGq1Iyw== X-Google-Smtp-Source: APXvYqx0gYnmurYhI/XfE2ppWsqLr9bJ3BrXqXqZDDvq0PYUABfYdLNIyDG1CD6rEvh3+QHpVcGIDg== X-Received: by 2002:a7b:cf33:: with SMTP id m19mr18259454wmg.101.1556541377417; Mon, 29 Apr 2019 05:36:17 -0700 (PDT) Return-Path: Received: from [192.168.1.33] (193.red-88-21-103.staticip.rima-tde.net. [88.21.103.193]) by smtp.gmail.com with ESMTPSA id q2sm9854275wrd.48.2019.04.29.05.36.16 (version=TLS1_3 cipher=AEAD-AES128-GCM-SHA256 bits=128/128); Mon, 29 Apr 2019 05:36:16 -0700 (PDT) Subject: Re: [edk2-devel] [PATCH 13/16] OvmfPkg/EnrollDefaultKeys: document the steps of the entry point function To: devel@edk2.groups.io, lersek@redhat.com Cc: Anthony Perard , Ard Biesheuvel , Jordan Justen , Julien Grall References: <20190427005328.27005-1-lersek@redhat.com> <20190427005328.27005-14-lersek@redhat.com> From: =?UTF-8?B?UGhpbGlwcGUgTWF0aGlldS1EYXVkw6k=?= Openpgp: id=89C1E78F601EE86C867495CBA2A3FD6EDEADC0DE; url=http://pgp.mit.edu/pks/lookup?op=get&search=0xA2A3FD6EDEADC0DE Message-ID: <5231b832-9086-a4e5-7bb0-919f69eb4701@redhat.com> Date: Mon, 29 Apr 2019 14:36:15 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: <20190427005328.27005-14-lersek@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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 > Cc: Ard Biesheuvel > Cc: Jordan Justen > Cc: Julien Grall > Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1747 > Signed-off-by: Laszlo Ersek > --- > 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 > Status = GetSettings (&Settings); > if (EFI_ERROR (Status)) { > return 1; > } > PrintSettings (&Settings); > > if (Settings.SetupMode != 0 || Settings.SecureBoot != 1 || > Settings.SecureBootEnable != 1 || Settings.CustomMode != 0 || >