Fix #2400 Add NO_RENEGOTIATE option

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3432)

15 files changed, 238 insertions, 9 deletions

diff --git a/apps/apps.h b/apps/apps.h
index 3c1da48ff8..31cf7b0961 100644
--- a/apps/apps.h
+++ b/apps/apps.h
@@ -215,7 +215,7 @@ int set_cert_times(X509 *x, const char *startdate, const char *enddate,
         OPT_S_ONRESUMP, OPT_S_NOLEGACYCONN, OPT_S_STRICT, OPT_S_SIGALGS, \
         OPT_S_CLIENTSIGALGS, OPT_S_GROUPS, OPT_S_CURVES, OPT_S_NAMEDCURVE, \
         OPT_S_CIPHER, OPT_S_DHPARAM, OPT_S_RECORD_PADDING, OPT_S_DEBUGBROKE, \
-        OPT_S_COMP, OPT_S__LAST
+        OPT_S_COMP, OPT_S_NO_RENEGOTIATION, OPT_S__LAST
 
 # define OPT_S_OPTIONS \
         {"no_ssl3", OPT_S_NOSSL3, '-',"Just disable SSLv3" }, \
@@ -231,6 +231,8 @@ int set_cert_times(X509 *x, const char *startdate, const char *enddate,
         {"serverpref", OPT_S_SERVERPREF, '-', "Use server's cipher preferences"}, \
         {"legacy_renegotiation", OPT_S_LEGACYRENEG, '-', \
             "Enable use of legacy renegotiation (dangerous)"}, \
+        {"no_renegotiation", OPT_S_NO_RENEGOTIATION, '-', \
+            "Disable all renegotiation."}, \
         {"legacy_server_connect", OPT_S_LEGACYCONN, '-', \
             "Allow initial connection to servers that don't support RI"}, \
         {"no_resumption_on_reneg", OPT_S_ONRESUMP, '-', \
@@ -284,6 +286,7 @@ int set_cert_times(X509 *x, const char *startdate, const char *enddate,
         case OPT_S_CIPHER: \
         case OPT_S_DHPARAM: \
         case OPT_S_RECORD_PADDING: \
+        case OPT_S_NO_RENEGOTIATION: \
         case OPT_S_DEBUGBROKE
 
 #define IS_NO_PROT_FLAG(o) \
diff --git a/apps/s_server.c b/apps/s_server.c
index 82fe5a4b74..d678e8e257 100644
--- a/apps/s_server.c
+++ b/apps/s_server.c
@@ -2785,6 +2785,9 @@ static void print_connection_info(SSL *con)
         BIO_printf(bio_s_out, "Reused session-id\n");
     BIO_printf(bio_s_out, "Secure Renegotiation IS%s supported\n",
                SSL_get_secure_renegotiation_support(con) ? "" : " NOT");
+    if ((SSL_get_options(con) & SSL_OP_NO_RENEGOTIATION))
+        BIO_printf(bio_s_out, "Renegotiation is DISABLED\n");
+
     if (keymatexportlabel != NULL) {
         BIO_printf(bio_s_out, "Keying material exporter:\n");
         BIO_printf(bio_s_out, "    Label: '%s'\n", keymatexportlabel);
diff --git a/doc/man3/SSL_CONF_cmd.pod b/doc/man3/SSL_CONF_cmd.pod
index 18cc88f59b..7b751fdef3 100644
--- a/doc/man3/SSL_CONF_cmd.pod
+++ b/doc/man3/SSL_CONF_cmd.pod
@@ -125,6 +125,11 @@ Attempts to pad TLS 1.3 records so that they are a multiple of B<value> in
 length on send. A B<value> of 0 or 1 turns off padding. Otherwise, the
 B<value> must be >1 or <=16384.
 
+=item B<-no_renegotiation>
+
+Disables all attempts at renegotiation in TLSv1.2 and earlier, same as setting
+B<SSL_OP_NO_RENEGOTIATION>.
+
 =item B<-min_protocol>, B<-max_protocol>
 
 Sets the minimum and maximum supported protocol.
@@ -257,6 +262,11 @@ Attempts to pad TLS 1.3 records so that they are a multiple of B<value> in
 length on send. A B<value> of 0 or 1 turns off padding. Otherwise, the
 B<value> must be >1 or <=16384.
 
+=item B<NoRenegotiation>
+
+Disables all attempts at renegotiation in TLSv1.2 and earlier, same as setting
+B<SSL_OP_NO_RENEGOTIATION>.
+
 =item B<SignatureAlgorithms>
 
 This sets the supported signature algorithms for TLS v1.2. For clients this
diff --git a/doc/man3/SSL_CTX_set_options.pod b/doc/man3/SSL_CTX_set_options.pod
index d12a0399c0..5155a1f679 100644
--- a/doc/man3/SSL_CTX_set_options.pod
+++ b/doc/man3/SSL_CTX_set_options.pod
@@ -170,6 +170,11 @@ RFC7366 Encrypt-then-MAC option on TLS and DTLS connection.
 If this option is set, Encrypt-then-MAC is disabled. Clients will not
 propose, and servers will not accept the extension.
 
+=item SSL_OP_NO_RENEGOTIATION
+
+Disable all renegotiation in TLSv1.2 and earlier. Do not send HelloRequest
+messages, and ignore renegotiation requests via ClientHello.
+
 =back
 
 The following options no longer have any effect but their identifiers are
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 8eb3c5389f..3b0ac5021c 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -370,6 +370,9 @@ typedef int (*SSL_verify_cb)(int preverify_ok, X509_STORE_CTX *x509_ctx);
         SSL_OP_NO_TLSv1|SSL_OP_NO_TLSv1_1|SSL_OP_NO_TLSv1_2|SSL_OP_NO_TLSv1_3)
 # define SSL_OP_NO_DTLS_MASK (SSL_OP_NO_DTLSv1|SSL_OP_NO_DTLSv1_2)
 
+/* Disallow all renegotiation */
+# define SSL_OP_NO_RENEGOTIATION                         0x40000000U
+
 /*
  * Make server add server-hello extension from early version of cryptopro
  * draft, when GOST ciphersuite is negotiated. Required for interoperability
@@ -2386,6 +2389,7 @@ int ERR_load_SSL_strings(void);
 # define SSL_F_SSL_READ_EX                                434
 # define SSL_F_SSL_READ_INTERNAL                          523
 # define SSL_F_SSL_RENEGOTIATE                            516
+# define SSL_F_SSL_RENEGOTIATE_ABBREVIATED                546
 # define SSL_F_SSL_SCAN_CLIENTHELLO_TLSEXT                320
 # define SSL_F_SSL_SCAN_SERVERHELLO_TLSEXT                321
 # define SSL_F_SSL_SESSION_DUP                            348
diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c
index 01caf4c372..045a74c113 100644
--- a/ssl/record/rec_layer_s3.c
+++ b/ssl/record/rec_layer_s3.c
@@ -1433,13 +1433,15 @@ int ssl3_read_bytes(SSL *s, int type, int *recvd_type, unsigned char *buf,
      */
     if (s->server &&
         SSL_is_init_finished(s) &&
-        !s->s3->send_connection_binding &&
         (s->version > SSL3_VERSION) &&
         !SSL_IS_TLS13(s) &&
+        (SSL3_RECORD_get_type(rr) == SSL3_RT_HANDSHAKE) &&
         (s->rlayer.handshake_fragment_len >= 4) &&
         (s->rlayer.handshake_fragment[0] == SSL3_MT_CLIENT_HELLO) &&
         (s->session != NULL) && (s->session->cipher != NULL) &&
-        !(s->options & SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION)) {
+        ((!s->s3->send_connection_binding &&
+          !(s->options & SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION)) ||
+         (s->options & SSL_OP_NO_RENEGOTIATION))) {
         SSL3_RECORD_set_length(rr, 0);
         SSL3_RECORD_set_read(rr);
         ssl3_send_alert(s, SSL3_AL_WARNING, SSL_AD_NO_RENEGOTIATION);
diff --git a/ssl/ssl_conf.c b/ssl/ssl_conf.c
index 484bb61feb..41c7ff7d83 100644
--- a/ssl/ssl_conf.c
+++ b/ssl/ssl_conf.c
@@ -358,6 +358,7 @@ static int cmd_Options(SSL_CONF_CTX *cctx, const char *value)
         SSL_FLAG_TBL("UnsafeLegacyRenegotiation",
                      SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION),
         SSL_FLAG_TBL_INV("EncryptThenMac", SSL_OP_NO_ENCRYPT_THEN_MAC),
+        SSL_FLAG_TBL("NoRenegotiation", SSL_OP_NO_RENEGOTIATION),
     };
     if (value == NULL)
         return -3;
@@ -573,6 +574,7 @@ static const ssl_conf_cmd_tbl ssl_conf_cmds[] = {
     SSL_CONF_CMD_SWITCH("serverpref", SSL_CONF_FLAG_SERVER),
     SSL_CONF_CMD_SWITCH("legacy_renegotiation", 0),
     SSL_CONF_CMD_SWITCH("legacy_server_connect", SSL_CONF_FLAG_SERVER),
+    SSL_CONF_CMD_SWITCH("no_renegotiation", 0),
     SSL_CONF_CMD_SWITCH("no_resumption_on_reneg", SSL_CONF_FLAG_SERVER),
     SSL_CONF_CMD_SWITCH("no_legacy_server_connect", SSL_CONF_FLAG_SERVER),
     SSL_CONF_CMD_SWITCH("strict", 0),
@@ -639,6 +641,8 @@ static const ssl_switch_tbl ssl_cmd_switches[] = {
     {SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION, 0},
     /* legacy_server_connect */
     {SSL_OP_LEGACY_SERVER_CONNECT, 0},
+    /* no_renegotiation */
+    {SSL_OP_NO_RENEGOTIATION, 0},
     /* no_resumption_on_reneg */
     {SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION, 0},
     /* no_legacy_server_connect */
diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c
index e334b00370..7152fa2701 100644
--- a/ssl/ssl_err.c
+++ b/ssl/ssl_err.c
@@ -216,6 +216,8 @@ static ERR_STRING_DATA SSL_str_functs[] = {
     {ERR_FUNC(SSL_F_SSL_READ_EX), "SSL_read_ex"},
     {ERR_FUNC(SSL_F_SSL_READ_INTERNAL), "ssl_read_internal"},
     {ERR_FUNC(SSL_F_SSL_RENEGOTIATE), "SSL_renegotiate"},
+    {ERR_FUNC(SSL_F_SSL_RENEGOTIATE_ABBREVIATED),
+     "SSL_renegotiate_abbreviated"},
     {ERR_FUNC(SSL_F_SSL_SCAN_CLIENTHELLO_TLSEXT),
      "ssl_scan_clienthello_tlsext"},
     {ERR_FUNC(SSL_F_SSL_SCAN_SERVERHELLO_TLSEXT),
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 028b69da08..e90c4b8732 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -1922,9 +1922,12 @@ int SSL_renegotiate(SSL *s)
         return 0;
     }
 
-    if (s->renegotiate == 0)
-        s->renegotiate = 1;
+    if ((s->options & SSL_OP_NO_RENEGOTIATION)) {
+        SSLerr(SSL_F_SSL_RENEGOTIATE, SSL_R_NO_RENEGOTIATION);
+        return 0;
+    }
 
+    s->renegotiate = 1;
     s->new_session = 1;
 
     return (s->method->ssl_renegotiate(s));
@@ -1932,12 +1935,17 @@ int SSL_renegotiate(SSL *s)
 
 int SSL_renegotiate_abbreviated(SSL *s)
 {
-    if (SSL_IS_TLS13(s))
+    if (SSL_IS_TLS13(s)) {
+        SSLerr(SSL_F_SSL_RENEGOTIATE_ABBREVIATED, SSL_R_WRONG_SSL_VERSION);
         return 0;
+    }
 
-    if (s->renegotiate == 0)
-        s->renegotiate = 1;
+    if ((s->options & SSL_OP_NO_RENEGOTIATION)) {
+        SSLerr(SSL_F_SSL_RENEGOTIATE_ABBREVIATED, SSL_R_NO_RENEGOTIATION);
+        return 0;
+    }
 
+    s->renegotiate = 1;
     s->new_session = 0;
 
     return (s->method->ssl_renegotiate(s));
diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c
index 95bacdffc4..fe181abad0 100644
--- a/ssl/statem/extensions_srvr.c
+++ b/ssl/statem/extensions_srvr.c
@@ -817,6 +817,7 @@ EXT_RETURN tls_construct_stoc_renegotiate(SSL *s, WPACKET *pkt,
     if (!s->s3->send_connection_binding)
         return EXT_RETURN_NOT_SENT;
 
+    /* Still add this even if SSL_OP_NO_RENEGOTIATION is set */
     if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_renegotiate)
             || !WPACKET_start_sub_packet_u16(pkt)
             || !WPACKET_start_sub_packet_u8(pkt)
diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c
index d4382e89cc..020589f1b1 100644
--- a/ssl/statem/statem_clnt.c
+++ b/ssl/statem/statem_clnt.c
@@ -3454,6 +3454,11 @@ MSG_PROCESS_RETURN tls_process_hello_req(SSL *s, PACKET *pkt)
         return MSG_PROCESS_ERROR;
     }
 
+    if ((s->options & SSL_OP_NO_RENEGOTIATION)) {
+        ssl3_send_alert(s, SSL3_AL_WARNING, SSL_AD_NO_RENEGOTIATION);
+        return MSG_PROCESS_FINISHED_READING;
+    }
+
     /*
      * This is a historical discrepancy (not in the RFC) maintained for
      * compatibility reasons. If a TLS client receives a HelloRequest it will
diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index c2b14853c2..29fb9dcf1f 100644
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -116,6 +116,10 @@ int tls_setup_handshake(SSL *s)
         }
         if (SSL_IS_FIRST_HANDSHAKE(s)) {
             s->ctx->stats.sess_accept++;
+        } else if ((s->options & SSL_OP_NO_RENEGOTIATION)) {
+            /* Renegotiation is disabled */
+            ssl3_send_alert(s, SSL3_AL_WARNING, SSL_AD_NO_RENEGOTIATION);
+            return 0;
         } else if (!s->s3->send_connection_binding &&
                    !(s->options &
                      SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION)) {
diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index 02c6e569d8..c26c93bdc9 100644
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -1246,6 +1246,10 @@ MSG_PROCESS_RETURN tls_process_client_hello(SSL *s, PACKET *pkt)
     }
     /* Check if this is actually an unexpected renegotiation ClientHello */
     if (s->renegotiate == 0 && !SSL_IS_FIRST_HANDSHAKE(s)) {
+        if ((s->options & SSL_OP_NO_RENEGOTIATION)) {
+            ssl3_send_alert(s, SSL3_AL_WARNING, SSL_AD_NO_RENEGOTIATION);
+            goto err;
+        }
         s->renegotiate = 1;
         s->new_session = 1;
     }
diff --git a/test/ssl-tests/17-renegotiate.conf b/test/ssl-tests/17-renegotiate.conf
index 3f3769ff02..12cf791310 100644
--- a/test/ssl-tests/17-renegotiate.conf
+++ b/test/ssl-tests/17-renegotiate.conf
@@ -1,6 +1,6 @@
 # Generated with generate_ssl_tests.pl
 
-num_tests = 10
+num_tests = 14
 
 test-0 = 0-renegotiate-client-no-resume
 test-1 = 1-renegotiate-client-resume
@@ -12,6 +12,10 @@ test-6 = 6-renegotiate-aead-to-non-aead
 test-7 = 7-renegotiate-non-aead-to-aead
 test-8 = 8-renegotiate-non-aead-to-non-aead
 test-9 = 9-renegotiate-aead-to-aead
+test-10 = 10-no-renegotiation-server-by-client
+test-11 = 11-no-renegotiation-server-by-server
+test-12 = 12-no-renegotiation-client-by-server
+test-13 = 13-no-renegotiation-client-by-client
 # ===========================================================
 
 [0-renegotiate-client-no-resume]
@@ -314,3 +318,115 @@ client = 9-renegotiate-aead-to-aead-client-extra
 RenegotiateCiphers = AES256-GCM-SHA384
 
 
+# ===========================================================
+
+[10-no-renegotiation-server-by-client]
+ssl_conf = 10-no-renegotiation-server-by-client-ssl
+
+[10-no-renegotiation-server-by-client-ssl]
+server = 10-no-renegotiation-server-by-client-server
+client = 10-no-renegotiation-server-by-client-client
+
+[10-no-renegotiation-server-by-client-server]
+Certificate = ${ENV::TEST_CERTS_DIR}/servercert.pem
+CipherString = DEFAULT
+MaxProtocol = TLSv1.2
+Options = NoRenegotiation
+PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
+
+[10-no-renegotiation-server-by-client-client]
+CipherString = DEFAULT
+VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
+VerifyMode = Peer
+
+[test-10]
+ExpectedResult = ClientFail
+HandshakeMode = RenegotiateClient
+Method = TLS
+ResumptionExpected = No
+
+
+# ===========================================================
+
+[11-no-renegotiation-server-by-server]
+ssl_conf = 11-no-renegotiation-server-by-server-ssl
+
+[11-no-renegotiation-server-by-server-ssl]
+server = 11-no-renegotiation-server-by-server-server
+client = 11-no-renegotiation-server-by-server-client
+
+[11-no-renegotiation-server-by-server-server]
+Certificate = ${ENV::TEST_CERTS_DIR}/servercert.pem
+CipherString = DEFAULT
+MaxProtocol = TLSv1.2
+Options = NoRenegotiation
+PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
+
+[11-no-renegotiation-server-by-server-client]
+CipherString = DEFAULT
+VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
+VerifyMode = Peer
+
+[test-11]
+ExpectedResult = ServerFail
+HandshakeMode = RenegotiateServer
+Method = TLS
+ResumptionExpected = No
+
+
+# ===========================================================
+
+[12-no-renegotiation-client-by-server]
+ssl_conf = 12-no-renegotiation-client-by-server-ssl
+
+[12-no-renegotiation-client-by-server-ssl]
+server = 12-no-renegotiation-client-by-server-server
+client = 12-no-renegotiation-client-by-server-client
+
+[12-no-renegotiation-client-by-server-server]
+Certificate = ${ENV::TEST_CERTS_DIR}/servercert.pem
+CipherString = DEFAULT
+MaxProtocol = TLSv1.2
+PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
+
+[12-no-renegotiation-client-by-server-client]
+CipherString = DEFAULT
+Options = NoRenegotiation
+VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
+VerifyMode = Peer
+
+[test-12]
+ExpectedResult = ServerFail
+HandshakeMode = RenegotiateServer
+Method = TLS
+ResumptionExpected = No
+
+
+# ===========================================================
+
+[13-no-renegotiation-client-by-client]
+ssl_conf = 13-no-renegotiation-client-by-client-ssl
+
+[13-no-renegotiation-client-by-client-ssl]
+server = 13-no-renegotiation-client-by-client-server
+client = 13-no-renegotiation-client-by-client-client
+
+[13-no-renegotiation-client-by-client-server]
+Certificate = ${ENV::TEST_CERTS_DIR}/servercert.pem
+CipherString = DEFAULT
+MaxProtocol = TLSv1.2
+PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
+
+[13-no-renegotiation-client-by-client-client]
+CipherString = DEFAULT
+Options = NoRenegotiation
+VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
+VerifyMode = Peer
+
+[test-13]
+ExpectedResult = ClientFail
+HandshakeMode = RenegotiateClient
+Method = TLS
+ResumptionExpected = No
+
+
diff --git a/test/ssl-tests/17-renegotiate.conf.in b/test/ssl-tests/17-renegotiate.conf.in
index b5d07b0705..35175dce51 100644
--- a/test/ssl-tests/17-renegotiate.conf.in
+++ b/test/ssl-tests/17-renegotiate.conf.in
@@ -185,6 +185,64 @@ our @tests_tls1_2 = (
             "ResumptionExpected" => "No",
             "ExpectedResult" => "Success"
         }
+    },
+    {
+        name => "no-renegotiation-server-by-client",
+        server => {
+            "Options" => "NoRenegotiation",
+            "MaxProtocol" => "TLSv1.2"
+        },
+        client => { },
+        test => {
+            "Method" => "TLS",
+            "HandshakeMode" => "RenegotiateClient",
+            "ResumptionExpected" => "No",
+            "ExpectedResult" => "ClientFail"
+        }
+    },
+    {
+        name => "no-renegotiation-server-by-server",
+        server => {
+            "Options" => "NoRenegotiation",
+            "MaxProtocol" => "TLSv1.2"
+        },
+        client => { },
+        test => {
+            "Method" => "TLS",
+            "HandshakeMode" => "RenegotiateServer",
+            "ResumptionExpected" => "No",
+            "ExpectedResult" => "ServerFail"
+        }
+    },
+    {
+        name => "no-renegotiation-client-by-server",
+        server => {
+            "MaxProtocol" => "TLSv1.2"
+        },
+        client => {
+            "Options" => "NoRenegotiation",
+        },
+        test => {
+            "Method" => "TLS",
+            "HandshakeMode" => "RenegotiateServer",
+            "ResumptionExpected" => "No",
+            "ExpectedResult" => "ServerFail"
+        }
+    },
+    {
+        name => "no-renegotiation-client-by-client",
+        server => {
+            "MaxProtocol" => "TLSv1.2"
+        },
+        client => {
+            "Options" => "NoRenegotiation",
+        },
+        test => {
+            "Method" => "TLS",
+            "HandshakeMode" => "RenegotiateClient",
+            "ResumptionExpected" => "No",
+            "ExpectedResult" => "ClientFail"
+        }
     }
 );