public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Wu, Jiaxin" <jiaxin.wu@intel.com>
To: "Palmer, Thomas" <thomas.palmer@hpe.com>,
	"Long, Qin" <qin.long@intel.com>,
	"edk2-devel@lists.01.org" <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
Date: Tue, 2 Aug 2016 02:03:02 +0000	[thread overview]
Message-ID: <895558F6EA4E3B41AC93A00D163B7274137C3471@SHSMSX103.ccr.corp.intel.com> (raw)
In-Reply-To: <CS1PR84MB015148946925A712F2E92DA6ED050@CS1PR84MB0151.NAMPRD84.PROD.OUTLOOK.COM>

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-52r1
> > .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


  reply	other threads:[~2016-08-02  2:03 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1468475478-145272-1-git-send-email-jiaxin.wu@intel.com>
2016-07-29 22:02 ` [staging/HTTPS-TLS][PATCH 0/4] Replace the TLS definitions with the standardized one Palmer, Thomas
2016-08-01  1:22   ` Wu, Jiaxin
2016-08-01  1:48     ` Long, Qin
2016-08-01 21:45       ` Palmer, Thomas
2016-08-02  1:34         ` Wu, Jiaxin
2016-08-02  1:51           ` Palmer, Thomas
2016-08-02  2:03             ` Wu, Jiaxin [this message]
2016-08-02  3:46               ` Long, Qin
2016-08-02 21:14               ` Palmer, Thomas
2016-08-03  3:27                 ` Wu, Jiaxin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=895558F6EA4E3B41AC93A00D163B7274137C3471@SHSMSX103.ccr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox