Re: [edk2-devel] [PATCH v2 5/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b

["Laszlo Ersek" <lersek@redhat.com>]

On 05/14/19 15:25, Wang, Jian J wrote:
> Laszlo,
> 
> 
>> -----Original Message-----
>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>> Laszlo Ersek
>> Sent: Tuesday, May 14, 2019 6:59 PM
>> To: Wang, Jian J <jian.j.wang@intel.com>; devel@edk2.groups.io; Lu, XiaoyuX
>> <xiaoyux.lu@intel.com>
>> Cc: Ye, Ting <ting.ye@intel.com>
>> Subject: Re: [edk2-devel] [PATCH v2 5/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b
>>
>> Hi Jian,
>>
>> On 05/14/19 09:03, Wang, Jian J wrote:
>>>> -----Original Message-----
>>>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>>>> Sent: Tuesday, May 14, 2019 12:15 AM
>>>> To: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; Lu,
>> XiaoyuX
>>>> <xiaoyux.lu@intel.com>
>>>> Cc: Ye, Ting <ting.ye@intel.com>
>>>> Subject: Re: [edk2-devel] [PATCH v2 5/6] CryptoPkg: Upgrade OpenSSL to
>> 1.1.1b
>>
>>>> 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.
>>
>>> Actually we wanted to use hardware seed/rand generator at first. I
>>> thought it might not be acceptable due to the fact it's processor
>>> dependent. So we hesitated. My understanding to above comment
>>> is that you agree to use rdrand/rdseed if available and fall back to
>>> TimerLib otherwise, right?
>>
>> I've now tried to read up a little bit on RDRAND. It seems that crypto
>> folks do not universally trust RDRAND. Some people reject RDRAND
>> completely, while others are willing to use RDRAND as *one* source for
>> entropy, but they always mix it with other entropy sources. But even
>> that practice is not acceptable to some people, saying that RDRAND can
>> be used to compromise those other entropy sources, dependent on the
>> mixing details. It seems that FreeBSD at the least uses the Yarrow
>> algorithm for mixing, which comes with its own complexities.
>>
>> As much as I dislike it, at the moment I cannot recommend anything
>> better than just TimerLib. I'm not satisfied with TimerLib, but I don't
>> know enough to suggest an improvement. RDRAND looked like a good entropy
>> source, but then reading up on people's opinions on it, gave me pause.
>>
>> In the longer term, I believe a serious reorganization of BaseCryptLib /
>> OpensslLib / etc might help. Namely, move the seeding / entropy
>> collection out of these low-level libraries, and force all dependent
>> modules (drivers and such) to provide their own entropy.
>>
>> Then, privileged drivers (e.g. SMM drivers) could use a low-level
>> platform device for collecting randomness, without depending on 3rd
>> party UEFI drivers. Less privileged drivers (such as for HTTPS boot)
>> could perhaps consume EFI_RNG_PROTOCOL, or maybe some protocol /
>> abstraction exposed by TPM drivers.
>>
>> I guess it's OK if we stick with TimerLib for this OpenSSL version upgrade.
>>
> 
> I'm not aware of the rdrand stories. Since there're concerns in the instruction,
> I agree not to use it in this patch series and we need a reorganization or even
> re-design for the random interface in the future.
> 
> To avoid any misunderstanding, let me summarize what will be done in the v4:
> 
>     TSC is used as first entropy source if it's available and fall back to TimerLib
>     otherwise. AES block cipher will be used to increase entropy rate or reduce
>     bias (NIST SP800-90B).

Side comment: I don't think you can increase the entropy rate by
choosing an AES block cipher. You can increase the entropy rate by using
a better entropy source, or by using additional entropy sources. What
AES can help with is to distribute the entropy over all bits in the
block, from the low order bits of the entropy source ("avalanche effect").

> Please confirm above implementation then we'll provide a v4 tomorrow.

Main comment: yes, the above approach looks fine to me, thank you.

Cheers
Laszlo

> 
>> Can we please document the use of platform timers as entropy sources
>> (including the TSC) in the following wiki article?
>>
>> https://github.com/tianocore/tianocore.github.io/wiki/CryptoPkg
>>
>> I'm not asking for many details, just a short summary of the fact and
>> why we do this.
>>
> 
> Sure.
> 
> Regards,
> Jian
> 
>> Thanks
>> Laszlo
>>
>> 
>