]> arthur.barton.de Git - ngircd-alex.git/commitdiff
S2S-TLS/OpenSSL: Postpone verification of TLS session right before server handshake
authorAlexander Barton <alex@barton.de>
Tue, 2 Jan 2024 21:02:46 +0000 (22:02 +0100)
committerAlexander Barton <alex@barton.de>
Sat, 23 Mar 2024 19:19:01 +0000 (20:19 +0100)
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
src/ngircd/conn.c

index d89c11fe1baa1d2252c856f9e9308e0b756b61d6..22b5d07ec3683aca4170901851382375922ffe10 100644 (file)
@@ -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
 
index 3882899f4406b85829d0f5e0ce04f0008aff093a..fab483e1ab2a288a230a39bce68db553df32d418 100644 (file)
@@ -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)
  */