Commit 128ae276 authored by Matt Caswell's avatar Matt Caswell
Browse files

Move session version consistency check 



Make sure the session version consistency check is inside
ssl_get_prev_session(). Also fixes a bug where an inconsistent version can
cause a seg fault in TLSv1.3.

Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2259)
parent b3ad72ce

ssl/ssl_sess.c

+7 −17
Original line number Diff line number Diff line
@@ -556,6 +556,10 @@ int ssl_get_prev_session(SSL *s, CLIENTHELLO_MSG *hello) 


    /* Now ret is non-NULL and we own one of its reference counts. */



    /* Check TLS version consistency */

    if (ret->ssl_version != s->version)

        goto err;



    if (ret->sid_ctx_length != s->sid_ctx_length

        || memcmp(ret->sid_ctx, s->sid_ctx, ret->sid_ctx_length)) {

        /*
@@ -606,23 +610,6 @@ int ssl_get_prev_session(SSL *s, CLIENTHELLO_MSG *hello) 
        goto err;

    }



    /*

     * TODO(TLS1.3): This is temporary, because TLSv1.3 resumption is completely

     * different. For now though we're still using the old resumption logic, so

     * to avoid test failures we need this. Remove this code!

     * 

     * Check TLS version consistency. We can't resume <=TLSv1.2 session if we

     * have negotiated TLSv1.3, and vice versa.

     */

    if (!SSL_IS_DTLS(s)

            && ((ret->ssl_version <= TLS1_2_VERSION

                 && s->version >=TLS1_3_VERSION)

                || (ret->ssl_version >= TLS1_3_VERSION

                    && s->version <= TLS1_2_VERSION))) {

        /* Continue but do not resume */

        goto err;

    }



    /* Check extended master secret extension consistency */

    if (ret->flags & SSL_SESS_FLAG_EXTMS) {

        /* If old session includes extms, but new does not: abort handshake */
@@ -651,6 +638,9 @@ int ssl_get_prev_session(SSL *s, CLIENTHELLO_MSG *hello) 
 err:

    if (ret != NULL) {

        SSL_SESSION_free(ret);

        /* In TLSv1.3 we already set s->session, so better NULL it out */

        if (SSL_IS_TLS13(s))

            s->session = NULL;



        if (!try_session_cache) {

            /*

ssl/statem/statem_srvr.c

+1 −10
Original line number Diff line number Diff line
@@ -1476,16 +1476,7 @@ MSG_PROCESS_RETURN tls_process_client_hello(SSL *s, PACKET *pkt) 
            goto err;

    } else {

        i = ssl_get_prev_session(s, &clienthello);

        /*

         * Only resume if the session's version matches the negotiated

         * version.

         * RFC 5246 does not provide much useful advice on resumption

         * with a different protocol version. It doesn't forbid it but

         * the sanity of such behaviour would be questionable.

         * In practice, clients do not accept a version mismatch and

         * will abort the handshake with an error.

         */

        if (i == 1 && s->version == s->session->ssl_version) {

        if (i == 1) {

            /* previous session */

            s->hit = 1;

        } else if (i == -1) {