From e458869b3fa94468f3babb702e9c533d0ec03806 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?=
 <manuel.pegourie-gonnard@arm.com>
Date: Mon, 29 Jul 2019 12:28:52 +0200
Subject: [PATCH] Improve reability and debugability of large if

Breaking into a series of statements makes things easier when stepping through
the code in a debugger.

Previous comments we stating the opposite or what the code tested for (what we
want vs what we're erroring out on) which was confusing.

Also expand a bit on the reasons for these restrictions.
---
 library/ssl_tls.c | 57 ++++++++++++++++++++++++++++-------------------
 1 file changed, 34 insertions(+), 23 deletions(-)

diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index 5960f3d2ea..138e1da0d8 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -11402,31 +11402,42 @@ int mbedtls_ssl_context_save( mbedtls_ssl_context *ssl,
     int ret = 0;
 
     /*
-     * Enforce current usage restrictions
+     * Enforce usage restrictions, see "return BAD_INPUT_DATA" in
+     * this function's documentation.
+     *
+     * These are due to assumptions/limitations in the implementation. Some of
+     * them are likely to stay (no handshake in progress) some might go away
+     * (only DTLS) but are currently used to simplify the implementation.
      */
-    if( /* The initial handshake is over ... */
-        ssl->state != MBEDTLS_SSL_HANDSHAKE_OVER ||
-        ssl->handshake != NULL ||
-        /* ... and the various sub-structures are indeed ready. */
-        ssl->transform == NULL ||
-        ssl->session == NULL ||
-        /* There is no pending incoming or outgoing data ... */
-        mbedtls_ssl_check_pending( ssl ) != 0 ||
-        ssl->out_left != 0 ||
-        /* We're using DTLS 1.2 ... */
-        ssl->conf->transport != MBEDTLS_SSL_TRANSPORT_DATAGRAM ||
-        ssl->major_ver != MBEDTLS_SSL_MAJOR_VERSION_3 ||
-        ssl->minor_ver != MBEDTLS_SSL_MINOR_VERSION_3 ||
-        /* ... with an AEAD ciphersuite. */
-        mbedtls_ssl_transform_uses_aead( ssl->transform ) != 1 ||
-        /* Renegotation is disabled. */
-#if defined(MBEDTLS_SSL_RENEGOTIATION)
-        ssl->conf->disable_renegotiation != MBEDTLS_SSL_RENEGOTIATION_DISABLED ||
-#endif
-        0 )
-    {
+    /* The initial handshake must be over */
+    if( ssl->state != MBEDTLS_SSL_HANDSHAKE_OVER )
         return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA );
-    }
+    if( ssl->handshake != NULL )
+        return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA );
+    /* Double-check that sub-structures are indeed ready */
+    if( ssl->transform == NULL || ssl->session == NULL )
+        return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA );
+    /* There must be no pending incoming or outgoing data */
+    if( mbedtls_ssl_check_pending( ssl ) != 0 )
+        return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA );
+    if( ssl->out_left != 0 )
+        return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA );
+    /* Protocol must be DLTS, not TLS */
+    if( ssl->conf->transport != MBEDTLS_SSL_TRANSPORT_DATAGRAM )
+        return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA );
+    /* Version must be 1.2 */
+    if( ssl->major_ver != MBEDTLS_SSL_MAJOR_VERSION_3 )
+        return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA );
+    if( ssl->minor_ver != MBEDTLS_SSL_MINOR_VERSION_3 )
+        return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA );
+    /* We must be using an AEAD ciphersuite */
+    if( mbedtls_ssl_transform_uses_aead( ssl->transform ) != 1 )
+        return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA );
+    /* Renegotiation must not be enabled */
+#if defined(MBEDTLS_SSL_RENEGOTIATION)
+    if( ssl->conf->disable_renegotiation != MBEDTLS_SSL_RENEGOTIATION_DISABLED )
+        return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA );
+#endif
 
     /*
      * Version and format identifier