BUG/MEDIUM: ssl: abort with the correct SSL error when SNI not found

Since commit c2aae74 ("MEDIUM: ssl: Handle early data with OpenSSL
1.1.1"), the codepath of the clientHello callback changed, letting an
unknown SNI escape with a 'return 1' instead of passing through the
abort label.

An error was still emitted because the frontend continued the handshake
with the initial_ctx, which can't be used to achieve an handshake.
However, it had the ugly side effect of letting the request pass in the
case of a TLS resume. Which could be surprising when combining strict-sni
with the removing of a crt-list entry over the CLI for example. (like
its done in the ssl/new_del_ssl_crlfile.vtc reg-test).

This patch switches the code path of the allow_early and abort label, so
the default code path is the abort one, letting the clientHello returns
the correct SSL_AD_UNRECOGNIZED_NAME in case of errors.

Which means the client will now receive:

	OpenSSL error[0x14094458] ssl3_read_bytes: tlsv1 unrecognized name

Instead of:

	OpenSSL error[0x14094410] ssl3_read_bytes: sslv3 alert handshake failure

Which was the error emitted before HAProxy 1.8.

This patch must be carrefuly backported as far as 1.8 once we validated
its impact.

(cherry picked from commit 7980dff10ce3dac957a8f0604f1de990bad3e1b4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 0f2291ba759e2eeab6ddb04460f21741d449e522)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 0894a89..99e52e0 100644 (file)
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -2511,16 +2511,11 @@ static int ssl_sock_switchctx_cbk(SSL *ssl, int *al, void *arg)
                HA_RWLOCK_RDLOCK(SNI_LOCK, &s->sni_lock);
                ssl_sock_switchctx_set(ssl, s->default_ctx);
                HA_RWLOCK_RDUNLOCK(SNI_LOCK, &s->sni_lock);
+               goto allow_early;
        }
-allow_early:
-#ifdef OPENSSL_IS_BORINGSSL
-       if (allow_early)
-               SSL_set_early_data_enabled(ssl, 1);
-#else
-       if (!allow_early)
-               SSL_set_max_early_data(ssl, 0);
-#endif
-       return 1;
+
+       /* other cases fallback on abort, if strict-sni is set but no node was found */
+
  abort:
        /* abort handshake (was SSL_TLSEXT_ERR_ALERT_FATAL) */
        conn->err_code = CO_ER_SSL_HANDSHAKE;
@@ -2530,6 +2525,16 @@ allow_early:
        *al = SSL_AD_UNRECOGNIZED_NAME;
        return 0;
 #endif
+
+allow_early:
+#ifdef OPENSSL_IS_BORINGSSL
+       if (allow_early)
+               SSL_set_early_data_enabled(ssl, 1);
+#else
+       if (!allow_early)
+               SSL_set_max_early_data(ssl, 0);
+#endif
+       return 1;
 }
 
 #else /* OPENSSL_IS_BORINGSSL */