From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.120]) by mx.groups.io with SMTP id smtpd.web11.5824.1580900438892393063 for ; Wed, 05 Feb 2020 03:00:39 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=WnH48jy2; spf=pass (domain: redhat.com, ip: 205.139.110.120, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1580900438; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=b3CtX8o/8rHPsxfh7P5+9gduthHef4AeEysPi/zzTMw=; b=WnH48jy2mFlp/yDp2WeA9zE56UlvELrLw3bjc4IZNanfZFgjAbESuDMqhSnfsTt3yOM/N4 CUjQv1x5LmNlo1Sh2X2xBxylag2to1qdL7eHSQg384pObx7XAMSoD9Jt9FMtfYDhuAW0j7 4aBKGbOljToTCX7W4Ew7EVP4LKRhBkQ= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-220-s3AqwV0rOlqpeXkqpfvMtQ-1; Wed, 05 Feb 2020 06:00:32 -0500 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 612EB8018B0; Wed, 5 Feb 2020 11:00:31 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-248.ams2.redhat.com [10.36.116.248]) by smtp.corp.redhat.com (Postfix) with ESMTP id 1809B65E85; Wed, 5 Feb 2020 11:00:29 +0000 (UTC) Subject: Re: [edk2-devel] [Patch v10 2/2] CryptoPkg/BaseHashApiLib: Implement Unified Hash Calculation API To: devel@edk2.groups.io, michael.d.kinney@intel.com Cc: Amol N Sukerkar , Jiewen Yao , Jian J Wang References: <20200203233548.7616-1-michael.d.kinney@intel.com> <20200203233548.7616-3-michael.d.kinney@intel.com> From: "Laszlo Ersek" Message-ID: <7a85ca5b-c083-959d-0064-73d45a422396@redhat.com> Date: Wed, 5 Feb 2020 12:00:29 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20200203233548.7616-3-michael.d.kinney@intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-MC-Unique: s3AqwV0rOlqpeXkqpfvMtQ-1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Hi, sorry I'm late to this discussion. I'd only like to mention a potential future improvement: On 02/04/20 00:35, Michael D Kinney wrote: > +[PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx] > + ## This PCD indicates the HASH algorithm to calculate hash of data > + # Based on the value set, the required algorithm is chosen to calculate > + # the hash of data.
> + # The default hashing algorithm for BaseHashApiLib is set to SHA256.
> + # 0x00000001 - MD4.
> + # 0x00000002 - MD5.
> + # 0x00000003 - SHA1.
> + # 0x00000004 - SHA256.
> + # 0x00000005 - SHA384.
> + # 0x00000006 - SHA512.
> + # 0x00000007 - SM3_256.
> + # @Prompt Set policy for hashing unsigned image for Secure Boot. > + # @ValidRange 0x80000001 | 0x00000001 - 0x00000007 > + gEfiCryptoPkgTokenSpaceGuid.PcdHashApiLibPolicy|0x04|UINT8|0x00000001 > + The platform may choose to make this PCD dynamic or dynamicEx. That's good. But: > +UINTN > +EFIAPI > +HashApiGetContextSize ( > + VOID > + ) > +{ > + switch (PcdGet8 (PcdHashApiLibPolicy)) { > + case HASH_API_ALGO_MD4: > + return Md4GetContextSize (); > + break; we have direct PcdGet8() calls in the lib API implementations. And: > +[Defines] > + INF_VERSION = 0x00010005 > + BASE_NAME = BaseHashApiLib > + MODULE_UNI_FILE = BaseHashApiLib.uni > + FILE_GUID = B1E566DD-DE7C-4F04-BDA0-B1295D3BE927 > + MODULE_TYPE = BASE > + VERSION_STRING = 1.0 > + LIBRARY_CLASS = BaseHashApiLib [...] > +[Pcd] > + gEfiCryptoPkgTokenSpaceGuid.PcdHashApiLibPolicy ## CONSUMES The lib class is not restricted to any particular firmware phase, or module type. This suggests that the lib instance is usable in DXE runtime drivers or SMM drivers. If the serives are called outside of the entry point functions, the dynamic PCD fetches would be a problem, I think. So the idea here would be to create a minimal separate INF file + C file for runtime applications (runtime DXE and SMM drivers), and there a constructor function could run PcdGet8(), and stash the value in a global variable. Alternatively, if this is overkill, we could improve safety by restricting LIBRARY_CLASS = BaseHashApiLib| ... to every module type except runtime DXE drivers and SMM drivers. Thanks Laszlo