From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.0 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, UNWANTED_LANGUAGE_BODY autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7E070C432BE for ; Fri, 27 Aug 2021 04:12:19 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id DBC1360F4A for ; Fri, 27 Aug 2021 04:12:18 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org DBC1360F4A Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 886248323F; Fri, 27 Aug 2021 06:12:16 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.b="I6gDb8Xe"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 0B41683248; Fri, 27 Aug 2021 06:12:14 +0200 (CEST) Received: from mail-pg1-x530.google.com (mail-pg1-x530.google.com [IPv6:2607:f8b0:4864:20::530]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 5F64882D92 for ; Fri, 27 Aug 2021 06:12:09 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=takahiro.akashi@linaro.org Received: by mail-pg1-x530.google.com with SMTP id c17so4997757pgc.0 for ; Thu, 26 Aug 2021 21:12:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:mail-followup-to:references :mime-version:content-disposition:in-reply-to; bh=TjZOSzyEWzP1zlD3Yt/dnYQpRc22WTOLVJvewaQ8tGI=; b=I6gDb8XeARAhwxF/DqT9OlIQMuCex5ZgP5z3/aMYjc4nM8a2dDNNl5EEU7CW4nVdt+ byuLcHtNeP4r/mKHXuOqFg/rtoDDd6rsd7J1yx7SVXeNWOyqS9wrunrOWFFB8Xvi+3rH wI8ZFvwgEWVwif5ygD0cSA3mhOYW3gN/D5L/aBmoVA8VQld0dlRo9gUYFCfHLPEqKpH/ bbfQRoHZaD0Sa6QdPlCWrFVXEOmbg8zQjTCkqE4MPhdJFcBXfaRa1IndOZiipWM4/5vS LUDknwdYgVRMLAOd1qndaE8lAe1uT0vXLJHbBD9gNJ3+VuVW99ECIa6n6m1hXAnIz3WP FA/A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id :mail-followup-to:references:mime-version:content-disposition :in-reply-to; bh=TjZOSzyEWzP1zlD3Yt/dnYQpRc22WTOLVJvewaQ8tGI=; b=H+wTqPsCNdV8LK+t/h2ncU2ac12/JRfaeVxVVAQlSFpU45Uprb3C5W1fBh/PxenKfc bYN7dOIJ671T87Op9xGUhvOvpQqPid9tpb5l3hUigqsd0bl4yKN9QFUaHAt2aLW+Ryhj 4gFQjpv0tD0r49Q7zdGtP7JiMN1Qj1OJPep3ZJS4RsSx+myEM976YW0nd1MNuj0R5OQx bCwcr1B70NK5dvXzDbVMu3+xhY7+X4Q6Hbx61fiYZwUUIe7dvp2FcHPIrC/WYiFha0cC uipO8YhmYLfW+trRsBZE2l2exH98SaKKrc2wVkTn6Uu12QLLfSV8WU1Hykd+1iqeaHCa AQ6w== X-Gm-Message-State: AOAM5300sqj3SSVom5Ja/oH51eUcFBTkEZHeg5cHMydVoX6KmOniZWmT AsOf8L9PKos9zXeE7v7EnBZVag== X-Google-Smtp-Source: ABdhPJw037v9Cb1rytwM/XvqpXotD7UgB9PNzoZTGv8QaQMQhYzwKj8jM9+fD/BNe46vtq1V6RQSlQ== X-Received: by 2002:a63:4e45:: with SMTP id o5mr6270254pgl.191.1630037527635; Thu, 26 Aug 2021 21:12:07 -0700 (PDT) Received: from laputa (p784a6698.tkyea130.ap.so-net.ne.jp. [120.74.102.152]) by smtp.gmail.com with ESMTPSA id fh2sm4319044pjb.12.2021.08.26.21.12.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 26 Aug 2021 21:12:07 -0700 (PDT) Date: Fri, 27 Aug 2021 13:12:03 +0900 From: AKASHI Takahiro To: Heinrich Schuchardt Cc: u-boot@lists.denx.de, Alexander Graf , Ilias Apalodimas , Heinrich Schuchardt Subject: Re: [PATCH v2 3/6] efi_loader: don't load signature database from file Message-ID: <20210827041203.GE52912@laputa> Mail-Followup-To: AKASHI Takahiro , Heinrich Schuchardt , u-boot@lists.denx.de, Alexander Graf , Ilias Apalodimas , Heinrich Schuchardt References: <20210826134805.148975-1-heinrich.schuchardt@canonical.com> <20210826134805.148975-4-heinrich.schuchardt@canonical.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210826134805.148975-4-heinrich.schuchardt@canonical.com> X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean On Thu, Aug 26, 2021 at 03:48:02PM +0200, Heinrich Schuchardt wrote: > The UEFI specification requires that the signature database may only be > stored in tamper-resistant storage. So these variable may not be read > from an unsigned file. I don't have a strong opinion here, but it seems to be too restrictive. Nobody expects that file-based variable implementation is *safe*. Leave it as it is so that people can easily experiment secure boot. -Takahiro Akashi > Signed-off-by: Heinrich Schuchardt > --- > v2: > no change > --- > include/efi_variable.h | 5 +++- > lib/efi_loader/efi_var_common.c | 2 -- > lib/efi_loader/efi_var_file.c | 41 ++++++++++++++++++++------------- > lib/efi_loader/efi_variable.c | 2 +- > 4 files changed, 30 insertions(+), 20 deletions(-) > > diff --git a/include/efi_variable.h b/include/efi_variable.h > index 4623a64142..2d97655e1f 100644 > --- a/include/efi_variable.h > +++ b/include/efi_variable.h > @@ -161,10 +161,13 @@ efi_status_t __maybe_unused efi_var_collect(struct efi_var_file **bufp, loff_t * > /** > * efi_var_restore() - restore EFI variables from buffer > * > + * Only if @safe is set secure boot related variables will be restored. > + * > * @buf: buffer > + * @safe: restoring from tamper-resistant storage > * Return: status code > */ > -efi_status_t efi_var_restore(struct efi_var_file *buf); > +efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe); > > /** > * efi_var_from_file() - read variables from file > diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c > index cf7afecd60..b0c5b672c5 100644 > --- a/lib/efi_loader/efi_var_common.c > +++ b/lib/efi_loader/efi_var_common.c > @@ -32,10 +32,8 @@ static const struct efi_auth_var_name_type name_type[] = { > {u"KEK", &efi_global_variable_guid, EFI_AUTH_VAR_KEK}, > {u"db", &efi_guid_image_security_database, EFI_AUTH_VAR_DB}, > {u"dbx", &efi_guid_image_security_database, EFI_AUTH_VAR_DBX}, > - /* not used yet > {u"dbt", &efi_guid_image_security_database, EFI_AUTH_VAR_DBT}, > {u"dbr", &efi_guid_image_security_database, EFI_AUTH_VAR_DBR}, > - */ > }; > > static bool efi_secure_boot; > diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c > index de076b8cbc..c7c6805ed0 100644 > --- a/lib/efi_loader/efi_var_file.c > +++ b/lib/efi_loader/efi_var_file.c > @@ -148,9 +148,10 @@ error: > #endif > } > > -efi_status_t efi_var_restore(struct efi_var_file *buf) > +efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe) > { > struct efi_var_entry *var, *last_var; > + u16 *data; > efi_status_t ret; > > if (buf->reserved || buf->magic != EFI_VAR_FILE_MAGIC || > @@ -160,21 +161,29 @@ efi_status_t efi_var_restore(struct efi_var_file *buf) > return EFI_INVALID_PARAMETER; > } > > - var = buf->var; > last_var = (struct efi_var_entry *)((u8 *)buf + buf->length); > - while (var < last_var) { > - u16 *data = var->name + u16_strlen(var->name) + 1; > - > - if (var->attr & EFI_VARIABLE_NON_VOLATILE && var->length) { > - ret = efi_var_mem_ins(var->name, &var->guid, var->attr, > - var->length, data, 0, NULL, > - var->time); > - if (ret != EFI_SUCCESS) > - log_err("Failed to set EFI variable %ls\n", > - var->name); > - } > - var = (struct efi_var_entry *) > - ALIGN((uintptr_t)data + var->length, 8); > + for (var = buf->var; var < last_var; > + var = (struct efi_var_entry *) > + ALIGN((uintptr_t)data + var->length, 8)) { > + > + data = var->name + u16_strlen(var->name) + 1; > + > + /* > + * Secure boot related and non-volatile variables shall only be > + * restored from U-Boot's preseed. > + */ > + if (!safe && > + (efi_auth_var_get_type(var->name, &var->guid) != > + EFI_AUTH_VAR_NONE || > + !(var->attr & EFI_VARIABLE_NON_VOLATILE))) > + continue; > + if (!var->length) > + continue; > + ret = efi_var_mem_ins(var->name, &var->guid, var->attr, > + var->length, data, 0, NULL, > + var->time); > + if (ret != EFI_SUCCESS) > + log_err("Failed to set EFI variable %ls\n", var->name); > } > return EFI_SUCCESS; > } > @@ -213,7 +222,7 @@ efi_status_t efi_var_from_file(void) > log_err("Failed to load EFI variables\n"); > goto error; > } > - if (buf->length != len || efi_var_restore(buf) != EFI_SUCCESS) > + if (buf->length != len || efi_var_restore(buf, false) != EFI_SUCCESS) > log_err("Invalid EFI variables file\n"); > error: > free(buf); > diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c > index ba0874e9e7..a7d305ffbc 100644 > --- a/lib/efi_loader/efi_variable.c > +++ b/lib/efi_loader/efi_variable.c > @@ -426,7 +426,7 @@ efi_status_t efi_init_variables(void) > > if (IS_ENABLED(CONFIG_EFI_VARIABLES_PRESEED)) { > ret = efi_var_restore((struct efi_var_file *) > - __efi_var_file_begin); > + __efi_var_file_begin, true); > if (ret != EFI_SUCCESS) > log_err("Invalid EFI variable seed\n"); > } > -- > 2.30.2 >