From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.120]) by mx.groups.io with SMTP id smtpd.web12.5851.1572692505036376435 for ; Sat, 02 Nov 2019 04:01:45 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=F1EhXLVx; spf=pass (domain: redhat.com, ip: 205.139.110.120, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1572692504; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=+TfKm0jOe0GBF9AQzThLPX7UybJq07iWFo2sdA9S/ok=; b=F1EhXLVxshmq73IiyvqPdaykve924lvciGBwnGrEwu5NAOG0I3ZVyjVtlVJ8t8Kpk+GxDp Y0QiXmGAnWHVoNDi6+YUrMJz14drKERgPZsKfKy7nMIwQsA5FZm7T1EHMTa51gbVlMb3yA +jLF6Fb3NDcJagmrKmGuaTIs7iqfxbs= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-338-RjUemK56OXCdjhbHmkDSPg-1; Sat, 02 Nov 2019 07:01:35 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id DFA912B4; Sat, 2 Nov 2019 11:01:33 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-108.ams2.redhat.com [10.36.116.108]) by smtp.corp.redhat.com (Postfix) with ESMTP id D46BD5D9CD; Sat, 2 Nov 2019 11:01:28 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v2 2/8] CryptoPkg/TlsLib: Add the new API "TlsSetVerifyHost" (CVE-2019-14553) To: devel@edk2.groups.io, philmd@redhat.com Cc: David Woodhouse , Jian J Wang , Jiaxin Wu , Sivaraman Nainar , Xiaoyu Lu References: <20191026053719.10453-1-lersek@redhat.com> <20191026053719.10453-3-lersek@redhat.com> <46d160b9-851d-ed02-4dab-4c3ca122d7af@redhat.com> From: "Laszlo Ersek" Message-ID: <15d198af-10bd-76fb-3698-6a2f4e2ba47e@redhat.com> Date: Sat, 2 Nov 2019 12:01:27 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <46d160b9-851d-ed02-4dab-4c3ca122d7af@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-MC-Unique: RjUemK56OXCdjhbHmkDSPg-1 X-Mimecast-Spam-Score: 0 Content-Language: en-US Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 10/26/19 13:51, Philippe Mathieu-Daud=C3=A9 wrote: > On 10/26/19 7:37 AM, Laszlo Ersek wrote: >> From: "Wu, Jiaxin" >> >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D960 >> CVE: CVE-2019-14553 >> In the patch, we add the new API "TlsSetVerifyHost" for the TLS >> protocol to set the specified host name that need to be verified. >> >> Signed-off-by: Wu Jiaxin >> Reviewed-by: Ye Ting >> Reviewed-by: Long Qin >> Reviewed-by: Fu Siyuan >> Acked-by: Laszlo Ersek >> Message-Id: <20190927034441.3096-3-Jiaxin.wu@intel.com> >> Cc: David Woodhouse >> Cc: Jian J Wang >> Cc: Jiaxin Wu >> Cc: Sivaraman Nainar >> Cc: Xiaoyu Lu >> Signed-off-by: Laszlo Ersek >> --- >> >> Notes: >> =C2=A0=C2=A0=C2=A0=C2=A0 v2: >> =C2=A0=C2=A0=C2=A0=C2=A0 - fix whitespace in subject line >> =C2=A0=C2=A0=C2=A0=C2=A0 - drop Contributed-under line per BZ#1373 >> >> =C2=A0 CryptoPkg/Include/Library/TlsLib.h=C2=A0=C2=A0 | 20 +++++++++++ >> =C2=A0 CryptoPkg/Library/TlsLib/TlsConfig.c | 38 +++++++++++++++++++- >> =C2=A0 2 files changed, 57 insertions(+), 1 deletion(-) >> >> diff --git a/CryptoPkg/Include/Library/TlsLib.h >> b/CryptoPkg/Include/Library/TlsLib.h >> index 9875cb6e746b..3af7d4bc095e 100644 >> --- a/CryptoPkg/Include/Library/TlsLib.h >> +++ b/CryptoPkg/Include/Library/TlsLib.h >> @@ -395,8 +395,28 @@ TlsSetVerify ( >> =C2=A0=C2=A0=C2=A0 IN=C2=A0=C2=A0=C2=A0=C2=A0 VOID=C2=A0=C2=A0=C2=A0=C2= = =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0 *Tls, >> =C2=A0=C2=A0=C2=A0 IN=C2=A0=C2=A0=C2=A0=C2=A0 UINT32=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0 VerifyMode >> =C2=A0=C2=A0=C2=A0 ); >> =C2=A0 +/** >> +=C2=A0 Set the specified host name to be verified. >> + >> +=C2=A0 @param[in]=C2=A0 Tls=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0 Pointer to the TLS object. >> +=C2=A0 @param[in]=C2=A0 Flags=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0 The setting flags during the validation. >> +=C2=A0 @param[in]=C2=A0 HostName=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 The spe= cified host name to be verified. >> + >> +=C2=A0 @retval=C2=A0 EFI_SUCCESS=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0 The HostName setting was set >> successfully. >> +=C2=A0 @retval=C2=A0 EFI_INVALID_PARAMETER The parameter is invalid. >> +=C2=A0 @retval=C2=A0 EFI_ABORTED=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0 Invalid HostName setting. >> + >> +**/ >> +EFI_STATUS >> +EFIAPI >> +TlsSetVerifyHost ( >> +=C2=A0 IN=C2=A0=C2=A0=C2=A0=C2=A0 VOID=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0 *Tls, >> +=C2=A0 IN=C2=A0=C2=A0=C2=A0=C2=A0 UINT32=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= = =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0 Flags, >> +=C2=A0 IN=C2=A0=C2=A0=C2=A0=C2=A0 CHAR8=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0 *HostName >> +=C2=A0 ); >> + >> =C2=A0 /** >> =C2=A0=C2=A0=C2=A0 Sets a TLS/SSL session ID to be used during TLS/SSL = connect. >> =C2=A0 =C2=A0=C2=A0=C2=A0 This function sets a session ID to be used wh= en the TLS/SSL >> connection is >> diff --git a/CryptoPkg/Library/TlsLib/TlsConfig.c >> b/CryptoPkg/Library/TlsLib/TlsConfig.c >> index 74b577d60ee3..2bf5aee7c093 100644 >> --- a/CryptoPkg/Library/TlsLib/TlsConfig.c >> +++ b/CryptoPkg/Library/TlsLib/TlsConfig.c >> @@ -1,8 +1,8 @@ >> =C2=A0 /** @file >> =C2=A0=C2=A0=C2=A0 SSL/TLS Configuration Library Wrapper Implementation= over OpenSSL. >> =C2=A0 -Copyright (c) 2016 - 2017, Intel Corporation. All rights reserv= ed.
>> +Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.
>> =C2=A0 (C) Copyright 2016 Hewlett Packard Enterprise Development LP
>> =C2=A0 SPDX-License-Identifier: BSD-2-Clause-Patent >> =C2=A0 =C2=A0 **/ >> @@ -496,8 +496,44 @@ TlsSetVerify ( >> =C2=A0=C2=A0=C2=A0 // >> =C2=A0=C2=A0=C2=A0 SSL_set_verify (TlsConn->Ssl, VerifyMode, NULL); >> =C2=A0 } >> =C2=A0 +/** >> +=C2=A0 Set the specified host name to be verified. >> + >> +=C2=A0 @param[in]=C2=A0 Tls=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0 Pointer to the TLS object. >> +=C2=A0 @param[in]=C2=A0 Flags=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0 The setting flags during the validation. >> +=C2=A0 @param[in]=C2=A0 HostName=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 The spe= cified host name to be verified. >> + >> +=C2=A0 @retval=C2=A0 EFI_SUCCESS=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0 The HostName setting was set >> successfully. >> +=C2=A0 @retval=C2=A0 EFI_INVALID_PARAMETER The parameter is invalid. >> +=C2=A0 @retval=C2=A0 EFI_ABORTED=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0 Invalid HostName setting. >> + >> +**/ >> +EFI_STATUS >> +EFIAPI >> +TlsSetVerifyHost ( >> +=C2=A0 IN=C2=A0=C2=A0=C2=A0=C2=A0 VOID=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0 *Tls, >> +=C2=A0 IN=C2=A0=C2=A0=C2=A0=C2=A0 UINT32=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= = =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0 Flags, >> +=C2=A0 IN=C2=A0=C2=A0=C2=A0=C2=A0 CHAR8=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0 *HostName >> +=C2=A0 ) >> +{ >> +=C2=A0 TLS_CONNECTION=C2=A0 *TlsConn; >> + >> +=C2=A0 TlsConn =3D (TLS_CONNECTION *) Tls; >> +=C2=A0 if (TlsConn =3D=3D NULL || TlsConn->Ssl =3D=3D NULL || HostName= =3D=3D NULL) { >=20 > Nitpicking, I'd check HostName first. Valid point... but this BZ / CVE has been in flight for ~1.5 years now... Let me just run with this patch please. > Reviewed-by: Philippe Mathieu-Daude Thanks! Laszlo >=20 >> +=C2=A0=C2=A0=C2=A0=C2=A0 return EFI_INVALID_PARAMETER; >> +=C2=A0 } >> + >> +=C2=A0 SSL_set_hostflags(TlsConn->Ssl, Flags); >> + >> +=C2=A0 if (SSL_set1_host(TlsConn->Ssl, HostName) =3D=3D 0) { >> +=C2=A0=C2=A0=C2=A0 return EFI_ABORTED; >> +=C2=A0 } >> + >> +=C2=A0 return EFI_SUCCESS; >> +} >> + >> =C2=A0 /** >> =C2=A0=C2=A0=C2=A0 Sets a TLS/SSL session ID to be used during TLS/SSL = connect. >> =C2=A0 =C2=A0=C2=A0=C2=A0 This function sets a session ID to be used wh= en the TLS/SSL >> connection is >> >=20 >=20 >=20