shinrich commented on a change in pull request #7219:
URL: https://github.com/apache/trafficserver/pull/7219#discussion_r496194344
##########
File path: iocore/net/SSLUtils.cc
##########
@@ -1146,19 +1148,25 @@ setClientCertLevel(SSL *ssl, uint8_t certLevel)
}
void
-setClientCertCACerts(SSL *ssl, X509_STORE *ca_certs)
+setClientCertCACerts(SSL *ssl, const char *file, const char *dir)
{
+ if ((file != nullptr && file[0] != '\0') || (dir != nullptr && dir[0] !=
'\0')) {
#if defined(SSL_set1_verify_cert_store)
+ // The set0 version will take ownership of the X509_STORE object
+ X509_STORE *ctx = X509_STORE_new();
+ if (X509_STORE_load_locations(ctx, file && file[0] != '\0' ? file :
nullptr, dir && dir[0] != '\0' ? dir : nullptr)) {
+ SSL_set0_verify_cert_store(ssl, ctx);
+ }
- // It is necessasry to use set1 and not set0 here. SSL_free() calls
ssl_cert_free(), which calls
- // X509_STORE_free() on the ca_certs store. Hard to see how set0 could ever
actually be useful, in
- // what scenario would SSL objects never be freed?
- //
- SSL_set1_verify_cert_store(ssl, ca_certs);
-
+ // SSL_set_client_CA_list takes ownership of the STACK_OF(X509) structure
+ // So we load it each time to pass into the SSL object
+ if (file != nullptr && file[0] != '\0') {
+ SSL_set_client_CA_list(ssl, SSL_load_client_CA_file(file));
+ }
Review comment:
True. I added this when debugging the issue. Based on the
documentation, it seemed like there were cases when the client would only look
for appropriate intermediates if this was set. Since we set this in the
SSL_CTX case, I added it here. Turns out it didn't fix the issue (at least not
alone). The ssl->cert->verify_store rewriting was the main problem.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
