[openssl-commits] [openssl] OpenSSL_1_0_2-stable update
Matt Caswell
matt at openssl.org
Wed May 13 10:30:07 UTC 2015
The branch OpenSSL_1_0_2-stable has been updated
via 464774d75f91ab84772de71743e3c8c0db9a96a6 (commit)
from 833518cf0e1e5224383a45cc68c8bb9c3a60865c (commit)
- Log -----------------------------------------------------------------
commit 464774d75f91ab84772de71743e3c8c0db9a96a6
Author: Matt Caswell <matt at openssl.org>
Date: Wed May 6 21:31:16 2015 +0100
Don't allow a CCS when expecting a CertificateVerify
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 at openssl.org>
(cherry picked from commit a0bd6493369d960abef11c2346b9bbb308b4285a)
-----------------------------------------------------------------------
Summary of changes:
ssl/d1_srvr.c | 18 ++++-----------
ssl/s3_srvr.c | 74 +++++++++++++++++++----------------------------------------
2 files changed, 28 insertions(+), 64 deletions(-)
diff --git a/ssl/d1_srvr.c b/ssl/d1_srvr.c
index 10726d6..655333a 100644
--- a/ssl/d1_srvr.c
+++ b/ssl/d1_srvr.c
@@ -695,15 +695,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;
@@ -720,11 +711,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.
diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index 2e0f989..f0a16c4 100644
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -684,15 +684,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;
@@ -712,11 +703,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.
@@ -735,10 +725,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
@@ -2951,39 +2939,31 @@ int ssl3_get_cert_verify(SSL *s)
EVP_MD_CTX mctx;
EVP_MD_CTX_init(&mctx);
+ /*
+ * 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;
+ }
+
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);
+ SSL3_MT_CERTIFICATE_VERIFY,
+ 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;
- }
- 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;
- }
+ 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,
@@ -2992,12 +2972,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 */
More information about the openssl-commits
mailing list