From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Mon, 13 May 2019 09:14:33 -0700 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E9D52C024920; Mon, 13 May 2019 16:14:32 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-123-237.rdu2.redhat.com [10.10.123.237]) by smtp.corp.redhat.com (Postfix) with ESMTP id 874F119C67; Mon, 13 May 2019 16:14:31 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v2 5/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b To: devel@edk2.groups.io, jian.j.wang@intel.com, "Lu, XiaoyuX" Cc: "Ye, Ting" References: <1557379429-7527-1-git-send-email-xiaoyux.lu@intel.com> <1557379429-7527-5-git-send-email-xiaoyux.lu@intel.com> <38ff9c2f-1d16-4719-18f6-07e84e4d8bf8@redhat.com> From: "Laszlo Ersek" Message-ID: <56bae128-9d26-3b5a-0c79-b27b39ee29df@redhat.com> Date: Mon, 13 May 2019 18:14:30 +0200 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: X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Mon, 13 May 2019 16:14:33 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 05/10/19 12:26, Wang, Jian J wrote: > Hi Laszlo, > > rand_* is needed by openssl itself. BaseCryptLib also provide RandomSeed() > and RandomBytes() interface to wrap openssl rand functionality. We can't > just drop them. From platform independent perspective, using performance > counter is the best choice we have. If we want to achieve the uttermost > secure mechanism, only hardware seed/rand generator can meet it. Do you > agree to add cpu specific instruction to do that? For those processors > which don't support seed/rand generation, we have to fall back to performance > counter. I have not been aware of RandomSeed() / RandomBytes(). The RandomSeed() function lets the caller specify a seed, which is good design, IMO. In case the caller does not pass in a seed, the function implements its own seed. For that default seeding, we have the following implementations: (1) "CryptoPkg/Library/BaseCryptLib/Rand/CryptRand.c" -- uses a constant string as seed. :( (2) "CryptoPkg/Library/BaseCryptLib/Rand/CryptRandItc.c" -- calls AsmReadItc() to calculate the seed. The function AsmReadItc() is not defined (or even declared) anywhere in edk2, so I don't know what it does. (3) "CryptoPkg/Library/BaseCryptLib/Rand/CryptRandNull.c" -- calls ASSERT(FALSE), then returns FALSE. (4) "CryptoPkg/Library/BaseCryptLib/Rand/CryptRandTsc.c" -- calls AsmReadTsc() to calculate the seed. PEIMs seem to use (3), which appears safe. DXE drivers, runtime DXE drivers, and SMM drivers, use (4) on IA32/X64, and (1) on ARM/AARCH64. All quite unfortunate. Option (2) is never used in edk2. Based on the above analysis (is it correct?), I must agree that the v1 and v2 approaches in the present patch set, namely "constant data" and "TimerLib", may not be worse than what we already have. > Another option is that we could make use of EFI_RNG_PROTOCOL. According > to UEFI spec (see below), it can be used to get entropy. > > "This protocol is used to provide random numbers for use in applications, > or entropy for seeding other random number generators." > > Again, we could use performance counter as a fall back if EFI_RNG_PROTOCOL > is not provided by a platform. So if a platform really care about the security, > it should provide a EFI_RNG_PROTOCOL. This can also help to hide the > hardware/platform dependency. Consuming EFI_RNG_PROTOCOL would be an improvement. But it looks quite tricky. Namely, EFI_RNG_PROTOCOL may be provided by a UEFI driver that follows the UEFI driver model, and so the protocol could become available first in the BDS phase (when the underlying hwrng device were connected). Until then, no driver that depends on entropy -- through BaseCryptLib or OpensslLib -- should be dispatched. On the other hand, if the platform hardware does not include a hwrng (and so EFI_RNG_PROTOCOL is never produced), then the entropy dependent drivers should be dispatched as soon as this fact is determined (dynamically, at every boot). In other words, entropy-dependent drivers should wait until platform code (likely in BDS) makes the determination whether EFI_RNG_PROTOCOL can offer high-quality entropy, or the fallbacks have to be used. To make it even more complex, SMM drivers that need entropy should likely never use EFI_RNG_PROTOCOL, because a 3rd party UEFI driver should not be able to feed entropy (sensitive crypto data) to a privileged (SMM) driver. Do you think this is possible to implement?... It looks extremely complex to me. Honestly the best I could suggest is, "use RDRAND if available, fall back to TimerLib otherwise". :( But I would understand if you wanted to postpone RDRAND until later. Given this situation, I think I can't give A-b or R-b for this patch in the series. I think I can only offer regression-testing (for the upcoming v3). And. I don't intend to block this work based on the entropy source alone, any more. ... Can we at least collect a detailed list of use cases for which we must provide entropy? I think we should document that somewhere. Thanks, Laszlo > > Regards, > Jian > >> -----Original Message----- >> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of >> Laszlo Ersek >> Sent: Friday, May 10, 2019 1:30 AM >> To: devel@edk2.groups.io; Lu, XiaoyuX >> Cc: Wang, Jian J ; Ye, Ting >> Subject: Re: [edk2-devel] [PATCH v2 5/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b >> >> On 05/09/19 19:15, Laszlo Ersek wrote: >> >>> How about the following: >>> >>> - It seems like we cannot convince OpenSSL to *never* call these >>> functions, under UEFI. >>> >>> - We also cannot provide an implementation that is *guaranteed* to be >>> secure enough, IMO. >>> >>> - It seems like these functions *should* never be called in the edk2 >>> build however, given that we're not trying to do anything "new" with >>> OpenSSL in edk2 -- we just want to use the new OpenSSL release for the >>> same old things. >>> >>> - So why not just ensure that these functions *never return*? >>> >>> (1) Basically implement all of the functions like this: >>> >>> ASSERT (FALSE); >>> CpuDeadLoop (); >>> // >>> // if a return value is needed >>> // >>> return 0; >>> >>> What do you think about this approach? >> >> I notice that "rand" is another module in OpenSSL. >> >> Can we try adding "no-rand" to our Configure invocation? Perhaps the >> need for all of the rand_* functions goes away then. >> >> Thanks >> Laszlo >> >> > > > >