Commit f01344cb authored by Matt Caswell's avatar Matt Caswell
Browse files

Do not reset SNI data in SSL_do_handshake() 



PR #3783 introduce coded to reset the server side SNI state in
SSL_do_handshake() to ensure any erroneous config time SNI changes are
cleared. Unfortunately SSL_do_handshake() can be called mid-handshake
multiple times so this is the wrong place to do this and can mean that
any SNI data is cleared later on in the handshake too.

Therefore move the code to a more appropriate place.

Fixes #7014

Reviewed-by: default avatarTim Hudson <tjh@openssl.org>
Reviewed-by: default avatarViktor Dukhovni <viktor@openssl.org>
Reviewed-by: default avatarBen Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/7149)
parent 328a0547

ssl/ssl_lib.c

+0 −6
Original line number Diff line number Diff line
@@ -3559,12 +3559,6 @@ int SSL_do_handshake(SSL *s) 


    s->method->ssl_renegotiate_check(s, 0);



    if (SSL_is_server(s)) {

        /* clear SNI settings at server-side */

        OPENSSL_free(s->ext.hostname);

        s->ext.hostname = NULL;

    }



    if (SSL_in_init(s) || SSL_in_before(s)) {

        if ((s->mode & SSL_MODE_ASYNC) && ASYNC_get_current_job() == NULL) {

            struct ssl_async_args args;

ssl/statem/extensions.c

+5 −1
Original line number Diff line number Diff line
@@ -904,9 +904,13 @@ static int final_renegotiate(SSL *s, unsigned int context, int sent) 


static int init_server_name(SSL *s, unsigned int context)

{

    if (s->server)

    if (s->server) {

        s->servername_done = 0;



        OPENSSL_free(s->ext.hostname);

        s->ext.hostname = NULL;

    }



    return 1;

}

test/build.info

+1 −1
Original line number Diff line number Diff line
@@ -364,7 +364,7 @@ INCLUDE_MAIN___test_libtestutil_OLB = /INCLUDE=MAIN 
  INCLUDE[ciphername_test]=../include

  DEPEND[ciphername_test]=../libcrypto ../libssl libtestutil.a



  SOURCE[servername_test]=servername_test.c

  SOURCE[servername_test]=servername_test.c ssltestlib.c

  INCLUDE[servername_test]=../include

  DEPEND[servername_test]=../libcrypto ../libssl libtestutil.a

test/recipes/70-test_servername.t

+6 −2
Original line number Diff line number Diff line
@@ -11,7 +11,7 @@ use strict; 
use warnings;



use OpenSSL::Test::Simple;

use OpenSSL::Test;

use OpenSSL::Test qw/:DEFAULT srctop_file/;

use OpenSSL::Test::Utils qw(alldisabled available_protocols);



setup("test_servername");
@@ -19,4 +19,8 @@ setup("test_servername"); 
plan skip_all => "No TLS/SSL protocols are supported by this OpenSSL build"

    if alldisabled(grep { $_ ne "ssl3" } available_protocols("tls"));



simple_test("test_servername", "servername_test");
 
plan tests => 1;



ok(run(test(["servername_test", srctop_file("apps", "server.pem"),

             srctop_file("apps", "server.pem")])),

             "running servername_test");

test/servername_test.c

+32 −31
Original line number Diff line number Diff line
@@ -22,11 +22,15 @@ 


#include "testutil.h"

#include "internal/nelem.h"

#include "ssltestlib.h"



#define CLIENT_VERSION_LEN      2



static const char *host = "dummy-host";



static char *cert = NULL;

static char *privkey = NULL;



static int get_sni_from_client_hello(BIO *bio, char **sni)

{

    long len;
@@ -176,45 +180,38 @@ end: 


static int server_setup_sni(void)

{

    SSL_CTX *ctx;

    SSL *con = NULL;

    BIO *rbio;

    BIO *wbio;

    int ret = 0;

    SSL_CTX *cctx = NULL, *sctx = NULL;

    SSL *clientssl = NULL, *serverssl = NULL;

    int testresult = 0;



    /* use TLS_server_method to choose 'server-side' */

    ctx = SSL_CTX_new(TLS_server_method());

    if (!TEST_ptr(ctx))

    if (!TEST_true(create_ssl_ctx_pair(TLS_server_method(),

                                       TLS_client_method(),

                                       TLS1_VERSION, TLS_MAX_VERSION,

                                       &sctx, &cctx, cert, privkey))

            || !TEST_true(create_ssl_objects(sctx, cctx, &serverssl, &clientssl,

                                             NULL, NULL)))

        goto end;



    con = SSL_new(ctx);

    if (!TEST_ptr(con))

    /* set SNI at server side */

    SSL_set_tlsext_host_name(serverssl, host);



    if (!TEST_true(create_ssl_connection(serverssl, clientssl, SSL_ERROR_NONE)))

        goto end;



    rbio = BIO_new(BIO_s_mem());

    wbio = BIO_new(BIO_s_mem());

    if (!TEST_ptr(rbio)|| !TEST_ptr(wbio)) {

        BIO_free(rbio);

        BIO_free(wbio);

    if (!TEST_ptr_null(SSL_get_servername(serverssl,

                                          TLSEXT_NAMETYPE_host_name))) {

        /* SNI should have been cleared during handshake */

        goto end;

    }

    

    SSL_set_bio(con, rbio, wbio);



    /* set SNI at server side */

    SSL_set_tlsext_host_name(con, host);



    if (!TEST_int_le(SSL_accept(con), 0))

        /* This shouldn't succeed because we have nothing to listen on */

        goto end;

    if (!TEST_ptr_null(SSL_get_servername(con, TLSEXT_NAMETYPE_host_name)))

        /* SNI should be cleared by SSL_accpet */

        goto end;

    ret = 1;

    testresult = 1;

end:

    SSL_free(con);

    SSL_CTX_free(ctx);

    return ret;

    SSL_free(serverssl);

    SSL_free(clientssl);

    SSL_CTX_free(sctx);

    SSL_CTX_free(cctx);



    return testresult;

}



typedef int (*sni_test_fn)(void);
@@ -236,6 +233,10 @@ static int test_servername(int test) 


int setup_tests(void)

{

    if (!TEST_ptr(cert = test_get_argument(0))

            || !TEST_ptr(privkey = test_get_argument(1)))

        return 0;



    ADD_ALL_TESTS(test_servername, OSSL_NELEM(sni_test_fns));

    return 1;

}