Re: [edk2-devel] [PATCH v4] CryptoPkg: Upgrade OpenSSL to 1.1.1d

["Laszlo Ersek" <lersek@redhat.com>]

On 11/04/19 17:04, Laszlo Ersek wrote:
> On 11/01/19 07:55, Zhang, Shenglei wrote:
>> Update openssl from 1.1.1b to 1.1.1d.
>> Something needs to be noticed is that, there is a bug existing in the
>> released 1_1_1d version(894da2fb7ed5d314ee5c2fc9fd2d9b8b74111596),
>> which causes build failure. So we switch the code base to a usable
>> version, which is 2 commits later than the stable tag.
>> Now we use the version c3656cc594daac8167721dde7220f0e59ae146fc.
>> This log is to fix the build failure.
>> https://bugzilla.tianocore.org/show_bug.cgi?id=2226
>>
>> Besides, the absense of "DSO_NONE" in dso_conf.h causes build failure
>> in OvmfPkg. So update process_files.pl to generate information from
>> "crypto/include/internal/dso_conf.h.in".
>>
>> shm.h and utsname.h are added to avoid GCC build failure.
>>
>> Cc: Jian J Wang <jian.j.wang@intel.com>
>> Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
>> Cc: Liming Gao <liming.gao@intel.com>
>> Signed-off-by: Shenglei Zhang <shenglei.zhang@intel.com>
>> ---
>> v2: Revert the changes in OpensslLib.inf and OpensslLibCrypto.inf.
>>     The removed header files could be auto-generated by process_files.pl now.
>>
>> v3: Add display information for dso_conf.h.
>>
>> v4: Add shm.h and utsname.h to avoid GCC build failure.
>>
>>  CryptoPkg/Library/Include/internal/dso_conf.h | 16 ++++++++++++++++
>>  CryptoPkg/Library/Include/sys/shm.h           |  9 +++++++++
>>  CryptoPkg/Library/Include/sys/utsname.h       | 10 ++++++++++
>>  CryptoPkg/Library/OpensslLib/openssl          |  2 +-
>>  CryptoPkg/Library/OpensslLib/process_files.pl | 15 ++++++++++++++-
>>  5 files changed, 50 insertions(+), 2 deletions(-)
>>  create mode 100644 CryptoPkg/Library/Include/sys/shm.h
>>  create mode 100644 CryptoPkg/Library/Include/sys/utsname.h
>>
>> diff --git a/CryptoPkg/Library/Include/internal/dso_conf.h b/CryptoPkg/Library/Include/internal/dso_conf.h
>> index e69de29bb2d1..43c891588bc2 100644
>> --- a/CryptoPkg/Library/Include/internal/dso_conf.h
>> +++ b/CryptoPkg/Library/Include/internal/dso_conf.h
>> @@ -0,0 +1,16 @@
>> +/* WARNING: do not edit! */
>> +/* Generated from crypto/include/internal/dso_conf.h.in */
>> +/*
>> + * Copyright 2016-2019 The OpenSSL Project Authors. All Rights Reserved.
>> + *
>> + * Licensed under the OpenSSL license (the "License").  You may not use
>> + * this file except in compliance with the License.  You can obtain a copy
>> + * in the file LICENSE in the source distribution or at
>> + * https://www.openssl.org/source/license.html
>> + */
>> +
>> +#ifndef HEADER_DSO_CONF_H
>> +# define HEADER_DSO_CONF_H
>> +# define DSO_NONE
>> +# define DSO_EXTENSION ".so"
>> +#endif
>> diff --git a/CryptoPkg/Library/Include/sys/shm.h b/CryptoPkg/Library/Include/sys/shm.h
>> new file mode 100644
>> index 000000000000..dc0b8e81c8b0
>> --- /dev/null
>> +++ b/CryptoPkg/Library/Include/sys/shm.h
>> @@ -0,0 +1,9 @@
>> +/** @file
>> +  Include file to support building the third-party cryptographic library.
>> +
>> +Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
>> +SPDX-License-Identifier: BSD-2-Clause-Patent
>> +
>> +**/
>> +
>> +#include <CrtLibSupport.h>
>> diff --git a/CryptoPkg/Library/Include/sys/utsname.h b/CryptoPkg/Library/Include/sys/utsname.h
>> new file mode 100644
>> index 000000000000..75955b0a4eb6
>> --- /dev/null
>> +++ b/CryptoPkg/Library/Include/sys/utsname.h
>> @@ -0,0 +1,10 @@
>> +/** @file
>> +  Include file to support building the third-party cryptographic library.
>> +
>> +Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
>> +SPDX-License-Identifier: BSD-2-Clause-Patent
>> +
>> +**/
>> +
>> +#include <CrtLibSupport.h>
>> +
> 
> (1) The trailing empty line should be removed.
> 
>> diff --git a/CryptoPkg/Library/OpensslLib/openssl b/CryptoPkg/Library/OpensslLib/openssl
>> index 50eaac9f3337..c3656cc594da 160000
>> --- a/CryptoPkg/Library/OpensslLib/openssl
>> +++ b/CryptoPkg/Library/OpensslLib/openssl
>> @@ -1 +1 @@
>> -Subproject commit 50eaac9f3337667259de725451f201e784599687
>> +Subproject commit c3656cc594daac8167721dde7220f0e59ae146fc
>> diff --git a/CryptoPkg/Library/OpensslLib/process_files.pl b/CryptoPkg/Library/OpensslLib/process_files.pl
>> index 4fe54cd808a5..dd93bd84da22 100755
>> --- a/CryptoPkg/Library/OpensslLib/process_files.pl
>> +++ b/CryptoPkg/Library/OpensslLib/process_files.pl
>> @@ -106,6 +106,14 @@ BEGIN {
>>                  ) == 0 ||
>>                      die "Failed to generate opensslconf.h!\n";
>>  
>> +            # Generate dso_conf.h per config data
>> +            system(
>> +                "perl -I. -Mconfigdata util/dofile.pl " .
>> +                "crypto/include/internal/dso_conf.h.in " .
>> +                "> include/internal/dso_conf.h"
>> +                ) == 0 ||
>> +                    die "Failed to generate dso_conf.h!\n";
>> +
>>              chdir($basedir) ||
>>                  die "Cannot change to base directory \"" . $basedir . "\"";
>>  
>> @@ -249,12 +257,17 @@ rename( $new_inf_file, $inf_file ) ||
>>  print "Done!";
>>  
>>  #
>> -# Copy opensslconf.h generated from OpenSSL Configuration
>> +# Copy opensslconf.h and dso_conf.h generated from OpenSSL Configuration
>>  #
>>  print "\n--> Duplicating opensslconf.h into Include/openssl ... ";
>>  copy($OPENSSL_PATH . "/include/openssl/opensslconf.h",
>>       $OPENSSL_PATH . "/../../Include/openssl/") ||
>>     die "Cannot copy opensslconf.h!";
>> +print "Done!";
>> +print "\n--> Duplicating dso_conf.h into Include/internal ... ";
>> +copy($OPENSSL_PATH . "/include/internal/dso_conf.h",
>> +     $OPENSSL_PATH . "/../../Include/internal/") ||
>> +   die "Cannot copy dso_conf.h!";
>>  print "Done!\n";
>>  
>>  print "\nProcessing Files Done!\n";
>>
> 
> (2) The comment block at the top of the script has not been extended:
> 
> # This script runs the OpenSSL Configure script, then processes the
> # resulting file list into our local OpensslLib[Crypto].inf and also
> # takes a copy of opensslconf.h.
> 
> It only refers to "opensslconf.h". For consistency, we should update
> that comment block too, with "dso_conf.h".
> 
> With (1) and (2) fixed:
> 
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> 
> I'll follow up with test results soon.

* Simple tests for Secure Boot:

- booting a VM with SB already enabled -> continues booting, and reports
SB enabled

- delete PK in UiApp manually + reboot; check from VM

- re-enroll using EnrollDefaultKeys.efi + reboot; check from VM

- with SB enabled, check rejection using an unsigned UEFI ISO -->
"DxeImageVerificationLib: Image is not signed and SHA256 hash of image
is not found in DB/DBX."

So this looks good.

* HTTPS boot:

- reused two of my earlier server certificates: DNS domain name in
subject Common Name, IP address in subject Alternative Name, and DNS
domain name resolves to IPv4 address (cert#1) vs. IPv6 address (cert#2)

- ran four HTTPS Boot tests in total: { DHCP presents URL with IP
address, DHCP presents URL with DNS domain name } x { IPv4, IPv6 }.

All worked fine.

Tested-by: Laszlo Ersek <lersek@redhat.com>

Thanks!
Laszlo