[GitHub] [trafficserver] jpeach commented on a diff in pull request #9840: Fix crash on config reload with BoringSSL

["jpeach (via GitHub)" <gi...@apache.org>]

jpeach commented on code in PR #9840:
URL: https://github.com/apache/trafficserver/pull/9840#discussion_r1230329582


##########
iocore/net/SSLConfig.cc:
##########
@@ -952,7 +952,8 @@ SSLConfigParams::getCTX(const std::string &client_cert, const std::string &key_f
           SSLError("failed to attach client chain certificate from %s", client_cert.c_str());
           goto fail;
         }
-        X509_free(cert);
+        // Do not free cert becasue SSL_CTX_add_extra_chain_cert takes ownership of cert if it succeeds, unlike
+        // SSL_CTX_use_certificate.

Review Comment:
   If `SSL_CTX_add_extra_chain_cert`, but a subsequent operation fails, we would free the certificate at the `failed` label. Wouldn't that also be a double free when the `SSL_CTX` is freed?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org