From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.88; helo=mga01.intel.com; envelope-from=qin.long@intel.com; receiver=edk2-devel@lists.01.org Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 7BB3D2270D350 for ; Tue, 10 Apr 2018 10:06:44 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 10 Apr 2018 10:06:44 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.48,433,1517904000"; d="scan'208,217";a="36082548" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by fmsmga002.fm.intel.com with ESMTP; 10 Apr 2018 10:06:43 -0700 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.319.2; Tue, 10 Apr 2018 10:06:43 -0700 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.151]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.197]) with mapi id 14.03.0319.002; Wed, 11 Apr 2018 01:06:41 +0800 From: "Long, Qin" To: Laszlo Ersek , "Wu, Jiaxin" , edk2-devel-01 CC: Ard Biesheuvel , "Ye, Ting" , "Justen, Jordan L" , "Gao, Liming" , Gary Ching-Pang Lin , "Kinney, Michael D" , "Fu, Siyuan" Thread-Topic: [edk2] [PATCH 00/13] {Ovmf, Mde, Network, Crypto}Pkg: fixes+features for setting HTTPS cipher suites Thread-Index: AQHT0LD8WFM9C73rSkaJAp7V2qYb2qP6OGvw Date: Tue, 10 Apr 2018 17:06:41 +0000 Message-ID: References: <20180403145149.8925-1-lersek@redhat.com> <895558F6EA4E3B41AC93A00D163B7274163B458F@SHSMSX103.ccr.corp.intel.com> In-Reply-To: Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMzEwZThjMDAtYTc5ZC00YTBlLWE3MjgtMjJjYTBjZjQ1MTRhIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjIuNS4xOCIsIlRydXN0ZWRMYWJlbEhhc2giOiJMMktQNVRpWEVVSkZkeEtSYXhFamkrU0RmbnpjTjVVMkRGXC9HTEZBa2JcL2lCaTZtMGN4Rk8rcjB2Q3ZxdEZuOEQifQ== x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 X-Content-Filtered-By: Mailman/MimeDel 2.1.26 Subject: Re: [PATCH 00/13] {Ovmf, Mde, Network, Crypto}Pkg: fixes+features for setting HTTPS cipher suites X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 10 Apr 2018 17:06:44 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi, Laszlo, I prefer to open a separate BZ for this TlsCipherMappingTable update. Current list was produced by some rough collections from Jiaxin and me, whi= ch meet the basic cipher requirement for TLS(v1.0/1.1/1.2) to set up one su= ccessful connection. Will re-sorted this table based on IANA & IETF-RFCs & EDKII-openssl build o= ptions. Best Regards & Thanks, LONG, Qin From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Lasz= lo Ersek Sent: Tuesday, April 10, 2018 5:48 PM To: Wu, Jiaxin ; edk2-devel-01 Cc: Ard Biesheuvel ; Ye, Ting ; Justen, Jordan L ; Gao, Liming ; Gary Ching-Pang Lin ; Long, Qin ; Kinney, Michael D ; Fu, Siyuan Subject: Re: [edk2] [PATCH 00/13] {Ovmf, Mde, Network, Crypto}Pkg: fixes+fe= atures for setting HTTPS cipher suites On 04/10/18 06:09, Wu, Jiaxin wrote: > Hi Laszlo > > Appreciate your contribution. I have reviewed the series patches you atta= ched here. First, I assume you have verified the patches on OVMF and the fu= nctionality works well, That's right; I tested cipher suite negotiation failures and successes. For example, I configured apache to "Disable All SSL and TLS Protocols Except TLS 1 and Up" , and then I verified that HTTPS boot would succeed vs. fail dependent on whether I passed strong ciphers too, or only weak ones, to OVMF. > then, below are my comments: > > 1. The patches for OvmfPkg/NetworkPkg (0001-0004) are good to me. Thanks. For patch #2, "MdePkg/Include/Protocol/Tls.h: pack structures from the TLS RFC", we exchanged some points with Liming earlier: http://mid.mail-archive.com/4A89E2EF3DFEDB4C8BFDE51014F606A14E1F1B10@SHSMSX= 104.ccr.corp.intel.com (Please see also my response to that.) I see that both you and Siyuan are OK with patch #2, i.e. with the separate #pragma directives. I'd also like Liming to confirm that he accepts the patch as-is. > > 2. For CryptoPkg, the major viewpoint is also related to the TlsCipherMap= pingTable. For this table, only include the supported ciphersuites looks mo= re reasonable. OK. I think this means that I should preserve patches #5 through #7, and drop patches #8 ('CryptoPkg/TlsLib: add the "TlsMappingTable.sh" POSIX shell script') and #9 ('CryptoPkg/TlsLib: extend "TlsCipherMappingTable"'). Is that correct? > After talked with Qin, I know the unsupported cipher suites won't be reje= cted/filtrated by the OpenSSL cipher list setting, if so, the cipher suite = list that passed from the client to the server in the ClientHello message m= ight also include such unsupported cipher suites. In such a case, the failu= re will happen once the server select the unsupported cipher suite. From th= e handshake process view, it's unreasonable since the client sent the desir= ed cipher suites, then the server selected one of them but still met the er= ror. Oh! You are totally right. I apologize for missing this -- I didn't realize this from Qin's comments on TianoCore #915. In other words, it is actually *important* that "TlsCipherMappingTable" match the cipher suites that we build into edk2. I understand now. Thanks! > Anyway, filtrating the unsupported cipher suites as early as possible is = a wise choice. So, TlsCipherMappingTable should only include the supported = cipher suites by reference the security requirement of CryptoPkg. Yes. > > 3. For patch 0006, it's good to me to optimize the searching algorithm si= nce the table is larger than before. > > 4. Can we combined some patches together to make the things simple? e.g. = Patches 0005/0007/0010/0011/0012/0013. Those patches are the same purpose t= o fix the issues in 0013. I'm not against squashing these patches together, but separating patch #6 (the binary search) out of the middle is not possible without a rewrite of that patch, because it has context dependencies on patch #5. Do you want me to do that? I.e., first implement the binary search for TlsGetCipherString() -- without changing the interface --, and *then* switch it over to TlsGetCipherMapping(), as part of the large squashed patch? > > 5. For patch 0008, I think it's unnecessary to provide such script. I pre= fer to maintain the TlsCipherMappingTable more statical since it's the inte= rnal mapping table. How about we keep it as internal assistant tool? Sure, given that TlsCipherMappingTable *must* match the ciphers that we build into OpensslLib, updating that table semi-automatically is out of question (it can only be done manually, in synch with OpensslLib.inf changes). Therefore the shell script is useless. I'll just drop the patch. > > 6. For patch 0009 to extend the TlsCipherMappingTable, I think Qin can he= lp us to provide the supported cipher suites by reference the security requ= irement of CryptoPkg. Right -- I think I'll simply drop patch #9 from the v2 series, and I'll let Qin post a separate patch later, so that TlsCipherMappingTable matches the ciphers we build in 100%. Qin, do you prefer that I open a separate TianoCore BZ for this? Also, would you like to post the patch for TlsCipherMappingTable before or after my v2? Thanks! Laszlo _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel