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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 3A6E9C433F5 for ; Fri, 28 Jan 2022 08:05:48 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 9D6A8830D0; Fri, 28 Jan 2022 09:05:46 +0100 (CET) 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="ZF8X/dRO"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 566AD830D0; Fri, 28 Jan 2022 09:05:45 +0100 (CET) Received: from mail-ed1-x52c.google.com (mail-ed1-x52c.google.com [IPv6:2a00:1450:4864:20::52c]) (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 74BC8837C5 for ; Fri, 28 Jan 2022 09:05:41 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=ilias.apalodimas@linaro.org Received: by mail-ed1-x52c.google.com with SMTP id w14so8169294edd.10 for ; Fri, 28 Jan 2022 00:05:41 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=uFBHwvIu7sBkTyslQZSKV4WSz/d53VnhjGyQpjpB++Y=; b=ZF8X/dROTRHZ17olRoROM7HksB0knT0oJInPh2s5X02yol/BI7BOQfiKqPNvwKe3XI ilmjN+O2k9c3rsJL96bJ0oYySLEP8ZPX4+s2dpCrWEk494/PWf4Z9UiKk2efL4MZHv9+ qRvsyiU5bEJ4mOA9unM6UqSdbsSQCNCIYq67tgzJW1lNkebr+K0Fdacxb74lc9uqNqvG tNBkZoTgnr4mauPHjB1x6GG1JnO7GFqMOxSGvOEeSgA5PW5Pp6wcP737O3YBX2zy9wEw bfwmSzuIVWS3qdbtX1kY8vrsJeaHU1xEB03RUp8DpkD9vJWIRP+yH3pNxFY3OfNjGTdN a/tQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=uFBHwvIu7sBkTyslQZSKV4WSz/d53VnhjGyQpjpB++Y=; b=RtB57Ofe0e1+GUgwjTE3ntYCUMot3Q06syZRlgLe8tnAQMR+Xa0BqDfjzVwMadshAq dgk8clKIcb/iK1qV1o3sBHyBT2915SA7KyfVQAVppqLp+Y+8ri3syqOGQ9ZMFTjgC+T/ qjcqIrEn6Nnc8jAYSwuRuP648M2RfNektcz0x1m3VLQsQx7faNNH/ycgTpSg4fTGx4Np Pr+JAIxuZ6ZdL1wYsJtHWz/13Lqkq4x2TwiJWa0YhUFIr4Ihn+O/BBdZ8WIVUQW4DKM6 iyVg/BkDsH7RJMkEDaSWXUnde4ubAFGZUBNCUEvQGEYydj+wdMwu4rpuNLYMvokz6FLF hrog== X-Gm-Message-State: AOAM531ziK16EookrlaFgkLT9SNa3HTHkexy/Bzmp9/tB6KHgOWrVfHL k+KUldHLayGXZTCXyGZ2b8bzFQ== X-Google-Smtp-Source: ABdhPJyNBCz0lcl4NFOmRcJq8yFWAP78mGfAW8/6alGQNsKbiDAtQnLyQT9JVMzMzhb83Yh4mOUnJQ== X-Received: by 2002:a05:6402:438d:: with SMTP id o13mr7063762edc.258.1643357140608; Fri, 28 Jan 2022 00:05:40 -0800 (PST) Received: from hades (athedsl-4461669.home.otenet.gr. [94.71.4.85]) by smtp.gmail.com with ESMTPSA id ce21sm8334431ejb.7.2022.01.28.00.05.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 28 Jan 2022 00:05:40 -0800 (PST) Date: Fri, 28 Jan 2022 10:05:37 +0200 From: Ilias Apalodimas To: Heinrich Schuchardt Cc: u-boot@lists.denx.de, AKASHI Takahiro Subject: Re: [PATCH] efi_loader: correctly handle mixed hashes and signatures in db Message-ID: References: <20220127223610.1642512-1-ilias.apalodimas@linaro.org> <7e2e78cf-3265-8961-6d72-bf4b1ec323fb@gmx.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <7e2e78cf-3265-8961-6d72-bf4b1ec323fb@gmx.de> X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 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.5 at phobos.denx.de X-Virus-Status: Clean On Fri, Jan 28, 2022 at 08:30:12AM +0100, Heinrich Schuchardt wrote: > On 1/27/22 23:36, Ilias Apalodimas wrote: > > A mix of signatures and hashes in db doesn't always work as intended. > > Currently if the digest algorithm is not supported we stop walking the > > security database and reject the image. > > That's problematic in case we find and try to check a signature before > > inspecting the sha256 hash. If the image is unsigned we will reject it > > even if the digest matches > > > > Signed-off-by: Ilias Apalodimas > > --- > > lib/efi_loader/efi_signature.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c > > index 3243e2c60de0..8fa82f8cea4c 100644 > > --- a/lib/efi_loader/efi_signature.c > > +++ b/lib/efi_loader/efi_signature.c > > @@ -176,7 +176,7 @@ bool efi_signature_lookup_digest(struct efi_image_regions *regs, > > if (guidcmp(&siglist->sig_type, &efi_guid_sha256)) { > > EFI_PRINT("Digest algorithm is not supported: %pUs\n", > > &siglist->sig_type); > > According to the commit message siglist->sig_type could be > EFI_CERT_RSA2048_SHA256_GUID which does not relate to a 'Digest > algorithm'. So the debug message text is wrong. As we expect guidcmp() > to report a mismatch we could remove the message. Ok > > > - break; > > + continue; > > This still is not correct: > > dbx containing a signature type that we do not support must disable the > loading of any image. > > The UEFI specification defines EFI_CERT_SHA1_GUID, EFI_CERT_SHA384_GUID > and EFI_CERT_SHA512_GUID. We should support all of these for dbx. > I don't really think we should go and support all of those. I have doubts about why we support time based revocation to begin with. > For security reasons we should not support EFI_CERT_SHA1_GUID for db. Nor dbx, sha1 should be completely ignored imho. > > The function lacks an argument indicating if we are dealing with db or > dbx which have to be treated differently. How about making it a bit more generic? We can add the default value of 'ret'. So we can easily handle cases were the algorithm requested is not supported. > > > } > > > > if (!efi_hash_regions(regs->reg, regs->num, &hash, &size)) { > > The subsequent code has a performance issue: > > We should not hash the image once per entry in db but once per hash > algorithm. Yea we seem to have this kind of issue in other parts as well. I'll fix that along the way Thanks /Ilias