bneradt commented on a change in pull request #8014:
URL: https://github.com/apache/trafficserver/pull/8014#discussion_r660884504
##########
File path: iocore/net/SSLUtils.cc
##########
@@ -1633,15 +1822,16 @@ SSLMultiCertConfigLoader::update_ssl_ctx(const
std::string &secret_name)
single_data.ca_list.push_back(i < data.ca_list.size() ? data.ca_list[i]
: "");
single_data.ocsp_list.push_back(i < data.ocsp_list.size() ?
data.ocsp_list[i] : "");
- shared_SSL_CTX unique_ctx(this->init_server_ssl_ctx(single_data,
policy_iter->get(), iter->second), SSL_CTX_free);
-
- if (!unique_ctx) {
- retval = false;
- } else {
- for (auto name : iter->second) {
- SSLCertContext *cc = lookup->find(name);
- if (cc && cc->userconfig.get() == policy_iter->get()) {
- cc->setCtx(unique_ctx);
+ std::vector<SSLLoadingContext> ctxs =
this->init_server_ssl_ctx(single_data, policy_iter->get(), iter->second);
+ for (auto loadingctx : ctxs) {
+ if (!loadingctx.ctx.get()) {
+ retval = false;
+ } else {
+ for (auto name : iter->second) {
Review comment:
I know this isn't new to your patch, but it would probably be good to
`auto const& name` rather than create a copy of the string each time via `auto
name`.
##########
File path: iocore/net/SSLUtils.cc
##########
@@ -1153,99 +1290,132 @@ setClientCertCACerts(SSL *ssl, const char *file, const
char *dir)
Initialize SSL_CTX for server
This is public function because of used by SSLCreateServerContext.
*/
-SSL_CTX *
+std::vector<SSLLoadingContext>
SSLMultiCertConfigLoader::init_server_ssl_ctx(CertLoadData const &data, const
SSLMultiCertConfigParams *sslMultCertSettings,
std::set<std::string> &names)
{
- const SSLConfigParams *params = this->_params;
+ SSL_CTX *ctx;
+ SSLCertContextType ctx_type;
+ std::vector<SSLLoadingContext> ret;
+ std::vector<std::vector<std::string>> cert_names;
+ std::vector<std::vector<std::string>> key_names;
+ std::vector<std::string> key_names_list;
+ ink_assert(data.cert_names_list.size() == data.cert_type_list.size());
+ // ink_assert(data.key_names_list.size() == data.key_names_list.size());
Review comment:
Maybe remove this commented line if we don't want to ship with it?
##########
File path: iocore/net/SSLUtils.cc
##########
@@ -1153,99 +1290,132 @@ setClientCertCACerts(SSL *ssl, const char *file, const
char *dir)
Initialize SSL_CTX for server
This is public function because of used by SSLCreateServerContext.
*/
-SSL_CTX *
+std::vector<SSLLoadingContext>
SSLMultiCertConfigLoader::init_server_ssl_ctx(CertLoadData const &data, const
SSLMultiCertConfigParams *sslMultCertSettings,
std::set<std::string> &names)
{
- const SSLConfigParams *params = this->_params;
+ SSL_CTX *ctx;
+ SSLCertContextType ctx_type;
+ std::vector<SSLLoadingContext> ret;
+ std::vector<std::vector<std::string>> cert_names;
+ std::vector<std::vector<std::string>> key_names;
+ std::vector<std::string> key_names_list;
+ ink_assert(data.cert_names_list.size() == data.cert_type_list.size());
+ // ink_assert(data.key_names_list.size() == data.key_names_list.size());
- SSL_CTX *ctx = this->default_server_ssl_ctx();
+#ifdef OPENSSL_IS_BORINGSSL
+ for (auto name : data.cert_names_list) {
Review comment:
`auto const& name`
##########
File path: iocore/net/SSLUtils.cc
##########
@@ -1153,99 +1290,132 @@ setClientCertCACerts(SSL *ssl, const char *file, const
char *dir)
Initialize SSL_CTX for server
This is public function because of used by SSLCreateServerContext.
*/
-SSL_CTX *
+std::vector<SSLLoadingContext>
SSLMultiCertConfigLoader::init_server_ssl_ctx(CertLoadData const &data, const
SSLMultiCertConfigParams *sslMultCertSettings,
std::set<std::string> &names)
{
- const SSLConfigParams *params = this->_params;
+ SSL_CTX *ctx;
+ SSLCertContextType ctx_type;
+ std::vector<SSLLoadingContext> ret;
+ std::vector<std::vector<std::string>> cert_names;
+ std::vector<std::vector<std::string>> key_names;
+ std::vector<std::string> key_names_list;
+ ink_assert(data.cert_names_list.size() == data.cert_type_list.size());
+ // ink_assert(data.key_names_list.size() == data.key_names_list.size());
- SSL_CTX *ctx = this->default_server_ssl_ctx();
+#ifdef OPENSSL_IS_BORINGSSL
+ for (auto name : data.cert_names_list) {
+ cert_names.emplace_back(std::vector({name}));
+ }
+ for (auto name : data.key_list) {
+ key_names.emplace_back(std::vector({name}));
+ }
+#else
+ cert_names.emplace_back(data.cert_names_list);
+ key_names.emplace_back(data.key_list);
+#endif
- // disable selected protocols
- SSL_CTX_set_options(ctx, params->ssl_ctx_options);
+ int i = 0;
+ for (auto cert_names_list : cert_names) {
Review comment:
`auto cosnt& cert_names_list`
##########
File path: iocore/net/SSLUtils.cc
##########
@@ -1473,27 +1643,37 @@ SSLCreateServerContext(const SSLConfigParams *params,
const SSLMultiCertConfigPa
std::vector<X509 *> cert_list;
std::set<std::string> common_names;
std::unordered_map<int, std::set<std::string>> unique_names;
+
SSLMultiCertConfigLoader::CertLoadData data;
- if (loader.load_certs_and_cross_reference_names(cert_list, data, params,
sslMultiCertSettings, common_names, unique_names)) {
- ctx.reset(loader.init_server_ssl_ctx(data, sslMultiCertSettings,
common_names));
- }
- for (auto &i : cert_list) {
- X509_free(i);
- }
- if (ctx && cert_path) {
- if (!SSL_CTX_use_certificate_file(ctx.get(), cert_path, SSL_FILETYPE_PEM))
{
- SSLError("SSLCreateServerContext(): failed to load server certificate.");
- ctx = nullptr;
- } else if (!key_path || key_path[0] == '\0') {
- key_path = cert_path;
- }
- if (ctx) {
- if (!SSL_CTX_use_PrivateKey_file(ctx.get(), key_path, SSL_FILETYPE_PEM))
{
- SSLError("SSLCreateServerContext(): failed to load server private
key.");
- ctx = nullptr;
- } else if (!SSL_CTX_check_private_key(ctx.get())) {
- SSLError("SSLCreateServerContext(): server private key does not match
server certificate.");
+ SSLCertContextType cert_type;
+ if (!loader.load_certs_and_cross_reference_names(cert_list, data, params,
sslMultiCertSettings, common_names, unique_names,
+ &cert_type)) {
+ return nullptr;
+ }
+
+ std::vector<SSLLoadingContext> ctxs = loader.init_server_ssl_ctx(data,
sslMultiCertSettings, common_names);
+ for (auto loaderctx : ctxs) {
Review comment:
`auto const&`
##########
File path: iocore/net/SSLUtils.cc
##########
@@ -1610,15 +1798,16 @@ SSLMultiCertConfigLoader::update_ssl_ctx(const
std::string &secret_name)
}
if (!common_names.empty()) {
- shared_SSL_CTX ctx(this->init_server_ssl_ctx(data, policy_iter->get(),
common_names), SSL_CTX_free);
-
- if (!ctx) {
- retval = false;
- } else {
- for (auto name : common_names) {
- SSLCertContext *cc = lookup->find(name);
- if (cc && cc->userconfig.get() == policy_iter->get()) {
- cc->setCtx(ctx);
+ std::vector<SSLLoadingContext> ctxs = this->init_server_ssl_ctx(data,
policy_iter->get(), common_names);
+ for (auto loadingctx : ctxs) {
+ if (!loadingctx.ctx.get()) {
+ retval = false;
+ } else {
+ for (auto name : common_names) {
Review comment:
I know this isn't new to your patch, but `auto const& name` would be
better to avoid the copy of each of the names.
##########
File path: iocore/net/SSLUtils.cc
##########
@@ -1549,17 +1733,17 @@ SSLMultiCertConfigLoader::_store_ssl_ctx(SSLCertLookup
*lookup, const shared_SSL
return false;
}
- if (!common_names.empty()) {
- shared_SSL_CTX ctx(this->init_server_ssl_ctx(data,
sslMultCertSettings.get(), common_names), SSL_CTX_free);
-
- if (!ctx || !sslMultCertSettings || !this->_store_single_ssl_ctx(lookup,
sslMultCertSettings, ctx, common_names)) {
+ std::vector<SSLLoadingContext> ctxs = this->init_server_ssl_ctx(data,
sslMultCertSettings.get(), common_names);
+ for (auto loadingctx : ctxs) {
Review comment:
`auto const&` ?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
