public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: Qin Long <qin.long@intel.com>, edk2-devel@ml01.01.org
Cc: ting.ye@intel.com, jiaxin.wu@intel.com
Subject: Re: [PATCH v2] CryptoPkg/OpensslLib: Upgrade OpenSSL version to 1.0.2k
Date: Mon, 27 Feb 2017 11:51:56 +0100	[thread overview]
Message-ID: <835bfb25-4ad8-89d0-73bc-1856455d9d31@redhat.com> (raw)
In-Reply-To: <20170227072044.7852-1-qin.long@intel.com>

On 02/27/17 08:20, Qin Long wrote:
> v2:
> Re-generate the patch after the new OpensslLibCrypto instance.
> 
> OpenSSL 1.0.2k was released with several severity fixes at
> 26-Jan-2017 (https://www.openssl.org/news/secadv/20170126.txt).
> This patch is to upgrade the supported OpenSSL version in
> CryptoPkg/OpensslLib to catch the latest release 1.0.2k.
> 
> Cc: Ye Ting <ting.ye@intel.com>
> Cc: Wu Jiaxin <jiaxin.wu@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Qin Long <qin.long@intel.com>
> ---
>  CryptoPkg/CryptoPkg.dec                            |  4 ++--
>  ...ssl-1.0.2j.patch => EDKII_openssl-1.0.2k.patch} | 26 +++++++++++-----------
>  CryptoPkg/Library/OpensslLib/Install.cmd           |  2 +-
>  CryptoPkg/Library/OpensslLib/Install.sh            |  2 +-
>  CryptoPkg/Library/OpensslLib/OpensslLib.inf        |  6 ++---
>  CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf  |  6 ++---
>  CryptoPkg/Library/OpensslLib/Patch-HOWTO.txt       | 26 +++++++++++-----------
>  7 files changed, 36 insertions(+), 36 deletions(-)
>  rename CryptoPkg/Library/OpensslLib/{EDKII_openssl-1.0.2j.patch => EDKII_openssl-1.0.2k.patch} (96%)

(1) For the next OpenSSL update, please push the patch to your personal
repository, so we can fetch the patch with "git". Patches from the list
can be applied, but this one is particularly hard for that, because it
contains hunks for both CRLF and LF files. I had to split the patch
email manually and to apply it in two steps.

(2) I test-built the patch / OpenSSL 1.0.2k with Ia32, Ia32X64 and X64
OVMF, with -D TLS_ENABLE.

(3) I test-built the same without -D TLS_ENABLE.

(4) I test-built the same for aarch64 / ArmVirtQemu.

(5) For functional testing, I used the Ia32X64 OVMF binary from (3)
(i.e., no TLS).

(5a) The new firmware binary continued to reject an unsigned image:

[Security] 3rd party image[0] can be loaded after EndOfDxe:
PciRoot(0x0)/Pci(0x1E,0x0)/Pci(0x1,0x0)/Pci(0x5,0x0)/Scsi(0x0,0x1)/CDROM(0x0,0x21,0x143C)/\EFI\BOOT\BOOTX64.EFI.
DxeImageVerificationLib: Image is not signed and SHA256 hash of image is
not found in DB/DBX.
The image doesn't pass verification:
PciRoot(0x0)/Pci(0x1E,0x0)/Pci(0x1,0x0)/Pci(0x5,0x0)/Scsi(0x0,0x1)/CDROM(0x0,0x21,0x143C)/\EFI\BOOT\BOOTX64.EFI

(5b) The new fw binary continued to accept signed images, using
previously enrolled certificates. I tested two operating systems (Fedora
and Windows 8.1), also using their internal methods to test whether SB
was enabled (dmesg and confirm-SecureBootUEFI, respectively).

(5c) after manually removing PK, I could boot the (obviously unsigned)
UEFI shell from removable media.

(5d) Also from the CD-ROM image / UEFI shell, I could then run our
downstream (but public, of course) "EnrollDefaultKeys.efi" utility.
After that, the behavior returned to 5a and 5b; i.e. Secure Boot mode
was reenabled. Hence,

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

(6) The patch looks good to me. Thankfully the downstream (edk2-only)
OpenSSL patch needed no real updates (only line numbers seem to differ a
bit). Also, both INF files are updated in sync. Thus,

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

Thanks!
Laszlo


> 
> diff --git a/CryptoPkg/CryptoPkg.dec b/CryptoPkg/CryptoPkg.dec
> index eee26cbccc..27c832707a 100644
> --- a/CryptoPkg/CryptoPkg.dec
> +++ b/CryptoPkg/CryptoPkg.dec
> @@ -4,7 +4,7 @@
>  #  This Package provides cryptographic-related libraries for UEFI security modules.
>  #  It also provides a test application to test libraries.
>  #
> -#  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
> +#  Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.<BR>
>  #  This program and the accompanying materials
>  #  are licensed and made available under the terms and conditions of the BSD License
>  #  which accompanies this distribution.  The full text of the license may be found at
> @@ -24,7 +24,7 @@
>  
>  [Includes]
>    Include
> -  Library/OpensslLib/openssl-1.0.2j/include
> +  Library/OpensslLib/openssl-1.0.2k/include
>  
>  [LibraryClasses]
>    ##  @libraryclass  Provides basic library functions for cryptographic primitives.
> diff --git a/CryptoPkg/Library/OpensslLib/EDKII_openssl-1.0.2j.patch b/CryptoPkg/Library/OpensslLib/EDKII_openssl-1.0.2k.patch
> similarity index 96%
> rename from CryptoPkg/Library/OpensslLib/EDKII_openssl-1.0.2j.patch
> rename to CryptoPkg/Library/OpensslLib/EDKII_openssl-1.0.2k.patch
> index ecd13a9d5f..cc0ce6822e 100644
> --- a/CryptoPkg/Library/OpensslLib/EDKII_openssl-1.0.2j.patch
> +++ b/CryptoPkg/Library/OpensslLib/EDKII_openssl-1.0.2k.patch
> @@ -1,8 +1,8 @@
>  diff --git a/Configure b/Configure
> -index c39f71a..98dd1d0 100755
> +index 5da7cad..c2cc9c5 100755
>  --- a/Configure
>  +++ b/Configure
> -@@ -609,6 +609,9 @@ my %table=(
> +@@ -611,6 +611,9 @@ my %table=(
>   # with itself, Applink is never engaged and can as well be omitted.
>   "mingw64", "gcc:-mno-cygwin -DL_ENDIAN -O3 -Wall -DWIN32_LEAN_AND_MEAN -DUNICODE -D_UNICODE::-D_MT:MINGW64:-lws2_32 -lgdi32 -lcrypt32:SIXTY_FOUR_BIT RC4_CHUNK_LL DES_INT EXPORT_VAR_AS_FN:${x86_64_asm}:mingw64:win32:cygwin-shared:-D_WINDLL:-mno-cygwin:.dll.a",
>   
> @@ -12,7 +12,7 @@ index c39f71a..98dd1d0 100755
>   # UWIN 
>   "UWIN", "cc:-DTERMIOS -DL_ENDIAN -O -Wall:::UWIN::BN_LLONG ${x86_gcc_des} ${x86_gcc_opts}:${no_asm}:win32",
>   
> -@@ -1083,7 +1086,7 @@ if (defined($disabled{"md5"}) || defined($disabled{"sha"})
> +@@ -1085,7 +1088,7 @@ if (defined($disabled{"md5"}) || defined($disabled{"sha"})
>   	}
>   
>   if (defined($disabled{"ec"}) || defined($disabled{"dsa"})
> @@ -22,10 +22,10 @@ index c39f71a..98dd1d0 100755
>   	$disabled{"gost"} = "forced";
>   	}
>  diff --git a/apps/apps.c b/apps/apps.c
> -index 9fdc3e0..6c183b0 100644
> +index c487bd9..64ade15 100644
>  --- a/apps/apps.c
>  +++ b/apps/apps.c
> -@@ -2375,6 +2375,8 @@ int args_verify(char ***pargs, int *pargc,
> +@@ -2386,6 +2386,8 @@ int args_verify(char ***pargs, int *pargc,
>           flags |= X509_V_FLAG_PARTIAL_CHAIN;
>       else if (!strcmp(arg, "-no_alt_chains"))
>           flags |= X509_V_FLAG_NO_ALT_CHAINS;
> @@ -254,7 +254,7 @@ index d5a5514..bede55c 100644
>           goto err;
>   
>  diff --git a/crypto/bn/bn_prime.c b/crypto/bn/bn_prime.c
> -index 1d25687..ad641c3 100644
> +index 8177fd2..4dab3bb 100644
>  --- a/crypto/bn/bn_prime.c
>  +++ b/crypto/bn/bn_prime.c
>  @@ -131,7 +131,7 @@
> @@ -298,7 +298,7 @@ index 1d25687..ad641c3 100644
>       if (ctx != NULL) {
>           BN_CTX_end(ctx);
>           BN_CTX_free(ctx);
> -@@ -375,10 +380,9 @@ static int witness(BIGNUM *w, const BIGNUM *a, const BIGNUM *a1,
> +@@ -376,10 +381,9 @@ static int witness(BIGNUM *w, const BIGNUM *a, const BIGNUM *a1,
>       return 1;
>   }
>   
> @@ -861,7 +861,7 @@ index 585aa8b..04c6cfc 100644
>   /*
>    * Borland C seems too stupid to be able to shift and do longs in the
>  diff --git a/crypto/evp/evp.h b/crypto/evp/evp.h
> -index 39ab793..ad1e350 100644
> +index d258ef8..376f260 100644
>  --- a/crypto/evp/evp.h
>  +++ b/crypto/evp/evp.h
>  @@ -602,11 +602,13 @@ int EVP_MD_CTX_copy(EVP_MD_CTX *out, const EVP_MD_CTX *in);
> @@ -1470,7 +1470,7 @@ index bbc3189..29695f9 100644
>  +
>  +#endif /* OPENSSL_NO_STDIO */
>  diff --git a/crypto/x509/x509_vfy.c b/crypto/x509/x509_vfy.c
> -index 8334b3f..d075f66 100644
> +index b147201..5bf3f07 100644
>  --- a/crypto/x509/x509_vfy.c
>  +++ b/crypto/x509/x509_vfy.c
>  @@ -1064,6 +1064,8 @@ static int check_crl_time(X509_STORE_CTX *ctx, X509_CRL *crl, int notify)
> @@ -1915,10 +1915,10 @@ index 499f0e8..5672f99 100644
>               os.data = NULL;
>               os.length = 0;
>  diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c
> -index f48ebae..ac4f08c 100644
> +index 1be6fb0..cbec97c 100644
>  --- a/ssl/ssl_cert.c
>  +++ b/ssl/ssl_cert.c
> -@@ -857,12 +857,12 @@ int SSL_CTX_add_client_CA(SSL_CTX *ctx, X509 *x)
> +@@ -855,12 +855,12 @@ int SSL_CTX_add_client_CA(SSL_CTX *ctx, X509 *x)
>       return (add_client_CA(&(ctx->client_CA), x));
>   }
>   
> @@ -1932,7 +1932,7 @@ index f48ebae..ac4f08c 100644
>   /**
>    * Load CA certs from a file into a ::STACK. Note that it is somewhat misnamed;
>    * it doesn't really have anything to do with clients (except that a common use
> -@@ -930,7 +930,6 @@ STACK_OF(X509_NAME) *SSL_load_client_CA_file(const char *file)
> +@@ -928,7 +928,6 @@ STACK_OF(X509_NAME) *SSL_load_client_CA_file(const char *file)
>           ERR_clear_error();
>       return (ret);
>   }
> @@ -1940,7 +1940,7 @@ index f48ebae..ac4f08c 100644
>   
>   /**
>    * Add a file of certs to a stack.
> -@@ -1050,6 +1049,7 @@ int SSL_add_dir_cert_subjects_to_stack(STACK_OF(X509_NAME) *stack,
> +@@ -1048,6 +1047,7 @@ int SSL_add_dir_cert_subjects_to_stack(STACK_OF(X509_NAME) *stack,
>       CRYPTO_w_unlock(CRYPTO_LOCK_READDIR);
>       return ret;
>   }
> diff --git a/CryptoPkg/Library/OpensslLib/Install.cmd b/CryptoPkg/Library/OpensslLib/Install.cmd
> index 093414d4b8..e040cda259 100755
> --- a/CryptoPkg/Library/OpensslLib/Install.cmd
> +++ b/CryptoPkg/Library/OpensslLib/Install.cmd
> @@ -1,4 +1,4 @@
> -cd openssl-1.0.2j
> +cd openssl-1.0.2k
>  copy ..\opensslconf.h           crypto
>  if not exist include\openssl mkdir include\openssl
>  copy e_os2.h                    include\openssl
> diff --git a/CryptoPkg/Library/OpensslLib/Install.sh b/CryptoPkg/Library/OpensslLib/Install.sh
> index 7bd55f6ae3..40811e20a6 100755
> --- a/CryptoPkg/Library/OpensslLib/Install.sh
> +++ b/CryptoPkg/Library/OpensslLib/Install.sh
> @@ -1,6 +1,6 @@
>  #!/bin/sh
>  
> -cd openssl-1.0.2j
> +cd openssl-1.0.2k
>  cp ../opensslconf.h           crypto
>  mkdir -p                      include/openssl
>  cp e_os2.h                    include/openssl
> diff --git a/CryptoPkg/Library/OpensslLib/OpensslLib.inf b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
> index 42f523a611..3acc397ace 100644
> --- a/CryptoPkg/Library/OpensslLib/OpensslLib.inf
> +++ b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
> @@ -1,7 +1,7 @@
>  ## @file
>  #  This module provides openSSL Library implementation.
>  #
> -#  Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.<BR>
> +#  Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.<BR>
>  #  This program and the accompanying materials
>  #  are licensed and made available under the terms and conditions of the BSD License
>  #  which accompanies this distribution.  The full text of the license may be found at
> @@ -20,7 +20,7 @@
>    MODULE_TYPE                    = BASE
>    VERSION_STRING                 = 1.0
>    LIBRARY_CLASS                  = OpensslLib
> -  DEFINE OPENSSL_PATH            = openssl-1.0.2j
> +  DEFINE OPENSSL_PATH            = openssl-1.0.2k
>    DEFINE OPENSSL_FLAGS           = -DL_ENDIAN -DOPENSSL_SMALL_FOOTPRINT -D_CRT_SECURE_NO_DEPRECATE -D_CRT_NONSTDC_NO_DEPRECATE
>  
>  #
> @@ -551,7 +551,7 @@
>    # C4702: Potentially uninitialized local variable name used
>    # C4311: pointer truncation from 'type' to 'type'
>    #
> -  MSFT:*_*_IA32_CC_FLAGS    = -U_WIN32 -U_WIN64 -U_MSC_VER $(OPENSSL_FLAGS) /wd4244 /wd4245 /wd4701 /wd4702 /wd4706
> +  MSFT:*_*_IA32_CC_FLAGS    = -U_WIN32 -U_WIN64 -U_MSC_VER $(OPENSSL_FLAGS) /wd4244 /wd4245 /wd4267 /wd4701 /wd4702 /wd4706
>    MSFT:*_*_X64_CC_FLAGS     = -U_WIN32 -U_WIN64 -U_MSC_VER $(OPENSSL_FLAGS) /wd4133 /wd4244 /wd4245 /wd4267 /wd4701 /wd4305 /wd4306 /wd4702 /wd4706 /wd4311
>    MSFT:*_*_IPF_CC_FLAGS     = -U_WIN32 -U_WIN64 -U_MSC_VER $(OPENSSL_FLAGS) /wd4133 /wd4244 /wd4245 /wd4267 /wd4701 /wd4305 /wd4306 /wd4702 /wd4706
>  
> diff --git a/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
> index 9a03c2cf10..b788e0c013 100644
> --- a/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
> +++ b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
> @@ -1,7 +1,7 @@
>  ## @file
>  #  This module provides openSSL Library implementation.
>  #
> -#  Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.<BR>
> +#  Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.<BR>
>  #  This program and the accompanying materials
>  #  are licensed and made available under the terms and conditions of the BSD License
>  #  which accompanies this distribution.  The full text of the license may be found at
> @@ -20,7 +20,7 @@
>    MODULE_TYPE                    = BASE
>    VERSION_STRING                 = 1.0
>    LIBRARY_CLASS                  = OpensslLib
> -  DEFINE OPENSSL_PATH            = openssl-1.0.2j
> +  DEFINE OPENSSL_PATH            = openssl-1.0.2k
>    DEFINE OPENSSL_FLAGS           = -DL_ENDIAN -DOPENSSL_SMALL_FOOTPRINT -D_CRT_SECURE_NO_DEPRECATE -D_CRT_NONSTDC_NO_DEPRECATE
>  
>  #
> @@ -501,7 +501,7 @@
>    # C4702: Potentially uninitialized local variable name used
>    # C4311: pointer truncation from 'type' to 'type'
>    #
> -  MSFT:*_*_IA32_CC_FLAGS    = -U_WIN32 -U_WIN64 -U_MSC_VER $(OPENSSL_FLAGS) /wd4244 /wd4245 /wd4701 /wd4702 /wd4706
> +  MSFT:*_*_IA32_CC_FLAGS    = -U_WIN32 -U_WIN64 -U_MSC_VER $(OPENSSL_FLAGS) /wd4244 /wd4245 /wd4267 /wd4701 /wd4702 /wd4706
>    MSFT:*_*_X64_CC_FLAGS     = -U_WIN32 -U_WIN64 -U_MSC_VER $(OPENSSL_FLAGS) /wd4133 /wd4244 /wd4245 /wd4267 /wd4701 /wd4305 /wd4306 /wd4702 /wd4706 /wd4311
>    MSFT:*_*_IPF_CC_FLAGS     = -U_WIN32 -U_WIN64 -U_MSC_VER $(OPENSSL_FLAGS) /wd4133 /wd4244 /wd4245 /wd4267 /wd4701 /wd4305 /wd4306 /wd4702 /wd4706
>  
> diff --git a/CryptoPkg/Library/OpensslLib/Patch-HOWTO.txt b/CryptoPkg/Library/OpensslLib/Patch-HOWTO.txt
> index d7e3d9e875..8418802ac7 100644
> --- a/CryptoPkg/Library/OpensslLib/Patch-HOWTO.txt
> +++ b/CryptoPkg/Library/OpensslLib/Patch-HOWTO.txt
> @@ -17,36 +17,36 @@ cryptography. This patch will enable openssl building under UEFI environment.
>  ================================================================================
>                                  OpenSSL-Version
>  ================================================================================
> -  Current supported OpenSSL version for UEFI Crypto Library is 1.0.2j.
> -    http://www.openssl.org/source/openssl-1.0.2j.tar.gz
> +  Current supported OpenSSL version for UEFI Crypto Library is 1.0.2k.
> +    http://www.openssl.org/source/openssl-1.0.2k.tar.gz
>  
>  
>  ================================================================================
>                        HOW to Install Openssl for UEFI Building
>  ================================================================================
> -1.  Download OpenSSL 1.0.2j from official website:
> -    http://www.openssl.org/source/openssl-1.0.2j.tar.gz
> +1.  Download OpenSSL 1.0.2k from official website:
> +    http://www.openssl.org/source/openssl-1.0.2k.tar.gz
>  
> -    NOTE: Some web browsers may rename the downloaded TAR file to openssl-1.0.2j.tar.tar.
> -          When you do the download, rename the "openssl-1.0.2j.tar.tar" to
> -          "openssl-1.0.2j.tar.gz" or rename the local downloaded file with ".tar.tar"
> +    NOTE: Some web browsers may rename the downloaded TAR file to openssl-1.0.2k.tar.tar.
> +          When you do the download, rename the "openssl-1.0.2k.tar.tar" to
> +          "openssl-1.0.2k.tar.gz" or rename the local downloaded file with ".tar.tar"
>            extension to ".tar.gz".
>  
> -2.  Extract TAR into CryptoPkg/Library/OpensslLib/openssl-1.0.2j
> +2.  Extract TAR into CryptoPkg/Library/OpensslLib/openssl-1.0.2k
>  
>      NOTE: If you use WinZip to unpack the openssl source in Windows, please
>            uncheck the WinZip smart CR/LF conversion option (WINZIP: Options -->
>            Configuration --> Miscellaneous --> "TAR file smart CR/LF conversion").
>  
> -3.  Apply this patch: EDKII_openssl-1.0.2j.patch, and make installation
> +3.  Apply this patch: EDKII_openssl-1.0.2k.patch, and make installation
>  
>      For Windows Environment:
>      ------------------------
>      1) Make sure the patch utility has been installed in your machine.
>         Install Cygwin or get the patch utility binary from
>            http://gnuwin32.sourceforge.net/packages/patch.htm
> -    2) cd $(WORKSPACE)\CryptoPkg\Library\OpensslLib\openssl-1.0.2j
> -    3) patch -p1 -i ..\EDKII_openssl-1.0.2j.patch
> +    2) cd $(WORKSPACE)\CryptoPkg\Library\OpensslLib\openssl-1.0.2k
> +    3) patch -p1 -i ..\EDKII_openssl-1.0.2k.patch
>      4) cd ..
>      5) Install.cmd
>  
> @@ -54,8 +54,8 @@ cryptography. This patch will enable openssl building under UEFI environment.
>      -----------------------
>      1) Make sure the patch utility has been installed in your machine.
>         Patch utility is available from http://directory.fsf.org/project/patch/
> -    2) cd $(WORKSPACE)/CryptoPkg/Library/OpensslLib/openssl-1.0.2j
> -    3) patch -p1 -i ../EDKII_openssl-1.0.2j.patch
> +    2) cd $(WORKSPACE)/CryptoPkg/Library/OpensslLib/openssl-1.0.2k
> +    3) patch -p1 -i ../EDKII_openssl-1.0.2k.patch
>      4) cd ..
>      5) ./Install.sh
>  
> 



  reply	other threads:[~2017-02-27 10:51 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-27  7:20 [PATCH v2] CryptoPkg/OpensslLib: Upgrade OpenSSL version to 1.0.2k Qin Long
2017-02-27 10:51 ` Laszlo Ersek [this message]
2017-02-27 15:39   ` Long, Qin
2017-02-28  5:58 ` Wu, Jiaxin
2017-02-28  6:50 ` Ye, Ting

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=835bfb25-4ad8-89d0-73bc-1856455d9d31@redhat.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox