* [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b
@ 2019-05-13 13:25 Xiaoyu lu
2019-05-13 13:25 ` [PATCH v3 1/6] CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL Xiaoyu lu
` (6 more replies)
0 siblings, 7 replies; 18+ messages in thread
From: Xiaoyu lu @ 2019-05-13 13:25 UTC (permalink / raw)
To: devel; +Cc: lersek, xiaoyux.lu, Jian J Wang, Ting Ye
(1) CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL
OpenSSL only support seeding NONE for UEFI(rand_unix.c line 93).
So add --with-rand-seed=none to process_files.pl.
(2) CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl
When running process_files.py to configure OpenSSL, we can exclude some unnecessary files. This can reduce porting time, compiling time and library size.
(3) CryptoPkg/IntrinsicLib: Fix possible unresolved external symbol issue
(4) CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL
Disable warning for building OpenSSL_1_1_1b
(5) CryptoPkg: Upgrade OpenSSL to 1.1.1b
Update OpenSSL submodule to OpenSSL_1_1_1b
OpenSSL_1_1_1b(50eaac9f3337667259de725451f201e784599687)
OpenSSL doesn't implement some rand_pool function for UEFI.
Use EFI_RNG_PROTOCOL to generate random for entropy.
If EFI_RNG_PROTOCOL is not avaliable, fall back to performance
counter, but we not sure about the amount of randomness it provides.
(6) CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward compatible
Note: Will be remove next update.
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1792
Ref: https://github.com/openssl/openssl/pull/4338
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Ting Ye <ting.ye@intel.com>
Xiaoyu Lu (3):
CryptoPkg/IntrinsicLib: Fix possible unresolved external symbol issue
CryptoPkg: Upgrade OpenSSL to 1.1.1b
CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward compatible
Xiaoyu lu (3):
CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL
CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl
CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL
CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacMd5.c | 8 +-
.../Library/BaseCryptLib/Hmac/CryptHmacSha1.c | 9 +-
.../Library/BaseCryptLib/Hmac/CryptHmacSha256.c | 8 +-
CryptoPkg/Library/Include/CrtLibSupport.h | 11 +
CryptoPkg/Library/Include/openssl/opensslconf.h | 54 +++-
CryptoPkg/Library/Include/sys/syscall.h | 9 +
CryptoPkg/Library/IntrinsicLib/Ia32/MathFtol.c | 22 ++
CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf | 4 +-
CryptoPkg/Library/OpensslLib/OpensslLib.inf | 63 +++-
CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf | 54 +++-
CryptoPkg/Library/OpensslLib/buildinf.h | 2 +
CryptoPkg/Library/OpensslLib/openssl | 2 +-
CryptoPkg/Library/OpensslLib/ossl_store.c | 17 ++
CryptoPkg/Library/OpensslLib/process_files.pl | 11 +-
CryptoPkg/Library/OpensslLib/rand_pool.c | 339 +++++++++++++++++++++
15 files changed, 564 insertions(+), 49 deletions(-)
create mode 100644 CryptoPkg/Library/Include/sys/syscall.h
create mode 100644 CryptoPkg/Library/IntrinsicLib/Ia32/MathFtol.c
create mode 100644 CryptoPkg/Library/OpensslLib/ossl_store.c
create mode 100644 CryptoPkg/Library/OpensslLib/rand_pool.c
--
2.7.4
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH v3 1/6] CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL 2019-05-13 13:25 [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b Xiaoyu lu @ 2019-05-13 13:25 ` Xiaoyu lu 2019-05-13 13:25 ` [PATCH v3 2/6] CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl Xiaoyu lu ` (5 subsequent siblings) 6 siblings, 0 replies; 18+ messages in thread From: Xiaoyu lu @ 2019-05-13 13:25 UTC (permalink / raw) To: devel; +Cc: lersek, xiaoyux.lu, Jian J Wang, Ting Ye REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1089 OpenSSL configure mechanism use --with-rand-seed=xxx option to configure random number generation. OpenSSL_1_1_0j(74f2d9c1ec5f5510e1d3da5a9f03c28df0977762) we use default --with-rand-seed=os option to for building it. But OpenSSL_1_1_1b(50eaac9f3337667259de725451f201e784599687) only support seeding NONE for UEFI(rand_unix.c line 93). This OpenSSL change was introduced in commit 8389ec4b4950 ("Add --with-rand-seed", 2017-07-22). So add --with-rand-seed=none to process_files.pl. 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/OpensslLib/process_files.pl | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CryptoPkg/Library/OpensslLib/process_files.pl b/CryptoPkg/Library/OpensslLib/process_files.pl index f6e1f43..6c136cc 100755 --- a/CryptoPkg/Library/OpensslLib/process_files.pl +++ b/CryptoPkg/Library/OpensslLib/process_files.pl @@ -90,7 +90,10 @@ BEGIN { "no-threads", "no-ts", "no-ui", - "no-whirlpool" + "no-whirlpool", + # OpenSSL1_1_1b doesn't support default rand-seed-os for UEFI + # UEFI only support --with-rand-seed=none + "--with-rand-seed=none" ) == 0 || die "OpenSSL Configure failed!\n"; -- 2.7.4 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 2/6] CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl 2019-05-13 13:25 [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b Xiaoyu lu 2019-05-13 13:25 ` [PATCH v3 1/6] CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL Xiaoyu lu @ 2019-05-13 13:25 ` Xiaoyu lu 2019-05-13 13:25 ` [PATCH v3 3/6] CryptoPkg/IntrinsicLib: Fix possible unresolved external symbol issue Xiaoyu lu ` (4 subsequent siblings) 6 siblings, 0 replies; 18+ messages in thread From: Xiaoyu lu @ 2019-05-13 13:25 UTC (permalink / raw) To: devel; +Cc: lersek, xiaoyux.lu, Jian J Wang, Ting Ye REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1089 When running process_files.py to configure OpenSSL, we can exclude some unnecessary files. This can reduce porting time, compiling time and library size. Upstream OpenSSL commit 71a5516dcc8a which was as part of OpenSSL_1_1_1b, Add the STORE module(crypto/store/*). But UEFI don't use them. So exclude these files. Functions in crypto/rand/randfile.c OpenSSL and edk2 don't call them. And it requires more crt runtime support. So exclude it. 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/OpensslLib/process_files.pl | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CryptoPkg/Library/OpensslLib/process_files.pl b/CryptoPkg/Library/OpensslLib/process_files.pl index 6c136cc..e277108 100755 --- a/CryptoPkg/Library/OpensslLib/process_files.pl +++ b/CryptoPkg/Library/OpensslLib/process_files.pl @@ -127,6 +127,12 @@ foreach my $product ((@{$unified_info{libraries}}, foreach my $s (@{$unified_info{sources}->{$o}}) { next if ($unified_info{generate}->{$s}); next if $s =~ "crypto/bio/b_print.c"; + + # No need to add unused files in UEFI. + # So it can reduce porting time, compile time, library size. + next if $s =~ "crypto/rand/randfile.c"; + next if $s =~ "crypto/store/"; + if ($product =~ "libssl") { push @sslfilelist, ' $(OPENSSL_PATH)/' . $s . "\r\n"; next; -- 2.7.4 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 3/6] CryptoPkg/IntrinsicLib: Fix possible unresolved external symbol issue 2019-05-13 13:25 [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b Xiaoyu lu 2019-05-13 13:25 ` [PATCH v3 1/6] CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL Xiaoyu lu 2019-05-13 13:25 ` [PATCH v3 2/6] CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl Xiaoyu lu @ 2019-05-13 13:25 ` Xiaoyu lu 2019-05-13 13:25 ` [PATCH v3 4/6] CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL Xiaoyu lu ` (3 subsequent siblings) 6 siblings, 0 replies; 18+ messages in thread From: Xiaoyu lu @ 2019-05-13 13:25 UTC (permalink / raw) To: devel; +Cc: lersek, xiaoyux.lu, Jian J Wang, Ting Ye From: Xiaoyu Lu <xiaoyux.lu@intel.com> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1089 This is for the upcoming upgrade to OpenSSL_1_1_1b Compiler optimization(Visual Studio) may automatically use _ftol2 instead of some type conversion. For example: OpensslLib.lib(drbg_lib.obj) : error LNK2001: unresolved external symbol __ftol2 This patch add _ftol2 function for the compiler intrinsic. 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/IntrinsicLib/Ia32/MathFtol.c | 22 ++++++++++++++++++++++ CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf | 4 +++- 2 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 CryptoPkg/Library/IntrinsicLib/Ia32/MathFtol.c diff --git a/CryptoPkg/Library/IntrinsicLib/Ia32/MathFtol.c b/CryptoPkg/Library/IntrinsicLib/Ia32/MathFtol.c new file mode 100644 index 0000000..147a19a --- /dev/null +++ b/CryptoPkg/Library/IntrinsicLib/Ia32/MathFtol.c @@ -0,0 +1,22 @@ +/** @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 + +**/ + +/* + * Floating point to integer conversion. + */ +__declspec(naked) void _ftol2 (void) +{ + _asm { + fistp qword ptr [esp-8] + mov edx, [esp-4] + mov eax, [esp-8] + ret + } +} diff --git a/CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf b/CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf index 5a20967..fcbb933 100644 --- a/CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf +++ b/CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf @@ -1,7 +1,7 @@ ## @file # Intrinsic Routines Wrapper Library Instance. # -# Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.<BR> +# Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR> # SPDX-License-Identifier: BSD-2-Clause-Patent # ## @@ -29,9 +29,11 @@ Ia32/MathLShiftS64.c | MSFT Ia32/MathRShiftU64.c | MSFT + Ia32/MathFtol.c | MSFT Ia32/MathLShiftS64.c | INTEL Ia32/MathRShiftU64.c | INTEL + Ia32/MathFtol.c | INTEL Ia32/MathLShiftS64.nasm | GCC Ia32/MathRShiftU64.nasm | GCC -- 2.7.4 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 4/6] CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL 2019-05-13 13:25 [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b Xiaoyu lu ` (2 preceding siblings ...) 2019-05-13 13:25 ` [PATCH v3 3/6] CryptoPkg/IntrinsicLib: Fix possible unresolved external symbol issue Xiaoyu lu @ 2019-05-13 13:25 ` Xiaoyu lu 2019-05-13 13:25 ` [PATCH v3 5/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b Xiaoyu lu ` (2 subsequent siblings) 6 siblings, 0 replies; 18+ messages in thread From: Xiaoyu lu @ 2019-05-13 13:25 UTC (permalink / raw) To: devel; +Cc: lersek, xiaoyux.lu, Jian J Wang, Ting Ye REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1089 Disable warning for building OpenSSL_1_1_1b add /wd4132 /wd4700 /wd4310 for Visual Studio in OpensslLib[Crypto].inf add -Wno-error=unused-but-set-variable for GCC in OpensslLib[Crypto].inf Although this option is set in some build environments by default. But this is only for OpenSSL compilation, no matter how the default options change. Due to --with-rand-seed=none, We need to provide a placeholder header file CryptoPkg/Library/Include/sys/syscall.h. 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/sys/syscall.h | 9 +++++++++ CryptoPkg/Library/OpensslLib/OpensslLib.inf | 16 ++++++++++------ CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf | 16 ++++++++++------ 3 files changed, 29 insertions(+), 12 deletions(-) create mode 100644 CryptoPkg/Library/Include/sys/syscall.h diff --git a/CryptoPkg/Library/Include/sys/syscall.h b/CryptoPkg/Library/Include/sys/syscall.h new file mode 100644 index 0000000..bd212b0 --- /dev/null +++ b/CryptoPkg/Library/Include/sys/syscall.h @@ -0,0 +1,9 @@ +/** @file + Include file to support building the third-party cryptographic library. + +Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR> +SPDX-License-Identifier: BSD-2-Clause-Patent + +**/ + +#include <CrtLibSupport.h> diff --git a/CryptoPkg/Library/OpensslLib/OpensslLib.inf b/CryptoPkg/Library/OpensslLib/OpensslLib.inf index 530ac5f..f4d7772 100644 --- a/CryptoPkg/Library/OpensslLib/OpensslLib.inf +++ b/CryptoPkg/Library/OpensslLib/OpensslLib.inf @@ -530,17 +530,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 +553,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..fd12d11 100644 --- a/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf +++ b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf @@ -491,17 +491,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 +514,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 -- 2.7.4 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 5/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b 2019-05-13 13:25 [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b Xiaoyu lu ` (3 preceding siblings ...) 2019-05-13 13:25 ` [PATCH v3 4/6] CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL Xiaoyu lu @ 2019-05-13 13:25 ` Xiaoyu lu 2019-05-13 13:25 ` [PATCH v3 6/6] CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward compatible Xiaoyu lu 2019-05-13 19:24 ` [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b Laszlo Ersek 6 siblings, 0 replies; 18+ messages in thread From: Xiaoyu lu @ 2019-05-13 13:25 UTC (permalink / raw) To: devel; +Cc: lersek, xiaoyux.lu, Jian J Wang, Ting Ye 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 OpenSSL_1_1_1b(50eaac9f3337667259de725451f201e784599687) Run process_files.pl script to regenerate OpensslLib[Crypto].inf and opensslconf.h Remove -DNO_SYSLOG from OPENSSL_FLAGS in OpensslLib[Crypto].inf, due to upstream OpenSSL commit cff55b90e95e("Cleaning UEFI Build with additional OPENSSL_SYS_UEFI flags", 2017-03-29), which was first released as part of OpenSSL_1_1_1. Starting with OpenSSL commit 8a8d9e1905(first release in OpenSSL_1_1_1), the OpenSSL_version() function can no longer return a pointer to the string literal "compiler: information not available", in the case CFLAGS macro is not defined. Instead, the function now has a hard dependency on the global variable 'compiler_flags'. This variable is normally placed by "util/mkbuildinf.pl" into "buildinf.h". In edk2 we don't run that script whenever we build OpenSSL, therefor we must provide our own dummy 'compiler_flags'. Since OpenSSL_1_1_1b doesn't fully implement rand pool functions for UEFI. So add a file(rand_pool.c) and implement them. * 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 We use EFI_RNG_PROTOCOL to generate random outputs for seeding entropy. And if EFI_RNG_PROTOCOL not provide by a platform, we fall back to performance counter. We don't need ossl_store functions. We exclude relative files through process_files.pl. And ossl_store_cleanup_int was first added in crypto/init.c OpenSSL_1_1_1(71a5516d). So add a new file(ossl_store.c) to implement ossl_store_cleanup_int function. BUFSIZ is used by crypto/evp/evp_key.c(OpenSSL_1_1_1b) And it is declared in stdio.h. So add it to CrtLibSupport.h. Ref: https://github.com/openssl/openssl/issues/8904 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 | 11 + CryptoPkg/Library/Include/openssl/opensslconf.h | 54 +++- CryptoPkg/Library/OpensslLib/OpensslLib.inf | 47 ++- CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf | 38 ++- CryptoPkg/Library/OpensslLib/buildinf.h | 2 + CryptoPkg/Library/OpensslLib/openssl | 2 +- CryptoPkg/Library/OpensslLib/ossl_store.c | 17 ++ CryptoPkg/Library/OpensslLib/rand_pool.c | 339 ++++++++++++++++++++++ 8 files changed, 481 insertions(+), 29 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..467f8c8 100644 --- a/CryptoPkg/Library/Include/CrtLibSupport.h +++ b/CryptoPkg/Library/Include/CrtLibSupport.h @@ -21,6 +21,17 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #define MAX_STRING_SIZE 0x1000 // +// We already have "no-ui" in out Configure invocation. +// but the code still fails to compile. +// Ref: https://github.com/openssl/openssl/issues/8904 +// +// This is defined in CRT library(stdio.h). +// +#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 diff --git a/CryptoPkg/Library/Include/openssl/opensslconf.h b/CryptoPkg/Library/Include/openssl/opensslconf.h index 28dd9ab..07fa2d3 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_NONE +# define OPENSSL_RAND_SEED_NONE +#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 diff --git a/CryptoPkg/Library/OpensslLib/OpensslLib.inf b/CryptoPkg/Library/OpensslLib/OpensslLib.inf index f4d7772..72f0a67 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 # # 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,10 +548,14 @@ [LibraryClasses] DebugLib + TimerLib [LibraryClasses.ARM] ArmSoftFloatLib +[Protocols] + gEfiRngProtocolGuid + [BuildOptions] # # Disables the following Visual Studio compiler warnings brought by openssl source, diff --git a/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf index fd12d11..4484143 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 # # 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,10 +504,14 @@ [LibraryClasses] DebugLib + TimerLib [LibraryClasses.ARM] ArmSoftFloatLib +[Protocols] + gEfiRngProtocolGuid + [BuildOptions] # # Disables the following Visual Studio compiler warnings brought by openssl source, diff --git a/CryptoPkg/Library/OpensslLib/buildinf.h b/CryptoPkg/Library/OpensslLib/buildinf.h index c5ca293..b840c86 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 = "compiler: information not available from edk2"; 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..29e1506 --- /dev/null +++ b/CryptoPkg/Library/OpensslLib/ossl_store.c @@ -0,0 +1,17 @@ +/** @file + Dummy implement ossl_store(Store retrieval functions) for UEFI. + +Copyright (c) 2019, Intel Corporation. All rights reserved.<BR> +SPDX-License-Identifier: BSD-2-Clause-Patent + +**/ + +/* + * This function is cleanup ossl store. + * + * 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..a88ad8b --- /dev/null +++ b/CryptoPkg/Library/OpensslLib/rand_pool.c @@ -0,0 +1,339 @@ +/** @file + OpenSSL_1_1_1b doesn't implement rand_pool_* functions for UEFI. + The file implement these functions. + +Copyright (c) 2019, Intel Corporation. All rights reserved.<BR> +SPDX-License-Identifier: BSD-2-Clause-Patent + +**/ + +#include "internal/rand_int.h" +#include <openssl/aes.h> +#include <Uefi.h> +#include <Library/TimerLib.h> +#include <Protocol/Rng.h> +#include <Library/UefiBootServicesTableLib.h> + +/** + Get some randomness from low-order bits of GetPerformanceCounter results. + And combine them to the 64-bit value + + @param[out] Rand Buffer pointer to store the 64-bit random value. + + @retval TRUE Random number generated successfully. + @retval FALSE Failed to generate. +**/ +STATIC +BOOLEAN +EFIAPI +RandNumber64FromPerformanceCounter( + OUT UINT64 *Rand + ) +{ + UINT32 Index; + UINT32 *RandPtr; + RandPtr = (UINT32 *)Rand; + + ASSERT (Rand != NULL); + + for (Index = 0; Index < 2; Index ++) { + *RandPtr = (UINT32)(GetPerformanceCounter() & 0xFF); + MicroSecondDelay(10); + RandPtr++; + } + + return TRUE; +} + + +/** + Generates a 64-bit random number. + if Rand is NULL, then ASSERT(). + @param[out] Rand Buffer pointer to store the 64-bit random value. + @retval TRUE Random number generated successfully. + @retval FALSE Failed to generate the random number. +**/ +STATIC +BOOLEAN +EFIAPI +RandomNumber64 ( + OUT UINT64 *Rand + ) +{ + EFI_RNG_PROTOCOL *Rng; + UINTN Number = 8; + EFI_STATUS Status; + BOOLEAN Ret; + + ASSERT (Rand != NULL); + + Ret = FALSE; + + Status = gBS->LocateProtocol(&gEfiRngProtocolGuid, NULL, (VOID **)&Rng); + if (Status == EFI_NOT_FOUND) { + // + // Fall back to use PerformanceCounter to generate rand nubmer. + // We are not sure about the amount of randomness it provides. + // If you really care about the security. please provide a EFI_RNG_PROTOCOL + // + Ret = RandNumber64FromPerformanceCounter(Rand); + return Ret; + } + + if (Status == EFI_SUCCESS) { + Status = Rng->GetRNG (Rng, NULL, Number, (UINT8 *)Rand); + } + + if (Status == EFI_SUCCESS) { + Ret = TRUE; + } + + return Ret; +} + +/** + Calls RandomNumber64 to fill + a buffer of arbitrary size with random bytes. + + @param[in] Length Size of the buffer, in bytes, to fill with. + @param[out] RandBuffer Pointer to the buffer to store the random result. + + @retval EFI_SUCCESS Random bytes generation succeeded. + @retval EFI_NOT_READY Failed to request random bytes. + +**/ +STATIC +BOOLEAN +EFIAPI +RandGetBytes ( + IN UINTN Length, + OUT UINT8 *RandBuffer + ) +{ + BOOLEAN Ret; + UINT64 TempRand; + + Ret = FALSE; + + while (Length > 0) { + Ret = RandomNumber64 (&TempRand); + if (!Ret) { + return Ret; + } + if (Length >= sizeof (TempRand)) { + *((UINT64*)RandBuffer) = TempRand; + RandBuffer += sizeof (UINT64); + Length -= sizeof (TempRand); + } else { + CopyMem (RandBuffer, &TempRand, Length); + Length = 0; + } + } + + return Ret; +} + +/** + Creates a 128bit random value that is fully forward and backward prediction resistant, + suitable for seeding a NIST SP800-90 Compliant. + This function takes multiple random numbers from PerformanceCounter to ensure reseeding + and performs AES-CBC-MAC over the data to compute the seed value. + + @param[out] SeedBuffer Pointer to a 128bit buffer to store the random seed. + + @retval TRUE Random seed generation succeeded. + @retval FALSE Failed to request random bytes. + +**/ +STATIC +BOOLEAN +EFIAPI +RandGetSeed128 ( + OUT UINT8 *SeedBuffer + ) +{ + BOOLEAN Ret; + UINT8 RandByte[16]; + UINT8 Key[16]; + UINT8 Ffv[16]; + UINT8 Xored[16]; + UINT32 Index; + UINT32 Index2; + AES_KEY AESKey; + + // + // Chose an arbitary key and zero the feed_forward_value (FFV) + // + for (Index = 0; Index < 16; Index++) { + Key[Index] = (UINT8) Index; + Ffv[Index] = 0; + } + + AES_set_encrypt_key(Key, 16 * 8, &AESKey); + + // + // Perform CBC_MAC over 32 * 128 bit values, with 10us gaps between 128 bit value + // The 10us gaps will ensure multiple reseeds within the system time with a large + // design margin. + // + for (Index = 0; Index < 32; Index++) { + MicroSecondDelay (10); + Ret = RandGetBytes (16, RandByte); + if (!Ret) { + return Ret; + } + + // + // Perform XOR operations on two 128-bit value. + // + for (Index2 = 0; Index2 < 16; Index2++) { + Xored[Index2] = RandByte[Index2] ^ Ffv[Index2]; + } + + AES_encrypt(Xored, Ffv, &AESKey); + } + + for (Index = 0; Index < 16; Index++) { + SeedBuffer[Index] = Ffv[Index]; + } + + return Ret; +} + +/** + Generate high-quality entropy source. + + @param[in] Length Size of the buffer, in bytes, to fill with. + @param[out] Entropy Pointer to the buffer to store the entropy data. + + @retval EFI_SUCCESS Entropy generation succeeded. + @retval EFI_NOT_READY Failed to request random data. + +**/ +STATIC +BOOLEAN +EFIAPI +RandGenerateEntropy ( + IN UINTN Length, + OUT UINT8 *Entropy + ) +{ + BOOLEAN Ret; + UINTN BlockCount; + UINT8 Seed[16]; + UINT8 *Ptr; + + BlockCount = Length / 16; + Ptr = (UINT8 *)Entropy; + + // + // Generate high-quality seed for DRBG Entropy + // + while (BlockCount > 0) { + Ret = RandGetSeed128 (Seed); + if (!Ret) { + return Ret; + } + CopyMem (Ptr, Seed, 16); + + BlockCount--; + Ptr = Ptr + 16; + } + + // + // Populate the remained data as request. + // + Ret = RandGetSeed128 (Seed); + if (!Ret) { + return Ret; + } + CopyMem (Ptr, Seed, (Length % 16)); + + return Ret; +} + +/* + * 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. + */ +size_t rand_pool_acquire_entropy(RAND_POOL *pool) +{ + BOOLEAN Ret; + size_t bytes_needed; + unsigned char * buffer; + + bytes_needed = rand_pool_bytes_needed(pool, 1 /*entropy_factor*/); + if (bytes_needed > 0) { + buffer = rand_pool_add_begin(pool, bytes_needed); + + if (buffer != NULL) { + Ret = RandGenerateEntropy(bytes_needed, buffer); + if (FALSE == Ret) { + rand_pool_add_end(pool, 0, 0); + } else { + rand_pool_add_end(pool, bytes_needed, 8 * bytes_needed); + } + } + } + + return rand_pool_entropy_available(pool); +} + +/* + * Implementation for UEFI + */ +int rand_pool_add_nonce_data(RAND_POOL *pool) +{ + struct { + UINT64 MonotonicCount; + UINT64 Rand; + UINT64 TimerValue; + } data = { 0 }; + + gBS->GetNextMonotonicCount(&(data.MonotonicCount)); + RandGetBytes(8, (UINT8 *)&(data.Rand)); + data.TimerValue = GetPerformanceCounter(); + + return rand_pool_add(pool, (unsigned char*)&data, sizeof(data), 0); +} + +/* + * Implementation for UEFI + */ +int rand_pool_add_additional_data(RAND_POOL *pool) +{ + struct { + UINT64 Rand; + UINT64 TimerValue; + } data = { 0 }; + + RandGetBytes(8, (UINT8 *)&(data.Rand)); + data.TimerValue = GetPerformanceCounter(); + + return rand_pool_add(pool, (unsigned char*)&data, sizeof(data), 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) +{ +} + -- 2.7.4 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH v3 6/6] CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward compatible 2019-05-13 13:25 [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b Xiaoyu lu ` (4 preceding siblings ...) 2019-05-13 13:25 ` [PATCH v3 5/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b Xiaoyu lu @ 2019-05-13 13:25 ` Xiaoyu lu 2019-05-13 19:24 ` [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b Laszlo Ersek 6 siblings, 0 replies; 18+ messages in thread From: Xiaoyu lu @ 2019-05-13 13:25 UTC (permalink / raw) To: devel; +Cc: lersek, xiaoyux.lu, Jian J Wang, Ting Ye From: Xiaoyu Lu <xiaoyux.lu@intel.com> Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1089 OpenSSL internally redefines the size of HMAC_CTX at crypto/hmac/hmac_lcl.h(OpenSSL commit e0810e35). Ref: https://github.com/openssl/openssl/pull/4338 We should not use it directly and should remove relevant functions(Hmac*GetContextSize). Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=1792 But for compatiblility, temporarily change these definition of HMAC_*_CTX_SIZE. 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/BaseCryptLib/Hmac/CryptHmacMd5.c | 8 ++++++-- CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha1.c | 9 +++++++-- CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha256.c | 8 ++++++-- 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacMd5.c b/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacMd5.c index 3134806..19e9fbe 100644 --- a/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacMd5.c +++ b/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacMd5.c @@ -9,8 +9,12 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #include "InternalCryptLib.h" #include <openssl/hmac.h> -#define HMAC_MD5_CTX_SIZE sizeof(void *) * 4 + sizeof(unsigned int) + \ - sizeof(unsigned char) * HMAC_MAX_MD_CBLOCK +// +// NOTE: OpenSSL redefines the size of HMAC_CTX at crypto/hmac/hmac_lcl.h +// #define HMAC_MAX_MD_CBLOCK_SIZE 144 +// +#define HMAC_MD5_CTX_SIZE (sizeof(void *) * 4 + sizeof(unsigned int) + \ + sizeof(unsigned char) * 144) /** Retrieves the size, in bytes, of the context buffer required for HMAC-MD5 operations. diff --git a/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha1.c b/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha1.c index bbe3df4..7d7df96 100644 --- a/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha1.c +++ b/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha1.c @@ -9,8 +9,13 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #include "InternalCryptLib.h" #include <openssl/hmac.h> -#define HMAC_SHA1_CTX_SIZE sizeof(void *) * 4 + sizeof(unsigned int) + \ - sizeof(unsigned char) * HMAC_MAX_MD_CBLOCK +// +// NOTE: OpenSSL redefines the size of HMAC_CTX at crypto/hmac/hmac_lcl.h +// #define HMAC_MAX_MD_CBLOCK_SIZE 144 +// +// +#define HMAC_SHA1_CTX_SIZE (sizeof(void *) * 4 + sizeof(unsigned int) + \ + sizeof(unsigned char) * 144) /** Retrieves the size, in bytes, of the context buffer required for HMAC-SHA1 operations. diff --git a/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha256.c b/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha256.c index ac9084f..f24443e 100644 --- a/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha256.c +++ b/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha256.c @@ -9,8 +9,12 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #include "InternalCryptLib.h" #include <openssl/hmac.h> -#define HMAC_SHA256_CTX_SIZE sizeof(void *) * 4 + sizeof(unsigned int) + \ - sizeof(unsigned char) * HMAC_MAX_MD_CBLOCK +// +// NOTE: OpenSSL redefines the size of HMAC_CTX at crypto/hmac/hmac_lcl.h +// #define HMAC_MAX_MD_CBLOCK_SIZE 144 +// +#define HMAC_SHA256_CTX_SIZE (sizeof(void *) * 4 + sizeof(unsigned int) + \ + sizeof(unsigned char) * 144) /** Retrieves the size, in bytes, of the context buffer required for HMAC-SHA256 operations. -- 2.7.4 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b 2019-05-13 13:25 [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b Xiaoyu lu ` (5 preceding siblings ...) 2019-05-13 13:25 ` [PATCH v3 6/6] CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward compatible Xiaoyu lu @ 2019-05-13 19:24 ` Laszlo Ersek 2019-05-14 6:16 ` Gary Lin 2019-05-14 11:58 ` Laszlo Ersek 6 siblings, 2 replies; 18+ messages in thread From: Laszlo Ersek @ 2019-05-13 19:24 UTC (permalink / raw) To: devel, xiaoyux.lu, Gary Lin; +Cc: Jian J Wang, Ting Ye On 05/13/19 15:25, Xiaoyu lu wrote: > (1) CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL > OpenSSL only support seeding NONE for UEFI(rand_unix.c line 93). > So add --with-rand-seed=none to process_files.pl. > > (2) CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl > When running process_files.py to configure OpenSSL, we can exclude some unnecessary files. This can reduce porting time, compiling time and library size. > > (3) CryptoPkg/IntrinsicLib: Fix possible unresolved external symbol issue > > (4) CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL > Disable warning for building OpenSSL_1_1_1b > > (5) CryptoPkg: Upgrade OpenSSL to 1.1.1b > Update OpenSSL submodule to OpenSSL_1_1_1b > OpenSSL_1_1_1b(50eaac9f3337667259de725451f201e784599687) > > OpenSSL doesn't implement some rand_pool function for UEFI. > Use EFI_RNG_PROTOCOL to generate random for entropy. > If EFI_RNG_PROTOCOL is not avaliable, fall back to performance > counter, but we not sure about the amount of randomness it provides. > > (6) CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward compatible > > Note: Will be remove next update. > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1792 > Ref: https://github.com/openssl/openssl/pull/4338 > > > Cc: Jian J Wang <jian.j.wang@intel.com> > Cc: Ting Ye <ting.ye@intel.com> I'm withdrawing from reviewing or testing this series. Gary, if you have the time, can you please regression test this (for HTTPS boot) in both OVMF and ArmVirtQemu? Thank you Laszlo ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b 2019-05-13 19:24 ` [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b Laszlo Ersek @ 2019-05-14 6:16 ` Gary Lin 2019-05-14 12:06 ` Laszlo Ersek 2019-05-14 11:58 ` Laszlo Ersek 1 sibling, 1 reply; 18+ messages in thread From: Gary Lin @ 2019-05-14 6:16 UTC (permalink / raw) To: devel, lersek; +Cc: xiaoyux.lu, Jian J Wang, Ting Ye On Mon, May 13, 2019 at 09:24:39PM +0200, Laszlo Ersek wrote: > On 05/13/19 15:25, Xiaoyu lu wrote: > > (1) CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL > > OpenSSL only support seeding NONE for UEFI(rand_unix.c line 93). > > So add --with-rand-seed=none to process_files.pl. > > > > (2) CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl > > When running process_files.py to configure OpenSSL, we can exclude some unnecessary files. This can reduce porting time, compiling time and library size. > > > > (3) CryptoPkg/IntrinsicLib: Fix possible unresolved external symbol issue > > > > (4) CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL > > Disable warning for building OpenSSL_1_1_1b > > > > (5) CryptoPkg: Upgrade OpenSSL to 1.1.1b > > Update OpenSSL submodule to OpenSSL_1_1_1b > > OpenSSL_1_1_1b(50eaac9f3337667259de725451f201e784599687) > > > > OpenSSL doesn't implement some rand_pool function for UEFI. > > Use EFI_RNG_PROTOCOL to generate random for entropy. > > If EFI_RNG_PROTOCOL is not avaliable, fall back to performance > > counter, but we not sure about the amount of randomness it provides. > > > > (6) CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward compatible > > > > Note: Will be remove next update. > > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1792 > > Ref: https://github.com/openssl/openssl/pull/4338 > > > > > > Cc: Jian J Wang <jian.j.wang@intel.com> > > Cc: Ting Ye <ting.ye@intel.com> > > I'm withdrawing from reviewing or testing this series. > > Gary, if you have the time, can you please regression test this (for > HTTPS boot) in both OVMF and ArmVirtQemu? > I'll find some time to do the regression test tomorrorw. Cheers, Gary Lin ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b 2019-05-14 6:16 ` Gary Lin @ 2019-05-14 12:06 ` Laszlo Ersek 2019-05-14 13:26 ` Wang, Jian J 0 siblings, 1 reply; 18+ messages in thread From: Laszlo Ersek @ 2019-05-14 12:06 UTC (permalink / raw) To: devel, glin; +Cc: xiaoyux.lu, Jian J Wang, Ting Ye On 05/14/19 08:16, Gary Lin wrote: > On Mon, May 13, 2019 at 09:24:39PM +0200, Laszlo Ersek wrote: >> On 05/13/19 15:25, Xiaoyu lu wrote: >>> (1) CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL >>> OpenSSL only support seeding NONE for UEFI(rand_unix.c line 93). >>> So add --with-rand-seed=none to process_files.pl. >>> >>> (2) CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl >>> When running process_files.py to configure OpenSSL, we can exclude some unnecessary files. This can reduce porting time, compiling time and library size. >>> >>> (3) CryptoPkg/IntrinsicLib: Fix possible unresolved external symbol issue >>> >>> (4) CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL >>> Disable warning for building OpenSSL_1_1_1b >>> >>> (5) CryptoPkg: Upgrade OpenSSL to 1.1.1b >>> Update OpenSSL submodule to OpenSSL_1_1_1b >>> OpenSSL_1_1_1b(50eaac9f3337667259de725451f201e784599687) >>> >>> OpenSSL doesn't implement some rand_pool function for UEFI. >>> Use EFI_RNG_PROTOCOL to generate random for entropy. >>> If EFI_RNG_PROTOCOL is not avaliable, fall back to performance >>> counter, but we not sure about the amount of randomness it provides. >>> >>> (6) CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward compatible >>> >>> Note: Will be remove next update. >>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1792 >>> Ref: https://github.com/openssl/openssl/pull/4338 >>> >>> >>> Cc: Jian J Wang <jian.j.wang@intel.com> >>> Cc: Ting Ye <ting.ye@intel.com> >> >> I'm withdrawing from reviewing or testing this series. >> >> Gary, if you have the time, can you please regression test this (for >> HTTPS boot) in both OVMF and ArmVirtQemu? >> > I'll find some time to do the regression test tomorrorw. Thanks, Gary! Xiaoyu might post a v4 with a remote topic branch for reviewers to fetch; I suggest awaiting that. (The series is difficult to apply with git-am.) Thanks Laszlo > Cheers, > > Gary Lin > > > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b 2019-05-14 12:06 ` Laszlo Ersek @ 2019-05-14 13:26 ` Wang, Jian J 2019-05-15 1:53 ` Gary Lin 0 siblings, 1 reply; 18+ messages in thread From: Wang, Jian J @ 2019-05-14 13:26 UTC (permalink / raw) To: Laszlo Ersek, devel@edk2.groups.io, glin@suse.com; +Cc: Lu, XiaoyuX, Ye, Ting Yes, please wait for v4 version of this patch series. Regards, Jian > -----Original Message----- > From: Laszlo Ersek [mailto:lersek@redhat.com] > Sent: Tuesday, May 14, 2019 8:06 PM > To: devel@edk2.groups.io; glin@suse.com > Cc: Lu, XiaoyuX <xiaoyux.lu@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; > Ye, Ting <ting.ye@intel.com> > Subject: Re: [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b > > On 05/14/19 08:16, Gary Lin wrote: > > On Mon, May 13, 2019 at 09:24:39PM +0200, Laszlo Ersek wrote: > >> On 05/13/19 15:25, Xiaoyu lu wrote: > >>> (1) CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL > >>> OpenSSL only support seeding NONE for UEFI(rand_unix.c line 93). > >>> So add --with-rand-seed=none to process_files.pl. > >>> > >>> (2) CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl > >>> When running process_files.py to configure OpenSSL, we can exclude some > unnecessary files. This can reduce porting time, compiling time and library size. > >>> > >>> (3) CryptoPkg/IntrinsicLib: Fix possible unresolved external symbol issue > >>> > >>> (4) CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL > >>> Disable warning for building OpenSSL_1_1_1b > >>> > >>> (5) CryptoPkg: Upgrade OpenSSL to 1.1.1b > >>> Update OpenSSL submodule to OpenSSL_1_1_1b > >>> OpenSSL_1_1_1b(50eaac9f3337667259de725451f201e784599687) > >>> > >>> OpenSSL doesn't implement some rand_pool function for UEFI. > >>> Use EFI_RNG_PROTOCOL to generate random for entropy. > >>> If EFI_RNG_PROTOCOL is not avaliable, fall back to performance > >>> counter, but we not sure about the amount of randomness it provides. > >>> > >>> (6) CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward compatible > >>> > >>> Note: Will be remove next update. > >>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1792 > >>> Ref: https://github.com/openssl/openssl/pull/4338 > >>> > >>> > >>> Cc: Jian J Wang <jian.j.wang@intel.com> > >>> Cc: Ting Ye <ting.ye@intel.com> > >> > >> I'm withdrawing from reviewing or testing this series. > >> > >> Gary, if you have the time, can you please regression test this (for > >> HTTPS boot) in both OVMF and ArmVirtQemu? > >> > > I'll find some time to do the regression test tomorrorw. > > Thanks, Gary! > > Xiaoyu might post a v4 with a remote topic branch for reviewers to > fetch; I suggest awaiting that. (The series is difficult to apply with > git-am.) > > Thanks > Laszlo > > > Cheers, > > > > Gary Lin > > > > > > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b 2019-05-14 13:26 ` Wang, Jian J @ 2019-05-15 1:53 ` Gary Lin 2019-05-15 2:00 ` Xiaoyu lu 0 siblings, 1 reply; 18+ messages in thread From: Gary Lin @ 2019-05-15 1:53 UTC (permalink / raw) To: devel, jian.j.wang; +Cc: Laszlo Ersek, Lu, XiaoyuX, Ye, Ting On Tue, May 14, 2019 at 01:26:15PM +0000, Wang, Jian J wrote: > Yes, please wait for v4 version of this patch series. Good. I'm looking forward to the new series :) Thanks, Gary Lin > > Regards, > Jian > > > > -----Original Message----- > > From: Laszlo Ersek [mailto:lersek@redhat.com] > > Sent: Tuesday, May 14, 2019 8:06 PM > > To: devel@edk2.groups.io; glin@suse.com > > Cc: Lu, XiaoyuX <xiaoyux.lu@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; > > Ye, Ting <ting.ye@intel.com> > > Subject: Re: [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b > > > > On 05/14/19 08:16, Gary Lin wrote: > > > On Mon, May 13, 2019 at 09:24:39PM +0200, Laszlo Ersek wrote: > > >> On 05/13/19 15:25, Xiaoyu lu wrote: > > >>> (1) CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL > > >>> OpenSSL only support seeding NONE for UEFI(rand_unix.c line 93). > > >>> So add --with-rand-seed=none to process_files.pl. > > >>> > > >>> (2) CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl > > >>> When running process_files.py to configure OpenSSL, we can exclude some > > unnecessary files. This can reduce porting time, compiling time and library size. > > >>> > > >>> (3) CryptoPkg/IntrinsicLib: Fix possible unresolved external symbol issue > > >>> > > >>> (4) CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL > > >>> Disable warning for building OpenSSL_1_1_1b > > >>> > > >>> (5) CryptoPkg: Upgrade OpenSSL to 1.1.1b > > >>> Update OpenSSL submodule to OpenSSL_1_1_1b > > >>> OpenSSL_1_1_1b(50eaac9f3337667259de725451f201e784599687) > > >>> > > >>> OpenSSL doesn't implement some rand_pool function for UEFI. > > >>> Use EFI_RNG_PROTOCOL to generate random for entropy. > > >>> If EFI_RNG_PROTOCOL is not avaliable, fall back to performance > > >>> counter, but we not sure about the amount of randomness it provides. > > >>> > > >>> (6) CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward compatible > > >>> > > >>> Note: Will be remove next update. > > >>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1792 > > >>> Ref: https://github.com/openssl/openssl/pull/4338 > > >>> > > >>> > > >>> Cc: Jian J Wang <jian.j.wang@intel.com> > > >>> Cc: Ting Ye <ting.ye@intel.com> > > >> > > >> I'm withdrawing from reviewing or testing this series. > > >> > > >> Gary, if you have the time, can you please regression test this (for > > >> HTTPS boot) in both OVMF and ArmVirtQemu? > > >> > > > I'll find some time to do the regression test tomorrorw. > > > > Thanks, Gary! > > > > Xiaoyu might post a v4 with a remote topic branch for reviewers to > > fetch; I suggest awaiting that. (The series is difficult to apply with > > git-am.) > > > > Thanks > > Laszlo > > > > > Cheers, > > > > > > Gary Lin > > > > > > > > > > > > > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b 2019-05-15 1:53 ` Gary Lin @ 2019-05-15 2:00 ` Xiaoyu lu 2019-05-15 4:33 ` Gary Lin 2019-05-15 8:06 ` Laszlo Ersek 0 siblings, 2 replies; 18+ messages in thread From: Xiaoyu lu @ 2019-05-15 2:00 UTC (permalink / raw) To: devel@edk2.groups.io, glin@suse.com, Wang, Jian J; +Cc: Laszlo Ersek, Ye, Ting Hi Gary Lin: I also need to modify the code about the entropy source today. But I have uploaded a TimerLib based implementation. https://github.com/xiaoyuxlu/edk2/commits/bz_1089_patch_v4 Thanks. Xiaoyu -----Original Message----- From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Gary Lin Sent: Wednesday, May 15, 2019 9:54 AM To: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com> Cc: Laszlo Ersek <lersek@redhat.com>; Lu, XiaoyuX <xiaoyux.lu@intel.com>; Ye, Ting <ting.ye@intel.com> Subject: Re: [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b On Tue, May 14, 2019 at 01:26:15PM +0000, Wang, Jian J wrote: > Yes, please wait for v4 version of this patch series. Good. I'm looking forward to the new series :) Thanks, Gary Lin > > Regards, > Jian > > > > -----Original Message----- > > From: Laszlo Ersek [mailto:lersek@redhat.com] > > Sent: Tuesday, May 14, 2019 8:06 PM > > To: devel@edk2.groups.io; glin@suse.com > > Cc: Lu, XiaoyuX <xiaoyux.lu@intel.com>; Wang, Jian J > > <jian.j.wang@intel.com>; Ye, Ting <ting.ye@intel.com> > > Subject: Re: [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL > > to 1.1.1b > > > > On 05/14/19 08:16, Gary Lin wrote: > > > On Mon, May 13, 2019 at 09:24:39PM +0200, Laszlo Ersek wrote: > > >> On 05/13/19 15:25, Xiaoyu lu wrote: > > >>> (1) CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL > > >>> OpenSSL only support seeding NONE for UEFI(rand_unix.c line 93). > > >>> So add --with-rand-seed=none to process_files.pl. > > >>> > > >>> (2) CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl > > >>> When running process_files.py to configure OpenSSL, we can > > >>> exclude some > > unnecessary files. This can reduce porting time, compiling time and library size. > > >>> > > >>> (3) CryptoPkg/IntrinsicLib: Fix possible unresolved external > > >>> symbol issue > > >>> > > >>> (4) CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL > > >>> Disable warning for building OpenSSL_1_1_1b > > >>> > > >>> (5) CryptoPkg: Upgrade OpenSSL to 1.1.1b > > >>> Update OpenSSL submodule to OpenSSL_1_1_1b > > >>> OpenSSL_1_1_1b(50eaac9f3337667259de725451f201e784599687) > > >>> > > >>> OpenSSL doesn't implement some rand_pool function for UEFI. > > >>> Use EFI_RNG_PROTOCOL to generate random for entropy. > > >>> If EFI_RNG_PROTOCOL is not avaliable, fall back to performance > > >>> counter, but we not sure about the amount of randomness it provides. > > >>> > > >>> (6) CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward > > >>> compatible > > >>> > > >>> Note: Will be remove next update. > > >>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1792 > > >>> Ref: https://github.com/openssl/openssl/pull/4338 > > >>> > > >>> > > >>> Cc: Jian J Wang <jian.j.wang@intel.com> > > >>> Cc: Ting Ye <ting.ye@intel.com> > > >> > > >> I'm withdrawing from reviewing or testing this series. > > >> > > >> Gary, if you have the time, can you please regression test this > > >> (for HTTPS boot) in both OVMF and ArmVirtQemu? > > >> > > > I'll find some time to do the regression test tomorrorw. > > > > Thanks, Gary! > > > > Xiaoyu might post a v4 with a remote topic branch for reviewers to > > fetch; I suggest awaiting that. (The series is difficult to apply > > with > > git-am.) > > > > Thanks > > Laszlo > > > > > Cheers, > > > > > > Gary Lin > > > > > > > > > > > > > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b 2019-05-15 2:00 ` Xiaoyu lu @ 2019-05-15 4:33 ` Gary Lin 2019-05-15 8:06 ` Laszlo Ersek 1 sibling, 0 replies; 18+ messages in thread From: Gary Lin @ 2019-05-15 4:33 UTC (permalink / raw) To: devel, xiaoyux.lu; +Cc: Wang, Jian J, Laszlo Ersek, Ye, Ting On Wed, May 15, 2019 at 02:00:27AM +0000, Xiaoyu lu wrote: > Hi Gary Lin: > I also need to modify the code about the entropy source today. > But I have uploaded a TimerLib based implementation. > > https://github.com/xiaoyuxlu/edk2/commits/bz_1089_patch_v4 Thanks! I'll go through the commits today and prepare for the incoming series. Gary Lin > > Thanks. > Xiaoyu > > -----Original Message----- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Gary Lin > Sent: Wednesday, May 15, 2019 9:54 AM > To: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com> > Cc: Laszlo Ersek <lersek@redhat.com>; Lu, XiaoyuX <xiaoyux.lu@intel.com>; Ye, Ting <ting.ye@intel.com> > Subject: Re: [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b > > On Tue, May 14, 2019 at 01:26:15PM +0000, Wang, Jian J wrote: > > Yes, please wait for v4 version of this patch series. > > Good. I'm looking forward to the new series :) > > Thanks, > > Gary Lin > > > > > Regards, > > Jian > > > > > > > -----Original Message----- > > > From: Laszlo Ersek [mailto:lersek@redhat.com] > > > Sent: Tuesday, May 14, 2019 8:06 PM > > > To: devel@edk2.groups.io; glin@suse.com > > > Cc: Lu, XiaoyuX <xiaoyux.lu@intel.com>; Wang, Jian J > > > <jian.j.wang@intel.com>; Ye, Ting <ting.ye@intel.com> > > > Subject: Re: [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL > > > to 1.1.1b > > > > > > On 05/14/19 08:16, Gary Lin wrote: > > > > On Mon, May 13, 2019 at 09:24:39PM +0200, Laszlo Ersek wrote: > > > >> On 05/13/19 15:25, Xiaoyu lu wrote: > > > >>> (1) CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL > > > >>> OpenSSL only support seeding NONE for UEFI(rand_unix.c line 93). > > > >>> So add --with-rand-seed=none to process_files.pl. > > > >>> > > > >>> (2) CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl > > > >>> When running process_files.py to configure OpenSSL, we can > > > >>> exclude some > > > unnecessary files. This can reduce porting time, compiling time and library size. > > > >>> > > > >>> (3) CryptoPkg/IntrinsicLib: Fix possible unresolved external > > > >>> symbol issue > > > >>> > > > >>> (4) CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL > > > >>> Disable warning for building OpenSSL_1_1_1b > > > >>> > > > >>> (5) CryptoPkg: Upgrade OpenSSL to 1.1.1b > > > >>> Update OpenSSL submodule to OpenSSL_1_1_1b > > > >>> OpenSSL_1_1_1b(50eaac9f3337667259de725451f201e784599687) > > > >>> > > > >>> OpenSSL doesn't implement some rand_pool function for UEFI. > > > >>> Use EFI_RNG_PROTOCOL to generate random for entropy. > > > >>> If EFI_RNG_PROTOCOL is not avaliable, fall back to performance > > > >>> counter, but we not sure about the amount of randomness it provides. > > > >>> > > > >>> (6) CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward > > > >>> compatible > > > >>> > > > >>> Note: Will be remove next update. > > > >>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1792 > > > >>> Ref: https://github.com/openssl/openssl/pull/4338 > > > >>> > > > >>> > > > >>> Cc: Jian J Wang <jian.j.wang@intel.com> > > > >>> Cc: Ting Ye <ting.ye@intel.com> > > > >> > > > >> I'm withdrawing from reviewing or testing this series. > > > >> > > > >> Gary, if you have the time, can you please regression test this > > > >> (for HTTPS boot) in both OVMF and ArmVirtQemu? > > > >> > > > > I'll find some time to do the regression test tomorrorw. > > > > > > Thanks, Gary! > > > > > > Xiaoyu might post a v4 with a remote topic branch for reviewers to > > > fetch; I suggest awaiting that. (The series is difficult to apply > > > with > > > git-am.) > > > > > > Thanks > > > Laszlo > > > > > > > Cheers, > > > > > > > > Gary Lin > > > > > > > > > > > > > > > > > > > > > > > > > > > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b 2019-05-15 2:00 ` Xiaoyu lu 2019-05-15 4:33 ` Gary Lin @ 2019-05-15 8:06 ` Laszlo Ersek 2019-05-15 8:58 ` Xiaoyu lu 1 sibling, 1 reply; 18+ messages in thread From: Laszlo Ersek @ 2019-05-15 8:06 UTC (permalink / raw) To: Lu, XiaoyuX, devel@edk2.groups.io, glin@suse.com, Wang, Jian J; +Cc: Ye, Ting Hi Xiaoyu, On 05/15/19 04:00, Lu, XiaoyuX wrote: > Hi Gary Lin: > I also need to modify the code about the entropy source today. > But I have uploaded a TimerLib based implementation. > > https://github.com/xiaoyuxlu/edk2/commits/bz_1089_patch_v4 This is not a good strategy. Please refer to contributor step 31: https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-31 You should push a topic branch called "xxxx_v4" *only* if you are ready to post it immediately to the list, as "PATCH v4". Topic branches in personal repos must be *identical* to the corresponding posting on edk2-devel. And once such a topic branch is pushed and referenced in an edk2-devel posting, the branch should never ever be modified again. Not rebased, not force-pushed, not fast-forwarded to additional commits on top. Once you have a v4 posting on edk2-devel, the topic branch *for that version* becomes read-only. If you need updates, you need to prepare a v5. It's OK to push (even force-push) branches to your personal repo that are work-in-progress. However, the name of the branch should be very clear about that. For example, you could call the branch "bz_1089_patch_v4_wip", with the "_wip" suffix standing for "work-in-progress". Then people fetching that branch will understand it's not final, and may easily change until the mailing list posting. When you decide it's time to post, you can rename the branch (drop the "_wip" suffix), from which point on you should treat the branch as read-only. Thanks Laszlo > -----Original Message----- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Gary Lin > Sent: Wednesday, May 15, 2019 9:54 AM > To: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com> > Cc: Laszlo Ersek <lersek@redhat.com>; Lu, XiaoyuX <xiaoyux.lu@intel.com>; Ye, Ting <ting.ye@intel.com> > Subject: Re: [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b > > On Tue, May 14, 2019 at 01:26:15PM +0000, Wang, Jian J wrote: >> Yes, please wait for v4 version of this patch series. > > Good. I'm looking forward to the new series :) > > Thanks, > > Gary Lin > >> >> Regards, >> Jian >> >> >>> -----Original Message----- >>> From: Laszlo Ersek [mailto:lersek@redhat.com] >>> Sent: Tuesday, May 14, 2019 8:06 PM >>> To: devel@edk2.groups.io; glin@suse.com >>> Cc: Lu, XiaoyuX <xiaoyux.lu@intel.com>; Wang, Jian J >>> <jian.j.wang@intel.com>; Ye, Ting <ting.ye@intel.com> >>> Subject: Re: [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL >>> to 1.1.1b >>> >>> On 05/14/19 08:16, Gary Lin wrote: >>>> On Mon, May 13, 2019 at 09:24:39PM +0200, Laszlo Ersek wrote: >>>>> On 05/13/19 15:25, Xiaoyu lu wrote: >>>>>> (1) CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL >>>>>> OpenSSL only support seeding NONE for UEFI(rand_unix.c line 93). >>>>>> So add --with-rand-seed=none to process_files.pl. >>>>>> >>>>>> (2) CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl >>>>>> When running process_files.py to configure OpenSSL, we can >>>>>> exclude some >>> unnecessary files. This can reduce porting time, compiling time and library size. >>>>>> >>>>>> (3) CryptoPkg/IntrinsicLib: Fix possible unresolved external >>>>>> symbol issue >>>>>> >>>>>> (4) CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL >>>>>> Disable warning for building OpenSSL_1_1_1b >>>>>> >>>>>> (5) CryptoPkg: Upgrade OpenSSL to 1.1.1b >>>>>> Update OpenSSL submodule to OpenSSL_1_1_1b >>>>>> OpenSSL_1_1_1b(50eaac9f3337667259de725451f201e784599687) >>>>>> >>>>>> OpenSSL doesn't implement some rand_pool function for UEFI. >>>>>> Use EFI_RNG_PROTOCOL to generate random for entropy. >>>>>> If EFI_RNG_PROTOCOL is not avaliable, fall back to performance >>>>>> counter, but we not sure about the amount of randomness it provides. >>>>>> >>>>>> (6) CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward >>>>>> compatible >>>>>> >>>>>> Note: Will be remove next update. >>>>>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1792 >>>>>> Ref: https://github.com/openssl/openssl/pull/4338 >>>>>> >>>>>> >>>>>> Cc: Jian J Wang <jian.j.wang@intel.com> >>>>>> Cc: Ting Ye <ting.ye@intel.com> >>>>> >>>>> I'm withdrawing from reviewing or testing this series. >>>>> >>>>> Gary, if you have the time, can you please regression test this >>>>> (for HTTPS boot) in both OVMF and ArmVirtQemu? >>>>> >>>> I'll find some time to do the regression test tomorrorw. >>> >>> Thanks, Gary! >>> >>> Xiaoyu might post a v4 with a remote topic branch for reviewers to >>> fetch; I suggest awaiting that. (The series is difficult to apply >>> with >>> git-am.) >>> >>> Thanks >>> Laszlo >>> >>>> Cheers, >>>> >>>> Gary Lin >>>> >>>> >>>> >> >> >> >> > > > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b 2019-05-15 8:06 ` Laszlo Ersek @ 2019-05-15 8:58 ` Xiaoyu lu 0 siblings, 0 replies; 18+ messages in thread From: Xiaoyu lu @ 2019-05-15 8:58 UTC (permalink / raw) To: Laszlo Ersek, devel@edk2.groups.io, glin@suse.com, Wang, Jian J; +Cc: Ye, Ting Hi Laszlo: Thanks for your information. If I send the patch v4, I will provide a new branch in my personal repos and not modify it. Thanks, Xiaoyu -----Original Message----- From: Laszlo Ersek [mailto:lersek@redhat.com] Sent: Wednesday, May 15, 2019 4:07 PM To: Lu, XiaoyuX <xiaoyux.lu@intel.com>; devel@edk2.groups.io; glin@suse.com; Wang, Jian J <jian.j.wang@intel.com> Cc: Ye, Ting <ting.ye@intel.com> Subject: Re: [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b Hi Xiaoyu, On 05/15/19 04:00, Lu, XiaoyuX wrote: > Hi Gary Lin: > I also need to modify the code about the entropy source today. > But I have uploaded a TimerLib based implementation. > > https://github.com/xiaoyuxlu/edk2/commits/bz_1089_patch_v4 This is not a good strategy. Please refer to contributor step 31: https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-31 You should push a topic branch called "xxxx_v4" *only* if you are ready to post it immediately to the list, as "PATCH v4". Topic branches in personal repos must be *identical* to the corresponding posting on edk2-devel. And once such a topic branch is pushed and referenced in an edk2-devel posting, the branch should never ever be modified again. Not rebased, not force-pushed, not fast-forwarded to additional commits on top. Once you have a v4 posting on edk2-devel, the topic branch *for that version* becomes read-only. If you need updates, you need to prepare a v5. It's OK to push (even force-push) branches to your personal repo that are work-in-progress. However, the name of the branch should be very clear about that. For example, you could call the branch "bz_1089_patch_v4_wip", with the "_wip" suffix standing for "work-in-progress". Then people fetching that branch will understand it's not final, and may easily change until the mailing list posting. When you decide it's time to post, you can rename the branch (drop the "_wip" suffix), from which point on you should treat the branch as read-only. Thanks Laszlo > -----Original Message----- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > Gary Lin > Sent: Wednesday, May 15, 2019 9:54 AM > To: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com> > Cc: Laszlo Ersek <lersek@redhat.com>; Lu, XiaoyuX > <xiaoyux.lu@intel.com>; Ye, Ting <ting.ye@intel.com> > Subject: Re: [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to > 1.1.1b > > On Tue, May 14, 2019 at 01:26:15PM +0000, Wang, Jian J wrote: >> Yes, please wait for v4 version of this patch series. > > Good. I'm looking forward to the new series :) > > Thanks, > > Gary Lin > >> >> Regards, >> Jian >> >> >>> -----Original Message----- >>> From: Laszlo Ersek [mailto:lersek@redhat.com] >>> Sent: Tuesday, May 14, 2019 8:06 PM >>> To: devel@edk2.groups.io; glin@suse.com >>> Cc: Lu, XiaoyuX <xiaoyux.lu@intel.com>; Wang, Jian J >>> <jian.j.wang@intel.com>; Ye, Ting <ting.ye@intel.com> >>> Subject: Re: [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL >>> to 1.1.1b >>> >>> On 05/14/19 08:16, Gary Lin wrote: >>>> On Mon, May 13, 2019 at 09:24:39PM +0200, Laszlo Ersek wrote: >>>>> On 05/13/19 15:25, Xiaoyu lu wrote: >>>>>> (1) CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL >>>>>> OpenSSL only support seeding NONE for UEFI(rand_unix.c line 93). >>>>>> So add --with-rand-seed=none to process_files.pl. >>>>>> >>>>>> (2) CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl >>>>>> When running process_files.py to configure OpenSSL, we can >>>>>> exclude some >>> unnecessary files. This can reduce porting time, compiling time and library size. >>>>>> >>>>>> (3) CryptoPkg/IntrinsicLib: Fix possible unresolved external >>>>>> symbol issue >>>>>> >>>>>> (4) CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL >>>>>> Disable warning for building OpenSSL_1_1_1b >>>>>> >>>>>> (5) CryptoPkg: Upgrade OpenSSL to 1.1.1b >>>>>> Update OpenSSL submodule to OpenSSL_1_1_1b >>>>>> OpenSSL_1_1_1b(50eaac9f3337667259de725451f201e784599687) >>>>>> >>>>>> OpenSSL doesn't implement some rand_pool function for UEFI. >>>>>> Use EFI_RNG_PROTOCOL to generate random for entropy. >>>>>> If EFI_RNG_PROTOCOL is not avaliable, fall back to performance >>>>>> counter, but we not sure about the amount of randomness it provides. >>>>>> >>>>>> (6) CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward >>>>>> compatible >>>>>> >>>>>> Note: Will be remove next update. >>>>>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1792 >>>>>> Ref: https://github.com/openssl/openssl/pull/4338 >>>>>> >>>>>> >>>>>> Cc: Jian J Wang <jian.j.wang@intel.com> >>>>>> Cc: Ting Ye <ting.ye@intel.com> >>>>> >>>>> I'm withdrawing from reviewing or testing this series. >>>>> >>>>> Gary, if you have the time, can you please regression test this >>>>> (for HTTPS boot) in both OVMF and ArmVirtQemu? >>>>> >>>> I'll find some time to do the regression test tomorrorw. >>> >>> Thanks, Gary! >>> >>> Xiaoyu might post a v4 with a remote topic branch for reviewers to >>> fetch; I suggest awaiting that. (The series is difficult to apply >>> with >>> git-am.) >>> >>> Thanks >>> Laszlo >>> >>>> Cheers, >>>> >>>> Gary Lin >>>> >>>> >>>> >> >> >> >> > > > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b 2019-05-13 19:24 ` [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b Laszlo Ersek 2019-05-14 6:16 ` Gary Lin @ 2019-05-14 11:58 ` Laszlo Ersek 2019-05-14 15:52 ` Xiaoyu lu 1 sibling, 1 reply; 18+ messages in thread From: Laszlo Ersek @ 2019-05-14 11:58 UTC (permalink / raw) To: xiaoyux.lu; +Cc: devel, Gary Lin, Jian J Wang, Ting Ye On 05/13/19 21:24, Laszlo Ersek wrote: > On 05/13/19 15:25, Xiaoyu lu wrote: >> (1) CryptoPkg/OpensslLib: Modify process_files.pl for upgrading >> OpenSSL OpenSSL only support seeding NONE for UEFI(rand_unix.c line >> 93). So add --with-rand-seed=none to process_files.pl. >> >> (2) CryptoPkg/OpensslLib: Exclude unnecessary files in >> process_files.pl >> When running process_files.py to configure OpenSSL, we can exclude >> some unnecessary files. This can reduce porting time, compiling >> time and library size. >> >> (3) CryptoPkg/IntrinsicLib: Fix possible unresolved external symbol >> issue >> >> (4) CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL >> Disable warning for building OpenSSL_1_1_1b >> >> (5) CryptoPkg: Upgrade OpenSSL to 1.1.1b >> Update OpenSSL submodule to OpenSSL_1_1_1b >> OpenSSL_1_1_1b(50eaac9f3337667259de725451f201e784599687) >> >> OpenSSL doesn't implement some rand_pool function for UEFI. >> Use EFI_RNG_PROTOCOL to generate random for entropy. >> If EFI_RNG_PROTOCOL is not avaliable, fall back to performance >> counter, but we not sure about the amount of randomness it >> provides. >> >> (6) CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward compatible >> >> Note: Will be remove next update. >> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1792 >> Ref: https://github.com/openssl/openssl/pull/4338 >> >> >> Cc: Jian J Wang <jian.j.wang@intel.com> >> Cc: Ting Ye <ting.ye@intel.com> > > I'm withdrawing from reviewing or testing this series. To be clear, the reason I abandoned reviewing / testing this series is not due to the use of TimerLib as entropy source, in patch #5. I addressed that separately, stating that I wouldn't review patch #5, only regression-test it. The reason I intend to leave upcoming reviews & testing of this series as a whole to others is that I've found a number of mistakes in relation to the development workflow. And, it's exhausting for me to repeat all the same guidelines, when I had documented them in the wiki [*]. At the same time, I realize that it may be difficult for a new edk2 contributor to adhere to everything described in [*] -- especially given that [*] is not an official edk2 document, just something that I personally distilled from experience. In other words, my insisting on [*] in many repeated emails is exhausting for both new contributors, and myself as a reviewer. Which is why I thought I'd save us both some busywork, by withdrawing from this series. If you'd like me to look over this series again, then a v4 will be necessary, just in order to remedy the following workflow-level problems. (Afterwards, a v5 may be necessary for further technical fixes.) (1) Some of your patches are authored by "Xiaoyu Lu", some others by "Xiaoyu lu" (lower case). This messes up the shortlog in the blurb (and other statistics collected from the git log); you are represented as two different people. Please pick *one* email address (name included), and stick with that. Rebase the series, and use git commit --amend --author=... for fixing up the authorship on the patches that need it. Make sure your Signed-off-by follows suit in the commit messages. (2) The series is hard to apply for local testing, with "git am", due to patch #5 modifying both CRLF and LF files. That's not necessarily a problem with the patch itself, but the norm has been, for non-trivial patch series, to push a topic branch to a personal repo, and to reference that repo & branch in the blurb. It permits easy fetching and easy commenting both. This wasn't done in v2, and I struggled with "git am". It hasn't been done in v3 either. Please do it in v4 and further versions. (3) In my review of the v1 series, I requested that the CC_FLAGS changes, for the "OpensslLib.inf" and "OpensslLibCrypto.inf" files, be isolated to their own standalone patch. In v2, this was nicely addressed in patch #4, and I gave my R-b. In v3 however, you squashed a totally unrelated -- but at the series level, necessary -- change into the same patch (namely "sys/syscall.h"). While that improved the end result of the series for sure, it *negated* the v2 improvement in the specific patch. In my v2 review, this was how I asked for "sys/syscall.h": So please include a patch in the v3 series that adds "CryptoPkg/Library/Include/sys/syscall.h" like suggested above. *Separate patch*. If you disagree with my request, that's 100% part of the process, but then please respond under the request, rather than dumping an entire new version of the series on me that does not comply with my request. (4) In version 3, you failed to pick up my Reviewed-by tags that I had given for v2 1/6 and v2 6/6. In more technical terms, this means that you should have run "git rebase -i", selected the "reword" action for patches v2 1/6 and v2 6/6, and appended -- using the clipboard -- my R-b tags, from my review emails, to the commit messages. This is documented in detail, in [*] (contributor step 28). (Referring to the previous bullet, you also failed to pick up my R-b for patch v2 4/6. However, ultimately, that was the correct action for that patch, given that you modified the patch in v3. If a patch is modified significantly in a revision, then review tags garnered earlier should be dropped, so that reviewers check the patch again.) (5) Jian had some questions still open under v2 5/6, when you posted v3. The questions were addressed to me. Sometimes I cannot answer on the next day, and yes, there was a weekend to. If you think a reviewer missed something, please wait one or two business days, and ping them off-list or on-list, before sending the next version. If there isn't enough time left to catch the upcoming stable tag with this work, then we should postpone this work to the next stable tag. [*] https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers Thanks Laszlo ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b 2019-05-14 11:58 ` Laszlo Ersek @ 2019-05-14 15:52 ` Xiaoyu lu 0 siblings, 0 replies; 18+ messages in thread From: Xiaoyu lu @ 2019-05-14 15:52 UTC (permalink / raw) To: devel@edk2.groups.io, lersek@redhat.com; +Cc: Gary Lin, Wang, Jian J, Ye, Ting Thank you, Laszlo. I am very appreciate to you for being so patient with me . (1) I cleaned the authored name. (2) CryptoPkg/Library/Include/openssl/opensslconf.h This file is LF file, It copy from openssl, I think it should not be modified. Pushed my private repository to https://github.com/xiaoyuxlu/edk2/commits/bz_1089_patch_v4 I have not finished yet. When I finish it, I will push v4 patches (3) Thank you for explaining this clearly. I changed it back and added a patch. (4) Now I know I should take R-b tags into commit message and the meaning to modify 'R-b tags patch'. If I modify it, should refer to R-b tags owner's opinion. I apologize for modify your R-b tags patch which makes you feel bad. (5) Got it. I think it is very useful for me. [*] https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers Thank you again. Xiaoyu. -----Original Message----- From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Laszlo Ersek Sent: Tuesday, May 14, 2019 7:59 PM To: Lu, XiaoyuX <xiaoyux.lu@intel.com> Cc: devel@edk2.groups.io; Gary Lin <glin@suse.com>; Wang, Jian J <jian.j.wang@intel.com>; Ye, Ting <ting.ye@intel.com> Subject: Re: [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b On 05/13/19 21:24, Laszlo Ersek wrote: > On 05/13/19 15:25, Xiaoyu lu wrote: >> (1) CryptoPkg/OpensslLib: Modify process_files.pl for upgrading >> OpenSSL OpenSSL only support seeding NONE for UEFI(rand_unix.c line >> 93). So add --with-rand-seed=none to process_files.pl. >> >> (2) CryptoPkg/OpensslLib: Exclude unnecessary files in >> process_files.pl >> When running process_files.py to configure OpenSSL, we can exclude >> some unnecessary files. This can reduce porting time, compiling >> time and library size. >> >> (3) CryptoPkg/IntrinsicLib: Fix possible unresolved external symbol >> issue >> >> (4) CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL >> Disable warning for building OpenSSL_1_1_1b >> >> (5) CryptoPkg: Upgrade OpenSSL to 1.1.1b >> Update OpenSSL submodule to OpenSSL_1_1_1b >> OpenSSL_1_1_1b(50eaac9f3337667259de725451f201e784599687) >> >> OpenSSL doesn't implement some rand_pool function for UEFI. >> Use EFI_RNG_PROTOCOL to generate random for entropy. >> If EFI_RNG_PROTOCOL is not avaliable, fall back to performance >> counter, but we not sure about the amount of randomness it >> provides. >> >> (6) CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward compatible >> >> Note: Will be remove next update. >> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1792 >> Ref: https://github.com/openssl/openssl/pull/4338 >> >> >> Cc: Jian J Wang <jian.j.wang@intel.com> >> Cc: Ting Ye <ting.ye@intel.com> > > I'm withdrawing from reviewing or testing this series. To be clear, the reason I abandoned reviewing / testing this series is not due to the use of TimerLib as entropy source, in patch #5. I addressed that separately, stating that I wouldn't review patch #5, only regression-test it. The reason I intend to leave upcoming reviews & testing of this series as a whole to others is that I've found a number of mistakes in relation to the development workflow. And, it's exhausting for me to repeat all the same guidelines, when I had documented them in the wiki [*]. At the same time, I realize that it may be difficult for a new edk2 contributor to adhere to everything described in [*] -- especially given that [*] is not an official edk2 document, just something that I personally distilled from experience. In other words, my insisting on [*] in many repeated emails is exhausting for both new contributors, and myself as a reviewer. Which is why I thought I'd save us both some busywork, by withdrawing from this series. If you'd like me to look over this series again, then a v4 will be necessary, just in order to remedy the following workflow-level problems. (Afterwards, a v5 may be necessary for further technical fixes.) (1) Some of your patches are authored by "Xiaoyu Lu", some others by "Xiaoyu lu" (lower case). This messes up the shortlog in the blurb (and other statistics collected from the git log); you are represented as two different people. Please pick *one* email address (name included), and stick with that. Rebase the series, and use git commit --amend --author=... for fixing up the authorship on the patches that need it. Make sure your Signed-off-by follows suit in the commit messages. (2) The series is hard to apply for local testing, with "git am", due to patch #5 modifying both CRLF and LF files. That's not necessarily a problem with the patch itself, but the norm has been, for non-trivial patch series, to push a topic branch to a personal repo, and to reference that repo & branch in the blurb. It permits easy fetching and easy commenting both. This wasn't done in v2, and I struggled with "git am". It hasn't been done in v3 either. Please do it in v4 and further versions. (3) In my review of the v1 series, I requested that the CC_FLAGS changes, for the "OpensslLib.inf" and "OpensslLibCrypto.inf" files, be isolated to their own standalone patch. In v2, this was nicely addressed in patch #4, and I gave my R-b. In v3 however, you squashed a totally unrelated -- but at the series level, necessary -- change into the same patch (namely "sys/syscall.h"). While that improved the end result of the series for sure, it *negated* the v2 improvement in the specific patch. In my v2 review, this was how I asked for "sys/syscall.h": So please include a patch in the v3 series that adds "CryptoPkg/Library/Include/sys/syscall.h" like suggested above. *Separate patch*. If you disagree with my request, that's 100% part of the process, but then please respond under the request, rather than dumping an entire new version of the series on me that does not comply with my request. (4) In version 3, you failed to pick up my Reviewed-by tags that I had given for v2 1/6 and v2 6/6. In more technical terms, this means that you should have run "git rebase -i", selected the "reword" action for patches v2 1/6 and v2 6/6, and appended -- using the clipboard -- my R-b tags, from my review emails, to the commit messages. This is documented in detail, in [*] (contributor step 28). (Referring to the previous bullet, you also failed to pick up my R-b for patch v2 4/6. However, ultimately, that was the correct action for that patch, given that you modified the patch in v3. If a patch is modified significantly in a revision, then review tags garnered earlier should be dropped, so that reviewers check the patch again.) (5) Jian had some questions still open under v2 5/6, when you posted v3. The questions were addressed to me. Sometimes I cannot answer on the next day, and yes, there was a weekend to. If you think a reviewer missed something, please wait one or two business days, and ping them off-list or on-list, before sending the next version. If there isn't enough time left to catch the upcoming stable tag with this work, then we should postpone this work to the next stable tag. [*] https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers Thanks Laszlo ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2019-05-15 8:58 UTC | newest] Thread overview: 18+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-05-13 13:25 [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b Xiaoyu lu 2019-05-13 13:25 ` [PATCH v3 1/6] CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL Xiaoyu lu 2019-05-13 13:25 ` [PATCH v3 2/6] CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl Xiaoyu lu 2019-05-13 13:25 ` [PATCH v3 3/6] CryptoPkg/IntrinsicLib: Fix possible unresolved external symbol issue Xiaoyu lu 2019-05-13 13:25 ` [PATCH v3 4/6] CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL Xiaoyu lu 2019-05-13 13:25 ` [PATCH v3 5/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b Xiaoyu lu 2019-05-13 13:25 ` [PATCH v3 6/6] CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward compatible Xiaoyu lu 2019-05-13 19:24 ` [edk2-devel] [PATCH v3 0/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b Laszlo Ersek 2019-05-14 6:16 ` Gary Lin 2019-05-14 12:06 ` Laszlo Ersek 2019-05-14 13:26 ` Wang, Jian J 2019-05-15 1:53 ` Gary Lin 2019-05-15 2:00 ` Xiaoyu lu 2019-05-15 4:33 ` Gary Lin 2019-05-15 8:06 ` Laszlo Ersek 2019-05-15 8:58 ` Xiaoyu lu 2019-05-14 11:58 ` Laszlo Ersek 2019-05-14 15:52 ` Xiaoyu lu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox