Re: [PATCH v1 9/9] CryptoPkg/TlsLib: Update TLS Wrapper Library to align with OpenSSL changes.

["Palmer, Thomas" <thomas.palmer@hpe.com>]

Good catch, thanks!


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: Wu, Jiaxin [mailto:jiaxin.wu@intel.com] 
Sent: Wednesday, March 22, 2017 8:21 PM
To: Long, Qin <qin.long@intel.com>; 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; glin@suse.com; lersek@redhat.com
Subject: RE: [edk2] [PATCH v1 9/9] CryptoPkg/TlsLib: Update TLS Wrapper Library to align with OpenSSL changes.

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