Commit 124037fd authored by Dr. Stephen Henson's avatar Dr. Stephen Henson
Browse files

Tidy up ssl3_digest_cached_records logic. 

Rewrite ssl3_digest_cached_records handling. Only digest cached records
if digest array is NULL: this means it is safe to call
ssl3_digest_cached_records multiple times (subsequent calls are no op).

Remove flag TLS1_FLAGS_KEEP_HANDSHAKE instead only update handshake buffer
if digest array is NULL.

Add additional "keep" parameter to ssl3_digest_cached_records to indicate
if the handshake buffer should be retained after digesting cached records
(needed for TLS 1.2 client authentication).

Reviewed-by: default avatarMatt Caswell <matt@openssl.org>
parent 74924dcb

include/openssl/ssl3.h

+0 −1
Changes for include/openssl/ssl3.h: 0 added lines, 1 removed line.
Original line number Diff line number Diff line
@@ -365,7 +365,6 @@ extern "C" { 
/* Removed from OpenSSL 1.1.0 */

# define TLS1_FLAGS_TLS_PADDING_BUG              0x0

# define TLS1_FLAGS_SKIP_CERT_VERIFY             0x0010

# define TLS1_FLAGS_KEEP_HANDSHAKE               0x0020

/*

 * Set when the handshake is ready to process peer's ChangeCipherSpec message.

 * Cleared after the message has been processed.

ssl/d1_srvr.c

+1 −4
Changes for ssl/d1_srvr.c: 1 added line, 4 removed lines.
Original line number Diff line number Diff line
@@ -640,13 +640,10 @@ int dtls1_accept(SSL *s) 
                 * For sigalgs freeze the handshake buffer. If we support

                 * extms we've done this already.

                 */

                if (!(s->s3->flags & SSL_SESS_FLAG_EXTMS)) {

                    s->s3->flags |= TLS1_FLAGS_KEEP_HANDSHAKE;

                    if (!ssl3_digest_cached_records(s)) {

                if (!ssl3_digest_cached_records(s, 1)) {

                    s->state = SSL_ST_ERR;

                    return -1;

                }

                }

            } else {

                s->state = SSL3_ST_SR_CERT_VRFY_A;

                s->init_num = 0;

ssl/s3_clnt.c

+5 −14
Changes for ssl/s3_clnt.c: 5 added lines, 14 removed lines.
Original line number Diff line number Diff line
@@ -1168,7 +1168,7 @@ int ssl3_get_server_hello(SSL *s) 
     * Don't digest cached records if no sigalgs: we may need them for client

     * authentication.

     */

    if (!SSL_USE_SIGALGS(s) && !ssl3_digest_cached_records(s))

    if (!SSL_USE_SIGALGS(s) && !ssl3_digest_cached_records(s, 0))

        goto f_err;

    /* lets get the compression algorithm */

    /* COMPRESSION */
@@ -2030,10 +2030,8 @@ int ssl3_get_certificate_request(SSL *s) 
         * If we get here we don't need any cached handshake records as we

         * wont be doing client auth.

         */

        if (s->s3->handshake_buffer) {

            if (!ssl3_digest_cached_records(s))

        if (!ssl3_digest_cached_records(s, 0))

            goto err;

        }

        return (1);

    }
@@ -3026,15 +3024,8 @@ int ssl3_send_client_verify(SSL *s) 
            }

            s2n(u, p);

            n = u + 4;

            /*

             * For extended master secret we've already digested cached

             * records.

             */

            if (s->session->flags & SSL_SESS_FLAG_EXTMS) {

                BIO_free(s->s3->handshake_buffer);

                s->s3->handshake_buffer = NULL;

                s->s3->flags &= ~TLS1_FLAGS_KEEP_HANDSHAKE;

            } else if (!ssl3_digest_cached_records(s))

            /* Digest cached records and discard handshake buffer */

            if (!ssl3_digest_cached_records(s, 0))

                goto err;

        } else

#ifndef OPENSSL_NO_RSA
@@ -3216,7 +3207,7 @@ int ssl3_send_client_certificate(SSL *s) 
                return (1);

            } else {

                s->s3->tmp.cert_req = 2;

                if (s->s3->handshake_buffer && !ssl3_digest_cached_records(s)) {

                if (!ssl3_digest_cached_records(s, 0)) {

                    ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);

                    s->state = SSL_ST_ERR;

                    return 0;

ssl/s3_enc.c

+8 −11
Changes for ssl/s3_enc.c: 8 added lines, 11 removed lines.
Original line number Diff line number Diff line
@@ -497,8 +497,7 @@ void ssl3_free_digest_list(SSL *s) 


void ssl3_finish_mac(SSL *s, const unsigned char *buf, int len)

{

    if (s->s3->handshake_buffer

        && !(s->s3->flags & TLS1_FLAGS_KEEP_HANDSHAKE)) {

    if (s->s3->handshake_dgst == NULL) {

        BIO_write(s->s3->handshake_buffer, (void *)buf, len);

    } else {

        int i;
@@ -509,7 +508,7 @@ void ssl3_finish_mac(SSL *s, const unsigned char *buf, int len) 
    }

}



int ssl3_digest_cached_records(SSL *s)

int ssl3_digest_cached_records(SSL *s, int keep)

{

    int i;

    long mask;
@@ -517,23 +516,21 @@ int ssl3_digest_cached_records(SSL *s) 
    long hdatalen;

    void *hdata;



    if (s->s3->handshake_dgst == NULL) {

        /* Allocate handshake_dgst array */

    ssl3_free_digest_list(s);

        s->s3->handshake_dgst =

            OPENSSL_malloc(sizeof(*s->s3->handshake_dgst) * SSL_MAX_DIGEST);

        if (s->s3->handshake_dgst == NULL) {

            SSLerr(SSL_F_SSL3_DIGEST_CACHED_RECORDS, ERR_R_MALLOC_FAILURE);

            return 0;

        }

    memset(s->s3->handshake_dgst, 0,

           sizeof(*s->s3->handshake_dgst) * SSL_MAX_DIGEST);

        hdatalen = BIO_get_mem_data(s->s3->handshake_buffer, &hdata);

        if (hdatalen <= 0) {

            SSLerr(SSL_F_SSL3_DIGEST_CACHED_RECORDS, SSL_R_BAD_HANDSHAKE_LENGTH);

            return 0;

        }



    /* Loop through bitso of algorithm2 field and create MD_CTX-es */

        /* Loop through bits of algorithm2 field and create MD_CTX-es */

        for (i = 0; ssl_get_handshake_digest(i, &mask, &md); i++) {

            if ((mask & ssl_get_algorithm2(s)) && md) {

                s->s3->handshake_dgst[i] = EVP_MD_CTX_create();
@@ -547,8 +544,9 @@ int ssl3_digest_cached_records(SSL *s) 
                s->s3->handshake_dgst[i] = NULL;

            }

        }

    if (!(s->s3->flags & TLS1_FLAGS_KEEP_HANDSHAKE)) {

        /* Free handshake_buffer BIO */



    }

    if (keep == 0) {

        BIO_free(s->s3->handshake_buffer);

        s->s3->handshake_buffer = NULL;

    }
@@ -588,8 +586,7 @@ static int ssl3_handshake_mac(SSL *s, int md_nid, 
    unsigned char md_buf[EVP_MAX_MD_SIZE];

    EVP_MD_CTX ctx, *d = NULL;



    if (s->s3->handshake_buffer)

        if (!ssl3_digest_cached_records(s))

    if (!ssl3_digest_cached_records(s, 0))

        return 0;



    /*

ssl/s3_srvr.c

+6 −14
Changes for ssl/s3_srvr.c: 6 added lines, 14 removed lines.
Original line number Diff line number Diff line
@@ -507,12 +507,10 @@ int ssl3_accept(SSL *s) 
                skip = 1;

                s->s3->tmp.cert_request = 0;

                s->state = SSL3_ST_SW_SRVR_DONE_A;

                if (s->s3->handshake_buffer) {

                    if (!ssl3_digest_cached_records(s)) {

                if (!ssl3_digest_cached_records(s, 0)) {

                    s->state = SSL_ST_ERR;

                    return -1;

                }

                }

            } else {

                s->s3->tmp.cert_request = 1;

                ret = ssl3_send_certificate_request(s);
@@ -598,15 +596,12 @@ int ssl3_accept(SSL *s) 
                }

                /*

                 * For sigalgs freeze the handshake buffer. If we support

                 * extms we've done this already.

                 * extms we've done this already so this is a no-op

                 */

                if (!(s->s3->flags & SSL_SESS_FLAG_EXTMS)) {

                    s->s3->flags |= TLS1_FLAGS_KEEP_HANDSHAKE;

                    if (!ssl3_digest_cached_records(s)) {

                if (!ssl3_digest_cached_records(s, 1)) {

                    s->state = SSL_ST_ERR;

                    return -1;

                }

                }

            } else {

                int offset = 0;

                int dgst_num;
@@ -620,12 +615,10 @@ int ssl3_accept(SSL *s) 
                 * CertificateVerify should be generalized. But it is next

                 * step

                 */

                if (s->s3->handshake_buffer) {

                    if (!ssl3_digest_cached_records(s)) {

                if (!ssl3_digest_cached_records(s, 0)) {

                    s->state = SSL_ST_ERR;

                    return -1;

                }

                }

                for (dgst_num = 0; dgst_num < SSL_MAX_DIGEST; dgst_num++)

                    if (s->s3->handshake_dgst[dgst_num]) {

                        int dgst_size;
@@ -1538,7 +1531,7 @@ int ssl3_get_client_hello(SSL *s) 
    }



    if (!SSL_USE_SIGALGS(s) || !(s->verify_mode & SSL_VERIFY_PEER)) {

        if (!ssl3_digest_cached_records(s))

        if (!ssl3_digest_cached_records(s, 0))

            goto f_err;

    }
@@ -3055,7 +3048,6 @@ int ssl3_get_cert_verify(SSL *s) 
 end:

    BIO_free(s->s3->handshake_buffer);

    s->s3->handshake_buffer = NULL;

    s->s3->flags &= ~TLS1_FLAGS_KEEP_HANDSHAKE;

    EVP_MD_CTX_cleanup(&mctx);

    EVP_PKEY_free(pkey);

    return (ret);
@@ -3163,7 +3155,7 @@ int ssl3_get_client_certificate(SSL *s) 
            goto f_err;

        }

        /* No client certificate so digest cached records */

        if (s->s3->handshake_buffer && !ssl3_digest_cached_records(s)) {

        if (s->s3->handshake_buffer && !ssl3_digest_cached_records(s, 0)) {

            al = SSL_AD_INTERNAL_ERROR;

            goto f_err;

        }