Tighten SSL_OP_LEGACY_SERVER_CONNECT to align with RFC 5746.
RFC 5746 forbids a server from downgrading or upgrading
renegotiation_info support. Even with SSL_OP_LEGACY_SERVER_CONNECT set
(the default), we can still enforce a few things.
I do not believe this has practical consequences. The attack variant
where the server half is prefixed does not involve a renegotiation on
the client. The converse where the client sees the renegotiation and
prefix does, but we only support renego for the mid-stream HTTP/1.1
client auth hack, which doesn't do this. (And with triple-handshake,
HTTPS clients should be requiring the certificate be unchanged across
renego which makes this moot.)
Ultimately, an application which makes the mistake of using
renegotiation needs to be aware of what exactly that means and how to
handle connection state changing mid-stream. We make renego opt-in now,
so this is a tenable requirement.
(Also the legacy -> secure direction would have been caught by the
server anyway since we send a non-empty RI extension.)
Change-Id: I915965c342f8a9cf3a4b6b32f0a87a00c3df3559
Reviewed-on: https://boringssl-review.googlesource.com/6559
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index c7e52a9..69d411b 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -929,6 +929,15 @@
static int ext_ri_parse_serverhello(SSL *ssl, uint8_t *out_alert,
CBS *contents) {
+ /* Servers may not switch between omitting the extension and supporting it.
+ * See RFC 5746, sections 3.5 and 4.2. */
+ if (ssl->s3->initial_handshake_complete &&
+ (contents != NULL) != ssl->s3->send_connection_binding) {
+ *out_alert = SSL_AD_HANDSHAKE_FAILURE;
+ OPENSSL_PUT_ERROR(SSL, SSL_R_RENEGOTIATION_MISMATCH);
+ return 0;
+ }
+
if (contents == NULL) {
/* No renegotiation extension received.
*
