[openssl-commits] [openssl] master update
Emilia Kasper
emilia at openssl.org
Mon Oct 5 17:07:08 UTC 2015
The branch master has been updated
via 67202973cf55eaac021706c183377b8040cf0c20 (commit)
via bf0fc41266f17311c5db1e0541d3dd12eb27deb6 (commit)
via 38a3cbfbf728da0282c7e4ba29502740d853b1e6 (commit)
via b3e2272c59a5720467045e2ae62940fdb708ce76 (commit)
from 2ff00bdbc4aad268e07df82541ff4a16b1f91fe8 (commit)
- Log -----------------------------------------------------------------
commit 67202973cf55eaac021706c183377b8040cf0c20
Author: Emilia Kasper <emilia at openssl.org>
Date: Thu Oct 1 13:54:11 2015 +0200
Add PACKET_copy_all
Reviewed-by: Matt Caswell <matt at openssl.org>
commit bf0fc41266f17311c5db1e0541d3dd12eb27deb6
Author: Emilia Kasper <emilia at openssl.org>
Date: Thu Oct 1 13:00:39 2015 +0200
ssl_sess.c: grab a copy of the session ID
The user callback takes a non-const pointer, so don't pass PACKET data
to it directly; rather, grab a local copy.
Reviewed-by: Matt Caswell <matt at openssl.org>
commit 38a3cbfbf728da0282c7e4ba29502740d853b1e6
Author: Emilia Kasper <emilia at openssl.org>
Date: Thu Oct 1 12:53:08 2015 +0200
PACKETize and clean up ssl_bytes_to_cipher_list.
Fix alerts.
Reviewed-by: Matt Caswell <matt at openssl.org>
commit b3e2272c59a5720467045e2ae62940fdb708ce76
Author: Emilia Kasper <emilia at openssl.org>
Date: Wed Sep 30 15:33:12 2015 +0200
ssl3_get_client_hello: rearrange logic
Move all packet parsing to the beginning of the method. This limits the
SSLv2 compatibility soup to the parsing, and makes the rest of the
processing uniform.
This is also needed for simpler EMS support: EMS servers need to do an
early scan for EMS to make resumption decisions. This'll be easier when
the entire ClientHello is parsed in the beginning.
As a side effect,
1) PACKETize ssl_get_prev_session and tls1_process_ticket; and
2) Delete dead code for SSL_OP_NETSCAPE_REUSE_CIPHER_CHANGE_BUG.
Reviewed-by: Matt Caswell <matt at openssl.org>
-----------------------------------------------------------------------
Summary of changes:
include/openssl/ssl.h | 1 +
ssl/packet_locl.h | 27 ++-
ssl/s3_srvr.c | 536 ++++++++++++++++++++++----------------------------
ssl/ssl_locl.h | 8 +-
ssl/ssl_sess.c | 35 ++--
ssl/t1_lib.c | 49 ++---
test/packettest.c | 37 +++-
7 files changed, 341 insertions(+), 352 deletions(-)
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 8fa9363..4b21d0f 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -359,6 +359,7 @@ typedef int (*custom_ext_parse_cb) (SSL *s, unsigned int ext_type,
/* Allow initial connection to servers that don't support RI */
# define SSL_OP_LEGACY_SERVER_CONNECT 0x00000004L
/* Removed from OpenSSL 0.9.8q and 1.0.0c */
+/* Dead forever, see CVE-2010-4180. */
# define SSL_OP_NETSCAPE_REUSE_CIPHER_CHANGE_BUG 0x0L
# define SSL_OP_TLSEXT_PADDING 0x00000010L
# define SSL_OP_MICROSOFT_BIG_SSLV3_BUFFER 0x00000020L
diff --git a/ssl/packet_locl.h b/ssl/packet_locl.h
index 88cd202..e73eb3d 100644
--- a/ssl/packet_locl.h
+++ b/ssl/packet_locl.h
@@ -117,6 +117,13 @@ static inline int PACKET_buf_init(PACKET *pkt, unsigned char *buf, size_t len)
return 1;
}
+/* Initialize a PACKET to hold zero bytes. */
+static inline void PACKET_null_init(PACKET *pkt)
+{
+ pkt->curr = NULL;
+ pkt->remaining = 0;
+}
+
/*
* Peek ahead and initialize |subpkt| with the next |len| bytes read from |pkt|.
* Data is not copied: the |subpkt| packet will share its underlying buffer with
@@ -294,7 +301,7 @@ __owur static inline int PACKET_get_4(PACKET *pkt, unsigned long *data)
* underlying buffer gets freed
*/
__owur static inline int PACKET_peek_bytes(const PACKET *pkt, unsigned char **data,
- size_t len)
+ size_t len)
{
if (PACKET_remaining(pkt) < len)
return 0;
@@ -349,6 +356,24 @@ __owur static inline int PACKET_copy_bytes(PACKET *pkt, unsigned char *data,
}
/*
+ * Copy packet data to |dest|, and set |len| to the number of copied bytes.
+ * If the packet has more than |dest_len| bytes, nothing is copied.
+ * Returns 1 if the packet data fits in |dest_len| bytes, 0 otherwise.
+ * Does not forward PACKET position (because it is typically the last thing
+ * done with a given PACKET).
+ */
+__owur static inline int PACKET_copy_all(const PACKET *pkt, unsigned char *dest,
+ size_t dest_len, size_t *len) {
+ if (PACKET_remaining(pkt) > dest_len) {
+ *len = 0;
+ return 0;
+ }
+ *len = pkt->remaining;
+ memcpy(dest, pkt->curr, pkt->remaining);
+ return 1;
+}
+
+/*
* Copy |pkt| bytes to a newly allocated buffer and store a pointer to the
* result in |*data|, and the length in |len|.
* If |*data| is not NULL, the old data is OPENSSL_free'd.
diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index f771bd9..82162d8 100644
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -164,8 +164,10 @@
#include <openssl/bn.h>
#include <openssl/md5.h>
-static STACK_OF(SSL_CIPHER) *ssl_bytes_to_cipher_list(SSL *s, unsigned char *p,
- int num, STACK_OF(SSL_CIPHER) **skp, int sslv2format);
+static STACK_OF(SSL_CIPHER) *ssl_bytes_to_cipher_list(SSL *s,
+ PACKET *cipher_suites,
+ STACK_OF(SSL_CIPHER) **skp,
+ int sslv2format, int *al);
#ifndef OPENSSL_NO_SRP
@@ -847,7 +849,9 @@ int ssl3_get_client_hello(SSL *s)
#endif
STACK_OF(SSL_CIPHER) *ciphers = NULL;
int protverr = 1;
- PACKET pkt, cipher_suite, compression;
+ /* |cookie| will only be initialized for DTLS. */
+ PACKET pkt, session_id, cipher_suites, compression, extensions, cookie;
+ int is_v2_record;
if (s->state == SSL3_ST_SR_CLNT_HELLO_C && !s->first_packet)
goto retry_cert;
@@ -877,8 +881,10 @@ int ssl3_get_client_hello(SSL *s)
goto f_err;
}
+ is_v2_record = RECORD_LAYER_is_sslv2_record(&s->rlayer);
+
/* First lets get s->client_version set correctly */
- if (RECORD_LAYER_is_sslv2_record(&s->rlayer)) {
+ if (is_v2_record) {
unsigned int version;
unsigned int mt;
/*-
@@ -1004,13 +1010,15 @@ int ssl3_get_client_hello(SSL *s)
goto f_err;
}
- if (RECORD_LAYER_is_sslv2_record(&s->rlayer)) {
+ /* Parse the message and load client random. */
+ if (is_v2_record) {
/*
* Handle an SSLv2 backwards compatible ClientHello
* Note, this is only for SSLv3+ using the backward compatible format.
* Real SSLv2 is not supported, and is rejected above.
*/
unsigned int cipher_len, session_id_len, challenge_len;
+ PACKET challenge;
if (!PACKET_get_net_2(&pkt, &cipher_len)
|| !PACKET_get_net_2(&pkt, &session_id_len)
@@ -1020,309 +1028,241 @@ int ssl3_get_client_hello(SSL *s)
goto f_err;
}
- if (cipher_len == 0) {
- /* we need at least one cipher */
- al = SSL_AD_ILLEGAL_PARAMETER;
- SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_NO_CIPHERS_SPECIFIED);
- goto f_err;
- }
-
- if (!PACKET_get_sub_packet(&pkt, &cipher_suite, cipher_len)) {
- SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_RECORD_LENGTH_MISMATCH);
- al = SSL_AD_DECODE_ERROR;
- goto f_err;
- }
-
- if (ssl_bytes_to_cipher_list(s, PACKET_data(&cipher_suite),
- cipher_len, &(ciphers), 1) == NULL) {
- goto err;
- }
-
- /*
- * Ignore any session id. We don't allow resumption in a backwards
- * compatible ClientHello
- */
- if (!PACKET_forward(&pkt, session_id_len)) {
+ if (!PACKET_get_sub_packet(&pkt, &cipher_suites, cipher_len)
+ || !PACKET_get_sub_packet(&pkt, &session_id, session_id_len)
+ || !PACKET_get_sub_packet(&pkt, &challenge, challenge_len)
+ /* No extensions. */
+ || PACKET_remaining(&pkt) != 0) {
SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_RECORD_LENGTH_MISMATCH);
al = SSL_AD_DECODE_ERROR;
goto f_err;
}
- s->hit = 0;
-
- if (!ssl_get_new_session(s, 1))
- goto err;
/* Load the client random */
- i = challenge_len > SSL3_RANDOM_SIZE ? SSL3_RANDOM_SIZE : challenge_len;
+ challenge_len = challenge_len > SSL3_RANDOM_SIZE ? SSL3_RANDOM_SIZE :
+ challenge_len;
memset(s->s3->client_random, 0, SSL3_RANDOM_SIZE);
- if (!PACKET_peek_copy_bytes(&pkt,
- s->s3->client_random + SSL3_RANDOM_SIZE - i,
- i)
- || !PACKET_forward(&pkt, challenge_len)
- || PACKET_remaining(&pkt) != 0) {
- SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_RECORD_LENGTH_MISMATCH);
- al = SSL_AD_DECODE_ERROR;
+ if (!PACKET_copy_bytes(&challenge,
+ s->s3->client_random + SSL3_RANDOM_SIZE -
+ challenge_len, challenge_len)) {
+ SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, ERR_R_INTERNAL_ERROR);
+ al = SSL_AD_INTERNAL_ERROR;
goto f_err;
}
+
+ PACKET_null_init(&compression);
+ PACKET_null_init(&extensions);
+ /* We're never DTLS here but just play safe and initialize. */
+ PACKET_null_init(&cookie);
} else {
- /* If we get here we've got SSLv3+ in an SSLv3+ record */
- PACKET session_id;
- unsigned int cookie_len;
- /* load the client random and get the session-id */
+ /* Regular ClientHello. */
if (!PACKET_copy_bytes(&pkt, s->s3->client_random, SSL3_RANDOM_SIZE)
- || !PACKET_get_length_prefixed_1(&pkt, &session_id)) {
+ || !PACKET_get_length_prefixed_1(&pkt, &session_id)) {
al = SSL_AD_DECODE_ERROR;
- SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_LENGTH_TOO_SHORT);
+ SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_LENGTH_MISMATCH);
goto f_err;
}
- /*
- * If we require cookies and this ClientHello doesn't contain one, just
- * return since we do not want to allocate any memory yet. So check
- * cookie length...
- */
- if (SSL_get_options(s) & SSL_OP_COOKIE_EXCHANGE) {
-
- if (!PACKET_peek_1(&pkt, &cookie_len)) {
+ if (SSL_IS_DTLS(s)) {
+ if (!PACKET_get_length_prefixed_1(&pkt, &cookie)) {
al = SSL_AD_DECODE_ERROR;
- SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_LENGTH_TOO_SHORT);
+ SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_LENGTH_MISMATCH);
goto f_err;
}
-
- if (cookie_len == 0)
+ /*
+ * If we require cookies and this ClientHello doesn't contain one,
+ * just return since we do not want to allocate any memory yet.
+ * So check cookie length...
+ */
+ if (SSL_get_options(s) & SSL_OP_COOKIE_EXCHANGE) {
+ if (PACKET_remaining(&cookie) == 0)
return 1;
+ }
}
- s->hit = 0;
+ if (!PACKET_get_length_prefixed_2(&pkt, &cipher_suites)
+ || !PACKET_get_length_prefixed_1(&pkt, &compression)) {
+ al = SSL_AD_DECODE_ERROR;
+ SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_LENGTH_MISMATCH);
+ goto f_err;
+ }
+ /* Could be empty. */
+ extensions = pkt;
+ }
+
+ s->hit = 0;
+
+ /*
+ * We don't allow resumption in a backwards compatible ClientHello.
+ * TODO(openssl-team): in TLS1.1+, session_id MUST be empty.
+ *
+ * Versions before 0.9.7 always allow clients to resume sessions in
+ * renegotiation. 0.9.7 and later allow this by default, but optionally
+ * ignore resumption requests with flag
+ * SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION (it's a new flag rather
+ * than a change to default behavior so that applications relying on
+ * this for security won't even compile against older library versions).
+ * 1.0.1 and later also have a function SSL_renegotiate_abbreviated() to
+ * request renegotiation but not a new session (s->new_session remains
+ * unset): for servers, this essentially just means that the
+ * SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION setting will be
+ * ignored.
+ */
+ if (is_v2_record ||
+ (s->new_session &&
+ (s->options & SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION))) {
+ if (!ssl_get_new_session(s, 1))
+ goto err;
+ } else {
+ i = ssl_get_prev_session(s, &extensions, &session_id);
/*
- * Versions before 0.9.7 always allow clients to resume sessions in
- * renegotiation. 0.9.7 and later allow this by default, but optionally
- * ignore resumption requests with flag
- * SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION (it's a new flag rather
- * than a change to default behavior so that applications relying on
- * this for security won't even compile against older library versions).
- * 1.0.1 and later also have a function SSL_renegotiate_abbreviated() to
- * request renegotiation but not a new session (s->new_session remains
- * unset): for servers, this essentially just means that the
- * SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION setting will be
- * ignored.
+ * Only resume if the session's version matches the negotiated
+ * version.
+ * RFC 5246 does not provide much useful advice on resumption
+ * with a different protocol version. It doesn't forbid it but
+ * the sanity of such behaviour would be questionable.
+ * In practice, clients do not accept a version mismatch and
+ * will abort the handshake with an error.
*/
- if ((s->new_session
- && (s->options & SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION))) {
- if (!ssl_get_new_session(s, 1))
- goto err;
+ if (i == 1 && s->version == s->session->ssl_version) {
+ /* previous session */
+ s->hit = 1;
+ } else if (i == -1) {
+ goto err;
} else {
- /*
- * TODO(openssl-team): ssl_get_prev_session passes a non-const
- * 'unsigned char*' session id to a user callback. Grab a copy of
- * the data?
- */
- i = ssl_get_prev_session(s, &pkt, PACKET_data(&session_id),
- PACKET_remaining(&session_id));
- /*
- * Only resume if the session's version matches the negotiated
- * version.
- * RFC 5246 does not provide much useful advice on resumption
- * with a different protocol version. It doesn't forbid it but
- * the sanity of such behaviour would be questionable.
- * In practice, clients do not accept a version mismatch and
- * will abort the handshake with an error.
- */
- if (i == 1 && s->version == s->session->ssl_version) {
- /* previous session */
- s->hit = 1;
- } else if (i == -1)
+ /* i == 0 */
+ if (!ssl_get_new_session(s, 1))
goto err;
- else {
- /* i == 0 */
- if (!ssl_get_new_session(s, 1))
- goto err;
- }
}
+ }
- if (SSL_IS_DTLS(s)) {
- PACKET cookie;
- if (!PACKET_get_length_prefixed_1(&pkt, &cookie)) {
- al = SSL_AD_DECODE_ERROR;
- SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_LENGTH_TOO_SHORT);
- goto f_err;
- }
- cookie_len = PACKET_remaining(&cookie);
+ if (SSL_IS_DTLS(s)) {
+ size_t cookie_len = PACKET_remaining(&cookie);
+ /*
+ * The ClientHello may contain a cookie even if the
+ * HelloVerify message has not been sent--make sure that it
+ * does not cause an overflow.
+ */
+ if (cookie_len > sizeof(s->d1->rcvd_cookie)) {
+ /* too much data */
+ al = SSL_AD_DECODE_ERROR;
+ SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_COOKIE_MISMATCH);
+ goto f_err;
+ }
+
+ /* verify the cookie if appropriate option is set. */
+ if ((SSL_get_options(s) & SSL_OP_COOKIE_EXCHANGE) && cookie_len > 0) {
+ /* Get cookie */
/*
- * The ClientHello may contain a cookie even if the
- * HelloVerify message has not been sent--make sure that it
- * does not cause an overflow.
+ * TODO(openssl-team): rcvd_cookie appears unused outside this
+ * function. Remove the field?
*/
- if (cookie_len > sizeof(s->d1->rcvd_cookie)) {
- /* too much data */
- al = SSL_AD_DECODE_ERROR;
- SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_COOKIE_MISMATCH);
+ if (!PACKET_copy_bytes(&cookie, s->d1->rcvd_cookie, cookie_len)) {
+ al = SSL_AD_INTERNAL_ERROR;
+ SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, ERR_R_INTERNAL_ERROR);
goto f_err;
}
- /* verify the cookie if appropriate option is set. */
- if ((SSL_get_options(s) & SSL_OP_COOKIE_EXCHANGE)
- && cookie_len > 0) {
- /* Get cookie */
- /*
- * TODO(openssl-team): rcvd_cookie appears unused outside this
- * function. Remove the field?
- */
- if (!PACKET_copy_bytes(&cookie, s->d1->rcvd_cookie, cookie_len)) {
- al = SSL_AD_INTERNAL_ERROR;
- SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, ERR_R_INTERNAL_ERROR);
- goto f_err;
- }
-
- if (s->ctx->app_verify_cookie_cb != NULL) {
- if (s->ctx->app_verify_cookie_cb(s, s->d1->rcvd_cookie,
- cookie_len) == 0) {
- al = SSL_AD_HANDSHAKE_FAILURE;
- SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO,
- SSL_R_COOKIE_MISMATCH);
- goto f_err;
- }
- /* else cookie verification succeeded */
- }
- /* default verification */
- else if (memcmp(s->d1->rcvd_cookie, s->d1->cookie,
- s->d1->cookie_len) != 0) {
+ if (s->ctx->app_verify_cookie_cb != NULL) {
+ if (s->ctx->app_verify_cookie_cb(s, s->d1->rcvd_cookie,
+ cookie_len) == 0) {
al = SSL_AD_HANDSHAKE_FAILURE;
- SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_COOKIE_MISMATCH);
- goto f_err;
- }
- /* Set to -2 so if successful we return 2 */
- ret = -2;
- }
- if (s->method->version == DTLS_ANY_VERSION) {
- /* Select version to use */
- if (s->client_version <= DTLS1_2_VERSION &&
- !(s->options & SSL_OP_NO_DTLSv1_2)) {
- s->version = DTLS1_2_VERSION;
- s->method = DTLSv1_2_server_method();
- } else if (tls1_suiteb(s)) {
SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO,
- SSL_R_ONLY_DTLS_1_2_ALLOWED_IN_SUITEB_MODE);
- s->version = s->client_version;
- al = SSL_AD_PROTOCOL_VERSION;
- goto f_err;
- } else if (s->client_version <= DTLS1_VERSION &&
- !(s->options & SSL_OP_NO_DTLSv1)) {
- s->version = DTLS1_VERSION;
- s->method = DTLSv1_server_method();
- } else {
- SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO,
- SSL_R_WRONG_VERSION_NUMBER);
- s->version = s->client_version;
- al = SSL_AD_PROTOCOL_VERSION;
+ SSL_R_COOKIE_MISMATCH);
goto f_err;
}
- s->session->ssl_version = s->version;
+ /* else cookie verification succeeded */
}
+ /* default verification */
+ else if (memcmp(s->d1->rcvd_cookie, s->d1->cookie,
+ s->d1->cookie_len) != 0) {
+ al = SSL_AD_HANDSHAKE_FAILURE;
+ SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_COOKIE_MISMATCH);
+ goto f_err;
+ }
+ /* Set to -2 so if successful we return 2 */
+ ret = -2;
}
-
- if (!PACKET_get_length_prefixed_2(&pkt, &cipher_suite)) {
- al = SSL_AD_DECODE_ERROR;
- SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_LENGTH_TOO_SHORT);
- goto f_err;
- }
-
- if (PACKET_remaining(&cipher_suite) == 0) {
- al = SSL_AD_ILLEGAL_PARAMETER;
- SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_NO_CIPHERS_SPECIFIED);
- goto f_err;
+ if (s->method->version == DTLS_ANY_VERSION) {
+ /* Select version to use */
+ if (s->client_version <= DTLS1_2_VERSION &&
+ !(s->options & SSL_OP_NO_DTLSv1_2)) {
+ s->version = DTLS1_2_VERSION;
+ s->method = DTLSv1_2_server_method();
+ } else if (tls1_suiteb(s)) {
+ SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO,
+ SSL_R_ONLY_DTLS_1_2_ALLOWED_IN_SUITEB_MODE);
+ s->version = s->client_version;
+ al = SSL_AD_PROTOCOL_VERSION;
+ goto f_err;
+ } else if (s->client_version <= DTLS1_VERSION &&
+ !(s->options & SSL_OP_NO_DTLSv1)) {
+ s->version = DTLS1_VERSION;
+ s->method = DTLSv1_server_method();
+ } else {
+ SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO,
+ SSL_R_WRONG_VERSION_NUMBER);
+ s->version = s->client_version;
+ al = SSL_AD_PROTOCOL_VERSION;
+ goto f_err;
+ }
+ s->session->ssl_version = s->version;
}
+ }
- if (ssl_bytes_to_cipher_list(s, PACKET_data(&cipher_suite),
- PACKET_remaining(&cipher_suite),
- &(ciphers), 0) == NULL) {
- goto err;
- }
+ if (ssl_bytes_to_cipher_list(s, &cipher_suites, &(ciphers),
+ is_v2_record, &al) == NULL) {
+ goto f_err;
+ }
- /* If it is a hit, check that the cipher is in the list */
- if (s->hit) {
- j = 0;
- id = s->session->cipher->id;
+ /* If it is a hit, check that the cipher is in the list */
+ if (s->hit) {
+ j = 0;
+ id = s->session->cipher->id;
#ifdef CIPHER_DEBUG
- fprintf(stderr, "client sent %d ciphers\n",
- sk_SSL_CIPHER_num(ciphers));
+ fprintf(stderr, "client sent %d ciphers\n",
+ sk_SSL_CIPHER_num(ciphers));
#endif
- for (i = 0; i < sk_SSL_CIPHER_num(ciphers); i++) {
- c = sk_SSL_CIPHER_value(ciphers, i);
+ for (i = 0; i < sk_SSL_CIPHER_num(ciphers); i++) {
+ c = sk_SSL_CIPHER_value(ciphers, i);
#ifdef CIPHER_DEBUG
- fprintf(stderr, "client [%2d of %2d]:%s\n",
- i, sk_SSL_CIPHER_num(ciphers), SSL_CIPHER_get_name(c));
+ fprintf(stderr, "client [%2d of %2d]:%s\n",
+ i, sk_SSL_CIPHER_num(ciphers), SSL_CIPHER_get_name(c));
#endif
- if (c->id == id) {
- j = 1;
- break;
- }
- }
- /*
- * Disabled because it can be used in a ciphersuite downgrade
- * attack:
- * CVE-2010-4180.
- */
-#if 0
- if (j == 0 && (s->options & SSL_OP_NETSCAPE_REUSE_CIPHER_CHANGE_BUG)
- && (sk_SSL_CIPHER_num(ciphers) == 1)) {
- /*
- * Special case as client bug workaround: the previously used
- * cipher may not be in the current list, the client instead
- * might be trying to continue using a cipher that before wasn't
- * chosen due to server preferences. We'll have to reject the
- * connection if the cipher is not enabled, though.
- */
- c = sk_SSL_CIPHER_value(ciphers, 0);
- if (sk_SSL_CIPHER_find(SSL_get_ciphers(s), c) >= 0) {
- s->session->cipher = c;
- j = 1;
- }
- }
-#endif
- if (j == 0) {
- /*
- * we need to have the cipher in the cipher list if we are asked
- * to reuse it
- */
- al = SSL_AD_ILLEGAL_PARAMETER;
- SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO,
- SSL_R_REQUIRED_CIPHER_MISSING);
- goto f_err;
+ if (c->id == id) {
+ j = 1;
+ break;
}
}
-
- /* compression */
- if (!PACKET_get_length_prefixed_1(&pkt, &compression)) {
- /* not enough data */
- al = SSL_AD_DECODE_ERROR;
+ if (j == 0) {
/*
- * TODO(openssl-team):
- * SSL_R_LENGTH_TOO_SHORT and SSL_R_LENGTH_MISMATCH are used
- * interchangeably. Pick one.
+ * we need to have the cipher in the cipher list if we are asked
+ * to reuse it
*/
- SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_LENGTH_MISMATCH);
+ al = SSL_AD_ILLEGAL_PARAMETER;
+ SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO,
+ SSL_R_REQUIRED_CIPHER_MISSING);
goto f_err;
}
+ }
- complen = PACKET_remaining(&compression);
- for (j = 0; j < complen; j++) {
- if (PACKET_data(&compression)[j] == 0)
- break;
- }
-
- if (j >= complen) {
- /* no compress */
- al = SSL_AD_DECODE_ERROR;
- SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_NO_COMPRESSION_SPECIFIED);
- goto f_err;
- }
+ complen = PACKET_remaining(&compression);
+ for (j = 0; j < complen; j++) {
+ if (PACKET_data(&compression)[j] == 0)
+ break;
}
+ if (j >= complen) {
+ /* no compress */
+ al = SSL_AD_DECODE_ERROR;
+ SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_NO_COMPRESSION_SPECIFIED);
+ goto f_err;
+ }
+
/* TLS extensions */
if (s->version >= SSL3_VERSION) {
- if (!ssl_parse_clienthello_tlsext(s, &pkt)) {
+ if (!ssl_parse_clienthello_tlsext(s, &extensions)) {
SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_PARSE_TLSEXT);
goto err;
}
@@ -3505,32 +3445,40 @@ err:
#define SSLV2_CIPHER_LEN 3
-STACK_OF(SSL_CIPHER) *ssl_bytes_to_cipher_list(SSL *s, unsigned char *p,
- int num,
+STACK_OF(SSL_CIPHER) *ssl_bytes_to_cipher_list(SSL *s,
+ PACKET *cipher_suites,
STACK_OF(SSL_CIPHER) **skp,
- int sslv2format)
+ int sslv2format, int *al
+ )
{
const SSL_CIPHER *c;
STACK_OF(SSL_CIPHER) *sk;
- int i, n;
+ int n;
+ /* 3 = SSLV2_CIPHER_LEN > TLS_CIPHER_LEN = 2. */
+ unsigned char cipher[SSLV2_CIPHER_LEN];
- if (s->s3)
- s->s3->send_connection_binding = 0;
+ s->s3->send_connection_binding = 0;
- if(sslv2format) {
- n = SSLV2_CIPHER_LEN;
- } else {
- n = TLS_CIPHER_LEN;
+ n = sslv2format ? SSLV2_CIPHER_LEN : TLS_CIPHER_LEN;
+
+ if (PACKET_remaining(cipher_suites) == 0) {
+ SSLerr(SSL_F_SSL_BYTES_TO_CIPHER_LIST, SSL_R_NO_CIPHERS_SPECIFIED);
+ *al = SSL_AD_ILLEGAL_PARAMETER;
+ return NULL;
}
- if (n == 0 || (num % n) != 0) {
+
+ if (PACKET_remaining(cipher_suites) % n != 0) {
SSLerr(SSL_F_SSL_BYTES_TO_CIPHER_LIST,
SSL_R_ERROR_IN_RECEIVED_CIPHER_LIST);
- return (NULL);
+ *al = SSL_AD_DECODE_ERROR;
+ return NULL;
}
+
if ((skp == NULL) || (*skp == NULL)) {
sk = sk_SSL_CIPHER_new_null(); /* change perhaps later */
if(sk == NULL) {
SSLerr(SSL_F_SSL_BYTES_TO_CIPHER_LIST, ERR_R_MALLOC_FAILURE);
+ *al = SSL_AD_INTERNAL_ERROR;
return NULL;
}
} else {
@@ -3538,28 +3486,33 @@ STACK_OF(SSL_CIPHER) *ssl_bytes_to_cipher_list(SSL *s, unsigned char *p,
sk_SSL_CIPHER_zero(sk);
}
- OPENSSL_free(s->s3->tmp.ciphers_raw);
- s->s3->tmp.ciphers_raw = BUF_memdup(p, num);
- if (s->s3->tmp.ciphers_raw == NULL) {
- SSLerr(SSL_F_SSL_BYTES_TO_CIPHER_LIST, ERR_R_MALLOC_FAILURE);
+ if (!PACKET_memdup(cipher_suites, &s->s3->tmp.ciphers_raw,
+ &s->s3->tmp.ciphers_rawlen)) {
+ *al = SSL_AD_INTERNAL_ERROR;
goto err;
}
- s->s3->tmp.ciphers_rawlen = (size_t)num;
- for (i = 0; i < num; i += n) {
+ while (PACKET_copy_bytes(cipher_suites, cipher, n)) {
+ /*
+ * We only support SSLv2 format ciphers in SSLv3+ using a
+ * SSLv2 backward compatible ClientHello. In this case the first
+ * byte is always 0 for SSLv3 compatible ciphers. Anything else
+ * is an SSLv2 cipher and we ignore it
+ */
+ if (sslv2format && cipher[0] != '\0')
+ continue;
+
/* Check for TLS_EMPTY_RENEGOTIATION_INFO_SCSV */
- if (s->s3 && (n != 3 || !p[0]) &&
- (p[n - 2] == ((SSL3_CK_SCSV >> 8) & 0xff)) &&
- (p[n - 1] == (SSL3_CK_SCSV & 0xff))) {
+ if ((cipher[n - 2] == ((SSL3_CK_SCSV >> 8) & 0xff)) &&
+ (cipher[n - 1] == (SSL3_CK_SCSV & 0xff))) {
/* SCSV fatal if renegotiating */
if (s->renegotiate) {
SSLerr(SSL_F_SSL_BYTES_TO_CIPHER_LIST,
SSL_R_SCSV_RECEIVED_WHEN_RENEGOTIATING);
- ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE);
+ *al = SSL_AD_HANDSHAKE_FAILURE;
goto err;
}
s->s3->send_connection_binding = 1;
- p += n;
#ifdef OPENSSL_RI_DEBUG
fprintf(stderr, "SCSV received by server\n");
#endif
@@ -3567,9 +3520,8 @@ STACK_OF(SSL_CIPHER) *ssl_bytes_to_cipher_list(SSL *s, unsigned char *p,
}
/* Check for TLS_FALLBACK_SCSV */
- if ((n != 3 || !p[0]) &&
- (p[n - 2] == ((SSL3_CK_FALLBACK_SCSV >> 8) & 0xff)) &&
- (p[n - 1] == (SSL3_CK_FALLBACK_SCSV & 0xff))) {
+ if ((cipher[n - 2] == ((SSL3_CK_FALLBACK_SCSV >> 8) & 0xff)) &&
+ (cipher[n - 1] == (SSL3_CK_FALLBACK_SCSV & 0xff))) {
/*
* The SCSV indicates that the client previously tried a higher
* version. Fail if the current version is an unexpected
@@ -3578,37 +3530,27 @@ STACK_OF(SSL_CIPHER) *ssl_bytes_to_cipher_list(SSL *s, unsigned char *p,
if (!SSL_ctrl(s, SSL_CTRL_CHECK_PROTO_VERSION, 0, NULL)) {
SSLerr(SSL_F_SSL_BYTES_TO_CIPHER_LIST,
SSL_R_INAPPROPRIATE_FALLBACK);
- if (s->s3)
- ssl3_send_alert(s, SSL3_AL_FATAL,
- SSL_AD_INAPPROPRIATE_FALLBACK);
+ *al = SSL_AD_INAPPROPRIATE_FALLBACK;
goto err;
}
- p += n;
continue;
}
- if(sslv2format) {
- /*
- * We only support SSLv2 format ciphers in SSLv3+ using a
- * SSLv2 backward compatible ClientHello. In this case the first
- * byte is always 0 for SSLv3 compatible ciphers. Anything else
- * is an SSLv2 cipher and we ignore it
- */
- if(p[0] == 0)
- c = ssl_get_cipher_by_char(s, &p[1]);
- else
- c = NULL;
- } else {
- c = ssl_get_cipher_by_char(s, p);
- }
- p += n;
+ /* For SSLv2-compat, ignore leading 0-byte. */
+ c = ssl_get_cipher_by_char(s, sslv2format ? &cipher[1] : cipher);
if (c != NULL) {
if (!sk_SSL_CIPHER_push(sk, c)) {
SSLerr(SSL_F_SSL_BYTES_TO_CIPHER_LIST, ERR_R_MALLOC_FAILURE);
+ *al = SSL_AD_INTERNAL_ERROR;
goto err;
}
}
}
+ if (PACKET_remaining(cipher_suites) > 0) {
+ *al = SSL_AD_INTERNAL_ERROR;
+ SSLerr(SSL_F_SSL_BYTES_TO_CIPHER_LIST, ERR_R_INTERNAL_ERROR);
+ goto err;
+ }
if (skp != NULL)
*skp = sk;
@@ -3616,5 +3558,5 @@ STACK_OF(SSL_CIPHER) *ssl_bytes_to_cipher_list(SSL *s, unsigned char *p,
err:
if ((skp == NULL) || (*skp == NULL))
sk_SSL_CIPHER_free(sk);
- return (NULL);
+ return NULL;
}
diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index 544c1ad..7c57509 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -1861,8 +1861,8 @@ __owur CERT *ssl_cert_dup(CERT *cert);
void ssl_cert_clear_certs(CERT *c);
void ssl_cert_free(CERT *c);
__owur int ssl_get_new_session(SSL *s, int session);
-__owur int ssl_get_prev_session(SSL *s, PACKET *pkt, unsigned char *session,
- int len);
+__owur int ssl_get_prev_session(SSL *s, const PACKET *ext,
+ const PACKET *session_id);
__owur SSL_SESSION *ssl_session_dup(SSL_SESSION *src, int ticket);
__owur int ssl_cipher_id_cmp(const SSL_CIPHER *a, const SSL_CIPHER *b);
DECLARE_OBJ_BSEARCH_GLOBAL_CMP_FN(SSL_CIPHER, SSL_CIPHER, ssl_cipher_id);
@@ -2113,8 +2113,8 @@ __owur int tls1_process_heartbeat(SSL *s, unsigned char *p, unsigned int length)
__owur int dtls1_process_heartbeat(SSL *s, unsigned char *p, unsigned int length);
# endif
-__owur int tls1_process_ticket(SSL *s, PACKET *pkt, unsigned char *session_id,
- int len, SSL_SESSION **ret);
+__owur int tls1_process_ticket(SSL *s, const PACKET *ext,
+ const PACKET *session_id, SSL_SESSION **ret);
__owur int tls12_get_sigandhash(unsigned char *p, const EVP_PKEY *pk,
const EVP_MD *md);
diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c
index 3774db4..7660292 100644
--- a/ssl/ssl_sess.c
+++ b/ssl/ssl_sess.c
@@ -513,11 +513,8 @@ int ssl_get_new_session(SSL *s, int session)
* ssl_get_prev attempts to find an SSL_SESSION to be used to resume this
* connection. It is only called by servers.
*
- * session_id: points at the session ID in the ClientHello. This code will
- * read past the end of this in order to parse out the session ticket
- * extension, if any.
- * len: the length of the session ID.
- * limit: a pointer to the first byte after the ClientHello.
+ * ext: ClientHello extensions (including length prefix)
+ * session_id: ClientHello session ID.
*
* Returns:
* -1: error
@@ -529,8 +526,7 @@ int ssl_get_new_session(SSL *s, int session)
* - Both for new and resumed sessions, s->tlsext_ticket_expected is set to 1
* if the server should issue a new session ticket (to 0 otherwise).
*/
-int ssl_get_prev_session(SSL *s, PACKET *pkt, unsigned char *session_id,
- int len)
+int ssl_get_prev_session(SSL *s, const PACKET *ext, const PACKET *session_id)
{
/* This is used only by servers. */
@@ -538,15 +534,16 @@ int ssl_get_prev_session(SSL *s, PACKET *pkt, unsigned char *session_id,
int fatal = 0;
int try_session_cache = 1;
int r;
+ size_t len = PACKET_remaining(session_id);
- if (len < 0 || len > SSL_MAX_SSL_SESSION_ID_LENGTH)
+ if (len > SSL_MAX_SSL_SESSION_ID_LENGTH)
goto err;
if (len == 0)
try_session_cache = 0;
/* sets s->tlsext_ticket_expected */
- r = tls1_process_ticket(s, pkt, session_id, len, &ret);
+ r = tls1_process_ticket(s, ext, session_id, &ret);
switch (r) {
case -1: /* Error during processing */
fatal = 1;
@@ -567,11 +564,14 @@ int ssl_get_prev_session(SSL *s, PACKET *pkt, unsigned char *session_id,
!(s->session_ctx->session_cache_mode &
SSL_SESS_CACHE_NO_INTERNAL_LOOKUP)) {
SSL_SESSION data;
+ size_t local_len;
data.ssl_version = s->version;
- data.session_id_length = len;
- if (len == 0)
- return 0;
- memcpy(data.session_id, session_id, len);
+ if (!PACKET_copy_all(session_id, data.session_id,
+ sizeof(data.session_id),
+ &local_len)) {
+ goto err;
+ }
+ data.session_id_length = local_len;
CRYPTO_r_lock(CRYPTO_LOCK_SSL_CTX);
ret = lh_SSL_SESSION_retrieve(s->session_ctx->sessions, &data);
if (ret != NULL) {
@@ -586,8 +586,15 @@ int ssl_get_prev_session(SSL *s, PACKET *pkt, unsigned char *session_id,
if (try_session_cache &&
ret == NULL && s->session_ctx->get_session_cb != NULL) {
int copy = 1;
+ /* The user callback takes a non-const pointer, so grab a local copy. */
+ unsigned char *sid = NULL;
+ size_t sid_len;
+ if (!PACKET_memdup(session_id, &sid, &sid_len))
+ goto err;
+ ret = s->session_ctx->get_session_cb(s, sid, sid_len, ©);
+ OPENSSL_free(sid);
- if ((ret = s->session_ctx->get_session_cb(s, session_id, len, ©))) {
+ if (ret != NULL) {
s->session_ctx->stats.sess_cb_hit++;
/*
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 463f34e..aeae5b0 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -2901,11 +2901,8 @@ int ssl_parse_serverhello_tlsext(SSL *s, PACKET *pkt)
* ClientHello, and other operations depend on the result, we need to handle
* any TLS session ticket extension at the same time.
*
- * session_id: points at the session ID in the ClientHello. This code will
- * read past the end of this in order to parse out the session ticket
- * extension, if any.
- * len: the length of the session ID.
- * limit: a pointer to the first byte after the ClientHello.
+ * session_id: ClientHello session ID.
+ * ext: ClientHello extensions (including length prefix)
* ret: (output) on return, if a ticket was decrypted, then this is set to
* point to the resulting session.
*
@@ -2930,11 +2927,11 @@ int ssl_parse_serverhello_tlsext(SSL *s, PACKET *pkt)
* s->ctx->tlsext_ticket_key_cb asked to renew the client's ticket.
* Otherwise, s->tlsext_ticket_expected is set to 0.
*/
-int tls1_process_ticket(SSL *s, PACKET *pkt, unsigned char *session_id,
- int len, SSL_SESSION **ret)
+int tls1_process_ticket(SSL *s, const PACKET *ext, const PACKET *session_id,
+ SSL_SESSION **ret)
{
unsigned int i;
- PACKET bookmark = *pkt;
+ PACKET local_ext = *ext;
int retv = -1;
*ret = NULL;
@@ -2949,38 +2946,20 @@ int tls1_process_ticket(SSL *s, PACKET *pkt, unsigned char *session_id,
if ((s->version <= SSL3_VERSION))
return 0;
- /* Skip past DTLS cookie */
- if (SSL_IS_DTLS(s)) {
- if (!PACKET_get_1(pkt, &i)
- || !PACKET_forward(pkt, i)) {
- retv = -1;
- goto end;
- }
- }
- /* Skip past cipher list and compression algorithm list */
- if (!PACKET_get_net_2(pkt, &i)
- || !PACKET_forward(pkt, i)
- || !PACKET_get_1(pkt, &i)
- || !PACKET_forward(pkt, i)) {
- retv = -1;
- goto end;
- }
-
- /* Now at start of extensions */
- if (!PACKET_get_net_2(pkt, &i)) {
+ if (!PACKET_get_net_2(&local_ext, &i)) {
retv = 0;
goto end;
}
- while (PACKET_remaining (pkt) >= 4) {
+ while (PACKET_remaining(&local_ext) >= 4) {
unsigned int type, size;
- if (!PACKET_get_net_2(pkt, &type)
- || !PACKET_get_net_2(pkt, &size)) {
+ if (!PACKET_get_net_2(&local_ext, &type)
+ || !PACKET_get_net_2(&local_ext, &size)) {
/* Shouldn't ever happen */
retv = -1;
goto end;
}
- if (PACKET_remaining(pkt) < size) {
+ if (PACKET_remaining(&local_ext) < size) {
retv = 0;
goto end;
}
@@ -3007,12 +2986,13 @@ int tls1_process_ticket(SSL *s, PACKET *pkt, unsigned char *session_id,
retv = 2;
goto end;
}
- if (!PACKET_get_bytes(pkt, &etick, size)) {
+ if (!PACKET_get_bytes(&local_ext, &etick, size)) {
/* Shouldn't ever happen */
retv = -1;
goto end;
}
- r = tls_decrypt_ticket(s, etick, size, session_id, len, ret);
+ r = tls_decrypt_ticket(s, etick, size, PACKET_data(session_id),
+ PACKET_remaining(session_id), ret);
switch (r) {
case 2: /* ticket couldn't be decrypted */
s->tlsext_ticket_expected = 1;
@@ -3031,7 +3011,7 @@ int tls1_process_ticket(SSL *s, PACKET *pkt, unsigned char *session_id,
}
goto end;
} else {
- if (!PACKET_forward(pkt, size)) {
+ if (!PACKET_forward(&local_ext, size)) {
retv = -1;
goto end;
}
@@ -3039,7 +3019,6 @@ int tls1_process_ticket(SSL *s, PACKET *pkt, unsigned char *session_id,
}
retv = 0;
end:
- *pkt = bookmark;
return retv;
}
diff --git a/test/packettest.c b/test/packettest.c
index 6ee2ab1..915b42b 100644
--- a/test/packettest.c
+++ b/test/packettest.c
@@ -240,6 +240,25 @@ static int test_PACKET_copy_bytes(unsigned char buf[BUF_LEN])
return 1;
}
+static int test_PACKET_copy_all(unsigned char buf[BUF_LEN])
+{
+ unsigned char dup[BUF_LEN];
+ PACKET pkt;
+ size_t len;
+
+ if ( !PACKET_buf_init(&pkt, buf, BUF_LEN)
+ || !PACKET_copy_all(&pkt, dup, BUF_LEN, &len)
+ || len != BUF_LEN
+ || memcmp(buf, dup, BUF_LEN) != 0
+ || PACKET_remaining(&pkt) != BUF_LEN
+ || PACKET_copy_all(&pkt, dup, BUF_LEN - 1, &len)) {
+ fprintf(stderr, "test_PACKET_copy_bytes() failed\n");
+ return 0;
+ }
+
+ return 1;
+}
+
static int test_PACKET_memdup(unsigned char buf[BUF_LEN])
{
unsigned char *data = NULL;
@@ -314,7 +333,7 @@ static int test_PACKET_buf_init()
unsigned char buf[BUF_LEN];
PACKET pkt;
- /* Also tests PACKET_get_len() */
+ /* Also tests PACKET_remaining() */
if ( !PACKET_buf_init(&pkt, buf, 4)
|| PACKET_remaining(&pkt) != 4
|| !PACKET_buf_init(&pkt, buf, BUF_LEN)
@@ -327,6 +346,20 @@ static int test_PACKET_buf_init()
return 1;
}
+static int test_PACKET_null_init()
+{
+ PACKET pkt;
+
+ PACKET_null_init(&pkt);
+ if ( PACKET_remaining(&pkt) != 0
+ || PACKET_forward(&pkt, 1)) {
+ fprintf(stderr, "test_PACKET_null_init() failed\n");
+ return 0;
+ }
+
+ return 1;
+}
+
static int test_PACKET_get_length_prefixed_1()
{
unsigned char buf[BUF_LEN];
@@ -417,6 +450,7 @@ int main(int argc, char **argv)
i = 0;
if ( !test_PACKET_buf_init()
+ || !test_PACKET_null_init()
|| !test_PACKET_remaining(buf)
|| !test_PACKET_get_1(buf)
|| !test_PACKET_get_4(buf)
@@ -426,6 +460,7 @@ int main(int argc, char **argv)
|| !test_PACKET_get_sub_packet(buf)
|| !test_PACKET_get_bytes(buf)
|| !test_PACKET_copy_bytes(buf)
+ || !test_PACKET_copy_all(buf)
|| !test_PACKET_memdup(buf)
|| !test_PACKET_strndup()
|| !test_PACKET_forward(buf)
More information about the openssl-commits
mailing list