[Asterisk-code-review] tcptls: Enable multiple TLS certificate chains (RSA+ECC+DSA)... (asterisk[master])

[Richard Mudgett]

Richard Mudgett has posted comments on this change.

Change subject: tcptls: Enable multiple TLS certificate chains (RSA+ECC+DSA) for server socket.
......................................................................


Patch Set 4:

(1 comment)

https://gerrit.asterisk.org/#/c/431/4/main/tcptls.c
File main/tcptls.c:

Line 761: 		if (SSL_CTX_use_certificate_chain_file(cfg->ssl_ctx, cert_file) == 0) {
        : 			ast_log(LOG_ERROR, "TLS/SSL error loading %s cert file. <%s>\n", key_type, cert_file);
        : 		} else if (SSL_CTX_use_PrivateKey_file(cfg->ssl_ctx, cert_file, SSL_FILETYPE_PEM) == 0) {
        : 			ast_log(LOG_ERROR, "TLS/SSL error loading %s cert file. <%s>\n", key_type, cert_file);
        : 		} else if (SSL_CTX_check_private_key(cfg->ssl_ctx) == 0) {
        : 			ast_log(LOG_ERROR, "TLS/SSL error loading %s cert file. <%s>\n", key_type, cert_file);
        : 		}
> In Asterisk, a log message is preceded by the line of code. This way, a use
The function and line number of the diagnostic message only shows up on the console output.  That information does not show up in the log files unfortunately.

Differentiating diagnostic messages by line number is not a good method.  I've seen enough people ask what a diagnostic message means without even bothering to read what the message plainly says is wrong.

You are doing three things here with the file:
TLS/SSL error loading ECC certificates from file <%s>.
TLS/SSL error loading DSA private keys from a file <%s>.
TLS/SSL error checking ECC private keys from a file <%s>.

These messages should be changed to LOG_WARNING instead of LOG_ERROR since they don't fail setting up the call.


-- 
To view, visit https://gerrit.asterisk.org/431
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iada5e00d326db5ef86e0af7069b4dfa1b979da9a
Gerrit-PatchSet: 4
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Alexander Traud <pabstraud at compuserve.com>
Gerrit-Reviewer: Alexander Traud <pabstraud at compuserve.com>
Gerrit-Reviewer: Ashley Sanders <asanders at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: Yes