Hi Roberto, I agree with that. And so the RFC says.
Manu > Le 2 mars 2017 à 23:26, Roberto Guimaraes <[email protected]> a écrit : > > hi thanks for the reply. > > You are right about SSL_TLSEXT_ERR_NOACK as it is a pretty innocuous return > in openssl-1.1 (merely sets a flag which may even seem redundant considering > the other checks). > > However, I’ve confirmed that the issue I see is indeed caused by a > SSL_TLSEXT_ERR_ALERT_WARNING return from the SNI callback. > > Reproducing this is fairly simple: build haproxy with openssl-1.1.x and cause > an SNI mismatch. > -- > GnuTLS base clients will fail consistently: > > apt-get gnutls_handshake() failed: A TLS warning alert has been received. > > wget > GnuTLS: A TLS warning alert has been received. > GnuTLS: received alert [112]: The server name sent was not recognized > > Secure Sockets Layer > TLSv1.2 Record Layer: Alert (Level: Warning, Description: Unrecognized > Name) > Content Type: Alert (21) > Version: TLS 1.2 (0x0303) > Length: 2 > Alert Message > Level: Warning (1) > Description: Unrecognized Name (112) > TLSv1.2 Record Layer: Handshake Protocol: Server Hello > > Sorry, I did not mean that s_server has changed, but openssl library itself > has changed in 1.1 concerning these warnings. I don’t see them getting sent > with openssl-1.0.2x upon SNI mismatch at all, but I do see them on the wire > (tcpdump, tshark) with openssl-1.1.x. > > Also, reading the RFC, it seems sending the alert is not recommended: > > https://tools.ietf.org/html/rfc6066#section-3 > <https://tools.ietf.org/html/rfc6066#section-3> > > “It is NOT RECOMMENDED to send a warning-level unrecognized_name(112) alert, > because the client's behavior in response to warning-level alerts is > unpredictable.“ > > therefore I think that openssl-1.1 support demands the following patch (works > just as well with prior openssl releases): > > diff --git a/src/ssl_sock.c b/src/ssl_sock.c > index 1a9c185..95b5dc7 100644 > --- a/src/ssl_sock.c > +++ b/src/ssl_sock.c > @@ -1707,7 +1707,7 @@ static int ssl_sock_switchctx_cbk(SSL *ssl, int *al, > void *priv) > #endif > return (s->strict_sni ? > SSL_TLSEXT_ERR_ALERT_FATAL : > - SSL_TLSEXT_ERR_ALERT_WARNING); > + SSL_TLSEXT_ERR_OK); > } > > /* switch ctx */ > > thanks, > roberto > > >> On Feb 17, 2017, at 10:22 AM, Emmanuel Hocdet <[email protected] >> <mailto:[email protected]>> wrote: >> >> Hi Roberto >> >>> Le 17 févr. 2017 à 01:27, Roberto Guimaraes <[email protected] >>> <mailto:[email protected]>> a écrit : >>> >>> greetings, >>> >>> just a heads up that we’ve seen client breakage when using haproxy with >>> openssl-1.1 — dunno how far along you are concerning ossl1.1 usage, but it >>> has become very clear that openssl-1.1 behaves differently in a number of >>> ways esp SNI callback. >>> >>> so, openssl-1.0.x does not actually send the warnings/alerts when the >>> callback returns SSL_TLSEXT_ERR_NOACK or SSL_TLSEXT_ERR_ALERT_WARNING, but >>> openssl-1.1 will send them. >>> >>> Some clients, such as java1.8 and gnutls, will fail upon receiving these >>> warnings, even if the RFC isn’t clear on the correct behavior. >>> >> It’s strange. I suppose it’s another problem. Need logs and more infos to >> help. >> >>> I don’t know if this 100% applicable to you, but perhaps consider removing >>> the warnings unless explicitly requested with strict_sni? Looking at the >>> “reference” s_server.c in openssl-1.1 and it does return ERR_OK for both >>> mismatch and missing servername. >>> >> I don’t seen any changes about that at s_server.c in openssl-1.1. >> >> SSL_TLSEXT_ERR_NOACK is used when client does not sent an SNI. Is not a >> warning and is useful. >> >> SSL_TLSEXT_ERR_ALERT_WARNING is used when certificate does not match the SNI >> sent by the client. >> it could be removed because clients check it and generate the alert. >> Replacing it with SSL_TLSEXT_ERR_OK will not change anything to correct >> clients. BoringSSL ignores it and >> sends SSL_TLSEXT_ERR_OK instead and it does not change anything for all >> known clients (via ssllabs). >> >> Manu >> >>> roberto >>> >>> diff --git a/src/ssl_sock.c b/src/ssl_sock.c >>> index e7eb5df..2b227fc 100644 >>> --- a/src/ssl_sock.c >>> +++ b/src/ssl_sock.c >>> @@ -1470,7 +1470,7 @@ static int ssl_sock_switchctx_cbk(SSL *ssl, int *al, >>> void *priv) >>> #endif >>> return (s->strict_sni ? >>> SSL_TLSEXT_ERR_ALERT_FATAL : >>> - SSL_TLSEXT_ERR_NOACK); >>> + SSL_TLSEXT_ERR_OK); >>> } >>> >>> for (i = 0; i < trash.size; i++) { >>> @@ -1507,7 +1507,7 @@ static int ssl_sock_switchctx_cbk(SSL *ssl, int *al, >>> void *priv) >>> #endif >>> return (s->strict_sni ? >>> SSL_TLSEXT_ERR_ALERT_FATAL : >>> - SSL_TLSEXT_ERR_ALERT_WARNING); >>> + SSL_TLSEXT_ERR_OK); >>> } >>> >>> /* switch ctx */ >> >
