mirror/openssl
2
0
Fork 0
mirror of https://github.com/openssl/openssl.git synced 2024-11-27 05:21:51 +08:00
Code Issues Packages Projects Releases Wiki Activity

Don't allow a CCS when expecting a CertificateVerify

Browse Source 
Currently we set change_cipher_spec_ok to 1 before calling
ssl3_get_cert_verify(). This is because this message is optional and if it
is not sent then the next thing we would expect to get is the CCS. However,
although it is optional, we do actually know whether we should be receiving
one in advance. If we have received a client cert then we should expect
a CertificateVerify message. By the time we get to this point we will
already have bombed out if we didn't get a Certificate when we should have
done, so it is safe just to check whether |peer| is NULL or not. If it is
we won't get a CertificateVerify, otherwise we will. Therefore we should
change the logic so that we only attempt to get the CertificateVerify if
we are expecting one, and not allow a CCS in this scenario.

Whilst this is good practice for TLS it is even more important for DTLS.
In DTLS messages can be lost. Therefore we may be in a situation where a
CertificateVerify message does not arrive even though one was sent. In that
case the next message the server will receive will be the CCS. This could
also happen if messages get re-ordered in-flight. In DTLS if
|change_cipher_spec_ok| is not set and a CCS is received it is ignored.
However if |change_cipher_spec_ok| *is* set then a CCS arrival will
immediately move the server into the next epoch. Any messages arriving for
the previous epoch will be ignored. This means that, in this scenario, the
handshake can never complete. The client will attempt to retransmit
missing messages, but the server will ignore them because they are the wrong
epoch. The server meanwhile will still be waiting for the CertificateVerify
which is never going to arrive.

RT#2958

Reviewed-by: Emilia Käsper <emilia@openssl.org>
This commit is contained in:
Matt Caswell 2015-05-06 21:31:16 +01:00
parent 56d88027f0
commit a0bd649336
2 changed files with 31 additions and 67 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
ssl/d1_srvr.c
View File

@ -683,15 +683,6 @@ 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);
if (ret <= 0)
goto end;
@ -708,11 +699,10 @@ 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.
* Enable CCS. Receiving a CCS clears the flag, so make
* sure not to re-enable it to ban duplicates. This *should* be the
* first time we have received one - but we check anyway to be
* cautious.
* 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.

80
ssl/s3_srvr.c
View File

@ -682,15 +682,6 @@ int ssl3_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->s3->flags |= SSL3_FLAGS_CCS_OK;
/* we should decide if we expected this one */
ret = ssl3_get_cert_verify(s);
if (ret <= 0)
goto end;
@ -710,11 +701,10 @@ int ssl3_accept(SSL *s)
case SSL3_ST_SR_NEXT_PROTO_A:
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.
* Enable CCS for NPN. Receiving a CCS clears the flag, so make
* sure not to re-enable it to ban duplicates. This *should* be the
* first time we have received one - but we check anyway to be
* cautious.
* 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.
@ -733,10 +723,8 @@ int ssl3_accept(SSL *s)
case SSL3_ST_SR_FINISHED_A:
case SSL3_ST_SR_FINISHED_B:
/*
* 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
* Enable CCS for handshakes without NPN. In NPN the CCS flag has
* already been 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
@ -2960,39 +2948,31 @@ int ssl3_get_cert_verify(SSL *s)
EVP_MD_CTX mctx;
EVP_MD_CTX_init(&mctx);
n = s->method->ssl_get_message(s,
SSL3_ST_SR_CERT_VRFY_A,
SSL3_ST_SR_CERT_VRFY_B,
-1, SSL3_RT_MAX_PLAIN_LENGTH, &ok);
if (!ok)
return ((int)n);
if (s->session->peer != NULL) {
peer = s->session->peer;
pkey = X509_get_pubkey(peer);
type = X509_certificate_type(peer, pkey);
} else {
peer = NULL;
pkey = NULL;
}
if (s->s3->tmp.message_type != SSL3_MT_CERTIFICATE_VERIFY) {
s->s3->tmp.reuse_message = 1;
if (peer != NULL) {
al = SSL_AD_UNEXPECTED_MESSAGE;
SSLerr(SSL_F_SSL3_GET_CERT_VERIFY, SSL_R_MISSING_VERIFY_MESSAGE);
goto f_err;
}
/*
* We should only process a CertificateVerify message if we have received
* a Certificate from the client. If so then |s->session->peer| will be non
* NULL. In some instances a CertificateVerify message is not required even
* if the peer has sent a Certificate (e.g. such as in the case of static
* DH). In that case the ClientKeyExchange processing will skip the
* CertificateVerify state so we should not arrive here.
*/
if (s->session->peer == NULL) {
ret = 1;
goto end;
}
if (peer == NULL) {
SSLerr(SSL_F_SSL3_GET_CERT_VERIFY, SSL_R_NO_CLIENT_CERT_RECEIVED);
al = SSL_AD_UNEXPECTED_MESSAGE;
goto f_err;
}
n = s->method->ssl_get_message(s,
SSL3_ST_SR_CERT_VRFY_A,
SSL3_ST_SR_CERT_VRFY_B,
SSL3_MT_CERTIFICATE_VERIFY,
SSL3_RT_MAX_PLAIN_LENGTH, &ok);
if (!ok)
return ((int)n);
peer = s->session->peer;
pkey = X509_get_pubkey(peer);
type = X509_certificate_type(peer, pkey);
if (!(type & EVP_PKT_SIGN)) {
SSLerr(SSL_F_SSL3_GET_CERT_VERIFY,
@ -3001,12 +2981,6 @@ int ssl3_get_cert_verify(SSL *s)
goto f_err;
}
if (s->s3->change_cipher_spec) {
SSLerr(SSL_F_SSL3_GET_CERT_VERIFY, SSL_R_CCS_RECEIVED_EARLY);
al = SSL_AD_UNEXPECTED_MESSAGE;
goto f_err;
}
/* we now have a signature that we need to verify */
p = (unsigned char *)s->init_msg;
/* Check for broken implementations of GOST ciphersuites */