[PATCH v2 0/5] ArmVirt- Nt32- Ovmf- CryptoPkg: conditionalize libssl presence in OpensslLib

[PATCH v2 0/5] ArmVirt- Nt32- Ovmf- CryptoPkg: conditionalize libssl presence in OpensslLib

[Laszlo Ersek]

Version 2 of the series at
<https://lists.01.org/pipermail/edk2-devel/2017-February/007761.html>.

In version 2, the new library instance is called OpensslLibCrypto, not
OpensslLibNoSsl. The changes are noted in detail on each patch.

Public repo and branch:
https://github.com/lersek/edk2.git conditionalize-ssl-v2

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Gary Lin <glin@suse.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Qin Long <qin.long@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Ting Ye <ting.ye@intel.com>
Cc: Tomas Hoger <thoger@redhat.com>

Thanks,
Laszlo

Laszlo Ersek (5):
  CryptoPkg/OpensslLib: refresh OpensslLib.inf, opensslconf.h after
    32387e00
  CryptoPkg/OpensslLib: introduce OpensslLibCrypto instance
  ArmVirtPkg: resolve OpensslLib to OpensslLibCrypto
  Nt32Pkg: exclude libssl functionality from OpensslLib if
    TLS_ENABLE=FALSE
  OvmfPkg: exclude libssl functionality from OpensslLib if
    TLS_ENABLE=FALSE

 ArmVirtPkg/ArmVirt.dsc.inc                                            |  2 +-
 Nt32Pkg/Nt32Pkg.dsc                                                   |  4 ++
 OvmfPkg/OvmfPkgIa32.dsc                                               |  4 ++
 OvmfPkg/OvmfPkgIa32X64.dsc                                            |  4 ++
 OvmfPkg/OvmfPkgX64.dsc                                                |  4 ++
 CryptoPkg/Library/OpensslLib/OpensslLib.inf                           |  1 +
 CryptoPkg/Library/OpensslLib/{OpensslLib.inf => OpensslLibCrypto.inf} | 55 ++------------------
 CryptoPkg/Library/OpensslLib/opensslconf.h                            |  6 ---
 CryptoPkg/Library/OpensslLib/{OpensslLib.uni => OpensslLibCrypto.uni} |  8 +--
 CryptoPkg/Library/OpensslLib/process_files.sh                         | 27 +++++++---
 10 files changed, 46 insertions(+), 69 deletions(-)
 copy CryptoPkg/Library/OpensslLib/{OpensslLib.inf => OpensslLibCrypto.inf} (90%)
 copy CryptoPkg/Library/OpensslLib/{OpensslLib.uni => OpensslLibCrypto.uni} (67%)

-- 
2.9.3

[PATCH v2 1/5] CryptoPkg/OpensslLib: refresh OpensslLib.inf, opensslconf.h after 32387e00

[Laszlo Ersek]

Commit 32387e0081db ("CryptoPkg: Enable ssl build in OpensslLib directly",
2016-12-14) removed the "no-queue" configuration option in
"process_files.sh", plus it enabled "process_files.sh" to place all libssl
source files into "OpensslLib.inf".

However, the patch apparently failed to capture two changes originating
from the above actions:
- the definitions of the OPENSSL_NO_PQUEUE and NO_PQUEUE macros were not
  removed from "opensslconf.h",
- "ssl/ssl_conf.c" was not added to "OpensslLib.inf".

Refresh these files, completing commit 32387e0081db.

I built OVMF with -D SECURE_BOOT_ENABLE -D TLS_ENABLE, and ArmVirtQemu
with -D SECURE_BOOT_ENABLE, after this fix, and experienced no regression.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Gary Lin <glin@suse.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Qin Long <qin.long@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Ting Ye <ting.ye@intel.com>
Cc: Tomas Hoger <thoger@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    v2:
    - no changes

 CryptoPkg/Library/OpensslLib/OpensslLib.inf | 1 +
 CryptoPkg/Library/OpensslLib/opensslconf.h  | 6 ------
 2 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/CryptoPkg/Library/OpensslLib/OpensslLib.inf b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
index c14e36d341f7..42f523a611e5 100644
--- a/CryptoPkg/Library/OpensslLib/OpensslLib.inf
+++ b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
@@ -516,6 +516,7 @@ [Sources]
   $(OPENSSL_PATH)/ssl/ssl_asn1.c
   $(OPENSSL_PATH)/ssl/ssl_txt.c
   $(OPENSSL_PATH)/ssl/ssl_algs.c
+  $(OPENSSL_PATH)/ssl/ssl_conf.c
   $(OPENSSL_PATH)/ssl/bio_ssl.c
   $(OPENSSL_PATH)/ssl/ssl_err.c
   $(OPENSSL_PATH)/ssl/kssl.c
diff --git a/CryptoPkg/Library/OpensslLib/opensslconf.h b/CryptoPkg/Library/OpensslLib/opensslconf.h
index adcaa01d6b1d..e0054a45fc5f 100644
--- a/CryptoPkg/Library/OpensslLib/opensslconf.h
+++ b/CryptoPkg/Library/OpensslLib/opensslconf.h
@@ -92,9 +92,6 @@ extern "C" {
 #ifndef OPENSSL_NO_POSIX_IO
 # define OPENSSL_NO_POSIX_IO
 #endif
-#ifndef OPENSSL_NO_PQUEUE
-# define OPENSSL_NO_PQUEUE
-#endif
 #ifndef OPENSSL_NO_RC2
 # define OPENSSL_NO_RC2
 #endif
@@ -263,9 +260,6 @@ extern "C" {
 # if defined(OPENSSL_NO_POSIX_IO) && !defined(NO_POSIX_IO)
 #  define NO_POSIX_IO
 # endif
-# if defined(OPENSSL_NO_PQUEUE) && !defined(NO_PQUEUE)
-#  define NO_PQUEUE
-# endif
 # if defined(OPENSSL_NO_RC2) && !defined(NO_RC2)
 #  define NO_RC2
 # endif
-- 
2.9.3

[PATCH v2 2/5] CryptoPkg/OpensslLib: introduce OpensslLibCrypto instance

[Laszlo Ersek]

Commit 32387e0081db ("CryptoPkg: Enable ssl build in OpensslLib directly",
2016-12-14) pulls OpenSSL's libssl files into the "OpensslLib.inf" library
instance unconditionally.

If a platform doesn't include the TLS modules, such as

- CryptoPkg/Library/TlsLib/TlsLib.inf
- NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
- NetworkPkg/TlsDxe/TlsDxe.inf

then the platform never actually uses the libssl functionality that gets
built into "OpensslLib.inf".

Tomas Hoger from Red Hat Product Security tells me that security
evaluation is less demanding if we can actually *exclude* the libssl files
from such OVMF builds that don't specify -D TLS_ENABLE (rather than just
trust modules not to call libssl functions if we don't specify -D
TLS_ENABLE).

This patch introduces a parallel OpensslLib instance called
"OpensslLibCrypto" that is appropriate for platform builds without TLS
enablement. It does not build C source files in vain, and it eases
security review -- all libssl vulnerabilities can be excluded at once.

"OpensslLibCrypto.inf" is created as a copy of "OpensslLib.inf", modifying
the BASE_NAME, MODULE_UNI_FILE and FILE_GUID defines.

"process_files.sh" is extended to auto-generate the list of OpenSSL files
for both library instances accordingly. This list is updated in
"OpensslLibCrypto.inf" at once.

"OpensslLibCrypto.uni" is introduced as a copy of "OpensslLib.uni",
highlighting the difference.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Gary Lin <glin@suse.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Qin Long <qin.long@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Ting Ye <ting.ye@intel.com>
Cc: Tomas Hoger <thoger@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    v2:
    - relace "OpensslLibNoSsl" with "OpensslLibCrypto" in commit message
    - rename OpensslLibNoSsl.{inf,uni} to OpensslLibCrypto.{inf,uni}
    - adapt BASE_NAME and MODULE_UNI_FILE in INF file
    - replace "without libssl" with "(libcrypto only, no libssl)" in UNI
      file
    - replace "OpensslLibNoSsl" with "OpensslLibCrypto" in shell script
    - rename variable OPENSSL_NOSSL_PATH to OPENSSL_CRYPTO_PATH in shell
      script
    - replace "with-ssl" parameter to "filelist" function with
      "crypto-and-ssl" in shell script
    - replace "without-ssl" parameter to "filelist" function with
      "crypto-only" in shell script
    - retest shell script

 CryptoPkg/Library/OpensslLib/{OpensslLib.inf => OpensslLibCrypto.inf} | 56 ++------------------
 CryptoPkg/Library/OpensslLib/{OpensslLib.uni => OpensslLibCrypto.uni} |  8 +--
 CryptoPkg/Library/OpensslLib/process_files.sh                         | 27 +++++++---
 3 files changed, 28 insertions(+), 63 deletions(-)

diff --git a/CryptoPkg/Library/OpensslLib/OpensslLib.inf b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
similarity index 90%
copy from CryptoPkg/Library/OpensslLib/OpensslLib.inf
copy to CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
index 42f523a611e5..9a03c2cf10c5 100644
--- a/CryptoPkg/Library/OpensslLib/OpensslLib.inf
+++ b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
@@ -14,9 +14,9 @@
 
 [Defines]
   INF_VERSION                    = 0x00010005
-  BASE_NAME                      = OpensslLib
-  MODULE_UNI_FILE                = OpensslLib.uni
-  FILE_GUID                      = C873A7D0-9824-409f-9B42-2C158B992E69
+  BASE_NAME                      = OpensslLibCrypto
+  MODULE_UNI_FILE                = OpensslLibCrypto.uni
+  FILE_GUID                      = E29FC209-8B64-4500-BD20-AF4EAE47EA0E
   MODULE_TYPE                    = BASE
   VERSION_STRING                 = 1.0
   LIBRARY_CLASS                  = OpensslLib
@@ -474,56 +474,6 @@ [Sources]
   $(OPENSSL_PATH)/crypto/cmac/cmac.c
   $(OPENSSL_PATH)/crypto/cmac/cm_ameth.c
   $(OPENSSL_PATH)/crypto/cmac/cm_pmeth.c
-  $(OPENSSL_PATH)/ssl/s2_meth.c
-  $(OPENSSL_PATH)/ssl/s2_srvr.c
-  $(OPENSSL_PATH)/ssl/s2_clnt.c
-  $(OPENSSL_PATH)/ssl/s2_lib.c
-  $(OPENSSL_PATH)/ssl/s2_enc.c
-  $(OPENSSL_PATH)/ssl/s2_pkt.c
-  $(OPENSSL_PATH)/ssl/s3_meth.c
-  $(OPENSSL_PATH)/ssl/s3_srvr.c
-  $(OPENSSL_PATH)/ssl/s3_clnt.c
-  $(OPENSSL_PATH)/ssl/s3_lib.c
-  $(OPENSSL_PATH)/ssl/s3_enc.c
-  $(OPENSSL_PATH)/ssl/s3_pkt.c
-  $(OPENSSL_PATH)/ssl/s3_both.c
-  $(OPENSSL_PATH)/ssl/s3_cbc.c
-  $(OPENSSL_PATH)/ssl/s23_meth.c
-  $(OPENSSL_PATH)/ssl/s23_srvr.c
-  $(OPENSSL_PATH)/ssl/s23_clnt.c
-  $(OPENSSL_PATH)/ssl/s23_lib.c
-  $(OPENSSL_PATH)/ssl/s23_pkt.c
-  $(OPENSSL_PATH)/ssl/t1_meth.c
-  $(OPENSSL_PATH)/ssl/t1_srvr.c
-  $(OPENSSL_PATH)/ssl/t1_clnt.c
-  $(OPENSSL_PATH)/ssl/t1_lib.c
-  $(OPENSSL_PATH)/ssl/t1_enc.c
-  $(OPENSSL_PATH)/ssl/t1_ext.c
-  $(OPENSSL_PATH)/ssl/d1_meth.c
-  $(OPENSSL_PATH)/ssl/d1_srvr.c
-  $(OPENSSL_PATH)/ssl/d1_clnt.c
-  $(OPENSSL_PATH)/ssl/d1_lib.c
-  $(OPENSSL_PATH)/ssl/d1_pkt.c
-  $(OPENSSL_PATH)/ssl/d1_both.c
-  $(OPENSSL_PATH)/ssl/d1_srtp.c
-  $(OPENSSL_PATH)/ssl/ssl_lib.c
-  $(OPENSSL_PATH)/ssl/ssl_err2.c
-  $(OPENSSL_PATH)/ssl/ssl_cert.c
-  $(OPENSSL_PATH)/ssl/ssl_sess.c
-  $(OPENSSL_PATH)/ssl/ssl_ciph.c
-  $(OPENSSL_PATH)/ssl/ssl_stat.c
-  $(OPENSSL_PATH)/ssl/ssl_rsa.c
-  $(OPENSSL_PATH)/ssl/ssl_asn1.c
-  $(OPENSSL_PATH)/ssl/ssl_txt.c
-  $(OPENSSL_PATH)/ssl/ssl_algs.c
-  $(OPENSSL_PATH)/ssl/ssl_conf.c
-  $(OPENSSL_PATH)/ssl/bio_ssl.c
-  $(OPENSSL_PATH)/ssl/ssl_err.c
-  $(OPENSSL_PATH)/ssl/kssl.c
-  $(OPENSSL_PATH)/ssl/t1_reneg.c
-  $(OPENSSL_PATH)/ssl/tls_srp.c
-  $(OPENSSL_PATH)/ssl/t1_trce.c
-  $(OPENSSL_PATH)/ssl/ssl_utst.c
 
 # Autogenerated files list ends here
 
diff --git a/CryptoPkg/Library/OpensslLib/OpensslLib.uni b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.uni
similarity index 67%
copy from CryptoPkg/Library/OpensslLib/OpensslLib.uni
copy to CryptoPkg/Library/OpensslLib/OpensslLibCrypto.uni
index 0dffec1c98a3..7891b135953b 100644
--- a/CryptoPkg/Library/OpensslLib/OpensslLib.uni
+++ b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.uni
@@ -1,7 +1,7 @@
 // /** @file
-// This module provides openSSL Library implementation.
+// This module provides openSSL Library implementation (libcrypto only, no libssl).
 //
-// This module provides OpenSSL Library implementation.
+// This module provides OpenSSL Library implementation (libcrypto only, no libssl).
 //
 // Copyright (c) 2010 - 2014, Intel Corporation. All rights reserved.<BR>
 //
@@ -16,7 +16,7 @@
 // **/
 
 
-#string STR_MODULE_ABSTRACT             #language en-US "OpenSSL Library implementation"
+#string STR_MODULE_ABSTRACT             #language en-US "OpenSSL Library implementation (libcrypto only, no libssl)"
 
-#string STR_MODULE_DESCRIPTION          #language en-US "This module provides OpenSSL Library implementation."
+#string STR_MODULE_DESCRIPTION          #language en-US "This module provides OpenSSL Library implementation (libcrypto only, no libssl)."
 
diff --git a/CryptoPkg/Library/OpensslLib/process_files.sh b/CryptoPkg/Library/OpensslLib/process_files.sh
index 6f069ce264ac..9f10409824d1 100755
--- a/CryptoPkg/Library/OpensslLib/process_files.sh
+++ b/CryptoPkg/Library/OpensslLib/process_files.sh
@@ -1,8 +1,8 @@
 #!/bin/sh
 #
-# This script runs the OpenSSL Configure script, then processes the
-# resulting file list into our local OpensslLib.inf and also takes
-# a copy of opensslconf.h.
+# This script runs the OpenSSL Configure script, then processes the resulting
+# file list into our local OpensslLib.inf and OpensslLibCrypto.inf, and also
+# takes a copy of opensslconf.h.
 #
 # This only needs to be done once by a developer when updating to a
 # new version of OpenSSL (or changing options, etc.). Normal users
@@ -10,6 +10,12 @@
 # git repository for them.
 
 OPENSSL_PATH=$(sed -n '/DEFINE OPENSSL_PATH/{s/.* \(openssl-[0-9.]*[a-z]*\)[[:space:]]*/\1/ p}' OpensslLib.inf)
+OPENSSL_CRYPTO_PATH=$(sed -n '/DEFINE OPENSSL_PATH/{s/.* \(openssl-[0-9.]*[a-z]*\)[[:space:]]*/\1/ p}' OpensslLibCrypto.inf)
+
+if [ "$OPENSSL_PATH" != "$OPENSSL_CRYPTO_PATH" ]; then
+    echo "OPENSSL_PATH diverges between OpensslLib.inf and OpensslLibCrypto.inf"
+    exit 1
+fi
 
 if ! cd "${OPENSSL_PATH}" ; then
     echo "Cannot change to OpenSSL directory \"${OPENSSL_PATH}\""
@@ -65,6 +71,8 @@ cd -
 
 function filelist ()
 {
+    SSL_SELECT="$1"
+
     echo '1,/# Autogenerated files list starts here/p'
     echo '/# Autogenerated files list ends here/,$p'
     echo '/# Autogenerated files list starts here/a\'
@@ -76,18 +84,25 @@ function filelist ()
 		;;
 	    LIBSRC=*)
 		LIBSRC=$(echo "$LINE" | sed s/^LIBSRC=//)
-		for FILE in $LIBSRC; do
+		if [ "$RELATIVE_DIRECTORY" != "ssl" ] ||
+		   [ "$SSL_SELECT" = "crypto-and-ssl" ]; then
+		    for FILE in $LIBSRC; do
 			if [ "$FILE" != "b_print.c" ]; then
 			    echo -e '  $(OPENSSL_PATH)/'$RELATIVE_DIRECTORY/$FILE\\r\\
 			fi
-		done
+		    done
+		fi
 		;;
 	esac
     done
     echo -e \\r
 }
 
-filelist < "${OPENSSL_PATH}/MINFO" |  sed -n -f - -i OpensslLib.inf
+filelist crypto-and-ssl < "${OPENSSL_PATH}/MINFO" \
+| sed -n -f - -i OpensslLib.inf
+
+filelist crypto-only < "${OPENSSL_PATH}/MINFO" \
+| sed -n -f - -i OpensslLibCrypto.inf
 
 # We can tell Windows users to put this back manually if they can't run
 # Configure. For now, until the git repository is fixed to store things
-- 
2.9.3

[PATCH v2 3/5] ArmVirtPkg: resolve OpensslLib to OpensslLibCrypto

[Laszlo Ersek]

The OpensslLibCrypto library instance (which does not contain libssl
functions) is sufficient for the Secure Boot feature. It would not be
sufficient for HTTPS booting (which requires TLS), but in ArmVirtPkg, we
don't even enable plaintext HTTP booting for the time being.

Ease security analysis by excluding libssl functionality from the
OpensslLib instance we use.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Tomas Hoger <thoger@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---

Notes:
    v2:
    - replace "OpensslLibNoSsl" with "OpensslLibCrypto" in commit message
    - fix typo "analsysis" in commit message
    - pick up Ard's R-b
    - resolve OpensslLib to OpensslLibCrypto.inf in ArmVirt.dsc.inc, rather
      than OpensslLibNoSsl.inf
    - test-build ArmVirtQemu

 ArmVirtPkg/ArmVirt.dsc.inc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index 43699cb9bdd6..b5de4163c1cb 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -136,7 +136,7 @@ [LibraryClasses.common]
   #
 !if $(SECURE_BOOT_ENABLE) == TRUE
   IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
-  OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
+  OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
   TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
   AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
-- 
2.9.3

[PATCH v2 4/5] Nt32Pkg: exclude libssl functionality from OpensslLib if TLS_ENABLE=FALSE

[Laszlo Ersek]

Ease security analysis by excluding libssl functionality from the
OpensslLib instance we use with TLS_ENABLE=FALSE.

Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Tomas Hoger <thoger@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---

Notes:
    v2:
    - fix typo "analsysis" in commit message
    - resolve OpensslLib to OpensslLibCrypto.inf rather than to
      OpensslLibNoSsl.inf in Nt32Pkg.dsc
    
    v1:
    - I can't build-test this.

 Nt32Pkg/Nt32Pkg.dsc | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Nt32Pkg/Nt32Pkg.dsc b/Nt32Pkg/Nt32Pkg.dsc
index 47e37ecae134..499b1fe8abe0 100644
--- a/Nt32Pkg/Nt32Pkg.dsc
+++ b/Nt32Pkg/Nt32Pkg.dsc
@@ -159,7 +159,11 @@ [LibraryClasses]
   CpuExceptionHandlerLib|MdeModulePkg/Library/CpuExceptionHandlerLibNull/CpuExceptionHandlerLibNull.inf
   LockBoxLib|MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.inf
   IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
+!if $(TLS_ENABLE) == TRUE
   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
+!else
+  OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
+!endif
   
 !if $(SECURE_BOOT_ENABLE) == TRUE
   PlatformSecureLib|Nt32Pkg/Library/PlatformSecureLib/PlatformSecureLib.inf
-- 
2.9.3

[PATCH v2 5/5] OvmfPkg: exclude libssl functionality from OpensslLib if TLS_ENABLE=FALSE

[Laszlo Ersek]

The OpensslLibCrypto library instance (which does not contain libssl
functions) is sufficient for the Secure Boot feature.

Ease security analysis by excluding libssl functionality from the
OpensslLib instance we use with TLS_ENABLE=FALSE.

Cc: Gary Lin <glin@suse.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tomas Hoger <thoger@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Gary Lin <glin@suse.com>
---

Notes:
    v2:
    - replace "OpensslLibNoSsl" with "OpensslLibCrypto" in commit message
    - fix typo "analsysis" in commit message
    - pick up Gary's R-b
    - resolve OpensslLib to OpensslLibCrypto.inf rather than to
      OpensslLibNoSsl.inf in OvmfPkgIa32.dsc, OvmfPkgIa32X64.dsc,
      OvmfPkgX64.dsc
    - test-build OVMF with TLS_ENABLE=TRUE and TLS_ENABLE=FALSE

 OvmfPkg/OvmfPkgIa32.dsc    | 4 ++++
 OvmfPkg/OvmfPkgIa32X64.dsc | 4 ++++
 OvmfPkg/OvmfPkgX64.dsc     | 4 ++++
 3 files changed, 12 insertions(+)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 993547d4859e..0bce56bc8323 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -143,7 +143,11 @@ [LibraryClasses]
   DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
 
   IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
+!if $(TLS_ENABLE) == TRUE
   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
+!else
+  OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
+!endif
 
 !if $(SECURE_BOOT_ENABLE) == TRUE
   PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index f36604ecb4d8..56f7ff9ad004 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -148,7 +148,11 @@ [LibraryClasses]
   DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
 
   IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
+!if $(TLS_ENABLE) == TRUE
   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
+!else
+  OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
+!endif
 
 !if $(SECURE_BOOT_ENABLE) == TRUE
   PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index c5bf1a672b1e..d0b0b0e1beb1 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -148,7 +148,11 @@ [LibraryClasses]
   DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
 
   IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
+!if $(TLS_ENABLE) == TRUE
   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
+!else
+  OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
+!endif
 
 !if $(SECURE_BOOT_ENABLE) == TRUE
   PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
-- 
2.9.3

Re: [PATCH v2 1/5] CryptoPkg/OpensslLib: refresh OpensslLib.inf, opensslconf.h after 32387e00

[Long, Qin]

Yes, these two items were missed before. 

Reviewed-by: Qin Long <qin.long@intel.com>


Best Regards & Thanks,
LONG, Qin

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Friday, February 24, 2017 7:01 PM
> To: edk2-devel-01 <edk2-devel@ml01.01.org>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Gary Lin <glin@suse.com>;
> Wu, Jiaxin <jiaxin.wu@intel.com>; Justen, Jordan L
> <jordan.l.justen@intel.com>; Long, Qin <qin.long@intel.com>; Ni, Ruiyu
> <ruiyu.ni@intel.com>; Ye, Ting <ting.ye@intel.com>; Tomas Hoger
> <thoger@redhat.com>
> Subject: [PATCH v2 1/5] CryptoPkg/OpensslLib: refresh OpensslLib.inf,
> opensslconf.h after 32387e00
> 
> Commit 32387e0081db ("CryptoPkg: Enable ssl build in OpensslLib directly",
> 2016-12-14) removed the "no-queue" configuration option in
> "process_files.sh", plus it enabled "process_files.sh" to place all libssl source
> files into "OpensslLib.inf".
> 
> However, the patch apparently failed to capture two changes originating
> from the above actions:
> - the definitions of the OPENSSL_NO_PQUEUE and NO_PQUEUE macros
> were not
>   removed from "opensslconf.h",
> - "ssl/ssl_conf.c" was not added to "OpensslLib.inf".
> 
> Refresh these files, completing commit 32387e0081db.
> 
> I built OVMF with -D SECURE_BOOT_ENABLE -D TLS_ENABLE, and
> ArmVirtQemu with -D SECURE_BOOT_ENABLE, after this fix, and
> experienced no regression.
> 
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Gary Lin <glin@suse.com>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Qin Long <qin.long@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Ting Ye <ting.ye@intel.com>
> Cc: Tomas Hoger <thoger@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> 
> Notes:
>     v2:
>     - no changes
> 
>  CryptoPkg/Library/OpensslLib/OpensslLib.inf | 1 +
> CryptoPkg/Library/OpensslLib/opensslconf.h  | 6 ------
>  2 files changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/CryptoPkg/Library/OpensslLib/OpensslLib.inf
> b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
> index c14e36d341f7..42f523a611e5 100644
> --- a/CryptoPkg/Library/OpensslLib/OpensslLib.inf
> +++ b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
> @@ -516,6 +516,7 @@ [Sources]
>    $(OPENSSL_PATH)/ssl/ssl_asn1.c
>    $(OPENSSL_PATH)/ssl/ssl_txt.c
>    $(OPENSSL_PATH)/ssl/ssl_algs.c
> +  $(OPENSSL_PATH)/ssl/ssl_conf.c
>    $(OPENSSL_PATH)/ssl/bio_ssl.c
>    $(OPENSSL_PATH)/ssl/ssl_err.c
>    $(OPENSSL_PATH)/ssl/kssl.c
> diff --git a/CryptoPkg/Library/OpensslLib/opensslconf.h
> b/CryptoPkg/Library/OpensslLib/opensslconf.h
> index adcaa01d6b1d..e0054a45fc5f 100644
> --- a/CryptoPkg/Library/OpensslLib/opensslconf.h
> +++ b/CryptoPkg/Library/OpensslLib/opensslconf.h
> @@ -92,9 +92,6 @@ extern "C" {
>  #ifndef OPENSSL_NO_POSIX_IO
>  # define OPENSSL_NO_POSIX_IO
>  #endif
> -#ifndef OPENSSL_NO_PQUEUE
> -# define OPENSSL_NO_PQUEUE
> -#endif
>  #ifndef OPENSSL_NO_RC2
>  # define OPENSSL_NO_RC2
>  #endif
> @@ -263,9 +260,6 @@ extern "C" {
>  # if defined(OPENSSL_NO_POSIX_IO) && !defined(NO_POSIX_IO)  #  define
> NO_POSIX_IO  # endif -# if defined(OPENSSL_NO_PQUEUE)
> && !defined(NO_PQUEUE) -#  define NO_PQUEUE -# endif  # if
> defined(OPENSSL_NO_RC2) && !defined(NO_RC2)  #  define NO_RC2  #
> endif
> --
> 2.9.3
> 

Re: [PATCH v2 2/5] CryptoPkg/OpensslLib: introduce OpensslLibCrypto instance

[Long, Qin]

Reviewed-by: Qin Long <qin.long@intel.com>


Best Regards & Thanks,
LONG, Qin

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Laszlo Ersek
> Sent: Friday, February 24, 2017 7:01 PM
> To: edk2-devel-01 <edk2-devel@ml01.01.org>
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>; Ye, Ting <ting.ye@intel.com>; Justen, Jordan L
> <jordan.l.justen@intel.com>; Wu, Jiaxin <jiaxin.wu@intel.com>; Gary Lin
> <glin@suse.com>; Long, Qin <qin.long@intel.com>; Tomas Hoger
> <thoger@redhat.com>
> Subject: [edk2] [PATCH v2 2/5] CryptoPkg/OpensslLib: introduce
> OpensslLibCrypto instance
> 
> Commit 32387e0081db ("CryptoPkg: Enable ssl build in OpensslLib directly",
> 2016-12-14) pulls OpenSSL's libssl files into the "OpensslLib.inf" library
> instance unconditionally.
> 
> If a platform doesn't include the TLS modules, such as
> 
> - CryptoPkg/Library/TlsLib/TlsLib.inf
> - NetworkPkg/TlsAuthConfigDxe/TlsAuthConfigDxe.inf
> - NetworkPkg/TlsDxe/TlsDxe.inf
> 
> then the platform never actually uses the libssl functionality that gets built
> into "OpensslLib.inf".
> 
> Tomas Hoger from Red Hat Product Security tells me that security evaluation
> is less demanding if we can actually *exclude* the libssl files from such OVMF
> builds that don't specify -D TLS_ENABLE (rather than just trust modules not
> to call libssl functions if we don't specify -D TLS_ENABLE).
> 
> This patch introduces a parallel OpensslLib instance called "OpensslLibCrypto"
> that is appropriate for platform builds without TLS enablement. It does not
> build C source files in vain, and it eases security review -- all libssl
> vulnerabilities can be excluded at once.
> 
> "OpensslLibCrypto.inf" is created as a copy of "OpensslLib.inf", modifying the
> BASE_NAME, MODULE_UNI_FILE and FILE_GUID defines.
> 
> "process_files.sh" is extended to auto-generate the list of OpenSSL files for
> both library instances accordingly. This list is updated in
> "OpensslLibCrypto.inf" at once.
> 
> "OpensslLibCrypto.uni" is introduced as a copy of "OpensslLib.uni",
> highlighting the difference.
> 
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Gary Lin <glin@suse.com>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Qin Long <qin.long@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Ting Ye <ting.ye@intel.com>
> Cc: Tomas Hoger <thoger@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> 
> Notes:
>     v2:
>     - relace "OpensslLibNoSsl" with "OpensslLibCrypto" in commit message
>     - rename OpensslLibNoSsl.{inf,uni} to OpensslLibCrypto.{inf,uni}
>     - adapt BASE_NAME and MODULE_UNI_FILE in INF file
>     - replace "without libssl" with "(libcrypto only, no libssl)" in UNI
>       file
>     - replace "OpensslLibNoSsl" with "OpensslLibCrypto" in shell script
>     - rename variable OPENSSL_NOSSL_PATH to OPENSSL_CRYPTO_PATH in
> shell
>       script
>     - replace "with-ssl" parameter to "filelist" function with
>       "crypto-and-ssl" in shell script
>     - replace "without-ssl" parameter to "filelist" function with
>       "crypto-only" in shell script
>     - retest shell script
> 
>  CryptoPkg/Library/OpensslLib/{OpensslLib.inf => OpensslLibCrypto.inf} | 56
> ++------------------  CryptoPkg/Library/OpensslLib/{OpensslLib.uni =>
> OpensslLibCrypto.uni} |  8 +--
>  CryptoPkg/Library/OpensslLib/process_files.sh                         | 27 +++++++---
>  3 files changed, 28 insertions(+), 63 deletions(-)
> 
> diff --git a/CryptoPkg/Library/OpensslLib/OpensslLib.inf
> b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
> similarity index 90%
> copy from CryptoPkg/Library/OpensslLib/OpensslLib.inf
> copy to CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
> index 42f523a611e5..9a03c2cf10c5 100644
> --- a/CryptoPkg/Library/OpensslLib/OpensslLib.inf
> +++ b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
> @@ -14,9 +14,9 @@
> 
>  [Defines]
>    INF_VERSION                    = 0x00010005
> -  BASE_NAME                      = OpensslLib
> -  MODULE_UNI_FILE                = OpensslLib.uni
> -  FILE_GUID                      = C873A7D0-9824-409f-9B42-2C158B992E69
> +  BASE_NAME                      = OpensslLibCrypto
> +  MODULE_UNI_FILE                = OpensslLibCrypto.uni
> +  FILE_GUID                      = E29FC209-8B64-4500-BD20-AF4EAE47EA0E
>    MODULE_TYPE                    = BASE
>    VERSION_STRING                 = 1.0
>    LIBRARY_CLASS                  = OpensslLib
> @@ -474,56 +474,6 @@ [Sources]
>    $(OPENSSL_PATH)/crypto/cmac/cmac.c
>    $(OPENSSL_PATH)/crypto/cmac/cm_ameth.c
>    $(OPENSSL_PATH)/crypto/cmac/cm_pmeth.c
> -  $(OPENSSL_PATH)/ssl/s2_meth.c
> -  $(OPENSSL_PATH)/ssl/s2_srvr.c
> -  $(OPENSSL_PATH)/ssl/s2_clnt.c
> -  $(OPENSSL_PATH)/ssl/s2_lib.c
> -  $(OPENSSL_PATH)/ssl/s2_enc.c
> -  $(OPENSSL_PATH)/ssl/s2_pkt.c
> -  $(OPENSSL_PATH)/ssl/s3_meth.c
> -  $(OPENSSL_PATH)/ssl/s3_srvr.c
> -  $(OPENSSL_PATH)/ssl/s3_clnt.c
> -  $(OPENSSL_PATH)/ssl/s3_lib.c
> -  $(OPENSSL_PATH)/ssl/s3_enc.c
> -  $(OPENSSL_PATH)/ssl/s3_pkt.c
> -  $(OPENSSL_PATH)/ssl/s3_both.c
> -  $(OPENSSL_PATH)/ssl/s3_cbc.c
> -  $(OPENSSL_PATH)/ssl/s23_meth.c
> -  $(OPENSSL_PATH)/ssl/s23_srvr.c
> -  $(OPENSSL_PATH)/ssl/s23_clnt.c
> -  $(OPENSSL_PATH)/ssl/s23_lib.c
> -  $(OPENSSL_PATH)/ssl/s23_pkt.c
> -  $(OPENSSL_PATH)/ssl/t1_meth.c
> -  $(OPENSSL_PATH)/ssl/t1_srvr.c
> -  $(OPENSSL_PATH)/ssl/t1_clnt.c
> -  $(OPENSSL_PATH)/ssl/t1_lib.c
> -  $(OPENSSL_PATH)/ssl/t1_enc.c
> -  $(OPENSSL_PATH)/ssl/t1_ext.c
> -  $(OPENSSL_PATH)/ssl/d1_meth.c
> -  $(OPENSSL_PATH)/ssl/d1_srvr.c
> -  $(OPENSSL_PATH)/ssl/d1_clnt.c
> -  $(OPENSSL_PATH)/ssl/d1_lib.c
> -  $(OPENSSL_PATH)/ssl/d1_pkt.c
> -  $(OPENSSL_PATH)/ssl/d1_both.c
> -  $(OPENSSL_PATH)/ssl/d1_srtp.c
> -  $(OPENSSL_PATH)/ssl/ssl_lib.c
> -  $(OPENSSL_PATH)/ssl/ssl_err2.c
> -  $(OPENSSL_PATH)/ssl/ssl_cert.c
> -  $(OPENSSL_PATH)/ssl/ssl_sess.c
> -  $(OPENSSL_PATH)/ssl/ssl_ciph.c
> -  $(OPENSSL_PATH)/ssl/ssl_stat.c
> -  $(OPENSSL_PATH)/ssl/ssl_rsa.c
> -  $(OPENSSL_PATH)/ssl/ssl_asn1.c
> -  $(OPENSSL_PATH)/ssl/ssl_txt.c
> -  $(OPENSSL_PATH)/ssl/ssl_algs.c
> -  $(OPENSSL_PATH)/ssl/ssl_conf.c
> -  $(OPENSSL_PATH)/ssl/bio_ssl.c
> -  $(OPENSSL_PATH)/ssl/ssl_err.c
> -  $(OPENSSL_PATH)/ssl/kssl.c
> -  $(OPENSSL_PATH)/ssl/t1_reneg.c
> -  $(OPENSSL_PATH)/ssl/tls_srp.c
> -  $(OPENSSL_PATH)/ssl/t1_trce.c
> -  $(OPENSSL_PATH)/ssl/ssl_utst.c
> 
>  # Autogenerated files list ends here
> 
> diff --git a/CryptoPkg/Library/OpensslLib/OpensslLib.uni
> b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.uni
> similarity index 67%
> copy from CryptoPkg/Library/OpensslLib/OpensslLib.uni
> copy to CryptoPkg/Library/OpensslLib/OpensslLibCrypto.uni
> index 0dffec1c98a3..7891b135953b 100644
> --- a/CryptoPkg/Library/OpensslLib/OpensslLib.uni
> +++ b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.uni
> @@ -1,7 +1,7 @@
>  // /** @file
> -// This module provides openSSL Library implementation.
> +// This module provides openSSL Library implementation (libcrypto only, no
> libssl).
>  //
> -// This module provides OpenSSL Library implementation.
> +// This module provides OpenSSL Library implementation (libcrypto only, no
> libssl).
>  //
>  // Copyright (c) 2010 - 2014, Intel Corporation. All rights reserved.<BR>  //
> @@ -16,7 +16,7 @@  // **/
> 
> 
> -#string STR_MODULE_ABSTRACT             #language en-US "OpenSSL Library
> implementation"
> +#string STR_MODULE_ABSTRACT             #language en-US "OpenSSL Library
> implementation (libcrypto only, no libssl)"
> 
> -#string STR_MODULE_DESCRIPTION          #language en-US "This module
> provides OpenSSL Library implementation."
> +#string STR_MODULE_DESCRIPTION          #language en-US "This module
> provides OpenSSL Library implementation (libcrypto only, no libssl)."
> 
> diff --git a/CryptoPkg/Library/OpensslLib/process_files.sh
> b/CryptoPkg/Library/OpensslLib/process_files.sh
> index 6f069ce264ac..9f10409824d1 100755
> --- a/CryptoPkg/Library/OpensslLib/process_files.sh
> +++ b/CryptoPkg/Library/OpensslLib/process_files.sh
> @@ -1,8 +1,8 @@
>  #!/bin/sh
>  #
> -# This script runs the OpenSSL Configure script, then processes the -#
> resulting file list into our local OpensslLib.inf and also takes -# a copy of
> opensslconf.h.
> +# This script runs the OpenSSL Configure script, then processes the
> +resulting # file list into our local OpensslLib.inf and
> +OpensslLibCrypto.inf, and also # takes a copy of opensslconf.h.
>  #
>  # This only needs to be done once by a developer when updating to a  # new
> version of OpenSSL (or changing options, etc.). Normal users @@ -10,6
> +10,12 @@  # git repository for them.
> 
>  OPENSSL_PATH=$(sed -n '/DEFINE OPENSSL_PATH/{s/.* \(openssl-[0-
> 9.]*[a-z]*\)[[:space:]]*/\1/ p}' OpensslLib.inf)
> +OPENSSL_CRYPTO_PATH=$(sed -n '/DEFINE OPENSSL_PATH/{s/.*
> +\(openssl-[0-9.]*[a-z]*\)[[:space:]]*/\1/ p}' OpensslLibCrypto.inf)
> +
> +if [ "$OPENSSL_PATH" != "$OPENSSL_CRYPTO_PATH" ]; then
> +    echo "OPENSSL_PATH diverges between OpensslLib.inf and
> OpensslLibCrypto.inf"
> +    exit 1
> +fi
> 
>  if ! cd "${OPENSSL_PATH}" ; then
>      echo "Cannot change to OpenSSL directory \"${OPENSSL_PATH}\""
> @@ -65,6 +71,8 @@ cd -
> 
>  function filelist ()
>  {
> +    SSL_SELECT="$1"
> +
>      echo '1,/# Autogenerated files list starts here/p'
>      echo '/# Autogenerated files list ends here/,$p'
>      echo '/# Autogenerated files list starts here/a\'
> @@ -76,18 +84,25 @@ function filelist ()
>  		;;
>  	    LIBSRC=*)
>  		LIBSRC=$(echo "$LINE" | sed s/^LIBSRC=//)
> -		for FILE in $LIBSRC; do
> +		if [ "$RELATIVE_DIRECTORY" != "ssl" ] ||
> +		   [ "$SSL_SELECT" = "crypto-and-ssl" ]; then
> +		    for FILE in $LIBSRC; do
>  			if [ "$FILE" != "b_print.c" ]; then
>  			    echo -e '
> $(OPENSSL_PATH)/'$RELATIVE_DIRECTORY/$FILE\\r\\
>  			fi
> -		done
> +		    done
> +		fi
>  		;;
>  	esac
>      done
>      echo -e \\r
>  }
> 
> -filelist < "${OPENSSL_PATH}/MINFO" |  sed -n -f - -i OpensslLib.inf
> +filelist crypto-and-ssl < "${OPENSSL_PATH}/MINFO" \
> +| sed -n -f - -i OpensslLib.inf
> +
> +filelist crypto-only < "${OPENSSL_PATH}/MINFO" \
> +| sed -n -f - -i OpensslLibCrypto.inf
> 
>  # We can tell Windows users to put this back manually if they can't run  #
> Configure. For now, until the git repository is fixed to store things
> --
> 2.9.3
> 
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH v2 0/5] ArmVirt- Nt32- Ovmf- CryptoPkg: conditionalize libssl presence in OpensslLib

[Laszlo Ersek]

I committed most of this series as follows:

On 02/24/17 12:01, Laszlo Ersek wrote:
> Version 2 of the series at
> <https://lists.01.org/pipermail/edk2-devel/2017-February/007761.html>.
> 
> In version 2, the new library instance is called OpensslLibCrypto, not
> OpensslLibNoSsl. The changes are noted in detail on each patch.
> 
> Public repo and branch:
> https://github.com/lersek/edk2.git conditionalize-ssl-v2
> 
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Gary Lin <glin@suse.com>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Qin Long <qin.long@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Ting Ye <ting.ye@intel.com>
> Cc: Tomas Hoger <thoger@redhat.com>
> 
> Thanks,
> Laszlo
> 
> Laszlo Ersek (5):
>   CryptoPkg/OpensslLib: refresh OpensslLib.inf, opensslconf.h after
>     32387e00

Committed with Qin Long's review, commit hash 4e719ab5d1de.

>   CryptoPkg/OpensslLib: introduce OpensslLibCrypto instance

Committed with Qin Long's review, commit hash 823005621ef4.

>   ArmVirtPkg: resolve OpensslLib to OpensslLibCrypto

Committed with Ard's review, commit hash 622627f80f25.

>   Nt32Pkg: exclude libssl functionality from OpensslLib if
>     TLS_ENABLE=FALSE

Not committed. Delaying this until Jiaxin & Ray approve it.

>   OvmfPkg: exclude libssl functionality from OpensslLib if
>     TLS_ENABLE=FALSE

Committed with Gary's review, commit hash df453e1b7c7c.

Thanks,
Laszlo

> 
>  ArmVirtPkg/ArmVirt.dsc.inc                                            |  2 +-
>  Nt32Pkg/Nt32Pkg.dsc                                                   |  4 ++
>  OvmfPkg/OvmfPkgIa32.dsc                                               |  4 ++
>  OvmfPkg/OvmfPkgIa32X64.dsc                                            |  4 ++
>  OvmfPkg/OvmfPkgX64.dsc                                                |  4 ++
>  CryptoPkg/Library/OpensslLib/OpensslLib.inf                           |  1 +
>  CryptoPkg/Library/OpensslLib/{OpensslLib.inf => OpensslLibCrypto.inf} | 55 ++------------------
>  CryptoPkg/Library/OpensslLib/opensslconf.h                            |  6 ---
>  CryptoPkg/Library/OpensslLib/{OpensslLib.uni => OpensslLibCrypto.uni} |  8 +--
>  CryptoPkg/Library/OpensslLib/process_files.sh                         | 27 +++++++---
>  10 files changed, 46 insertions(+), 69 deletions(-)
>  copy CryptoPkg/Library/OpensslLib/{OpensslLib.inf => OpensslLibCrypto.inf} (90%)
>  copy CryptoPkg/Library/OpensslLib/{OpensslLib.uni => OpensslLibCrypto.uni} (67%)
> 

Re: [PATCH v2 4/5] Nt32Pkg: exclude libssl functionality from OpensslLib if TLS_ENABLE=FALSE

[Wu, Jiaxin]

Reviewed-by: Wu Jiaxin <jiaxin.wu@intel.com>


Thanks,
Jiaxin

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Laszlo Ersek
> Sent: Friday, February 24, 2017 7:02 PM
> To: edk2-devel-01 <edk2-devel@ml01.01.org>
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Tomas Hoger <thoger@redhat.com>
> Subject: [edk2] [PATCH v2 4/5] Nt32Pkg: exclude libssl functionality from
> OpensslLib if TLS_ENABLE=FALSE
> 
> Ease security analysis by excluding libssl functionality from the
> OpensslLib instance we use with TLS_ENABLE=FALSE.
> 
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Tomas Hoger <thoger@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> 
> Notes:
>     v2:
>     - fix typo "analsysis" in commit message
>     - resolve OpensslLib to OpensslLibCrypto.inf rather than to
>       OpensslLibNoSsl.inf in Nt32Pkg.dsc
> 
>     v1:
>     - I can't build-test this.
> 
>  Nt32Pkg/Nt32Pkg.dsc | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Nt32Pkg/Nt32Pkg.dsc b/Nt32Pkg/Nt32Pkg.dsc
> index 47e37ecae134..499b1fe8abe0 100644
> --- a/Nt32Pkg/Nt32Pkg.dsc
> +++ b/Nt32Pkg/Nt32Pkg.dsc
> @@ -159,7 +159,11 @@ [LibraryClasses]
> 
> CpuExceptionHandlerLib|MdeModulePkg/Library/CpuExceptionHandlerLibN
> ull/CpuExceptionHandlerLibNull.inf
>    LockBoxLib|MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.inf
>    IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
> +!if $(TLS_ENABLE) == TRUE
>    OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
> +!else
> +  OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
> +!endif
> 
>  !if $(SECURE_BOOT_ENABLE) == TRUE
> 
> PlatformSecureLib|Nt32Pkg/Library/PlatformSecureLib/PlatformSecureLib.in
> f
> --
> 2.9.3
> 
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH v2 1/5] CryptoPkg/OpensslLib: refresh OpensslLib.inf, opensslconf.h after 32387e00

[Wu, Jiaxin]

Reviewed-by: Wu Jiaxin <jiaxin.wu@intel.com>


> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Friday, February 24, 2017 7:01 PM
> To: edk2-devel-01 <edk2-devel@ml01.01.org>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Gary Lin <glin@suse.com>;
> Wu, Jiaxin <jiaxin.wu@intel.com>; Justen, Jordan L
> <jordan.l.justen@intel.com>; Long, Qin <qin.long@intel.com>; Ni, Ruiyu
> <ruiyu.ni@intel.com>; Ye, Ting <ting.ye@intel.com>; Tomas Hoger
> <thoger@redhat.com>
> Subject: [PATCH v2 1/5] CryptoPkg/OpensslLib: refresh OpensslLib.inf,
> opensslconf.h after 32387e00
> 
> Commit 32387e0081db ("CryptoPkg: Enable ssl build in OpensslLib directly",
> 2016-12-14) removed the "no-queue" configuration option in
> "process_files.sh", plus it enabled "process_files.sh" to place all libssl
> source files into "OpensslLib.inf".
> 
> However, the patch apparently failed to capture two changes originating
> from the above actions:
> - the definitions of the OPENSSL_NO_PQUEUE and NO_PQUEUE macros
> were not
>   removed from "opensslconf.h",
> - "ssl/ssl_conf.c" was not added to "OpensslLib.inf".
> 
> Refresh these files, completing commit 32387e0081db.
> 
> I built OVMF with -D SECURE_BOOT_ENABLE -D TLS_ENABLE, and
> ArmVirtQemu
> with -D SECURE_BOOT_ENABLE, after this fix, and experienced no regression.
> 
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Gary Lin <glin@suse.com>
> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Qin Long <qin.long@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Ting Ye <ting.ye@intel.com>
> Cc: Tomas Hoger <thoger@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> 
> Notes:
>     v2:
>     - no changes
> 
>  CryptoPkg/Library/OpensslLib/OpensslLib.inf | 1 +
>  CryptoPkg/Library/OpensslLib/opensslconf.h  | 6 ------
>  2 files changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/CryptoPkg/Library/OpensslLib/OpensslLib.inf
> b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
> index c14e36d341f7..42f523a611e5 100644
> --- a/CryptoPkg/Library/OpensslLib/OpensslLib.inf
> +++ b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
> @@ -516,6 +516,7 @@ [Sources]
>    $(OPENSSL_PATH)/ssl/ssl_asn1.c
>    $(OPENSSL_PATH)/ssl/ssl_txt.c
>    $(OPENSSL_PATH)/ssl/ssl_algs.c
> +  $(OPENSSL_PATH)/ssl/ssl_conf.c
>    $(OPENSSL_PATH)/ssl/bio_ssl.c
>    $(OPENSSL_PATH)/ssl/ssl_err.c
>    $(OPENSSL_PATH)/ssl/kssl.c
> diff --git a/CryptoPkg/Library/OpensslLib/opensslconf.h
> b/CryptoPkg/Library/OpensslLib/opensslconf.h
> index adcaa01d6b1d..e0054a45fc5f 100644
> --- a/CryptoPkg/Library/OpensslLib/opensslconf.h
> +++ b/CryptoPkg/Library/OpensslLib/opensslconf.h
> @@ -92,9 +92,6 @@ extern "C" {
>  #ifndef OPENSSL_NO_POSIX_IO
>  # define OPENSSL_NO_POSIX_IO
>  #endif
> -#ifndef OPENSSL_NO_PQUEUE
> -# define OPENSSL_NO_PQUEUE
> -#endif
>  #ifndef OPENSSL_NO_RC2
>  # define OPENSSL_NO_RC2
>  #endif
> @@ -263,9 +260,6 @@ extern "C" {
>  # if defined(OPENSSL_NO_POSIX_IO) && !defined(NO_POSIX_IO)
>  #  define NO_POSIX_IO
>  # endif
> -# if defined(OPENSSL_NO_PQUEUE) && !defined(NO_PQUEUE)
> -#  define NO_PQUEUE
> -# endif
>  # if defined(OPENSSL_NO_RC2) && !defined(NO_RC2)
>  #  define NO_RC2
>  # endif
> --
> 2.9.3
> 

Re: [PATCH v2 4/5] Nt32Pkg: exclude libssl functionality from OpensslLib if TLS_ENABLE=FALSE

[Ni, Ruiyu]

Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>

Thanks/Ray

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Laszlo Ersek
> Sent: Friday, February 24, 2017 7:02 PM
> To: edk2-devel-01 <edk2-devel@ml01.01.org>
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Tomas Hoger <thoger@redhat.com>
> Subject: [edk2] [PATCH v2 4/5] Nt32Pkg: exclude libssl functionality from
> OpensslLib if TLS_ENABLE=FALSE
> 
> Ease security analysis by excluding libssl functionality from the OpensslLib
> instance we use with TLS_ENABLE=FALSE.
> 
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Tomas Hoger <thoger@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> 
> Notes:
>     v2:
>     - fix typo "analsysis" in commit message
>     - resolve OpensslLib to OpensslLibCrypto.inf rather than to
>       OpensslLibNoSsl.inf in Nt32Pkg.dsc
> 
>     v1:
>     - I can't build-test this.
> 
>  Nt32Pkg/Nt32Pkg.dsc | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Nt32Pkg/Nt32Pkg.dsc b/Nt32Pkg/Nt32Pkg.dsc index
> 47e37ecae134..499b1fe8abe0 100644
> --- a/Nt32Pkg/Nt32Pkg.dsc
> +++ b/Nt32Pkg/Nt32Pkg.dsc
> @@ -159,7 +159,11 @@ [LibraryClasses]
> 
> CpuExceptionHandlerLib|MdeModulePkg/Library/CpuExceptionHandlerLibN
> ull/CpuExceptionHandlerLibNull.inf
>    LockBoxLib|MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.inf
>    IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
> +!if $(TLS_ENABLE) == TRUE
>    OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
> +!else
> +  OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
> +!endif
> 
>  !if $(SECURE_BOOT_ENABLE) == TRUE
> 
> PlatformSecureLib|Nt32Pkg/Library/PlatformSecureLib/PlatformSecureLib.in
> f
> --
> 2.9.3
> 
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH v2 4/5] Nt32Pkg: exclude libssl functionality from OpensslLib if TLS_ENABLE=FALSE

[Laszlo Ersek]

On 02/27/17 01:52, Wu, Jiaxin wrote:
> Reviewed-by: Wu Jiaxin <jiaxin.wu@intel.com>

Thank you both, patch committed as 9fba024ed8f7.

Cheers
Laszlo

> 
> 
> Thanks,
> Jiaxin
> 
>> -----Original Message-----
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
>> Laszlo Ersek
>> Sent: Friday, February 24, 2017 7:02 PM
>> To: edk2-devel-01 <edk2-devel@ml01.01.org>
>> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Tomas Hoger <thoger@redhat.com>
>> Subject: [edk2] [PATCH v2 4/5] Nt32Pkg: exclude libssl functionality from
>> OpensslLib if TLS_ENABLE=FALSE
>>
>> Ease security analysis by excluding libssl functionality from the
>> OpensslLib instance we use with TLS_ENABLE=FALSE.
>>
>> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>> Cc: Tomas Hoger <thoger@redhat.com>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>
>> Notes:
>>     v2:
>>     - fix typo "analsysis" in commit message
>>     - resolve OpensslLib to OpensslLibCrypto.inf rather than to
>>       OpensslLibNoSsl.inf in Nt32Pkg.dsc
>>
>>     v1:
>>     - I can't build-test this.
>>
>>  Nt32Pkg/Nt32Pkg.dsc | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/Nt32Pkg/Nt32Pkg.dsc b/Nt32Pkg/Nt32Pkg.dsc
>> index 47e37ecae134..499b1fe8abe0 100644
>> --- a/Nt32Pkg/Nt32Pkg.dsc
>> +++ b/Nt32Pkg/Nt32Pkg.dsc
>> @@ -159,7 +159,11 @@ [LibraryClasses]
>>
>> CpuExceptionHandlerLib|MdeModulePkg/Library/CpuExceptionHandlerLibN
>> ull/CpuExceptionHandlerLibNull.inf
>>    LockBoxLib|MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.inf
>>    IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
>> +!if $(TLS_ENABLE) == TRUE
>>    OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
>> +!else
>> +  OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
>> +!endif
>>
>>  !if $(SECURE_BOOT_ENABLE) == TRUE
>>
>> PlatformSecureLib|Nt32Pkg/Library/PlatformSecureLib/PlatformSecureLib.in
>> f
>> --
>> 2.9.3
>>
>>
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
>