Fix DTLS handling when receiving a no_renegotiation alert

no_renegotiation is a warning alert sent from the server when it is not
prepared to accept a renegotiation attempt. In TLS we abort the connection
when we receive one of these - which is a reasonable response. However,
in DTLS we incorrectly ignore this and keep trying to renegotiate.

We bring the DTLS handling of a no_renegotiation alert into line with
how TLS handles this. In versions prior to 3.2 handling of a warning
alert in DTLS was mishandled resulting in a failure of the connection,
which ends up being the right thing to do "by accident" in the case of
"no_renegotiation". From 3.2 this mishandling was fixed, but exposed this
latent bug.

Fixes #27419

Reviewed-by: Frederik Wedel-Heinen <fwh.openssl@gmail.com>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/27591)

(cherry picked from commit e5feca0659ef6119f6cedfab1b6af034735723ff)

diff --git a/ssl/record/rec_layer_d1.c b/ssl/record/rec_layer_d1.c
index ee45f8117dcd5e47f9f0a704ed1521bf55ac806a..33bc9cb8f6c7a3d47f8690f0aa5ad94368de1878 100644 (file)
--- a/ssl/record/rec_layer_d1.c
+++ b/ssl/record/rec_layer_d1.c
@@ -435,6 +435,17 @@ int dtls1_read_bytes(SSL *s, uint8_t type, uint8_t *recvd_type,
 #endif
                 sc->shutdown |= SSL_RECEIVED_SHUTDOWN;
                 return 0;
+            } else if (alert_descr == SSL_AD_NO_RENEGOTIATION) {
+                /*
+                 * This is a warning but we receive it if we requested
+                 * renegotiation and the peer denied it. Terminate with a fatal
+                 * alert because if the application tried to renegotiate it
+                 * presumably had a good reason and expects it to succeed. In
+                 * the future we might have a renegotiation where we don't care
+                 * if the peer refused it where we carry on.
+                 */
+                SSLfatal(sc, SSL_AD_HANDSHAKE_FAILURE, SSL_R_NO_RENEGOTIATION);
+                return -1;
             }
         } else if (alert_level == SSL3_AL_FATAL) {
             sc->rwstate = SSL_NOTHING;

diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c
index 303df23c12da3286081fb646596d6c51fabc2fac..218df8ffe13100961c14688a63a8a90696eb2646 100644 (file)
--- a/ssl/record/rec_layer_s3.c
+++ b/ssl/record/rec_layer_s3.c
@@ -921,10 +921,10 @@ int ssl3_read_bytes(SSL *ssl, uint8_t type, uint8_t *recvd_type,
             /*
              * This is a warning but we receive it if we requested
              * renegotiation and the peer denied it. Terminate with a fatal
-             * alert because if application tried to renegotiate it
+             * alert because if the application tried to renegotiate it
              * presumably had a good reason and expects it to succeed. In
-             * future we might have a renegotiation where we don't care if
-             * the peer refused it where we carry on.
+             * the future we might have a renegotiation where we don't care
+             * if the peer refused it where we carry on.
              */
             SSLfatal(s, SSL_AD_HANDSHAKE_FAILURE, SSL_R_NO_RENEGOTIATION);
             return -1;