ywkaras commented on a change in pull request #7219:
URL: https://github.com/apache/trafficserver/pull/7219#discussion_r496053251
##########
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:
I'm really lost with all this. Doesn't this just set the _names_ of the
CAs that are sent to the client, not the CA certs used to validate? How is
this relevant to Vinith's (or my) test, where the curl command only has a
single cert it can send in any case? Seems like there should be a way to avoid
having to read a disk file on every single TLS handshake.
----------------------------------------------------------------
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]
