From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by mx.groups.io with SMTP id smtpd.web11.2737.1572243158467445300 for ; Sun, 27 Oct 2019 23:12:38 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.93, mailfrom: jian.j.wang@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 27 Oct 2019 23:12:38 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.68,239,1569308400"; d="scan'208";a="400715353" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by fmsmga006.fm.intel.com with ESMTP; 27 Oct 2019 23:12:38 -0700 Received: from fmsmsx114.amr.corp.intel.com (10.18.116.8) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.439.0; Sun, 27 Oct 2019 23:12:37 -0700 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by FMSMSX114.amr.corp.intel.com (10.18.116.8) with Microsoft SMTP Server (TLS) id 14.3.439.0; Sun, 27 Oct 2019 23:12:37 -0700 Received: from shsmsx107.ccr.corp.intel.com ([169.254.9.63]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.213]) with mapi id 14.03.0439.000; Mon, 28 Oct 2019 14:12:35 +0800 From: "Wang, Jian J" To: Laszlo Ersek , edk2-devel-groups-io CC: David Woodhouse , "Wu, Jiaxin" , Sivaraman Nainar , "Lu, XiaoyuX" Subject: Re: [PATCH v2 6/8] CryptoPkg/TlsLib: TlsSetVerifyHost: parse IP address literals as such (CVE-2019-14553) Thread-Topic: [PATCH v2 6/8] CryptoPkg/TlsLib: TlsSetVerifyHost: parse IP address literals as such (CVE-2019-14553) Thread-Index: AQHVi7+CB9Kbd7DbTUecx4fsHAU5AqdvlMJw Date: Mon, 28 Oct 2019 06:12:35 +0000 Message-ID: References: <20191026053719.10453-1-lersek@redhat.com> <20191026053719.10453-7-lersek@redhat.com> In-Reply-To: <20191026053719.10453-7-lersek@redhat.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZDM1MDFhNWItNmVjZS00YzNkLThkNTAtNjZjMDlhZWM1ZTQxIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiR0ZkUHJjN1dBMkRWR2N2KzA1QjYrQzFxUlRKTktxXC85RUlBS09zQ3hEcGc2UVdRRndjXC93N2tCV0RMSU9jNndCIn0= x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.2.0.6 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: jian.j.wang@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable I'm aware of the change is necessary for the solution. But I'm not expert o= n TLS. I'd others to give more professional opinions. Acked-by: Jian J Wang Regards, Jian > -----Original Message----- > From: Laszlo Ersek > Sent: Saturday, October 26, 2019 1:37 PM > To: edk2-devel-groups-io > Cc: David Woodhouse ; Wang, Jian J > ; Wu, Jiaxin ; Sivaraman Nain= ar > ; Lu, XiaoyuX > Subject: [PATCH v2 6/8] CryptoPkg/TlsLib: TlsSetVerifyHost: parse IP addr= ess > literals as such (CVE-2019-14553) >=20 > Using the inet_pton() function that we imported in the previous patches, > recognize if "HostName" is an IP address literal, and then parse it into > binary representation. Passing the latter to OpenSSL for server > certificate validation is important, per RFC-2818 > : >=20 > > In some cases, the URI is specified as an IP address rather than a > > hostname. In this case, the iPAddress subjectAltName must be present in > > the certificate and must exactly match the IP in the URI. >=20 > Note: we cannot use X509_VERIFY_PARAM_set1_ip_asc() because in the > OpenSSL > version that is currently consumed by edk2, said function depends on > sscanf() for parsing IPv4 literals. In > "CryptoPkg/Library/BaseCryptLib/SysCall/CrtWrapper.c", we only provide an > empty -- always failing -- stub for sscanf(), however. >=20 > Cc: David Woodhouse > Cc: Jian J Wang > Cc: Jiaxin Wu > Cc: Sivaraman Nainar > Cc: Xiaoyu Lu > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3D960 > CVE: CVE-2019-14553 > Suggested-by: David Woodhouse > Signed-off-by: Laszlo Ersek > --- >=20 > Notes: > v2: > - new patch >=20 > CryptoPkg/Library/TlsLib/TlsConfig.c | 28 +++++++++++++++++--- > 1 file changed, 24 insertions(+), 4 deletions(-) >=20 > diff --git a/CryptoPkg/Library/TlsLib/TlsConfig.c > b/CryptoPkg/Library/TlsLib/TlsConfig.c > index 2bf5aee7c093..307eb57896dc 100644 > --- a/CryptoPkg/Library/TlsLib/TlsConfig.c > +++ b/CryptoPkg/Library/TlsLib/TlsConfig.c > @@ -516,22 +516,42 @@ TlsSetVerifyHost ( > IN UINT32 Flags, > IN CHAR8 *HostName > ) > { > - TLS_CONNECTION *TlsConn; > + TLS_CONNECTION *TlsConn; > + X509_VERIFY_PARAM *VerifyParam; > + UINTN BinaryAddressSize; > + UINT8 BinaryAddress[MAX (NS_INADDRSZ, NS_IN6ADDRSZ)]; > + INTN ParamStatus; >=20 > TlsConn =3D (TLS_CONNECTION *) Tls; > if (TlsConn =3D=3D NULL || TlsConn->Ssl =3D=3D NULL || HostName =3D=3D= NULL) { > return EFI_INVALID_PARAMETER; > } >=20 > SSL_set_hostflags(TlsConn->Ssl, Flags); >=20 > - if (SSL_set1_host(TlsConn->Ssl, HostName) =3D=3D 0) { > - return EFI_ABORTED; > + VerifyParam =3D SSL_get0_param (TlsConn->Ssl); > + ASSERT (VerifyParam !=3D NULL); > + > + BinaryAddressSize =3D 0; > + if (inet_pton (AF_INET6, HostName, BinaryAddress) =3D=3D 1) { > + BinaryAddressSize =3D NS_IN6ADDRSZ; > + } else if (inet_pton (AF_INET, HostName, BinaryAddress) =3D=3D 1) { > + BinaryAddressSize =3D NS_INADDRSZ; > + } > + > + if (BinaryAddressSize > 0) { > + DEBUG ((DEBUG_VERBOSE, "%a:%a: parsed \"%a\" as an IPv%c address " > + "literal\n", gEfiCallerBaseName, __FUNCTION__, HostName, > + (UINTN)((BinaryAddressSize =3D=3D NS_IN6ADDRSZ) ? '6' : '4'))); > + ParamStatus =3D X509_VERIFY_PARAM_set1_ip (VerifyParam, BinaryAddres= s, > + BinaryAddressSize); > + } else { > + ParamStatus =3D X509_VERIFY_PARAM_set1_host (VerifyParam, HostName, = 0); > } >=20 > - return EFI_SUCCESS; > + return (ParamStatus =3D=3D 1) ? EFI_SUCCESS : EFI_ABORTED; > } >=20 > /** > Sets a TLS/SSL session ID to be used during TLS/SSL connect. > -- > 2.19.1.3.g30247aa5d201 >=20