From: Ben Kaduk <kaduk@mit.edu>
Date: Tue, 4 Sep 2018 17:30:00 +0000 (-0500)
Subject: Simplify SSL_get_servername() to avoid session references
X-Git-Tag: OpenSSL_1_1_1~20
X-Git-Url: https://git.librecmc.org/?a=commitdiff_plain;h=328a0547ad61d9e260fca73a280d2288714f2b92;p=oweals%2Fopenssl.git

Simplify SSL_get_servername() to avoid session references

Ideally, SSL_get_servername() would do exactly as it is documented
and return exactly what the client sent (i.e., what we currently
are stashing in the SSL's ext.hostname), without needing to refer
to an SSL_SESSION object.  For historical reasons, including the
parsed SNI value from the ClientHello originally being stored in the
SSL_SESSION's ext.hostname field, we have had references to the
SSL_SESSION in this function.  We cannot fully excise them due to
the interaction between user-supplied callbacks and TLS 1.2 resumption
flows, where we call all callbacks but the client did not supply an
SNI value.  Existing callbacks expect to receive a valid SNI value
in this case, so we must fake one up from the resumed session in
order to avoid breakage.

Otherwise, greatly simplify the implementation and just return the
value in the SSL, as sent by the client.

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/7115)
---

diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 7e8093bcfd..3d25da637d 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -2600,18 +2600,14 @@ const char *SSL_get_servername(const SSL *s, const int type)
         return NULL;
 
     /*
-     * TODO(OpenSSL1.2) clean up this compat mess.  This API is
-     * currently a mix of "what did I configure" and "what did the
-     * peer send" and "what was actually negotiated"; we should have
-     * a clear distinction amongst those three.
+     * SNI is not negotiated in pre-TLS-1.3 resumption flows, so fake up an
+     * SNI value to return if we are resuming/resumed.  N.B. that we still
+     * call the relevant callbacks for such resumption flows, and callbacks
+     * might error out if there is not a SNI value available.
      */
-    if (SSL_in_init(s)) {
-        if (s->hit)
-            return s->session->ext.hostname;
-        return s->ext.hostname;
-    }
-    return (s->session != NULL && s->ext.hostname == NULL) ?
-        s->session->ext.hostname : s->ext.hostname;
+    if (s->hit)
+        return s->session->ext.hostname;
+    return s->ext.hostname;
 }
 
 int SSL_get_servername_type(const SSL *s)