Commit 1bf4cb0f authored by Matt Caswell's avatar Matt Caswell
Browse files

Process KeyUpdate and NewSessionTicket messages after a close_notify 



If we've sent a close_notify then we are restricted about what we can do
in response to handshake messages that we receive. However we can sensibly
process NewSessionTicket messages. We can also process a KeyUpdate message
as long as we also ignore any request for us to update our sending keys.

Reviewed-by: default avatarTim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/7114)
parent f922dac8

ssl/record/rec_layer_s3.c

+22 −20
Original line number Diff line number Diff line
@@ -1554,23 +1554,21 @@ int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf, 
        return -1;

    }



    /*

     * If we've sent a close_notify but not yet received one back then ditch

     * anything we read.

     */

    if ((s->shutdown & SSL_SENT_SHUTDOWN) != 0) {

        if (SSL3_RECORD_get_type(rr) == SSL3_RT_HANDSHAKE) {

            BIO *rbio;



            /*

         * In TLSv1.3 this could get problematic if we receive a KeyUpdate

         * message after we sent a close_notify because we're about to ditch it,

         * so we won't be able to read a close_notify sent afterwards! We don't

         * support that.

             * We ignore any handshake messages sent to us unless they are

             * TLSv1.3 in which case we want to process them. For all other

             * handshake messages we can't do anything reasonable with them

             * because we are unable to write any response due to having already

             * sent close_notify.

             */

            if (!SSL_IS_TLS13(s)) {

                SSL3_RECORD_set_length(rr, 0);

                SSL3_RECORD_set_read(rr);



        if (SSL3_RECORD_get_type(rr) == SSL3_RT_HANDSHAKE) {

            BIO *rbio;



                if ((s->mode & SSL_MODE_AUTO_RETRY) != 0)

                    goto start;
@@ -1578,6 +1576,8 @@ int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf, 
                rbio = SSL_get_rbio(s);

                BIO_clear_retry_flags(rbio);

                BIO_set_retry_read(rbio);

                return -1;

            }

        } else {

            /*

             * The peer is continuing to send application data, but we have
@@ -1586,11 +1586,13 @@ int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf, 
             * above.

             * No alert sent because we already sent close_notify

             */

            SSL3_RECORD_set_length(rr, 0);

            SSL3_RECORD_set_read(rr);

            SSLfatal(s, SSL_AD_NO_ALERT, SSL_F_SSL3_READ_BYTES,

                     SSL_R_APPLICATION_DATA_AFTER_CLOSE_NOTIFY);

        }

            return -1;

        }

    }



    /*

     * For handshake data we have 'fragment' storage, so fill that so that we

ssl/statem/statem_clnt.c

+22 −5
Original line number Diff line number Diff line
@@ -423,11 +423,19 @@ static WRITE_TRAN ossl_statem_client13_write_transition(SSL *s) 
            st->hand_state = TLS_ST_CW_CERT;

            return WRITE_TRAN_CONTINUE;

        }

        /*

         * We should only get here if we received a CertificateRequest after

         * we already sent close_notify

         */

        if (!ossl_assert((s->shutdown & SSL_SENT_SHUTDOWN) != 0)) {

            /* Shouldn't happen - same as default case */

            SSLfatal(s, SSL_AD_INTERNAL_ERROR,

                     SSL_F_OSSL_STATEM_CLIENT13_WRITE_TRANSITION,

                     ERR_R_INTERNAL_ERROR);

            return WRITE_TRAN_ERROR;

        }

        st->hand_state = TLS_ST_OK;

        return WRITE_TRAN_CONTINUE;



    case TLS_ST_CR_FINISHED:

        if (s->early_data_state == SSL_EARLY_DATA_WRITE_RETRY
@@ -2446,6 +2454,15 @@ MSG_PROCESS_RETURN tls_process_certificate_request(SSL *s, PACKET *pkt) 
        PACKET reqctx, extensions;

        RAW_EXTENSION *rawexts = NULL;



        if ((s->shutdown & SSL_SENT_SHUTDOWN) != 0) {

            /*

             * We already sent close_notify. This can only happen in TLSv1.3

             * post-handshake messages. We can't reasonably respond to this, so

             * we just ignore it

             */

            return MSG_PROCESS_FINISHED_READING;

        }



        /* Free and zero certificate types: it is not present in TLS 1.3 */

        OPENSSL_free(s->s3->tmp.ctype);

        s->s3->tmp.ctype = NULL;

ssl/statem/statem_lib.c

+5 −2
Original line number Diff line number Diff line
@@ -638,9 +638,12 @@ MSG_PROCESS_RETURN tls_process_key_update(SSL *s, PACKET *pkt) 
    /*

     * If we get a request for us to update our sending keys too then, we need

     * to additionally send a KeyUpdate message. However that message should

     * not also request an update (otherwise we get into an infinite loop).

     * not also request an update (otherwise we get into an infinite loop). We

     * ignore a request for us to update our sending keys too if we already

     * sent close_notify.

     */

    if (updatetype == SSL_KEY_UPDATE_REQUESTED)

    if (updatetype == SSL_KEY_UPDATE_REQUESTED

            && (s->shutdown & SSL_SENT_SHUTDOWN) == 0)

        s->key_update = SSL_KEY_UPDATE_NOT_REQUESTED;



    if (!tls13_update_key(s, 0)) {