Merge pull request #6648 from gilles-peskine-arm/psa-ecb-null-0

Fix NULL+0 undefined behavior in PSA crypto ECB
diff --git a/.travis.yml b/.travis.yml
index 67cb3ca..eaf817a 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -25,8 +25,40 @@
         - tests/scripts/all.sh -k build_arm_linux_gnueabi_gcc_arm5vte build_arm_none_eabi_gcc_m0plus
 
     - name: full configuration
+      os: linux
+      dist: focal
+      addons:
+        apt:
+          packages:
+          - clang-10
+          - gnutls-bin
       script:
-        - tests/scripts/all.sh -k test_full_cmake_gcc_asan
+        # Do a manual build+test sequence rather than using all.sh,
+        # because there's no all.sh component that does what we want,
+        # which is a build with Clang >= 10 and ASan, running all the SSL
+        # testing.
+        #   - The clang executable in the default PATH is Clang 7 on
+        #     Travis's focal instances, but we want Clang >= 10.
+        #   - Running all the SSL testing requires a specific set of
+        #     OpenSSL and GnuTLS versions and we don't want to bother
+        #     with those on Travis.
+        # So we explicitly select clang-10 as the compiler, and we
+        # have ad hoc restrictions on SSL testing based on what is
+        # passing at the time of writing. We will remove these limitations
+        # gradually.
+        - make generated_files
+        - make CC=clang-10 CFLAGS='-Werror -Wall -Wextra -fsanitize=address,undefined -fno-sanitize-recover=all -O2' LDFLAGS='-Werror -Wall -Wextra -fsanitize=address,undefined -fno-sanitize-recover=all'
+        - make test
+        - programs/test/selftest
+        - tests/scripts/test_psa_constant_names.py
+        - tests/ssl-opt.sh
+        # Modern OpenSSL does not support fixed ECDH or null ciphers.
+        - tests/compat.sh -p OpenSSL -e 'NULL\|ECDH-'
+        - tests/scripts/travis-log-failure.sh
+        # GnuTLS supports CAMELLIA but compat.sh doesn't properly enable it.
+        - tests/compat.sh -p GnuTLS -e 'CAMELLIA'
+        - tests/scripts/travis-log-failure.sh
+        - tests/context-info.sh
 
     - name: Windows
       os: windows
diff --git a/ChangeLog.d/psa-ecb-ub.txt b/ChangeLog.d/psa-ecb-ub.txt
new file mode 100644
index 0000000..9d725ac
--- /dev/null
+++ b/ChangeLog.d/psa-ecb-ub.txt
@@ -0,0 +1,3 @@
+Bugfix
+   * Fix undefined behavior (typically harmless in practice) in PSA ECB
+     encryption and decryption.
diff --git a/include/mbedtls/aes.h b/include/mbedtls/aes.h
index c359011..1cd20fe 100644
--- a/include/mbedtls/aes.h
+++ b/include/mbedtls/aes.h
@@ -61,11 +61,6 @@
 /** Invalid input data. */
 #define MBEDTLS_ERR_AES_BAD_INPUT_DATA                    -0x0021
 
-#if ( defined(__ARMCC_VERSION) || defined(_MSC_VER) ) && \
-    !defined(inline) && !defined(__cplusplus)
-#define inline __inline
-#endif
-
 #ifdef __cplusplus
 extern "C" {
 #endif
diff --git a/include/mbedtls/build_info.h b/include/mbedtls/build_info.h
index 170cbeb..362ce2f 100644
--- a/include/mbedtls/build_info.h
+++ b/include/mbedtls/build_info.h
@@ -53,6 +53,12 @@
 #define _CRT_SECURE_NO_DEPRECATE 1
 #endif
 
+/* Define `inline` on some non-C99-compliant compilers. */
+#if ( defined(__ARMCC_VERSION) || defined(_MSC_VER) ) && \
+    !defined(inline) && !defined(__cplusplus)
+#define inline __inline
+#endif
+
 #if !defined(MBEDTLS_CONFIG_FILE)
 #include "mbedtls/mbedtls_config.h"
 #else
diff --git a/include/mbedtls/cipher.h b/include/mbedtls/cipher.h
index a3f52ea..151da1d 100644
--- a/include/mbedtls/cipher.h
+++ b/include/mbedtls/cipher.h
@@ -46,11 +46,6 @@
 #define MBEDTLS_CIPHER_MODE_STREAM
 #endif
 
-#if ( defined(__ARMCC_VERSION) || defined(_MSC_VER) ) && \
-    !defined(inline) && !defined(__cplusplus)
-#define inline __inline
-#endif
-
 /** The selected feature is not available. */
 #define MBEDTLS_ERR_CIPHER_FEATURE_UNAVAILABLE  -0x6080
 /** Bad input parameters. */
diff --git a/include/mbedtls/error.h b/include/mbedtls/error.h
index 841b75b..4a97d65 100644
--- a/include/mbedtls/error.h
+++ b/include/mbedtls/error.h
@@ -26,11 +26,6 @@
 
 #include <stddef.h>
 
-#if ( defined(__ARMCC_VERSION) || defined(_MSC_VER) ) && \
-    !defined(inline) && !defined(__cplusplus)
-#define inline __inline
-#endif
-
 /**
  * Error code layout.
  *
diff --git a/include/mbedtls/pem.h b/include/mbedtls/pem.h
index c75a124..a4c6fb8 100644
--- a/include/mbedtls/pem.h
+++ b/include/mbedtls/pem.h
@@ -27,11 +27,6 @@
 
 #include <stddef.h>
 
-#if ( defined(__ARMCC_VERSION) || defined(_MSC_VER) ) && \
-    !defined(inline) && !defined(__cplusplus)
-#define inline __inline
-#endif
-
 /**
  * \name PEM Error codes
  * These error codes are returned in case of errors reading the
diff --git a/include/mbedtls/pk.h b/include/mbedtls/pk.h
index 867961d..db0bfac 100644
--- a/include/mbedtls/pk.h
+++ b/include/mbedtls/pk.h
@@ -44,11 +44,6 @@
 #include "psa/crypto.h"
 #endif
 
-#if ( defined(__ARMCC_VERSION) || defined(_MSC_VER) ) && \
-    !defined(inline) && !defined(__cplusplus)
-#define inline __inline
-#endif
-
 /** Memory allocation failed. */
 #define MBEDTLS_ERR_PK_ALLOC_FAILED        -0x3F80
 /** Type mismatch, eg attempt to encrypt with an ECDSA key */
diff --git a/include/psa/crypto_platform.h b/include/psa/crypto_platform.h
index 47ab1cf..573b33c 100644
--- a/include/psa/crypto_platform.h
+++ b/include/psa/crypto_platform.h
@@ -45,11 +45,6 @@
 /* PSA requires several types which C99 provides in stdint.h. */
 #include <stdint.h>
 
-#if ( defined(__ARMCC_VERSION) || defined(_MSC_VER) ) && \
-    !defined(inline) && !defined(__cplusplus)
-#define inline __inline
-#endif
-
 #if defined(MBEDTLS_PSA_CRYPTO_KEY_ID_ENCODES_OWNER)
 
 /* Building for the PSA Crypto service on a PSA platform, a key owner is a PSA
diff --git a/library/aria.c b/library/aria.c
index 924f952..5e52eea 100644
--- a/library/aria.c
+++ b/library/aria.c
@@ -37,11 +37,6 @@
 
 #include "mbedtls/platform_util.h"
 
-#if ( defined(__ARMCC_VERSION) || defined(_MSC_VER) ) && \
-    !defined(inline) && !defined(__cplusplus)
-#define inline __inline
-#endif
-
 /* Parameter validation macros */
 #define ARIA_VALIDATE_RET( cond )                                       \
     MBEDTLS_INTERNAL_VALIDATE_RET( cond, MBEDTLS_ERR_ARIA_BAD_INPUT_DATA )
diff --git a/library/chacha20.c b/library/chacha20.c
index e53eb82..85d7461 100644
--- a/library/chacha20.c
+++ b/library/chacha20.c
@@ -36,11 +36,6 @@
 
 #if !defined(MBEDTLS_CHACHA20_ALT)
 
-#if ( defined(__ARMCC_VERSION) || defined(_MSC_VER) ) && \
-    !defined(inline) && !defined(__cplusplus)
-#define inline __inline
-#endif
-
 #define ROTL32( value, amount ) \
     ( (uint32_t) ( (value) << (amount) ) | ( (value) >> ( 32 - (amount) ) ) )
 
diff --git a/library/common.h b/library/common.h
index a630fcc..25d5294 100644
--- a/library/common.h
+++ b/library/common.h
@@ -25,6 +25,7 @@
 
 #include "mbedtls/build_info.h"
 
+#include <stddef.h>
 #include <stdint.h>
 
 /** Helper to define a function as static except when building invasive tests.
@@ -68,6 +69,44 @@
  */
 #define MBEDTLS_ALLOW_PRIVATE_ACCESS
 
+/** Return an offset into a buffer.
+ *
+ * This is just the addition of an offset to a pointer, except that this
+ * function also accepts an offset of 0 into a buffer whose pointer is null.
+ * (`p + n` has undefined behavior when `p` is null, even when `n == 0`.
+ * A null pointer is a valid buffer pointer when the size is 0, for example
+ * as the result of `malloc(0)` on some platforms.)
+ *
+ * \param p     Pointer to a buffer of at least n bytes.
+ *              This may be \p NULL if \p n is zero.
+ * \param n     An offset in bytes.
+ * \return      Pointer to offset \p n in the buffer \p p.
+ *              Note that this is only a valid pointer if the size of the
+ *              buffer is at least \p n + 1.
+ */
+static inline unsigned char *mbedtls_buffer_offset(
+    unsigned char *p, size_t n )
+{
+    return( p == NULL ? NULL : p + n );
+}
+
+/** Return an offset into a read-only buffer.
+ *
+ * Similar to mbedtls_buffer_offset(), but for const pointers.
+ *
+ * \param p     Pointer to a buffer of at least n bytes.
+ *              This may be \p NULL if \p n is zero.
+ * \param n     An offset in bytes.
+ * \return      Pointer to offset \p n in the buffer \p p.
+ *              Note that this is only a valid pointer if the size of the
+ *              buffer is at least \p n + 1.
+ */
+static inline const unsigned char *mbedtls_buffer_offset_const(
+    const unsigned char *p, size_t n )
+{
+    return( p == NULL ? NULL : p + n );
+}
+
 /** Byte Reading Macros
  *
  * Given a multi-byte integer \p x, MBEDTLS_BYTE_n retrieves the n-th
diff --git a/library/debug.c b/library/debug.c
index bdbf6dd..6114a46 100644
--- a/library/debug.c
+++ b/library/debug.c
@@ -30,11 +30,6 @@
 #include <stdio.h>
 #include <string.h>
 
-#if ( defined(__ARMCC_VERSION) || defined(_MSC_VER) ) && \
-    !defined(inline) && !defined(__cplusplus)
-#define inline __inline
-#endif
-
 #define DEBUG_BUF_SIZE      512
 
 static int debug_threshold = 0;
diff --git a/library/ecp.c b/library/ecp.c
index 37f6090..cd7d554 100644
--- a/library/ecp.c
+++ b/library/ecp.c
@@ -88,11 +88,6 @@
 
 #include "ecp_internal_alt.h"
 
-#if ( defined(__ARMCC_VERSION) || defined(_MSC_VER) ) && \
-    !defined(inline) && !defined(__cplusplus)
-#define inline __inline
-#endif
-
 #if defined(MBEDTLS_SELF_TEST)
 /*
  * Counts of point addition and doubling, and field multiplications.
diff --git a/library/ecp_curves.c b/library/ecp_curves.c
index 7b14237..5cd2828 100644
--- a/library/ecp_curves.c
+++ b/library/ecp_curves.c
@@ -39,11 +39,6 @@
 #define ECP_VALIDATE( cond )        \
     MBEDTLS_INTERNAL_VALIDATE( cond )
 
-#if ( defined(__ARMCC_VERSION) || defined(_MSC_VER) ) && \
-    !defined(inline) && !defined(__cplusplus)
-#define inline __inline
-#endif
-
 #define ECP_MPI_INIT(s, n, p) {s, (n), (mbedtls_mpi_uint *)(p)}
 
 #define ECP_MPI_INIT_ARRAY(x)   \
diff --git a/library/mps_reader.c b/library/mps_reader.c
index 36958b4..6f823bd 100644
--- a/library/mps_reader.c
+++ b/library/mps_reader.c
@@ -29,11 +29,6 @@
 
 #include <string.h>
 
-#if ( defined(__ARMCC_VERSION) || defined(_MSC_VER) ) && \
-    !defined(inline) && !defined(__cplusplus)
-#define inline __inline
-#endif
-
 #if defined(MBEDTLS_MPS_ENABLE_TRACE)
 static int mbedtls_mps_trace_id = MBEDTLS_MPS_TRACE_BIT_READER;
 #endif /* MBEDTLS_MPS_ENABLE_TRACE */
diff --git a/library/poly1305.c b/library/poly1305.c
index 0850f66..4d0cdee 100644
--- a/library/poly1305.c
+++ b/library/poly1305.c
@@ -32,11 +32,6 @@
 
 #if !defined(MBEDTLS_POLY1305_ALT)
 
-#if ( defined(__ARMCC_VERSION) || defined(_MSC_VER) ) && \
-    !defined(inline) && !defined(__cplusplus)
-#define inline __inline
-#endif
-
 #define POLY1305_BLOCK_SIZE_BYTES ( 16U )
 
 /*
diff --git a/library/psa_crypto.c b/library/psa_crypto.c
index 8c9deff..e881f2f 100644
--- a/library/psa_crypto.c
+++ b/library/psa_crypto.c
@@ -3454,8 +3454,8 @@
     status = psa_driver_wrapper_cipher_encrypt(
         &attributes, slot->key.data, slot->key.bytes,
         alg, local_iv, default_iv_length, input, input_length,
-        output + default_iv_length, output_size - default_iv_length,
-        output_length );
+        mbedtls_buffer_offset( output, default_iv_length ),
+        output_size - default_iv_length, output_length );
 
 exit:
     unlock_status = psa_unlock_key_slot( slot );
diff --git a/library/psa_crypto_cipher.c b/library/psa_crypto_cipher.c
index 70dc74d..91a0e3b 100644
--- a/library/psa_crypto_cipher.c
+++ b/library/psa_crypto_cipher.c
@@ -516,10 +516,10 @@
     if( status != PSA_SUCCESS )
         goto exit;
 
-    status = mbedtls_psa_cipher_finish( &operation,
-                                        output + update_output_length,
-                                        output_size - update_output_length,
-                                        &finish_output_length );
+    status = mbedtls_psa_cipher_finish(
+        &operation,
+        mbedtls_buffer_offset( output, update_output_length ),
+        output_size - update_output_length, &finish_output_length );
     if( status != PSA_SUCCESS )
         goto exit;
 
@@ -563,17 +563,20 @@
             goto exit;
     }
 
-    status = mbedtls_psa_cipher_update( &operation, input + operation.iv_length,
-                                        input_length - operation.iv_length,
-                                        output, output_size, &olength );
+    status = mbedtls_psa_cipher_update(
+        &operation,
+        mbedtls_buffer_offset_const( input, operation.iv_length ),
+        input_length - operation.iv_length,
+        output, output_size, &olength );
     if( status != PSA_SUCCESS )
         goto exit;
 
     accumulated_length = olength;
 
-    status = mbedtls_psa_cipher_finish( &operation, output + accumulated_length,
-                                        output_size - accumulated_length,
-                                        &olength );
+    status = mbedtls_psa_cipher_finish(
+        &operation,
+        mbedtls_buffer_offset( output, accumulated_length ),
+        output_size - accumulated_length, &olength );
     if( status != PSA_SUCCESS )
         goto exit;
 
diff --git a/library/ssl_misc.h b/library/ssl_misc.h
index 9998e5b..1902d71 100644
--- a/library/ssl_misc.h
+++ b/library/ssl_misc.h
@@ -57,11 +57,6 @@
 
 #include "common.h"
 
-#if ( defined(__ARMCC_VERSION) || defined(_MSC_VER) ) && \
-    !defined(inline) && !defined(__cplusplus)
-#define inline __inline
-#endif
-
 /* Shorthand for restartable ECC */
 #if defined(MBEDTLS_ECP_RESTARTABLE) && \
     defined(MBEDTLS_SSL_CLI_C) && \
diff --git a/tests/compat.sh b/tests/compat.sh
index d681217..529c2c5 100755
--- a/tests/compat.sh
+++ b/tests/compat.sh
@@ -595,6 +595,20 @@
     G_CLIENT_ARGS="-p $PORT --debug 3 $G_MODE"
     G_CLIENT_PRIO="NONE:$G_PRIO_MODE:+COMP-NULL:+CURVE-ALL:+SIGN-ALL"
 
+    # Newer versions of OpenSSL have a syntax to enable all "ciphers", even
+    # low-security ones. This covers not just cipher suites but also protocol
+    # versions. It is necessary, for example, to use (D)TLS 1.0/1.1 on
+    # OpenSSL 1.1.1f from Ubuntu 20.04. The syntax was only introduced in
+    # OpenSSL 1.1.0 (21e0c1d23afff48601eb93135defddae51f7e2e3) and I can't find
+    # a way to discover it from -help, so check the openssl version.
+    case $($OPENSSL_CMD version) in
+        "OpenSSL 0"*|"OpenSSL 1.0"*) :;;
+        *)
+            O_CLIENT_ARGS="$O_CLIENT_ARGS -cipher ALL@SECLEVEL=0"
+            O_SERVER_ARGS="$O_SERVER_ARGS -cipher ALL@SECLEVEL=0"
+            ;;
+    esac
+
     if [ "X$VERIFY" = "XYES" ];
     then
         M_SERVER_ARGS="$M_SERVER_ARGS ca_file=data_files/test-ca_cat12.crt auth_mode=required"
diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh
index b460c67..c6f6e29 100755
--- a/tests/ssl-opt.sh
+++ b/tests/ssl-opt.sh
@@ -1689,6 +1689,20 @@
     O_LEGACY_CLI="$O_LEGACY_CLI -connect 127.0.0.1:+SRV_PORT"
 fi
 
+# Newer versions of OpenSSL have a syntax to enable all "ciphers", even
+# low-security ones. This covers not just cipher suites but also protocol
+# versions. It is necessary, for example, to use (D)TLS 1.0/1.1 on
+# OpenSSL 1.1.1f from Ubuntu 20.04. The syntax was only introduced in
+# OpenSSL 1.1.0 (21e0c1d23afff48601eb93135defddae51f7e2e3) and I can't find
+# a way to discover it from -help, so check the openssl version.
+case $($OPENSSL_CMD version) in
+    "OpenSSL 0"*|"OpenSSL 1.0"*) :;;
+    *)
+        O_CLI="$O_CLI -cipher ALL@SECLEVEL=0"
+        O_SRV="$O_SRV -cipher ALL@SECLEVEL=0"
+        ;;
+esac
+
 if [ -n "${OPENSSL_NEXT:-}" ]; then
     O_NEXT_SRV="$O_NEXT_SRV -accept $SRV_PORT"
     O_NEXT_SRV_NO_CERT="$O_NEXT_SRV_NO_CERT -accept $SRV_PORT"
diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function
index ca1614b..1f3b3b6 100644
--- a/tests/suites/test_suite_psa_crypto.function
+++ b/tests/suites/test_suite_psa_crypto.function
@@ -4,6 +4,7 @@
 #include "mbedtls/asn1.h"
 #include "mbedtls/asn1write.h"
 #include "mbedtls/oid.h"
+#include "common.h"
 
 /* For MBEDTLS_CTR_DRBG_MAX_REQUEST, knowing that psa_generate_random()
  * uses mbedtls_ctr_drbg internally. */
@@ -3983,7 +3984,7 @@
     TEST_LE_U( length, output_buffer_size );
     output_length += length;
     PSA_ASSERT( psa_cipher_finish( &operation,
-                                   output + output_length,
+                                   mbedtls_buffer_offset( output, output_length ),
                                    output_buffer_size - output_length,
                                    &length ) );
     output_length += length;
@@ -4001,7 +4002,7 @@
     TEST_LE_U( length, output_buffer_size );
     output_length += length;
     PSA_ASSERT( psa_cipher_finish( &operation,
-                                   output + output_length,
+                                   mbedtls_buffer_offset( output, output_length ),
                                    output_buffer_size - output_length,
                                    &length ) );
     output_length += length;