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

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 23 February 2017 at 21:57, Laszlo Ersek <lersek@redhat.com> wrote:
> In commit 32387e0081db ("CryptoPkg: Enable ssl build in OpensslLib
> directly", 2016-12-14), we enabled libssl functionality in
> CryptoPkg/OpensslLib unconditionally.
>
> While that's real convenient, it is also overkill for platforms (or
> platform builds) that don't want TLS. The impact (beyond wasted build
> time) is that when the next vulnerability comes out that affects the
> libssl subset of OpenSSL, security teams all around will look at build
> logs and INF files, see the libssl files being built, and get nervous --
> without a good reason for such builds that don't actually *use* TLS.
>
> Let's make this easier on them (and thereby on ourselves!), and
> introduce an OpensslLibNoSsl instance, which excludes libssl.
>

I think it would be nicer to align with OpenSSL more closely, and
split the functionality into a libcrypto and a libssl library, and
include the latter only if TLS functionality is needed. However, I am
not volunteering to do the work, and this approach comes down to the
same thing, given that libssl depends on libcrypto, and so libcrypto
and libcrypto+libssl are the only combinations that make any sense.

> The edk2 integration script "process_files.sh" is updated to process
> both INF files in the same invocation.
>
> If noone disagrees with the concept, I'd appreciate if we could review &
> merge this series real fast. (Sorry about that, but a downstream
> deadline looms close, and I consider this sort of a blocker for the next
> rebase.)
>
> I updated the following platform packages:
> - ArmVirtPkg, because I know it never uses TLS (or HTTP boot for that
>   matter),
> - Nt32Pkg, because it exposes the TLS_ENABLE build flag,
> - OvmfPkg, because it exposes the TLS_ENABLE build flag.
>
> I didn't touch other packages because they don't expose TLS_ENABLE, and
> I don't have time to figure out if they want TLS built-in.
>
> I tested the new OpensslLibNoSsl instance with Secure Boot under OVMF.
>
> The series was formatted with "--find-copies-harder", which makes a real
> difference for patch #2.
>
> Tomas: if you would like to comment on this series, please subscribe to
> the edk2-devel list at
> <https://lists.01.org/mailman/listinfo/edk2-devel>, and also wait for
> your subscription request to complete, *before* responding.
>
> 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 OpensslLibNoSsl instance
>   ArmVirtPkg: resolve OpensslLib to OpensslLibNoSsl
>   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 => OpensslLibNoSsl.inf} | 55 ++------------------
>  CryptoPkg/Library/OpensslLib/opensslconf.h                           |  6 ---
>  CryptoPkg/Library/OpensslLib/{OpensslLib.uni => OpensslLibNoSsl.uni} |  8 +--
>  CryptoPkg/Library/OpensslLib/process_files.sh                        | 27 +++++++---
>  10 files changed, 46 insertions(+), 69 deletions(-)
>  copy CryptoPkg/Library/OpensslLib/{OpensslLib.inf => OpensslLibNoSsl.inf} (90%)
>  copy CryptoPkg/Library/OpensslLib/{OpensslLib.uni => OpensslLibNoSsl.uni} (71%)
>
> --
> 2.9.3
>