public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Xiaoyu lu" <xiaoyux.lu@intel.com>
To: Laszlo Ersek <lersek@redhat.com>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Wang, Jian J" <jian.j.wang@intel.com>, "Ye, Ting" <ting.ye@intel.com>
Subject: Re: [edk2-devel] [PATCH 2/3] CryptoPkg: Upgrade openssl to 1.1.1b
Date: Tue, 30 Apr 2019 10:00:56 +0000	[thread overview]
Message-ID: <BFD21A70FD4B3446B866B6088E3259E50B95A1C0@SHSMSX101.ccr.corp.intel.com> (raw)
In-Reply-To: <52ce3b53-83dc-9fdd-2580-1b39535f6b93@redhat.com>

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.

(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.

(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.

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

(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.

-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com] 
Sent: Tuesday, April 30, 2019 2:02 AM
To: devel@edk2.groups.io; Lu, XiaoyuX <xiaoyux.lu@intel.com>
Cc: Wang, Jian J <jian.j.wang@intel.com>; Ye, Ting <ting.ye@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@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@intel.com>
> Cc: Ting Ye <ting.ye@intel.com>
> Signed-off-by: Xiaoyu Lu <xiaoyux.lu@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

  reply	other threads:[~2019-04-30 10:01 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-29  8:15 [PATCH 0/3] CryptoPkg: Upgrade OpenSSL to 1_1_1b xiaoyux.lu
2019-04-29  8:15 ` [PATCH 1/3] CryptoPkg/IntrinsicLib: add ftol2 function Xiaoyu lu
2019-04-29  8:15 ` [PATCH 2/3] CryptoPkg: Upgrade openssl to 1.1.1b Xiaoyu lu
2019-04-29 18:01   ` [edk2-devel] " Laszlo Ersek
2019-04-30 10:00     ` Xiaoyu lu [this message]
2019-04-30 15:31       ` Laszlo Ersek
2019-05-14  5:21         ` Xiaoyu lu
2019-04-29  8:15 ` [PATCH 3/3] CryptoPkg/BaseCryptLib: updata HMAC_ctx size Xiaoyu lu
2019-04-29 14:28   ` [edk2-devel] " Philippe Mathieu-Daudé
2019-04-29 18:31   ` Laszlo Ersek
2019-04-30  1:16     ` Wang, Jian J
2019-04-30 10:26       ` Laszlo Ersek
2019-04-30 10:47         ` Wang, Jian J

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BFD21A70FD4B3446B866B6088E3259E50B95A1C0@SHSMSX101.ccr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox