Skip to content

Commit

Permalink
Don't fail the connection in SSLv3 if server selects ECDHE
Browse files Browse the repository at this point in the history
ECDHE is not properly defined for SSLv3. Commit fe55c4a prevented ECDHE
from being selected in that protocol. However, historically, servers do
still select ECDHE anyway so that commit causes interoperability problems.
Clients that previously worked when talking to an SSLv3 server could now
fail.

This commit introduces an exception which enables a client to continue in
SSLv3 if the server selected ECDHE.

(cherry picked from commit 8af91fd)

Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from openssl#3734)
  • Loading branch information
mattcaswell authored and pracj3am committed Aug 22, 2017
1 parent 00ad845 commit 33810f6
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 8 deletions.
2 changes: 1 addition & 1 deletion ssl/ssl_lib.c
Original file line number Diff line number Diff line change
Expand Up @@ -2274,7 +2274,7 @@ STACK_OF(SSL_CIPHER) *SSL_get1_supported_ciphers(SSL *s)
ssl_set_client_disabled(s);
for (i = 0; i < sk_SSL_CIPHER_num(ciphers); i++) {
const SSL_CIPHER *c = sk_SSL_CIPHER_value(ciphers, i);
if (!ssl_cipher_disabled(s, c, SSL_SECOP_CIPHER_SUPPORTED)) {
if (!ssl_cipher_disabled(s, c, SSL_SECOP_CIPHER_SUPPORTED, 0)) {
if (!sk)
sk = sk_SSL_CIPHER_new_null();
if (!sk)
Expand Down
2 changes: 1 addition & 1 deletion ssl/ssl_locl.h
Original file line number Diff line number Diff line change
Expand Up @@ -2368,7 +2368,7 @@ __owur int tls1_set_peer_legacy_sigalg(SSL *s, const EVP_PKEY *pkey);
__owur size_t tls12_get_psigalgs(SSL *s, int sent, const uint16_t **psigs);
__owur int tls12_check_peer_sigalg(SSL *s, uint16_t, EVP_PKEY *pkey);
void ssl_set_client_disabled(SSL *s);
__owur int ssl_cipher_disabled(SSL *s, const SSL_CIPHER *c, int op);
__owur int ssl_cipher_disabled(SSL *s, const SSL_CIPHER *c, int op, int echde);

__owur int ssl_handshake_hash(SSL *s, unsigned char *out, size_t outlen,
size_t *hashlen);
Expand Down
4 changes: 2 additions & 2 deletions ssl/statem/statem_clnt.c
Original file line number Diff line number Diff line change
Expand Up @@ -1406,7 +1406,7 @@ MSG_PROCESS_RETURN tls_process_server_hello(SSL *s, PACKET *pkt)
* If it is a disabled cipher we either didn't send it in client hello,
* or it's not allowed for the selected protocol. So we return an error.
*/
if (ssl_cipher_disabled(s, c, SSL_SECOP_CIPHER_CHECK)) {
if (ssl_cipher_disabled(s, c, SSL_SECOP_CIPHER_CHECK, 1)) {
al = SSL_AD_ILLEGAL_PARAMETER;
SSLerr(SSL_F_TLS_PROCESS_SERVER_HELLO, SSL_R_WRONG_CIPHER_RETURNED);
goto f_err;
Expand Down Expand Up @@ -3485,7 +3485,7 @@ int ssl_cipher_list_to_bytes(SSL *s, STACK_OF(SSL_CIPHER) *sk, WPACKET *pkt)

c = sk_SSL_CIPHER_value(sk, i);
/* Skip disabled ciphers */
if (ssl_cipher_disabled(s, c, SSL_SECOP_CIPHER_SUPPORTED))
if (ssl_cipher_disabled(s, c, SSL_SECOP_CIPHER_SUPPORTED, 0))
continue;

if (!s->method->put_cipher_by_char(c, pkt, &len)) {
Expand Down
20 changes: 16 additions & 4 deletions ssl/t1_lib.c
Original file line number Diff line number Diff line change
Expand Up @@ -1034,19 +1034,31 @@ void ssl_set_client_disabled(SSL *s)
* @s: SSL connection that you want to use the cipher on
* @c: cipher to check
* @op: Security check that you want to do
* @ecdhe: If set to 1 then TLSv1 ECDHE ciphers are also allowed in SSLv3
*
* Returns 1 when it's disabled, 0 when enabled.
*/
int ssl_cipher_disabled(SSL *s, const SSL_CIPHER *c, int op)
int ssl_cipher_disabled(SSL *s, const SSL_CIPHER *c, int op, int ecdhe)
{
if (c->algorithm_mkey & s->s3->tmp.mask_k
|| c->algorithm_auth & s->s3->tmp.mask_a)
return 1;
if (s->s3->tmp.max_ver == 0)
return 1;
if (!SSL_IS_DTLS(s) && ((c->min_tls > s->s3->tmp.max_ver)
|| (c->max_tls < s->s3->tmp.min_ver)))
return 1;
if (!SSL_IS_DTLS(s)) {
int min_tls = c->min_tls;

/*
* For historical reasons we will allow ECHDE to be selected by a server
* in SSLv3 if we are a client
*/
if (min_tls == TLS1_VERSION && ecdhe
&& (c->algorithm_mkey & (SSL_kECDHE | SSL_kECDHEPSK)) != 0)
min_tls = SSL3_VERSION;

if ((min_tls > s->s3->tmp.max_ver) || (c->max_tls < s->s3->tmp.min_ver))
return 1;
}
if (SSL_IS_DTLS(s) && (DTLS_VERSION_GT(c->min_dtls, s->s3->tmp.max_ver)
|| DTLS_VERSION_LT(c->max_dtls, s->s3->tmp.min_ver)))
return 1;
Expand Down

0 comments on commit 33810f6

Please sign in to comment.