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

["Wang, Jian J" <jian.j.wang@intel.com>]

Push at 1bcc65b9a1408cf445b7b3f9499b27d9c235db71

Regards,
Jian

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wang, Jian
> J
> Sent: Tuesday, November 05, 2019 9:34 AM
> To: Laszlo Ersek <lersek@redhat.com>; devel@edk2.groups.io; Zhang, Shenglei
> <shenglei.zhang@intel.com>
> Cc: Lu, XiaoyuX <xiaoyux.lu@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: Re: [edk2-devel] [PATCH v4] CryptoPkg: Upgrade OpenSSL to 1.1.1d
> 
> Laszlo,
> 
> Thank you for the tests. I also tried secure boot (linux, windows) on both OVMF
> and
> Intel platform. All work well.
> 
> Regards,
> Jian
> 
> > -----Original Message-----
> > From: Laszlo Ersek <lersek@redhat.com>
> > Sent: Tuesday, November 05, 2019 1:45 AM
> > To: devel@edk2.groups.io; Zhang, Shenglei <shenglei.zhang@intel.com>
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, XiaoyuX
> <xiaoyux.lu@intel.com>;
> > Gao, Liming <liming.gao@intel.com>
> > Subject: Re: [edk2-devel] [PATCH v4] CryptoPkg: Upgrade OpenSSL to 1.1.1d
> >
> > 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
> 
> 
>