Samer, I have merged the pull request, but seems it's not a clean way to contribution the patch (with another merge operation). I would like to commit the patch manually next time.
Thanks. Jiaxin > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of El- > Haj-Mahmoud, Samer > Sent: Wednesday, April 27, 2016 12:36 AM > To: Wu, Jiaxin <jiaxin...@intel.com>; Carsey, Jaben > <jaben.car...@intel.com> > Cc: edk2-devel@lists.01.org > Subject: Re: [edk2] [PATCH] CryptoPkg: Fix TLS Lib issue when certificate is > added to store multiple times > > Does not look like I have write permission to edk2-staging... I sent a > separate > email with question on the write access permissions for edk2-staging > > I also created a pull request against the edk2-staging/https-tls branch: > https://github.com/tianocore/edk2-staging/pull/1 > > > > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > Wu, Jiaxin > Sent: Monday, April 25, 2016 8:54 PM > To: El-Haj-Mahmoud, Samer <samer.el-haj-mahm...@hpe.com>; Carsey, > Jaben <jaben.car...@intel.com> > Cc: edk2-devel@lists.01.org > Subject: Re: [edk2] [PATCH] CryptoPkg: Fix TLS Lib issue when certificate is > added to store multiple times > > Please let me know whether you can commit the patch:). > > Thanks. > Jiaxin > > From: El-Haj-Mahmoud, Samer [mailto:samer.el-haj-mahm...@hpe.com] > Sent: Tuesday, April 26, 2016 9:41 AM > To: Carsey, Jaben <jaben.car...@intel.com>; Wu, Jiaxin > <jiaxin...@intel.com> > Cc: edk2-devel@lists.01.org; Palmer, Thomas <thomas.pal...@hpe.com> > Subject: RE: [edk2] [PATCH] CryptoPkg: Fix TLS Lib issue when certificate is > added to store multiple times > > Thanks for the prefix tip! Will try that next time. > > I don't know if I have write access to the branch. I will give it a try. > > > > > -----Original Message----- > From: Wu, Jiaxin [jiaxin...@intel.com] > Received: Monday, 25 Apr 2016, 7:43PM > To: Carsey, Jaben [jaben.car...@intel.com]; El-Haj-Mahmoud, Samer > [samer.el-haj-mahm...@hpe.com] > CC: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> [edk2- > de...@lists.01.org]; Palmer, Thomas [thomas.pal...@hpe.com] > Subject: RE: [edk2] [PATCH] CryptoPkg: Fix TLS Lib issue when certificate is > added to store multiple times Hi Samer, > > I think the prefix with " staging/branch][PATCH " should be good to the > staging patch subject. > > For example: > *git format-patch --subject-prefix="staging/HTTPS-TLS][PATCH"* > > Then, you will get the patch with "[staging/HTTPS-TLS][PATCH]" subject. > > The patch fix is good to me. Thanks for this contribution. > > Reviewed-by: Jiaxin Wu <jiaxin...@intel.com<mailto:jiaxin...@intel.com>> > > BTW, Do you commit the patch by yourself or need I help to check in the > patch? > > Best Regards! > Jiaxin > > > -----Original Message----- > > From: Carsey, Jaben > > Sent: Tuesday, April 26, 2016 4:00 AM > > To: El-Haj-Mahmoud, Samer > > <samer.el-haj-mahm...@hpe.com<mailto:samer.el-haj- > mahm...@hpe.com>> > > Cc: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Wu, > > Jiaxin <jiaxin...@intel.com<mailto:jiaxin...@intel.com>>; Palmer, > > Thomas <thomas.pal...@hpe.com<mailto:thomas.pal...@hpe.com>> > > Subject: Re: [edk2] [PATCH] CryptoPkg: Fix TLS Lib issue when > > certificate is added to store multiple times > > > > Ok. You can rb me, but I don't maintain that pkg so... > > > > Reviewed-by Jaben Carsey > > <jaben.car...@intel.com<mailto:jaben.car...@intel.com>> > > > > -Jaben > > > > > On Apr 25, 2016, at 11:17 AM, El-Haj-Mahmoud, Samer <samer.el-haj- > > mahm...@hpe.com<mailto:mahm...@hpe.com>> wrote: > > > > > > Thanks Jaben. I did notice this and tried to submit the patch with > > > the > > modified subject (using git send-email with both --subject and > > --compose), but that didn't work for some reason. > > > > > > Some answers below... > > > > > > > > > -----Original Message----- > > > From: Carsey, Jaben [mailto:jaben.car...@intel.com] > > > Sent: Monday, April 25, 2016 12:54 PM > > > To: El-Haj-Mahmoud, Samer > > > <samer.el-haj-mahm...@hpe.com<mailto:samer.el-haj- > mahm...@hpe.com>>; > > > edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> > > > Cc: El-Haj-Mahmoud, Samer > > > <samer.el-haj-mahm...@hpe.com<mailto:samer.el-haj- > mahm...@hpe.com>>; > > > Wu, > > Jiaxin > > > <jiaxin...@intel.com<mailto:jiaxin...@intel.com>>; Carsey, Jaben > > > <jaben.car...@intel.com<mailto:jaben.car...@intel.com>> > > > Subject: RE: [edk2] [PATCH] CryptoPkg: Fix TLS Lib issue when > > > certificate is added to store multiple times > > > > > > Samer, > > > > > > Note: This is not in the EDK2 yet, but in the staging... change > > > [PATCH] to [Staging/HTTPS-TLS PATCH] (or something like that per the > > > rules...) > > > > > > 2 questions inline also. > > > > > > -Jaben > > > > > >> -----Original Message----- > > >> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf > > >> Of Samer El-Haj-Mahmoud > > >> Sent: Monday, April 25, 2016 10:15 AM > > >> To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> > > >> Cc: Samer El-Haj-Mahmoud <el...@hpe.com<mailto:el...@hpe.com>>; > Wu, > > >> Jiaxin <jiaxin...@intel.com<mailto:jiaxin...@intel.com>> > > >> Subject: [edk2] [PATCH] CryptoPkg: Fix TLS Lib issue when > > >> certificate is added to store multiple times > > >> > > >> Removed unnecessary error condition in TLS Lib that that would > > >> report an error if a certificate is being added to the X509_STORE more > than once. > > >> This causes HTTPS to fail on second attempt with the same certificate. > > >> > > >> Contributed-under: TianoCore Contribution Agreement 1.0 > > >> Signed-off-by: Samer El-Haj-Mahmoud > > >> <el...@hpe.com<mailto:el...@hpe.com>> > > >> Signed-off-by: Thomas Palmer > > >> <thomas.pal...@hpe.com<mailto:thomas.pal...@hpe.com>> > > >> --- > > >> CryptoPkg/Library/TlsLib/TlsLib.c | 15 +++++++++++++-- > > >> 1 file changed, 13 insertions(+), 2 deletions(-) > > >> > > >> diff --git a/CryptoPkg/Library/TlsLib/TlsLib.c > > >> b/CryptoPkg/Library/TlsLib/TlsLib.c > > >> index e661375..0818653 100644 > > >> --- a/CryptoPkg/Library/TlsLib/TlsLib.c > > >> +++ b/CryptoPkg/Library/TlsLib/TlsLib.c > > >> @@ -2,6 +2,7 @@ > > >> SSL/TLS Library Wrapper Implementation over OpenSSL. > > >> > > >> Copyright (c) 2016, 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 which > > >> accompanies this distribution. The full text of the license may be > > >> found at @@ -16,6 +17,7 @@ WITHOUT WARRANTIES OR > > REPRESENTATIONS OF > > >> ANY KIND, EITHER EXPRESS OR IMPLIED. > > >> > > >> #include <openssl/ssl.h> > > >> #include <openssl/bio.h> > > >> +#include <openssl/err.h> > > >> > > >> #define MAX_BUFFER_SIZE 32768 > > >> > > >> @@ -1429,6 +1431,7 @@ TlsSetCaCertificate ( > > >> EFI_STATUS Status; > > >> TLS_CONNECTION *TlsConn; > > >> INTN Ret; > > >> + unsigned long ErrorCode; > > >> > > >> BioCert = NULL; > > >> Cert = NULL; > > >> @@ -1481,8 +1484,16 @@ TlsSetCaCertificate ( > > >> > > >> Ret = X509_STORE_add_cert (X509Store, Cert); > > >> if (Ret != 1) { > > > > > > [Jaben] do we want to leave hardcoded things like this 1? > > > [Samer] Did not want to make additional changes in this patch. Could > > > consider including it in a future cleanup patch, but the oepnSSL / > > > crypto code is using 0 (for error) and 1 (for OK) all over the place > > > > > > > > >> - Status = EFI_ABORTED; > > >> - goto ON_EXIT; > > >> + ErrorCode = ERR_peek_last_error (); > > >> + // > > >> + // Ignore "already in table" errors > > >> + // > > >> + if (!(ERR_GET_FUNC (ErrorCode) == > X509_F_X509_STORE_ADD_CERT > > && > > >> + ERR_GET_REASON (ErrorCode) == > > >> X509_R_CERT_ALREADY_IN_HASH_TABLE)) { > > >> + Status = EFI_ABORTED; > > >> + goto ON_EXIT; > > >> + } > > >> + > > >> } > > >> > > >> X509_STORE_set_flags ( > > > > > > [Jaben] Is there a reason to still change the flags on the cert? > > > [Samer] Did not want to make additional changes in this patch. This > > > patch is > > to fix a specific error. The next patch I will submit (after this one > > is cleared) has some cleanup, and includes moving the store flags set to the > init function. > > > > > >> -- > > >> 2.6.3.windows.1 > > >> > > >> _______________________________________________ > > >> edk2-devel mailing list > > >> edk2-devel@lists.01.org<mailto: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
