From 3db3b47fc7172a69b7d99d66eddb07a323dc6e74 Mon Sep 17 00:00:00 2001 From: Alexander Barton Date: Tue, 2 Jan 2024 22:02:46 +0100 Subject: [PATCH] S2S-TLS/OpenSSL: Postpone verification of TLS session right before server handshake The verify callback in OpenSSL is called pretty early, and at that time it is not possible yet to check which connection it belongs to, and some connections may have relaxed requirements. So always return success in the Verify_openssl() callback, and postpone validation of the TLS session until starting the server handshake in cb_connserver_login_ssl(), when we know which server this connection belongs to and which options (like "SSLVerify") are in effect. The code doing this was already present in cb_connserver_login_ssl(), but this patch adds a more prominent comment to the function. --- src/ngircd/conn-ssl.c | 21 +++++++++++++++------ src/ngircd/conn.c | 7 +++++++ 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/ngircd/conn-ssl.c b/src/ngircd/conn-ssl.c index d89c11fe..22b5d07e 100644 --- a/src/ngircd/conn-ssl.c +++ b/src/ngircd/conn-ssl.c @@ -211,14 +211,23 @@ pem_passwd_cb(char *buf, int size, int rwflag, void *password) static int Verify_openssl(int preverify_ok, X509_STORE_CTX * ctx) { - int err; - +#ifdef DEBUG if (!preverify_ok) { - err = X509_STORE_CTX_get_error(ctx); - Log(LOG_ERR, "Certificate validation failed: %s", - X509_verify_cert_error_string(err)); + int err = X509_STORE_CTX_get_error(ctx); + LogDebug("Certificate validation failed: %s", + X509_verify_cert_error_string(err)); } - return preverify_ok; +#else + (void)preverify_ok; + (void)ctx; +#endif + + /* Always(!) return success as we have to deal with invalid + * (self-signed, expired, ...) client certificates and with invalid + * server certificates when "SSLVerify" is disabled, which we don't + * know at this stage. Therefore we postpone this check, it will be + * (and has to be!) handled in cb_connserver_login_ssl(). */ + return 1; } #endif diff --git a/src/ngircd/conn.c b/src/ngircd/conn.c index 3882899f..fab483e1 100644 --- a/src/ngircd/conn.c +++ b/src/ngircd/conn.c @@ -2556,6 +2556,13 @@ cb_listen_ssl(int sock, short irrelevant) /** * IO callback for new outgoing SSL-enabled server connections. * + * IMPORTANT: The SSL session has been validated before, but all errors have + * been ignored so far! The reason for this is that the generic SSL code has no + * idea if the new session actually belongs to a server, as this only becomes + * clear when the remote peer sends its PASS command (and we have to handle + * invalid client certificates!). Therefore, it is important to check the + * status of the SSL session first before continuing the server handshake here! + * * @param sock Socket descriptor. * @param unused (ignored IO specification) */ -- 2.39.2