Commit 70c22888 authored by Emilia Kasper's avatar Emilia Kasper
Browse files

Fix two bugs in clienthello processing 



- Always process ALPN (previously there was an early return in the
  certificate status handling)
- Don't send a duplicate alert. Previously, both
  ssl_check_clienthello_tlsext_late and its caller would send an
  alert. Consolidate alert sending code in the caller.

Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
parent ce2cdac2

ssl/ssl_locl.h

+1 −1
Original line number Diff line number Diff line
@@ -2004,7 +2004,7 @@ __owur unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *buf, 
__owur int ssl_parse_clienthello_tlsext(SSL *s, PACKET *pkt);

void ssl_set_default_md(SSL *s);

__owur int tls1_set_server_sigalgs(SSL *s);

__owur int ssl_check_clienthello_tlsext_late(SSL *s);

__owur int ssl_check_clienthello_tlsext_late(SSL *s, int *al);

__owur int ssl_parse_serverhello_tlsext(SSL *s, PACKET *pkt);

__owur int ssl_prepare_clienthello_tlsext(SSL *s);

__owur int ssl_prepare_serverhello_tlsext(SSL *s);

ssl/statem/statem_srvr.c

+1 −1
Original line number Diff line number Diff line
@@ -1454,7 +1454,7 @@ WORK_STATE tls_post_process_client_hello(SSL *s, WORK_STATE wst) 


        /* Handles TLS extensions that we couldn't check earlier */

        if (s->version >= SSL3_VERSION) {

            if (ssl_check_clienthello_tlsext_late(s) <= 0) {

            if (!ssl_check_clienthello_tlsext_late(s, &al)) {

                SSLerr(SSL_F_TLS_POST_PROCESS_CLIENT_HELLO,

                       SSL_R_CLIENTHELLO_TLSEXT);

                goto f_err;

ssl/t1_lib.c

+35 −52
Original line number Diff line number Diff line
@@ -1678,11 +1678,10 @@ static int tls1_alpn_handle_client_hello(SSL *s, PACKET *pkt, int *al) 


/*

 * Process the ALPN extension in a ClientHello.

 * ret: a pointer to the TLSEXT return value: SSL_TLSEXT_ERR_*

 * al: a pointer to the alert value to send in the event of a failure.

 * returns 1 on success, 0

 * returns 1 on success, 0 on error.

 */

static int tls1_alpn_handle_client_hello_late(SSL *s, int *ret, int *al)

static int tls1_alpn_handle_client_hello_late(SSL *s, int *al)

{

    const unsigned char *selected = NULL;

    unsigned char selected_len = 0;
@@ -1698,7 +1697,6 @@ static int tls1_alpn_handle_client_hello_late(SSL *s, int *ret, int *al) 
            s->s3->alpn_selected = OPENSSL_memdup(selected, selected_len);

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

                *al = SSL_AD_INTERNAL_ERROR;

                *ret = SSL_TLSEXT_ERR_ALERT_FATAL;

                return 0;

            }

            s->s3->alpn_selected_len = selected_len;
@@ -1708,7 +1706,6 @@ static int tls1_alpn_handle_client_hello_late(SSL *s, int *ret, int *al) 
#endif

        } else {

            *al = SSL_AD_NO_APPLICATION_PROTOCOL;

            *ret = SSL_TLSEXT_ERR_ALERT_FATAL;

            return 0;

        }

    }
@@ -2661,10 +2658,13 @@ int tls1_set_server_sigalgs(SSL *s) 
    return 0;

}



int ssl_check_clienthello_tlsext_late(SSL *s)

/*

 * Upon success, returns 1.

 * Upon failure, returns 0 and sets |al| to the appropriate fatal alert.

 */

int ssl_check_clienthello_tlsext_late(SSL *s, int *al)

{

    int ret = SSL_TLSEXT_ERR_OK;

    int al = SSL_AD_INTERNAL_ERROR;

    s->tlsext_status_expected = 0;



    /*

     * If status request then ask callback what to do. Note: this must be
@@ -2673,21 +2673,18 @@ int ssl_check_clienthello_tlsext_late(SSL *s) 
     * influence which certificate is sent

     */

    if ((s->tlsext_status_type != -1) && s->ctx && s->ctx->tlsext_status_cb) {

        int r;

        int ret;

        CERT_PKEY *certpkey;

        certpkey = ssl_get_server_send_pkey(s);

        /* If no certificate can't return certificate status */

        if (certpkey == NULL) {

            s->tlsext_status_expected = 0;

            return 1;

        }

        if (certpkey != NULL) {

            /*

             * Set current certificate to one we will use so SSL_get_certificate

             * et al can pick it up.

             */

            s->cert->key = certpkey;

        r = s->ctx->tlsext_status_cb(s, s->ctx->tlsext_status_arg);

        switch (r) {

            ret = s->ctx->tlsext_status_cb(s, s->ctx->tlsext_status_arg);

            switch (ret) {

                /* We don't want to send a status request response */

            case SSL_TLSEXT_ERR_NOACK:

                s->tlsext_status_expected = 0;
@@ -2696,36 +2693,22 @@ int ssl_check_clienthello_tlsext_late(SSL *s) 
            case SSL_TLSEXT_ERR_OK:

                if (s->tlsext_ocsp_resp)

                    s->tlsext_status_expected = 1;

            else

                s->tlsext_status_expected = 0;

                break;

                /* something bad happened */

            case SSL_TLSEXT_ERR_ALERT_FATAL:

            ret = SSL_TLSEXT_ERR_ALERT_FATAL;

            al = SSL_AD_INTERNAL_ERROR;

            goto err;

            default:

                *al = SSL_AD_INTERNAL_ERROR;

                return 0;

            }

        }

    } else

        s->tlsext_status_expected = 0;



    if (!tls1_alpn_handle_client_hello_late(s, &ret, &al)) {

        goto err;

    }



 err:

    switch (ret) {

    case SSL_TLSEXT_ERR_ALERT_FATAL:

        ssl3_send_alert(s, SSL3_AL_FATAL, al);

        return -1;



    case SSL_TLSEXT_ERR_ALERT_WARNING:

        ssl3_send_alert(s, SSL3_AL_WARNING, al);

        return 1;

    if (!tls1_alpn_handle_client_hello_late(s, al)) {

        return 0;

    }



    default:

    return 1;

}

}



int ssl_check_serverhello_tlsext(SSL *s)

{