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 [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 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> 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> > Cc: edk2-devel@lists.01.org; Wu, Jiaxin <jiaxin...@intel.com>; Palmer, > Thomas <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> > > -Jaben > > > On Apr 25, 2016, at 11:17 AM, El-Haj-Mahmoud, Samer <samer.el-haj- > 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>; > > edk2-devel@lists.01.org > > Cc: El-Haj-Mahmoud, Samer <samer.el-haj-mahm...@hpe.com>; Wu, > Jiaxin > > <jiaxin...@intel.com>; Carsey, Jaben <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 > >> Cc: Samer El-Haj-Mahmoud <el...@hpe.com>; Wu, Jiaxin > >> <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> > >> Signed-off-by: Thomas Palmer <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 > >> 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
