Commit e94a6c0e authored by Emilia Kasper's avatar Emilia Kasper
Browse files

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


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: default avatarMatt Caswell <matt@openssl.org>
parent de2c7504

CHANGES

+10 −0
Original line number Diff line number Diff line
@@ -305,6 +305,11 @@ 


 Changes between 1.0.1j and 1.0.2 [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:

      ensure that the client only accepts a session ticket if the server sends

      the extension anew in the ServerHello. Previously, a TLS client would
@@ -638,6 +643,11 @@ 


 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:

      ensure that the client only accepts a session ticket if the server sends

      the extension anew in the ServerHello. Previously, a TLS client would

ssl/d1_clnt.c

+3 −2
Original line number Diff line number Diff line
@@ -267,6 +267,9 @@ int dtls1_connect(SSL *s) 
			memset(s->s3->client_random,0,sizeof(s->s3->client_random));

			s->d1->send_cookie = 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;



#ifndef OPENSSL_NO_SCTP
@@ -510,7 +513,6 @@ int dtls1_connect(SSL *s) 
				else

#endif

					s->state=SSL3_ST_CW_CHANGE_A;

				s->s3->change_cipher_spec=0;

				}



			s->init_num=0;
@@ -531,7 +533,6 @@ int dtls1_connect(SSL *s) 
#endif

				s->state=SSL3_ST_CW_CHANGE_A;

			s->init_num=0;

			s->s3->change_cipher_spec=0;

			break;



		case SSL3_ST_CW_CHANGE_A:

ssl/d1_srvr.c

+23 −3
Original line number Diff line number Diff line
@@ -264,6 +264,9 @@ int dtls1_accept(SSL *s) 
				}



			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)

				{
@@ -694,7 +697,13 @@ int dtls1_accept(SSL *s) 


		case SSL3_ST_SR_CERT_VRFY_A:

		case SSL3_ST_SR_CERT_VRFY_B:



			/*

			 * 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 */

			ret=ssl3_get_cert_verify(s);
@@ -711,6 +720,17 @@ int dtls1_accept(SSL *s) 


		case SSL3_ST_SR_FINISHED_A:

		case SSL3_ST_SR_FINISHED_B:

			/*

			 * 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,

				SSL3_ST_SR_FINISHED_B);

ssl/dtls1.h

+4 −0
Original line number Diff line number Diff line
@@ -256,6 +256,10 @@ typedef struct dtls1_state_st 
	unsigned int handshake_fragment_len;



	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;



#ifndef OPENSSL_NO_SCTP

ssl/s3_clnt.c

+3 −7
Original line number Diff line number Diff line
@@ -280,6 +280,9 @@ int ssl3_connect(SSL *s) 
			s->state=SSL3_ST_CW_CLNT_HELLO_A;

			s->ctx->stats.sess_connect++;

			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;



		case SSL3_ST_CW_CLNT_HELLO_A:
@@ -428,12 +431,10 @@ int ssl3_connect(SSL *s) 
			else

				{

				s->state=SSL3_ST_CW_CHANGE_A;

				s->s3->change_cipher_spec=0;

				}

			if (s->s3->flags & TLS1_FLAGS_SKIP_CERT_VERIFY)

				{

				s->state=SSL3_ST_CW_CHANGE_A;

				s->s3->change_cipher_spec=0;

				}



			s->init_num=0;
@@ -445,7 +446,6 @@ int ssl3_connect(SSL *s) 
			if (ret <= 0) goto end;

			s->state=SSL3_ST_CW_CHANGE_A;

			s->init_num=0;

			s->s3->change_cipher_spec=0;

			break;



		case SSL3_ST_CW_CHANGE_A:
@@ -505,7 +505,6 @@ int ssl3_connect(SSL *s) 
				s->method->ssl3_enc->client_finished_label,

				s->method->ssl3_enc->client_finished_label_len);

			if (ret <= 0) goto end;

			s->s3->flags |= SSL3_FLAGS_CCS_OK;

			s->state=SSL3_ST_CW_FLUSH;



			/* clear flags */
@@ -554,7 +553,6 @@ int ssl3_connect(SSL *s) 


		case SSL3_ST_CR_FINISHED_A:

		case SSL3_ST_CR_FINISHED_B:



			s->s3->flags |= SSL3_FLAGS_CCS_OK;

			ret=ssl3_get_finished(s,SSL3_ST_CR_FINISHED_A,

				SSL3_ST_CR_FINISHED_B);
@@ -992,7 +990,6 @@ int ssl3_get_server_hello(SSL *s) 
			s->session->cipher = pref_cipher ?

				pref_cipher : ssl_get_cipher_by_char(s, p+j);

			s->hit = 1;

			s->s3->flags |= SSL3_FLAGS_CCS_OK;

			}

		}

#endif /* OPENSSL_NO_TLSEXT */
@@ -1008,7 +1005,6 @@ int ssl3_get_server_hello(SSL *s) 
		SSLerr(SSL_F_SSL3_GET_SERVER_HELLO,SSL_R_ATTEMPT_TO_REUSE_SESSION_IN_DIFFERENT_CONTEXT);

		goto f_err;

		}

	    s->s3->flags |= SSL3_FLAGS_CCS_OK;

	    s->hit=1;

	    }

	/* a miss or crap from the other end */