* [PATCH v2 1/6] CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL
@ 2019-05-09 5:23 Xiaoyu lu
2019-05-09 5:23 ` [PATCH v2 2/6] CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl Xiaoyu lu
` (5 more replies)
0 siblings, 6 replies; 27+ messages in thread
From: Xiaoyu lu @ 2019-05-09 5:23 UTC (permalink / raw)
To: devel; +Cc: lersek, Jian J Wang, Ting Ye, Xiaoyu Lu
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).
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] 27+ messages in thread
* [PATCH v2 2/6] CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl
2019-05-09 5:23 [PATCH v2 1/6] CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL Xiaoyu lu
@ 2019-05-09 5:23 ` Xiaoyu lu
2019-05-09 13:42 ` [edk2-devel] " Laszlo Ersek
2019-05-09 5:23 ` [PATCH v2 3/6] CryptoPkg/IntrinsicLib: Fix possible unresolved external symbol issue Xiaoyu lu
` (4 subsequent siblings)
5 siblings, 1 reply; 27+ messages in thread
From: Xiaoyu lu @ 2019-05-09 5:23 UTC (permalink / raw)
To: devel; +Cc: lersek, Jian J Wang, Ting Ye, Xiaoyu Lu
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.
OpenSSL_1_1_1(1708e3e85b4a8) add a STORE module (crypto/store/*).
But UEFI don't use them. So exclude these files.
This file, crypto/rand/randfile.c, have been modified between
OpenSSL_1_1_0j(74f2d9c1ec5f5) and OpenSSL_1_1_1b(50eaac9f33376672).
It requires more crt runtime support. But UEFI don't use it.
So exclude the file.
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] 27+ messages in thread
* [PATCH v2 3/6] CryptoPkg/IntrinsicLib: Fix possible unresolved external symbol issue
2019-05-09 5:23 [PATCH v2 1/6] CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL Xiaoyu lu
2019-05-09 5:23 ` [PATCH v2 2/6] CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl Xiaoyu lu
@ 2019-05-09 5:23 ` Xiaoyu lu
2019-05-09 17:16 ` [edk2-devel] " Laszlo Ersek
2019-05-09 5:23 ` [PATCH v2 4/6] CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL Xiaoyu lu
` (3 subsequent siblings)
5 siblings, 1 reply; 27+ messages in thread
From: Xiaoyu lu @ 2019-05-09 5:23 UTC (permalink / raw)
To: devel; +Cc: lersek, Xiaoyu 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] 27+ messages in thread
* [PATCH v2 4/6] CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL
2019-05-09 5:23 [PATCH v2 1/6] CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL Xiaoyu lu
2019-05-09 5:23 ` [PATCH v2 2/6] CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl Xiaoyu lu
2019-05-09 5:23 ` [PATCH v2 3/6] CryptoPkg/IntrinsicLib: Fix possible unresolved external symbol issue Xiaoyu lu
@ 2019-05-09 5:23 ` Xiaoyu lu
2019-05-09 13:48 ` [edk2-devel] " Laszlo Ersek
2019-05-09 5:23 ` [PATCH v2 5/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b Xiaoyu lu
` (2 subsequent siblings)
5 siblings, 1 reply; 27+ messages in thread
From: Xiaoyu lu @ 2019-05-09 5:23 UTC (permalink / raw)
To: devel; +Cc: lersek, Jian J Wang, Ting Ye, Xiaoyu Lu
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.
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/OpensslLib.inf | 16 ++++++++++------
CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf | 16 ++++++++++------
2 files changed, 20 insertions(+), 12 deletions(-)
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] 27+ messages in thread
* [PATCH v2 5/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b
2019-05-09 5:23 [PATCH v2 1/6] CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL Xiaoyu lu
` (2 preceding siblings ...)
2019-05-09 5:23 ` [PATCH v2 4/6] CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL Xiaoyu lu
@ 2019-05-09 5:23 ` Xiaoyu lu
2019-05-09 17:15 ` [edk2-devel] " Laszlo Ersek
2019-05-09 20:58 ` Laszlo Ersek
2019-05-09 5:23 ` [PATCH v2 6/6] CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward compatible Xiaoyu lu
2019-05-09 11:32 ` [edk2-devel] [PATCH v2 1/6] CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL Laszlo Ersek
5 siblings, 2 replies; 27+ messages in thread
From: Xiaoyu lu @ 2019-05-09 5:23 UTC (permalink / raw)
To: devel; +Cc: lersek, Xiaoyu 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 NO_SYSLOG from OpensslLib[Crypto].inf
When OPENSSL_SYS_UEFI is defined, NO_SYSLOG not be defined
in OpenSSL_1_1_0j(74f2d9c1ec5f), but in
OpenSSL_1_1_1b(50eaac9f333), NO_SYSLOG will
be defined(e_os.h line 47).
Add compiler_flags to buildinf.h file.
>From OpenSSL_1_1_0i(97c0959f27b294fe1eb10b547145ebef2524b896) to
OpenSSL_1_1_1b(50eaac9f3337667259de725451f201e784599687), OpenSSL
updated DRBG / RAND to request nonce and additional low entropy
randomness from system(line 229 openssl/CHANGES).
git diff OpenSSL_1_1_0i OpenSSL_1_1_1b crypto/include/internal/rand_int.h
git diff OpenSSL_1_1_0i OpenSSL_1_1_1b crypto/rand/rand_unix.c
But it is not implement for UEFI.
Since OpenSSL_1_1_1b doesn't fully implement it. So add a new
file(rand_pool.c) and implement it base on TimerLib.
* 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 don't need ossl_store functions. So dummy implement them.
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.
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 | 8 +
CryptoPkg/Library/Include/openssl/opensslconf.h | 54 ++--
CryptoPkg/Library/OpensslLib/OpensslLib.inf | 44 +++-
CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf | 35 ++-
CryptoPkg/Library/OpensslLib/buildinf.h | 2 +
CryptoPkg/Library/OpensslLib/openssl | 2 +-
CryptoPkg/Library/OpensslLib/ossl_store.c | 17 ++
CryptoPkg/Library/OpensslLib/rand_pool.c | 292 ++++++++++++++++++++++
8 files changed, 425 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..193f8de 100644
--- a/CryptoPkg/Library/Include/CrtLibSupport.h
+++ b/CryptoPkg/Library/Include/CrtLibSupport.h
@@ -21,6 +21,14 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#define MAX_STRING_SIZE 0x1000
//
+// BUFSIZ used in evp_key.c
+// 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..5e6b99e 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,6 +548,7 @@
[LibraryClasses]
DebugLib
+ TimerLib
[LibraryClasses.ARM]
ArmSoftFloatLib
diff --git a/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
index fd12d11..1362a46 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,6 +504,7 @@
[LibraryClasses]
DebugLib
+ TimerLib
[LibraryClasses.ARM]
ArmSoftFloatLib
diff --git a/CryptoPkg/Library/OpensslLib/buildinf.h b/CryptoPkg/Library/OpensslLib/buildinf.h
index c5ca293..5b3b50b 100644
--- a/CryptoPkg/Library/OpensslLib/buildinf.h
+++ b/CryptoPkg/Library/OpensslLib/buildinf.h
@@ -1,2 +1,4 @@
#define PLATFORM "UEFI"
#define DATE "Fri Dec 22 01:23:45 PDT 2017"
+
+const char * compiler_flags = "";
diff --git a/CryptoPkg/Library/OpensslLib/openssl b/CryptoPkg/Library/OpensslLib/openssl
index 74f2d9c..50eaac9 160000
--- a/CryptoPkg/Library/OpensslLib/openssl
+++ b/CryptoPkg/Library/OpensslLib/openssl
@@ -1 +1 @@
-Subproject commit 74f2d9c1ec5f5510e1d3da5a9f03c28df0977762
+Subproject commit 50eaac9f3337667259de725451f201e784599687
diff --git a/CryptoPkg/Library/OpensslLib/ossl_store.c b/CryptoPkg/Library/OpensslLib/ossl_store.c
new file mode 100644
index 0000000..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..c7cdeb0
--- /dev/null
+++ b/CryptoPkg/Library/OpensslLib/rand_pool.c
@@ -0,0 +1,292 @@
+/** @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>
+
+/**
+ 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
+GetRandomSourceFromPerformanceCounter(
+ OUT UINT64 *Rand
+ )
+{
+ UINT32 Index;
+ UINT32 *RandPtr;
+ RandPtr = (UINT32 *)Rand;
+
+ if (Rand == NULL) {
+ return FALSE;
+ }
+
+ for (Index = 0; Index < 2; Index ++) {
+ *RandPtr = (UINT32)(GetPerformanceCounter() & 0xFF);
+ MicroSecondDelay(10);
+ RandPtr++;
+ }
+
+ return TRUE;
+}
+
+/**
+ Calls GetRandomSourceFromPerformanceCounter 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 = GetRandomSourceFromPerformanceCounter (&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)
+{
+ EFI_STATUS Status;
+ 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) {
+ Status = RandGenerateEntropy(bytes_needed, buffer);
+ if (EFI_ERROR (Status)) {
+ 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 Rand;
+ UINT64 TimerValue;
+ } data = { 0 };
+
+ 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] 27+ messages in thread
* [PATCH v2 6/6] CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward compatible
2019-05-09 5:23 [PATCH v2 1/6] CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL Xiaoyu lu
` (3 preceding siblings ...)
2019-05-09 5:23 ` [PATCH v2 5/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b Xiaoyu lu
@ 2019-05-09 5:23 ` Xiaoyu lu
2019-05-09 14:01 ` [edk2-devel] " Laszlo Ersek
2019-05-09 11:32 ` [edk2-devel] [PATCH v2 1/6] CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL Laszlo Ersek
5 siblings, 1 reply; 27+ messages in thread
From: Xiaoyu lu @ 2019-05-09 5:23 UTC (permalink / raw)
To: devel; +Cc: lersek, Xiaoyu 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).
We should not use it directly and should remove relevant
functions(Hmac*GetContextSize).
But for compatiblility, still updated 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..3a90e03 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..a8e8d44 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..fec60b1 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] 27+ messages in thread
* Re: [edk2-devel] [PATCH v2 1/6] CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL
2019-05-09 5:23 [PATCH v2 1/6] CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL Xiaoyu lu
` (4 preceding siblings ...)
2019-05-09 5:23 ` [PATCH v2 6/6] CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward compatible Xiaoyu lu
@ 2019-05-09 11:32 ` Laszlo Ersek
5 siblings, 0 replies; 27+ messages in thread
From: Laszlo Ersek @ 2019-05-09 11:32 UTC (permalink / raw)
To: devel, xiaoyux.lu; +Cc: Jian J Wang, Ting Ye
On 05/09/19 07:23, Xiaoyu lu wrote:
> 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).
(1) Please insert the following sentence here (no need to repost just
for this; can be done before pushing):
----
This OpenSSL change was introduced in commit
8389ec4b4950 ("Add --with-rand-seed", 2017-07-22).
----
with that:
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks
Laszlo
> 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";
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v2 2/6] CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl
2019-05-09 5:23 ` [PATCH v2 2/6] CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl Xiaoyu lu
@ 2019-05-09 13:42 ` Laszlo Ersek
2019-05-10 8:51 ` Xiaoyu lu
0 siblings, 1 reply; 27+ messages in thread
From: Laszlo Ersek @ 2019-05-09 13:42 UTC (permalink / raw)
To: devel, xiaoyux.lu; +Cc: Jian J Wang, Ting Ye
On 05/09/19 07:23, Xiaoyu lu wrote:
> 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.
Indeed.
> OpenSSL_1_1_1(1708e3e85b4a8) add a STORE module (crypto/store/*).
This statement is incorrect (or, minimally, inexact). According to the
following command:
$ git log --oneline --reverse OpenSSL_1_1_1b -- crypto/store/ \
| head -n 1
the first OpenSSL commit that added files to crypto/store/ was:
> commit a5db6fa5760f21d16d59e025e930c02456e00fef
> Author: Richard Levitte <levitte@openssl.org>
> Date: Thu May 1 03:53:12 2003 +0000
>
> Define a STORE type. For documentation, read the entry in CHANGES,
> crypto/store/README, crypto/store/store.h and crypto/store/str_locl.h.
This commit goes back to 2003, and is part of releae OpenSSL_0_9_7d.
Instead, let's check what the following command reports:
$ git log --oneline --reverse \
OpenSSL_1_1_0j..OpenSSL_1_1_1b -- crypto/store/ \
| head -1
It states that the first commit after OpenSSL_1_1_0j, but not after
OpenSSL_1_1_1b, to modify the "crypto/store/" subdirectory, was commit
71a5516dcc8a ("Add the STORE module", 2017-06-29).
If we investigate that commit:
$ git show --stat 71a5516dcc8a
we see that the commit modifies the Configure script:
> Configure | 2 +-
So let's check that part of the diff in detail:
$ git show 71a5516dcc8a -- Configure
And we get:
> diff --git a/Configure b/Configure
> index 2eacb2312e34..e302a58abb71 100755
> --- a/Configure
> +++ b/Configure
> @@ -310,7 +310,7 @@ $config{sdirs} = [
> "bn", "ec", "rsa", "dsa", "dh", "dso", "engine",
> "buffer", "bio", "stack", "lhash", "rand", "err",
> "evp", "asn1", "pem", "x509", "x509v3", "conf", "txt_db", "pkcs7",
> "pkcs12", "comp", "ocsp", "ui",
> - "cms", "ts", "srp", "cmac", "ct", "async", "kdf"
> + "cms", "ts", "srp", "cmac", "ct", "async", "kdf", "store"
> ];
> # test/ subdirectories to build
> $config{tdirs} = [ "ossl_shim" ];
We can see that the "store" module is added after modules such as "cms",
"ts", "srp", and so on.
Now, if you look at "CryptoPkg/Library/OpensslLib/process_files.pl", you
find (with edk2 master being at commit 49693202ec9c):
49 "./Configure",
50 "UEFI",
51 "no-afalgeng",
52 "no-asm",
53 "no-async", <---- disables "async"
54 "no-autoalginit",
55 "no-autoerrinit",
56 "no-bf",
57 "no-blake2",
58 "no-camellia",
59 "no-capieng",
60 "no-cast",
61 "no-chacha",
62 "no-cms", <---- disables "cms"
63 "no-ct", <---- disables "ct"
64 "no-deprecated",
65 "no-dgram",
66 "no-dsa",
67 "no-dynamic-engine",
68 "no-ec",
69 "no-ec2m",
70 "no-engine",
71 "no-err",
72 "no-filenames",
73 "no-gost",
74 "no-hw",
75 "no-idea",
76 "no-mdc2",
77 "no-pic",
78 "no-ocb",
79 "no-poly1305",
80 "no-posix-io",
81 "no-rc2",
82 "no-rfc3779",
83 "no-rmd160",
84 "no-scrypt",
85 "no-seed",
86 "no-sock",
87 "no-srp", <---- disables "srp"
88 "no-ssl",
89 "no-stdio",
90 "no-threads",
91 "no-ts", <---- disables "ts"
92 "no-ui",
93 "no-whirlpool"
(1) Therefore, the right thing to do here is to add "no-store" to the
above list, in my opinion. Can you try that, please?
And, this change should be a standalone patch, similarly to patch v2 1/6
in this series.
> But UEFI don't use them. So exclude these files.
> This file, crypto/rand/randfile.c, have been modified between
> OpenSSL_1_1_0j(74f2d9c1ec5f5) and OpenSSL_1_1_1b(50eaac9f33376672).
> It requires more crt runtime support. But UEFI don't use it.
> So exclude the file.
I think I disagree with this approach.
In OpenSSL commit fb4844bbc62f -- "Add UEFI flag for rand build",
2015-09-03, part of OpenSSL_1_1_0 --, Qin Long customized
"crypto/rand/rand_unix.c". So that, when OPENSSL_SYS_UEFI was #defined,
the real RAND_poll() function was replaced by a stub that would always
report failure. (So this was a safe stub.)
In OpenSSL commit 8389ec4b4950 -- "Add --with-rand-seed", 2017-07-22 --,
the feature test itself has been reworked (see the previous patch in
this series). However, it remains the case that "rand_unix.c" consumes
and honors the OPENSSL_SYS_UEFI macro.
So, let's check the "randfile.c" file. It defines three functions:
- RAND_load_file
- RAND_write_file
- RAND_file_name
Nothing inside the OpenSSL library calls them (they exist purely for
client code), and nothing in edk2 calls them either.
(2a) Therefore, we should modify the "randfile.c" source file, with an
upstream OpenSSL contribution, to hide the function definitions, when
OPENSSL_SYS_UEFI is defined. In other words, continue with Qin Long's
approach from commit fb4844bbc62f.
(2b) Alternatively, I'm noticing that "rand" is just another module
(similar to "store", see above). Assuming we really don't need RAND_*
functions for anything in edk2: have we tried configuring OpenSSL, for
the edk2 build, with the "no-rand" parameter?
Thanks,
Laszlo
>
> 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;
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v2 4/6] CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL
2019-05-09 5:23 ` [PATCH v2 4/6] CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL Xiaoyu lu
@ 2019-05-09 13:48 ` Laszlo Ersek
0 siblings, 0 replies; 27+ messages in thread
From: Laszlo Ersek @ 2019-05-09 13:48 UTC (permalink / raw)
To: devel, xiaoyux.lu; +Cc: Jian J Wang, Ting Ye
On 05/09/19 07:23, Xiaoyu lu wrote:
> 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.
>
> 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/OpensslLib.inf | 16 ++++++++++------
> CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf | 16 ++++++++++------
> 2 files changed, 20 insertions(+), 12 deletions(-)
>
> 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
>
I've only checked the GCC changes.
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v2 6/6] CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward compatible
2019-05-09 5:23 ` [PATCH v2 6/6] CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward compatible Xiaoyu lu
@ 2019-05-09 14:01 ` Laszlo Ersek
2019-05-09 14:20 ` Wang, Jian J
0 siblings, 1 reply; 27+ messages in thread
From: Laszlo Ersek @ 2019-05-09 14:01 UTC (permalink / raw)
To: devel, xiaoyux.lu; +Cc: Jian J Wang, Ting Ye
On 05/09/19 07:23, Xiaoyu lu wrote:
> 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).
In my review at <https://edk2.groups.io/g/devel/message/39837>, I *also*
asked for referencing <https://github.com/openssl/openssl/pull/4338>,
because that PULL request discussion provides the rationale for the
change. Well, whatever.
> We should not use it directly and should remove relevant
> functions(Hmac*GetContextSize).
In the same review, in bullet (2), I asked that the v2 commit message
please reference the *new* TianoCore BZ -- the one that tracks the
HMAC_xxx_CTX_SIZE / Hmac*GetContextSize() removal.
Jiaxin said in <https://edk2.groups.io/g/devel/message/39840> that he'd
file such a BZ. Do we have that BZ now? Can we please mention it in the
commit message?
>
> But for compatiblility, still updated 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..3a90e03 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.
In my review linked above, I asked for the comment to be removed. I
guess you disagreed, ultimately.
I agree that the new comment is acceptable. However, it does not conform
to the edk2 coding style. We should use the // format, for comments like
these.
Summary:
- please file the new BZ, and mention it too, in the commit message
- please clean up the comment style.
With both of those fixed, you can add
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks
Laszlo
> diff --git a/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha1.c b/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha1.c
> index bbe3df4..a8e8d44 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..fec60b1 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.
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v2 6/6] CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward compatible
2019-05-09 14:01 ` [edk2-devel] " Laszlo Ersek
@ 2019-05-09 14:20 ` Wang, Jian J
2019-05-09 21:34 ` Laszlo Ersek
0 siblings, 1 reply; 27+ messages in thread
From: Wang, Jian J @ 2019-05-09 14:20 UTC (permalink / raw)
To: Laszlo Ersek, devel@edk2.groups.io, Lu, XiaoyuX; +Cc: Ye, Ting
Laszlo,
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Thursday, May 09, 2019 10:02 PM
> To: devel@edk2.groups.io; Lu, XiaoyuX <xiaoyux.lu@intel.com>
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Ye, Ting <ting.ye@intel.com>
> Subject: Re: [edk2-devel] [PATCH v2 6/6] CryptoPkg/BaseCryptLib: Make
> HMAC_CTX size backward compatible
>
> On 05/09/19 07:23, Xiaoyu lu wrote:
> > 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).
>
> In my review at <https://edk2.groups.io/g/devel/message/39837>, I *also*
> asked for referencing <https://github.com/openssl/openssl/pull/4338>,
> because that PULL request discussion provides the rationale for the
> change. Well, whatever.
>
> > We should not use it directly and should remove relevant
> > functions(Hmac*GetContextSize).
>
> In the same review, in bullet (2), I asked that the v2 commit message
> please reference the *new* TianoCore BZ -- the one that tracks the
> HMAC_xxx_CTX_SIZE / Hmac*GetContextSize() removal.
>
> Jiaxin said in <https://edk2.groups.io/g/devel/message/39840> that he'd
> file such a BZ. Do we have that BZ now? Can we please mention it in the
> commit message?
>
My apology. I forgot to file one. Just entered one
https://bugzilla.tianocore.org/show_bug.cgi?id=1792
Regards,
Jian
> >
> > But for compatiblility, still updated 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..3a90e03 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.
>
> In my review linked above, I asked for the comment to be removed. I
> guess you disagreed, ultimately.
>
> I agree that the new comment is acceptable. However, it does not conform
> to the edk2 coding style. We should use the // format, for comments like
> these.
>
> Summary:
> - please file the new BZ, and mention it too, in the commit message
> - please clean up the comment style.
>
> With both of those fixed, you can add
>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>
> Thanks
> Laszlo
>
> > diff --git a/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha1.c
> b/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha1.c
> > index bbe3df4..a8e8d44 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..fec60b1 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.
> >
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v2 5/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b
2019-05-09 5:23 ` [PATCH v2 5/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b Xiaoyu lu
@ 2019-05-09 17:15 ` Laszlo Ersek
2019-05-09 17:30 ` Laszlo Ersek
2019-05-09 20:58 ` Laszlo Ersek
1 sibling, 1 reply; 27+ messages in thread
From: Laszlo Ersek @ 2019-05-09 17:15 UTC (permalink / raw)
To: devel, xiaoyux.lu; +Cc: Jian J Wang, Ting Ye
(please read my email until my signature)
On 05/09/19 07:23, Xiaoyu lu wrote:
> From: Xiaoyu Lu <xiaoyux.lu@intel.com>
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1089
>
> Update OpenSSL submodule to OpenSSL_1_1_1b
> OpenSSL_1_1_1b(50eaac9f3337667259de725451f201e784599687)
>
> Run process_files.pl script to regenerate OpensslLib[Crypto].inf
> and opensslconf.h
>
> Remove NO_SYSLOG from OpensslLib[Crypto].inf
> When OPENSSL_SYS_UEFI is defined, NO_SYSLOG not be defined
> in OpenSSL_1_1_0j(74f2d9c1ec5f), but in
> OpenSSL_1_1_1b(50eaac9f333), NO_SYSLOG will
> be defined(e_os.h line 47).
This is still not a *commit reference* that I asked for, in
<https://edk2.groups.io/g/devel/message/39795>, bullet (1).
At this point, I'm no longer requesting an update to this part of the
commit message. However, I will explain what you should have done,
because I would like you to learn using "git blame".
(i) Run the following command:
$ git blame OpenSSL_1_1_1b -- e_os.h
This will produce a listing that specifies the origin of each line
in "e_os.h", at OpenSSL_1_1_1b.
In other words, for each line of the file, being investigated at tag
OpenSSL_1_1_1b, the command will tell you what the most recent
commit was (not later than OpenSSL_1_1_1b), that modified that line.
In this listing, scroll to line 47. This is what we get:
45 cff55b90e95e1 (Qin Long 2017-03-15 23:33:57 +0800 45) # if defined(OPENSSL_SYS_VXWORKS) || defined(OPENSSL_SYS_UEFI)
46 3e83e686ba2e2 (Richard Levitte 2002-02-14 15:37:38 +0000 46) # define NO_CHMOD
47 3e83e686ba2e2 (Richard Levitte 2002-02-14 15:37:38 +0000 47) # define NO_SYSLOG
48 0f113f3ee4d62 (Matt Caswell 2015-01-22 03:40:55 +0000 48) # endif
You can see that NO_SYSLOG itself (line 47) comes from commit
3e83e686ba2e2. But, that commit was authored on 2002-02-14, so it's
likely not what we are after (it's too old). So let's look at the
context instead.
Line 45 looks relevant. Maybe NO_SYSLOG had already been there, and
Qin Long just modified the condition? The authorship date
(2017-03-15) also looks promising. So let's check commit
cff55b90e95e1:
(ii) Run the following command:
$ git show cff55b90e95e1
It prints:
| commit cff55b90e95e1fa6c90154f93f12363e761d88c7
| Author: Qin Long <qin.long@intel.com>
| Date: Wed Mar 15 23:33:57 2017 +0800
|
| Cleaning UEFI Build with additional OPENSSL_SYS_UEFI flags
|
| Add OPENSSL_SYS_UEFI to remove unused syslog and uid stuffs for
| more clean UEFI build.
|
| Reviewed-by: Rich Salz <rsalz@openssl.org>
| Reviewed-by: Richard Levitte <levitte@openssl.org>
| (Merged from https://github.com/openssl/openssl/pull/2961)
|
| diff --git a/e_os.h b/e_os.h
| index f255aa9c2228..241e0bac5451 100644
| --- a/e_os.h
| +++ b/e_os.h
| @@ -87,7 +87,7 @@ extern "C" {
| # define DEVRANDOM_EGD "/var/run/egd-pool","/dev/egd-pool","/etc/egd-pool","/etc/entropy"
| # endif
|
| -# if defined(OPENSSL_SYS_VXWORKS)
| +# if defined(OPENSSL_SYS_VXWORKS) || defined(OPENSSL_SYS_UEFI)
| # define NO_SYS_PARAM_H
| # define NO_CHMOD
| # define NO_SYSLOG
| [...]
Yes, this is exactly the change we're looking for.
(iii) Let's double check that this commit appeared after OpenSSL_1_1_0j.
Run the following command:
$ git tag --contains cff55b90e95e1
It prints the following list of tags:
OpenSSL_1_1_1
OpenSSL_1_1_1-pre1
OpenSSL_1_1_1-pre2
OpenSSL_1_1_1-pre3
OpenSSL_1_1_1-pre4
OpenSSL_1_1_1-pre5
OpenSSL_1_1_1-pre6
OpenSSL_1_1_1-pre7
OpenSSL_1_1_1-pre8
OpenSSL_1_1_1-pre9
OpenSSL_1_1_1a
OpenSSL_1_1_1b
We can see that tag "OpenSSL_1_1_0j" is *not* in the list. And,
knowing the structure of the OpenSSL tag names, we can also
determine the commit was first included in OpenSSL_1_1_1.
This result is good -- it confirms that the NO_SYSLOG flag should
be removed from edk2 *right now*, when we are skipping over
OpenSSL_1_1_1.
(iv) As a result of the above investigation, the commit message is
supposed to say,
Remove -DNO_SYSLOG from OPENSSL_FLAGS in the INF file, 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."
This is it -- one sentence, and it lets reviewers verify the change
very quickly.
Anyway: I'm no longer requesting that you update the commit message in
this paragraph. I just wanted to explain how "git blame" should be used.
> Add compiler_flags to buildinf.h file.
Same story as above: in <https://edk2.groups.io/g/devel/message/39795>,
bullet (4), I asked for a commit reference.
Let me spell out the steps again, in the OpenSSL tree:
$ git checkout OpenSSL_1_1_1b
$ git grep compiler_flags
This gives us "util/mkbuildinf.pl". Let's investigate the origin of the
lines in that file:
$ git blame -- util/mkbuildinf.pl
This gives us:
34 8a8d9e190533e (Rich Salz 2017-11-27 14:28:15 -0500 34) * Generate compiler_flags as an array of individual characters. This is a
35 f4a748a17d6a3 (Richard Levitte 2016-02-10 19:11:40 +0100 35) * workaround for the situation where CFLAGS gets too long for a C90 string
36 f4a748a17d6a3 (Richard Levitte 2016-02-10 19:11:40 +0100 36) * literal
37 f4a748a17d6a3 (Richard Levitte 2016-02-10 19:11:40 +0100 37) */
38 8a8d9e190533e (Rich Salz 2017-11-27 14:28:15 -0500 38) static const char compiler_flags[] = {
Okay, so let's check commit 8a8d9e190533e:
$ git show 8a8d9e190533e
$ git tag --contains 8a8d9e190533e
Yes, that's the right commit.
So, in the edk2 commit message, we should say:
Starting with OpenSSL commit 8a8d9e190533e (first released in
OpenSSL_1_1_1), the OpenSSL_version() function can no longer return a
pointer to the string literal "compiler: information not available",
in case the CFLAGS macro is not defined. Instead, the function now has
a hard dependency on the global variable 'compiler_flags'. This global
variable is normally placed by "util/mkbuildinf.pl" into "buildinf.h".
In edk2, we don't run that script whenever we build OpenSSL, therefore
we must provide our own dummy 'compiler_flags'.
But, I rest my case. :(
> From OpenSSL_1_1_0i(97c0959f27b294fe1eb10b547145ebef2524b896) to
> OpenSSL_1_1_1b(50eaac9f3337667259de725451f201e784599687), OpenSSL
> updated DRBG / RAND to request nonce and additional low entropy
> randomness from system(line 229 openssl/CHANGES).
> git diff OpenSSL_1_1_0i OpenSSL_1_1_1b crypto/include/internal/rand_int.h
> git diff OpenSSL_1_1_0i OpenSSL_1_1_1b crypto/rand/rand_unix.c
> But it is not implement for UEFI.
> Since OpenSSL_1_1_1b doesn't fully implement it. So add a new
> file(rand_pool.c) and implement it base on TimerLib.
> * 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
I'm sorry but I still disagree with this implementation.
I understand that CHANGES says "low entropy":
229 *) Updated DRBG / RAND to request nonce and additional low entropy
230 randomness from the system.
231 [Matthias St. Pierre]
But what does "low entropy" mean?
How do we know that GetPerformanceCounter() provides enough randomness?
(TimerLib is usually based on a chipset timer, and not on measuring
timings of peripherals, such as spindle disk head movement, keyboard and
mouse delays, and so on.)
In "crypto/include/internal/rand_int.h", there is a comment,
> /* |entropy_factor| expresses how many bits of data contain 1 bit of entropy */
> size_t rand_pool_bytes_needed(RAND_POOL *pool, unsigned int entropy_factor);
and we pass "1" for "entropy_factor".
How do we know that an "entropy factor" of constant 1 is correct, when:
- the randomness ultimately comes from GetPerformanceCounter() +
MicroSecondDelay(10),
- and TimerLib is platform specific?
Honestly, I have even *less* confidence in this version than in the
previous version. This code is more *obscure*, because it uses a
non-constant data source, and it uses AES-CBC-MAC for mixing it, but how
do we know it is secure enough?
I'm not a crypto expert, so I could easily be wrong about this, but just
because I cannot strongly imply that this code is wrong (like I could
imply for v1), that doesn't make it good.
How about the following:
- It seems like we cannot convince OpenSSL to *never* call these
functions, under UEFI.
- We also cannot provide an implementation that is *guaranteed* to be
secure enough, IMO.
- It seems like these functions *should* never be called in the edk2
build however, given that we're not trying to do anything "new" with
OpenSSL in edk2 -- we just want to use the new OpenSSL release for the
same old things.
- So why not just ensure that these functions *never return*?
(1) Basically implement all of the functions like this:
ASSERT (FALSE);
CpuDeadLoop ();
//
// if a return value is needed
//
return 0;
What do you think about this approach?
Continuing:
On 05/09/19 07:23, Xiaoyu lu wrote:
> We don't need ossl_store functions. So dummy implement them.
> add a new file(ossl_store.c) to implement ossl_store_cleanup_int function.
(2) If you configure OpenSSL with "no-store" -- as I suggest under v2
2/6, bullet (1) --, is the ossl_store_cleanup_int() function still
needed?
If not, then we can drop the file "ossl_store.c".
> 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.
The source file "crypto/evp/evp_key.c" has been referring to BUFSIZ
since ancient commit a63d5eaab28a (authored on 2001-05-06). In other
words, the BUFSIZ dependency is not new. What must have changed is the
definition of BUFSIZ.
In my previous review (link above), in bullet (7), I asked that you
please track down the change.
But, I guess I can try that myself. :(
$ git diff OpenSSL_1_1_0j..OpenSSL_1_1_1b -- crypto/evp/evp_key.c
Bingo; in OpenSSL_1_1_1b, the following preprocessor directives were
*removed* from around the BUFSIZ references (and more):
| -#ifndef OPENSSL_NO_UI
| -#endif /* OPENSSL_NO_UI */
When we're tracking down the removal of some lines, we can't use "git
blame", because the lines no longer exist, for "git blame" to analyze.
Therefore, we have to use:
$ git log --reverse --patch -G'OPENSSL_NO_UI' \
OpenSSL_1_1_0j..OpenSSL_1_1_1b -- crypto/evp/evp_key.c
And we immediately get:
| commit 48feaceb53fa6ae924e298b8eba0e247019313e4
| Author: Richard Levitte <levitte@openssl.org>
| Date: Sat Jul 1 12:14:37 2017 +0200
|
| Remove the possibility to disable the UI module entirely
|
| Instead, make it possible to disable the console reader that's part of
| the UI module. This makes it possible to use the UI API and other UI
| methods in environments where the console reader isn't useful.
|
| To disable the console reader, configure with 'no-ui-console' /
| 'disable-ui-console'.
|
| 'no-ui' / 'disable-ui' is now an alias for 'no-ui-console' /
| 'disable-ui-console'.
|
| Fixes #3806
|
| Reviewed-by: Rich Salz <rsalz@openssl.org>
| (Merged from https://github.com/openssl/openssl/pull/3820)
The commit message states that "no-ui" is *supposed* to automatically
disable the "console reader", by virtue of being an alias for
"no-ui-console".
However, we already have "no-ui" in our Configure invocation, and the
code still fails to compile. Therefore, this is an OpenSSL bug.
I have now filed the following upstream OpenSSL ticket:
https://github.com/openssl/openssl/issues/8904
(3) In "CryptoPkg/Library/Include/CrtLibSupport.h", please replace the
current comment ("BUFSIZ used in evp_key.c ..."), with a reference to
the above upstream OpenSSL ticket.
Please also reference this ticket in the commit message, where you
mention BUFSIZ.
> 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 | 8 +
> CryptoPkg/Library/Include/openssl/opensslconf.h | 54 ++--
> CryptoPkg/Library/OpensslLib/OpensslLib.inf | 44 +++-
> CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf | 35 ++-
> CryptoPkg/Library/OpensslLib/buildinf.h | 2 +
> CryptoPkg/Library/OpensslLib/openssl | 2 +-
> CryptoPkg/Library/OpensslLib/ossl_store.c | 17 ++
> CryptoPkg/Library/OpensslLib/rand_pool.c | 292 ++++++++++++++++++++++
> 8 files changed, 425 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..193f8de 100644
> --- a/CryptoPkg/Library/Include/CrtLibSupport.h
> +++ b/CryptoPkg/Library/Include/CrtLibSupport.h
> @@ -21,6 +21,14 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #define MAX_STRING_SIZE 0x1000
>
> //
> +// BUFSIZ used in evp_key.c
> +// 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..5e6b99e 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,6 +548,7 @@
>
> [LibraryClasses]
> DebugLib
> + TimerLib
>
> [LibraryClasses.ARM]
> ArmSoftFloatLib
(4) If you agree with my request under (1), then a TimerLib dependency
should not be added to [LibraryClasses], in either INF file.
> diff --git a/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
> index fd12d11..1362a46 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,6 +504,7 @@
>
> [LibraryClasses]
> DebugLib
> + TimerLib
>
> [LibraryClasses.ARM]
> ArmSoftFloatLib
> diff --git a/CryptoPkg/Library/OpensslLib/buildinf.h b/CryptoPkg/Library/OpensslLib/buildinf.h
> index c5ca293..5b3b50b 100644
> --- a/CryptoPkg/Library/OpensslLib/buildinf.h
> +++ b/CryptoPkg/Library/OpensslLib/buildinf.h
> @@ -1,2 +1,4 @@
> #define PLATFORM "UEFI"
> #define DATE "Fri Dec 22 01:23:45 PDT 2017"
> +
> +const char * compiler_flags = "";
(5) I suggest the following string literal here, instead:
"compiler: information not available from edk2"
Thank you,
Laszlo
> 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..c7cdeb0
> --- /dev/null
> +++ b/CryptoPkg/Library/OpensslLib/rand_pool.c
> @@ -0,0 +1,292 @@
> +/** @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>
> +
> +/**
> + 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
> +GetRandomSourceFromPerformanceCounter(
> + OUT UINT64 *Rand
> + )
> +{
> + UINT32 Index;
> + UINT32 *RandPtr;
> + RandPtr = (UINT32 *)Rand;
> +
> + if (Rand == NULL) {
> + return FALSE;
> + }
> +
> + for (Index = 0; Index < 2; Index ++) {
> + *RandPtr = (UINT32)(GetPerformanceCounter() & 0xFF);
> + MicroSecondDelay(10);
> + RandPtr++;
> + }
> +
> + return TRUE;
> +}
> +
> +/**
> + Calls GetRandomSourceFromPerformanceCounter 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 = GetRandomSourceFromPerformanceCounter (&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)
> +{
> + EFI_STATUS Status;
> + 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) {
> + Status = RandGenerateEntropy(bytes_needed, buffer);
> + if (EFI_ERROR (Status)) {
> + 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 Rand;
> + UINT64 TimerValue;
> + } data = { 0 };
> +
> + 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)
> +{
> +}
> +
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v2 3/6] CryptoPkg/IntrinsicLib: Fix possible unresolved external symbol issue
2019-05-09 5:23 ` [PATCH v2 3/6] CryptoPkg/IntrinsicLib: Fix possible unresolved external symbol issue Xiaoyu lu
@ 2019-05-09 17:16 ` Laszlo Ersek
0 siblings, 0 replies; 27+ messages in thread
From: Laszlo Ersek @ 2019-05-09 17:16 UTC (permalink / raw)
To: devel, xiaoyux.lu; +Cc: Jian J Wang, Ting Ye
On 05/09/19 07:23, Xiaoyu lu wrote:
> 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
>
Thank you for splitting this out; this is a nice improvement for the
structure of the patch series.
Thanks
Laszlo
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v2 5/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b
2019-05-09 17:15 ` [edk2-devel] " Laszlo Ersek
@ 2019-05-09 17:30 ` Laszlo Ersek
2019-05-10 10:26 ` Wang, Jian J
0 siblings, 1 reply; 27+ messages in thread
From: Laszlo Ersek @ 2019-05-09 17:30 UTC (permalink / raw)
To: devel, xiaoyux.lu; +Cc: Jian J Wang, Ting Ye
On 05/09/19 19:15, Laszlo Ersek wrote:
> How about the following:
>
> - It seems like we cannot convince OpenSSL to *never* call these
> functions, under UEFI.
>
> - We also cannot provide an implementation that is *guaranteed* to be
> secure enough, IMO.
>
> - It seems like these functions *should* never be called in the edk2
> build however, given that we're not trying to do anything "new" with
> OpenSSL in edk2 -- we just want to use the new OpenSSL release for the
> same old things.
>
> - So why not just ensure that these functions *never return*?
>
> (1) Basically implement all of the functions like this:
>
> ASSERT (FALSE);
> CpuDeadLoop ();
> //
> // if a return value is needed
> //
> return 0;
>
> What do you think about this approach?
I notice that "rand" is another module in OpenSSL.
Can we try adding "no-rand" to our Configure invocation? Perhaps the
need for all of the rand_* functions goes away then.
Thanks
Laszlo
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v2 5/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b
2019-05-09 5:23 ` [PATCH v2 5/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b Xiaoyu lu
2019-05-09 17:15 ` [edk2-devel] " Laszlo Ersek
@ 2019-05-09 20:58 ` Laszlo Ersek
2019-05-10 8:51 ` Xiaoyu lu
1 sibling, 1 reply; 27+ messages in thread
From: Laszlo Ersek @ 2019-05-09 20:58 UTC (permalink / raw)
To: devel, xiaoyux.lu; +Cc: Jian J Wang, Ting Ye
Hi Xiaoyu,
On 05/09/19 07:23, Xiaoyu lu wrote:
> From: Xiaoyu Lu <xiaoyux.lu@intel.com>
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1089
>
> Update OpenSSL submodule to OpenSSL_1_1_1b
> OpenSSL_1_1_1b(50eaac9f3337667259de725451f201e784599687)
I found another issue, while trying to cross-build this series for
AARCH64.
I ran the commands below:
> export GCC5_AARCH64_PREFIX=aarch64-linux-gnu-
> build \
> -a AARCH64 \
> -b NOOPT \
> -p CryptoPkg/CryptoPkg.dsc \
> -t GCC5 \
> --cmd-len=65536 \
> -m CryptoPkg/Library/OpensslLib/OpensslLib.inf
The following cross-compilation command failed:
> "aarch64-linux-gnu-gcc" \
> -g \
> -fshort-wchar \
> -fno-builtin \
> -fno-strict-aliasing \
> -Wall \
> -Werror \
> -Wno-array-bounds \
> -ffunction-sections \
> -fdata-sections \
> -include AutoGen.h \
> -fno-common \
> -DSTRING_ARRAY_NAME=OpensslLibStrings \
> -g \
> -Os \
> -fshort-wchar \
> -fno-builtin \
> -fno-strict-aliasing \
> -Wall \
> -Werror \
> -Wno-array-bounds \
> -include AutoGen.h \
> -fno-common \
> -mlittle-endian \
> -fno-short-enums \
> -fverbose-asm \
> -funsigned-char \
> -ffunction-sections \
> -fdata-sections \
> -Wno-address \
> -fno-asynchronous-unwind-tables \
> -fno-unwind-tables \
> -fno-pic \
> -fno-pie \
> -ffixed-x18 \
> -mcmodel=small \
> -O0 \
> -DL_ENDIAN \
> -DOPENSSL_SMALL_FOOTPRINT \
> -D_CRT_SECURE_NO_DEPRECATE \
> -D_CRT_NONSTDC_NO_DEPRECATE \
> -Wno-error=maybe-uninitialized \
> -Wno-format \
> -Wno-error=unused-but-set-variable \
> -D DISABLE_NEW_DEPRECATED_INTERFACES \
> -c \
> -o $WORKSPACE/Build/CryptoPkg/NOOPT_GCC5/AARCH64/CryptoPkg/Library/OpensslLib/OpensslLib/OUTPUT/openssl/crypto/rand/rand_unix.obj \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/ssl/statem \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/ssl/record \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/ssl \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/x509v3 \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/x509 \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/ui \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/txt_db \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/stack \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/sm4 \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/sm3 \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/siphash \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/sha \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/rsa \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/rc4 \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/rand \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/pkcs7 \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/pkcs12 \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/pem \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/ocsp \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/objects \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/modes \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/md5 \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/md4 \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/lhash \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/kdf \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/hmac \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/evp \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/err \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/dso \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/dh \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/des \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/conf \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/comp \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/cmac \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/buffer \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/bn \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/bio \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/async \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/async/arch \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/asn1 \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/aria \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/aes \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib \
> -I$WORKSPACE/Build/CryptoPkg/NOOPT_GCC5/AARCH64/CryptoPkg/Library/OpensslLib/OpensslLib/DEBUG \
> -I$WORKSPACE/MdePkg \
> -I$WORKSPACE/MdePkg/Include \
> -I$WORKSPACE/MdePkg/Include/AArch64 \
> -I$WORKSPACE/CryptoPkg \
> -I$WORKSPACE/CryptoPkg/Include \
> -I$WORKSPACE/CryptoPkg/Library/Include \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/include \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/include \
> $WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/rand/rand_unix.c
The error message was:
> $WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/rand/rand_unix.c:22:26:
> fatal error: sys/syscall.h: No such file or directory
> # include <sys/syscall.h>
> ^
> compilation terminated.
The "rand_unix.c" source file contains:
21 #if defined(__linux)
22 # include <sys/syscall.h>
23 #endif
This code originates from OpenSSL commit 148796291e47 ("Add support for
getrandom() or equivalent system calls and use them by default",
2018-04-22).
This is a problem because the aarch64 cross-compiler in Fedora only
supports "freestanding" programs (such as the Linux kernel, and edk2);
it does not support userspace (hosted) programs. The cross-compiler's
description says,
> Cross-build GNU C compiler.
>
> Only building kernels is currently supported. Support for cross-building
> user space programs is not currently provided as that would massively multiply
> the number of packages.
(This is the case as of
gcc-aarch64-linux-gnu-8.2.1-1.fc30.2.aarch64.rpm, from
<https://koji.fedoraproject.org/koji/buildinfo?buildID=1185346>.)
And, <sys/syscall.h> is a header that only userspace programs may
include.
Now, I see that we already have the following files in CryptoPkg:
CryptoPkg/Library/Include/sys/types.h
CryptoPkg/Library/Include/sys/time.h
The following patch allows the build to complete:
> diff --git a/CryptoPkg/Library/Include/sys/syscall.h b/CryptoPkg/Library/Include/sys/syscall.h
> new file mode 100644
> index 000000000000..bfe1c7ff1473
> --- /dev/null
> +++ b/CryptoPkg/Library/Include/sys/syscall.h
> @@ -0,0 +1,10 @@
> +/** @file
> + Include file to support building the third-party cryptographic library.
> +
> +Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2019, Red Hat, Inc.
> +SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <CrtLibSupport.h>
This file is sufficient for the following reason. In "rand_unix.c", at
tag OpenSSL_1_1_1b, we have:
80 #if defined(OPENSSL_RAND_SEED_NONE)
81 /* none means none. this simplifies the following logic */
82 # undef OPENSSL_RAND_SEED_OS
83 # undef OPENSSL_RAND_SEED_GETRANDOM
84 # undef OPENSSL_RAND_SEED_LIBRANDOM
85 # undef OPENSSL_RAND_SEED_DEVRANDOM
86 # undef OPENSSL_RAND_SEED_RDTSC
87 # undef OPENSSL_RAND_SEED_RDCPU
88 # undef OPENSSL_RAND_SEED_EGD
89 #endif
Due to your patch v2 1/6, the macro OPENSSL_RAND_SEED_NONE will be
defined, as a consequence of "--with-rand-seed=none".
And the following "naked" Linux syscall in "rand_unix.c":
326 /* Linux supports this since version 3.17 */
327 # if defined(__linux) && defined(SYS_getrandom)
328 return syscall(SYS_getrandom, buf, buflen, 0);
is located in the function syscall_random() -- which entirely depends on
OPENSSL_RAND_SEED_GETRANDOM.
In other words, due to "--with-rand-seed=none" from patch v2 1/6, the
actual contents of "sys/syscall.h" will never be necessary. We just need
to provide a placeholder header file.
So please include a patch in the v3 series that adds
"CryptoPkg/Library/Include/sys/syscall.h" like suggested above.
Thanks
Laszlo
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v2 6/6] CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward compatible
2019-05-09 14:20 ` Wang, Jian J
@ 2019-05-09 21:34 ` Laszlo Ersek
0 siblings, 0 replies; 27+ messages in thread
From: Laszlo Ersek @ 2019-05-09 21:34 UTC (permalink / raw)
To: Wang, Jian J, devel@edk2.groups.io, Lu, XiaoyuX; +Cc: Ye, Ting
On 05/09/19 16:20, Wang, Jian J wrote:
> Laszlo,
>
>
>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Thursday, May 09, 2019 10:02 PM
>> To: devel@edk2.groups.io; Lu, XiaoyuX <xiaoyux.lu@intel.com>
>> Cc: Wang, Jian J <jian.j.wang@intel.com>; Ye, Ting <ting.ye@intel.com>
>> Subject: Re: [edk2-devel] [PATCH v2 6/6] CryptoPkg/BaseCryptLib: Make
>> HMAC_CTX size backward compatible
>>
>> On 05/09/19 07:23, Xiaoyu lu wrote:
>>> 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).
>>
>> In my review at <https://edk2.groups.io/g/devel/message/39837>, I *also*
>> asked for referencing <https://github.com/openssl/openssl/pull/4338>,
>> because that PULL request discussion provides the rationale for the
>> change. Well, whatever.
>>
>>> We should not use it directly and should remove relevant
>>> functions(Hmac*GetContextSize).
>>
>> In the same review, in bullet (2), I asked that the v2 commit message
>> please reference the *new* TianoCore BZ -- the one that tracks the
>> HMAC_xxx_CTX_SIZE / Hmac*GetContextSize() removal.
>>
>> Jiaxin said in <https://edk2.groups.io/g/devel/message/39840> that he'd
>> file such a BZ. Do we have that BZ now? Can we please mention it in the
>> commit message?
>>
>
> My apology. I forgot to file one. Just entered one
>
> https://bugzilla.tianocore.org/show_bug.cgi?id=1792
Much appreciated!
Laszlo
>
> Regards,
> Jian
>
>>>
>>> But for compatiblility, still updated 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..3a90e03 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.
>>
>> In my review linked above, I asked for the comment to be removed. I
>> guess you disagreed, ultimately.
>>
>> I agree that the new comment is acceptable. However, it does not conform
>> to the edk2 coding style. We should use the // format, for comments like
>> these.
>>
>> Summary:
>> - please file the new BZ, and mention it too, in the commit message
>> - please clean up the comment style.
>>
>> With both of those fixed, you can add
>>
>> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>>
>> Thanks
>> Laszlo
>>
>>> diff --git a/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha1.c
>> b/CryptoPkg/Library/BaseCryptLib/Hmac/CryptHmacSha1.c
>>> index bbe3df4..a8e8d44 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..fec60b1 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.
>>>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v2 2/6] CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl
2019-05-09 13:42 ` [edk2-devel] " Laszlo Ersek
@ 2019-05-10 8:51 ` Xiaoyu lu
2019-05-13 15:12 ` Laszlo Ersek
0 siblings, 1 reply; 27+ messages in thread
From: Xiaoyu lu @ 2019-05-10 8:51 UTC (permalink / raw)
To: Laszlo Ersek, devel@edk2.groups.io; +Cc: Wang, Jian J, Ye, Ting
Hi, Laszlo:
Thank you for your time.
I try the method you mentioned.
> (1) Therefore, the right thing to do here is to add "no-store" to the above list, in my opinion. Can you try that, please?
>
> And, this change should be a standalone patch, similarly to patch v2 1/6 in this series.
(1) OpenSSL configure script don't support no-store option.
It will lead to configure error.
Unsupported options: no-store
> (2a) Therefore, we should modify the "randfile.c" source file, with an upstream OpenSSL contribution, to hide the function definitions, when OPENSSL_SYS_UEFI is defined. In other words, continue with Qin Long's approach from commit fb4844bbc62f.
I think this is the best way. But the openssl community takes time to accept the patch.
I just let OpenSSL work for UEFI. So UEFI can use the new algorithm in OpenSSL_1_1_1.
I am willing to continue to modify this later.
> (2b) Alternatively, I'm noticing that "rand" is just another module (similar to "store", see above). Assuming we really don't need RAND_* functions for anything in edk2: have we tried configuring OpenSSL, for the edk2 build, with the "no-rand" parameter?
(2) I'm afraid not. Same as (1)
***** Unsupported options: no-rand
Thanks,
Xiaoyu.
-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com]
Sent: Thursday, May 9, 2019 9:43 PM
To: devel@edk2.groups.io; Lu, XiaoyuX <xiaoyux.lu@intel.com>
Cc: Wang, Jian J <jian.j.wang@intel.com>; Ye, Ting <ting.ye@intel.com>
Subject: Re: [edk2-devel] [PATCH v2 2/6] CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl
On 05/09/19 07:23, Xiaoyu lu wrote:
> 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.
Indeed.
> OpenSSL_1_1_1(1708e3e85b4a8) add a STORE module (crypto/store/*).
This statement is incorrect (or, minimally, inexact). According to the following command:
$ git log --oneline --reverse OpenSSL_1_1_1b -- crypto/store/ \
| head -n 1
the first OpenSSL commit that added files to crypto/store/ was:
> commit a5db6fa5760f21d16d59e025e930c02456e00fef
> Author: Richard Levitte <levitte@openssl.org>
> Date: Thu May 1 03:53:12 2003 +0000
>
> Define a STORE type. For documentation, read the entry in CHANGES,
> crypto/store/README, crypto/store/store.h and crypto/store/str_locl.h.
This commit goes back to 2003, and is part of releae OpenSSL_0_9_7d.
Instead, let's check what the following command reports:
$ git log --oneline --reverse \
OpenSSL_1_1_0j..OpenSSL_1_1_1b -- crypto/store/ \
| head -1
It states that the first commit after OpenSSL_1_1_0j, but not after OpenSSL_1_1_1b, to modify the "crypto/store/" subdirectory, was commit 71a5516dcc8a ("Add the STORE module", 2017-06-29).
If we investigate that commit:
$ git show --stat 71a5516dcc8a
we see that the commit modifies the Configure script:
> Configure | 2 +-
So let's check that part of the diff in detail:
$ git show 71a5516dcc8a -- Configure
And we get:
> diff --git a/Configure b/Configure
> index 2eacb2312e34..e302a58abb71 100755
> --- a/Configure
> +++ b/Configure
> @@ -310,7 +310,7 @@ $config{sdirs} = [
> "bn", "ec", "rsa", "dsa", "dh", "dso", "engine",
> "buffer", "bio", "stack", "lhash", "rand", "err",
> "evp", "asn1", "pem", "x509", "x509v3", "conf", "txt_db", "pkcs7",
> "pkcs12", "comp", "ocsp", "ui",
> - "cms", "ts", "srp", "cmac", "ct", "async", "kdf"
> + "cms", "ts", "srp", "cmac", "ct", "async", "kdf", "store"
> ];
> # test/ subdirectories to build
> $config{tdirs} = [ "ossl_shim" ];
We can see that the "store" module is added after modules such as "cms", "ts", "srp", and so on.
Now, if you look at "CryptoPkg/Library/OpensslLib/process_files.pl", you find (with edk2 master being at commit 49693202ec9c):
49 "./Configure",
50 "UEFI",
51 "no-afalgeng",
52 "no-asm",
53 "no-async", <---- disables "async"
54 "no-autoalginit",
55 "no-autoerrinit",
56 "no-bf",
57 "no-blake2",
58 "no-camellia",
59 "no-capieng",
60 "no-cast",
61 "no-chacha",
62 "no-cms", <---- disables "cms"
63 "no-ct", <---- disables "ct"
64 "no-deprecated",
65 "no-dgram",
66 "no-dsa",
67 "no-dynamic-engine",
68 "no-ec",
69 "no-ec2m",
70 "no-engine",
71 "no-err",
72 "no-filenames",
73 "no-gost",
74 "no-hw",
75 "no-idea",
76 "no-mdc2",
77 "no-pic",
78 "no-ocb",
79 "no-poly1305",
80 "no-posix-io",
81 "no-rc2",
82 "no-rfc3779",
83 "no-rmd160",
84 "no-scrypt",
85 "no-seed",
86 "no-sock",
87 "no-srp", <---- disables "srp"
88 "no-ssl",
89 "no-stdio",
90 "no-threads",
91 "no-ts", <---- disables "ts"
92 "no-ui",
93 "no-whirlpool"
(1) Therefore, the right thing to do here is to add "no-store" to the above list, in my opinion. Can you try that, please?
And, this change should be a standalone patch, similarly to patch v2 1/6 in this series.
> But UEFI don't use them. So exclude these files.
> This file, crypto/rand/randfile.c, have been modified between
> OpenSSL_1_1_0j(74f2d9c1ec5f5) and OpenSSL_1_1_1b(50eaac9f33376672).
> It requires more crt runtime support. But UEFI don't use it.
> So exclude the file.
I think I disagree with this approach.
In OpenSSL commit fb4844bbc62f -- "Add UEFI flag for rand build", 2015-09-03, part of OpenSSL_1_1_0 --, Qin Long customized "crypto/rand/rand_unix.c". So that, when OPENSSL_SYS_UEFI was #defined, the real RAND_poll() function was replaced by a stub that would always report failure. (So this was a safe stub.)
In OpenSSL commit 8389ec4b4950 -- "Add --with-rand-seed", 2017-07-22 --, the feature test itself has been reworked (see the previous patch in this series). However, it remains the case that "rand_unix.c" consumes and honors the OPENSSL_SYS_UEFI macro.
So, let's check the "randfile.c" file. It defines three functions:
- RAND_load_file
- RAND_write_file
- RAND_file_name
Nothing inside the OpenSSL library calls them (they exist purely for client code), and nothing in edk2 calls them either.
(2a) Therefore, we should modify the "randfile.c" source file, with an upstream OpenSSL contribution, to hide the function definitions, when OPENSSL_SYS_UEFI is defined. In other words, continue with Qin Long's approach from commit fb4844bbc62f.
(2b) Alternatively, I'm noticing that "rand" is just another module (similar to "store", see above). Assuming we really don't need RAND_* functions for anything in edk2: have we tried configuring OpenSSL, for the edk2 build, with the "no-rand" parameter?
Thanks,
Laszlo
>
> 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;
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v2 5/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b
2019-05-09 20:58 ` Laszlo Ersek
@ 2019-05-10 8:51 ` Xiaoyu lu
0 siblings, 0 replies; 27+ messages in thread
From: Xiaoyu lu @ 2019-05-10 8:51 UTC (permalink / raw)
To: devel@edk2.groups.io, lersek@redhat.com; +Cc: Wang, Jian J, Ye, Ting
Thank you. Lersek.
This is a big mistake. I haven't test it.
-----Original Message-----
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Laszlo Ersek
Sent: Friday, May 10, 2019 4:58 AM
To: devel@edk2.groups.io; Lu, XiaoyuX <xiaoyux.lu@intel.com>
Cc: Wang, Jian J <jian.j.wang@intel.com>; Ye, Ting <ting.ye@intel.com>
Subject: Re: [edk2-devel] [PATCH v2 5/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b
Hi Xiaoyu,
On 05/09/19 07:23, Xiaoyu lu wrote:
> From: Xiaoyu Lu <xiaoyux.lu@intel.com>
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1089
>
> Update OpenSSL submodule to OpenSSL_1_1_1b
> OpenSSL_1_1_1b(50eaac9f3337667259de725451f201e784599687)
I found another issue, while trying to cross-build this series for AARCH64.
I ran the commands below:
> export GCC5_AARCH64_PREFIX=aarch64-linux-gnu-
> build \
> -a AARCH64 \
> -b NOOPT \
> -p CryptoPkg/CryptoPkg.dsc \
> -t GCC5 \
> --cmd-len=65536 \
> -m CryptoPkg/Library/OpensslLib/OpensslLib.inf
The following cross-compilation command failed:
> "aarch64-linux-gnu-gcc" \
> -g \
> -fshort-wchar \
> -fno-builtin \
> -fno-strict-aliasing \
> -Wall \
> -Werror \
> -Wno-array-bounds \
> -ffunction-sections \
> -fdata-sections \
> -include AutoGen.h \
> -fno-common \
> -DSTRING_ARRAY_NAME=OpensslLibStrings \
> -g \
> -Os \
> -fshort-wchar \
> -fno-builtin \
> -fno-strict-aliasing \
> -Wall \
> -Werror \
> -Wno-array-bounds \
> -include AutoGen.h \
> -fno-common \
> -mlittle-endian \
> -fno-short-enums \
> -fverbose-asm \
> -funsigned-char \
> -ffunction-sections \
> -fdata-sections \
> -Wno-address \
> -fno-asynchronous-unwind-tables \
> -fno-unwind-tables \
> -fno-pic \
> -fno-pie \
> -ffixed-x18 \
> -mcmodel=small \
> -O0 \
> -DL_ENDIAN \
> -DOPENSSL_SMALL_FOOTPRINT \
> -D_CRT_SECURE_NO_DEPRECATE \
> -D_CRT_NONSTDC_NO_DEPRECATE \
> -Wno-error=maybe-uninitialized \
> -Wno-format \
> -Wno-error=unused-but-set-variable \
> -D DISABLE_NEW_DEPRECATED_INTERFACES \
> -c \
> -o $WORKSPACE/Build/CryptoPkg/NOOPT_GCC5/AARCH64/CryptoPkg/Library/OpensslLib/OpensslLib/OUTPUT/openssl/crypto/rand/rand_unix.obj \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/ssl/statem \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/ssl/record \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/ssl \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/x509v3 \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/x509 \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/ui \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/txt_db \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/stack \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/sm4 \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/sm3 \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/siphash \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/sha \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/rsa \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/rc4 \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/rand \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/pkcs7 \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/pkcs12 \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/pem \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/ocsp \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/objects \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/modes \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/md5 \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/md4 \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/lhash \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/kdf \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/hmac \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/evp \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/err \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/dso \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/dh \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/des \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/conf \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/comp \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/cmac \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/buffer \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/bn \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/bio \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/async \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/async/arch \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/asn1 \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/aria \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/aes \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib \
> -I$WORKSPACE/Build/CryptoPkg/NOOPT_GCC5/AARCH64/CryptoPkg/Library/OpensslLib/OpensslLib/DEBUG \
> -I$WORKSPACE/MdePkg \
> -I$WORKSPACE/MdePkg/Include \
> -I$WORKSPACE/MdePkg/Include/AArch64 \
> -I$WORKSPACE/CryptoPkg \
> -I$WORKSPACE/CryptoPkg/Include \
> -I$WORKSPACE/CryptoPkg/Library/Include \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/include \
> -I$WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/include \
>
> $WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/rand/rand_unix.
> c
The error message was:
> $WORKSPACE/CryptoPkg/Library/OpensslLib/openssl/crypto/rand/rand_unix.c:22:26:
> fatal error: sys/syscall.h: No such file or directory # include
> <sys/syscall.h>
> ^
> compilation terminated.
The "rand_unix.c" source file contains:
21 #if defined(__linux)
22 # include <sys/syscall.h>
23 #endif
This code originates from OpenSSL commit 148796291e47 ("Add support for
getrandom() or equivalent system calls and use them by default", 2018-04-22).
This is a problem because the aarch64 cross-compiler in Fedora only supports "freestanding" programs (such as the Linux kernel, and edk2); it does not support userspace (hosted) programs. The cross-compiler's description says,
> Cross-build GNU C compiler.
>
> Only building kernels is currently supported. Support for
> cross-building user space programs is not currently provided as that
> would massively multiply the number of packages.
(This is the case as of
gcc-aarch64-linux-gnu-8.2.1-1.fc30.2.aarch64.rpm, from
<https://koji.fedoraproject.org/koji/buildinfo?buildID=1185346>.)
And, <sys/syscall.h> is a header that only userspace programs may include.
Now, I see that we already have the following files in CryptoPkg:
CryptoPkg/Library/Include/sys/types.h
CryptoPkg/Library/Include/sys/time.h
The following patch allows the build to complete:
> diff --git a/CryptoPkg/Library/Include/sys/syscall.h
> b/CryptoPkg/Library/Include/sys/syscall.h
> new file mode 100644
> index 000000000000..bfe1c7ff1473
> --- /dev/null
> +++ b/CryptoPkg/Library/Include/sys/syscall.h
> @@ -0,0 +1,10 @@
> +/** @file
> + Include file to support building the third-party cryptographic library.
> +
> +Copyright (c) 2010 - 2017, Intel Corporation. All rights
> +reserved.<BR> Copyright (c) 2019, Red Hat, Inc.
> +SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <CrtLibSupport.h>
This file is sufficient for the following reason. In "rand_unix.c", at tag OpenSSL_1_1_1b, we have:
80 #if defined(OPENSSL_RAND_SEED_NONE)
81 /* none means none. this simplifies the following logic */
82 # undef OPENSSL_RAND_SEED_OS
83 # undef OPENSSL_RAND_SEED_GETRANDOM
84 # undef OPENSSL_RAND_SEED_LIBRANDOM
85 # undef OPENSSL_RAND_SEED_DEVRANDOM
86 # undef OPENSSL_RAND_SEED_RDTSC
87 # undef OPENSSL_RAND_SEED_RDCPU
88 # undef OPENSSL_RAND_SEED_EGD
89 #endif
Due to your patch v2 1/6, the macro OPENSSL_RAND_SEED_NONE will be defined, as a consequence of "--with-rand-seed=none".
And the following "naked" Linux syscall in "rand_unix.c":
326 /* Linux supports this since version 3.17 */
327 # if defined(__linux) && defined(SYS_getrandom)
328 return syscall(SYS_getrandom, buf, buflen, 0);
is located in the function syscall_random() -- which entirely depends on OPENSSL_RAND_SEED_GETRANDOM.
In other words, due to "--with-rand-seed=none" from patch v2 1/6, the actual contents of "sys/syscall.h" will never be necessary. We just need to provide a placeholder header file.
So please include a patch in the v3 series that adds "CryptoPkg/Library/Include/sys/syscall.h" like suggested above.
Thanks
Laszlo
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v2 5/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b
2019-05-09 17:30 ` Laszlo Ersek
@ 2019-05-10 10:26 ` Wang, Jian J
2019-05-13 16:14 ` Laszlo Ersek
0 siblings, 1 reply; 27+ messages in thread
From: Wang, Jian J @ 2019-05-10 10:26 UTC (permalink / raw)
To: devel@edk2.groups.io, lersek@redhat.com, Lu, XiaoyuX; +Cc: Ye, Ting
Hi Laszlo,
rand_* is needed by openssl itself. BaseCryptLib also provide RandomSeed()
and RandomBytes() interface to wrap openssl rand functionality. We can't
just drop them. From platform independent perspective, using performance
counter is the best choice we have. If we want to achieve the uttermost
secure mechanism, only hardware seed/rand generator can meet it. Do you
agree to add cpu specific instruction to do that? For those processors
which don't support seed/rand generation, we have to fall back to performance
counter.
Another option is that we could make use of EFI_RNG_PROTOCOL. According
to UEFI spec (see below), it can be used to get entropy.
"This protocol is used to provide random numbers for use in applications,
or entropy for seeding other random number generators."
Again, we could use performance counter as a fall back if EFI_RNG_PROTOCOL
is not provided by a platform. So if a platform really care about the security,
it should provide a EFI_RNG_PROTOCOL. This can also help to hide the
hardware/platform dependency.
Regards,
Jian
> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Laszlo Ersek
> Sent: Friday, May 10, 2019 1:30 AM
> To: devel@edk2.groups.io; Lu, XiaoyuX <xiaoyux.lu@intel.com>
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Ye, Ting <ting.ye@intel.com>
> Subject: Re: [edk2-devel] [PATCH v2 5/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b
>
> On 05/09/19 19:15, Laszlo Ersek wrote:
>
> > How about the following:
> >
> > - It seems like we cannot convince OpenSSL to *never* call these
> > functions, under UEFI.
> >
> > - We also cannot provide an implementation that is *guaranteed* to be
> > secure enough, IMO.
> >
> > - It seems like these functions *should* never be called in the edk2
> > build however, given that we're not trying to do anything "new" with
> > OpenSSL in edk2 -- we just want to use the new OpenSSL release for the
> > same old things.
> >
> > - So why not just ensure that these functions *never return*?
> >
> > (1) Basically implement all of the functions like this:
> >
> > ASSERT (FALSE);
> > CpuDeadLoop ();
> > //
> > // if a return value is needed
> > //
> > return 0;
> >
> > What do you think about this approach?
>
> I notice that "rand" is another module in OpenSSL.
>
> Can we try adding "no-rand" to our Configure invocation? Perhaps the
> need for all of the rand_* functions goes away then.
>
> Thanks
> Laszlo
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v2 2/6] CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl
2019-05-10 8:51 ` Xiaoyu lu
@ 2019-05-13 15:12 ` Laszlo Ersek
2019-05-14 12:41 ` Xiaoyu lu
0 siblings, 1 reply; 27+ messages in thread
From: Laszlo Ersek @ 2019-05-13 15:12 UTC (permalink / raw)
To: devel, xiaoyux.lu; +Cc: Wang, Jian J, Ye, Ting
On 05/10/19 10:51, Xiaoyu lu wrote:
> Hi, Laszlo:
>
> Thank you for your time.
>
> I try the method you mentioned.
>
>> (1) Therefore, the right thing to do here is to add "no-store" to the above list, in my opinion. Can you try that, please?
>>
>> And, this change should be a standalone patch, similarly to patch v2 1/6 in this series.
>
> (1) OpenSSL configure script don't support no-store option.
> It will lead to configure error.
>
> Unsupported options: no-store
>
>> (2a) Therefore, we should modify the "randfile.c" source file, with an upstream OpenSSL contribution, to hide the function definitions, when OPENSSL_SYS_UEFI is defined. In other words, continue with Qin Long's approach from commit fb4844bbc62f.
>
> I think this is the best way. But the openssl community takes time to accept the patch.
> I just let OpenSSL work for UEFI. So UEFI can use the new algorithm in OpenSSL_1_1_1.
> I am willing to continue to modify this later.
Please pick one of two:
- file a new TianoCore BZ about cleaning up this technical debt, and
paste the BZ URL into the code, as a comment
- delay TianoCore BZ#1089 to the next edk2-stable release, and work with
upstream OpenSSL to compile out parts of "randfile.c".
Thanks
Laszlo
>
>> (2b) Alternatively, I'm noticing that "rand" is just another module (similar to "store", see above). Assuming we really don't need RAND_* functions for anything in edk2: have we tried configuring OpenSSL, for the edk2 build, with the "no-rand" parameter?
>
> (2) I'm afraid not. Same as (1)
>
> ***** Unsupported options: no-rand
>
> Thanks,
> Xiaoyu.
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Thursday, May 9, 2019 9:43 PM
> To: devel@edk2.groups.io; Lu, XiaoyuX <xiaoyux.lu@intel.com>
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Ye, Ting <ting.ye@intel.com>
> Subject: Re: [edk2-devel] [PATCH v2 2/6] CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl
>
> On 05/09/19 07:23, Xiaoyu lu wrote:
>> 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.
>
> Indeed.
>
>> OpenSSL_1_1_1(1708e3e85b4a8) add a STORE module (crypto/store/*).
>
> This statement is incorrect (or, minimally, inexact). According to the following command:
>
> $ git log --oneline --reverse OpenSSL_1_1_1b -- crypto/store/ \
> | head -n 1
>
> the first OpenSSL commit that added files to crypto/store/ was:
>
>> commit a5db6fa5760f21d16d59e025e930c02456e00fef
>> Author: Richard Levitte <levitte@openssl.org>
>> Date: Thu May 1 03:53:12 2003 +0000
>>
>> Define a STORE type. For documentation, read the entry in CHANGES,
>> crypto/store/README, crypto/store/store.h and crypto/store/str_locl.h.
>
> This commit goes back to 2003, and is part of releae OpenSSL_0_9_7d.
>
> Instead, let's check what the following command reports:
>
> $ git log --oneline --reverse \
> OpenSSL_1_1_0j..OpenSSL_1_1_1b -- crypto/store/ \
> | head -1
>
> It states that the first commit after OpenSSL_1_1_0j, but not after OpenSSL_1_1_1b, to modify the "crypto/store/" subdirectory, was commit 71a5516dcc8a ("Add the STORE module", 2017-06-29).
>
> If we investigate that commit:
>
> $ git show --stat 71a5516dcc8a
>
> we see that the commit modifies the Configure script:
>
>> Configure | 2 +-
>
> So let's check that part of the diff in detail:
>
> $ git show 71a5516dcc8a -- Configure
>
> And we get:
>
>> diff --git a/Configure b/Configure
>> index 2eacb2312e34..e302a58abb71 100755
>> --- a/Configure
>> +++ b/Configure
>> @@ -310,7 +310,7 @@ $config{sdirs} = [
>> "bn", "ec", "rsa", "dsa", "dh", "dso", "engine",
>> "buffer", "bio", "stack", "lhash", "rand", "err",
>> "evp", "asn1", "pem", "x509", "x509v3", "conf", "txt_db", "pkcs7",
>> "pkcs12", "comp", "ocsp", "ui",
>> - "cms", "ts", "srp", "cmac", "ct", "async", "kdf"
>> + "cms", "ts", "srp", "cmac", "ct", "async", "kdf", "store"
>> ];
>> # test/ subdirectories to build
>> $config{tdirs} = [ "ossl_shim" ];
>
> We can see that the "store" module is added after modules such as "cms", "ts", "srp", and so on.
>
> Now, if you look at "CryptoPkg/Library/OpensslLib/process_files.pl", you find (with edk2 master being at commit 49693202ec9c):
>
> 49 "./Configure",
> 50 "UEFI",
> 51 "no-afalgeng",
> 52 "no-asm",
> 53 "no-async", <---- disables "async"
> 54 "no-autoalginit",
> 55 "no-autoerrinit",
> 56 "no-bf",
> 57 "no-blake2",
> 58 "no-camellia",
> 59 "no-capieng",
> 60 "no-cast",
> 61 "no-chacha",
> 62 "no-cms", <---- disables "cms"
> 63 "no-ct", <---- disables "ct"
> 64 "no-deprecated",
> 65 "no-dgram",
> 66 "no-dsa",
> 67 "no-dynamic-engine",
> 68 "no-ec",
> 69 "no-ec2m",
> 70 "no-engine",
> 71 "no-err",
> 72 "no-filenames",
> 73 "no-gost",
> 74 "no-hw",
> 75 "no-idea",
> 76 "no-mdc2",
> 77 "no-pic",
> 78 "no-ocb",
> 79 "no-poly1305",
> 80 "no-posix-io",
> 81 "no-rc2",
> 82 "no-rfc3779",
> 83 "no-rmd160",
> 84 "no-scrypt",
> 85 "no-seed",
> 86 "no-sock",
> 87 "no-srp", <---- disables "srp"
> 88 "no-ssl",
> 89 "no-stdio",
> 90 "no-threads",
> 91 "no-ts", <---- disables "ts"
> 92 "no-ui",
> 93 "no-whirlpool"
>
> (1) Therefore, the right thing to do here is to add "no-store" to the above list, in my opinion. Can you try that, please?
>
> And, this change should be a standalone patch, similarly to patch v2 1/6 in this series.
>
>> But UEFI don't use them. So exclude these files.
>
>> This file, crypto/rand/randfile.c, have been modified between
>> OpenSSL_1_1_0j(74f2d9c1ec5f5) and OpenSSL_1_1_1b(50eaac9f33376672).
>> It requires more crt runtime support. But UEFI don't use it.
>> So exclude the file.
>
> I think I disagree with this approach.
>
> In OpenSSL commit fb4844bbc62f -- "Add UEFI flag for rand build", 2015-09-03, part of OpenSSL_1_1_0 --, Qin Long customized "crypto/rand/rand_unix.c". So that, when OPENSSL_SYS_UEFI was #defined, the real RAND_poll() function was replaced by a stub that would always report failure. (So this was a safe stub.)
>
> In OpenSSL commit 8389ec4b4950 -- "Add --with-rand-seed", 2017-07-22 --, the feature test itself has been reworked (see the previous patch in this series). However, it remains the case that "rand_unix.c" consumes and honors the OPENSSL_SYS_UEFI macro.
>
> So, let's check the "randfile.c" file. It defines three functions:
> - RAND_load_file
> - RAND_write_file
> - RAND_file_name
>
> Nothing inside the OpenSSL library calls them (they exist purely for client code), and nothing in edk2 calls them either.
>
> (2a) Therefore, we should modify the "randfile.c" source file, with an upstream OpenSSL contribution, to hide the function definitions, when OPENSSL_SYS_UEFI is defined. In other words, continue with Qin Long's approach from commit fb4844bbc62f.
>
> (2b) Alternatively, I'm noticing that "rand" is just another module (similar to "store", see above). Assuming we really don't need RAND_* functions for anything in edk2: have we tried configuring OpenSSL, for the edk2 build, with the "no-rand" parameter?
>
> Thanks,
> Laszlo
>
>>
>> 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;
>>
>
>
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v2 5/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b
2019-05-10 10:26 ` Wang, Jian J
@ 2019-05-13 16:14 ` Laszlo Ersek
2019-05-14 7:03 ` Wang, Jian J
0 siblings, 1 reply; 27+ messages in thread
From: Laszlo Ersek @ 2019-05-13 16:14 UTC (permalink / raw)
To: devel, jian.j.wang, Lu, XiaoyuX; +Cc: Ye, Ting
On 05/10/19 12:26, Wang, Jian J wrote:
> Hi Laszlo,
>
> rand_* is needed by openssl itself. BaseCryptLib also provide RandomSeed()
> and RandomBytes() interface to wrap openssl rand functionality. We can't
> just drop them. From platform independent perspective, using performance
> counter is the best choice we have. If we want to achieve the uttermost
> secure mechanism, only hardware seed/rand generator can meet it. Do you
> agree to add cpu specific instruction to do that? For those processors
> which don't support seed/rand generation, we have to fall back to performance
> counter.
I have not been aware of RandomSeed() / RandomBytes().
The RandomSeed() function lets the caller specify a seed, which is good
design, IMO. In case the caller does not pass in a seed, the function
implements its own seed.
For that default seeding, we have the following implementations:
(1) "CryptoPkg/Library/BaseCryptLib/Rand/CryptRand.c" -- uses a constant
string as seed. :(
(2) "CryptoPkg/Library/BaseCryptLib/Rand/CryptRandItc.c" -- calls
AsmReadItc() to calculate the seed. The function AsmReadItc() is not
defined (or even declared) anywhere in edk2, so I don't know what it does.
(3) "CryptoPkg/Library/BaseCryptLib/Rand/CryptRandNull.c" -- calls
ASSERT(FALSE), then returns FALSE.
(4) "CryptoPkg/Library/BaseCryptLib/Rand/CryptRandTsc.c" -- calls
AsmReadTsc() to calculate the seed.
PEIMs seem to use (3), which appears safe.
DXE drivers, runtime DXE drivers, and SMM drivers, use (4) on IA32/X64,
and (1) on ARM/AARCH64. All quite unfortunate.
Option (2) is never used in edk2.
Based on the above analysis (is it correct?), I must agree that the v1
and v2 approaches in the present patch set, namely "constant data" and
"TimerLib", may not be worse than what we already have.
> Another option is that we could make use of EFI_RNG_PROTOCOL. According
> to UEFI spec (see below), it can be used to get entropy.
>
> "This protocol is used to provide random numbers for use in applications,
> or entropy for seeding other random number generators."
>
> Again, we could use performance counter as a fall back if EFI_RNG_PROTOCOL
> is not provided by a platform. So if a platform really care about the security,
> it should provide a EFI_RNG_PROTOCOL. This can also help to hide the
> hardware/platform dependency.
Consuming EFI_RNG_PROTOCOL would be an improvement.
But it looks quite tricky. Namely, EFI_RNG_PROTOCOL may be provided by a
UEFI driver that follows the UEFI driver model, and so the protocol
could become available first in the BDS phase (when the underlying hwrng
device were connected). Until then, no driver that depends on entropy --
through BaseCryptLib or OpensslLib -- should be dispatched.
On the other hand, if the platform hardware does not include a hwrng
(and so EFI_RNG_PROTOCOL is never produced), then the entropy dependent
drivers should be dispatched as soon as this fact is determined
(dynamically, at every boot).
In other words, entropy-dependent drivers should wait until platform
code (likely in BDS) makes the determination whether EFI_RNG_PROTOCOL
can offer high-quality entropy, or the fallbacks have to be used.
To make it even more complex, SMM drivers that need entropy should
likely never use EFI_RNG_PROTOCOL, because a 3rd party UEFI driver
should not be able to feed entropy (sensitive crypto data) to a
privileged (SMM) driver.
Do you think this is possible to implement?... It looks extremely
complex to me.
Honestly the best I could suggest is, "use RDRAND if available, fall
back to TimerLib otherwise". :( But I would understand if you wanted to
postpone RDRAND until later.
Given this situation, I think I can't give A-b or R-b for this patch in
the series. I think I can only offer regression-testing (for the
upcoming v3). And. I don't intend to block this work based on the
entropy source alone, any more.
... Can we at least collect a detailed list of use cases for which we
must provide entropy? I think we should document that somewhere.
Thanks,
Laszlo
>
> Regards,
> Jian
>
>> -----Original Message-----
>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>> Laszlo Ersek
>> Sent: Friday, May 10, 2019 1:30 AM
>> To: devel@edk2.groups.io; Lu, XiaoyuX <xiaoyux.lu@intel.com>
>> Cc: Wang, Jian J <jian.j.wang@intel.com>; Ye, Ting <ting.ye@intel.com>
>> Subject: Re: [edk2-devel] [PATCH v2 5/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b
>>
>> On 05/09/19 19:15, Laszlo Ersek wrote:
>>
>>> How about the following:
>>>
>>> - It seems like we cannot convince OpenSSL to *never* call these
>>> functions, under UEFI.
>>>
>>> - We also cannot provide an implementation that is *guaranteed* to be
>>> secure enough, IMO.
>>>
>>> - It seems like these functions *should* never be called in the edk2
>>> build however, given that we're not trying to do anything "new" with
>>> OpenSSL in edk2 -- we just want to use the new OpenSSL release for the
>>> same old things.
>>>
>>> - So why not just ensure that these functions *never return*?
>>>
>>> (1) Basically implement all of the functions like this:
>>>
>>> ASSERT (FALSE);
>>> CpuDeadLoop ();
>>> //
>>> // if a return value is needed
>>> //
>>> return 0;
>>>
>>> What do you think about this approach?
>>
>> I notice that "rand" is another module in OpenSSL.
>>
>> Can we try adding "no-rand" to our Configure invocation? Perhaps the
>> need for all of the rand_* functions goes away then.
>>
>> Thanks
>> Laszlo
>>
>>
>
>
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v2 5/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b
2019-05-13 16:14 ` Laszlo Ersek
@ 2019-05-14 7:03 ` Wang, Jian J
2019-05-14 10:58 ` Laszlo Ersek
0 siblings, 1 reply; 27+ messages in thread
From: Wang, Jian J @ 2019-05-14 7:03 UTC (permalink / raw)
To: Laszlo Ersek, devel@edk2.groups.io, Lu, XiaoyuX; +Cc: Ye, Ting
Laszlo,
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Tuesday, May 14, 2019 12:15 AM
> To: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; Lu, XiaoyuX
> <xiaoyux.lu@intel.com>
> Cc: Ye, Ting <ting.ye@intel.com>
> Subject: Re: [edk2-devel] [PATCH v2 5/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b
>
> On 05/10/19 12:26, Wang, Jian J wrote:
> > Hi Laszlo,
> >
> > rand_* is needed by openssl itself. BaseCryptLib also provide RandomSeed()
> > and RandomBytes() interface to wrap openssl rand functionality. We can't
> > just drop them. From platform independent perspective, using performance
> > counter is the best choice we have. If we want to achieve the uttermost
> > secure mechanism, only hardware seed/rand generator can meet it. Do you
> > agree to add cpu specific instruction to do that? For those processors
> > which don't support seed/rand generation, we have to fall back to
> performance
> > counter.
>
> I have not been aware of RandomSeed() / RandomBytes().
>
> The RandomSeed() function lets the caller specify a seed, which is good
> design, IMO. In case the caller does not pass in a seed, the function
> implements its own seed.
>
> For that default seeding, we have the following implementations:
>
> (1) "CryptoPkg/Library/BaseCryptLib/Rand/CryptRand.c" -- uses a constant
> string as seed. :(
>
> (2) "CryptoPkg/Library/BaseCryptLib/Rand/CryptRandItc.c" -- calls
> AsmReadItc() to calculate the seed. The function AsmReadItc() is not
> defined (or even declared) anywhere in edk2, so I don't know what it does.
>
> (3) "CryptoPkg/Library/BaseCryptLib/Rand/CryptRandNull.c" -- calls
> ASSERT(FALSE), then returns FALSE.
>
> (4) "CryptoPkg/Library/BaseCryptLib/Rand/CryptRandTsc.c" -- calls
> AsmReadTsc() to calculate the seed.
>
>
> PEIMs seem to use (3), which appears safe.
>
> DXE drivers, runtime DXE drivers, and SMM drivers, use (4) on IA32/X64,
> and (1) on ARM/AARCH64. All quite unfortunate.
>
> Option (2) is never used in edk2.
>
> Based on the above analysis (is it correct?), I must agree that the v1
> and v2 approaches in the present patch set, namely "constant data" and
> "TimerLib", may not be worse than what we already have.
>
I think you're right. Current porting in tree simply returns 0.
>
> > Another option is that we could make use of EFI_RNG_PROTOCOL. According
> > to UEFI spec (see below), it can be used to get entropy.
> >
> > "This protocol is used to provide random numbers for use in applications,
> > or entropy for seeding other random number generators."
> >
> > Again, we could use performance counter as a fall back if
> EFI_RNG_PROTOCOL
> > is not provided by a platform. So if a platform really care about the security,
> > it should provide a EFI_RNG_PROTOCOL. This can also help to hide the
> > hardware/platform dependency.
>
> Consuming EFI_RNG_PROTOCOL would be an improvement.
>
> But it looks quite tricky. Namely, EFI_RNG_PROTOCOL may be provided by a
> UEFI driver that follows the UEFI driver model, and so the protocol
> could become available first in the BDS phase (when the underlying hwrng
> device were connected). Until then, no driver that depends on entropy --
> through BaseCryptLib or OpensslLib -- should be dispatched.
>
> On the other hand, if the platform hardware does not include a hwrng
> (and so EFI_RNG_PROTOCOL is never produced), then the entropy dependent
> drivers should be dispatched as soon as this fact is determined
> (dynamically, at every boot).
>
> In other words, entropy-dependent drivers should wait until platform
> code (likely in BDS) makes the determination whether EFI_RNG_PROTOCOL
> can offer high-quality entropy, or the fallbacks have to be used.
>
> To make it even more complex, SMM drivers that need entropy should
> likely never use EFI_RNG_PROTOCOL, because a 3rd party UEFI driver
> should not be able to feed entropy (sensitive crypto data) to a
> privileged (SMM) driver.
>
> Do you think this is possible to implement?... It looks extremely
> complex to me.
>
I have to admit I forgot the SMM part. You're right it's complex to do.
Let's forget this option at present.
> Honestly the best I could suggest is, "use RDRAND if available, fall
> back to TimerLib otherwise". :( But I would understand if you wanted to
> postpone RDRAND until later.
>
Sorry about that Xiaoyu sent out update patch series before
we get consensus. He misunderstood some details in the
review process. (Apparently I didn't coach him well.)
Actually we wanted to use hardware seed/rand generator at first. I
thought it might not be acceptable due to the fact it's processor
dependent. So we hesitated. My understanding to above comment
is that you agree to use rdrand/rdseed if available and fall back to
TimerLib otherwise, right?
> Given this situation, I think I can't give A-b or R-b for this patch in
> the series. I think I can only offer regression-testing (for the
> upcoming v3). And. I don't intend to block this work based on the
> entropy source alone, any more.
>
> ... Can we at least collect a detailed list of use cases for which we
> must provide entropy? I think we should document that somewhere.
>
Sorry again. I understand the situation here. Thanks for every effort
you put in the review and test. Really appreciate it.
As to the use cases, I checked all code in edk2 and found following
code which call RandSeed/RandBytes interface. RandSeed is called
with (NULL, 0). My understanding is this will let openssl to generate
the seed.
- CryptoPkg\Library\TlsLib\TlsInit.c
- SecurityPkg\HddPassword\HddPasswordDxe.c
For openssl itself, I found components asn1, bn, evp, ocsp, pem, pkcs7, pkcs12,
rsa, ssl (in addition to cms, dsa, srp, which are disabled in edk2) will call rand_*
interface.
Thanks,
Jian
> Thanks,
> Laszlo
>
> >
> > Regards,
> > Jian
> >
> >> -----Original Message-----
> >> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> >> Laszlo Ersek
> >> Sent: Friday, May 10, 2019 1:30 AM
> >> To: devel@edk2.groups.io; Lu, XiaoyuX <xiaoyux.lu@intel.com>
> >> Cc: Wang, Jian J <jian.j.wang@intel.com>; Ye, Ting <ting.ye@intel.com>
> >> Subject: Re: [edk2-devel] [PATCH v2 5/6] CryptoPkg: Upgrade OpenSSL to
> 1.1.1b
> >>
> >> On 05/09/19 19:15, Laszlo Ersek wrote:
> >>
> >>> How about the following:
> >>>
> >>> - It seems like we cannot convince OpenSSL to *never* call these
> >>> functions, under UEFI.
> >>>
> >>> - We also cannot provide an implementation that is *guaranteed* to be
> >>> secure enough, IMO.
> >>>
> >>> - It seems like these functions *should* never be called in the edk2
> >>> build however, given that we're not trying to do anything "new" with
> >>> OpenSSL in edk2 -- we just want to use the new OpenSSL release for the
> >>> same old things.
> >>>
> >>> - So why not just ensure that these functions *never return*?
> >>>
> >>> (1) Basically implement all of the functions like this:
> >>>
> >>> ASSERT (FALSE);
> >>> CpuDeadLoop ();
> >>> //
> >>> // if a return value is needed
> >>> //
> >>> return 0;
> >>>
> >>> What do you think about this approach?
> >>
> >> I notice that "rand" is another module in OpenSSL.
> >>
> >> Can we try adding "no-rand" to our Configure invocation? Perhaps the
> >> need for all of the rand_* functions goes away then.
> >>
> >> Thanks
> >> Laszlo
> >>
> >>
> >
> >
> >
> >
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v2 5/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b
2019-05-14 7:03 ` Wang, Jian J
@ 2019-05-14 10:58 ` Laszlo Ersek
2019-05-14 13:25 ` Wang, Jian J
0 siblings, 1 reply; 27+ messages in thread
From: Laszlo Ersek @ 2019-05-14 10:58 UTC (permalink / raw)
To: Wang, Jian J, devel@edk2.groups.io, Lu, XiaoyuX; +Cc: Ye, Ting
Hi Jian,
On 05/14/19 09:03, Wang, Jian J wrote:
>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Tuesday, May 14, 2019 12:15 AM
>> To: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; Lu, XiaoyuX
>> <xiaoyux.lu@intel.com>
>> Cc: Ye, Ting <ting.ye@intel.com>
>> Subject: Re: [edk2-devel] [PATCH v2 5/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b
>> Honestly the best I could suggest is, "use RDRAND if available, fall
>> back to TimerLib otherwise". :( But I would understand if you wanted to
>> postpone RDRAND until later.
> Actually we wanted to use hardware seed/rand generator at first. I
> thought it might not be acceptable due to the fact it's processor
> dependent. So we hesitated. My understanding to above comment
> is that you agree to use rdrand/rdseed if available and fall back to
> TimerLib otherwise, right?
I've now tried to read up a little bit on RDRAND. It seems that crypto
folks do not universally trust RDRAND. Some people reject RDRAND
completely, while others are willing to use RDRAND as *one* source for
entropy, but they always mix it with other entropy sources. But even
that practice is not acceptable to some people, saying that RDRAND can
be used to compromise those other entropy sources, dependent on the
mixing details. It seems that FreeBSD at the least uses the Yarrow
algorithm for mixing, which comes with its own complexities.
As much as I dislike it, at the moment I cannot recommend anything
better than just TimerLib. I'm not satisfied with TimerLib, but I don't
know enough to suggest an improvement. RDRAND looked like a good entropy
source, but then reading up on people's opinions on it, gave me pause.
In the longer term, I believe a serious reorganization of BaseCryptLib /
OpensslLib / etc might help. Namely, move the seeding / entropy
collection out of these low-level libraries, and force all dependent
modules (drivers and such) to provide their own entropy.
Then, privileged drivers (e.g. SMM drivers) could use a low-level
platform device for collecting randomness, without depending on 3rd
party UEFI drivers. Less privileged drivers (such as for HTTPS boot)
could perhaps consume EFI_RNG_PROTOCOL, or maybe some protocol /
abstraction exposed by TPM drivers.
I guess it's OK if we stick with TimerLib for this OpenSSL version upgrade.
Can we please document the use of platform timers as entropy sources
(including the TSC) in the following wiki article?
https://github.com/tianocore/tianocore.github.io/wiki/CryptoPkg
I'm not asking for many details, just a short summary of the fact and
why we do this.
Thanks
Laszlo
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v2 2/6] CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl
2019-05-13 15:12 ` Laszlo Ersek
@ 2019-05-14 12:41 ` Xiaoyu lu
2019-05-14 15:11 ` Laszlo Ersek
0 siblings, 1 reply; 27+ messages in thread
From: Xiaoyu lu @ 2019-05-14 12:41 UTC (permalink / raw)
To: devel@edk2.groups.io, lersek@redhat.com; +Cc: Wang, Jian J, Ye, Ting
Hi Laszlo,
I think process_files.pl is used to control which of OpenSSL source files we need which we don't need.
If we have unwanted files, the effective way is exclude them directly in process_files.pl.
You can see process_files.pl
> 129 ┆ ┆ ┆ ┆ ┆ next if $s =~ "crypto/bio/b_print.c";
Qing Long also use this way to exclude unwanted file.
If the file (example: rand_unix.c) is used by OpenSSL internal, We can't exclude it in process_files.pl,
Than we consider submitting patches for OpenSSL.
What do you think?
Thanks,
Xiaoyu
-----Original Message-----
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Laszlo Ersek
Sent: Monday, May 13, 2019 11:13 PM
To: devel@edk2.groups.io; Lu, XiaoyuX <xiaoyux.lu@intel.com>
Cc: Wang, Jian J <jian.j.wang@intel.com>; Ye, Ting <ting.ye@intel.com>
Subject: Re: [edk2-devel] [PATCH v2 2/6] CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl
On 05/10/19 10:51, Xiaoyu lu wrote:
> Hi, Laszlo:
>
> Thank you for your time.
>
> I try the method you mentioned.
>
>> (1) Therefore, the right thing to do here is to add "no-store" to the above list, in my opinion. Can you try that, please?
>>
>> And, this change should be a standalone patch, similarly to patch v2 1/6 in this series.
>
> (1) OpenSSL configure script don't support no-store option.
> It will lead to configure error.
>
> Unsupported options: no-store
>
>> (2a) Therefore, we should modify the "randfile.c" source file, with an upstream OpenSSL contribution, to hide the function definitions, when OPENSSL_SYS_UEFI is defined. In other words, continue with Qin Long's approach from commit fb4844bbc62f.
>
> I think this is the best way. But the openssl community takes time to accept the patch.
> I just let OpenSSL work for UEFI. So UEFI can use the new algorithm in OpenSSL_1_1_1.
> I am willing to continue to modify this later.
Please pick one of two:
- file a new TianoCore BZ about cleaning up this technical debt, and paste the BZ URL into the code, as a comment
- delay TianoCore BZ#1089 to the next edk2-stable release, and work with upstream OpenSSL to compile out parts of "randfile.c".
Thanks
Laszlo
>
>> (2b) Alternatively, I'm noticing that "rand" is just another module (similar to "store", see above). Assuming we really don't need RAND_* functions for anything in edk2: have we tried configuring OpenSSL, for the edk2 build, with the "no-rand" parameter?
>
> (2) I'm afraid not. Same as (1)
>
> ***** Unsupported options: no-rand
>
> Thanks,
> Xiaoyu.
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Thursday, May 9, 2019 9:43 PM
> To: devel@edk2.groups.io; Lu, XiaoyuX <xiaoyux.lu@intel.com>
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Ye, Ting <ting.ye@intel.com>
> Subject: Re: [edk2-devel] [PATCH v2 2/6] CryptoPkg/OpensslLib: Exclude
> unnecessary files in process_files.pl
>
> On 05/09/19 07:23, Xiaoyu lu wrote:
>> 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.
>
> Indeed.
>
>> OpenSSL_1_1_1(1708e3e85b4a8) add a STORE module (crypto/store/*).
>
> This statement is incorrect (or, minimally, inexact). According to the following command:
>
> $ git log --oneline --reverse OpenSSL_1_1_1b -- crypto/store/ \
> | head -n 1
>
> the first OpenSSL commit that added files to crypto/store/ was:
>
>> commit a5db6fa5760f21d16d59e025e930c02456e00fef
>> Author: Richard Levitte <levitte@openssl.org>
>> Date: Thu May 1 03:53:12 2003 +0000
>>
>> Define a STORE type. For documentation, read the entry in CHANGES,
>> crypto/store/README, crypto/store/store.h and crypto/store/str_locl.h.
>
> This commit goes back to 2003, and is part of releae OpenSSL_0_9_7d.
>
> Instead, let's check what the following command reports:
>
> $ git log --oneline --reverse \
> OpenSSL_1_1_0j..OpenSSL_1_1_1b -- crypto/store/ \
> | head -1
>
> It states that the first commit after OpenSSL_1_1_0j, but not after OpenSSL_1_1_1b, to modify the "crypto/store/" subdirectory, was commit 71a5516dcc8a ("Add the STORE module", 2017-06-29).
>
> If we investigate that commit:
>
> $ git show --stat 71a5516dcc8a
>
> we see that the commit modifies the Configure script:
>
>> Configure | 2 +-
>
> So let's check that part of the diff in detail:
>
> $ git show 71a5516dcc8a -- Configure
>
> And we get:
>
>> diff --git a/Configure b/Configure
>> index 2eacb2312e34..e302a58abb71 100755
>> --- a/Configure
>> +++ b/Configure
>> @@ -310,7 +310,7 @@ $config{sdirs} = [
>> "bn", "ec", "rsa", "dsa", "dh", "dso", "engine",
>> "buffer", "bio", "stack", "lhash", "rand", "err",
>> "evp", "asn1", "pem", "x509", "x509v3", "conf", "txt_db", "pkcs7",
>> "pkcs12", "comp", "ocsp", "ui",
>> - "cms", "ts", "srp", "cmac", "ct", "async", "kdf"
>> + "cms", "ts", "srp", "cmac", "ct", "async", "kdf", "store"
>> ];
>> # test/ subdirectories to build
>> $config{tdirs} = [ "ossl_shim" ];
>
> We can see that the "store" module is added after modules such as "cms", "ts", "srp", and so on.
>
> Now, if you look at "CryptoPkg/Library/OpensslLib/process_files.pl", you find (with edk2 master being at commit 49693202ec9c):
>
> 49 "./Configure",
> 50 "UEFI",
> 51 "no-afalgeng",
> 52 "no-asm",
> 53 "no-async", <---- disables "async"
> 54 "no-autoalginit",
> 55 "no-autoerrinit",
> 56 "no-bf",
> 57 "no-blake2",
> 58 "no-camellia",
> 59 "no-capieng",
> 60 "no-cast",
> 61 "no-chacha",
> 62 "no-cms", <---- disables "cms"
> 63 "no-ct", <---- disables "ct"
> 64 "no-deprecated",
> 65 "no-dgram",
> 66 "no-dsa",
> 67 "no-dynamic-engine",
> 68 "no-ec",
> 69 "no-ec2m",
> 70 "no-engine",
> 71 "no-err",
> 72 "no-filenames",
> 73 "no-gost",
> 74 "no-hw",
> 75 "no-idea",
> 76 "no-mdc2",
> 77 "no-pic",
> 78 "no-ocb",
> 79 "no-poly1305",
> 80 "no-posix-io",
> 81 "no-rc2",
> 82 "no-rfc3779",
> 83 "no-rmd160",
> 84 "no-scrypt",
> 85 "no-seed",
> 86 "no-sock",
> 87 "no-srp", <---- disables "srp"
> 88 "no-ssl",
> 89 "no-stdio",
> 90 "no-threads",
> 91 "no-ts", <---- disables "ts"
> 92 "no-ui",
> 93 "no-whirlpool"
>
> (1) Therefore, the right thing to do here is to add "no-store" to the above list, in my opinion. Can you try that, please?
>
> And, this change should be a standalone patch, similarly to patch v2 1/6 in this series.
>
>> But UEFI don't use them. So exclude these files.
>
>> This file, crypto/rand/randfile.c, have been modified between
>> OpenSSL_1_1_0j(74f2d9c1ec5f5) and OpenSSL_1_1_1b(50eaac9f33376672).
>> It requires more crt runtime support. But UEFI don't use it.
>> So exclude the file.
>
> I think I disagree with this approach.
>
> In OpenSSL commit fb4844bbc62f -- "Add UEFI flag for rand build",
> 2015-09-03, part of OpenSSL_1_1_0 --, Qin Long customized
> "crypto/rand/rand_unix.c". So that, when OPENSSL_SYS_UEFI was
> #defined, the real RAND_poll() function was replaced by a stub that
> would always report failure. (So this was a safe stub.)
>
> In OpenSSL commit 8389ec4b4950 -- "Add --with-rand-seed", 2017-07-22 --, the feature test itself has been reworked (see the previous patch in this series). However, it remains the case that "rand_unix.c" consumes and honors the OPENSSL_SYS_UEFI macro.
>
> So, let's check the "randfile.c" file. It defines three functions:
> - RAND_load_file
> - RAND_write_file
> - RAND_file_name
>
> Nothing inside the OpenSSL library calls them (they exist purely for client code), and nothing in edk2 calls them either.
>
> (2a) Therefore, we should modify the "randfile.c" source file, with an upstream OpenSSL contribution, to hide the function definitions, when OPENSSL_SYS_UEFI is defined. In other words, continue with Qin Long's approach from commit fb4844bbc62f.
>
> (2b) Alternatively, I'm noticing that "rand" is just another module (similar to "store", see above). Assuming we really don't need RAND_* functions for anything in edk2: have we tried configuring OpenSSL, for the edk2 build, with the "no-rand" parameter?
>
> Thanks,
> Laszlo
>
>>
>> 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;
>>
>
>
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v2 5/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b
2019-05-14 10:58 ` Laszlo Ersek
@ 2019-05-14 13:25 ` Wang, Jian J
2019-05-14 15:08 ` Laszlo Ersek
0 siblings, 1 reply; 27+ messages in thread
From: Wang, Jian J @ 2019-05-14 13:25 UTC (permalink / raw)
To: devel@edk2.groups.io, lersek@redhat.com, Lu, XiaoyuX; +Cc: Ye, Ting
Laszlo,
> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Laszlo Ersek
> Sent: Tuesday, May 14, 2019 6:59 PM
> To: Wang, Jian J <jian.j.wang@intel.com>; devel@edk2.groups.io; Lu, XiaoyuX
> <xiaoyux.lu@intel.com>
> Cc: Ye, Ting <ting.ye@intel.com>
> Subject: Re: [edk2-devel] [PATCH v2 5/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b
>
> Hi Jian,
>
> On 05/14/19 09:03, Wang, Jian J wrote:
> >> -----Original Message-----
> >> From: Laszlo Ersek [mailto:lersek@redhat.com]
> >> Sent: Tuesday, May 14, 2019 12:15 AM
> >> To: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; Lu,
> XiaoyuX
> >> <xiaoyux.lu@intel.com>
> >> Cc: Ye, Ting <ting.ye@intel.com>
> >> Subject: Re: [edk2-devel] [PATCH v2 5/6] CryptoPkg: Upgrade OpenSSL to
> 1.1.1b
>
> >> Honestly the best I could suggest is, "use RDRAND if available, fall
> >> back to TimerLib otherwise". :( But I would understand if you wanted to
> >> postpone RDRAND until later.
>
> > Actually we wanted to use hardware seed/rand generator at first. I
> > thought it might not be acceptable due to the fact it's processor
> > dependent. So we hesitated. My understanding to above comment
> > is that you agree to use rdrand/rdseed if available and fall back to
> > TimerLib otherwise, right?
>
> I've now tried to read up a little bit on RDRAND. It seems that crypto
> folks do not universally trust RDRAND. Some people reject RDRAND
> completely, while others are willing to use RDRAND as *one* source for
> entropy, but they always mix it with other entropy sources. But even
> that practice is not acceptable to some people, saying that RDRAND can
> be used to compromise those other entropy sources, dependent on the
> mixing details. It seems that FreeBSD at the least uses the Yarrow
> algorithm for mixing, which comes with its own complexities.
>
> As much as I dislike it, at the moment I cannot recommend anything
> better than just TimerLib. I'm not satisfied with TimerLib, but I don't
> know enough to suggest an improvement. RDRAND looked like a good entropy
> source, but then reading up on people's opinions on it, gave me pause.
>
> In the longer term, I believe a serious reorganization of BaseCryptLib /
> OpensslLib / etc might help. Namely, move the seeding / entropy
> collection out of these low-level libraries, and force all dependent
> modules (drivers and such) to provide their own entropy.
>
> Then, privileged drivers (e.g. SMM drivers) could use a low-level
> platform device for collecting randomness, without depending on 3rd
> party UEFI drivers. Less privileged drivers (such as for HTTPS boot)
> could perhaps consume EFI_RNG_PROTOCOL, or maybe some protocol /
> abstraction exposed by TPM drivers.
>
> I guess it's OK if we stick with TimerLib for this OpenSSL version upgrade.
>
I'm not aware of the rdrand stories. Since there're concerns in the instruction,
I agree not to use it in this patch series and we need a reorganization or even
re-design for the random interface in the future.
To avoid any misunderstanding, let me summarize what will be done in the v4:
TSC is used as first entropy source if it's available and fall back to TimerLib
otherwise. AES block cipher will be used to increase entropy rate or reduce
bias (NIST SP800-90B).
Please confirm above implementation then we'll provide a v4 tomorrow.
> Can we please document the use of platform timers as entropy sources
> (including the TSC) in the following wiki article?
>
> https://github.com/tianocore/tianocore.github.io/wiki/CryptoPkg
>
> I'm not asking for many details, just a short summary of the fact and
> why we do this.
>
Sure.
Regards,
Jian
> Thanks
> Laszlo
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v2 5/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b
2019-05-14 13:25 ` Wang, Jian J
@ 2019-05-14 15:08 ` Laszlo Ersek
0 siblings, 0 replies; 27+ messages in thread
From: Laszlo Ersek @ 2019-05-14 15:08 UTC (permalink / raw)
To: Wang, Jian J, devel@edk2.groups.io, Lu, XiaoyuX; +Cc: Ye, Ting
On 05/14/19 15:25, Wang, Jian J wrote:
> Laszlo,
>
>
>> -----Original Message-----
>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>> Laszlo Ersek
>> Sent: Tuesday, May 14, 2019 6:59 PM
>> To: Wang, Jian J <jian.j.wang@intel.com>; devel@edk2.groups.io; Lu, XiaoyuX
>> <xiaoyux.lu@intel.com>
>> Cc: Ye, Ting <ting.ye@intel.com>
>> Subject: Re: [edk2-devel] [PATCH v2 5/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b
>>
>> Hi Jian,
>>
>> On 05/14/19 09:03, Wang, Jian J wrote:
>>>> -----Original Message-----
>>>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>>>> Sent: Tuesday, May 14, 2019 12:15 AM
>>>> To: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@intel.com>; Lu,
>> XiaoyuX
>>>> <xiaoyux.lu@intel.com>
>>>> Cc: Ye, Ting <ting.ye@intel.com>
>>>> Subject: Re: [edk2-devel] [PATCH v2 5/6] CryptoPkg: Upgrade OpenSSL to
>> 1.1.1b
>>
>>>> Honestly the best I could suggest is, "use RDRAND if available, fall
>>>> back to TimerLib otherwise". :( But I would understand if you wanted to
>>>> postpone RDRAND until later.
>>
>>> Actually we wanted to use hardware seed/rand generator at first. I
>>> thought it might not be acceptable due to the fact it's processor
>>> dependent. So we hesitated. My understanding to above comment
>>> is that you agree to use rdrand/rdseed if available and fall back to
>>> TimerLib otherwise, right?
>>
>> I've now tried to read up a little bit on RDRAND. It seems that crypto
>> folks do not universally trust RDRAND. Some people reject RDRAND
>> completely, while others are willing to use RDRAND as *one* source for
>> entropy, but they always mix it with other entropy sources. But even
>> that practice is not acceptable to some people, saying that RDRAND can
>> be used to compromise those other entropy sources, dependent on the
>> mixing details. It seems that FreeBSD at the least uses the Yarrow
>> algorithm for mixing, which comes with its own complexities.
>>
>> As much as I dislike it, at the moment I cannot recommend anything
>> better than just TimerLib. I'm not satisfied with TimerLib, but I don't
>> know enough to suggest an improvement. RDRAND looked like a good entropy
>> source, but then reading up on people's opinions on it, gave me pause.
>>
>> In the longer term, I believe a serious reorganization of BaseCryptLib /
>> OpensslLib / etc might help. Namely, move the seeding / entropy
>> collection out of these low-level libraries, and force all dependent
>> modules (drivers and such) to provide their own entropy.
>>
>> Then, privileged drivers (e.g. SMM drivers) could use a low-level
>> platform device for collecting randomness, without depending on 3rd
>> party UEFI drivers. Less privileged drivers (such as for HTTPS boot)
>> could perhaps consume EFI_RNG_PROTOCOL, or maybe some protocol /
>> abstraction exposed by TPM drivers.
>>
>> I guess it's OK if we stick with TimerLib for this OpenSSL version upgrade.
>>
>
> I'm not aware of the rdrand stories. Since there're concerns in the instruction,
> I agree not to use it in this patch series and we need a reorganization or even
> re-design for the random interface in the future.
>
> To avoid any misunderstanding, let me summarize what will be done in the v4:
>
> TSC is used as first entropy source if it's available and fall back to TimerLib
> otherwise. AES block cipher will be used to increase entropy rate or reduce
> bias (NIST SP800-90B).
Side comment: I don't think you can increase the entropy rate by
choosing an AES block cipher. You can increase the entropy rate by using
a better entropy source, or by using additional entropy sources. What
AES can help with is to distribute the entropy over all bits in the
block, from the low order bits of the entropy source ("avalanche effect").
> Please confirm above implementation then we'll provide a v4 tomorrow.
Main comment: yes, the above approach looks fine to me, thank you.
Cheers
Laszlo
>
>> Can we please document the use of platform timers as entropy sources
>> (including the TSC) in the following wiki article?
>>
>> https://github.com/tianocore/tianocore.github.io/wiki/CryptoPkg
>>
>> I'm not asking for many details, just a short summary of the fact and
>> why we do this.
>>
>
> Sure.
>
> Regards,
> Jian
>
>> Thanks
>> Laszlo
>>
>>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [edk2-devel] [PATCH v2 2/6] CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl
2019-05-14 12:41 ` Xiaoyu lu
@ 2019-05-14 15:11 ` Laszlo Ersek
0 siblings, 0 replies; 27+ messages in thread
From: Laszlo Ersek @ 2019-05-14 15:11 UTC (permalink / raw)
To: Lu, XiaoyuX, devel@edk2.groups.io; +Cc: Wang, Jian J, Ye, Ting
On 05/14/19 14:41, Lu, XiaoyuX wrote:
> Hi Laszlo,
> I think process_files.pl is used to control which of OpenSSL source files we need which we don't need.
> If we have unwanted files, the effective way is exclude them directly in process_files.pl.
> You can see process_files.pl
>
> > 129 ┆ ┆ ┆ ┆ ┆ next if $s =~ "crypto/bio/b_print.c";
>
> Qing Long also use this way to exclude unwanted file.
>
> If the file (example: rand_unix.c) is used by OpenSSL internal, We can't exclude it in process_files.pl,
> Than we consider submitting patches for OpenSSL.
>
> What do you think?
I agree to excluding "rand_unix.c" similarly to "b_print.c"; that is,
with "next if" in the "process_files.pl" script.
Thanks
Laszlo
> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Laszlo Ersek
> Sent: Monday, May 13, 2019 11:13 PM
> To: devel@edk2.groups.io; Lu, XiaoyuX <xiaoyux.lu@intel.com>
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Ye, Ting <ting.ye@intel.com>
> Subject: Re: [edk2-devel] [PATCH v2 2/6] CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl
>
> On 05/10/19 10:51, Xiaoyu lu wrote:
>> Hi, Laszlo:
>>
>> Thank you for your time.
>>
>> I try the method you mentioned.
>>
>>> (1) Therefore, the right thing to do here is to add "no-store" to the above list, in my opinion. Can you try that, please?
>>>
>>> And, this change should be a standalone patch, similarly to patch v2 1/6 in this series.
>>
>> (1) OpenSSL configure script don't support no-store option.
>> It will lead to configure error.
>>
>> Unsupported options: no-store
>>
>>> (2a) Therefore, we should modify the "randfile.c" source file, with an upstream OpenSSL contribution, to hide the function definitions, when OPENSSL_SYS_UEFI is defined. In other words, continue with Qin Long's approach from commit fb4844bbc62f.
>>
>> I think this is the best way. But the openssl community takes time to accept the patch.
>> I just let OpenSSL work for UEFI. So UEFI can use the new algorithm in OpenSSL_1_1_1.
>> I am willing to continue to modify this later.
>
> Please pick one of two:
>
> - file a new TianoCore BZ about cleaning up this technical debt, and paste the BZ URL into the code, as a comment
>
> - delay TianoCore BZ#1089 to the next edk2-stable release, and work with upstream OpenSSL to compile out parts of "randfile.c".
>
> Thanks
> Laszlo
>
>>
>>> (2b) Alternatively, I'm noticing that "rand" is just another module (similar to "store", see above). Assuming we really don't need RAND_* functions for anything in edk2: have we tried configuring OpenSSL, for the edk2 build, with the "no-rand" parameter?
>>
>> (2) I'm afraid not. Same as (1)
>>
>> ***** Unsupported options: no-rand
>>
>> Thanks,
>> Xiaoyu.
>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Thursday, May 9, 2019 9:43 PM
>> To: devel@edk2.groups.io; Lu, XiaoyuX <xiaoyux.lu@intel.com>
>> Cc: Wang, Jian J <jian.j.wang@intel.com>; Ye, Ting <ting.ye@intel.com>
>> Subject: Re: [edk2-devel] [PATCH v2 2/6] CryptoPkg/OpensslLib: Exclude
>> unnecessary files in process_files.pl
>>
>> On 05/09/19 07:23, Xiaoyu lu wrote:
>>> 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.
>>
>> Indeed.
>>
>>> OpenSSL_1_1_1(1708e3e85b4a8) add a STORE module (crypto/store/*).
>>
>> This statement is incorrect (or, minimally, inexact). According to the following command:
>>
>> $ git log --oneline --reverse OpenSSL_1_1_1b -- crypto/store/ \
>> | head -n 1
>>
>> the first OpenSSL commit that added files to crypto/store/ was:
>>
>>> commit a5db6fa5760f21d16d59e025e930c02456e00fef
>>> Author: Richard Levitte <levitte@openssl.org>
>>> Date: Thu May 1 03:53:12 2003 +0000
>>>
>>> Define a STORE type. For documentation, read the entry in CHANGES,
>>> crypto/store/README, crypto/store/store.h and crypto/store/str_locl.h.
>>
>> This commit goes back to 2003, and is part of releae OpenSSL_0_9_7d.
>>
>> Instead, let's check what the following command reports:
>>
>> $ git log --oneline --reverse \
>> OpenSSL_1_1_0j..OpenSSL_1_1_1b -- crypto/store/ \
>> | head -1
>>
>> It states that the first commit after OpenSSL_1_1_0j, but not after OpenSSL_1_1_1b, to modify the "crypto/store/" subdirectory, was commit 71a5516dcc8a ("Add the STORE module", 2017-06-29).
>>
>> If we investigate that commit:
>>
>> $ git show --stat 71a5516dcc8a
>>
>> we see that the commit modifies the Configure script:
>>
>>> Configure | 2 +-
>>
>> So let's check that part of the diff in detail:
>>
>> $ git show 71a5516dcc8a -- Configure
>>
>> And we get:
>>
>>> diff --git a/Configure b/Configure
>>> index 2eacb2312e34..e302a58abb71 100755
>>> --- a/Configure
>>> +++ b/Configure
>>> @@ -310,7 +310,7 @@ $config{sdirs} = [
>>> "bn", "ec", "rsa", "dsa", "dh", "dso", "engine",
>>> "buffer", "bio", "stack", "lhash", "rand", "err",
>>> "evp", "asn1", "pem", "x509", "x509v3", "conf", "txt_db", "pkcs7",
>>> "pkcs12", "comp", "ocsp", "ui",
>>> - "cms", "ts", "srp", "cmac", "ct", "async", "kdf"
>>> + "cms", "ts", "srp", "cmac", "ct", "async", "kdf", "store"
>>> ];
>>> # test/ subdirectories to build
>>> $config{tdirs} = [ "ossl_shim" ];
>>
>> We can see that the "store" module is added after modules such as "cms", "ts", "srp", and so on.
>>
>> Now, if you look at "CryptoPkg/Library/OpensslLib/process_files.pl", you find (with edk2 master being at commit 49693202ec9c):
>>
>> 49 "./Configure",
>> 50 "UEFI",
>> 51 "no-afalgeng",
>> 52 "no-asm",
>> 53 "no-async", <---- disables "async"
>> 54 "no-autoalginit",
>> 55 "no-autoerrinit",
>> 56 "no-bf",
>> 57 "no-blake2",
>> 58 "no-camellia",
>> 59 "no-capieng",
>> 60 "no-cast",
>> 61 "no-chacha",
>> 62 "no-cms", <---- disables "cms"
>> 63 "no-ct", <---- disables "ct"
>> 64 "no-deprecated",
>> 65 "no-dgram",
>> 66 "no-dsa",
>> 67 "no-dynamic-engine",
>> 68 "no-ec",
>> 69 "no-ec2m",
>> 70 "no-engine",
>> 71 "no-err",
>> 72 "no-filenames",
>> 73 "no-gost",
>> 74 "no-hw",
>> 75 "no-idea",
>> 76 "no-mdc2",
>> 77 "no-pic",
>> 78 "no-ocb",
>> 79 "no-poly1305",
>> 80 "no-posix-io",
>> 81 "no-rc2",
>> 82 "no-rfc3779",
>> 83 "no-rmd160",
>> 84 "no-scrypt",
>> 85 "no-seed",
>> 86 "no-sock",
>> 87 "no-srp", <---- disables "srp"
>> 88 "no-ssl",
>> 89 "no-stdio",
>> 90 "no-threads",
>> 91 "no-ts", <---- disables "ts"
>> 92 "no-ui",
>> 93 "no-whirlpool"
>>
>> (1) Therefore, the right thing to do here is to add "no-store" to the above list, in my opinion. Can you try that, please?
>>
>> And, this change should be a standalone patch, similarly to patch v2 1/6 in this series.
>>
>>> But UEFI don't use them. So exclude these files.
>>
>>> This file, crypto/rand/randfile.c, have been modified between
>>> OpenSSL_1_1_0j(74f2d9c1ec5f5) and OpenSSL_1_1_1b(50eaac9f33376672).
>>> It requires more crt runtime support. But UEFI don't use it.
>>> So exclude the file.
>>
>> I think I disagree with this approach.
>>
>> In OpenSSL commit fb4844bbc62f -- "Add UEFI flag for rand build",
>> 2015-09-03, part of OpenSSL_1_1_0 --, Qin Long customized
>> "crypto/rand/rand_unix.c". So that, when OPENSSL_SYS_UEFI was
>> #defined, the real RAND_poll() function was replaced by a stub that
>> would always report failure. (So this was a safe stub.)
>>
>> In OpenSSL commit 8389ec4b4950 -- "Add --with-rand-seed", 2017-07-22 --, the feature test itself has been reworked (see the previous patch in this series). However, it remains the case that "rand_unix.c" consumes and honors the OPENSSL_SYS_UEFI macro.
>>
>> So, let's check the "randfile.c" file. It defines three functions:
>> - RAND_load_file
>> - RAND_write_file
>> - RAND_file_name
>>
>> Nothing inside the OpenSSL library calls them (they exist purely for client code), and nothing in edk2 calls them either.
>>
>> (2a) Therefore, we should modify the "randfile.c" source file, with an upstream OpenSSL contribution, to hide the function definitions, when OPENSSL_SYS_UEFI is defined. In other words, continue with Qin Long's approach from commit fb4844bbc62f.
>>
>> (2b) Alternatively, I'm noticing that "rand" is just another module (similar to "store", see above). Assuming we really don't need RAND_* functions for anything in edk2: have we tried configuring OpenSSL, for the edk2 build, with the "no-rand" parameter?
>>
>> Thanks,
>> Laszlo
>>
>>>
>>> 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;
>>>
>>
>>
>>
>>
>
>
>
>
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2019-05-14 15:11 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-05-09 5:23 [PATCH v2 1/6] CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL Xiaoyu lu
2019-05-09 5:23 ` [PATCH v2 2/6] CryptoPkg/OpensslLib: Exclude unnecessary files in process_files.pl Xiaoyu lu
2019-05-09 13:42 ` [edk2-devel] " Laszlo Ersek
2019-05-10 8:51 ` Xiaoyu lu
2019-05-13 15:12 ` Laszlo Ersek
2019-05-14 12:41 ` Xiaoyu lu
2019-05-14 15:11 ` Laszlo Ersek
2019-05-09 5:23 ` [PATCH v2 3/6] CryptoPkg/IntrinsicLib: Fix possible unresolved external symbol issue Xiaoyu lu
2019-05-09 17:16 ` [edk2-devel] " Laszlo Ersek
2019-05-09 5:23 ` [PATCH v2 4/6] CryptoPkg/OpensslLib: Prepare for upgrading OpenSSL Xiaoyu lu
2019-05-09 13:48 ` [edk2-devel] " Laszlo Ersek
2019-05-09 5:23 ` [PATCH v2 5/6] CryptoPkg: Upgrade OpenSSL to 1.1.1b Xiaoyu lu
2019-05-09 17:15 ` [edk2-devel] " Laszlo Ersek
2019-05-09 17:30 ` Laszlo Ersek
2019-05-10 10:26 ` Wang, Jian J
2019-05-13 16:14 ` Laszlo Ersek
2019-05-14 7:03 ` Wang, Jian J
2019-05-14 10:58 ` Laszlo Ersek
2019-05-14 13:25 ` Wang, Jian J
2019-05-14 15:08 ` Laszlo Ersek
2019-05-09 20:58 ` Laszlo Ersek
2019-05-10 8:51 ` Xiaoyu lu
2019-05-09 5:23 ` [PATCH v2 6/6] CryptoPkg/BaseCryptLib: Make HMAC_CTX size backward compatible Xiaoyu lu
2019-05-09 14:01 ` [edk2-devel] " Laszlo Ersek
2019-05-09 14:20 ` Wang, Jian J
2019-05-09 21:34 ` Laszlo Ersek
2019-05-09 11:32 ` [edk2-devel] [PATCH v2 1/6] CryptoPkg/OpensslLib: Modify process_files.pl for upgrading OpenSSL Laszlo Ersek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox