public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Wu, Jiaxin" <jiaxin.wu@intel.com>
To: "Long, Qin" <qin.long@intel.com>,
	"Palmer, Thomas" <thomas.palmer@hpe.com>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "ard.biesheuvel@linaro.org" <ard.biesheuvel@linaro.org>,
	"Ye, Ting" <ting.ye@intel.com>,
	"ronald.cron@arm.com" <ronald.cron@arm.com>,
	"glin@suse.com" <glin@suse.com>,
	"lersek@redhat.com" <lersek@redhat.com>
Subject: Re: [PATCH v1 9/9] CryptoPkg/TlsLib: Update TLS Wrapper Library to align with OpenSSL changes.
Date: Thu, 23 Mar 2017 01:20:54 +0000	[thread overview]
Message-ID: <895558F6EA4E3B41AC93A00D163B7274162A57BA@SHSMSX103.ccr.corp.intel.com> (raw)
In-Reply-To: <BF2CCE9263284D428840004653A28B6E53F70280@SHSMSX103.ccr.corp.intel.com>

Hi Thomas,

I agree with the update for TlsSetVersion/TlsCtxNew. But for TlsSetVersion, we should use SSL_set_min_proto_version instead of  SSL_CTX_set_min_proto_version to avoid the SSL CONTEXT change directly.

Thanks,
Jiaxin

> -----Original Message-----
> From: Long, Qin
> Sent: Wednesday, March 22, 2017 9:32 AM
> To: Palmer, Thomas <thomas.palmer@hpe.com>; edk2-devel@lists.01.org
> Cc: ard.biesheuvel@linaro.org; Ye, Ting <ting.ye@intel.com>;
> ronald.cron@arm.com; Wu, Jiaxin <jiaxin.wu@intel.com>; glin@suse.com;
> lersek@redhat.com
> Subject: RE: [edk2] [PATCH v1 9/9] CryptoPkg/TlsLib: Update TLS Wrapper
> Library to align with OpenSSL changes.
> 
> Thomas,
> 
> Thanks for the comments. I will check this with Jiaxin, and make the possible
> updates in V2.
> 
> 
> Best Regards & Thanks,
> LONG, Qin
> 
> > -----Original Message-----
> > From: Palmer, Thomas [mailto:thomas.palmer@hpe.com]
> > Sent: Wednesday, March 22, 2017 1:43 AM
> > To: Long, Qin; edk2-devel@lists.01.org
> > Cc: ard.biesheuvel@linaro.org; Ye, Ting; ronald.cron@arm.com; Wu, Jiaxin;
> > glin@suse.com; lersek@redhat.com
> > Subject: RE: [edk2] [PATCH v1 9/9] CryptoPkg/TlsLib: Update TLS Wrapper
> > Library to align with OpenSSL changes.
> >
> > Qin,
> >
> > Please update TlsSetVersion to use SSL_CTX_set_min_proto_version and
> > SSL_CTX_set_max_proto_version  in the switch statement.  We do not
> want
> > auto-negotitate but only to restrict to a particular version.
> >
> > Also, lets update TlsCtxNew to use only SSL_CTX_set_min_proto_version.
> > TlsCtxNew will auto-negotiate, but the version provided will put in a lower
> > floor to what is allowed.
> >
> > Regards,
> >
> > Thomas Palmer
> >
> > "I have only made this letter longer because I have not had the time to
> > make it shorter" - Blaise Pascal
> >
> >
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > Qin Long
> > Sent: Tuesday, March 21, 2017 10:56 AM
> > To: edk2-devel@lists.01.org
> > Cc: ard.biesheuvel@linaro.org; ting.ye@intel.com; ronald.cron@arm.com;
> > jiaxin.wu@intel.com; glin@suse.com; lersek@redhat.com
> > Subject: [edk2] [PATCH v1 9/9] CryptoPkg/TlsLib: Update TLS Wrapper
> Library
> > to align with OpenSSL changes.
> >
> > This patch update the wrapper implementation in TlsLib to align with the
> > latest OpenSSL-1.1.0xx API changes.
> >
> > Cc: Jiaxin Wu <jiaxin.wu@intel.com>
> > Cc: Ting Ye <ting.ye@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Cc: Gary Lin <glin@suse.com>
> > Cc: Ronald Cron <ronald.cron@arm.com>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Qin Long <qin.long@intel.com>
> > ---
> >  CryptoPkg/Library/TlsLib/InternalTlsLib.h |  6 +++++-
> >  CryptoPkg/Library/TlsLib/TlsConfig.c      | 21 +++++++++++++--------
> >  CryptoPkg/Library/TlsLib/TlsInit.c        | 19 ++++++++++---------
> >  3 files changed, 28 insertions(+), 18 deletions(-)
> >
> > diff --git a/CryptoPkg/Library/TlsLib/InternalTlsLib.h
> > b/CryptoPkg/Library/TlsLib/InternalTlsLib.h
> > index e75146648d..f3a662afea 100644
> > --- a/CryptoPkg/Library/TlsLib/InternalTlsLib.h
> > +++ b/CryptoPkg/Library/TlsLib/InternalTlsLib.h
> > @@ -1,7 +1,7 @@
> >  /** @file
> >    Internal include file for TlsLib.
> >
> > -Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
> > +Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.<BR>
> >  This program and the accompanying materials  are licensed and made
> > available under the terms and conditions of the BSD License  which
> > accompanies this distribution.  The full text of the license may be found at
> > @@ -15,6 +15,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF
> ANY
> > KIND, EITHER EXPRESS OR IMPLIED.
> >  #ifndef __INTERNAL_TLS_LIB_H__
> >  #define __INTERNAL_TLS_LIB_H__
> >
> > +#undef _WIN32
> > +#undef _WIN64
> > +#undef _MSC_VER
> > +
> >  #include <Library/BaseCryptLib.h>
> >  #include <openssl/ssl.h>
> >  #include <openssl/bio.h>
> > diff --git a/CryptoPkg/Library/TlsLib/TlsConfig.c
> > b/CryptoPkg/Library/TlsLib/TlsConfig.c
> > index f103da4321..3586be3945 100644
> > --- a/CryptoPkg/Library/TlsLib/TlsConfig.c
> > +++ b/CryptoPkg/Library/TlsLib/TlsConfig.c
> > @@ -128,24 +128,30 @@ TlsSetVersion (
> >
> >    ProtoVersion = (MajorVer << 8) | MinorVer;
> >
> > +  //
> > +  // Using the general-purpose version-flexible SSL/TLS methods here.
> > +  // The actual protocol version used in OpenSSL-1.1.xx will be
> > + negoriated  // to the highest version mutually supported by the client and
> > server.
> > +  // Old TLSv1_x_method() was marked as deprecated.
> > +  //
> >    switch (ProtoVersion) {
> >    case TLS1_VERSION:
> >      //
> >      // TLS 1.0
> >      //
> > -    SSL_set_ssl_method (TlsConn->Ssl, TLSv1_method ());
> > +    SSL_set_ssl_method (TlsConn->Ssl, TLS_method ());
> >      break;
> >    case TLS1_1_VERSION:
> >      //
> >      // TLS 1.1
> >      //
> > -    SSL_set_ssl_method (TlsConn->Ssl, TLSv1_1_method ());
> > +    SSL_set_ssl_method (TlsConn->Ssl, TLS_method ());
> >      break;
> >    case TLS1_2_VERSION:
> >      //
> >      // TLS 1.2
> >      //
> > -    SSL_set_ssl_method (TlsConn->Ssl, TLSv1_2_method ());
> > +    SSL_set_ssl_method (TlsConn->Ssl, TLS_method ());
> >      break;
> >    default:
> >      //
> > @@ -384,8 +390,7 @@ TlsSetSessionId (
> >      return EFI_UNSUPPORTED;
> >    }
> >
> > -  Session->session_id_length = SessionIdLen;
> > -  CopyMem (Session->session_id, SessionId, Session->session_id_length);
> > +  SSL_SESSION_set1_id (Session, (const unsigned char *)SessionId,
> > + SessionIdLen);
> >
> >    return EFI_SUCCESS;
> >  }
> > @@ -847,7 +852,7 @@ TlsGetClientRandom (
> >      return;
> >    }
> >
> > -  CopyMem (ClientRandom, TlsConn->Ssl->s3->client_random,
> > SSL3_RANDOM_SIZE);
> > +  SSL_get_client_random (TlsConn->Ssl, ClientRandom,
> > SSL3_RANDOM_SIZE);
> >  }
> >
> >  /**
> > @@ -876,7 +881,7 @@ TlsGetServerRandom (
> >      return;
> >    }
> >
> > -  CopyMem (ServerRandom, TlsConn->Ssl->s3->server_random,
> > SSL3_RANDOM_SIZE);
> > +  SSL_get_server_random (TlsConn->Ssl, ServerRandom,
> > SSL3_RANDOM_SIZE);
> >  }
> >
> >  /**
> > @@ -916,7 +921,7 @@ TlsGetKeyMaterial (
> >      return EFI_UNSUPPORTED;
> >    }
> >
> > -  CopyMem (KeyMaterial, Session->master_key, Session-
> > >master_key_length);
> > +  SSL_SESSION_get_master_key (Session, KeyMaterial,
> > + SSL3_MASTER_SECRET_SIZE);
> >
> >    return EFI_SUCCESS;
> >  }
> > diff --git a/CryptoPkg/Library/TlsLib/TlsInit.c
> > b/CryptoPkg/Library/TlsLib/TlsInit.c
> > index 6b1fd93ea9..d7b8899ac2 100644
> > --- a/CryptoPkg/Library/TlsLib/TlsInit.c
> > +++ b/CryptoPkg/Library/TlsLib/TlsInit.c
> > @@ -1,7 +1,7 @@
> >  /** @file
> >    SSL/TLS Initialization Library Wrapper Implementation over OpenSSL.
> >
> > -Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
> > +Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.<BR>
> >  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>  This
> > program and the accompanying materials  are licensed and made available
> > under the terms and conditions of the BSD License @@ -33,14 +33,10 @@
> > TlsInitialize (
> >    // Performs initialization of crypto and ssl library, and loads required
> >    // algorithms.
> >    //
> > -  SSL_library_init ();
> > -
> > -  //
> > -  // Loads error strings from both crypto and ssl library.
> > -  //
> > -  SSL_load_error_strings ();
> > -
> > -  /// OpenSSL_add_all_algorithms();
> > +  OPENSSL_init_ssl (
> > +    OPENSSL_INIT_LOAD_SSL_STRINGS |
> > OPENSSL_INIT_LOAD_CRYPTO_STRINGS,
> > +    NULL
> > +    );
> >
> >    //
> >    // Initialize the pseudorandom number generator.
> > @@ -220,6 +216,11 @@ TlsNew (
> >    }
> >
> >    //
> > +  // This retains compatibility with previous version of OpenSSL.
> > +  //
> > +  SSL_set_security_level (TlsConn->Ssl, 0);
> > +
> > +  //
> >    // Initialize the created SSL Object
> >    //
> >    SSL_set_info_callback (TlsConn->Ssl, NULL);
> > --
> > 2.11.1.windows.1
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel



  reply	other threads:[~2017-03-23  1:20 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-21 15:56 [PATCH v1 0/9] *** Upgrade CryptoPkg to use the latest OpenSSL 1.1.0xx/stable release *** Qin Long
2017-03-21 15:56 ` [PATCH v1 1/9] CryptoPkg/OpensslLib: Update INF files to support OpenSSL-1.1.0xx build Qin Long
2017-03-22 12:02   ` Laszlo Ersek
2017-03-22 12:18   ` Laszlo Ersek
2017-03-21 15:56 ` [PATCH v1 2/9] CryptoPkg/OpensslLib: Remove patch file and installation scripts Qin Long
2017-03-22 12:05   ` Laszlo Ersek
2017-03-21 15:56 ` [PATCH v1 3/9] CryptoPkg: Fix handling of &strcmp function pointers Qin Long
2017-03-22 10:11   ` Gary Lin
2017-03-23  2:16     ` Long, Qin
2017-03-23  3:39       ` Long, Qin
2017-03-21 15:56 ` [PATCH v1 4/9] CryptoPkg/OpensslLib: Use new Perl script for file list generation Qin Long
2017-03-21 15:56 ` [PATCH v1 5/9] CryptoPkg: Clean-up CRT Library Wrapper Qin Long
2017-03-21 15:56 ` [PATCH v1 6/9] CryptoPkg: Add extra build option to disable VS build warning Qin Long
2017-03-21 15:56 ` [PATCH v1 7/9] CryptoPkg: Update HMAC Wrapper implementation with opaque HMAC_CTX object Qin Long
2017-03-21 15:56 ` [PATCH v1 8/9] CryptoPkg: Update PK Ciphers Wrapper Implementations work with opaque objects Qin Long
2017-03-21 15:56 ` [PATCH v1 9/9] CryptoPkg/TlsLib: Update TLS Wrapper Library to align with OpenSSL changes Qin Long
2017-03-21 17:42   ` Palmer, Thomas
2017-03-22  1:32     ` Long, Qin
2017-03-23  1:20       ` Wu, Jiaxin [this message]
2017-03-23 16:23         ` Palmer, Thomas
2017-03-22  2:22 ` [PATCH v1 0/9] *** Upgrade CryptoPkg to use the latest OpenSSL 1.1.0xx/stable release *** Gao, Liming
2017-03-22  2:44   ` Long, Qin
2017-03-22 13:02 ` Laszlo Ersek
2017-03-22 16:20   ` Long, Qin

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=895558F6EA4E3B41AC93A00D163B7274162A57BA@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