Commit 90134d98 authored by Benjamin Kaduk's avatar Benjamin Kaduk Committed by Richard Levitte
Browse files

Refactor SSL_bytes_to_cipher_list() 



Split off the portions that mutate the SSL object into a separate
function that the state machine calls, so that the public API can
be a pure function.  (It still needs the SSL parameter in order
to determine what SSL_METHOD's get_cipher_by_char() routine to use,
though.)

Instead of returning the stack of ciphers (functionality that was
not used internally), require using the output parameter, and add
a separate output parameter for the SCSVs contained in the supplied
octets, if desired.  This lets us move to the standard return value
convention.  Also make both output stacks optional parameters.

Reviewed-by: default avatarMatt Caswell <matt@openssl.org>
Reviewed-by: default avatarRichard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2279)
parent ccb8e6e0

doc/man3/SSL_get_ciphers.pod

+7 −12
Original line number Original line Diff line number Diff line
@@ -15,9 +15,9 @@ SSL_bytes_to_cipher_list, SSL_get_cipher_list 
 STACK_OF(SSL_CIPHER) *SSL_CTX_get_ciphers(const SSL_CTX *ctx);

 STACK_OF(SSL_CIPHER) *SSL_CTX_get_ciphers(const SSL_CTX *ctx);

 STACK_OF(SSL_CIPHER) *SSL_get1_supported_ciphers(SSL *s);

 STACK_OF(SSL_CIPHER) *SSL_get1_supported_ciphers(SSL *s);

 STACK_OF(SSL_CIPHER) *SSL_get_client_ciphers(const SSL *ssl);

 STACK_OF(SSL_CIPHER) *SSL_get_client_ciphers(const SSL *ssl);

 STACK_OF(SSL_CIPHER) *SSL_bytes_to_cipher_list(SSL *s,

 int SSL_bytes_to_cipher_list(SSL *s, const unsigned char *bytes, size_t len,

                                                const unsigned char *bytes,

                              int isv2format, STACK_OF(SSL_CIPHER) **sk,

                                                size_t len, int isv2format)

                              STACK_OF(SSL_CIPHER) **scsvs);

 const char *SSL_get_cipher_list(const SSL *ssl, int priority);

 const char *SSL_get_cipher_list(const SSL *ssl, int priority);





=head1 DESCRIPTION

=head1 DESCRIPTION
@@ -49,8 +49,9 @@ SSL_bytes_to_cipher_list() treats the supplied B<len> octets in B<bytes> 
as a wire-protocol cipher suite specification (in the three-octet-per-cipher

as a wire-protocol cipher suite specification (in the three-octet-per-cipher

SSLv2 wire format if B<isv2format> is nonzero; otherwise the two-octet

SSLv2 wire format if B<isv2format> is nonzero; otherwise the two-octet

SSLv3/TLS wire format), and parses the cipher suites supported by the library

SSLv3/TLS wire format), and parses the cipher suites supported by the library

into the returned stack of SSL_CIPHER objects.  Unsupported cipher suites

into the returned stacks of SSL_CIPHER objects sk and Signalling Cipher-Suite

are ignored, and NULL is returned on error.

Values scsvs.  Unsupported cipher suites are ignored.  Returns 1 on success

and 0 on failure.





SSL_get_cipher_list() returns a pointer to the name of the SSL_CIPHER

SSL_get_cipher_list() returns a pointer to the name of the SSL_CIPHER

listed for B<ssl> with B<priority>. If B<ssl> is NULL, no ciphers are

listed for B<ssl> with B<priority>. If B<ssl> is NULL, no ciphers are
@@ -74,19 +75,13 @@ free the return value itself. 
The stack returned by SSL_get1_supported_ciphers() should be freed using

The stack returned by SSL_get1_supported_ciphers() should be freed using

sk_SSL_CIPHER_free().

sk_SSL_CIPHER_free().





The stack returned by SSL_bytes_to_cipher_list() should be freed using

The stacks returned by SSL_bytes_to_cipher_list() should be freed using

sk_SSL_CIPHER_free().

sk_SSL_CIPHER_free().





=head1 RETURN VALUES

=head1 RETURN VALUES





See DESCRIPTION

See DESCRIPTION





=head1 BUGS



The implementation of SSL_bytes_to_cipher_list() mutates state in the

supplied SSL object B<s>; SSL_bytes_to_cipher_list() should not be called

on a server SSL object after that server has processed the received ClientHello.



=head1 SEE ALSO

=head1 SEE ALSO





L<ssl(7)>, L<SSL_CTX_set_cipher_list(3)>,

L<ssl(7)>, L<SSL_CTX_set_cipher_list(3)>,

include/openssl/ssl.h

+4 −3
Original line number Original line Diff line number Diff line
@@ -1820,9 +1820,9 @@ __owur int SSL_COMP_add_compression_method(int id, COMP_METHOD *cm); 
const SSL_CIPHER *SSL_CIPHER_find(SSL *ssl, const unsigned char *ptr);

const SSL_CIPHER *SSL_CIPHER_find(SSL *ssl, const unsigned char *ptr);

int SSL_CIPHER_get_cipher_nid(const SSL_CIPHER *c);

int SSL_CIPHER_get_cipher_nid(const SSL_CIPHER *c);

int SSL_CIPHER_get_digest_nid(const SSL_CIPHER *c);

int SSL_CIPHER_get_digest_nid(const SSL_CIPHER *c);

STACK_OF(SSL_CIPHER) *SSL_bytes_to_cipher_list(SSL *s,

int SSL_bytes_to_cipher_list(SSL *s, const unsigned char *bytes, size_t len,

                                               const unsigned char *bytes,

                             int isv2format, STACK_OF(SSL_CIPHER) **sk,

                                               size_t len, int isv2format);

                             STACK_OF(SSL_CIPHER) **scsvs);





/* TLS extensions functions */

/* TLS extensions functions */

__owur int SSL_set_session_ticket_ext(SSL *s, void *ext_data, int ext_len);

__owur int SSL_set_session_ticket_ext(SSL *s, void *ext_data, int ext_len);
@@ -2164,6 +2164,7 @@ int ERR_load_SSL_strings(void); 
# define SSL_F_SSL_BAD_METHOD                             160

# define SSL_F_SSL_BAD_METHOD                             160

# define SSL_F_SSL_BUILD_CERT_CHAIN                       332

# define SSL_F_SSL_BUILD_CERT_CHAIN                       332

# define SSL_F_SSL_BYTES_TO_CIPHER_LIST                   161

# define SSL_F_SSL_BYTES_TO_CIPHER_LIST                   161

# define SSL_F_SSL_CACHE_CIPHERLIST                       520

# define SSL_F_SSL_CERT_ADD0_CHAIN_CERT                   346

# define SSL_F_SSL_CERT_ADD0_CHAIN_CERT                   346

# define SSL_F_SSL_CERT_DUP                               221

# define SSL_F_SSL_CERT_DUP                               221

# define SSL_F_SSL_CERT_NEW                               162

# define SSL_F_SSL_CERT_NEW                               162

ssl/ssl_err.c

+1 −0
Original line number Original line Diff line number Diff line
@@ -118,6 +118,7 @@ static ERR_STRING_DATA SSL_str_functs[] = { 
    {ERR_FUNC(SSL_F_SSL_BAD_METHOD), "ssl_bad_method"},

    {ERR_FUNC(SSL_F_SSL_BAD_METHOD), "ssl_bad_method"},

    {ERR_FUNC(SSL_F_SSL_BUILD_CERT_CHAIN), "ssl_build_cert_chain"},

    {ERR_FUNC(SSL_F_SSL_BUILD_CERT_CHAIN), "ssl_build_cert_chain"},

    {ERR_FUNC(SSL_F_SSL_BYTES_TO_CIPHER_LIST), "SSL_bytes_to_cipher_list"},

    {ERR_FUNC(SSL_F_SSL_BYTES_TO_CIPHER_LIST), "SSL_bytes_to_cipher_list"},

    {ERR_FUNC(SSL_F_SSL_CACHE_CIPHERLIST), "ssl_cache_cipherlist"},

    {ERR_FUNC(SSL_F_SSL_CERT_ADD0_CHAIN_CERT), "ssl_cert_add0_chain_cert"},

    {ERR_FUNC(SSL_F_SSL_CERT_ADD0_CHAIN_CERT), "ssl_cert_add0_chain_cert"},

    {ERR_FUNC(SSL_F_SSL_CERT_DUP), "ssl_cert_dup"},

    {ERR_FUNC(SSL_F_SSL_CERT_DUP), "ssl_cert_dup"},

    {ERR_FUNC(SSL_F_SSL_CERT_NEW), "ssl_cert_new"},

    {ERR_FUNC(SSL_F_SSL_CERT_NEW), "ssl_cert_new"},

ssl/ssl_lib.c

+70 −69
Original line number Original line Diff line number Diff line
@@ -4402,54 +4402,26 @@ int ssl_log_secret(SSL *ssl, 
                          secret_len);

                          secret_len);

}

}







STACK_OF(SSL_CIPHER) *SSL_bytes_to_cipher_list(SSL *s,

                                               const unsigned char *bytes,

                                               size_t len, int isv2format)

{

    int alert;

    PACKET pkt;



    if (!PACKET_buf_init(&pkt, bytes, len))

        return 0;

    return bytes_to_cipher_list(s, &pkt, NULL, isv2format, &alert);

}



#define SSLV2_CIPHER_LEN    3

#define SSLV2_CIPHER_LEN    3





STACK_OF(SSL_CIPHER) *bytes_to_cipher_list(SSL *s,

int ssl_cache_cipherlist(SSL *s, PACKET *cipher_suites, int sslv2format,

                                           PACKET *cipher_suites,

                         int *al)

                                           STACK_OF(SSL_CIPHER) **skp,

                                           int sslv2format, int *al)

{

{

    const SSL_CIPHER *c;

    STACK_OF(SSL_CIPHER) *sk;

    int n;

    int n;

    /* 3 = SSLV2_CIPHER_LEN > TLS_CIPHER_LEN = 2. */

    unsigned char cipher[SSLV2_CIPHER_LEN];



    s->s3->send_connection_binding = 0;





    n = sslv2format ? SSLV2_CIPHER_LEN : TLS_CIPHER_LEN;

    n = sslv2format ? SSLV2_CIPHER_LEN : TLS_CIPHER_LEN;





    if (PACKET_remaining(cipher_suites) == 0) {

    if (PACKET_remaining(cipher_suites) == 0) {

        SSLerr(SSL_F_BYTES_TO_CIPHER_LIST, SSL_R_NO_CIPHERS_SPECIFIED);

        SSLerr(SSL_F_SSL_CACHE_CIPHERLIST, SSL_R_NO_CIPHERS_SPECIFIED);

        *al = SSL_AD_ILLEGAL_PARAMETER;

        *al = SSL_AD_ILLEGAL_PARAMETER;

        return NULL;

        return 0;

    }

    }





    if (PACKET_remaining(cipher_suites) % n != 0) {

    if (PACKET_remaining(cipher_suites) % n != 0) {

        SSLerr(SSL_F_BYTES_TO_CIPHER_LIST,

        SSLerr(SSL_F_SSL_CACHE_CIPHERLIST,

               SSL_R_ERROR_IN_RECEIVED_CIPHER_LIST);

               SSL_R_ERROR_IN_RECEIVED_CIPHER_LIST);

        *al = SSL_AD_DECODE_ERROR;

        *al = SSL_AD_DECODE_ERROR;

        return NULL;

        return 0;

    }



    sk = sk_SSL_CIPHER_new_null();

    if (sk == NULL) {

        SSLerr(SSL_F_BYTES_TO_CIPHER_LIST, ERR_R_MALLOC_FAILURE);

        *al = SSL_AD_INTERNAL_ERROR;

        return NULL;

    }

    }





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

    OPENSSL_free(s->s3->tmp.ciphers_raw);
@@ -4498,51 +4470,72 @@ STACK_OF(SSL_CIPHER) *bytes_to_cipher_list(SSL *s, 
        *al = SSL_AD_INTERNAL_ERROR;

        *al = SSL_AD_INTERNAL_ERROR;

        goto err;

        goto err;

    }

    }

    return 1;

 err:

    return 0;

}





    while (PACKET_copy_bytes(cipher_suites, cipher, n)) {

int SSL_bytes_to_cipher_list(SSL *s, const unsigned char *bytes, size_t len,

        /*

                             int isv2format, STACK_OF(SSL_CIPHER) **sk,

         * SSLv3 ciphers wrapped in an SSLv2-compatible ClientHello have the

                             STACK_OF(SSL_CIPHER) **scsvs)

         * first byte set to zero, while true SSLv2 ciphers have a non-zero

{

         * first byte. We don't support any true SSLv2 ciphers, so skip them.

    int alert;

         */

    PACKET pkt;

        if (sslv2format && cipher[0] != '\0')

            continue;





        /* Check for TLS_EMPTY_RENEGOTIATION_INFO_SCSV */

    if (!PACKET_buf_init(&pkt, bytes, len))

        if ((cipher[n - 2] == ((SSL3_CK_SCSV >> 8) & 0xff)) &&

        return 0;

            (cipher[n - 1] == (SSL3_CK_SCSV & 0xff))) {

    return bytes_to_cipher_list(s, &pkt, sk, scsvs, isv2format, &alert);

            /* SCSV fatal if renegotiating */

            if (s->renegotiate) {

                SSLerr(SSL_F_BYTES_TO_CIPHER_LIST,

                       SSL_R_SCSV_RECEIVED_WHEN_RENEGOTIATING);

                *al = SSL_AD_HANDSHAKE_FAILURE;

                goto err;

}

}

            s->s3->send_connection_binding = 1;



            continue;

int bytes_to_cipher_list(SSL *s, PACKET *cipher_suites,

                         STACK_OF(SSL_CIPHER) **skp,

                         STACK_OF(SSL_CIPHER) **scsvs_out,

                         int sslv2format, int *al)

{

    const SSL_CIPHER *c;

    STACK_OF(SSL_CIPHER) *sk = NULL;

    STACK_OF(SSL_CIPHER) *scsvs = NULL;

    int n;

    /* 3 = SSLV2_CIPHER_LEN > TLS_CIPHER_LEN = 2. */

    unsigned char cipher[SSLV2_CIPHER_LEN];



    n = sslv2format ? SSLV2_CIPHER_LEN : TLS_CIPHER_LEN;



    if (PACKET_remaining(cipher_suites) == 0) {

        SSLerr(SSL_F_BYTES_TO_CIPHER_LIST, SSL_R_NO_CIPHERS_SPECIFIED);

        *al = SSL_AD_ILLEGAL_PARAMETER;

        return 0;

    }

    }





        /* Check for TLS_FALLBACK_SCSV */

    if (PACKET_remaining(cipher_suites) % n != 0) {

        if ((cipher[n - 2] == ((SSL3_CK_FALLBACK_SCSV >> 8) & 0xff)) &&

            (cipher[n - 1] == (SSL3_CK_FALLBACK_SCSV & 0xff))) {

            /*

             * The SCSV indicates that the client previously tried a higher

             * version. Fail if the current version is an unexpected

             * downgrade.

             */

            if (!ssl_check_version_downgrade(s)) {

        SSLerr(SSL_F_BYTES_TO_CIPHER_LIST,

        SSLerr(SSL_F_BYTES_TO_CIPHER_LIST,

                       SSL_R_INAPPROPRIATE_FALLBACK);

               SSL_R_ERROR_IN_RECEIVED_CIPHER_LIST);

                *al = SSL_AD_INAPPROPRIATE_FALLBACK;

        *al = SSL_AD_DECODE_ERROR;

        return 0;

    }



    sk = sk_SSL_CIPHER_new_null();

    scsvs = sk_SSL_CIPHER_new_null();

    if (sk == NULL || scsvs == NULL) {

        SSLerr(SSL_F_BYTES_TO_CIPHER_LIST, ERR_R_MALLOC_FAILURE);

        *al = SSL_AD_INTERNAL_ERROR;

        goto err;

        goto err;

    }

    }



    while (PACKET_copy_bytes(cipher_suites, cipher, n)) {

        /*

         * SSLv3 ciphers wrapped in an SSLv2-compatible ClientHello have the

         * first byte set to zero, while true SSLv2 ciphers have a non-zero

         * first byte. We don't support any true SSLv2 ciphers, so skip them.

         */

        if (sslv2format && cipher[0] != '\0')

            continue;

            continue;

        }





        /* For SSLv2-compat, ignore leading 0-byte. */

        /* For SSLv2-compat, ignore leading 0-byte. */

        c = ssl_get_cipher_by_char(s, sslv2format ? &cipher[1] : cipher, 1);

        c = ssl_get_cipher_by_char(s, sslv2format ? &cipher[1] : cipher, 1);

        if (c != NULL) {

        if (c != NULL) {

            if (!sk_SSL_CIPHER_push(sk, c)) {

            if ((c->valid && !sk_SSL_CIPHER_push(sk, c)) ||

                (!c->valid && !sk_SSL_CIPHER_push(scsvs, c))) {

                SSLerr(SSL_F_BYTES_TO_CIPHER_LIST, ERR_R_MALLOC_FAILURE);

                SSLerr(SSL_F_BYTES_TO_CIPHER_LIST, ERR_R_MALLOC_FAILURE);

                *al = SSL_AD_INTERNAL_ERROR;

                *al = SSL_AD_INTERNAL_ERROR;

                goto err;

                goto err;
@@ -4555,9 +4548,17 @@ STACK_OF(SSL_CIPHER) *bytes_to_cipher_list(SSL *s, 
        goto err;

        goto err;

    }

    }





    if (skp != NULL)

        *skp = sk;

        *skp = sk;

    return sk;

    else

        sk_SSL_CIPHER_free(sk);

    if (scsvs_out != NULL)

        *scsvs_out = scsvs;

    else

        sk_SSL_CIPHER_free(scsvs);

    return 1;

 err:

 err:

    sk_SSL_CIPHER_free(sk);

    sk_SSL_CIPHER_free(sk);

    return NULL;

    sk_SSL_CIPHER_free(scsvs);

    return 0;

}
 
}

ssl/ssl_locl.h

+6 −5
Original line number Original line Diff line number Diff line
@@ -1991,10 +1991,11 @@ __owur STACK_OF(SSL_CIPHER) *ssl_create_cipher_list(const SSL_METHOD *meth, 
                                                    **sorted,

                                                    **sorted,

                                                    const char *rule_str,

                                                    const char *rule_str,

                                                    CERT *c);

                                                    CERT *c);

__owur STACK_OF(SSL_CIPHER) *bytes_to_cipher_list(SSL *s,

__owur int ssl_cache_cipherlist(SSL *s, PACKET *cipher_suites,

                                                  PACKET *cipher_suites,

                                int sslv2format, int *al);

                                                  STACK_OF(SSL_CIPHER)

__owur int bytes_to_cipher_list(SSL *s, PACKET *cipher_suites,

                                                  **skp, int sslv2format,

                                STACK_OF(SSL_CIPHER) **skp,

                                STACK_OF(SSL_CIPHER) **scsvs, int sslv2format,

                                int *al);

                                int *al);

void ssl_update_cache(SSL *s, int mode);

void ssl_update_cache(SSL *s, int mode);

__owur int ssl_cipher_get_evp(const SSL_SESSION *s, const EVP_CIPHER **enc,

__owur int ssl_cipher_get_evp(const SSL_SESSION *s, const EVP_CIPHER **enc,