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

Tolerate a Certificate using a non-supported group on server side 



If a server has been configured to use an ECDSA certificate, we should
allow it regardless of whether the server's own supported groups list
includes the certificate's group.

Fixes #2033

Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
Reviewed-by: default avatarBernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from https://github.com/openssl/openssl/pull/5601)
parent 7814cdf3

ssl/ssl_locl.h

+1 −1
Original line number Diff line number Diff line
@@ -2453,7 +2453,7 @@ SSL_COMP *ssl3_comp_find(STACK_OF(SSL_COMP) *sk, int n); 
#  ifndef OPENSSL_NO_EC



__owur const TLS_GROUP_INFO *tls1_group_id_lookup(uint16_t curve_id);

__owur int tls1_check_group_id(SSL *s, uint16_t group_id);

__owur int tls1_check_group_id(SSL *s, uint16_t group_id, int check_own_curves);

__owur uint16_t tls1_shared_group(SSL *s, int nmatch);

__owur int tls1_set_groups(uint16_t **pext, size_t *pextlen,

                           int *curves, size_t ncurves);

ssl/statem/statem_clnt.c

+2 −1
Original line number Diff line number Diff line
@@ -2192,7 +2192,8 @@ static int tls_process_ske_ecdhe(SSL *s, PACKET *pkt, EVP_PKEY **pkey) 
     * Check curve is named curve type and one of our preferences, if not

     * server has sent an invalid curve.

     */

    if (curve_type != NAMED_CURVE_TYPE || !tls1_check_group_id(s, curve_id)) {

    if (curve_type != NAMED_CURVE_TYPE

            || !tls1_check_group_id(s, curve_id, 1)) {

        SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER, SSL_F_TLS_PROCESS_SKE_ECDHE,

                 SSL_R_WRONG_CURVE);

        return 0;

ssl/t1_lib.c

+15 −9
Original line number Diff line number Diff line
@@ -467,7 +467,7 @@ static int tls1_check_pkey_comp(SSL *s, EVP_PKEY *pkey) 
}



/* Check a group id matches preferences */

int tls1_check_group_id(SSL *s, uint16_t group_id)

int tls1_check_group_id(SSL *s, uint16_t group_id, int check_own_groups)

    {

    const uint16_t *groups;

    size_t groups_len;
@@ -491,10 +491,12 @@ int tls1_check_group_id(SSL *s, uint16_t group_id) 
        }

    }



    if (check_own_groups) {

        /* Check group is one of our preferences */

        tls1_get_supported_groups(s, &groups, &groups_len);

        if (!tls1_in_list(group_id, groups, groups_len))

            return 0;

    }



    if (!tls_curve_allowed(s, group_id, SSL_SECOP_CURVE_CHECK))

        return 0;
@@ -554,7 +556,11 @@ static int tls1_check_cert_param(SSL *s, X509 *x, int check_ee_md) 
    if (!tls1_check_pkey_comp(s, pkey))

        return 0;

    group_id = tls1_get_group_id(pkey);

    if (!tls1_check_group_id(s, group_id))

    /*

     * For a server we allow the certificate to not be in our list of supported

     * groups.

     */

    if (!tls1_check_group_id(s, group_id, !s->server))

        return 0;

    /*

     * Special case for suite B. We *MUST* sign using SHA256+P-256 or
@@ -601,9 +607,9 @@ int tls1_check_ec_tmp_key(SSL *s, unsigned long cid) 
     * curves permitted.

     */

    if (cid == TLS1_CK_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256)

        return tls1_check_group_id(s, TLSEXT_curve_P_256);

        return tls1_check_group_id(s, TLSEXT_curve_P_256, 1);

    if (cid == TLS1_CK_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384)

        return tls1_check_group_id(s, TLSEXT_curve_P_384);

        return tls1_check_group_id(s, TLSEXT_curve_P_384, 1);



    return 0;

}
@@ -979,7 +985,7 @@ int tls12_check_peer_sigalg(SSL *s, uint16_t sig, EVP_PKEY *pkey) 
        }

        if (!SSL_IS_TLS13(s)) {

            /* Check curve matches extensions */

            if (!tls1_check_group_id(s, tls1_get_group_id(pkey))) {

            if (!tls1_check_group_id(s, tls1_get_group_id(pkey), 1)) {

                SSLfatal(s, SSL_AD_ILLEGAL_PARAMETER,

                         SSL_F_TLS12_CHECK_PEER_SIGALG, SSL_R_WRONG_CURVE);

                return 0;

test/ssl-tests/20-cert-select.conf

+462 −397

File changed.

Preview size limit exceeded, changes collapsed.

test/ssl-tests/20-cert-select.conf.in

+44 −0
Original line number Diff line number Diff line
@@ -53,6 +53,50 @@ our @tests = ( 
            "ExpectedResult" => "Success"

        },

    },

    {

        name => "ECDSA CipherString Selection",

        server => {

            "ECDSA.Certificate" => test_pem("server-ecdsa-cert.pem"),

            "ECDSA.PrivateKey" => test_pem("server-ecdsa-key.pem"),

            "MaxProtocol" => "TLSv1.2",

            #Deliberately set supported_groups to one not in the cert. This

            #should be tolerated

            "Groups" => "P-384"

        },

        client => {

            "CipherString" => "aECDSA",

            "MaxProtocol" => "TLSv1.2",

            "Groups" => "P-256:P-384",

            "RequestCAFile" => test_pem("root-cert.pem"),

        },

        test   => {

            "ExpectedServerCertType" =>, "P-256",

            "ExpectedServerSignType" =>, "EC",

            # Note: certificate_authorities not sent for TLS < 1.3

            "ExpectedServerCANames" =>, "empty",

            "ExpectedResult" => "Success"

        },

    },

    {

        name => "ECDSA CipherString Selection",

        server => {

            "ECDSA.Certificate" => test_pem("server-ecdsa-cert.pem"),

            "ECDSA.PrivateKey" => test_pem("server-ecdsa-key.pem"),

            "MaxProtocol" => "TLSv1.2",

            "Groups" => "P-256:P-384"

        },

        client => {

            "CipherString" => "aECDSA",

            "MaxProtocol" => "TLSv1.2",

            #Deliberately set groups to not include the certificate group. This

            #should fail

            "Groups" => "P-384",

            "RequestCAFile" => test_pem("root-cert.pem"),

        },

        test   => {

            "ExpectedResult" => "ServerFail"

        },

    },

    {

        name => "Ed25519 CipherString and Signature Algorithm Selection",

        server => $server,