generic-library/openssl
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Ensure SSL3_FLAGS_CCS_OK (or d1->change_cipher_spec_ok for DTLS) is reset

Browse Source 
once the ChangeCipherSpec message is received. Previously, the server would
set the flag once at SSL3_ST_SR_CERT_VRFY and again at SSL3_ST_SR_FINISHED.
This would allow a second CCS to arrive and would corrupt the server state.

(Because the first CCS would latch the correct keys and subsequent CCS
messages would have to be encrypted, a MitM attacker cannot exploit this,
though.)

Thanks to Joeri de Ruiter for reporting this issue.

Reviewed-by: Matt Caswell <matt@openssl.org>
(cherry picked from commit e94a6c0ede623960728415b68650a595e48f5a43)
This commit is contained in:
Emilia Kasper 2014-11-19 17:01:36 +01:00
parent 9baee0216f
commit e5f261df73
8 changed files with 90 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
CHANGES
View File

@ -43,6 +43,11 @@
(CVE-2014-3566) (CVE-2014-3566)
[Adam Langley, Bodo Moeller] [Adam Langley, Bodo Moeller]
*) Tighten handling of the ChangeCipherSpec (CCS) message: reject
early CCS messages during renegotiation. (Note that because
renegotiation is encrypted, this early CCS was not exploitable.)
[Emilia Käsper]
*) Tighten client-side session ticket handling during renegotiation: *) Tighten client-side session ticket handling during renegotiation:
ensure that the client only accepts a session ticket if the server sends ensure that the client only accepts a session ticket if the server sends
the extension anew in the ServerHello. Previously, a TLS client would the extension anew in the ServerHello. Previously, a TLS client would
@ -376,6 +381,11 @@
Changes between 1.0.1j and 1.0.1k [xx XXX xxxx] Changes between 1.0.1j and 1.0.1k [xx XXX xxxx]
*) Tighten handling of the ChangeCipherSpec (CCS) message: reject
early CCS messages during renegotiation. (Note that because
renegotiation is encrypted, this early CCS was not exploitable.)
[Emilia Käsper]
*) Tighten client-side session ticket handling during renegotiation: *) Tighten client-side session ticket handling during renegotiation:
ensure that the client only accepts a session ticket if the server sends ensure that the client only accepts a session ticket if the server sends
the extension anew in the ServerHello. Previously, a TLS client would the extension anew in the ServerHello. Previously, a TLS client would

5
ssl/d1_clnt.c
View File

@ -267,6 +267,9 @@ int dtls1_connect(SSL *s)
memset(s->s3->client_random,0,sizeof(s->s3->client_random)); memset(s->s3->client_random,0,sizeof(s->s3->client_random));
s->d1->send_cookie = 0; s->d1->send_cookie = 0;
s->hit = 0; s->hit = 0;
s->d1->change_cipher_spec_ok = 0;
/* Should have been reset by ssl3_get_finished, too. */
s->s3->change_cipher_spec = 0;
break; break;
#ifndef OPENSSL_NO_SCTP #ifndef OPENSSL_NO_SCTP
@ -510,7 +513,6 @@ int dtls1_connect(SSL *s)
else else
#endif #endif
s->state=SSL3_ST_CW_CHANGE_A; s->state=SSL3_ST_CW_CHANGE_A;
s->s3->change_cipher_spec=0;
} }
s->init_num=0; s->init_num=0;
@ -531,7 +533,6 @@ int dtls1_connect(SSL *s)
#endif #endif
s->state=SSL3_ST_CW_CHANGE_A; s->state=SSL3_ST_CW_CHANGE_A;
s->init_num=0; s->init_num=0;
s->s3->change_cipher_spec=0;
break; break;
case SSL3_ST_CW_CHANGE_A: case SSL3_ST_CW_CHANGE_A:

26
ssl/d1_srvr.c
View File

@ -264,6 +264,9 @@ int dtls1_accept(SSL *s)
} }
s->init_num=0; s->init_num=0;
s->d1->change_cipher_spec_ok = 0;
/* Should have been reset by ssl3_get_finished, too. */
s->s3->change_cipher_spec = 0;
if (s->state != SSL_ST_RENEGOTIATE) if (s->state != SSL_ST_RENEGOTIATE)
{ {
@ -694,8 +697,14 @@ int dtls1_accept(SSL *s)
case SSL3_ST_SR_CERT_VRFY_A: case SSL3_ST_SR_CERT_VRFY_A:
case SSL3_ST_SR_CERT_VRFY_B: case SSL3_ST_SR_CERT_VRFY_B:
/*
s->d1->change_cipher_spec_ok = 1; * This *should* be the first time we enable CCS, but be
* extra careful about surrounding code changes. We need
* to set this here because we don't know if we're
* expecting a CertificateVerify or not.
*/
if (!s->s3->change_cipher_spec)
s->d1->change_cipher_spec_ok = 1;
/* we should decide if we expected this one */ /* we should decide if we expected this one */
ret=ssl3_get_cert_verify(s); ret=ssl3_get_cert_verify(s);
if (ret <= 0) goto end; if (ret <= 0) goto end;
@ -711,7 +720,18 @@ int dtls1_accept(SSL *s)
case SSL3_ST_SR_FINISHED_A: case SSL3_ST_SR_FINISHED_A:
case SSL3_ST_SR_FINISHED_B: case SSL3_ST_SR_FINISHED_B:
s->d1->change_cipher_spec_ok = 1; /*
* Enable CCS for resumed handshakes.
* In a full handshake, we end up here through
* SSL3_ST_SR_CERT_VRFY_B, so change_cipher_spec_ok was
* already set. Receiving a CCS clears the flag, so make
* sure not to re-enable it to ban duplicates.
* s->s3->change_cipher_spec is set when a CCS is
* processed in d1_pkt.c, and remains set until
* the client's Finished message is read.
*/
if (!s->s3->change_cipher_spec)
s->d1->change_cipher_spec_ok = 1;
ret=ssl3_get_finished(s,SSL3_ST_SR_FINISHED_A, ret=ssl3_get_finished(s,SSL3_ST_SR_FINISHED_A,
SSL3_ST_SR_FINISHED_B); SSL3_ST_SR_FINISHED_B);
if (ret <= 0) goto end; if (ret <= 0) goto end;

4
ssl/dtls1.h
View File

@ -256,6 +256,10 @@ typedef struct dtls1_state_st
unsigned int handshake_fragment_len; unsigned int handshake_fragment_len;
unsigned int retransmitting; unsigned int retransmitting;
/*
* Set when the handshake is ready to process peer's ChangeCipherSpec message.
* Cleared after the message has been processed.
*/
unsigned int change_cipher_spec_ok; unsigned int change_cipher_spec_ok;
#ifndef OPENSSL_NO_SCTP #ifndef OPENSSL_NO_SCTP

10
ssl/s3_clnt.c
View File

@ -273,6 +273,9 @@ int ssl3_connect(SSL *s)
s->state=SSL3_ST_CW_CLNT_HELLO_A; s->state=SSL3_ST_CW_CLNT_HELLO_A;
s->ctx->stats.sess_connect++; s->ctx->stats.sess_connect++;
s->init_num=0; s->init_num=0;
s->s3->flags &= ~SSL3_FLAGS_CCS_OK;
/* Should have been reset by ssl3_get_finished, too. */
s->s3->change_cipher_spec = 0;
break; break;
case SSL3_ST_CW_CLNT_HELLO_A: case SSL3_ST_CW_CLNT_HELLO_A:
@ -421,12 +424,10 @@ int ssl3_connect(SSL *s)
else else
{ {
s->state=SSL3_ST_CW_CHANGE_A; s->state=SSL3_ST_CW_CHANGE_A;
s->s3->change_cipher_spec=0;
} }
if (s->s3->flags & TLS1_FLAGS_SKIP_CERT_VERIFY) if (s->s3->flags & TLS1_FLAGS_SKIP_CERT_VERIFY)
{ {
s->state=SSL3_ST_CW_CHANGE_A; s->state=SSL3_ST_CW_CHANGE_A;
s->s3->change_cipher_spec=0;
} }
s->init_num=0; s->init_num=0;
@ -438,7 +439,6 @@ int ssl3_connect(SSL *s)
if (ret <= 0) goto end; if (ret <= 0) goto end;
s->state=SSL3_ST_CW_CHANGE_A; s->state=SSL3_ST_CW_CHANGE_A;
s->init_num=0; s->init_num=0;
s->s3->change_cipher_spec=0;
break; break;
case SSL3_ST_CW_CHANGE_A: case SSL3_ST_CW_CHANGE_A:
@ -498,7 +498,6 @@ int ssl3_connect(SSL *s)
s->method->ssl3_enc->client_finished_label, s->method->ssl3_enc->client_finished_label,
s->method->ssl3_enc->client_finished_label_len); s->method->ssl3_enc->client_finished_label_len);
if (ret <= 0) goto end; if (ret <= 0) goto end;
s->s3->flags |= SSL3_FLAGS_CCS_OK;
s->state=SSL3_ST_CW_FLUSH; s->state=SSL3_ST_CW_FLUSH;
/* clear flags */ /* clear flags */
@ -547,7 +546,6 @@ int ssl3_connect(SSL *s)
case SSL3_ST_CR_FINISHED_A: case SSL3_ST_CR_FINISHED_A:
case SSL3_ST_CR_FINISHED_B: case SSL3_ST_CR_FINISHED_B:
s->s3->flags |= SSL3_FLAGS_CCS_OK; s->s3->flags |= SSL3_FLAGS_CCS_OK;
ret=ssl3_get_finished(s,SSL3_ST_CR_FINISHED_A, ret=ssl3_get_finished(s,SSL3_ST_CR_FINISHED_A,
SSL3_ST_CR_FINISHED_B); SSL3_ST_CR_FINISHED_B);
@ -986,7 +984,6 @@ int ssl3_get_server_hello(SSL *s)
s->session->cipher = pref_cipher ? s->session->cipher = pref_cipher ?
pref_cipher : ssl_get_cipher_by_char(s, p+j); pref_cipher : ssl_get_cipher_by_char(s, p+j);
s->hit = 1; s->hit = 1;
s->s3->flags |= SSL3_FLAGS_CCS_OK;
} }
} }
#endif /* OPENSSL_NO_TLSEXT */ #endif /* OPENSSL_NO_TLSEXT */
@ -1002,7 +999,6 @@ int ssl3_get_server_hello(SSL *s)
SSLerr(SSL_F_SSL3_GET_SERVER_HELLO,SSL_R_ATTEMPT_TO_REUSE_SESSION_IN_DIFFERENT_CONTEXT); SSLerr(SSL_F_SSL3_GET_SERVER_HELLO,SSL_R_ATTEMPT_TO_REUSE_SESSION_IN_DIFFERENT_CONTEXT);
goto f_err; goto f_err;
} }
s->s3->flags |= SSL3_FLAGS_CCS_OK;
s->hit=1; s->hit=1;
} }
/* a miss or crap from the other end */ /* a miss or crap from the other end */

40
ssl/s3_srvr.c
View File

@ -301,6 +301,9 @@ int ssl3_accept(SSL *s)
s->init_num=0; s->init_num=0;
s->s3->flags &= ~SSL3_FLAGS_SGC_RESTART_DONE; s->s3->flags &= ~SSL3_FLAGS_SGC_RESTART_DONE;
s->s3->flags &= ~TLS1_FLAGS_SKIP_CERT_VERIFY; s->s3->flags &= ~TLS1_FLAGS_SKIP_CERT_VERIFY;
s->s3->flags &= ~SSL3_FLAGS_CCS_OK;
/* Should have been reset by ssl3_get_finished, too. */
s->s3->change_cipher_spec = 0;
if (s->state != SSL_ST_RENEGOTIATE) if (s->state != SSL_ST_RENEGOTIATE)
{ {
@ -676,8 +679,14 @@ int ssl3_accept(SSL *s)
case SSL3_ST_SR_CERT_VRFY_A: case SSL3_ST_SR_CERT_VRFY_A:
case SSL3_ST_SR_CERT_VRFY_B: case SSL3_ST_SR_CERT_VRFY_B:
/*
s->s3->flags |= SSL3_FLAGS_CCS_OK; * This *should* be the first time we enable CCS, but be
* extra careful about surrounding code changes. We need
* to set this here because we don't know if we're
* expecting a CertificateVerify or not.
*/
if (!s->s3->change_cipher_spec)
s->s3->flags |= SSL3_FLAGS_CCS_OK;
/* we should decide if we expected this one */ /* we should decide if we expected this one */
ret=ssl3_get_cert_verify(s); ret=ssl3_get_cert_verify(s);
if (ret <= 0) goto end; if (ret <= 0) goto end;
@ -696,6 +705,19 @@ int ssl3_accept(SSL *s)
#if !defined(OPENSSL_NO_TLSEXT) && !defined(OPENSSL_NO_NEXTPROTONEG) #if !defined(OPENSSL_NO_TLSEXT) && !defined(OPENSSL_NO_NEXTPROTONEG)
case SSL3_ST_SR_NEXT_PROTO_A: case SSL3_ST_SR_NEXT_PROTO_A:
case SSL3_ST_SR_NEXT_PROTO_B: case SSL3_ST_SR_NEXT_PROTO_B:
/*
* Enable CCS for resumed handshakes with NPN.
* In a full handshake with NPN, we end up here through
* SSL3_ST_SR_CERT_VRFY_B, where SSL3_FLAGS_CCS_OK was
* already set. Receiving a CCS clears the flag, so make
* sure not to re-enable it to ban duplicates.
* s->s3->change_cipher_spec is set when a CCS is
* processed in s3_pkt.c, and remains set until
* the client's Finished message is read.
*/
if (!s->s3->change_cipher_spec)
s->s3->flags |= SSL3_FLAGS_CCS_OK;
ret=ssl3_get_next_proto(s); ret=ssl3_get_next_proto(s);
if (ret <= 0) goto end; if (ret <= 0) goto end;
s->init_num = 0; s->init_num = 0;
@ -705,7 +727,18 @@ int ssl3_accept(SSL *s)
case SSL3_ST_SR_FINISHED_A: case SSL3_ST_SR_FINISHED_A:
case SSL3_ST_SR_FINISHED_B: case SSL3_ST_SR_FINISHED_B:
s->s3->flags |= SSL3_FLAGS_CCS_OK; /*
* Enable CCS for resumed handshakes without NPN.
* In a full handshake, we end up here through
* SSL3_ST_SR_CERT_VRFY_B, where SSL3_FLAGS_CCS_OK was
* already set. Receiving a CCS clears the flag, so make
* sure not to re-enable it to ban duplicates.
* s->s3->change_cipher_spec is set when a CCS is
* processed in s3_pkt.c, and remains set until
* the client's Finished message is read.
*/
if (!s->s3->change_cipher_spec)
s->s3->flags |= SSL3_FLAGS_CCS_OK;
ret=ssl3_get_finished(s,SSL3_ST_SR_FINISHED_A, ret=ssl3_get_finished(s,SSL3_ST_SR_FINISHED_A,
SSL3_ST_SR_FINISHED_B); SSL3_ST_SR_FINISHED_B);
if (ret <= 0) goto end; if (ret <= 0) goto end;
@ -777,7 +810,6 @@ int ssl3_accept(SSL *s)
#else #else
if (s->s3->next_proto_neg_seen) if (s->s3->next_proto_neg_seen)
{ {
s->s3->flags |= SSL3_FLAGS_CCS_OK;
s->s3->tmp.next_state=SSL3_ST_SR_NEXT_PROTO_A; s->s3->tmp.next_state=SSL3_ST_SR_NEXT_PROTO_A;
} }
else else

13
ssl/ssl3.h
View File

@ -429,8 +429,12 @@ typedef struct ssl3_buffer_st
#define TLS1_FLAGS_TLS_PADDING_BUG 0x0008 #define TLS1_FLAGS_TLS_PADDING_BUG 0x0008
#define TLS1_FLAGS_SKIP_CERT_VERIFY 0x0010 #define TLS1_FLAGS_SKIP_CERT_VERIFY 0x0010
#define TLS1_FLAGS_KEEP_HANDSHAKE 0x0020 #define TLS1_FLAGS_KEEP_HANDSHAKE 0x0020
/*
* Set when the handshake is ready to process peer's ChangeCipherSpec message.
* Cleared after the message has been processed.
*/
#define SSL3_FLAGS_CCS_OK 0x0080 #define SSL3_FLAGS_CCS_OK 0x0080
/* SSL3_FLAGS_SGC_RESTART_DONE is set when we /* SSL3_FLAGS_SGC_RESTART_DONE is set when we
* restart a handshake because of MS SGC and so prevents us * restart a handshake because of MS SGC and so prevents us
* from restarting the handshake in a loop. It's reset on a * from restarting the handshake in a loop. It's reset on a
@ -492,8 +496,11 @@ typedef struct ssl3_state_st
* and freed and MD_CTX-es for all required digests are stored in * and freed and MD_CTX-es for all required digests are stored in
* this array */ * this array */
EVP_MD_CTX **handshake_dgst; EVP_MD_CTX **handshake_dgst;
/* this is set whenerver we see a change_cipher_spec message /*
* come in when we are not looking for one */ * Set whenever an expected ChangeCipherSpec message is processed.
* Unset when the peer's Finished message is received.
* Unexpected ChangeCipherSpec messages trigger a fatal alert.
*/
int change_cipher_spec; int change_cipher_spec;
int warn_alert; int warn_alert;

2
ssl/t1_lib.c
View File

@ -2560,7 +2560,7 @@ static int ssl_scan_serverhello_tlsext(SSL *s, unsigned char **p, unsigned char
#ifndef OPENSSL_NO_NEXTPROTONEG #ifndef OPENSSL_NO_NEXTPROTONEG
s->s3->next_proto_neg_seen = 0; s->s3->next_proto_neg_seen = 0;
#endif #endif
s->tlsext_ticket_expected = 0; s->tlsext_ticket_expected = 0;
if (s->s3->alpn_selected) if (s->s3->alpn_selected)
{ {