Re: [staging/HTTPS-TLS][PATCH 0/4] Replace the TLS definitions with the standardized one

["Wu, Jiaxin" <jiaxin.wu@intel.com>]

Agree to remove the below cipher sets to make it consistent with current openssl configuration.  

IDEA-CBC-SHA
DHE-DSS-AES256-SHA
DHE-DSS-AES256-SHA256
DHE-DSS-AES128-SHA
DHE-DSS-AES128-SHA256
EDH-DSS-DES-CBC3-SHA
DES-CBC-SHA

Thanks,
Jiaxin

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Palmer, Thomas
> Sent: Wednesday, August 3, 2016 5:14 AM
> To: Wu, Jiaxin <jiaxin.wu@intel.com>; Long, Qin <qin.long@intel.com>;
> edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Gao,
> Liming <liming.gao@intel.com>
> Subject: Re: [edk2] [staging/HTTPS-TLS][PATCH 0/4] Replace the TLS
> definitions with the standardized one
> 
> I have two sets of lists,  one for the ciphers the OpenSSL by default
> configures in a new CTX and the other after setting all ciphers available in the
> mapping table.  For both sets I show the affect of removing no-idea/no-dsa
> and adding enable-weak-ciphers
> 
> These are the ciphers that are supported by TLS immediately after a
> TLS_CTX_new operation with current OpenSSL config (34):
> AES128-GCM-SHA256
> AES128-SHA
> AES128-SHA256
> AES256-GCM-SHA384
> AES256-SHA
> AES256-SHA256
> DES-CBC3-SHA
> DH-DSS-AES128-GCM-SHA256
> DH-DSS-AES128-SHA
> DH-DSS-AES128-SHA256
> DH-DSS-AES256-GCM-SHA384
> DH-DSS-AES256-SHA
> DH-DSS-AES256-SHA256
> DH-DSS-DES-CBC3-SHA
> DHE-RSA-AES128-GCM-SHA256
> DHE-RSA-AES128-SHA
> DHE-RSA-AES128-SHA256
> DHE-RSA-AES256-GCM-SHA384
> DHE-RSA-AES256-SHA
> DHE-RSA-AES256-SHA256
> DH-RSA-AES128-GCM-SHA256
> DH-RSA-AES128-SHA
> DH-RSA-AES128-SHA256
> DH-RSA-AES256-GCM-SHA384
> DH-RSA-AES256-SHA
> DH-RSA-AES256-SHA256
> DH-RSA-DES-CBC3-SHA
> EDH-RSA-DES-CBC3-SHA
> PSK-3DES-EDE-CBC-SHA
> PSK-AES128-CBC-SHA
> PSK-AES256-CBC-SHA
> PSK-RC4-SHA
> RC4-MD5
> RC4-SHA
> 
> By removing "no-idea" in process_files we gain (1):
> IDEA-CBC-SHA
> 
> By removing "no-dsa" in process_files we gain (7):
> DHE-DSS-AES128-GCM-SHA256
> DHE-DSS-AES128-SHA
> DHE-DSS-AES128-SHA256
> DHE-DSS-AES256-GCM-SHA384
> DHE-DSS-AES256-SHA
> DHE-DSS-AES256-SHA256
> EDH-DSS-DES-CBC3-SHA
> 
> We do not gain any more ciphers with enable-weak-ssl-ciphers at this point.
> 
> 
> Now here are the ciphers after TlsSetCipherList has been run with setting all
> ciphers currently in TlsCipherMappingTable.
> With original OpenSSL configuration (23):
> AES128-SHA
> AES128-SHA256
> AES256-SHA
> AES256-SHA256
> DES-CBC3-SHA
> DH-DSS-AES128-SHA
> DH-DSS-AES128-SHA256
> DH-DSS-AES256-SHA
> DH-DSS-AES256-SHA256
> DHE-RSA-AES128-SHA
> DHE-RSA-AES128-SHA256
> DHE-RSA-AES256-SHA
> DHE-RSA-AES256-SHA256
> DH-RSA-AES128-SHA
> DH-RSA-AES128-SHA256
> DH-RSA-AES256-SHA
> DH-RSA-AES256-SHA256
> EDH-RSA-DES-CBC3-SHA
> NULL-MD5
> NULL-SHA
> NULL-SHA256
> RC4-MD5
> RC4-SHA
> 
> By removing "no-idea" in process_files we gain (1):
> IDEA-CBC-SHA
> 
> By removing "no-dsa" in process_files we gain (5):
> DHE-DSS-AES256-SHA
> DHE-DSS-AES256-SHA256
> DHE-DSS-AES128-SHA
> DHE-DSS-AES128-SHA256
> EDH-DSS-DES-CBC3-SHA
> 
> Be adding enable-weak-ssl-ciphers we gain (1):
> DES-CBC-SHA
> 
> 
> Thomas
> 
> -----Original Message-----
> From: Wu, Jiaxin [mailto:jiaxin.wu@intel.com]
> Sent: Monday, August 1, 2016 9:03 PM
> To: Palmer, Thomas <thomas.palmer@hpe.com>; Long, Qin
> <qin.long@intel.com>; edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Gao,
> Liming <liming.gao@intel.com>
> Subject: RE: [staging/HTTPS-TLS][PATCH 0/4] Replace the TLS definitions with
> the standardized one
> 
> Thomas,
> 
> Thanks your effort to test the new ciphers, can you provide the info which
> one is unsupported currently?
> 
> As Qin's comments, "we'd better to keep the current supported cipher suite
> for our UEFI- TLS enabling". If so, I agree to remove the unsupported one in
> TlsCipherMappingTable instead of changing the current openssl configuration.
> If dsa /idea is required in future, then we can consider how to enable the
> configuration.
> 
> So, can you provide the patch to remove the unsupported one in
> TlsCipherMappingTable?
> 
> 
> Thanks,
> Jiaxin
> 
> 
> 
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > Palmer, Thomas
> > Sent: Tuesday, August 2, 2016 9:51 AM
> > To: Wu, Jiaxin <jiaxin.wu@intel.com>; Long, Qin <qin.long@intel.com>;
> > edk2-devel@lists.01.org
> > Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>;
> > Gao, Liming <liming.gao@intel.com>
> > Subject: Re: [edk2] [staging/HTTPS-TLS][PATCH 0/4] Replace the TLS
> > definitions with the standardized one
> >
> >
> > Hi Jiaxin,
> >
> > 	It sounds like we both agree that TlsCipherMappingTable is the list
> > of what UEFI officially supports.  If it is advertised in
> > TlsCipherMappingTable then OpenSSL needs to support it.
> >
> > 	My proposal of removing no-dsa / no-idea and adding weak-ciphers
> is
> > specifically aimed to syncing how OpenSSL is configured/built to what
> > is in TlsCipherMappingTable.  I was busy last week testing out the new
> > ciphers and realized a few were not even getting configured in OpenSSL.
> >
> > Thomas
> >
> > -----Original Message-----
> > From: Wu, Jiaxin [mailto:jiaxin.wu@intel.com]
> > Sent: Monday, August 1, 2016 8:34 PM
> > To: Palmer, Thomas <thomas.palmer@hpe.com>; Long, Qin
> > <qin.long@intel.com>; edk2-devel@lists.01.org
> > Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>;
> > Gao, Liming <liming.gao@intel.com>
> > Subject: RE: [staging/HTTPS-TLS][PATCH 0/4] Replace the TLS
> > definitions with the standardized one
> >
> > Hi Thomas,
> >
> > Since the Tls1.h is used to hold the standardized definitions, openssl
> > part is not taken into consideration. The Cipher Suites added in
> > Tls1.h only refers to
> > A.5 of rfc-2246, rfc-4346 and rfc-5246. The criteria is removing all
> > the limited/insecurity/deprecated ones that specified in RFC -- "Note
> > that this mode is vulnerable to man-in-the middle attacks and is
> > therefore deprecated." I know the IDEA and DES cipher suites are also
> > deprecated in TLS1.2, but it does means to TLS1.1. So, some of them are
> still kept in Tls1.h.
> >
> > As for the TlsCipherMappingTable, it takes on the link between Tls1.h
> > defined cipher suites and openssl supported cipher suites. If we
> > eliminate the factors of configuration, I believe the cipher suites in
> > TlsCipherMappingTable should be implemented in OpenSSL lib. I haven't
> > check them one by one but openssl official document is referred @
> > https://www.openssl.org/docs/manmaster/apps/ciphers.html, which
> gives
> > the lists of TLS cipher suites names from the relevant specification
> > and their OpenSSL equivalents.
> >
> > If the cipher suites in Tls1.h is not found in TlsCipherMappingTable,
> > EFI_UNSUPPORTED will be returned. I think it's reasonable.
> >
> > Thanks,
> > Jiaxin
> >
> > > -----Original Message-----
> > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf
> > > Of Palmer, Thomas
> > > Sent: Tuesday, August 2, 2016 5:46 AM
> > > To: Long, Qin <qin.long@intel.com>; Wu, Jiaxin
> > > <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> > > Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>;
> > > Gao, Liming <liming.gao@intel.com>
> > > Subject: Re: [edk2] [staging/HTTPS-TLS][PATCH 0/4] Replace the TLS
> > > definitions with the standardized one
> > >
> > > Jiaxin / Qin,
> > >
> > >
> > > 	I'm unaware of what criteria is required for a cipher to be in this
> > > TlsCipherMappingTable.  I had presumed that it would be b/c UEFI
> > > supported the cipher for TLS operation.  If unsupported ciphers are
> > > allowed ... then logically wouldn't we need to add all ciphers?
> > > What advantage do we gain by having an entry in this table but not
> > > actually use
> > the cipher in communication?
> > >
> > > 	Currently TlsGetCipherString is the only means we have to validate
> > > the cipher string.  If a cipher is in the table but not in OpenSSL
> > > lib, then we will provide imperfect feedback if the unsupported
> > > cipher is buried in a list of supported ciphers.  OpenSSL will
> > > simply use only the ciphers it supports and quietly drop the unsupported
> cipher.
> > > A user that inspects the list of set ciphers would be curious why a
> > > certain
> > cipher was being "dropped" /
> > > "filtered".   However, if TlsGetCipherString sees that the cipher is not in
> our
> > > mapping table the TlsSetCipherList function will return immediate
> feedback.
> > >
> > > 	I'm not enthralled with supporting weak/idea ciphers either.  I
> > > would vouch for us removing those ciphers from
> > > TlsCipherMappingTable.  It is not our responsibility to document the
> > > IANA/Description string description in code.
> > >
> > > 	This document
> > > (http://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-52
> > > r1
> > > .pdf) would be a good list for initial cipher support.  We have some
> > > of the ciphers on the list already.  Here is the sorted list:
> > >
> > > TLS_DH_DSS_WITH_3DES_EDE_CBC_SHA
> > > TLS_DH_DSS_WITH_AES_128_CBC_SHA
> > > TLS_DH_DSS_WITH_AES_128_CBC_SHA256
> > > TLS_DH_DSS_WITH_AES_128_GCM_SHA256
> > > TLS_DH_DSS_WITH_AES_256_CBC_SHA
> > > TLS_DH_DSS_WITH_AES_256_CBC_SHA256
> > > TLS_DH_DSS_WITH_AES_256_GCM_SHA384
> > > TLS_DHE_DSS_WITH_3DES_EDE_CBC_SHA
> > > TLS_DHE_DSS_WITH_AES_128_CBC_SHA
> > > TLS_DHE_DSS_WITH_AES_128_CBC_SHA256
> > > TLS_DHE_DSS_WITH_AES_128_GCM_SHA256
> > > TLS_DHE_DSS_WITH_AES_256_CBC_SHA
> > > TLS_DHE_DSS_WITH_AES_256_CBC_SHA256
> > > TLS_DHE_DSS_WITH_AES_256_GCM_SHA384
> > > TLS_ECDH_ECDSA_WITH_3DES_EDE_CBC_SHA
> > > TLS_ECDH_ECDSA_WITH_AES_128_CBC_SHA
> > > TLS_ECDH_ECDSA_WITH_AES_128_CBC_SHA256
> > > TLS_ECDH_ECDSA_WITH_AES_128_GCM_SHA256
> > > TLS_ECDH_ECDSA_WITH_AES_256_CBC_SHA
> > > TLS_ECDH_ECDSA_WITH_AES_256_CBC_SHA384
> > > TLS_ECDH_ECDSA_WITH_AES_256_GCM_SHA384
> > > TLS_ECDHE_ECDSA_WITH_3DES_EDE_CBC_SHA
> > > TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA
> > > TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256
> > > TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256
> > > TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA
> > > TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384
> > > TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384
> > > TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA
> > > TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA
> > > TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256
> > > TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256
> > > TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA
> > > TLS_RSA_WITH_3DES_EDE_CBC_SHA
> > > TLS_RSA_WITH_AES_128_CBC_SHA
> > > TLS_RSA_WITH_AES_128_CBC_SHA256
> > > TLS_RSA_WITH_AES_128_CCM17
> > > TLS_RSA_WITH_AES_128_GCM_SHA256
> > > TLS_RSA_WITH_AES_256_CBC_SHA
> > > TLS_RSA_WITH_AES_256_CBC_SHA256
> > > TLS_RSA_WITH_AES_256_CCM
> > > TLS_RSA_WITH_AES_256_GCM_SHA384
> > >
> > > Thomas
> > >
> > > -----Original Message-----
> > > From: Long, Qin [mailto:qin.long@intel.com]
> > > Sent: Sunday, July 31, 2016 8:48 PM
> > > To: Wu, Jiaxin <jiaxin.wu@intel.com>; Palmer, Thomas
> > > <thomas.palmer@hpe.com>; edk2-devel@lists.01.org
> > > Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>;
> > > Gao, Liming <liming.gao@intel.com>
> > > Subject: RE: [staging/HTTPS-TLS][PATCH 0/4] Replace the TLS
> > > definitions with the standardized one
> > >
> > > I personally prefer to keep the current supported cipher suite for
> > > our
> > > UEFI- TLS enabling. We can have the full RFC definitions, and
> > > platform specific cipher sets for validation now. It's better to
> > > maintain one minimal scope in this phase.
> > >
> > > "enable-weak-ssl-ciphers" looks odd. Disabling weak ciphers is the
> > > recommendation for hardening SSL communications.
> > > For other ciphers (idea, dsa, etc), we can enable them step-by-step
> > > depending on the real requirements.
> > >
> > >
> > > Best Regards & Thanks,
> > > LONG, Qin
> > >
> > > > -----Original Message-----
> > > > From: Wu, Jiaxin
> > > > Sent: Monday, August 01, 2016 9:23 AM
> > > > To: Palmer, Thomas; Long, Qin; edk2-devel@lists.01.org
> > > > Cc: Ye, Ting; Fu, Siyuan; Gao, Liming
> > > > Subject: RE: [staging/HTTPS-TLS][PATCH 0/4] Replace the TLS
> > > > definitions with the standardized one
> > > >
> > > > Thomas,
> > > > I agree some of them are not supported due to the UEFI OpenSSL
> > > > configuration, but it doesn't affect those mapping relationship
> > > > added in the patch. So, I have no strong opinion whether to
> > > > support it by modifying the current OpenSSL configuration. Since
> > > > Qin is the OpenSSL expert, I'd like to hear his views.
> > > >
> > > > Qin,
> > > > What's your opinion?
> > > >
> > > > Thanks.
> > > > Jiaxin
> > > >
> > > > > -----Original Message-----
> > > > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On
> > > > > Behalf Of Palmer, Thomas
> > > > > Sent: Saturday, July 30, 2016 6:03 AM
> > > > > To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> > > > > Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan
> > > > > <siyuan.fu@intel.com>; Gao, Liming <liming.gao@intel.com>; Long,
> > > > > Qin <qin.long@intel.com>
> > > > > Subject: Re: [edk2] [staging/HTTPS-TLS][PATCH 0/4] Replace the
> > > > > TLS definitions with the standardized one
> > > > >
> > > > > Jiaxin,
> > > > >
> > > > > 	UEFI's OpenSSL library does not support all the ciphers that
> > > > > were added in your patch due to the UEFI configuration.  We need
> > > > > to remove
> > > > > "no- idea" and "no-dsa" from the process_files.sh and add
> > > > > "enable-weak-ssl- ciphers"
> > > > >
> > > > > 	While we are modifying process_files.sh, we can remove "no-
> > > > pqueue"
> > > > > from process_files.sh so that OpensslLib.inf is in sync.
> > > > >
> > > > > 	I can send out a patch to do so if you wish.
> > > > >
> > > > > Thomas
> > > > >
> > > > > -----Original Message-----
> > > > > From: Jiaxin Wu [mailto:jiaxin.wu@intel.com]
> > > > > Sent: Thursday, July 14, 2016 12:51 AM
> > > > > To: edk2-devel@lists.01.org
> > > > > Cc: Liming Gao <liming.gao@intel.com>; Palmer, Thomas
> > > > > <thomas.palmer@hpe.com>; Long Qin <qin.long@intel.com>; Ye Ting
> > > > > <ting.ye@intel.com>; Fu Siyuan <siyuan.fu@intel.com>; Wu Jiaxin
> > > > > <jiaxin.wu@intel.com>
> > > > > Subject: [staging/HTTPS-TLS][PATCH 0/4] Replace the TLS
> > > > > definitions with the standardized one
> > > > >
> > > > > The series patches are used to replace the TLS definitions with
> > > > > the standardized one. In addition, more TLS cipher suite mapping
> > > > > between Cipher Suite definitions and OpenSSL-used Cipher Suite
> > > > > name
> > are added.
> > > > >
> > > > > Cc: Liming Gao <liming.gao@intel.com>
> > > > > Cc: Palmer Thomas <thomas.palmer@hpe.com>
> > > > > Cc: Long Qin <qin.long@intel.com>
> > > > > Cc: Ye Ting <ting.ye@intel.com>
> > > > > Cc: Fu Siyuan <siyuan.fu@intel.com>
> > > > > Contributed-under: TianoCore Contribution Agreement 1.0
> > > > > Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> > > > > Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
> > > > >
> > > > > Jiaxin Wu (4):
> > > > >   MdePkg: Add a header to standardize TLS definitions
> > > > >   CryptoPkg: Add more TLS cipher suite mapping
> > > > >   NetworkPkg/TlsDxe: Replace the definitions with the
> > > > > standardized
> > one
> > > > >   NetworkPkg/HttpDxe: Replace the definitions with the
> > > > > standardized one
> > > > >
> > > > >  CryptoPkg/Library/TlsLib/TlsLib.c      | 3585 ++++++++++++++++-------
> --
> > ---
> > > --
> > > > --
> > > > >  MdePkg/Include/IndustryStandard/Tls1.h |   93 +
> > > > >  NetworkPkg/HttpDxe/HttpDriver.h        |    2 +
> > > > >  NetworkPkg/HttpDxe/HttpProto.c         |   12 +-
> > > > >  NetworkPkg/HttpDxe/HttpsSupport.c      |   22 +-
> > > > >  NetworkPkg/HttpDxe/HttpsSupport.h      |   44 -
> > > > >  NetworkPkg/TlsDxe/TlsImpl.c            |   56 +-
> > > > >  NetworkPkg/TlsDxe/TlsImpl.h            |   30 +-
> > > > >  NetworkPkg/TlsDxe/TlsProtocol.c        |    2 +-
> > > > >  9 files changed, 1945 insertions(+), 1901 deletions(-)  create
> > > > > mode
> > > > > 100644 MdePkg/Include/IndustryStandard/Tls1.h
> > > > >
> > > > > --
> > > > > 1.9.5.msysgit.1
> > > > >
> > > > > _______________________________________________
> > > > > 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
> > _______________________________________________
> > 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