From: AKASHI Takahiro <takahiro.akashi@linaro.org>
To: u-boot@lists.denx.de
Subject: [PATCH v7 05/17] efi_loader: variable: add secure boot state transition
Date: Fri, 17 Apr 2020 07:53:06 +0900 [thread overview]
Message-ID: <20200416225306.GA20683@laputa> (raw)
In-Reply-To: <85679d4e-979c-dde9-226e-6ce9d371945f@gmx.de>
Heinrich,
On Thu, Apr 16, 2020 at 04:20:35PM +0200, Heinrich Schuchardt wrote:
> On 14.04.20 04:51, AKASHI Takahiro wrote:
> > UEFI specification defines several global variables which are related to
> > the current secure boot state. In this commit, those values will be
> > maintained according to operations. Currently, AuditMode and DeployedMode
> > are defined but not implemented.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> > lib/efi_loader/efi_variable.c | 231 +++++++++++++++++++++++++++++++++-
> > 1 file changed, 228 insertions(+), 3 deletions(-)
> >
> > diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
> > index adb78470f2d6..fd5c41f830b1 100644
> > --- a/lib/efi_loader/efi_variable.c
> > +++ b/lib/efi_loader/efi_variable.c
> > @@ -16,8 +16,16 @@
> > #include <u-boot/crc.h>
> > #include "../lib/crypto/pkcs7_parser.h"
> >
> > +enum efi_secure_mode {
> > + EFI_MODE_SETUP,
> > + EFI_MODE_USER,
> > + EFI_MODE_AUDIT,
> > + EFI_MODE_DEPLOYED,
> > +};
> > +
> > const efi_guid_t efi_guid_cert_type_pkcs7 = EFI_CERT_TYPE_PKCS7_GUID;
> > static bool efi_secure_boot;
> > +static int efi_secure_mode;
> >
> > #define READ_ONLY BIT(31)
> >
> > @@ -160,6 +168,210 @@ static const char *parse_attr(const char *str, u32 *attrp, u64 *timep)
> > return str;
> > }
> >
> > +static efi_status_t efi_set_variable_internal(u16 *variable_name,
> > + const efi_guid_t *vendor,
> > + u32 attributes,
> > + efi_uintn_t data_size,
> > + const void *data,
> > + bool ro_check);
> > +
> > +/**
> > + * efi_transfer_secure_state - handle a secure boot state transition
> > + * @mode: new state
> > + *
> > + * Depending on @mode, secure boot related variables are updated.
> > + * Those variables are *read-only* for users, efi_set_variable_internal()
> > + * is called here.
> > + *
> > + * Return: EFI_SUCCESS on success, status code (negative) on error
> > + */
> > +static efi_status_t efi_transfer_secure_state(enum efi_secure_mode mode)
> > +{
> > + u32 attributes;
> > + u8 val;
> > + efi_status_t ret;
> > +
> > + debug("Secure state from %d to %d\n", efi_secure_mode, mode);
>
> Do you mean "Switching secure state from %d to %d\n"?
>
> > +
> > + attributes = EFI_VARIABLE_BOOTSERVICE_ACCESS |
> > + EFI_VARIABLE_RUNTIME_ACCESS;
> > + if (mode == EFI_MODE_DEPLOYED) {
>
> Please, use switch.
>
> > + val = 1;
> > + ret = efi_set_variable_internal(L"SecureBoot",
> > + &efi_global_variable_guid,
> > + attributes | READ_ONLY,
> > + sizeof(val), &val,
> > + false);
> > + if (ret != EFI_SUCCESS)
> > + goto err;
> > + val = 0;
> > + ret = efi_set_variable_internal(L"SetupMode",
> > + &efi_global_variable_guid,
> > + attributes | READ_ONLY,
> > + sizeof(val), &val,
> > + false);
> > + if (ret != EFI_SUCCESS)
> > + goto err;
> > + val = 0;
> > + ret = efi_set_variable_internal(L"AuditMode",
> > + &efi_global_variable_guid,
> > + attributes | READ_ONLY,
> > + sizeof(val), &val,
> > + false);
> > + if (ret != EFI_SUCCESS)
> > + goto err;
> > + val = 1;
> > + ret = efi_set_variable_internal(L"DeployedMode",
> > + &efi_global_variable_guid,
> > + attributes | READ_ONLY,
> > + sizeof(val), &val,
> > + false);
> > + if (ret != EFI_SUCCESS)
> > + goto err;
> > +
>
> You are repeating this code block four times. Please extract a function,
> eg. efi_set_secure_state(). Than this reduces to
>
> switch (mode) {
> case EFI_MODE_DEPLOYED:
> ret = efi_set_secure_stat(1, 0, 0, 1);
> break;
> case EFI_MODE_AUDIT:
> ret = efi_set_variable_internal(L"PK",
> &efi_global_variable_guid,
> attributes,
> 0, NULL, false);
> if (ret == EFI_SUCCESS)
> ret = efi_set_secure_stat(0, 1, 1, 0);
> break;
> case EFI_MODE_USER:
> ret = efi_set_secure_stat(1, 0, 0, 0);
> break;
> ...
Okay, but please let me know when you finish to review
*all* the patches.
I have not changed the code in this commit for a long time
(probably since v1) and don't want to repeatedly submit the whole set
which is only partially reviewed.
Thanks,
-Takahiro Akashi
> Best regards
>
> Heinrich
>
> > + efi_secure_boot = true;
> > + } else if (mode == EFI_MODE_AUDIT) {
> > + ret = efi_set_variable_internal(L"PK",
> > + &efi_global_variable_guid,
> > + attributes,
> > + 0, NULL,
> > + false);
> > + if (ret != EFI_SUCCESS)
> > + goto err;
> > + val = 0;
> > + ret = efi_set_variable_internal(L"SecureBoot",
> > + &efi_global_variable_guid,
> > + attributes | READ_ONLY,
> > + sizeof(val), &val,
> > + false);
> > + if (ret != EFI_SUCCESS)
> > + goto err;
> > + val = 1;
> > + ret = efi_set_variable_internal(L"SetupMode",
> > + &efi_global_variable_guid,
> > + attributes | READ_ONLY,
> > + sizeof(val), &val,
> > + false);
> > + if (ret != EFI_SUCCESS)
> > + goto err;
> > + val = 1;
> > + ret = efi_set_variable_internal(L"AuditMode",
> > + &efi_global_variable_guid,
> > + attributes | READ_ONLY,
> > + sizeof(val), &val,
> > + false);
> > + if (ret != EFI_SUCCESS)
> > + goto err;
> > + val = 0;
> > + ret = efi_set_variable_internal(L"DeployedMode",
> > + &efi_global_variable_guid,
> > + attributes | READ_ONLY,
> > + sizeof(val), &val,
> > + false);
> > + if (ret != EFI_SUCCESS)
> > + goto err;
> > +
> > + efi_secure_boot = true;
> > + } else if (mode == EFI_MODE_USER) {
> > + val = 1;
> > + ret = efi_set_variable_internal(L"SecureBoot",
> > + &efi_global_variable_guid,
> > + attributes | READ_ONLY,
> > + sizeof(val), &val,
> > + false);
> > + if (ret != EFI_SUCCESS)
> > + goto err;
> > + val = 0;
> > + ret = efi_set_variable_internal(L"SetupMode",
> > + &efi_global_variable_guid,
> > + attributes | READ_ONLY,
> > + sizeof(val), &val,
> > + false);
> > + if (ret != EFI_SUCCESS)
> > + goto err;
> > + val = 0;
> > + ret = efi_set_variable_internal(L"AuditMode",
> > + &efi_global_variable_guid,
> > + attributes,
> > + sizeof(val), &val,
> > + false);
> > + if (ret != EFI_SUCCESS)
> > + goto err;
> > + val = 0;
> > + ret = efi_set_variable_internal(L"DeployedMode",
> > + &efi_global_variable_guid,
> > + attributes,
> > + sizeof(val), &val,
> > + false);
> > + if (ret != EFI_SUCCESS)
> > + goto err;
> > +
> > + efi_secure_boot = true;
> > + } else if (mode == EFI_MODE_SETUP) {
> > + val = 0;
> > + ret = efi_set_variable_internal(L"SecureBoot",
> > + &efi_global_variable_guid,
> > + attributes | READ_ONLY,
> > + sizeof(val), &val,
> > + false);
> > + if (ret != EFI_SUCCESS)
> > + goto err;
> > + val = 1;
> > + ret = efi_set_variable_internal(L"SetupMode",
> > + &efi_global_variable_guid,
> > + attributes | READ_ONLY,
> > + sizeof(val), &val,
> > + false);
> > + if (ret != EFI_SUCCESS)
> > + goto err;
> > + val = 0;
> > + ret = efi_set_variable_internal(L"AuditMode",
> > + &efi_global_variable_guid,
> > + attributes,
> > + sizeof(val), &val,
> > + false);
> > + if (ret != EFI_SUCCESS)
> > + goto err;
> > + val = 0;
> > + ret = efi_set_variable_internal(L"DeployedMode",
> > + &efi_global_variable_guid,
> > + attributes | READ_ONLY,
> > + sizeof(val), &val,
> > + false);
> > + if (ret != EFI_SUCCESS)
> > + goto err;
> > + } else {
> > + return EFI_INVALID_PARAMETER;
> > + }
> > +
> > + return EFI_SUCCESS;
> > +
> > +err:
> > + /* TODO: What action should be taken here? */
> > + printf("ERROR: Secure state transition failed\n");
> > + return ret;
> > +}
> > +
> > +/**
> > + * efi_init_secure_state - initialize secure boot state
> > + *
> > + * Return: EFI_SUCCESS on success, status code (negative) on error
> > + */
> > +static efi_status_t efi_init_secure_state(void)
> > +{
> > + efi_uintn_t size = 0;
> > + efi_status_t ret;
> > +
> > + ret = EFI_CALL(efi_get_variable(L"PK", &efi_global_variable_guid,
> > + NULL, &size, NULL));
> > + if (ret == EFI_BUFFER_TOO_SMALL && IS_ENABLED(CONFIG_EFI_SECURE_BOOT))
> > + ret = efi_transfer_secure_state(EFI_MODE_USER);
> > + else
> > + ret = efi_transfer_secure_state(EFI_MODE_SETUP);
> > +
> > + return ret;
> > +}
> > +
> > /**
> > * efi_secure_boot_enabled - return if secure boot is enabled or not
> > *
> > @@ -910,10 +1122,19 @@ efi_status_t EFIAPI efi_set_variable_common(u16 *variable_name,
> > EFI_PRINT("setting: %s=%s\n", native_name, val);
> >
> > out:
> > - if (env_set(native_name, val))
> > + if (env_set(native_name, val)) {
> > ret = EFI_DEVICE_ERROR;
> > - else
> > + } else {
> > + if ((u16_strcmp(variable_name, L"PK") == 0 &&
> > + guidcmp(vendor, &efi_global_variable_guid) == 0)) {
> > + ret = efi_transfer_secure_state(
> > + (delete ? EFI_MODE_SETUP :
> > + EFI_MODE_USER));
> > + if (ret != EFI_SUCCESS)
> > + goto err;
> > + }
> > ret = EFI_SUCCESS;
> > + }
> >
> > err:
> > free(native_name);
> > @@ -1098,5 +1319,9 @@ void efi_variables_boot_exit_notify(void)
> > */
> > efi_status_t efi_init_variables(void)
> > {
> > - return EFI_SUCCESS;
> > + efi_status_t ret;
> > +
> > + ret = efi_init_secure_state();
> > +
> > + return ret;
> > }
> >
>
next prev parent reply other threads:[~2020-04-16 22:53 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-14 2:51 [PATCH v7 00/17] efi_loader: add secure boot support AKASHI Takahiro
2020-04-14 2:51 ` [PATCH v7 01/17] efi_loader: add CONFIG_EFI_SECURE_BOOT config option AKASHI Takahiro
2020-04-14 2:51 ` [PATCH v7 02/17] efi_loader: add signature verification functions AKASHI Takahiro
2020-04-14 14:36 ` Heinrich Schuchardt
2020-04-20 1:55 ` AKASHI Takahiro
2020-04-14 14:52 ` Heinrich Schuchardt
2020-04-14 15:35 ` Heinrich Schuchardt
2020-04-17 18:16 ` Heinrich Schuchardt
2020-04-20 2:04 ` AKASHI Takahiro
2020-04-14 2:51 ` [PATCH v7 03/17] efi_loader: add signature database parser AKASHI Takahiro
2020-04-17 18:05 ` Heinrich Schuchardt
2020-04-20 2:19 ` AKASHI Takahiro
2020-04-14 2:51 ` [PATCH v7 04/17] efi_loader: variable: support variable authentication AKASHI Takahiro
2020-04-20 19:22 ` Sughosh Ganu
2020-04-20 19:30 ` Heinrich Schuchardt
2020-04-20 19:35 ` Sughosh Ganu
2020-04-14 2:51 ` [PATCH v7 05/17] efi_loader: variable: add secure boot state transition AKASHI Takahiro
2020-04-16 14:20 ` Heinrich Schuchardt
2020-04-16 22:53 ` AKASHI Takahiro [this message]
2020-04-14 2:51 ` [PATCH v7 06/17] efi_loader: variable: add VendorKeys variable AKASHI Takahiro
2020-04-14 2:51 ` [PATCH v7 07/17] efi_loader: image_loader: support image authentication AKASHI Takahiro
2020-04-14 2:51 ` [PATCH v7 08/17] efi_loader: set up secure boot AKASHI Takahiro
2020-04-14 2:51 ` [PATCH v7 09/17] cmd: env: use appropriate guid for authenticated UEFI variable AKASHI Takahiro
2020-04-14 2:51 ` [PATCH v7 10/17] cmd: env: add "-at" option to "env set -e" command AKASHI Takahiro
2020-04-14 2:51 ` [PATCH v7 11/17] cmd: efidebug: add "test bootmgr" sub-command AKASHI Takahiro
2020-04-14 2:51 ` [PATCH v7 12/17] efi_loader, pytest: set up secure boot environment AKASHI Takahiro
2020-04-18 11:35 ` Heinrich Schuchardt
2020-04-14 2:51 ` [PATCH v7 13/17] efi_loader, pytest: add UEFI secure boot tests (authenticated variables) AKASHI Takahiro
2020-04-14 2:51 ` [PATCH v7 14/17] efi_loader, pytest: add UEFI secure boot tests (image) AKASHI Takahiro
2020-04-14 2:51 ` [PATCH v7 15/17] sandbox: add extra configurations for UEFI and related tests AKASHI Takahiro
2020-04-14 2:51 ` [PATCH v7 16/17] travis: add packages for UEFI secure boot test AKASHI Takahiro
2020-04-14 15:33 ` Heinrich Schuchardt
2020-04-15 0:22 ` AKASHI Takahiro
2020-04-14 2:51 ` [PATCH v7 17/17] efi_loader: add some description about UEFI secure boot AKASHI Takahiro
2020-04-17 7:21 ` [PATCH v7 00/17] efi_loader: add secure boot support AKASHI Takahiro
2020-04-17 10:01 ` Sughosh Ganu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200416225306.GA20683@laputa \
--to=takahiro.akashi@linaro.org \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox