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 024ACC433F5 for ; Wed, 19 Jan 2022 04:47:31 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 3E12C837CA; Wed, 19 Jan 2022 05:47:29 +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="xfuQNFk+"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id E6E60837CB; Wed, 19 Jan 2022 05:47:27 +0100 (CET) Received: from mail-pl1-x62d.google.com (mail-pl1-x62d.google.com [IPv6:2607:f8b0:4864:20::62d]) (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 7F607835CA for ; Wed, 19 Jan 2022 05:47:24 +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=takahiro.akashi@linaro.org Received: by mail-pl1-x62d.google.com with SMTP id h13so490929plf.2 for ; Tue, 18 Jan 2022 20:47:24 -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:mail-followup-to:references :mime-version:content-disposition:in-reply-to; bh=fqkRUBAIikz8DUG/CjwqOJr52u3Aj5OiPF0Slvt9SvY=; b=xfuQNFk+GYPCx6n5YiTMHahZh0fAi7t4gO0gHBJEMR0IvRWDyHsp7Cu6j+TBmF0EfM JSzeAUDiziKd5PS8xmaH4x7/0rMLn8olULlUQfrxipeaNpNfhlJTg3nohKb2FJIvDCrY BhCziVcIcOBvsp/IoPHBb4WMpL/IAlnqIpIP4yIxON/d6HE5+vuGxaCtWATtVYFl6NiD 2pdMGHH7D5iRUkT6vcnKEyUkpsBFDbgQWrHCkA7RK43qq6deXskGIz84KDWgO6rRp/o1 QafWPf3VknastB5sZSQ/WNjpcYz6BsVgB/mrg5gGfM5uxHcnXiRS/Ls9sXh+6mhYodR9 1cYQ== 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 :mail-followup-to:references:mime-version:content-disposition :in-reply-to; bh=fqkRUBAIikz8DUG/CjwqOJr52u3Aj5OiPF0Slvt9SvY=; b=CtGEpvCpO/BFEBXq5880HoMX93Jgs9SdBqbjxTMeqIzGR7F4toEZwRh1aT/OsqvPLa +qsupqKnaA+c8YBVHSq0DVj17Grqc1yL5dENXj7NsWa8mC/RfO8h48P4XR3clrSClsFj ChSQNoWy+MTrkC+7VxXPuD9GQuUTUIYpfqZrrzkUqAHqYg1BSj/2osuCRxFhk4ZlCd0+ ACGaTeWAMXgpu8rDytizo++Rh6Xrh0qkDmhyv/f6WDHIH+G4lF+zRUtCzY6wrbcStQFT EDbEUXy3YxD633VH0JfvDbIKM1BRhAHpkWkEvThyb0lBFpm9w8JwGx8HJO23i4igN5mE dx3A== X-Gm-Message-State: AOAM530nSFdOI4M8DO1EOI4a+5MCZsojCtA3GtzrYCwTqTaa1o4i34NY 7lOK0GJHR+WwrsojLhr8rnPvyQ== X-Google-Smtp-Source: ABdhPJwTwyly8rqINV/MxKyFrzUf5+mbfl8VJZKoJBK+EtnScrxjL5VlyTwDh2tF6+IGWizpnF92/g== X-Received: by 2002:a17:90b:4c09:: with SMTP id na9mr2205698pjb.96.1642567642783; Tue, 18 Jan 2022 20:47:22 -0800 (PST) Received: from laputa (p914133-ipoe.ipoe.ocn.ne.jp. [153.243.15.132]) by smtp.gmail.com with ESMTPSA id p15sm4085146pjj.52.2022.01.18.20.47.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 18 Jan 2022 20:47:22 -0800 (PST) Date: Wed, 19 Jan 2022 13:47:10 +0900 From: AKASHI Takahiro To: Ilias Apalodimas Cc: Heinrich Schuchardt , u-boot@lists.denx.de Subject: Re: [PATCH] lib/crypto: Enable more algorithms in cert verification Message-ID: <20220119044710.GB61490@laputa> Mail-Followup-To: AKASHI Takahiro , Ilias Apalodimas , Heinrich Schuchardt , u-boot@lists.denx.de References: <20220118111238.321742-1-ilias.apalodimas@linaro.org> <20220118123822.GC30001@laputa> <1ddb38a8-b998-4917-a645-bf7356b32e9d@gmx.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: 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.2 at phobos.denx.de X-Virus-Status: Clean On Tue, Jan 18, 2022 at 08:12:22PM +0200, Ilias Apalodimas wrote: > Hi Heinrich, > > On Tue, 18 Jan 2022 at 18:22, Heinrich Schuchardt wrote: > > > > On 1/18/22 15:03, Ilias Apalodimas wrote: > > > Hi Heinrich, > > > > > >>>>> - info.checksum = image_get_checksum_algo("sha256,rsa2048"); > > > > > > [...] > > > > > >>>>> - info.name = "sha256,rsa2048"; > > >>>>> - } else { > > >>>>> - pr_warn("unknown msg digest algo: %s\n", sig->hash_algo); > > >>>>> + if (strcmp(sig->pkey_algo, "rsa")) { > > >>>>> + pr_err("Encryption is not RSA: %s\n", sig->pkey_algo); > > >>>>> return -ENOPKG; > > >>>>> } > > >>>>> + ret = snprintf(algo, sizeof(algo), "%s,%s%d", sig->hash_algo, > > >>>>> + sig->pkey_algo, sig->s_size * 8); > > >> > > >> How do we ensure that the unsafe SHA1 algorithm is not used? > > > > > > We don't, but the current code allows it as well. Should we enforce this > > > from U-Boot though? The spec doesn't forbid it as far as I remember > > > > Collisions for SHA1 have been first created successfully in 2017. > > > > It is feasible to create two different EFI binaries with the same SHA1. > > One will be reviewed and signed. After copying the signature to the > > other one it will happily boot on U-Boot. Ouch. This is exactly what > > signatures are meant to avoid. > > > > We must not accept SHA1 for signatures. > > Right, but is this the right place to do it? This is function to > verify signatures. Isn't it better to keep this as is and then > explicitly deny adding sha1 hashed keys into db? If you don't want to trust SHA1, just disable it with !CONFIG_SHA1. -Takahiro Akashi > Cheers > /Ilias > > > > Best regards > > > > Heinrich > > > > > > > > Regards > > > /Ilias > > >> > > >> Best regards > > >> > > >> Heinrich > > >> > > >>>> > > >>>> I'm not sure that this naming rule, in particular the latter part, will > > >>>> always hold in the future while all the existing algo's observe it. > > >>>> (Maybe we need some note somewhere?) > > >>> > > >>> The if a few lines below will shield us and return -EINVAL. How about > > >>> adding an error message there? > > >>> > > >>> Cheers > > >>> /Ilias > > >>>> > > >>>> -Takahiro Akashi > > >>>> > > >>>>> + > > >>>>> + if (ret >= sizeof(algo)) > > >>>>> + return -EINVAL; > > >>>>> + > > >>>>> + info.checksum = image_get_checksum_algo((const char *)algo); > > >>>>> + info.name = (const char *)algo; > > >>>>> info.crypto = image_get_crypto_algo(info.name); > > >>>>> - if (IS_ERR(info.checksum) || IS_ERR(info.crypto)) > > >>>>> + if (!info.checksum || !info.crypto) > > >>>>> return -ENOPKG; > > >>>>> > > >>>>> info.key = pkey->key; > > >>>>> -- > > >>>>> 2.30.2 > > >>>>> > > >> > >