Nice
On Mon, Jul 8, 2019, 5:30 PM Lukas Tribus <[email protected]> wrote:
> Commit 54832b97 ("BUILD: enable several LibreSSL hacks, including")
> changed empty handshake detection in OpenSSL <= 1.0.2 and LibreSSL,
> from accessing packet_length directly (not available in LibreSSL) to
> calling SSL_state() instead.
>
> However, SSL_state() appears to be fully broken in both OpenSSL and
> LibreSSL.
>
> Since there is no possibility in LibreSSL to detect an empty handshake,
> let's not try (like BoringSSL) and restore this functionality for
> OpenSSL 1.0.2 and older, by reverting to the previous behavior.
>
> Should be backported to 2.0.
> ---
>
> changes in V2:
> add code comments
>
> ---
> src/ssl_sock.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/src/ssl_sock.c b/src/ssl_sock.c
> index c9fffbe..3ddacb6 100644
> --- a/src/ssl_sock.c
> +++ b/src/ssl_sock.c
> @@ -5352,15 +5352,18 @@ static int ssl_sock_handshake(struct connection
> *conn, unsigned int flag)
> if (!errno && conn->flags &
> CO_FL_WAIT_L4_CONN)
> conn->flags &= ~CO_FL_WAIT_L4_CONN;
> if (!conn->err_code) {
> -#ifdef OPENSSL_IS_BORINGSSL /* BoringSSL */
> +#if defined(OPENSSL_IS_BORINGSSL) || defined(LIBRESSL_VERSION_NUMBER)
> + /* do not handle empty handshakes
> in BoringSSL or LibreSSL */
> conn->err_code =
> CO_ER_SSL_HANDSHAKE;
> #else
> int empty_handshake;
> #if (HA_OPENSSL_VERSION_NUMBER >= 0x1010000fL)
> + /* use SSL_get_state() in OpenSSL
> >= 1.1.0; SSL_state() is broken */
> OSSL_HANDSHAKE_STATE state =
> SSL_get_state((SSL *)ctx->ssl);
> empty_handshake = state ==
> TLS_ST_BEFORE;
> #else
> - empty_handshake = SSL_state((SSL
> *)ctx->ssl) == SSL_ST_BEFORE;
> + /* access packet_length directly
> in OpenSSL <= 1.0.2; SSL_state() is broken */
> + empty_handshake =
> !ctx->ssl->packet_length;
> #endif
> if (empty_handshake) {
> if (!errno) {
> @@ -5382,7 +5385,7 @@ static int ssl_sock_handshake(struct connection
> *conn, unsigned int flag)
> else
> conn->err_code =
> CO_ER_SSL_HANDSHAKE;
> }
> -#endif
> +#endif /* BoringSSL or LibreSSL */
> }
> goto out_error;
> }
> @@ -5433,15 +5436,18 @@ check_error:
> if (!errno && conn->flags & CO_FL_WAIT_L4_CONN)
> conn->flags &= ~CO_FL_WAIT_L4_CONN;
> if (!conn->err_code) {
> -#ifdef OPENSSL_IS_BORINGSSL /* BoringSSL */
> +#if defined(OPENSSL_IS_BORINGSSL) || defined(LIBRESSL_VERSION_NUMBER)
> + /* do not handle empty handshakes in
> BoringSSL or LibreSSL */
> conn->err_code = CO_ER_SSL_HANDSHAKE;
> #else
> int empty_handshake;
> #if (HA_OPENSSL_VERSION_NUMBER >= 0x1010000fL)
> + /* use SSL_get_state() in OpenSSL >=
> 1.1.0; SSL_state() is broken */
> OSSL_HANDSHAKE_STATE state =
> SSL_get_state(ctx->ssl);
> empty_handshake = state == TLS_ST_BEFORE;
> #else
> - empty_handshake = SSL_state((SSL
> *)ctx->ssl) == SSL_ST_BEFORE;
> + /* access packet_length directly in
> OpenSSL <= 1.0.2; SSL_state() is broken */
> + empty_handshake = !ctx->ssl->packet_length;
> #endif
> if (empty_handshake) {
> if (!errno) {
> @@ -5463,7 +5469,7 @@ check_error:
> else
> conn->err_code =
> CO_ER_SSL_HANDSHAKE;
> }
> -#endif
> +#endif /* BoringSSL or LibreSSL */
> }
> goto out_error;
> }
> --
> 2.7.4
>