* [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
* 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 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 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 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 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
* [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
* 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
* [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
* 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
* [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
* 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 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 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 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 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 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 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
* [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 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 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 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
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