Commit a84e5c9a authored by Todd Short's avatar Todd Short Committed by Pauli
Browse files

Session resume broken switching contexts 



When an SSL's context is swtiched from a ticket-enabled context to
a ticket-disabled context in the servername callback, no session-id
is generated, so the session can't be resumed.

If a servername callback changes the SSL_OP_NO_TICKET option, check
to see if it's changed to disable, and whether a session ticket is
expected (i.e. the client indicated ticket support and the SSL had
tickets enabled at the time), and whether we already have a previous
session (i.e. s->hit is set).

In this case, clear the ticket-expected flag, remove any ticket data
and generate a session-id in the session.

If the SSL hit (resumed) and switched to a ticket-disabled context,
assume that the resumption was via session-id, and don't bother to
update the session.

Before this fix, the updated unit-tests in 06-sni-ticket.conf would
fail test #4 (server1 = SNI, server2 = no SNI).

Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
Reviewed-by: default avatarRichard Levitte <levitte@openssl.org>
Reviewed-by: default avatarMatt Caswell <matt@openssl.org>
Reviewed-by: default avatarPaul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/1529)
parent 270a4bba

crypto/err/openssl.txt

+1 −0
Original line number Diff line number Diff line
@@ -1071,6 +1071,7 @@ SSL_F_SSL_DO_CONFIG:391:ssl_do_config 
SSL_F_SSL_DO_HANDSHAKE:180:SSL_do_handshake

SSL_F_SSL_DUP_CA_LIST:408:SSL_dup_CA_list

SSL_F_SSL_ENABLE_CT:402:SSL_enable_ct

SSL_F_SSL_GENERATE_SESSION_ID:547:ssl_generate_session_id

SSL_F_SSL_GET_NEW_SESSION:181:ssl_get_new_session

SSL_F_SSL_GET_PREV_SESSION:217:ssl_get_prev_session

SSL_F_SSL_GET_SERVER_CERT_INDEX:322:*

include/openssl/sslerr.h

+1 −0
Original line number Diff line number Diff line
@@ -148,6 +148,7 @@ int ERR_load_SSL_strings(void); 
# define SSL_F_SSL_DO_HANDSHAKE                           180

# define SSL_F_SSL_DUP_CA_LIST                            408

# define SSL_F_SSL_ENABLE_CT                              402

# define SSL_F_SSL_GENERATE_SESSION_ID                    547

# define SSL_F_SSL_GET_NEW_SESSION                        181

# define SSL_F_SSL_GET_PREV_SESSION                       217

# define SSL_F_SSL_GET_SERVER_CERT_INDEX                  322

ssl/ssl_err.c

+2 −0
Original line number Diff line number Diff line
@@ -208,6 +208,8 @@ static const ERR_STRING_DATA SSL_str_functs[] = { 
    {ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_DO_HANDSHAKE, 0), "SSL_do_handshake"},

    {ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_DUP_CA_LIST, 0), "SSL_dup_CA_list"},

    {ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_ENABLE_CT, 0), "SSL_enable_ct"},

    {ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_GENERATE_SESSION_ID, 0),

     "ssl_generate_session_id"},

    {ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_GET_NEW_SESSION, 0),

     "ssl_get_new_session"},

    {ERR_PACK(ERR_LIB_SSL, SSL_F_SSL_GET_PREV_SESSION, 0),

ssl/ssl_locl.h

+1 −0
Original line number Diff line number Diff line
@@ -2095,6 +2095,7 @@ __owur CERT *ssl_cert_new(void); 
__owur CERT *ssl_cert_dup(CERT *cert);

void ssl_cert_clear_certs(CERT *c);

void ssl_cert_free(CERT *c);

__owur int ssl_generate_session_id(SSL *s, SSL_SESSION *ss);

__owur int ssl_get_new_session(SSL *s, int session);

__owur int ssl_get_prev_session(SSL *s, CLIENTHELLO_MSG *hello, int *al);

__owur SSL_SESSION *ssl_session_dup(SSL_SESSION *src, int ticket);

ssl/ssl_sess.c

+84 −91
Original line number Diff line number Diff line
@@ -305,55 +305,25 @@ static int def_generate_session_id(SSL *ssl, unsigned char *id, 
    return 0;

}



int ssl_get_new_session(SSL *s, int session)

int ssl_generate_session_id(SSL *s, SSL_SESSION *ss)

{

    /* This gets used by clients and servers. */



    unsigned int tmp;

    SSL_SESSION *ss = NULL;

    GEN_SESSION_CB cb = def_generate_session_id;



    if ((ss = SSL_SESSION_new()) == NULL)

        return (0);



    /* If the context has a default timeout, use it */

    if (s->session_ctx->session_timeout == 0)

        ss->timeout = SSL_get_default_timeout(s);

    else

        ss->timeout = s->session_ctx->session_timeout;



    SSL_SESSION_free(s->session);

    s->session = NULL;



    if (session) {

        if (s->version == SSL3_VERSION) {

            ss->ssl_version = SSL3_VERSION;

    switch (s->version) {

    case SSL3_VERSION:

    case TLS1_VERSION:

    case TLS1_1_VERSION:

    case TLS1_2_VERSION:

    case TLS1_3_VERSION:

    case DTLS1_BAD_VER:

    case DTLS1_VERSION:

    case DTLS1_2_VERSION:

        ss->session_id_length = SSL3_SSL_SESSION_ID_LENGTH;

        } else if (s->version == TLS1_VERSION) {

            ss->ssl_version = TLS1_VERSION;

            ss->session_id_length = SSL3_SSL_SESSION_ID_LENGTH;

        } else if (s->version == TLS1_1_VERSION) {

            ss->ssl_version = TLS1_1_VERSION;

            ss->session_id_length = SSL3_SSL_SESSION_ID_LENGTH;

        } else if (s->version == TLS1_2_VERSION) {

            ss->ssl_version = TLS1_2_VERSION;

            ss->session_id_length = SSL3_SSL_SESSION_ID_LENGTH;

        } else if (s->version == TLS1_3_VERSION) {

            ss->ssl_version = TLS1_3_VERSION;

            ss->session_id_length = SSL3_SSL_SESSION_ID_LENGTH;

        } else if (s->version == DTLS1_BAD_VER) {

            ss->ssl_version = DTLS1_BAD_VER;

            ss->session_id_length = SSL3_SSL_SESSION_ID_LENGTH;

        } else if (s->version == DTLS1_VERSION) {

            ss->ssl_version = DTLS1_VERSION;

            ss->session_id_length = SSL3_SSL_SESSION_ID_LENGTH;

        } else if (s->version == DTLS1_2_VERSION) {

            ss->ssl_version = DTLS1_2_VERSION;

            ss->session_id_length = SSL3_SSL_SESSION_ID_LENGTH;

        } else {

            SSLerr(SSL_F_SSL_GET_NEW_SESSION, SSL_R_UNSUPPORTED_SSL_VERSION);

            SSL_SESSION_free(ss);

            return (0);

        break;

    default:

        SSLerr(SSL_F_SSL_GENERATE_SESSION_ID, SSL_R_UNSUPPORTED_SSL_VERSION);

        return 0;

    }



    /*-
@@ -373,7 +343,7 @@ int ssl_get_new_session(SSL *s, int session) 
     */

    if (s->ext.ticket_expected) {

        ss->session_id_length = 0;

            goto sess_id_done;

        return 1;

    }



    /* Choose which callback will set the session ID */
@@ -390,10 +360,9 @@ int ssl_get_new_session(SSL *s, int session) 
    tmp = (int)ss->session_id_length;

    if (!cb(s, ss->session_id, &tmp)) {

        /* The callback failed */

            SSLerr(SSL_F_SSL_GET_NEW_SESSION,

        SSLerr(SSL_F_SSL_GENERATE_SESSION_ID,

               SSL_R_SSL_SESSION_ID_CALLBACK_FAILED);

            SSL_SESSION_free(ss);

            return (0);

        return 0;

    }

    /*

     * Don't allow the callback to set the session length to zero. nor
@@ -401,21 +370,45 @@ int ssl_get_new_session(SSL *s, int session) 
     */

    if (tmp == 0 || tmp > ss->session_id_length) {

        /* The callback set an illegal length */

            SSLerr(SSL_F_SSL_GET_NEW_SESSION,

        SSLerr(SSL_F_SSL_GENERATE_SESSION_ID,

               SSL_R_SSL_SESSION_ID_HAS_BAD_LENGTH);

            SSL_SESSION_free(ss);

            return (0);

        return 0;

    }

    ss->session_id_length = tmp;

    /* Finally, check for a conflict */

    if (SSL_has_matching_session_id(s, ss->session_id,

                                    (unsigned int)ss->session_id_length)) {

            SSLerr(SSL_F_SSL_GET_NEW_SESSION, SSL_R_SSL_SESSION_ID_CONFLICT);

        SSLerr(SSL_F_SSL_GENERATE_SESSION_ID, SSL_R_SSL_SESSION_ID_CONFLICT);

        return 0;

    }



    return 1;

}



int ssl_get_new_session(SSL *s, int session)

{

    /* This gets used by clients and servers. */



    SSL_SESSION *ss = NULL;



    if ((ss = SSL_SESSION_new()) == NULL)

        return 0;



    /* If the context has a default timeout, use it */

    if (s->session_ctx->session_timeout == 0)

        ss->timeout = SSL_get_default_timeout(s);

    else

        ss->timeout = s->session_ctx->session_timeout;



    SSL_SESSION_free(s->session);

    s->session = NULL;



    if (session) {

        if (!ssl_generate_session_id(s, ss)) {

            SSL_SESSION_free(ss);

            return (0);

            return 0;

        }



 sess_id_done:

        if (s->ext.hostname) {

            ss->ext.hostname = OPENSSL_strdup(s->ext.hostname);

            if (ss->ext.hostname == NULL) {
@@ -443,7 +436,7 @@ int ssl_get_new_session(SSL *s, int session) 
    if (s->s3->flags & TLS1_FLAGS_RECEIVED_EXTMS)

        ss->flags |= SSL_SESS_FLAG_EXTMS;



    return (1);

    return 1;

}



/*-