[ https://issues.apache.org/jira/browse/TS-2967?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14091882#comment-14091882 ]
Jack Bates commented on TS-2967: -------------------------------- bq. The only place I can see that needs a NULL check is {{SSLNetVConnection::sslStartHandShake()}} Hmm what should {{SSLNetVConnection::sslStartHandShake()}} do if {{SSLCertificateConfig::scoped_config}} is NULL? I think it makes sense for it to behave the same if {{ssl_multicert.config}} doesn't exist as if it is blank so if {{SSLCertificateConfig::scoped_config}} is NULL because {{ssl_multicert.config}} doesn't exist then I think {{SSLNetVConnection::sslStartHandShake()}} needs the {{lookup->defaultContext()}} that results from the following lines in {{SSLParseCertificateConfiguration()}} {code} // We *must* have a default context even if it can't possibly work. The default context is used to // bootstrap the SSL handshake so that we can subsequently do the SNI lookup to switch to the real // context. if (lookup->ssl_default == NULL) { ssl_user_config sslMultiCertSettings; sslMultiCertSettings.addr = ats_strdup("*"); ssl_store_ssl_context(params, lookup, sslMultiCertSettings); } {code} What about the following change? {code} --- a/iocore/net/SSLUtils.cc +++ b/iocore/net/SSLUtils.cc @@ -1476,10 +1476,7 @@ SSLParseCertificateConfiguration( file_buf = readIntoBuffer(params->configFilePath, __func__, NULL); } - if (!file_buf) { - Error("failed to read SSL certificate configuration from %s", params->confi - return false; - } + if (file_buf) { // elevate/allow file access to root read only files/certs uint32_t elevate_setting = 0; @@ -1521,6 +1518,9 @@ SSLParseCertificateConfiguration( line = tokLine(NULL, &tok_state); } + } else { + Error("failed to read SSL certificate configuration from %s", params->confi + } // We *must* have a default context even if it can't possibly work. The defau // bootstrap the SSL handshake so that we can subsequently do the SNI lookup {code} This change resolves this issue because {{SSLCertificateConfig::reconfigure()}} will now initialize {{configid}} when {{ssl_multicert.config}} doesn't exist and it makes {{SSLNetVConnection::sslStartHandShake()}} behave the same if {{ssl_multicert.config}} doesn't exist as if it is blank. > failed assert if ssl_multicert.config doesn't exist > --------------------------------------------------- > > Key: TS-2967 > URL: https://issues.apache.org/jira/browse/TS-2967 > Project: Traffic Server > Issue Type: Bug > Reporter: Jack Bates > Assignee: Jack Bates > Fix For: 5.1.0 > > > If an ssl_multicert.config file doesn't exist then > SSLParseCertificateConfiguration() returns false (SSLUtils.cc line 1435) > and SSLCertificateConfig::reconfigure() doesn't initialize configid > (SSLConfig.cc line 333) > Then when SSLRecRawStatSyncCount() calls SSLCertificateConfig::acquire() > (SSLUtils.cc line 502) > it calls ConfigProcessor::get() with configid zero (SSLConfig.cc line 342) > and there's an ink_assert(id != 0) (ProxyConfig.cc line 175) > One way to avoid the failed assert is if SSLCertificateConfig::acquire() and > SSLCertificateConfig::release() only call ConfigProcessor::get/release() if > (configid !=0) > Another way might be if SSLCertificateConfig::reconfigure() initialized > configid with configProcessor.set(configid, NULL) if > SSLParseCertificateConfiguration() returns false? > Or SSLParseCertificateConfiguration() could treat a nonexistent > ssl_multicert.config the same as it treats a blank file? ({{touch > ssl_multicert.config}} makes the failed assert go away.) > Or maybe there's another better way to avoid the failed assert? -- This message was sent by Atlassian JIRA (v6.2#6252)