Convert the renegotiation extension to the new system.
This change also switches the behaviour of the client. Previously the
client would send the SCSV rather than the extension, but now it'll only
do that for SSLv3 connections.
Change-Id: I67a04b8abbef2234747c0dac450458deb6b0cd0a
Reviewed-on: https://boringssl-review.googlesource.com/5143
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/crypto/err/ssl.errordata b/crypto/err/ssl.errordata
index e926724..96daf20 100644
--- a/crypto/err/ssl.errordata
+++ b/crypto/err/ssl.errordata
@@ -82,6 +82,8 @@
SSL,function,279,dtls1_seal_record
SSL,function,164,dtls1_send_hello_verify_request
SSL,function,165,dtls1_write_app_data
+SSL,function,285,ext_ri_parse_clienthello
+SSL,function,286,ext_ri_parse_serverhello
SSL,function,166,i2d_SSL_SESSION
SSL,function,167,ssl3_accept
SSL,function,169,ssl3_cert_verify_hash
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 93d5efe..5259793 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -349,10 +349,6 @@
/* SSL_OP_NO_TICKET disables session ticket support (RFC 4507). */
#define SSL_OP_NO_TICKET 0x00004000L
-/* SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION permits unsafe legacy renegotiation
- * without renegotiation_info (RFC 5746) support. */
-#define SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION 0x00040000L
-
/* SSL_OP_CIPHER_SERVER_PREFERENCE configures servers to select ciphers and
* ECDHE curves according to the server's preferences instead of the
* client's. */
@@ -380,6 +376,7 @@
#define SSL_OP_SSLREF2_REUSE_CERT_TYPE_BUG 0
#define SSL_OP_TLS_BLOCK_PADDING_BUG 0
#define SSL_OP_TLS_ROLLBACK_BUG 0
+#define SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION 0
/* SSL_CTX_set_options enables all options set in |options| (which should be one
* or more of the |SSL_OP_*| values, ORed together) in |ctx|. It returns a
@@ -2874,6 +2871,8 @@
#define SSL_F_ssl3_check_certificate_for_cipher 282
#define SSL_F_SSL_CTX_get_tlsext_ticket_keys 283
#define SSL_F_SSL_CTX_set_tlsext_ticket_keys 284
+#define SSL_F_ext_ri_parse_clienthello 285
+#define SSL_F_ext_ri_parse_serverhello 286
#define SSL_R_APP_DATA_IN_HANDSHAKE 100
#define SSL_R_ATTEMPT_TO_REUSE_SESSION_IN_DIFFERENT_CONTEXT 101
#define SSL_R_BAD_ALERT 102
diff --git a/ssl/CMakeLists.txt b/ssl/CMakeLists.txt
index cf5a29d..4379060 100644
--- a/ssl/CMakeLists.txt
+++ b/ssl/CMakeLists.txt
@@ -31,7 +31,6 @@
ssl_txt.c
t1_enc.c
t1_lib.c
- t1_reneg.c
$<TARGET_OBJECTS:pqueue>
)
diff --git a/ssl/internal.h b/ssl/internal.h
index 1736fb0..c136167 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -1160,12 +1160,6 @@
* the wire version except at API boundaries. */
uint16_t ssl3_version_from_wire(SSL *s, uint16_t wire_version);
-int ssl_add_serverhello_renegotiate_ext(SSL *s, uint8_t *p, int *len,
- int maxlen);
-int ssl_parse_serverhello_renegotiate_ext(SSL *s, CBS *cbs, int *out_alert);
-int ssl_add_clienthello_renegotiate_ext(SSL *s, uint8_t *p, int *len,
- int maxlen);
-int ssl_parse_clienthello_renegotiate_ext(SSL *s, CBS *cbs, int *out_alert);
uint32_t ssl_get_algorithm2(SSL *s);
int tls1_process_sigalgs(SSL *s, const CBS *sigalgs);
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index b7b094f..61546ca 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -1268,9 +1268,13 @@
return 0;
}
- /* Add SCSVs. */
- if (!s->s3->initial_handshake_complete) {
+ /* For SSLv3, the SCSV is added. Otherwise the renegotiation extension is
+ * added. */
+ if (s->client_version == SSL3_VERSION &&
+ !s->s3->initial_handshake_complete) {
s2n(SSL3_CK_SCSV & 0xffff, p);
+ /* The renegotiation extension is required to be at index zero. */
+ s->s3->tmp.extensions.sent |= (1u << 0);
}
if (s->mode & SSL_MODE_SEND_FALLBACK_SCSV) {
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index c7090ae..654cf6a 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -976,9 +976,169 @@
}
+/* Renegotiation indication.
+ *
+ * https://tools.ietf.org/html/rfc5746 */
+
+static int ext_ri_add_clienthello(SSL *ssl, CBB *out) {
+ CBB contents, prev_finished;
+ if (!CBB_add_u16(out, TLSEXT_TYPE_renegotiate) ||
+ !CBB_add_u16_length_prefixed(out, &contents) ||
+ !CBB_add_u8_length_prefixed(&contents, &prev_finished) ||
+ !CBB_add_bytes(&prev_finished, ssl->s3->previous_client_finished,
+ ssl->s3->previous_client_finished_len) ||
+ !CBB_flush(out)) {
+ return 0;
+ }
+
+ return 1;
+}
+
+static int ext_ri_parse_serverhello(SSL *ssl, uint8_t *out_alert,
+ CBS *contents) {
+ if (contents == NULL) {
+ /* No renegotiation extension received.
+ *
+ * Strictly speaking if we want to avoid an attack we should *always* see
+ * RI even on initial ServerHello because the client doesn't see any
+ * renegotiation during an attack. However this would mean we could not
+ * connect to any server which doesn't support RI.
+ *
+ * A lack of the extension is allowed if SSL_OP_LEGACY_SERVER_CONNECT is
+ * defined. */
+ if (ssl->options & SSL_OP_LEGACY_SERVER_CONNECT) {
+ return 1;
+ }
+
+ *out_alert = SSL_AD_HANDSHAKE_FAILURE;
+ OPENSSL_PUT_ERROR(SSL, ext_ri_parse_serverhello,
+ SSL_R_UNSAFE_LEGACY_RENEGOTIATION_DISABLED);
+ return 0;
+ }
+
+ const size_t expected_len = ssl->s3->previous_client_finished_len +
+ ssl->s3->previous_server_finished_len;
+
+ /* Check for logic errors */
+ assert(!expected_len || ssl->s3->previous_client_finished_len);
+ assert(!expected_len || ssl->s3->previous_server_finished_len);
+
+ /* Parse out the extension contents. */
+ CBS renegotiated_connection;
+ if (!CBS_get_u8_length_prefixed(contents, &renegotiated_connection) ||
+ CBS_len(contents) != 0) {
+ OPENSSL_PUT_ERROR(SSL, ext_ri_parse_serverhello,
+ SSL_R_RENEGOTIATION_ENCODING_ERR);
+ *out_alert = SSL_AD_ILLEGAL_PARAMETER;
+ return 0;
+ }
+
+ /* Check that the extension matches. */
+ if (CBS_len(&renegotiated_connection) != expected_len) {
+ OPENSSL_PUT_ERROR(SSL, ext_ri_parse_serverhello,
+ SSL_R_RENEGOTIATION_MISMATCH);
+ *out_alert = SSL_AD_HANDSHAKE_FAILURE;
+ return 0;
+ }
+
+ const uint8_t *d = CBS_data(&renegotiated_connection);
+ if (CRYPTO_memcmp(d, ssl->s3->previous_client_finished,
+ ssl->s3->previous_client_finished_len)) {
+ OPENSSL_PUT_ERROR(SSL, ext_ri_parse_serverhello,
+ SSL_R_RENEGOTIATION_MISMATCH);
+ *out_alert = SSL_AD_HANDSHAKE_FAILURE;
+ return 0;
+ }
+ d += ssl->s3->previous_client_finished_len;
+
+ if (CRYPTO_memcmp(d, ssl->s3->previous_server_finished,
+ ssl->s3->previous_server_finished_len)) {
+ OPENSSL_PUT_ERROR(SSL, ext_ri_parse_serverhello,
+ SSL_R_RENEGOTIATION_MISMATCH);
+ *out_alert = SSL_AD_ILLEGAL_PARAMETER;
+ return 0;
+ }
+ ssl->s3->send_connection_binding = 1;
+
+ return 1;
+}
+
+static int ext_ri_parse_clienthello(SSL *ssl, uint8_t *out_alert,
+ CBS *contents) {
+ /* Renegotiation isn't supported as a server so this function should never be
+ * called after the initial handshake. */
+ assert(!ssl->s3->initial_handshake_complete);
+
+ CBS fake_contents;
+ static const uint8_t kFakeExtension[] = {0};
+
+ if (contents == NULL) {
+ if (ssl->s3->send_connection_binding) {
+ /* The renegotiation SCSV was received so pretend that we received a
+ * renegotiation extension. */
+ CBS_init(&fake_contents, kFakeExtension, sizeof(kFakeExtension));
+ contents = &fake_contents;
+ /* We require that the renegotiation extension is at index zero of
+ * kExtensions. */
+ ssl->s3->tmp.extensions.received |= (1u << 0);
+ } else {
+ return 1;
+ }
+ }
+
+ CBS renegotiated_connection;
+
+ if (!CBS_get_u8_length_prefixed(contents, &renegotiated_connection) ||
+ CBS_len(contents) != 0) {
+ OPENSSL_PUT_ERROR(SSL, ext_ri_parse_clienthello,
+ SSL_R_RENEGOTIATION_ENCODING_ERR);
+ return 0;
+ }
+
+ /* Check that the extension matches */
+ if (!CBS_mem_equal(&renegotiated_connection, ssl->s3->previous_client_finished,
+ ssl->s3->previous_client_finished_len)) {
+ OPENSSL_PUT_ERROR(SSL, ext_ri_parse_clienthello,
+ SSL_R_RENEGOTIATION_MISMATCH);
+ *out_alert = SSL_AD_HANDSHAKE_FAILURE;
+ return 0;
+ }
+
+ ssl->s3->send_connection_binding = 1;
+
+ return 1;
+}
+
+static int ext_ri_add_serverhello(SSL *ssl, CBB *out) {
+ CBB contents, prev_finished;
+ if (!CBB_add_u16(out, TLSEXT_TYPE_renegotiate) ||
+ !CBB_add_u16_length_prefixed(out, &contents) ||
+ !CBB_add_u8_length_prefixed(&contents, &prev_finished) ||
+ !CBB_add_bytes(&prev_finished, ssl->s3->previous_client_finished,
+ ssl->s3->previous_client_finished_len) ||
+ !CBB_add_bytes(&prev_finished, ssl->s3->previous_server_finished,
+ ssl->s3->previous_server_finished_len) ||
+ !CBB_flush(out)) {
+ return 0;
+ }
+
+ return 1;
+}
+
/* kExtensions contains all the supported extensions. */
static const struct tls_extension kExtensions[] = {
{
+ /* The renegotiation extension must always be at index zero because the
+ * |received| and |sent| bitsets need to be tweaked when the "extension" is
+ * sent as an SCSV. */
+ TLSEXT_TYPE_renegotiate,
+ NULL,
+ ext_ri_add_clienthello,
+ ext_ri_parse_serverhello,
+ ext_ri_parse_clienthello,
+ ext_ri_add_serverhello,
+ },
+ {
TLSEXT_TYPE_server_name,
ext_sni_init,
ext_sni_add_clienthello,
@@ -1082,30 +1242,6 @@
ret = limit - CBB_len(&cbb);
CBB_cleanup(&cbb);
- /* Add RI if renegotiating */
- if (s->s3->initial_handshake_complete) {
- int el;
-
- if (!ssl_add_clienthello_renegotiate_ext(s, 0, &el, 0)) {
- OPENSSL_PUT_ERROR(SSL, ssl_add_clienthello_tlsext, ERR_R_INTERNAL_ERROR);
- return NULL;
- }
-
- if ((limit - ret - 4 - el) < 0) {
- return NULL;
- }
-
- s2n(TLSEXT_TYPE_renegotiate, ret);
- s2n(el, ret);
-
- if (!ssl_add_clienthello_renegotiate_ext(s, ret, &el, el)) {
- OPENSSL_PUT_ERROR(SSL, ssl_add_clienthello_tlsext, ERR_R_INTERNAL_ERROR);
- return NULL;
- }
-
- ret += el;
- }
-
/* Add extended master secret. */
if (s->version != SSL3_VERSION) {
if (limit - ret - 4 < 0) {
@@ -1382,29 +1518,6 @@
ret = limit - CBB_len(&cbb);
CBB_cleanup(&cbb);
- if (s->s3->send_connection_binding) {
- int el;
-
- if (!ssl_add_serverhello_renegotiate_ext(s, 0, &el, 0)) {
- OPENSSL_PUT_ERROR(SSL, ssl_add_serverhello_tlsext, ERR_R_INTERNAL_ERROR);
- return NULL;
- }
-
- if ((limit - ret - 4 - el) < 0) {
- return NULL;
- }
-
- s2n(TLSEXT_TYPE_renegotiate, ret);
- s2n(el, ret);
-
- if (!ssl_add_serverhello_renegotiate_ext(s, ret, &el, el)) {
- OPENSSL_PUT_ERROR(SSL, ssl_add_serverhello_tlsext, ERR_R_INTERNAL_ERROR);
- return NULL;
- }
-
- ret += el;
- }
-
if (s->s3->tmp.extended_master_secret) {
if ((long)(limit - ret - 4) < 0) {
return NULL;
@@ -1589,7 +1702,6 @@
}
static int ssl_scan_clienthello_tlsext(SSL *s, CBS *cbs, int *out_alert) {
- int renegotiate_seen = 0;
CBS extensions;
s->srtp_profile = NULL;
@@ -1627,6 +1739,10 @@
}
s->s3->tmp.extensions.received = 0;
+ /* The renegotiation extension must always be at index zero because the
+ * |received| and |sent| bitsets need to be tweaked when the "extension" is
+ * sent as an SCSV. */
+ assert(kExtensions[0].value == TLSEXT_TYPE_renegotiate);
/* There may be no extensions. */
if (CBS_len(cbs) == 0) {
@@ -1718,11 +1834,6 @@
}
s->s3->tmp.peer_ellipticcurvelist_length = num_curves;
- } else if (type == TLSEXT_TYPE_renegotiate) {
- if (!ssl_parse_clienthello_renegotiate_ext(s, &extension, out_alert)) {
- return 0;
- }
- renegotiate_seen = 1;
} else if (type == TLSEXT_TYPE_signature_algorithms) {
CBS supported_signature_algorithms;
@@ -1815,16 +1926,6 @@
}
}
- /* Need RI if renegotiating */
-
- if (!renegotiate_seen && s->s3->initial_handshake_complete &&
- !(s->options & SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION)) {
- *out_alert = SSL_AD_HANDSHAKE_FAILURE;
- OPENSSL_PUT_ERROR(SSL, ssl_scan_clienthello_tlsext,
- SSL_R_UNSAFE_LEGACY_RENEGOTIATION_DISABLED);
- return 0;
- }
-
return 1;
}
@@ -1861,7 +1962,6 @@
}
static int ssl_scan_serverhello_tlsext(SSL *s, CBS *cbs, int *out_alert) {
- int renegotiate_seen = 0;
CBS extensions;
/* TODO(davidben): Move all of these to some per-handshake state that gets
@@ -2058,12 +2158,6 @@
*out_alert = SSL_AD_INTERNAL_ERROR;
return 0;
}
- } else if (type == TLSEXT_TYPE_renegotiate) {
- if (!ssl_parse_serverhello_renegotiate_ext(s, &extension, out_alert)) {
- return 0;
- }
-
- renegotiate_seen = 1;
} else if (type == TLSEXT_TYPE_use_srtp) {
if (!ssl_parse_serverhello_use_srtp_ext(s, &extension, out_alert)) {
return 0;
@@ -2093,19 +2187,6 @@
}
}
- /* Determine if we need to see RI. Strictly speaking if we want to avoid an
- * attack we should *always* see RI even on initial server hello because the
- * client doesn't see any renegotiation during an attack. However this would
- * mean we could not connect to any server which doesn't support RI so for
- * the immediate future tolerate RI absence on initial connect only. */
- if (!renegotiate_seen && !(s->options & SSL_OP_LEGACY_SERVER_CONNECT) &&
- !(s->options & SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION)) {
- *out_alert = SSL_AD_HANDSHAKE_FAILURE;
- OPENSSL_PUT_ERROR(SSL, ssl_scan_serverhello_tlsext,
- SSL_R_UNSAFE_LEGACY_RENEGOTIATION_DISABLED);
- return 0;
- }
-
return 1;
}
diff --git a/ssl/t1_reneg.c b/ssl/t1_reneg.c
deleted file mode 100644
index d0009c1..0000000
--- a/ssl/t1_reneg.c
+++ /dev/null
@@ -1,246 +0,0 @@
-/* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
- * All rights reserved.
- *
- * This package is an SSL implementation written
- * by Eric Young (eay@cryptsoft.com).
- * The implementation was written so as to conform with Netscapes SSL.
- *
- * This library is free for commercial and non-commercial use as long as
- * the following conditions are aheared to. The following conditions
- * apply to all code found in this distribution, be it the RC4, RSA,
- * lhash, DES, etc., code; not just the SSL code. The SSL documentation
- * included with this distribution is covered by the same copyright terms
- * except that the holder is Tim Hudson (tjh@cryptsoft.com).
- *
- * Copyright remains Eric Young's, and as such any Copyright notices in
- * the code are not to be removed.
- * If this package is used in a product, Eric Young should be given attribution
- * as the author of the parts of the library used.
- * This can be in the form of a textual message at program startup or
- * in documentation (online or textual) provided with the package.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the copyright
- * notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution.
- * 3. All advertising materials mentioning features or use of this software
- * must display the following acknowledgement:
- * "This product includes cryptographic software written by
- * Eric Young (eay@cryptsoft.com)"
- * The word 'cryptographic' can be left out if the rouines from the library
- * being used are not cryptographic related :-).
- * 4. If you include any Windows specific code (or a derivative thereof) from
- * the apps directory (application code) you must include an acknowledgement:
- * "This product includes software written by Tim Hudson (tjh@cryptsoft.com)"
- *
- * THIS SOFTWARE IS PROVIDED BY ERIC YOUNG ``AS IS'' AND
- * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
- * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
- * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
- * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
- * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
- * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
- * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
- * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
- * SUCH DAMAGE.
- *
- * The licence and distribution terms for any publically available version or
- * derivative of this code cannot be changed. i.e. this code cannot simply be
- * copied and put under another distribution licence
- * [including the GNU Public Licence.]
- */
-/* ====================================================================
- * Copyright (c) 1998-2009 The OpenSSL Project. All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- *
- * 1. Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- *
- * 2. Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in
- * the documentation and/or other materials provided with the
- * distribution.
- *
- * 3. All advertising materials mentioning features or use of this
- * software must display the following acknowledgment:
- * "This product includes software developed by the OpenSSL Project
- * for use in the OpenSSL Toolkit. (http://www.openssl.org/)"
- *
- * 4. The names "OpenSSL Toolkit" and "OpenSSL Project" must not be used to
- * endorse or promote products derived from this software without
- * prior written permission. For written permission, please contact
- * openssl-core@openssl.org.
- *
- * 5. Products derived from this software may not be called "OpenSSL"
- * nor may "OpenSSL" appear in their names without prior written
- * permission of the OpenSSL Project.
- *
- * 6. Redistributions of any form whatsoever must retain the following
- * acknowledgment:
- * "This product includes software developed by the OpenSSL Project
- * for use in the OpenSSL Toolkit (http://www.openssl.org/)"
- *
- * THIS SOFTWARE IS PROVIDED BY THE OpenSSL PROJECT ``AS IS'' AND ANY
- * EXPRESSED OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
- * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE OpenSSL PROJECT OR
- * ITS CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
- * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
- * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
- * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
- * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
- * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
- * OF THE POSSIBILITY OF SUCH DAMAGE.
- * ====================================================================
- *
- * This product includes cryptographic software written by Eric Young
- * (eay@cryptsoft.com). This product includes software written by Tim
- * Hudson (tjh@cryptsoft.com). */
-
-#include <assert.h>
-#include <stdio.h>
-#include <string.h>
-
-#include <openssl/bytestring.h>
-#include <openssl/err.h>
-
-#include "internal.h"
-
-
-/* Add the client's renegotiation binding */
-int ssl_add_clienthello_renegotiate_ext(SSL *s, unsigned char *p, int *len,
- int maxlen) {
- if (p) {
- if (s->s3->previous_client_finished_len + 1 > maxlen) {
- OPENSSL_PUT_ERROR(SSL, ssl_add_clienthello_renegotiate_ext,
- SSL_R_RENEGOTIATE_EXT_TOO_LONG);
- return 0;
- }
-
- /* Length byte */
- *p = s->s3->previous_client_finished_len;
- p++;
-
- memcpy(p, s->s3->previous_client_finished,
- s->s3->previous_client_finished_len);
- }
-
- *len = s->s3->previous_client_finished_len + 1;
-
- return 1;
-}
-
-/* Parse the client's renegotiation binding and abort if it's not right */
-int ssl_parse_clienthello_renegotiate_ext(SSL *s, CBS *cbs, int *out_alert) {
- CBS renegotiated_connection;
-
- if (!CBS_get_u8_length_prefixed(cbs, &renegotiated_connection) ||
- CBS_len(cbs) != 0) {
- OPENSSL_PUT_ERROR(SSL, ssl_parse_clienthello_renegotiate_ext,
- SSL_R_RENEGOTIATION_ENCODING_ERR);
- *out_alert = SSL_AD_DECODE_ERROR;
- return 0;
- }
-
- /* Check that the extension matches */
- if (!CBS_mem_equal(&renegotiated_connection, s->s3->previous_client_finished,
- s->s3->previous_client_finished_len)) {
- OPENSSL_PUT_ERROR(SSL, ssl_parse_clienthello_renegotiate_ext,
- SSL_R_RENEGOTIATION_MISMATCH);
- *out_alert = SSL_AD_HANDSHAKE_FAILURE;
- return 0;
- }
-
- s->s3->send_connection_binding = 1;
-
- return 1;
-}
-
-/* Add the server's renegotiation binding */
-int ssl_add_serverhello_renegotiate_ext(SSL *s, unsigned char *p, int *len,
- int maxlen) {
- if (p) {
- if (s->s3->previous_client_finished_len +
- s->s3->previous_server_finished_len + 1 > maxlen) {
- OPENSSL_PUT_ERROR(SSL, ssl_add_serverhello_renegotiate_ext,
- SSL_R_RENEGOTIATE_EXT_TOO_LONG);
- return 0;
- }
-
- /* Length byte */
- *p = s->s3->previous_client_finished_len +
- s->s3->previous_server_finished_len;
- p++;
-
- memcpy(p, s->s3->previous_client_finished,
- s->s3->previous_client_finished_len);
- p += s->s3->previous_client_finished_len;
-
- memcpy(p, s->s3->previous_server_finished,
- s->s3->previous_server_finished_len);
- }
-
- *len = s->s3->previous_client_finished_len +
- s->s3->previous_server_finished_len + 1;
-
- return 1;
-}
-
-/* Parse the server's renegotiation binding and abort if it's not right */
-int ssl_parse_serverhello_renegotiate_ext(SSL *s, CBS *cbs, int *out_alert) {
- int expected_len =
- s->s3->previous_client_finished_len + s->s3->previous_server_finished_len;
- CBS renegotiated_connection;
- const uint8_t *d;
-
- /* Check for logic errors */
- assert(!expected_len || s->s3->previous_client_finished_len);
- assert(!expected_len || s->s3->previous_server_finished_len);
-
- /* Parse out the extension contents. */
- if (!CBS_get_u8_length_prefixed(cbs, &renegotiated_connection) ||
- CBS_len(cbs) != 0) {
- OPENSSL_PUT_ERROR(SSL, ssl_parse_serverhello_renegotiate_ext,
- SSL_R_RENEGOTIATION_ENCODING_ERR);
- *out_alert = SSL_AD_ILLEGAL_PARAMETER;
- return 0;
- }
-
- /* Check that the extension matches. */
- if (CBS_len(&renegotiated_connection) != expected_len) {
- OPENSSL_PUT_ERROR(SSL, ssl_parse_serverhello_renegotiate_ext,
- SSL_R_RENEGOTIATION_MISMATCH);
- *out_alert = SSL_AD_HANDSHAKE_FAILURE;
- return 0;
- }
-
- d = CBS_data(&renegotiated_connection);
- if (memcmp(d, s->s3->previous_client_finished,
- s->s3->previous_client_finished_len)) {
- OPENSSL_PUT_ERROR(SSL, ssl_parse_serverhello_renegotiate_ext,
- SSL_R_RENEGOTIATION_MISMATCH);
- *out_alert = SSL_AD_HANDSHAKE_FAILURE;
- return 0;
- }
- d += s->s3->previous_client_finished_len;
-
- if (memcmp(d, s->s3->previous_server_finished,
- s->s3->previous_server_finished_len)) {
- OPENSSL_PUT_ERROR(SSL, ssl_parse_serverhello_renegotiate_ext,
- SSL_R_RENEGOTIATION_MISMATCH);
- *out_alert = SSL_AD_ILLEGAL_PARAMETER;
- return 0;
- }
- s->s3->send_connection_binding = 1;
-
- return 1;
-}
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index f324d39..b8b20b9 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -875,9 +875,6 @@
if (config->tls_d5_bug) {
SSL_set_options(ssl.get(), SSL_OP_TLS_D5_BUG);
}
- if (config->allow_unsafe_legacy_renegotiation) {
- SSL_set_options(ssl.get(), SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION);
- }
if (config->no_legacy_server_connect) {
SSL_clear_options(ssl.get(), SSL_OP_LEGACY_SERVER_CONNECT);
}
diff --git a/ssl/test/runner/cipher_suites.go b/ssl/test/runner/cipher_suites.go
index 70c7262..98c833b 100644
--- a/ssl/test/runner/cipher_suites.go
+++ b/ssl/test/runner/cipher_suites.go
@@ -406,6 +406,7 @@
TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 uint16 = 0xc030
TLS_ECDHE_PSK_WITH_AES_128_CBC_SHA uint16 = 0xc035
TLS_ECDHE_PSK_WITH_AES_256_CBC_SHA uint16 = 0xc036
+ renegotiationSCSV uint16 = 0x00ff
fallbackSCSV uint16 = 0x5600
)
diff --git a/ssl/test/runner/common.go b/ssl/test/runner/common.go
index c7ccf80..2e30208 100644
--- a/ssl/test/runner/common.go
+++ b/ssl/test/runner/common.go
@@ -476,6 +476,10 @@
// TLS_FALLBACK_SCSV in the ClientHello.
SendFallbackSCSV bool
+ // SendRenegotiationSCSV causes the client to include the renegotiation
+ // SCSV in the ClientHello.
+ SendRenegotiationSCSV bool
+
// MaxHandshakeRecordLength, if non-zero, is the maximum size of a
// handshake record. Handshake messages will be split into multiple
// records at the specified size, except that the client_version will
@@ -572,6 +576,10 @@
// didn't support the renegotiation info extension.
NoRenegotiationInfo bool
+ // RequireRenegotiationInfo, if true, causes the client to return an
+ // error if the server doesn't reply with the renegotiation extension.
+ RequireRenegotiationInfo bool
+
// SequenceNumberIncrement, if non-zero, causes outgoing sequence
// numbers in DTLS to increment by that value rather by 1. This is to
// stress the replay bitmap window by simulating extreme packet loss and
diff --git a/ssl/test/runner/handshake_client.go b/ssl/test/runner/handshake_client.go
index 00bff0e..951b956 100644
--- a/ssl/test/runner/handshake_client.go
+++ b/ssl/test/runner/handshake_client.go
@@ -123,6 +123,10 @@
}
}
+ if c.config.Bugs.SendRenegotiationSCSV {
+ hello.cipherSuites = append(hello.cipherSuites, renegotiationSCSV)
+ }
+
if c.config.Bugs.SendFallbackSCSV {
hello.cipherSuites = append(hello.cipherSuites, fallbackSCSV)
}
@@ -272,6 +276,10 @@
return fmt.Errorf("tls: server selected an unsupported cipher suite")
}
+ if c.config.Bugs.RequireRenegotiationInfo && serverHello.secureRenegotiation == nil {
+ return errors.New("tls: renegotiation extension missing")
+ }
+
if len(c.clientVerify) > 0 && !c.config.Bugs.NoRenegotiationInfo {
var expectedRenegInfo []byte
expectedRenegInfo = append(expectedRenegInfo, c.clientVerify...)
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 259d43f..6d04354 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -3082,7 +3082,33 @@
expectedError: ":NO_RENEGOTIATION:",
expectedLocalError: "remote error: no renegotiation",
})
- // TODO(agl): test the renegotiation info SCSV.
+ // The server shouldn't echo the renegotiation extension unless
+ // requested by the client.
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: "Renegotiate-Server-NoExt",
+ config: Config{
+ Bugs: ProtocolBugs{
+ NoRenegotiationInfo: true,
+ RequireRenegotiationInfo: true,
+ },
+ },
+ shouldFail: true,
+ expectedLocalError: "renegotiation extension missing",
+ })
+ // The renegotiation SCSV should be sufficient for the server to echo
+ // the extension.
+ testCases = append(testCases, testCase{
+ testType: serverTest,
+ name: "Renegotiate-Server-NoExt-SCSV",
+ config: Config{
+ Bugs: ProtocolBugs{
+ NoRenegotiationInfo: true,
+ SendRenegotiationSCSV: true,
+ RequireRenegotiationInfo: true,
+ },
+ },
+ })
testCases = append(testCases, testCase{
name: "Renegotiate-Client",
config: Config{
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc
index b4da5ce..6b831f0 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -65,8 +65,6 @@
{ "-expect-session-miss", &TestConfig::expect_session_miss },
{ "-expect-extended-master-secret",
&TestConfig::expect_extended_master_secret },
- { "-allow-unsafe-legacy-renegotiation",
- &TestConfig::allow_unsafe_legacy_renegotiation },
{ "-enable-ocsp-stapling", &TestConfig::enable_ocsp_stapling },
{ "-enable-signed-cert-timestamps",
&TestConfig::enable_signed_cert_timestamps },
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h
index 6e0b2e4..b05db16 100644
--- a/ssl/test/test_config.h
+++ b/ssl/test/test_config.h
@@ -54,7 +54,6 @@
bool expect_extended_master_secret = false;
std::string psk;
std::string psk_identity;
- bool allow_unsafe_legacy_renegotiation = false;
std::string srtp_profiles;
bool enable_ocsp_stapling = false;
std::string expected_ocsp_response;
