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]>; [email protected] Cc: El-Haj-Mahmoud, Samer <[email protected]>; Wu, Jiaxin <[email protected]>; Carsey, Jaben <[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] > Cc: Samer El-Haj-Mahmoud <[email protected]>; Wu, Jiaxin > <[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]> > Signed-off-by: Thomas Palmer <[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] > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
