Please let me know whether you can commit the patch:). Thanks. Jiaxin
From: El-Haj-Mahmoud, Samer [mailto:[email protected]] Sent: Tuesday, April 26, 2016 9:41 AM To: Carsey, Jaben <[email protected]>; Wu, Jiaxin <[email protected]> Cc: [email protected]; Palmer, Thomas <[email protected]> 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 [[email protected]] Received: Monday, 25 Apr 2016, 7:43PM To: Carsey, Jaben [[email protected]]; El-Haj-Mahmoud, Samer [[email protected]] CC: [email protected]<mailto:[email protected]> [[email protected]]; Palmer, Thomas [[email protected]] 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 <[email protected]<mailto:[email protected]>> 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 > <[email protected]<mailto:[email protected]>> > Cc: [email protected]<mailto:[email protected]>; Wu, Jiaxin > <[email protected]<mailto:[email protected]>>; Palmer, > Thomas <[email protected]<mailto:[email protected]>> > 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 > <[email protected]<mailto:[email protected]>> > > -Jaben > > > On Apr 25, 2016, at 11:17 AM, El-Haj-Mahmoud, Samer <samer.el-haj- > [email protected]<mailto:[email protected]>> 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:[email protected]] > > Sent: Monday, April 25, 2016 12:54 PM > > To: El-Haj-Mahmoud, Samer > > <[email protected]<mailto:[email protected]>>; > > [email protected]<mailto:[email protected]> > > Cc: El-Haj-Mahmoud, Samer > > <[email protected]<mailto:[email protected]>>; Wu, > Jiaxin > > <[email protected]<mailto:[email protected]>>; Carsey, Jaben > > <[email protected]<mailto:[email protected]>> > > 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:[email protected]] On Behalf > >> Of Samer El-Haj-Mahmoud > >> Sent: Monday, April 25, 2016 10:15 AM > >> To: [email protected]<mailto:[email protected]> > >> Cc: Samer El-Haj-Mahmoud <[email protected]<mailto:[email protected]>>; Wu, Jiaxin > >> <[email protected]<mailto:[email protected]>> > >> 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 <[email protected]<mailto:[email protected]>> > >> Signed-off-by: Thomas Palmer > >> <[email protected]<mailto:[email protected]>> > >> --- > >> 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 > >> [email protected]<mailto:[email protected]> > >> https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
