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.

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)
  */