[edk2-devel] [PATCH 2/3] CryptoPkg: Upgrade openssl to 1.1.1b

Xiaoyu lu xiaoyux.lu at intel.com
Tue May 14 05:21:31 UTC 2019


Hi Laszlo:
	I'm sorry I missed this email, and also sorry for I didn't understand your points.
	I focus on what the current code has, so what to add. I should pay more attention to why I need to change it. Your approach is better. Better to make review easier.
	About 'stupid', Can I change 'stupid' to 'bad'. I used the wrong word made you feel bad, I apologize. Comment (11) it is not offensive to me. I agree with your point. When you point out, I immediately felt that my implementation was bad. And this dummy implement is unsafe, it should be pointed out. 

    I'm grateful for your patience and step by step guiding me. Can you give me a change and don't give up on this?

Thanks
Xiaoyu
-----Original Message-----
From: devel at edk2.groups.io [mailto:devel at edk2.groups.io] On Behalf Of Laszlo Ersek
Sent: Tuesday, April 30, 2019 11:32 PM
To: Lu, XiaoyuX <xiaoyux.lu at intel.com>; devel at edk2.groups.io
Cc: Wang, Jian J <jian.j.wang at intel.com>; Ye, Ting <ting.ye at intel.com>
Subject: Re: [edk2-devel] [PATCH 2/3] CryptoPkg: Upgrade openssl to 1.1.1b

Hi,

On 04/30/19 12:00, Lu, XiaoyuX wrote:
> Thank you Laszlo.
> I will modify the pointed out.
> 
> (1): 
> When OPENSSL_SYS_UEFI is defined, NO_SYSLOG will be defined (e_os.h line 47).
> 
> (2):
> OpenSSL only support seeding NONE in UEFI(rand_unix.c line 93). 
> This flag will not be added into opensslconf.h automatically after run 
> openssl Configure script with no-seed paramater. so add it manually.

For the above two points, please realize that I'm asking for OpenSSL *commit* references, not just code locations. It means that once you locate the code responsible, you should please run "git blame" on the affected code location, and mention the OpenSSL *git commit hash* in the edk2 patch that is responsible for the change.

For example: in order to address my request (1), you would run

$ git blame -- e_os.h

then look at the following context:

     44 cff55b90e95e1 (Qin Long                2017-03-15 23:33:57 +0800  44) # if defined(OPENSSL_SYS_VXWORKS) || defined(OPENSSL_SYS_UEFI)
     45 3e83e686ba2e2 (Richard Levitte         2002-02-14 15:37:38 +0000  45) #  define NO_CHMOD
     46 3e83e686ba2e2 (Richard Levitte         2002-02-14 15:37:38 +0000  46) #  define NO_SYSLOG
     47 0f113f3ee4d62 (Matt Caswell            2015-01-22 03:40:55 +0000  47) # endif

and then write in the commit message,

"""
Due to OpenSSL commit cff55b90e95e ("Cleaning UEFI Build with additional OPENSSL_SYS_UEFI flags", 2017-03-29), we can remove NO_SYSLOG from the INF file.
"""

And then, when I review the patch, I can run

$ git show cff55b90e95e

on my end, and confirm that you are correct.

The same applies to every other request, where I ask for an upstream OpenSSL *commit* reference.

> 
> (3):
> Although this variable has already been set in some build environment, 
> this is just for OpenSSL compilation for other environment which not defined the flag.

This doesn't answer my questions / requests. I made two comments:

(a) please consider spelling out these gcc warning options with finer granularity, that is, to cover *only* the following toolchains:

- DEBUG_(GCC48|GCC49)_(ARM|AARCH64)   -- 4 cases
- DEBUG_(GCC48|GCC49|GCC5)_(IA32|X64) -- 6 cases

but, I'd be fine if the CryptoPkg maintainer disagreed with me, and preferred your current solution.

(b) regardless of *how* you add these flags, with coarse granularity or fine granularity, please split them to a separate patch, before the current patch.

> (4):
> agree
> 
> (5):
> My mistake. I will change the commit messages.
> 
> (6):
> Between OpenSSL_1_1_0i(97c0959f27b294fe1eb10b547145ebef2524b896) and 
> OpenSSL_1_1_1b(50eaac9f3337667259de725451f201e784599687), OpenSSL 
> updated DRBG / RAND to request nonce and additional low entropy 
> randomness from system(line 229 openssl/CHANGES).
> 
> Since OpenSSL_1_1_1b doesn't fully implement randomness generation for UEFI.
> So I added these functions.

Let me re-state my point (6), so it's clearer:

- the commit message on the patch currently states something that is not true (because we are not implementing functions that the commit message lists)

- the commit message on the patch does not state functions that we *do* implement.

- it's indeed very useful if you point out *when* the separate random pool requirement was formed in OpenSSL, but the above code location is insufficient, in itself. Please provide an OpenSSL commit hash, using "git blame". (Hopefully, the OpenSSL commit will also lead the reader to an OpenSSL pull request.)

> 
> (7):
> BUFSIZ used in evp_key.c 
> OpenSSL_1_1_1b(50eaac9f3337667259de725451f201e784599687)
> This is defined in CRT library(stdio.h).

I think you misunderstood my question.

My question is: why is it necessary for edk2 to define BUFSIZ *now*, when the edk2 #define used to be unnecessary before?

What is the OpenSSL change *exactly* (= the OpenSSL commit) that forces us to define BUFSIZ *now*?

And once we identify that change, can we determine if it is an intentional change in OpenSSL (that is, an explicit new requirement on edk2), or a bug in OpenSSL (i.e. edk2 shouldn't be expected, in fact, to #define BUFSIZ)?

Please consider the following (ancient!) OpenSSL commit:

commit a63d5eaab28a20463818b43a76ce8acd19d58812
Author: Richard Levitte <levitte at openssl.org>
Date:   Sun May 6 23:19:37 2001 +0000

    Add a general user interface API.  This is designed to replace things
    like des_read_password and friends (backward compatibility functions
    using this new API are provided).  The purpose is to remove prompting
    functions from the DES code section as well as provide for prompting
    through dialog boxes in a window system and the like.

This commit added multiple BUFSIZ references to "evp_key.c' in the year *2001*.

$ git show a63d5eaab28a -- crypto/evp/evp_key.c

So why is it necessary for edk2 to #define BUFSIZ only now? What is the *recent* OpenSSL commit that makes it necessary?

> 
> (8):
> agree
> 
> (9):
> My mistake. I will remove it.
> 
> (10):
> My mistake.
> 
> (11):
> I think the random function in openssl is not very import, because it isn't used to generate keys in UEFI, it is only used to verify things. 
> So I dummy implement it.
> 
> I will consult relevant experts how to implement it. 
> 
> Thank you for your review. And Sorry for my stupid function implementation.

I never said (or implied) it was stupid -- it is naive, yes, and insecure, but I don't use the word "stupid" on this mailing list.

  https://www.tianocore.org/coc.html

I did say,

"If we ever ship code like this, we'll be the laughing stock of the security world, even though the code is clearly 100% well-intended, i.e. as a stub only"

which I think was not offensive. A good portion of the security analyst population is known to be abrasive, and prone to ridicule naive solutions. My statement didn't mean that *I* would laugh at the code, quite the opposite: I stated the code was clearly 100% well-intended.

If my comment still felt offensive, I apologize; it wasn't my intent.

Thanks
Laszlo

> 
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek at redhat.com]
> Sent: Tuesday, April 30, 2019 2:02 AM
> To: devel at edk2.groups.io; Lu, XiaoyuX <xiaoyux.lu at intel.com>
> Cc: Wang, Jian J <jian.j.wang at intel.com>; Ye, Ting <ting.ye at intel.com>
> Subject: Re: [edk2-devel] [PATCH 2/3] CryptoPkg: Upgrade openssl to 
> 1.1.1b
> 
> Preliminary comments:
> 
> On 04/29/19 10:15, Xiaoyu lu wrote:
>> From: Xiaoyu Lu <xiaoyux.lu at intel.com>
>>
>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1089
>>
>> * update openssl submodule to OpenSSL_1_1_1b
> 
> Seems OK (at 50eaac9f3337667259de725451f201e784599687).
> 
>> * run process_files.pl script to regenerate OpensslLib[Crypto].inf
> 
> OK, I agree this has to be in the same commit as the submodule update.
> 
>> * remove NO_SYSLOG from OpensslLib[Crypto].inf
> 
> (1) In the commit message, indicate the upstream OpenSSL commit that justifies the edk2 change.
> 
>> * add -DOPENSSL_RAND_SEED_NONE to OpensslLib[Crypto].inf
> 
> (2) The upstream OpenSSL change justifying this change should be identified in the commit message, please.
> 
>> * add -Wno-error=unused-but-set-variable for GCC in 
>> OpensslLib[Crypto].inf
> 
> (3) I don't understand why this is necessary. In the INF files, under 
> [BuildOptions], we use the "=", not "==" operator, to append (not
> overwrite) the build options.
> 
> And "-Wno-unused-but-set-variable" is already part of "BaseTools/Conf/tools_def.template"...
> 
> Ah, building OpenSSL requires wider coverage than what we prefer in "BaseTools/Conf/tools_def.template". Namely, the following build configs are (intentionally) not silenced in BaseTools:
> 
> - DEBUG_(GCC48|GCC49)_(ARM|AARCH64)   -- 4 cases
> - DEBUG_(GCC48|GCC49|GCC5)_(IA32|X64) -- 6 cases
> 
> I think I'd prefer if we didn't include redundant build options, and only specified in OpensslLib*.inf what's not covered in BaseTools. But, I'll let the CryptoPkg maintainers decide, of course.
> 
> 
> Regardless of the options we choose, I think this change (and the parallel change for VS) should be done in a separate patch, *before* advancing the OpenSSL submodule. The pre-patch submodule will build just fine with these additional "silencer" flags, and putting them in place before the submodule update helps decrease the size of the important patch.
> 
>> * add /wd4132 /wd4700 /wd4310 for Visual Studio compiler in 
>> OpensslLib[Crypto].inf
>> * add compiler_flags to buildinf.h file.
> 
> (4) I guess this has to be in the patch that advances the submodule, yes.
> 
> (Technically we could add it earlier, but it's just a small hunk, and wouldn't make much sense in separation).
> 
> However, please provide an upstream OpenSSL commit hash, in the edk2 commit message, that requires this change.
> 
> 
>> openssl 1.1.1b doesn't implement randomness generation for UEFI.
>> So add a new file(rand_pool.c) to implement these following functions.
> 
> (5) Please provide an OpenSSL commit reference that requires this functionality in edk2.
> 
>> * do_rand_init
>> * rand_drbg_get_nonce
>> * rand_drbg_get_entropy
>> * rand_drbg_get_additional_data
> 
> (6) This patch implements none of the above functions.
> 
> The patch does implement:
> - rand_pool_acquire_entropy
> - rand_pool_add_nonce_data
> - rand_pool_add_additional_data
> - rand_pool_init
> - rand_pool_cleanup
> - rand_pool_keep_random_devices_open
> 
> but those are not documented in the commit message.
> 
> If the functions that you listed in the commit message call our dummy functions, that's fine, we can point that out separately, but we're not implementing do_rand_init or rand_drbg_*.
> 
> Please clean up the commit message, including a mention of the OpenSSL commit where the platform-specific random pool feature was introduced.
> 
> (More on this below.)
> 
> 
>> We don't need ossl_store functions. So dummy implement them.
>> add a new file(ossl_store.c) to implement ossl_store_cleanup_int function.
>>
>> Cc: Jian J Wang <jian.j.wang at intel.com>
>> Cc: Ting Ye <ting.ye at intel.com>
>> Signed-off-by: Xiaoyu Lu <xiaoyux.lu at intel.com>
>> ---
>>  CryptoPkg/Library/Include/CrtLibSupport.h         |  7 +++
>>  CryptoPkg/Library/Include/openssl/opensslconf.h   | 54 +++++++++++++-----
>>  CryptoPkg/Library/OpensslLib/OpensslLib.inf       | 60 +++++++++++++++-----
>>  CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf | 51 +++++++++++++----
>>  CryptoPkg/Library/OpensslLib/buildinf.h           |  2 +
>>  CryptoPkg/Library/OpensslLib/openssl              |  2 +-
>>  CryptoPkg/Library/OpensslLib/ossl_store.c         | 21 +++++++
>>  CryptoPkg/Library/OpensslLib/rand_pool.c          | 69 +++++++++++++++++++++++
>>  8 files changed, 225 insertions(+), 41 deletions(-)  create mode
>> 100644 CryptoPkg/Library/OpensslLib/ossl_store.c
>>  create mode 100644 CryptoPkg/Library/OpensslLib/rand_pool.c
>>
>> diff --git a/CryptoPkg/Library/Include/CrtLibSupport.h
>> b/CryptoPkg/Library/Include/CrtLibSupport.h
>> index b05c5d9..7c73a0c 100644
>> --- a/CryptoPkg/Library/Include/CrtLibSupport.h
>> +++ b/CryptoPkg/Library/Include/CrtLibSupport.h
>> @@ -21,6 +21,13 @@ SPDX-License-Identifier: BSD-2-Clause-Patent 
>> #define MAX_STRING_SIZE  0x1000
>>  
>>  //
>> +// Default buffer size used in evp_key.c // #ifndef BUFSIZ #define 
>> +BUFSIZ  8192 #endif
>> +
>> +//
>>  // OpenSSL relies on explicit configuration for word size in 
>> crypto/bn,  // but we want it to be automatically inferred from the 
>> target. So we  // bypass what's in <openssl/opensslconf.h> for 
>> OPENSSL_SYS_UEFI, and
> 
> (7) Is this a bug in OpenSSL (that is, absence of BUFSIZ), or really a new requirement?
> 
> If it's a bug, we should file a ticket, and reference that in the commit message.
> 
> If it is a justified requirement, then we should identify the OpenSSL commit hash in the commit message.
> 
>> diff --git a/CryptoPkg/Library/Include/openssl/opensslconf.h
>> b/CryptoPkg/Library/Include/openssl/opensslconf.h
>> index 28dd9ab..570319b 100644
>> --- a/CryptoPkg/Library/Include/openssl/opensslconf.h
>> +++ b/CryptoPkg/Library/Include/openssl/opensslconf.h
>> @@ -10,6 +10,8 @@
>>   * https://www.openssl.org/source/license.html
>>   */
>>  
>> +#include <openssl/opensslv.h>
>> +
>>  #ifdef  __cplusplus
>>  extern "C" {
>>  #endif
>> @@ -77,18 +79,21 @@ extern "C" {
>>  #ifndef OPENSSL_NO_SEED
>>  # define OPENSSL_NO_SEED
>>  #endif
>> +#ifndef OPENSSL_NO_SM2
>> +# define OPENSSL_NO_SM2
>> +#endif
>>  #ifndef OPENSSL_NO_SRP
>>  # define OPENSSL_NO_SRP
>>  #endif
>>  #ifndef OPENSSL_NO_TS
>>  # define OPENSSL_NO_TS
>>  #endif
>> -#ifndef OPENSSL_NO_UI
>> -# define OPENSSL_NO_UI
>> -#endif
>>  #ifndef OPENSSL_NO_WHIRLPOOL
>>  # define OPENSSL_NO_WHIRLPOOL
>>  #endif
>> +#ifndef OPENSSL_RAND_SEED_OS
>> +# define OPENSSL_RAND_SEED_OS
>> +#endif
>>  #ifndef OPENSSL_NO_AFALGENG
>>  # define OPENSSL_NO_AFALGENG
>>  #endif
>> @@ -122,6 +127,9 @@ extern "C" {
>>  #ifndef OPENSSL_NO_DEPRECATED
>>  # define OPENSSL_NO_DEPRECATED
>>  #endif
>> +#ifndef OPENSSL_NO_DEVCRYPTOENG
>> +# define OPENSSL_NO_DEVCRYPTOENG
>> +#endif
>>  #ifndef OPENSSL_NO_DGRAM
>>  # define OPENSSL_NO_DGRAM
>>  #endif
>> @@ -155,6 +163,9 @@ extern "C" {
>>  #ifndef OPENSSL_NO_ERR
>>  # define OPENSSL_NO_ERR
>>  #endif
>> +#ifndef OPENSSL_NO_EXTERNAL_TESTS
>> +# define OPENSSL_NO_EXTERNAL_TESTS
>> +#endif
>>  #ifndef OPENSSL_NO_FILENAMES
>>  # define OPENSSL_NO_FILENAMES
>>  #endif
>> @@ -209,15 +220,24 @@ extern "C" {
>>  #ifndef OPENSSL_NO_TESTS
>>  # define OPENSSL_NO_TESTS
>>  #endif
>> +#ifndef OPENSSL_NO_TLS1_3
>> +# define OPENSSL_NO_TLS1_3
>> +#endif
>>  #ifndef OPENSSL_NO_UBSAN
>>  # define OPENSSL_NO_UBSAN
>>  #endif
>> +#ifndef OPENSSL_NO_UI_CONSOLE
>> +# define OPENSSL_NO_UI_CONSOLE
>> +#endif
>>  #ifndef OPENSSL_NO_UNIT_TEST
>>  # define OPENSSL_NO_UNIT_TEST
>>  #endif
>>  #ifndef OPENSSL_NO_WEAK_SSL_CIPHERS
>>  # define OPENSSL_NO_WEAK_SSL_CIPHERS  #endif
>> +#ifndef OPENSSL_NO_DYNAMIC_ENGINE
>> +# define OPENSSL_NO_DYNAMIC_ENGINE
>> +#endif
>>  #ifndef OPENSSL_NO_AFALGENG
>>  # define OPENSSL_NO_AFALGENG
>>  #endif
>> @@ -236,15 +256,11 @@ extern "C" {
>>   * functions.
>>   */
>>  #ifndef DECLARE_DEPRECATED
>> -# if defined(OPENSSL_NO_DEPRECATED)
>> -#  define DECLARE_DEPRECATED(f)
>> -# else
>> -#  define DECLARE_DEPRECATED(f)   f;
>> -#  ifdef __GNUC__
>> -#   if __GNUC__ > 3 || (__GNUC__ == 3 && __GNUC_MINOR__ > 0)
>> -#    undef DECLARE_DEPRECATED
>> -#    define DECLARE_DEPRECATED(f)    f __attribute__ ((deprecated));
>> -#   endif
>> +# define DECLARE_DEPRECATED(f)   f;
>> +# ifdef __GNUC__
>> +#  if __GNUC__ > 3 || (__GNUC__ == 3 && __GNUC_MINOR__ > 0)
>> +#   undef DECLARE_DEPRECATED
>> +#   define DECLARE_DEPRECATED(f)    f __attribute__ ((deprecated));
>>  #  endif
>>  # endif
>>  #endif
>> @@ -268,6 +284,18 @@ extern "C" {
>>  # define OPENSSL_API_COMPAT OPENSSL_MIN_API  #endif
>>  
>> +/*
>> + * Do not deprecate things to be deprecated in version 1.2.0 before 
>> +the
>> + * OpenSSL version number matches.
>> + */
>> +#if OPENSSL_VERSION_NUMBER < 0x10200000L
>> +# define DEPRECATEDIN_1_2_0(f)   f;
>> +#elif OPENSSL_API_COMPAT < 0x10200000L
>> +# define DEPRECATEDIN_1_2_0(f)   DECLARE_DEPRECATED(f)
>> +#else
>> +# define DEPRECATEDIN_1_2_0(f)
>> +#endif
>> +
>>  #if OPENSSL_API_COMPAT < 0x10100000L
>>  # define DEPRECATEDIN_1_1_0(f)   DECLARE_DEPRECATED(f)
>>  #else
>> @@ -286,8 +314,6 @@ extern "C" {
>>  # define DEPRECATEDIN_0_9_8(f)
>>  #endif
>>  
>> -
>> -
>>  /* Generate 80386 code? */
>>  #undef I386_ONLY
>>  
> 
> My understanding is that "opensslconf.h" is a generated file, so I 
> won't ask for comments on these changes. However,
> 
> (8) please go back to the part of the commit message where it mentions "process_files.pl", and please also state that "opensslconf.h" is regenerated, in addition to the file lists in the INF files.
> 
>> diff --git a/CryptoPkg/Library/OpensslLib/OpensslLib.inf
>> b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
>> index 530ac5f..aee9032 100644
>> --- a/CryptoPkg/Library/OpensslLib/OpensslLib.inf
>> +++ b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
>> @@ -15,13 +15,15 @@
>>    VERSION_STRING                 = 1.0
>>    LIBRARY_CLASS                  = OpensslLib
>>    DEFINE OPENSSL_PATH            = openssl
>> -  DEFINE OPENSSL_FLAGS           = -DL_ENDIAN -DOPENSSL_SMALL_FOOTPRINT -D_CRT_SECURE_NO_DEPRECATE -D_CRT_NONSTDC_NO_DEPRECATE -DNO_SYSLOG
>> +  DEFINE OPENSSL_FLAGS           = -DL_ENDIAN -DOPENSSL_SMALL_FOOTPRINT -D_CRT_SECURE_NO_DEPRECATE -D_CRT_NONSTDC_NO_DEPRECATE -DOPENSSL_RAND_SEED_NONE
>>  
>>  #
>>  #  VALID_ARCHITECTURES           = IA32 X64 ARM AARCH64
>>  #
>>  
>>  [Sources]
>> +  ossl_store.c
>> +  rand_pool.c
>>    $(OPENSSL_PATH)/e_os.h
>>  # Autogenerated files list starts here
>>    $(OPENSSL_PATH)/crypto/aes/aes_cbc.c
>> @@ -32,6 +34,7 @@
>>    $(OPENSSL_PATH)/crypto/aes/aes_misc.c
>>    $(OPENSSL_PATH)/crypto/aes/aes_ofb.c
>>    $(OPENSSL_PATH)/crypto/aes/aes_wrap.c
>> +  $(OPENSSL_PATH)/crypto/aria/aria.c
>>    $(OPENSSL_PATH)/crypto/asn1/a_bitstr.c
>>    $(OPENSSL_PATH)/crypto/asn1/a_d2i_fp.c
>>    $(OPENSSL_PATH)/crypto/asn1/a_digest.c
>> @@ -54,6 +57,7 @@
>>    $(OPENSSL_PATH)/crypto/asn1/ameth_lib.c
>>    $(OPENSSL_PATH)/crypto/asn1/asn1_err.c
>>    $(OPENSSL_PATH)/crypto/asn1/asn1_gen.c
>> +  $(OPENSSL_PATH)/crypto/asn1/asn1_item_list.c
>>    $(OPENSSL_PATH)/crypto/asn1/asn1_lib.c
>>    $(OPENSSL_PATH)/crypto/asn1/asn1_par.c
>>    $(OPENSSL_PATH)/crypto/asn1/asn_mime.c
>> @@ -172,6 +176,7 @@
>>    $(OPENSSL_PATH)/crypto/conf/conf_ssl.c
>>    $(OPENSSL_PATH)/crypto/cpt_err.c
>>    $(OPENSSL_PATH)/crypto/cryptlib.c
>> +  $(OPENSSL_PATH)/crypto/ctype.c
>>    $(OPENSSL_PATH)/crypto/cversion.c
>>    $(OPENSSL_PATH)/crypto/des/cbc_cksm.c
>>    $(OPENSSL_PATH)/crypto/des/cbc_enc.c
>> @@ -189,7 +194,6 @@
>>    $(OPENSSL_PATH)/crypto/des/pcbc_enc.c
>>    $(OPENSSL_PATH)/crypto/des/qud_cksm.c
>>    $(OPENSSL_PATH)/crypto/des/rand_key.c
>> -  $(OPENSSL_PATH)/crypto/des/rpc_enc.c
>>    $(OPENSSL_PATH)/crypto/des/set_key.c
>>    $(OPENSSL_PATH)/crypto/des/str2key.c
>>    $(OPENSSL_PATH)/crypto/des/xcbc_enc.c
>> @@ -206,6 +210,7 @@
>>    $(OPENSSL_PATH)/crypto/dh/dh_pmeth.c
>>    $(OPENSSL_PATH)/crypto/dh/dh_prn.c
>>    $(OPENSSL_PATH)/crypto/dh/dh_rfc5114.c
>> +  $(OPENSSL_PATH)/crypto/dh/dh_rfc7919.c
>>    $(OPENSSL_PATH)/crypto/dso/dso_dl.c
>>    $(OPENSSL_PATH)/crypto/dso/dso_dlfcn.c
>>    $(OPENSSL_PATH)/crypto/dso/dso_err.c
>> @@ -228,6 +233,7 @@
>>    $(OPENSSL_PATH)/crypto/evp/e_aes.c
>>    $(OPENSSL_PATH)/crypto/evp/e_aes_cbc_hmac_sha1.c
>>    $(OPENSSL_PATH)/crypto/evp/e_aes_cbc_hmac_sha256.c
>> +  $(OPENSSL_PATH)/crypto/evp/e_aria.c
>>    $(OPENSSL_PATH)/crypto/evp/e_bf.c
>>    $(OPENSSL_PATH)/crypto/evp/e_camellia.c
>>    $(OPENSSL_PATH)/crypto/evp/e_cast.c
>> @@ -242,6 +248,7 @@
>>    $(OPENSSL_PATH)/crypto/evp/e_rc4_hmac_md5.c
>>    $(OPENSSL_PATH)/crypto/evp/e_rc5.c
>>    $(OPENSSL_PATH)/crypto/evp/e_seed.c
>> +  $(OPENSSL_PATH)/crypto/evp/e_sm4.c
>>    $(OPENSSL_PATH)/crypto/evp/e_xcbc_d.c
>>    $(OPENSSL_PATH)/crypto/evp/encode.c
>>    $(OPENSSL_PATH)/crypto/evp/evp_cnf.c
>> @@ -259,6 +266,7 @@
>>    $(OPENSSL_PATH)/crypto/evp/m_null.c
>>    $(OPENSSL_PATH)/crypto/evp/m_ripemd.c
>>    $(OPENSSL_PATH)/crypto/evp/m_sha1.c
>> +  $(OPENSSL_PATH)/crypto/evp/m_sha3.c
>>    $(OPENSSL_PATH)/crypto/evp/m_sigver.c
>>    $(OPENSSL_PATH)/crypto/evp/m_wp.c
>>    $(OPENSSL_PATH)/crypto/evp/names.c
>> @@ -271,10 +279,10 @@
>>    $(OPENSSL_PATH)/crypto/evp/p_seal.c
>>    $(OPENSSL_PATH)/crypto/evp/p_sign.c
>>    $(OPENSSL_PATH)/crypto/evp/p_verify.c
>> +  $(OPENSSL_PATH)/crypto/evp/pbe_scrypt.c
>>    $(OPENSSL_PATH)/crypto/evp/pmeth_fn.c
>>    $(OPENSSL_PATH)/crypto/evp/pmeth_gn.c
>>    $(OPENSSL_PATH)/crypto/evp/pmeth_lib.c
>> -  $(OPENSSL_PATH)/crypto/evp/scrypt.c
>>    $(OPENSSL_PATH)/crypto/ex_data.c
>>    $(OPENSSL_PATH)/crypto/getenv.c
>>    $(OPENSSL_PATH)/crypto/hmac/hm_ameth.c
>> @@ -283,6 +291,7 @@
>>    $(OPENSSL_PATH)/crypto/init.c
>>    $(OPENSSL_PATH)/crypto/kdf/hkdf.c
>>    $(OPENSSL_PATH)/crypto/kdf/kdf_err.c
>> +  $(OPENSSL_PATH)/crypto/kdf/scrypt.c
>>    $(OPENSSL_PATH)/crypto/kdf/tls1_prf.c
>>    $(OPENSSL_PATH)/crypto/lhash/lh_stats.c
>>    $(OPENSSL_PATH)/crypto/lhash/lhash.c
>> @@ -360,14 +369,14 @@
>>    $(OPENSSL_PATH)/crypto/pkcs7/pk7_mime.c
>>    $(OPENSSL_PATH)/crypto/pkcs7/pk7_smime.c
>>    $(OPENSSL_PATH)/crypto/pkcs7/pkcs7err.c
>> -  $(OPENSSL_PATH)/crypto/rand/md_rand.c
>> +  $(OPENSSL_PATH)/crypto/rand/drbg_ctr.c
>> +  $(OPENSSL_PATH)/crypto/rand/drbg_lib.c
>>    $(OPENSSL_PATH)/crypto/rand/rand_egd.c
>>    $(OPENSSL_PATH)/crypto/rand/rand_err.c
>>    $(OPENSSL_PATH)/crypto/rand/rand_lib.c
>>    $(OPENSSL_PATH)/crypto/rand/rand_unix.c
>>    $(OPENSSL_PATH)/crypto/rand/rand_vms.c
>>    $(OPENSSL_PATH)/crypto/rand/rand_win.c
>> -  $(OPENSSL_PATH)/crypto/rand/randfile.c
>>    $(OPENSSL_PATH)/crypto/rc4/rc4_enc.c
>>    $(OPENSSL_PATH)/crypto/rc4/rc4_skey.c
>>    $(OPENSSL_PATH)/crypto/rsa/rsa_ameth.c
>> @@ -379,8 +388,8 @@
>>    $(OPENSSL_PATH)/crypto/rsa/rsa_gen.c
>>    $(OPENSSL_PATH)/crypto/rsa/rsa_lib.c
>>    $(OPENSSL_PATH)/crypto/rsa/rsa_meth.c
>> +  $(OPENSSL_PATH)/crypto/rsa/rsa_mp.c
>>    $(OPENSSL_PATH)/crypto/rsa/rsa_none.c
>> -  $(OPENSSL_PATH)/crypto/rsa/rsa_null.c
>>    $(OPENSSL_PATH)/crypto/rsa/rsa_oaep.c
>>    $(OPENSSL_PATH)/crypto/rsa/rsa_ossl.c
>>    $(OPENSSL_PATH)/crypto/rsa/rsa_pk1.c
>> @@ -392,15 +401,27 @@
>>    $(OPENSSL_PATH)/crypto/rsa/rsa_ssl.c
>>    $(OPENSSL_PATH)/crypto/rsa/rsa_x931.c
>>    $(OPENSSL_PATH)/crypto/rsa/rsa_x931g.c
>> +  $(OPENSSL_PATH)/crypto/sha/keccak1600.c
>>    $(OPENSSL_PATH)/crypto/sha/sha1_one.c
>>    $(OPENSSL_PATH)/crypto/sha/sha1dgst.c
>>    $(OPENSSL_PATH)/crypto/sha/sha256.c
>>    $(OPENSSL_PATH)/crypto/sha/sha512.c
>> +  $(OPENSSL_PATH)/crypto/siphash/siphash.c
>> +  $(OPENSSL_PATH)/crypto/siphash/siphash_ameth.c
>> +  $(OPENSSL_PATH)/crypto/siphash/siphash_pmeth.c
>> +  $(OPENSSL_PATH)/crypto/sm3/m_sm3.c
>> +  $(OPENSSL_PATH)/crypto/sm3/sm3.c
>> +  $(OPENSSL_PATH)/crypto/sm4/sm4.c
>>    $(OPENSSL_PATH)/crypto/stack/stack.c
>>    $(OPENSSL_PATH)/crypto/threads_none.c
>>    $(OPENSSL_PATH)/crypto/threads_pthread.c
>>    $(OPENSSL_PATH)/crypto/threads_win.c
>>    $(OPENSSL_PATH)/crypto/txt_db/txt_db.c
>> +  $(OPENSSL_PATH)/crypto/ui/ui_err.c
>> +  $(OPENSSL_PATH)/crypto/ui/ui_lib.c
>> +  $(OPENSSL_PATH)/crypto/ui/ui_null.c
>> +  $(OPENSSL_PATH)/crypto/ui/ui_openssl.c
>> +  $(OPENSSL_PATH)/crypto/ui/ui_util.c
>>    $(OPENSSL_PATH)/crypto/uid.c
>>    $(OPENSSL_PATH)/crypto/x509/by_dir.c
>>    $(OPENSSL_PATH)/crypto/x509/by_file.c
>> @@ -445,6 +466,7 @@
>>    $(OPENSSL_PATH)/crypto/x509v3/pcy_node.c
>>    $(OPENSSL_PATH)/crypto/x509v3/pcy_tree.c
>>    $(OPENSSL_PATH)/crypto/x509v3/v3_addr.c
>> +  $(OPENSSL_PATH)/crypto/x509v3/v3_admis.c
>>    $(OPENSSL_PATH)/crypto/x509v3/v3_akey.c
>>    $(OPENSSL_PATH)/crypto/x509v3/v3_akeya.c
>>    $(OPENSSL_PATH)/crypto/x509v3/v3_alt.c
>> @@ -479,12 +501,14 @@
>>    $(OPENSSL_PATH)/ssl/d1_msg.c
>>    $(OPENSSL_PATH)/ssl/d1_srtp.c
>>    $(OPENSSL_PATH)/ssl/methods.c
>> +  $(OPENSSL_PATH)/ssl/packet.c
>>    $(OPENSSL_PATH)/ssl/pqueue.c
>>    $(OPENSSL_PATH)/ssl/record/dtls1_bitmap.c
>>    $(OPENSSL_PATH)/ssl/record/rec_layer_d1.c
>>    $(OPENSSL_PATH)/ssl/record/rec_layer_s3.c
>>    $(OPENSSL_PATH)/ssl/record/ssl3_buffer.c
>>    $(OPENSSL_PATH)/ssl/record/ssl3_record.c
>> +  $(OPENSSL_PATH)/ssl/record/ssl3_record_tls13.c
>>    $(OPENSSL_PATH)/ssl/s3_cbc.c
>>    $(OPENSSL_PATH)/ssl/s3_enc.c
>>    $(OPENSSL_PATH)/ssl/s3_lib.c
>> @@ -502,16 +526,19 @@
>>    $(OPENSSL_PATH)/ssl/ssl_stat.c
>>    $(OPENSSL_PATH)/ssl/ssl_txt.c
>>    $(OPENSSL_PATH)/ssl/ssl_utst.c
>> +  $(OPENSSL_PATH)/ssl/statem/extensions.c
>> +  $(OPENSSL_PATH)/ssl/statem/extensions_clnt.c
>> +  $(OPENSSL_PATH)/ssl/statem/extensions_cust.c
>> +  $(OPENSSL_PATH)/ssl/statem/extensions_srvr.c
>>    $(OPENSSL_PATH)/ssl/statem/statem.c
>>    $(OPENSSL_PATH)/ssl/statem/statem_clnt.c
>>    $(OPENSSL_PATH)/ssl/statem/statem_dtls.c
>>    $(OPENSSL_PATH)/ssl/statem/statem_lib.c
>>    $(OPENSSL_PATH)/ssl/statem/statem_srvr.c
>>    $(OPENSSL_PATH)/ssl/t1_enc.c
>> -  $(OPENSSL_PATH)/ssl/t1_ext.c
>>    $(OPENSSL_PATH)/ssl/t1_lib.c
>> -  $(OPENSSL_PATH)/ssl/t1_reneg.c
>>    $(OPENSSL_PATH)/ssl/t1_trce.c
>> +  $(OPENSSL_PATH)/ssl/tls13_enc.c
>>    $(OPENSSL_PATH)/ssl/tls_srp.c
>>  # Autogenerated files list ends here
>>  
>> @@ -521,6 +548,7 @@
>>  
>>  [LibraryClasses]
>>    DebugLib
>> +  IntrinsicLib
> 
> (9) IntrinsicLib makes sense (from the previous patch), but the commit message on this patch should state why exactly it is needed.
> 
> What calls _ftol2() in OpenSSL? And which commit introduced it?
> 
>>  
>>  [LibraryClasses.ARM]
>>    ArmSoftFloatLib
>> @@ -530,17 +558,20 @@
>>    # Disables the following Visual Studio compiler warnings brought by openssl source,
>>    # so we do not break the build with /WX option:
>>    #   C4090: 'function' : different 'const' qualifiers
>> +  #   C4132: 'object' : const object should be initialized (tls13_enc.c)
>>    #   C4244: conversion from type1 to type2, possible loss of data
>>    #   C4245: conversion from type1 to type2, signed/unsigned mismatch
>>    #   C4267: conversion from size_t to type, possible loss of data
>>    #   C4306: 'identifier' : conversion from 'type1' to 'type2' of greater size
>> +  #   C4310: cast truncates constant value
>>    #   C4389: 'operator' : signed/unsigned mismatch (xxxx)
>> +  #   C4700: uninitialized local variable 'name' used. (conf_sap.c(71))
>>    #   C4702: unreachable code
>>    #   C4706: assignment within conditional expression
>>    #   C4819: The file contains a character that cannot be represented in the current code page
>>    #
>> -  MSFT:*_*_IA32_CC_FLAGS   = -U_WIN32 -U_WIN64 -U_MSC_VER $(OPENSSL_FLAGS) /wd4090 /wd4244 /wd4245 /wd4267 /wd4389 /wd4702 /wd4706 /wd4819
>> -  MSFT:*_*_X64_CC_FLAGS    = -U_WIN32 -U_WIN64 -U_MSC_VER $(OPENSSL_FLAGS) /wd4090 /wd4244 /wd4245 /wd4267 /wd4306 /wd4389 /wd4702 /wd4706 /wd4819
>> +  MSFT:*_*_IA32_CC_FLAGS   = -U_WIN32 -U_WIN64 -U_MSC_VER $(OPENSSL_FLAGS) /wd4090 /wd4132 /wd4244 /wd4245 /wd4267 /wd4310 /wd4389 /wd4700 /wd4702 /wd4706 /wd4819
>> +  MSFT:*_*_X64_CC_FLAGS    = -U_WIN32 -U_WIN64 -U_MSC_VER $(OPENSSL_FLAGS) /wd4090 /wd4132 /wd4244 /wd4245 /wd4267 /wd4306 /wd4310 /wd4700 /wd4389 /wd4702 /wd4706 /wd4819
>>  
>>    INTEL:*_*_IA32_CC_FLAGS  = -U_WIN32 -U_WIN64 -U_MSC_VER -U__ICC $(OPENSSL_FLAGS) /w
>>    INTEL:*_*_X64_CC_FLAGS   = -U_WIN32 -U_WIN64 -U_MSC_VER -U__ICC $(OPENSSL_FLAGS) /w
>> @@ -550,11 +581,12 @@
>>    #   -Werror=maybe-uninitialized: there exist some other paths for which the variable is not initialized.
>>    #   -Werror=format: Check calls to printf and scanf, etc., to make sure that the arguments supplied have
>>    #                   types appropriate to the format string specified.
>> +  #   -Werror=unused-but-set-variable: Warn whenever a local variable is assigned to, but otherwise unused (aside from its declaration).
>>    #
>> -  GCC:*_*_IA32_CC_FLAGS    = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -Wno-error=maybe-uninitialized
>> -  GCC:*_*_X64_CC_FLAGS     = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -Wno-error=maybe-uninitialized -Wno-error=format -Wno-format -DNO_MSABI_VA_FUNCS
>> -  GCC:*_*_ARM_CC_FLAGS     = $(OPENSSL_FLAGS) -Wno-error=maybe-uninitialized
>> -  GCC:*_*_AARCH64_CC_FLAGS = $(OPENSSL_FLAGS) 
>> -Wno-error=maybe-uninitialized -Wno-format
>> +  GCC:*_*_IA32_CC_FLAGS    = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -Wno-error=maybe-uninitialized -Wno-error=unused-but-set-variable
>> +  GCC:*_*_X64_CC_FLAGS     = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -Wno-error=maybe-uninitialized -Wno-error=format -Wno-format -Wno-error=unused-but-set-variable -DNO_MSABI_VA_FUNCS
>> +  GCC:*_*_ARM_CC_FLAGS     = $(OPENSSL_FLAGS) -Wno-error=maybe-uninitialized -Wno-error=unused-but-set-variable
>> +  GCC:*_*_AARCH64_CC_FLAGS = $(OPENSSL_FLAGS) 
>> + -Wno-error=maybe-uninitialized -Wno-format 
>> + -Wno-error=unused-but-set-variable
>>  
>>    # suppress the following warnings in openssl so we don't break the build with warnings-as-errors:
>>    # 1295: Deprecated declaration <entity> - give arg types diff 
>> --git a/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
>> b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
>> index 2310100..0a51f17 100644
>> --- a/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
>> +++ b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
>> @@ -15,13 +15,15 @@
>>    VERSION_STRING                 = 1.0
>>    LIBRARY_CLASS                  = OpensslLib
>>    DEFINE OPENSSL_PATH            = openssl
>> -  DEFINE OPENSSL_FLAGS           = -DL_ENDIAN -DOPENSSL_SMALL_FOOTPRINT -D_CRT_SECURE_NO_DEPRECATE -D_CRT_NONSTDC_NO_DEPRECATE -DNO_SYSLOG
>> +  DEFINE OPENSSL_FLAGS           = -DL_ENDIAN -DOPENSSL_SMALL_FOOTPRINT -D_CRT_SECURE_NO_DEPRECATE -D_CRT_NONSTDC_NO_DEPRECATE -DOPENSSL_RAND_SEED_NONE
>>  
>>  #
>>  #  VALID_ARCHITECTURES           = IA32 X64 ARM AARCH64
>>  #
>>  
>>  [Sources]
>> +  ossl_store.c
>> +  rand_pool.c
>>    $(OPENSSL_PATH)/e_os.h
>>  # Autogenerated files list starts here
>>    $(OPENSSL_PATH)/crypto/aes/aes_cbc.c
>> @@ -32,6 +34,7 @@
>>    $(OPENSSL_PATH)/crypto/aes/aes_misc.c
>>    $(OPENSSL_PATH)/crypto/aes/aes_ofb.c
>>    $(OPENSSL_PATH)/crypto/aes/aes_wrap.c
>> +  $(OPENSSL_PATH)/crypto/aria/aria.c
>>    $(OPENSSL_PATH)/crypto/asn1/a_bitstr.c
>>    $(OPENSSL_PATH)/crypto/asn1/a_d2i_fp.c
>>    $(OPENSSL_PATH)/crypto/asn1/a_digest.c
>> @@ -54,6 +57,7 @@
>>    $(OPENSSL_PATH)/crypto/asn1/ameth_lib.c
>>    $(OPENSSL_PATH)/crypto/asn1/asn1_err.c
>>    $(OPENSSL_PATH)/crypto/asn1/asn1_gen.c
>> +  $(OPENSSL_PATH)/crypto/asn1/asn1_item_list.c
>>    $(OPENSSL_PATH)/crypto/asn1/asn1_lib.c
>>    $(OPENSSL_PATH)/crypto/asn1/asn1_par.c
>>    $(OPENSSL_PATH)/crypto/asn1/asn_mime.c
>> @@ -172,6 +176,7 @@
>>    $(OPENSSL_PATH)/crypto/conf/conf_ssl.c
>>    $(OPENSSL_PATH)/crypto/cpt_err.c
>>    $(OPENSSL_PATH)/crypto/cryptlib.c
>> +  $(OPENSSL_PATH)/crypto/ctype.c
>>    $(OPENSSL_PATH)/crypto/cversion.c
>>    $(OPENSSL_PATH)/crypto/des/cbc_cksm.c
>>    $(OPENSSL_PATH)/crypto/des/cbc_enc.c
>> @@ -189,7 +194,6 @@
>>    $(OPENSSL_PATH)/crypto/des/pcbc_enc.c
>>    $(OPENSSL_PATH)/crypto/des/qud_cksm.c
>>    $(OPENSSL_PATH)/crypto/des/rand_key.c
>> -  $(OPENSSL_PATH)/crypto/des/rpc_enc.c
>>    $(OPENSSL_PATH)/crypto/des/set_key.c
>>    $(OPENSSL_PATH)/crypto/des/str2key.c
>>    $(OPENSSL_PATH)/crypto/des/xcbc_enc.c
>> @@ -206,6 +210,7 @@
>>    $(OPENSSL_PATH)/crypto/dh/dh_pmeth.c
>>    $(OPENSSL_PATH)/crypto/dh/dh_prn.c
>>    $(OPENSSL_PATH)/crypto/dh/dh_rfc5114.c
>> +  $(OPENSSL_PATH)/crypto/dh/dh_rfc7919.c
>>    $(OPENSSL_PATH)/crypto/dso/dso_dl.c
>>    $(OPENSSL_PATH)/crypto/dso/dso_dlfcn.c
>>    $(OPENSSL_PATH)/crypto/dso/dso_err.c
>> @@ -228,6 +233,7 @@
>>    $(OPENSSL_PATH)/crypto/evp/e_aes.c
>>    $(OPENSSL_PATH)/crypto/evp/e_aes_cbc_hmac_sha1.c
>>    $(OPENSSL_PATH)/crypto/evp/e_aes_cbc_hmac_sha256.c
>> +  $(OPENSSL_PATH)/crypto/evp/e_aria.c
>>    $(OPENSSL_PATH)/crypto/evp/e_bf.c
>>    $(OPENSSL_PATH)/crypto/evp/e_camellia.c
>>    $(OPENSSL_PATH)/crypto/evp/e_cast.c
>> @@ -242,6 +248,7 @@
>>    $(OPENSSL_PATH)/crypto/evp/e_rc4_hmac_md5.c
>>    $(OPENSSL_PATH)/crypto/evp/e_rc5.c
>>    $(OPENSSL_PATH)/crypto/evp/e_seed.c
>> +  $(OPENSSL_PATH)/crypto/evp/e_sm4.c
>>    $(OPENSSL_PATH)/crypto/evp/e_xcbc_d.c
>>    $(OPENSSL_PATH)/crypto/evp/encode.c
>>    $(OPENSSL_PATH)/crypto/evp/evp_cnf.c
>> @@ -259,6 +266,7 @@
>>    $(OPENSSL_PATH)/crypto/evp/m_null.c
>>    $(OPENSSL_PATH)/crypto/evp/m_ripemd.c
>>    $(OPENSSL_PATH)/crypto/evp/m_sha1.c
>> +  $(OPENSSL_PATH)/crypto/evp/m_sha3.c
>>    $(OPENSSL_PATH)/crypto/evp/m_sigver.c
>>    $(OPENSSL_PATH)/crypto/evp/m_wp.c
>>    $(OPENSSL_PATH)/crypto/evp/names.c
>> @@ -271,10 +279,10 @@
>>    $(OPENSSL_PATH)/crypto/evp/p_seal.c
>>    $(OPENSSL_PATH)/crypto/evp/p_sign.c
>>    $(OPENSSL_PATH)/crypto/evp/p_verify.c
>> +  $(OPENSSL_PATH)/crypto/evp/pbe_scrypt.c
>>    $(OPENSSL_PATH)/crypto/evp/pmeth_fn.c
>>    $(OPENSSL_PATH)/crypto/evp/pmeth_gn.c
>>    $(OPENSSL_PATH)/crypto/evp/pmeth_lib.c
>> -  $(OPENSSL_PATH)/crypto/evp/scrypt.c
>>    $(OPENSSL_PATH)/crypto/ex_data.c
>>    $(OPENSSL_PATH)/crypto/getenv.c
>>    $(OPENSSL_PATH)/crypto/hmac/hm_ameth.c
>> @@ -283,6 +291,7 @@
>>    $(OPENSSL_PATH)/crypto/init.c
>>    $(OPENSSL_PATH)/crypto/kdf/hkdf.c
>>    $(OPENSSL_PATH)/crypto/kdf/kdf_err.c
>> +  $(OPENSSL_PATH)/crypto/kdf/scrypt.c
>>    $(OPENSSL_PATH)/crypto/kdf/tls1_prf.c
>>    $(OPENSSL_PATH)/crypto/lhash/lh_stats.c
>>    $(OPENSSL_PATH)/crypto/lhash/lhash.c
>> @@ -360,14 +369,14 @@
>>    $(OPENSSL_PATH)/crypto/pkcs7/pk7_mime.c
>>    $(OPENSSL_PATH)/crypto/pkcs7/pk7_smime.c
>>    $(OPENSSL_PATH)/crypto/pkcs7/pkcs7err.c
>> -  $(OPENSSL_PATH)/crypto/rand/md_rand.c
>> +  $(OPENSSL_PATH)/crypto/rand/drbg_ctr.c
>> +  $(OPENSSL_PATH)/crypto/rand/drbg_lib.c
>>    $(OPENSSL_PATH)/crypto/rand/rand_egd.c
>>    $(OPENSSL_PATH)/crypto/rand/rand_err.c
>>    $(OPENSSL_PATH)/crypto/rand/rand_lib.c
>>    $(OPENSSL_PATH)/crypto/rand/rand_unix.c
>>    $(OPENSSL_PATH)/crypto/rand/rand_vms.c
>>    $(OPENSSL_PATH)/crypto/rand/rand_win.c
>> -  $(OPENSSL_PATH)/crypto/rand/randfile.c
>>    $(OPENSSL_PATH)/crypto/rc4/rc4_enc.c
>>    $(OPENSSL_PATH)/crypto/rc4/rc4_skey.c
>>    $(OPENSSL_PATH)/crypto/rsa/rsa_ameth.c
>> @@ -379,8 +388,8 @@
>>    $(OPENSSL_PATH)/crypto/rsa/rsa_gen.c
>>    $(OPENSSL_PATH)/crypto/rsa/rsa_lib.c
>>    $(OPENSSL_PATH)/crypto/rsa/rsa_meth.c
>> +  $(OPENSSL_PATH)/crypto/rsa/rsa_mp.c
>>    $(OPENSSL_PATH)/crypto/rsa/rsa_none.c
>> -  $(OPENSSL_PATH)/crypto/rsa/rsa_null.c
>>    $(OPENSSL_PATH)/crypto/rsa/rsa_oaep.c
>>    $(OPENSSL_PATH)/crypto/rsa/rsa_ossl.c
>>    $(OPENSSL_PATH)/crypto/rsa/rsa_pk1.c
>> @@ -392,15 +401,27 @@
>>    $(OPENSSL_PATH)/crypto/rsa/rsa_ssl.c
>>    $(OPENSSL_PATH)/crypto/rsa/rsa_x931.c
>>    $(OPENSSL_PATH)/crypto/rsa/rsa_x931g.c
>> +  $(OPENSSL_PATH)/crypto/sha/keccak1600.c
>>    $(OPENSSL_PATH)/crypto/sha/sha1_one.c
>>    $(OPENSSL_PATH)/crypto/sha/sha1dgst.c
>>    $(OPENSSL_PATH)/crypto/sha/sha256.c
>>    $(OPENSSL_PATH)/crypto/sha/sha512.c
>> +  $(OPENSSL_PATH)/crypto/siphash/siphash.c
>> +  $(OPENSSL_PATH)/crypto/siphash/siphash_ameth.c
>> +  $(OPENSSL_PATH)/crypto/siphash/siphash_pmeth.c
>> +  $(OPENSSL_PATH)/crypto/sm3/m_sm3.c
>> +  $(OPENSSL_PATH)/crypto/sm3/sm3.c
>> +  $(OPENSSL_PATH)/crypto/sm4/sm4.c
>>    $(OPENSSL_PATH)/crypto/stack/stack.c
>>    $(OPENSSL_PATH)/crypto/threads_none.c
>>    $(OPENSSL_PATH)/crypto/threads_pthread.c
>>    $(OPENSSL_PATH)/crypto/threads_win.c
>>    $(OPENSSL_PATH)/crypto/txt_db/txt_db.c
>> +  $(OPENSSL_PATH)/crypto/ui/ui_err.c
>> +  $(OPENSSL_PATH)/crypto/ui/ui_lib.c
>> +  $(OPENSSL_PATH)/crypto/ui/ui_null.c
>> +  $(OPENSSL_PATH)/crypto/ui/ui_openssl.c
>> +  $(OPENSSL_PATH)/crypto/ui/ui_util.c
>>    $(OPENSSL_PATH)/crypto/uid.c
>>    $(OPENSSL_PATH)/crypto/x509/by_dir.c
>>    $(OPENSSL_PATH)/crypto/x509/by_file.c
>> @@ -445,6 +466,7 @@
>>    $(OPENSSL_PATH)/crypto/x509v3/pcy_node.c
>>    $(OPENSSL_PATH)/crypto/x509v3/pcy_tree.c
>>    $(OPENSSL_PATH)/crypto/x509v3/v3_addr.c
>> +  $(OPENSSL_PATH)/crypto/x509v3/v3_admis.c
>>    $(OPENSSL_PATH)/crypto/x509v3/v3_akey.c
>>    $(OPENSSL_PATH)/crypto/x509v3/v3_akeya.c
>>    $(OPENSSL_PATH)/crypto/x509v3/v3_alt.c
>> @@ -482,6 +504,7 @@
>>  
>>  [LibraryClasses]
>>    DebugLib
>> +  IntrinsicLib
>>  
>>  [LibraryClasses.ARM]
>>    ArmSoftFloatLib
>> @@ -491,17 +514,20 @@
>>    # Disables the following Visual Studio compiler warnings brought by openssl source,
>>    # so we do not break the build with /WX option:
>>    #   C4090: 'function' : different 'const' qualifiers
>> +  #   C4132: 'object' : const object should be initialized (tls13_enc.c)
>>    #   C4244: conversion from type1 to type2, possible loss of data
>>    #   C4245: conversion from type1 to type2, signed/unsigned mismatch
>>    #   C4267: conversion from size_t to type, possible loss of data
>>    #   C4306: 'identifier' : conversion from 'type1' to 'type2' of greater size
>> +  #   C4310: cast truncates constant value
>>    #   C4389: 'operator' : signed/unsigned mismatch (xxxx)
>> +  #   C4700: uninitialized local variable 'name' used. (conf_sap.c(71))
>>    #   C4702: unreachable code
>>    #   C4706: assignment within conditional expression
>>    #   C4819: The file contains a character that cannot be represented in the current code page
>>    #
>> -  MSFT:*_*_IA32_CC_FLAGS   = -U_WIN32 -U_WIN64 -U_MSC_VER $(OPENSSL_FLAGS) /wd4090 /wd4244 /wd4245 /wd4267 /wd4389 /wd4702 /wd4706 /wd4819
>> -  MSFT:*_*_X64_CC_FLAGS    = -U_WIN32 -U_WIN64 -U_MSC_VER $(OPENSSL_FLAGS) /wd4090 /wd4244 /wd4245 /wd4267 /wd4306 /wd4389 /wd4702 /wd4706 /wd4819
>> +  MSFT:*_*_IA32_CC_FLAGS   = -U_WIN32 -U_WIN64 -U_MSC_VER $(OPENSSL_FLAGS) /wd4090 /wd4132 /wd4244 /wd4245 /wd4267 /wd4310 /wd4389 /wd4700 /wd4702 /wd4706 /wd4819
>> +  MSFT:*_*_X64_CC_FLAGS    = -U_WIN32 -U_WIN64 -U_MSC_VER $(OPENSSL_FLAGS) /wd4090 /wd4132 /wd4244 /wd4245 /wd4267 /wd4306 /wd4310 /wd4700 /wd4389 /wd4702 /wd4706 /wd4819
>>  
>>    INTEL:*_*_IA32_CC_FLAGS  = -U_WIN32 -U_WIN64 -U_MSC_VER -U__ICC $(OPENSSL_FLAGS) /w
>>    INTEL:*_*_X64_CC_FLAGS   = -U_WIN32 -U_WIN64 -U_MSC_VER -U__ICC $(OPENSSL_FLAGS) /w
>> @@ -511,11 +537,12 @@
>>    #   -Werror=maybe-uninitialized: there exist some other paths for which the variable is not initialized.
>>    #   -Werror=format: Check calls to printf and scanf, etc., to make sure that the arguments supplied have
>>    #                   types appropriate to the format string specified.
>> +  #   -Werror=unused-but-set-variable: Warn whenever a local variable is assigned to, but otherwise unused (aside from its declaration).
>>    #
>> -  GCC:*_*_IA32_CC_FLAGS    = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -Wno-error=maybe-uninitialized
>> -  GCC:*_*_X64_CC_FLAGS     = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -Wno-error=maybe-uninitialized -Wno-error=format -Wno-format -DNO_MSABI_VA_FUNCS
>> -  GCC:*_*_ARM_CC_FLAGS     = $(OPENSSL_FLAGS) -Wno-error=maybe-uninitialized
>> -  GCC:*_*_AARCH64_CC_FLAGS = $(OPENSSL_FLAGS) 
>> -Wno-error=maybe-uninitialized -Wno-format
>> +  GCC:*_*_IA32_CC_FLAGS    = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -Wno-error=maybe-uninitialized -Wno-error=unused-but-set-variable
>> +  GCC:*_*_X64_CC_FLAGS     = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -Wno-error=maybe-uninitialized -Wno-error=format -Wno-format -Wno-error=unused-but-set-variable -DNO_MSABI_VA_FUNCS
>> +  GCC:*_*_ARM_CC_FLAGS     = $(OPENSSL_FLAGS) -Wno-error=maybe-uninitialized -Wno-error=unused-but-set-variable
>> +  GCC:*_*_AARCH64_CC_FLAGS = $(OPENSSL_FLAGS) 
>> + -Wno-error=maybe-uninitialized -Wno-format 
>> + -Wno-error=unused-but-set-variable
>>  
>>    # suppress the following warnings in openssl so we don't break the build with warnings-as-errors:
>>    # 1295: Deprecated declaration <entity> - give arg types diff 
>> --git a/CryptoPkg/Library/OpensslLib/buildinf.h
>> b/CryptoPkg/Library/OpensslLib/buildinf.h
>> index c5ca293..5b3b50b 100644
>> --- a/CryptoPkg/Library/OpensslLib/buildinf.h
>> +++ b/CryptoPkg/Library/OpensslLib/buildinf.h
>> @@ -1,2 +1,4 @@
>>  #define PLATFORM  "UEFI"
>>  #define DATE      "Fri Dec 22 01:23:45 PDT 2017"
>> +
>> +const char * compiler_flags = "";
>> diff --git a/CryptoPkg/Library/OpensslLib/openssl
>> b/CryptoPkg/Library/OpensslLib/openssl
>> index 74f2d9c..50eaac9 160000
>> --- a/CryptoPkg/Library/OpensslLib/openssl
>> +++ b/CryptoPkg/Library/OpensslLib/openssl
>> @@ -1 +1 @@
>> -Subproject commit 74f2d9c1ec5f5510e1d3da5a9f03c28df0977762
>> +Subproject commit 50eaac9f3337667259de725451f201e784599687
>> diff --git a/CryptoPkg/Library/OpensslLib/ossl_store.c
>> b/CryptoPkg/Library/OpensslLib/ossl_store.c
>> new file mode 100644
>> index 0000000..f049edd
>> --- /dev/null
>> +++ b/CryptoPkg/Library/OpensslLib/ossl_store.c
>> @@ -0,0 +1,21 @@
>> +/** @file
>> +  64-bit Math Worker Function.
>> +  The 32-bit versions of C compiler generate calls to library 
>> +routines
>> +  to handle 64-bit math. These functions use non-standard calling conventions.
>> +
>> +Copyright (c) 2014, Intel Corporation. All rights reserved.<BR>
> 
> (10) Didn't you want to include "2019" here?
> 
>> +SPDX-License-Identifier: BSD-2-Clause-Patent
>> +
>> +**/
>> +
>> +
>> +/*
>> + * This function is cleanup ossl store.
>> + *
>> + * In UEFI environment we don't use ossl store functions.
>> + *
>> + * Dummy Implement for UEFI
>> + */
>> +void ossl_store_cleanup_int(void)
>> +{
>> +}
>> diff --git a/CryptoPkg/Library/OpensslLib/rand_pool.c
>> b/CryptoPkg/Library/OpensslLib/rand_pool.c
>> new file mode 100644
>> index 0000000..11d0646
>> --- /dev/null
>> +++ b/CryptoPkg/Library/OpensslLib/rand_pool.c
>> @@ -0,0 +1,69 @@
>> +/** @file
>> +  64-bit Math Worker Function.
>> +  The 32-bit versions of C compiler generate calls to library 
>> +routines
>> +  to handle 64-bit math. These functions use non-standard calling conventions.
>> +
>> +Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
>> +SPDX-License-Identifier: BSD-2-Clause-Patent
>> +
>> +**/
>> +
>> +#include "internal/rand_int.h"
>> +
>> +/*
>> + * Add random bytes to the pool to acquire requested amount of 
>> +entropy
>> + *
>> + * This function is platform specific and tries to acquire the 
>> +requested
>> + * amount of entropy by polling platform specific entropy sources.
>> + *
>> + * If the function succeeds in acquiring at least 
>> +|entropy_requested| bits
>> + * of entropy, the total entropy count is returned. If it fails, it 
>> +returns
>> + * an entropy count of 0.
>> + *
>> + * Dummy Implement for UEFI
>> + */
>> +size_t rand_pool_acquire_entropy(RAND_POOL *pool) {
>> +  return 100;
>> +}
>> +
>> +/*
>> + * Dummy Implementation for UEFI
>> + */
>> +int rand_pool_add_nonce_data(RAND_POOL *pool) {
>> +  static char default_nonce[] = "uefi default nonce data.";
>> +  return rand_pool_add(pool, (unsigned char*)&default_nonce, 
>> +sizeof(default_nonce), 0); }
>> +
>> +/*
>> + * Dummy Implementation for UEFI
>> + */
>> +int rand_pool_add_additional_data(RAND_POOL *pool) {
>> +  static char default_additional[] = "uefi default additional 
>> +data.";
>> +
>> +  return rand_pool_add(pool, (unsigned char*)&default_additional, 
>> +sizeof(default_additional), 0); }
>> +
>> +/*
>> + * Dummy Implememtation for UEFI
>> + */
>> +int rand_pool_init(void)
>> +{
>> +  return 1;
>> +}
>> +
>> +/*
>> + * Dummy Implememtation for UEFI
>> + */
>> +void rand_pool_cleanup(void)
>> +{
>> +}
>> +
>> +/*
>> + * Dummy Implememtation for UEFI
>> + */
>> +void rand_pool_keep_random_devices_open(int keep) { }
>>
> 
> (11) I think these function definitions are wrong, and insecure.
> 
> They suggest that we are capable of populating an entropy pool. We aren't.
> 
> Therefore, all of the above functions that *can* fail, from an API perspective, *should* fail. The following functions should all return zero:
> 
> - rand_pool_init
> - rand_pool_add_nonce_data
> - rand_pool_add_additional_data
> - rand_pool_acquire_entropy
> 
> (I've checked the API docs in "crypto/include/internal/rand_int.h".)
> 
> Here's why. In edk2, we cannot provide the requested random pool functionality, therefore no higher-level facility in OpenSSL should ever rely on these low-level functions, when we build OpenSSL for edk2.
> 
> If we miss a code-path in OpenSSL and OpenSSL ends up calling one of our stub functions, then the affected higher-level OpenSSL functionality should fail loud and clear. Otherwise, we open up ourselves to catastrophic security issues.
> 
> The variable name "default_nonce" is, in itself, a complete security fiasco, given that "nonce" stands for "number used once" in cryptography. If we ever ship code like this, we'll be the laughing stock of the security world, even though the code is clearly 100% well-intended, i.e. as a stub only.
> 
>   https://xkcd.com/221/
> 
> Now, if OpenSSL 1.1.1b cannot work for us, in case we return zero from each of the above functions, then OpenSSL 1.1.1b is unsuitable for adoption in edk2. Then we have some work to do on OpenSSL 1.1.1:
> preferably, the entire higher-level facility in OpenSSL that depends on the random pool API should be possible to disable at build time.
> 
> Please let me know if / where additional code contributions to OpenSSL are necessary. I'm not a security professional myself and I probably wouldn't offer immediately to write OpenSSL patches, but I think I could reach out to crypto people @RH for help.
> 
> Thanks!
> Laszlo
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#40572): https://edk2.groups.io/g/devel/message/40572
Mute This Topic: https://groups.io/mt/31381054/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-





More information about the edk2-devel-archive mailing list