Hi,
in master (and 1.1.1), SSL_get_servername() returns either
s->session->ext.hostname (when s->hit == 1), or s->ext.hostname
(otherwise).
It seems, according to final_server_name(), that
s->session->ext.hostname is set only:
if (sent && ret == SSL_TLSEXT_ERR_OK && (!s->hit || SSL_IS_TLS13(s))) {
...
s->session->ext.hostname = OPENSSL_strdup(s->ext.hostname);
...
}
that is if a SNI callback exists and returns OK (no callback
defaulting to NOACK).
And it _seems_ that browsers (or Chrome only?) don't set a
tlsext_hostname in their session (ASN1) on resumption, so
s->session->ext.hostname isn't set by that either. Note that this
leaves s->servername_done = 0 in the below code from
tls_parse_ctos_server_name():
if (s->hit) {
/*
* TODO(openssl-team): if the SNI doesn't match, we MUST
* fall back to a full handshake.
*/
s->servername_done = (s->session->ext.hostname != NULL)
&& PACKET_equal(&hostname, s->session->ext.hostname,
strlen(s->session->ext.hostname));
if (!s->servername_done && s->session->ext.hostname != NULL)
s->ext.early_data_ok = 0;
}
So it's possible that, on session resumption (s->hit == 1),
SSL_get_servername() returns NULL (unset s->session->ext.hostname)
even though an SNI is provided by ClientHello. Shouldn't it return the
ClientHello's SNI regardless of the presence/return of the callback?
Something like the below (where s->servername_done is also tested)?
const char *SSL_get_servername(const SSL *s, const int type)
{
if (type != TLSEXT_NAMETYPE_host_name)
return NULL;
/*
* 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 (s->hit && s->servername_done)
return s->session->ext.hostname;
return s->ext.hostname;
}
Regards,
Yann.
