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)

diff --git a/ssl/record/rec_layer_d1.c b/ssl/record/rec_layer_d1.c
index deebbdc315148f0250383494fb11cd9f23663cd8..111fbaf7d3a4028a7902f3bd20449aabbe32929d 100644 (file)
--- a/ssl/record/rec_layer_d1.c
+++ b/ssl/record/rec_layer_d1.c
@@ -436,6 +436,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 ddddc09c951753e0ad671d092344e2c8b1bb3b55..889b12dd66d0a219c705008e26262b6d69210d7e 100644 (file)
--- a/ssl/record/rec_layer_s3.c
+++ b/ssl/record/rec_layer_s3.c
@@ -925,10 +925,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;