From 5114d3e4e1cb6e5a71ceafa56dd7da5f9182f9d9 Mon Sep 17 00:00:00 2001
From: Gilles Peskine <Gilles.Peskine@arm.com>
Date: Fri, 30 Mar 2018 07:12:15 +0200
Subject: [PATCH 1/2] Clarify the use of MBEDTLS_ERR_PK_SIG_LEN_MISMATCH
Clarify what MBEDTLS_ERR_ECP_SIG_LEN_MISMATCH and
MBEDTLS_ERR_PK_SIG_LEN_MISMATCH mean. Add comments to highlight that
this indicates that a valid signature is present, unlike other error
codes. See
https://github.com/ARMmbed/mbedtls/pull/1149#discussion_r178130705
---
include/mbedtls/ecdsa.h | 4 ++--
include/mbedtls/ecp.h | 2 +-
include/mbedtls/pk.h | 12 ++++++------
library/ecdsa.c | 3 +++
library/pk_wrap.c | 5 +++++
5 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/include/mbedtls/ecdsa.h b/include/mbedtls/ecdsa.h
index aa23d67f9..ff6efbc3f 100644
--- a/include/mbedtls/ecdsa.h
+++ b/include/mbedtls/ecdsa.h
@@ -272,8 +272,8 @@ int mbedtls_ecdsa_write_signature_det( mbedtls_ecdsa_context *ctx,
*
* \return \c 0 on success,
* #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 \p siglen,
+ * #MBEDTLS_ERR_ECP_SIG_LEN_MISMATCH if there is a valid
+ * signature in sig but its length is less than \p siglen,
* or an \c MBEDTLS_ERR_ECP_XXX or \c MBEDTLS_ERR_MPI_XXX
* error code on failure for any other reason.
*
diff --git a/include/mbedtls/ecp.h b/include/mbedtls/ecp.h
index b00ba4da8..7b8ffff44 100644
--- a/include/mbedtls/ecp.h
+++ b/include/mbedtls/ecp.h
@@ -36,7 +36,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. */
#define MBEDTLS_ERR_ECP_HW_ACCEL_FAILED -0x4B80 /**< ECP hardware accelerator failed. */
#if !defined(MBEDTLS_ECP_ALT)
diff --git a/include/mbedtls/pk.h b/include/mbedtls/pk.h
index 1059bdaa5..ee06b2fd2 100644
--- a/include/mbedtls/pk.h
+++ b/include/mbedtls/pk.h
@@ -63,7 +63,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. */
#define MBEDTLS_ERR_PK_HW_ACCEL_FAILED -0x3880 /**< PK hardware accelerator failed. */
#ifdef __cplusplus
@@ -269,8 +269,8 @@ int mbedtls_pk_can_do( const mbedtls_pk_context *ctx, mbedtls_pk_type_t type );
* \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.
@@ -300,10 +300,10 @@ int mbedtls_pk_verify( mbedtls_pk_context *ctx, mbedtls_md_type_t md_alg,
* \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 826fefe5c..17a88bdd2 100644
--- a/library/ecdsa.c
+++ b/library/ecdsa.c
@@ -400,6 +400,9 @@ int mbedtls_ecdsa_read_signature( mbedtls_ecdsa_context *ctx,
&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/pk_wrap.c b/library/pk_wrap.c
index a4bb35fc8..5446e2350 100644
--- a/library/pk_wrap.c
+++ b/library/pk_wrap.c
@@ -93,6 +93,11 @@ static int rsa_verify_wrap( void *ctx, mbedtls_md_type_t md_alg,
(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 > rsa_len )
return( MBEDTLS_ERR_PK_SIG_LEN_MISMATCH );
From cc78ac46e79d5c5ea6850c9f56483ea94a2a646b Mon Sep 17 00:00:00 2001
From: Gilles Peskine <Gilles.Peskine@arm.com>
Date: Fri, 30 Mar 2018 18:52:10 +0200
Subject: [PATCH 2/2] Update error.c
---
library/error.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/library/error.c b/library/error.c
index 0292480ae..b173c7e8e 100644
--- a/library/error.c
+++ b/library/error.c
@@ -266,7 +266,7 @@ void mbedtls_strerror( int ret, char *buf, size_t buflen )
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" );
if( use_ret == -(MBEDTLS_ERR_ECP_HW_ACCEL_FAILED) )
mbedtls_snprintf( buf, buflen, "ECP - ECP hardware accelerator failed" );
#endif /* MBEDTLS_ECP_C */
@@ -333,7 +333,7 @@ void mbedtls_strerror( int ret, char *buf, size_t buflen )
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" );
if( use_ret == -(MBEDTLS_ERR_PK_HW_ACCEL_FAILED) )
mbedtls_snprintf( buf, buflen, "PK - PK hardware accelerator failed" );
#endif /* MBEDTLS_PK_C */