Merge remote-tracking branch 'restricted/pr/469' into mbedtls-2.1-restricted-proposed
* restricted/pr/469:
Improve comments style
Remove a redundant test
Add buffer size check before cert_type_len read
Update change log
Adjust 2.1 specific code to match the buffer verification tests
Add a missing buffer size check
Correct buffer size check
diff --git a/ChangeLog b/ChangeLog
index 9f995dc..5084c48 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -30,6 +30,8 @@
ECPrivateKey structure. Found by jethrogb, fixed in #1379.
* Return plaintext data sooner on unpadded CBC decryption, as stated in
the mbedtls_cipher_update() documentation. Contributed by Andy Leiserson.
+ * Fix overriding and ignoring return values when parsing and writing to
+ a file in pk_sign program. Found by kevlut in #1142.
* Fix buffer length assertions in the ssl_parse_certificate_request()
function which leads to a potential one byte overread of the message
buffer.
@@ -52,6 +54,12 @@
Alex Hixon.
* Allow configuring the shared library extension by setting the DLEXT
environment variable when using the project makefiles.
+ * In the SSL module, when f_send, f_recv or f_recv_timeout report
+ transmitting more than the required length, return an error. Raised by
+ Sam O'Connor in #1245.
+ * Improve robustness of mbedtls_ssl_derive_keys against the use of
+ HMAC functions with non-HMAC ciphersuites. Independently contributed
+ by Jiayuan Chen in #1377. Fixes #1437.
= mbed TLS 2.1.11 branch released 2018-03-16
diff --git a/doxygen/mbedtls.doxyfile b/doxygen/mbedtls.doxyfile
index af9d5ef..729d57a 100644
--- a/doxygen/mbedtls.doxyfile
+++ b/doxygen/mbedtls.doxyfile
@@ -702,7 +702,7 @@
# directories that are symbolic links (a Unix file system feature) are excluded
# from the input.
-EXCLUDE_SYMLINKS = NO
+EXCLUDE_SYMLINKS = YES
# If the value of the INPUT tag contains directories, you can use the
# EXCLUDE_PATTERNS tag to specify one or more wildcard patterns to exclude
diff --git a/include/mbedtls/ecdsa.h b/include/mbedtls/ecdsa.h
index a277715..b258700 100644
--- a/include/mbedtls/ecdsa.h
+++ b/include/mbedtls/ecdsa.h
@@ -219,8 +219,8 @@
*
* \return 0 if successful,
* MBEDTLS_ERR_ECP_BAD_INPUT_DATA if signature is invalid,
- * MBEDTLS_ERR_ECP_SIG_LEN_MISMATCH if the signature is
- * valid but its actual length is less than siglen,
+ * MBEDTLS_ERR_ECP_SIG_LEN_MISMATCH if there is a valid
+ * signature in sig but its length is less than siglen,
* or a MBEDTLS_ERR_ECP_XXX or MBEDTLS_ERR_MPI_XXX error code
*/
int mbedtls_ecdsa_read_signature( mbedtls_ecdsa_context *ctx,
diff --git a/include/mbedtls/ecp.h b/include/mbedtls/ecp.h
index ef65326..3c2fbaa 100644
--- a/include/mbedtls/ecp.h
+++ b/include/mbedtls/ecp.h
@@ -35,7 +35,7 @@
#define MBEDTLS_ERR_ECP_ALLOC_FAILED -0x4D80 /**< Memory allocation failed. */
#define MBEDTLS_ERR_ECP_RANDOM_FAILED -0x4D00 /**< Generation of random value, such as (ephemeral) key, failed. */
#define MBEDTLS_ERR_ECP_INVALID_KEY -0x4C80 /**< Invalid private or public key. */
-#define MBEDTLS_ERR_ECP_SIG_LEN_MISMATCH -0x4C00 /**< Signature is valid but shorter than the user-supplied length. */
+#define MBEDTLS_ERR_ECP_SIG_LEN_MISMATCH -0x4C00 /**< The buffer contains a valid signature followed by more data. */
#ifdef __cplusplus
extern "C" {
diff --git a/include/mbedtls/pk.h b/include/mbedtls/pk.h
index 458bb51..3a6a1b3 100644
--- a/include/mbedtls/pk.h
+++ b/include/mbedtls/pk.h
@@ -62,7 +62,7 @@
#define MBEDTLS_ERR_PK_INVALID_ALG -0x3A80 /**< The algorithm tag or value is invalid. */
#define MBEDTLS_ERR_PK_UNKNOWN_NAMED_CURVE -0x3A00 /**< Elliptic curve is unsupported (only NIST curves are supported). */
#define MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE -0x3980 /**< Unavailable feature, e.g. RSA disabled for RSA key. */
-#define MBEDTLS_ERR_PK_SIG_LEN_MISMATCH -0x3900 /**< The signature is valid but its length is less than expected. */
+#define MBEDTLS_ERR_PK_SIG_LEN_MISMATCH -0x3900 /**< The buffer contains a valid signature followed by more data. */
#ifdef __cplusplus
extern "C" {
@@ -267,8 +267,8 @@
* \param sig_len Signature length
*
* \return 0 on success (signature is valid),
- * MBEDTLS_ERR_PK_SIG_LEN_MISMATCH if the signature is
- * valid but its actual length is less than sig_len,
+ * #MBEDTLS_ERR_PK_SIG_LEN_MISMATCH if there is a valid
+ * signature in sig but its length is less than \p siglen,
* or a specific error code.
*
* \note For RSA keys, the default padding type is PKCS#1 v1.5.
@@ -298,10 +298,10 @@
* \param sig_len Signature length
*
* \return 0 on success (signature is valid),
- * MBEDTLS_ERR_PK_TYPE_MISMATCH if the PK context can't be
+ * #MBEDTLS_ERR_PK_TYPE_MISMATCH if the PK context can't be
* used for this type of signatures,
- * MBEDTLS_ERR_PK_SIG_LEN_MISMATCH if the signature is
- * valid but its actual length is less than sig_len,
+ * #MBEDTLS_ERR_PK_SIG_LEN_MISMATCH if there is a valid
+ * signature in sig but its length is less than \p siglen,
* or a specific error code.
*
* \note If hash_len is 0, then the length associated with md_alg
diff --git a/library/ecdsa.c b/library/ecdsa.c
index 8892317..70fd202 100644
--- a/library/ecdsa.c
+++ b/library/ecdsa.c
@@ -396,6 +396,9 @@
&ctx->Q, &r, &s ) ) != 0 )
goto cleanup;
+ /* At this point we know that the buffer starts with a valid signature.
+ * Return 0 if the buffer just contains the signature, and a specific
+ * error code if the valid signature is followed by more data. */
if( p != end )
ret = MBEDTLS_ERR_ECP_SIG_LEN_MISMATCH;
diff --git a/library/error.c b/library/error.c
index 3854ab5..9d758df 100644
--- a/library/error.c
+++ b/library/error.c
@@ -221,7 +221,7 @@
if( use_ret == -(MBEDTLS_ERR_ECP_INVALID_KEY) )
mbedtls_snprintf( buf, buflen, "ECP - Invalid private or public key" );
if( use_ret == -(MBEDTLS_ERR_ECP_SIG_LEN_MISMATCH) )
- mbedtls_snprintf( buf, buflen, "ECP - Signature is valid but shorter than the user-supplied length" );
+ mbedtls_snprintf( buf, buflen, "ECP - The buffer contains a valid signature followed by more data" );
#endif /* MBEDTLS_ECP_C */
#if defined(MBEDTLS_MD_C)
@@ -284,7 +284,7 @@
if( use_ret == -(MBEDTLS_ERR_PK_FEATURE_UNAVAILABLE) )
mbedtls_snprintf( buf, buflen, "PK - Unavailable feature, e.g. RSA disabled for RSA key" );
if( use_ret == -(MBEDTLS_ERR_PK_SIG_LEN_MISMATCH) )
- mbedtls_snprintf( buf, buflen, "PK - The signature is valid but its length is less than expected" );
+ mbedtls_snprintf( buf, buflen, "PK - The buffer contains a valid signature followed by more data" );
#endif /* MBEDTLS_PK_C */
#if defined(MBEDTLS_PKCS12_C)
diff --git a/library/pk_wrap.c b/library/pk_wrap.c
index 2c164b7..23b41e7 100644
--- a/library/pk_wrap.c
+++ b/library/pk_wrap.c
@@ -90,6 +90,11 @@
(unsigned int) hash_len, hash, sig ) ) != 0 )
return( ret );
+ /* The buffer contains a valid signature followed by extra data.
+ * We have a special error code for that so that so that callers can
+ * use mbedtls_pk_verify() to check "Does the buffer start with a
+ * valid signature?" and not just "Does the buffer contain a valid
+ * signature?". */
if( sig_len > ((mbedtls_rsa_context *) ctx)->len )
return( MBEDTLS_ERR_PK_SIG_LEN_MISMATCH );
diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index 4f0392a..94a8088 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -844,8 +844,13 @@
defined(MBEDTLS_SSL_PROTO_TLS1_2)
if( ssl->minor_ver >= MBEDTLS_SSL_MINOR_VERSION_1 )
{
- mbedtls_md_hmac_starts( &transform->md_ctx_enc, mac_enc, mac_key_len );
- mbedtls_md_hmac_starts( &transform->md_ctx_dec, mac_dec, mac_key_len );
+ /* For HMAC-based ciphersuites, initialize the HMAC transforms.
+ For AEAD-based ciphersuites, there is nothing to do here. */
+ if( mac_key_len != 0 )
+ {
+ mbedtls_md_hmac_starts( &transform->md_ctx_enc, mac_enc, mac_key_len );
+ mbedtls_md_hmac_starts( &transform->md_ctx_dec, mac_dec, mac_key_len );
+ }
}
else
#endif
@@ -2413,6 +2418,14 @@
if( ret < 0 )
return( ret );
+ if ( (size_t)ret > len || ( INT_MAX > SIZE_MAX && ret > SIZE_MAX ) )
+ {
+ MBEDTLS_SSL_DEBUG_MSG( 1,
+ ( "f_recv returned %d bytes but only %lu were requested",
+ ret, (unsigned long)len ) );
+ return( MBEDTLS_ERR_SSL_INTERNAL_ERROR );
+ }
+
ssl->in_left += ret;
}
}
@@ -2460,6 +2473,14 @@
if( ret <= 0 )
return( ret );
+ if( (size_t)ret > ssl->out_left || ( INT_MAX > SIZE_MAX && ret > SIZE_MAX ) )
+ {
+ MBEDTLS_SSL_DEBUG_MSG( 1,
+ ( "f_send returned %d bytes but only %lu bytes were sent",
+ ret, (unsigned long)ssl->out_left ) );
+ return( MBEDTLS_ERR_SSL_INTERNAL_ERROR );
+ }
+
ssl->out_left -= ret;
}
diff --git a/programs/pkey/pk_sign.c b/programs/pkey/pk_sign.c
index 322e8af..0207446 100644
--- a/programs/pkey/pk_sign.c
+++ b/programs/pkey/pk_sign.c
@@ -29,6 +29,7 @@
#include "mbedtls/platform.h"
#else
#include <stdio.h>
+#include <stdlib.h>
#define mbedtls_snprintf snprintf
#define mbedtls_printf printf
#endif
@@ -100,8 +101,7 @@
if( ( ret = mbedtls_pk_parse_keyfile( &pk, argv[1], "" ) ) != 0 )
{
- ret = 1;
- mbedtls_printf( " failed\n ! Could not open '%s'\n", argv[1] );
+ mbedtls_printf( " failed\n ! Could not parse '%s'\n", argv[1] );
goto exit;
}
@@ -141,6 +141,7 @@
if( fwrite( buf, 1, olen, f ) != olen )
{
+ ret = 1;
mbedtls_printf( "failed\n ! fwrite failed\n\n" );
goto exit;
}
@@ -167,7 +168,7 @@
fflush( stdout ); getchar();
#endif
- return( ret );
+ return( ret ? EXIT_FAILURE : EXIT_SUCCESS );
}
#endif /* MBEDTLS_BIGNUM_C && MBEDTLS_ENTROPY_C &&
MBEDTLS_SHA256_C && MBEDTLS_PK_PARSE_C && MBEDTLS_FS_IO &&
diff --git a/tests/compat.sh b/tests/compat.sh
index 27d712b..758a6fe 100755
--- a/tests/compat.sh
+++ b/tests/compat.sh
@@ -1072,7 +1072,7 @@
cp $CLI_OUT c-cli-${TESTS}.log
echo " ! outputs saved to c-srv-${TESTS}.log, c-cli-${TESTS}.log"
- if [ "X${USER:-}" = Xbuildbot -o "X${LOGNAME:-}" = Xbuildbot ]; then
+ if [ "X${USER:-}" = Xbuildbot -o "X${LOGNAME:-}" = Xbuildbot -o "${LOG_FAILURE_ON_STDOUT:-0}" != 0 ]; then
echo " ! server output:"
cat c-srv-${TESTS}.log
echo " ! ==================================================="
diff --git a/tests/ssl-opt.sh b/tests/ssl-opt.sh
index 705e5ea..d33cc0f 100755
--- a/tests/ssl-opt.sh
+++ b/tests/ssl-opt.sh
@@ -177,7 +177,7 @@
fi
echo " ! outputs saved to o-XXX-${TESTS}.log"
- if [ "X${USER:-}" = Xbuildbot -o "X${LOGNAME:-}" = Xbuildbot ]; then
+ if [ "X${USER:-}" = Xbuildbot -o "X${LOGNAME:-}" = Xbuildbot -o "${LOG_FAILURE_ON_STDOUT:-0}" != 0 ]; then
echo " ! server output:"
cat o-srv-${TESTS}.log
echo " ! ========================================================"