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

Fix NULL+0 undefined behavior in PSA crypto ECB
This commit is contained in:
Dave Rodgman 2022-11-25 17:07:46 +00:00 committed by GitHub
commit bf9b23abf8
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
23 changed files with 127 additions and 85 deletions

View file

@ -25,8 +25,40 @@ jobs:
- 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

View file

@ -0,0 +1,3 @@
Bugfix
* Fix undefined behavior (typically harmless in practice) in PSA ECB
encryption and decryption.

View file

@ -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

View file

@ -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

View file

@ -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. */

View file

@ -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.
*

View file

@ -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

View file

@ -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 */

View file

@ -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

View file

@ -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 )

View file

@ -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) ) ) )

View file

@ -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 @@ extern void (*mbedtls_test_hook_test_fail)( const char * test, int line, const c
*/
#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

View file

@ -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;

View file

@ -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.

View file

@ -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) \

View file

@ -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 */

View file

@ -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 )
/*

View file

@ -3454,8 +3454,8 @@ psa_status_t psa_cipher_encrypt( mbedtls_svc_key_id_t key,
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 );

View file

@ -516,10 +516,10 @@ psa_status_t mbedtls_psa_cipher_encrypt(
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 @@ psa_status_t mbedtls_psa_cipher_decrypt(
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;

View file

@ -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) && \

View file

@ -595,6 +595,20 @@ setup_arguments()
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"

View file

@ -1689,6 +1689,20 @@ if [ -n "${OPENSSL_LEGACY:-}" ]; then
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"

View file

@ -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 @@ void cipher_alg_without_iv( int alg_arg, int key_type_arg, data_t *key_data,
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 @@ void cipher_alg_without_iv( int alg_arg, int key_type_arg, data_t *key_data,
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;