Commit fadd9a1e authored by Matt Caswell's avatar Matt Caswell
Browse files

Verify that extensions are used in the correct context 



Perl changes reviewed by Richard Levitte. Non-perl changes reviewed by Rich
Salz

Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
Reviewed-by: default avatarRichard Levitte <levitte@openssl.org>
parent 91b60e2a

include/openssl/ssl.h

+1 −0
Original line number Diff line number Diff line
@@ -2324,6 +2324,7 @@ int ERR_load_SSL_strings(void); 
# define SSL_R_BAD_DIGEST_LENGTH                          111

# define SSL_R_BAD_ECC_CERT                               304

# define SSL_R_BAD_ECPOINT                                306

# define SSL_R_BAD_EXTENSION                              110

# define SSL_R_BAD_HANDSHAKE_LENGTH                       332

# define SSL_R_BAD_HELLO_REQUEST                          105

# define SSL_R_BAD_KEY_SHARE                              108

ssl/ssl_err.c

+1 −0
Original line number Diff line number Diff line
@@ -354,6 +354,7 @@ static ERR_STRING_DATA SSL_str_reasons[] = { 
    {ERR_REASON(SSL_R_BAD_DIGEST_LENGTH), "bad digest length"},

    {ERR_REASON(SSL_R_BAD_ECC_CERT), "bad ecc cert"},

    {ERR_REASON(SSL_R_BAD_ECPOINT), "bad ecpoint"},

    {ERR_REASON(SSL_R_BAD_EXTENSION), "bad extension"},

    {ERR_REASON(SSL_R_BAD_HANDSHAKE_LENGTH), "bad handshake length"},

    {ERR_REASON(SSL_R_BAD_HELLO_REQUEST), "bad hello request"},

    {ERR_REASON(SSL_R_BAD_KEY_SHARE), "bad key share"},

ssl/statem/statem_extensions.c

+141 −4
Original line number Diff line number Diff line
@@ -11,6 +11,107 @@ 
#include "../ssl_locl.h"

#include "statem_locl.h"



typedef struct {

    /* The ID for the extension */

    unsigned int type;

    int (*server_parse)(SSL *s, PACKET *pkt);

    int (*client_parse)(SSL *s, PACKET *pkt);

    unsigned int context;

} EXTENSION_DEFINITION;





static const EXTENSION_DEFINITION ext_defs[] = {

    {

        TLSEXT_TYPE_renegotiate,

        NULL, NULL,

        EXT_CLIENT_HELLO | EXT_TLS1_2_SERVER_HELLO

    },

    {

        TLSEXT_TYPE_server_name,

        NULL, NULL,

        EXT_CLIENT_HELLO | EXT_TLS1_2_SERVER_HELLO

        | EXT_TLS1_3_ENCRYPTED_EXTENSIONS

    },

    {

        TLSEXT_TYPE_srp,

        NULL, NULL,

        EXT_CLIENT_HELLO | EXT_TLS1_2_SERVER_HELLO

    },

    {

        TLSEXT_TYPE_ec_point_formats,

        NULL, NULL,

        EXT_CLIENT_HELLO

    },

    {

        TLSEXT_TYPE_supported_groups,

        NULL, NULL,

        EXT_CLIENT_HELLO | EXT_TLS1_3_ENCRYPTED_EXTENSIONS

    },

    {

        TLSEXT_TYPE_session_ticket,

        NULL, NULL,

        EXT_CLIENT_HELLO | EXT_TLS1_2_SERVER_HELLO

    },

    {

        TLSEXT_TYPE_signature_algorithms,

        NULL, NULL,

        EXT_CLIENT_HELLO

    },

    {

        TLSEXT_TYPE_status_request,

        NULL, NULL,

        EXT_CLIENT_HELLO | EXT_TLS1_2_SERVER_HELLO | EXT_TLS1_3_CERTIFICATE

    },

    {

        TLSEXT_TYPE_next_proto_neg,

        NULL, NULL,

        EXT_CLIENT_HELLO | EXT_TLS1_2_SERVER_HELLO

    },

    {

        TLSEXT_TYPE_application_layer_protocol_negotiation,

        NULL, NULL,

        EXT_CLIENT_HELLO | EXT_TLS1_2_SERVER_HELLO

        | EXT_TLS1_3_ENCRYPTED_EXTENSIONS

    },

    {

        TLSEXT_TYPE_use_srtp,

        NULL, NULL,

        EXT_CLIENT_HELLO | EXT_TLS1_2_SERVER_HELLO

        | EXT_TLS1_3_ENCRYPTED_EXTENSIONS | EXT_DTLS_ONLY

    },

    {

        TLSEXT_TYPE_encrypt_then_mac,

        NULL, NULL,

        EXT_CLIENT_HELLO | EXT_TLS1_2_SERVER_HELLO

    },

    {

        TLSEXT_TYPE_signed_certificate_timestamp,

        NULL, NULL,

        EXT_CLIENT_HELLO | EXT_TLS1_2_SERVER_HELLO | EXT_TLS1_3_CERTIFICATE

    },

    {

        TLSEXT_TYPE_extended_master_secret,

        NULL, NULL,

        EXT_CLIENT_HELLO | EXT_TLS1_2_SERVER_HELLO

    },

    {

        TLSEXT_TYPE_supported_versions,

        NULL, NULL,

        EXT_CLIENT_HELLO

    },

    {

        TLSEXT_TYPE_padding,

        NULL, NULL,

        EXT_CLIENT_HELLO

    },

    {

        TLSEXT_TYPE_key_share,

        NULL, NULL,

        EXT_CLIENT_HELLO | EXT_TLS1_3_SERVER_HELLO

        | EXT_TLS1_3_HELLO_RETRY_REQUEST

    }

};



/*

 * Comparison function used in a call to qsort (see tls_collect_extensions()

 * below.)
@@ -35,8 +136,37 @@ static int compare_extensions(const void *p1, const void *p2) 
}



/*

 * Gather a list of all the extensions. We don't actually process the content

 * of the extensions yet, except to check their types.

 * Verify whether we are allowed to use the extension |type| in the current

 * |context|. Returns 1 to indicate the extension is allowed or unknown or 0 to

 * indicate the extension is not allowed.

 */

static int verify_extension(SSL *s, unsigned int context, unsigned int type)

{

    size_t i;



    for (i = 0; i < OSSL_NELEM(ext_defs); i++) {

        if (type == ext_defs[i].type) {

            /* Check we're allowed to use this extension in this context */

            if ((context & ext_defs[i].context) == 0)

                return 0;

            /* Make sure we don't use DTLS extensions in TLS */

            if ((ext_defs[i].context & EXT_DTLS_ONLY) && !SSL_IS_DTLS(s))

                return 0;



            return 1;

        }

    }



    /* Unknown extension. We allow it */

    return 1;

}



/*

 * Gather a list of all the extensions from the data in |packet]. |context|

 * tells us which message this extension is for. The raw extension data is

 * stored in |*res| with the number of found extensions in |*numfound|. In the

 * event of an error the alert type to use is stored in |*ad|. We don't actually

 * process the content of the extensions yet, except to check their types.

 *

 * Per http://tools.ietf.org/html/rfc5246#section-7.4.1.4, there may not be

 * more than one extension of the same type in a ClientHello or ServerHello.
@@ -48,8 +178,8 @@ static int compare_extensions(const void *p1, const void *p2) 
 * TODO(TLS1.3): Refactor ServerHello extension parsing to use this and then

 * remove tls1_check_duplicate_extensions()

 */

int tls_collect_extensions(PACKET *packet, RAW_EXTENSION **res,

                             size_t *numfound, int *ad)

int tls_collect_extensions(SSL *s, PACKET *packet, unsigned int context,

                           RAW_EXTENSION **res, size_t *numfound, int *ad)

{

    PACKET extensions = *packet;

    size_t num_extensions = 0, i = 0;
@@ -62,9 +192,16 @@ int tls_collect_extensions(PACKET *packet, RAW_EXTENSION **res, 


        if (!PACKET_get_net_2(&extensions, &type) ||

            !PACKET_get_length_prefixed_2(&extensions, &extension)) {

            SSLerr(SSL_F_TLS_COLLECT_EXTENSIONS, SSL_R_BAD_EXTENSION);

            *ad = SSL_AD_DECODE_ERROR;

            goto err;

        }

        /* Verify this extension is allowed */

        if (!verify_extension(s, context, type)) {

            SSLerr(SSL_F_TLS_COLLECT_EXTENSIONS, SSL_R_BAD_EXTENSION);

            *ad = SSL_AD_ILLEGAL_PARAMETER;

            goto err;

        }

        num_extensions++;

    }

ssl/statem/statem_locl.h

+13 −2
Original line number Diff line number Diff line
@@ -26,6 +26,17 @@ 
/* Max should actually be 36 but we are generous */

#define FINISHED_MAX_LENGTH             64



/* Extension context codes */

#define EXT_DTLS_ONLY                       0x01

#define EXT_CLIENT_HELLO                    0x02

/* Really means TLS1.2 or below */

#define EXT_TLS1_2_SERVER_HELLO             0x04

#define EXT_TLS1_3_SERVER_HELLO             0x08

#define EXT_TLS1_3_ENCRYPTED_EXTENSIONS     0x10

#define EXT_TLS1_3_HELLO_RETRY_REQUEST      0x20

#define EXT_TLS1_3_CERTIFICATE              0x40

#define EXT_TLS1_3_NEW_SESSION_TICKET       0x80



/* Message processing return codes */

typedef enum {

    /* Something bad happened */
@@ -88,8 +99,8 @@ __owur int tls_construct_finished(SSL *s, WPACKET *pkt); 
__owur WORK_STATE tls_finish_handshake(SSL *s, WORK_STATE wst);

__owur WORK_STATE dtls_wait_for_dry(SSL *s);



int tls_collect_extensions(PACKET *packet, RAW_EXTENSION **res,

                             size_t *numfound, int *ad);

int tls_collect_extensions(SSL *s, PACKET *packet, unsigned int context,

                           RAW_EXTENSION **res, size_t *numfound, int *ad);



/* some client-only functions */

__owur int tls_construct_client_hello(SSL *s, WPACKET *pkt);

ssl/statem/statem_srvr.c

+3 −2
Original line number Diff line number Diff line
@@ -1249,7 +1249,8 @@ MSG_PROCESS_RETURN tls_process_client_hello(SSL *s, PACKET *pkt) 


    /* Preserve the raw extensions PACKET for later use */

    extensions = clienthello.extensions;

    if (!tls_collect_extensions(&extensions, &clienthello.pre_proc_exts,

    if (!tls_collect_extensions(s, &extensions, EXT_CLIENT_HELLO,

                                &clienthello.pre_proc_exts,

                                &clienthello.num_extensions, &al)) {

        /* SSLerr already been called */

        goto f_err;