Improve backwards compat for SSL_get_servername()

Commit 1c4aa31d79821dee9be98e915159d52cc30d8403 changed how we process
and store SNI information during the handshake, so that a hostname is
only saved in the SSL_SESSION structure if that SNI value has actually
been negotiated.  SSL_get_servername() was adjusted to match, with a new
conditional being added to handle the case when the handshake processing
is ongoing, and a different location should be consulted for the offered
SNI value.  This was done in an attempt to preserve the historical
behavior of SSL_get_servername(), a function whose behavior only mostly
matches its documentation, and whose documentation is both lacking and
does not necessarily reflect the actual desired behavior for such an
API.  Unfortunately, sweeping changes that would bring more sanity to
this space are not possible until OpenSSL 1.2.0, for ABI compatibility
reasons, so we must attempt to maintain the existing behavior to the
extent possible.

The above-mentioned commit did not take into account the behavior
of SSL_get_servername() during resumption handshakes for TLS 1.2 and
prior, where no SNI negotiation is performed.  In that case we would
not properly parse the incoming SNI and erroneously return NULL as
the servername, when instead the logical session is associated with
the SNI value cached in the SSL_SESSION.  (Note that in some cases an
SNI callback may not need to do anything in a TLS 1.2 or prior resumption
flow, but we are calling the callbacks and did not provide any guidance
that they should no-op if the connection is being resumed, so we must
handle this case in a usable fashion.)  Update our behavior accordingly to
return the session's cached value during the handshake, when resuming.
This fixes the boringssl tests.

[extended tests]

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6792)

diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 10a76940dd144690b9edeab11a357f58679a0be0..15380e1b72d4e8bc60414e0ddfa2a844a6d2dd0d 100644 (file)
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -2618,8 +2618,11 @@ const char *SSL_get_servername(const SSL *s, const int type)
      * peer send" and "what was actually negotiated"; we should have
      * a clear distinction amongst those three.
      */
      * peer send" and "what was actually negotiated"; we should have
      * a clear distinction amongst those three.
      */

-    if (SSL_in_init(s))
+    if (SSL_in_init(s)) {
+        if (s->hit)
+            return s->session->ext.hostname;

         return s->ext.hostname;
         return s->ext.hostname;

+    }

     return (s->session != NULL && s->ext.hostname == NULL) ?
         s->session->ext.hostname : s->ext.hostname;
 }
     return (s->session != NULL && s->ext.hostname == NULL) ?
         s->session->ext.hostname : s->ext.hostname;
 }