You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by GitBox <gi...@apache.org> on 2021/06/29 19:08:44 UTC
[GitHub] [trafficserver] bneradt commented on a change in pull request #8014: Dual cert support for BoringSSL
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: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org