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/01/19 06:28:10 UTC
[GitHub] [trafficserver] maskit opened a new pull request #7434: Split SSL_CTX initialization logic into small functions
maskit opened a new pull request #7434:
URL: https://github.com/apache/trafficserver/pull/7434
We have two `init_server_ssl_ctx`, one in `SSLMultiCertConfigLoader` and the other in `QUICMultiCertConfigLoader`. They are almost same but there is a few difference. Some parts were removed or commented out from the one for QUIC because callback functions are not compatible with QUICNetVC in many cases. Duplicating entire function and removing some parts was the easiest way to make it work I guess.
However, it's hard to maintain the both. You can't tell which part was removed, which part is missing, and which part is added only for one of them.
This PR unifies the two functions by splitting the logic into small functions. `QUICMultiCertConfigLoader ` overrides some of the functions to disable the initialization processes. In the future the opposite thing may happen (SSLMultiCertConfigLoader has an empty function and only QUICMultiCertConfigLoader has the implementation).
Ideally we may want to have an abstract MultiCertLoader but that would be next step.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [trafficserver] bryancall commented on pull request #7434: Split SSL_CTX initialization logic into small functions
Posted by GitBox <gi...@apache.org>.
bryancall commented on pull request #7434:
URL: https://github.com/apache/trafficserver/pull/7434#issuecomment-773655105
@shinrich is going to look at it.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [trafficserver] maskit commented on a change in pull request #7434: Split SSL_CTX initialization logic into small functions
Posted by GitBox <gi...@apache.org>.
maskit commented on a change in pull request #7434:
URL: https://github.com/apache/trafficserver/pull/7434#discussion_r578839630
##########
File path: iocore/net/SSLUtils.cc
##########
@@ -1226,90 +1314,98 @@ SSLMultiCertConfigLoader::init_server_ssl_ctx(CertLoadData const &data, const SS
break;
}
}
+ return true;
+}
-#ifdef SSL_MODE_RELEASE_BUFFERS
- Debug("ssl", "enabling SSL_MODE_RELEASE_BUFFERS");
- SSL_CTX_set_mode(ctx, SSL_MODE_RELEASE_BUFFERS);
-#endif
-
-#ifdef SSL_OP_SAFARI_ECDHE_ECDSA_BUG
- SSL_CTX_set_options(ctx, SSL_OP_SAFARI_ECDHE_ECDSA_BUG);
-#endif
-
- if (sslMultCertSettings) {
- if (sslMultCertSettings->dialog) {
- passphrase_cb_userdata ud(params, sslMultCertSettings->dialog, sslMultCertSettings->first_cert, sslMultCertSettings->key);
- // pass phrase dialog configuration
- pem_password_cb *passwd_cb = nullptr;
- if (strncmp(sslMultCertSettings->dialog, "exec:", 5) == 0) {
- ud._serverDialog = &sslMultCertSettings->dialog[5];
- // validate the exec program
- if (!ssl_private_key_validate_exec(ud._serverDialog)) {
- SSLError("failed to access '%s' pass phrase program: %s", (const char *)ud._serverDialog, strerror(errno));
- memset(static_cast<void *>(&ud), 0, sizeof(ud));
- goto fail;
- }
- passwd_cb = ssl_private_key_passphrase_callback_exec;
- } else if (strcmp(sslMultCertSettings->dialog, "builtin") == 0) {
- passwd_cb = ssl_private_key_passphrase_callback_builtin;
- } else { // unknown config
- SSLError("unknown %s configuration value '%s'", SSL_KEY_DIALOG.data(), (const char *)sslMultCertSettings->dialog);
+bool
+SSLMultiCertConfigLoader::_setup_dialog(SSL_CTX *ctx, const SSLMultiCertConfigParams *sslMultCertSettings)
+{
+ if (sslMultCertSettings->dialog) {
Review comment:
I think it would be better to ask on users@. It's not a something from the initial commit.
https://issues.apache.org/jira/browse/TS-612
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [trafficserver] maskit merged pull request #7434: Split SSL_CTX initialization logic into small functions
Posted by GitBox <gi...@apache.org>.
maskit merged pull request #7434:
URL: https://github.com/apache/trafficserver/pull/7434
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [trafficserver] shinrich commented on a change in pull request #7434: Split SSL_CTX initialization logic into small functions
Posted by GitBox <gi...@apache.org>.
shinrich commented on a change in pull request #7434:
URL: https://github.com/apache/trafficserver/pull/7434#discussion_r578706813
##########
File path: iocore/net/SSLUtils.cc
##########
@@ -1226,90 +1314,98 @@ SSLMultiCertConfigLoader::init_server_ssl_ctx(CertLoadData const &data, const SS
break;
}
}
+ return true;
+}
-#ifdef SSL_MODE_RELEASE_BUFFERS
- Debug("ssl", "enabling SSL_MODE_RELEASE_BUFFERS");
- SSL_CTX_set_mode(ctx, SSL_MODE_RELEASE_BUFFERS);
-#endif
-
-#ifdef SSL_OP_SAFARI_ECDHE_ECDSA_BUG
- SSL_CTX_set_options(ctx, SSL_OP_SAFARI_ECDHE_ECDSA_BUG);
-#endif
-
- if (sslMultCertSettings) {
- if (sslMultCertSettings->dialog) {
- passphrase_cb_userdata ud(params, sslMultCertSettings->dialog, sslMultCertSettings->first_cert, sslMultCertSettings->key);
- // pass phrase dialog configuration
- pem_password_cb *passwd_cb = nullptr;
- if (strncmp(sslMultCertSettings->dialog, "exec:", 5) == 0) {
- ud._serverDialog = &sslMultCertSettings->dialog[5];
- // validate the exec program
- if (!ssl_private_key_validate_exec(ud._serverDialog)) {
- SSLError("failed to access '%s' pass phrase program: %s", (const char *)ud._serverDialog, strerror(errno));
- memset(static_cast<void *>(&ud), 0, sizeof(ud));
- goto fail;
- }
- passwd_cb = ssl_private_key_passphrase_callback_exec;
- } else if (strcmp(sslMultCertSettings->dialog, "builtin") == 0) {
- passwd_cb = ssl_private_key_passphrase_callback_builtin;
- } else { // unknown config
- SSLError("unknown %s configuration value '%s'", SSL_KEY_DIALOG.data(), (const char *)sslMultCertSettings->dialog);
+bool
+SSLMultiCertConfigLoader::_setup_dialog(SSL_CTX *ctx, const SSLMultiCertConfigParams *sslMultCertSettings)
+{
+ if (sslMultCertSettings->dialog) {
Review comment:
Unrelated to this PR really, but has anyone actually tried to use this password dialog stuff in the past 5-10 years? This is something I'd like to pull out at some point.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [trafficserver] bryancall commented on pull request #7434: Split SSL_CTX initialization logic into small functions
Posted by GitBox <gi...@apache.org>.
bryancall commented on pull request #7434:
URL: https://github.com/apache/trafficserver/pull/7434#issuecomment-773655105
@shinrich is going to look at it.
----------------------------------------------------------------
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:
users@infra.apache.org