Merge pull request #6510 from gilles-peskine-arm/all.sh-simplify-20221028-development
Remove a few redundancies from all.sh
diff --git a/ChangeLog.d/ecdsa-verify-fixes.txt b/ChangeLog.d/ecdsa-verify-fixes.txt
new file mode 100644
index 0000000..b41b046
--- /dev/null
+++ b/ChangeLog.d/ecdsa-verify-fixes.txt
@@ -0,0 +1,5 @@
+Bugfix
+ * Fix ECDSA verification, where it was not always validating the
+ public key. This bug meant that it was possible to verify a
+ signature with an invalid public key, in some cases. Reported by
+ Guido Vranken using Cryptofuzz in #4420.
diff --git a/include/mbedtls/ecdsa.h b/include/mbedtls/ecdsa.h
index 71b73ee..967f07b 100644
--- a/include/mbedtls/ecdsa.h
+++ b/include/mbedtls/ecdsa.h
@@ -245,10 +245,8 @@
* This must be initialized.
*
* \return \c 0 on success.
- * \return #MBEDTLS_ERR_ECP_BAD_INPUT_DATA if the signature
- * is invalid.
* \return An \c MBEDTLS_ERR_ECP_XXX or \c MBEDTLS_MPI_XXX
- * error code on failure for any other reason.
+ * error code on failure.
*/
int mbedtls_ecdsa_verify( mbedtls_ecp_group *grp,
const unsigned char *buf, size_t blen,
diff --git a/library/bignum_mod.c b/library/bignum_mod.c
index f2c11a5..60c073a 100644
--- a/library/bignum_mod.c
+++ b/library/bignum_mod.c
@@ -77,7 +77,14 @@
switch( m->int_rep )
{
case MBEDTLS_MPI_MOD_REP_MONTGOMERY:
- mbedtls_free( m->rep.mont );
+ if (m->rep.mont.rr != NULL)
+ {
+ mbedtls_platform_zeroize( (mbedtls_mpi_uint *) m->rep.mont.rr,
+ m->limbs );
+ mbedtls_free( (mbedtls_mpi_uint *)m->rep.mont.rr );
+ m->rep.mont.rr = NULL;
+ }
+ m->rep.mont.mm = 0;
break;
case MBEDTLS_MPI_MOD_REP_OPT_RED:
mbedtls_free( m->rep.ored );
@@ -93,6 +100,41 @@
m->int_rep = MBEDTLS_MPI_MOD_REP_INVALID;
}
+static int set_mont_const_square( const mbedtls_mpi_uint **X,
+ const mbedtls_mpi_uint *A,
+ size_t limbs )
+{
+ int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
+ mbedtls_mpi N;
+ mbedtls_mpi RR;
+ *X = NULL;
+
+ mbedtls_mpi_init( &N );
+ mbedtls_mpi_init( &RR );
+
+ if ( A == NULL || limbs == 0 || limbs >= ( MBEDTLS_MPI_MAX_LIMBS / 2 ) - 2 )
+ goto cleanup;
+
+ if ( mbedtls_mpi_grow( &N, limbs ) )
+ goto cleanup;
+
+ memcpy( N.p, A, sizeof(mbedtls_mpi_uint) * limbs );
+
+ ret = mbedtls_mpi_core_get_mont_r2_unsafe(&RR, &N);
+
+ if ( ret == 0 )
+ {
+ *X = RR.p;
+ RR.p = NULL;
+ }
+
+cleanup:
+ mbedtls_mpi_free(&N);
+ mbedtls_mpi_free(&RR);
+ ret = ( ret != 0 ) ? MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED : 0;
+ return( ret );
+}
+
int mbedtls_mpi_mod_modulus_setup( mbedtls_mpi_mod_modulus *m,
const mbedtls_mpi_uint *p,
size_t p_limbs,
@@ -120,7 +162,8 @@
{
case MBEDTLS_MPI_MOD_REP_MONTGOMERY:
m->int_rep = int_rep;
- m->rep.mont = NULL;
+ m->rep.mont.mm = mbedtls_mpi_core_montmul_init( m->p );
+ ret = set_mont_const_square( &m->rep.mont.rr, m->p, m->limbs );
break;
case MBEDTLS_MPI_MOD_REP_OPT_RED:
m->int_rep = int_rep;
diff --git a/library/bignum_mod.h b/library/bignum_mod.h
index c25eb87..3b3338c 100644
--- a/library/bignum_mod.h
+++ b/library/bignum_mod.h
@@ -53,7 +53,11 @@
size_t limbs;
} mbedtls_mpi_mod_residue;
-typedef void *mbedtls_mpi_mont_struct;
+typedef struct {
+ mbedtls_mpi_uint const *rr; /* The residue for 2^{2*n*biL} mod N */
+ mbedtls_mpi_uint mm; /* Montgomery const for -N^{-1} mod 2^{ciL} */
+} mbedtls_mpi_mont_struct;
+
typedef void *mbedtls_mpi_opt_red_struct;
typedef struct {
diff --git a/library/ecp.c b/library/ecp.c
index 08e33e2..37f6090 100644
--- a/library/ecp.c
+++ b/library/ecp.c
@@ -2662,14 +2662,17 @@
if( mbedtls_mpi_cmp_int( m, 0 ) == 0 )
{
+ MBEDTLS_MPI_CHK( mbedtls_ecp_check_pubkey( grp, P ) );
MBEDTLS_MPI_CHK( mbedtls_ecp_set_zero( R ) );
}
else if( mbedtls_mpi_cmp_int( m, 1 ) == 0 )
{
+ MBEDTLS_MPI_CHK( mbedtls_ecp_check_pubkey( grp, P ) );
MBEDTLS_MPI_CHK( mbedtls_ecp_copy( R, P ) );
}
else if( mbedtls_mpi_cmp_int( m, -1 ) == 0 )
{
+ MBEDTLS_MPI_CHK( mbedtls_ecp_check_pubkey( grp, P ) );
MBEDTLS_MPI_CHK( mbedtls_ecp_copy( R, P ) );
MPI_ECP_NEG( &R->Y );
}
diff --git a/tests/suites/test_suite_bignum_mod.function b/tests/suites/test_suite_bignum_mod.function
index 9f73209..ad89bdf 100644
--- a/tests/suites/test_suite_bignum_mod.function
+++ b/tests/suites/test_suite_bignum_mod.function
@@ -25,12 +25,28 @@
ret = mbedtls_mpi_mod_modulus_setup( &m, mp, MLIMBS, ext_rep, int_rep );
TEST_EQUAL( ret, iret );
+ /* Only test if the constants have been set-up */
+ if ( ret == 0 && int_rep == MBEDTLS_MPI_MOD_REP_MONTGOMERY )
+ {
+ /* Test that the consts have been calculated */
+ TEST_ASSERT( m.rep.mont.rr != NULL );
+ TEST_ASSERT( m.rep.mont.mm != 0 );
+
+ }
+
/* Address sanitiser should catch if we try to free mp */
mbedtls_mpi_mod_modulus_free( &m );
/* Make sure that the modulus doesn't have reference to mp anymore */
TEST_ASSERT( m.p != mp );
+ /* Only test if the constants have been set-up */
+ if ( ret == 0 && int_rep == MBEDTLS_MPI_MOD_REP_MONTGOMERY )
+ {
+ /* Verify the data and pointers allocated have been properly wiped */
+ TEST_ASSERT( m.rep.mont.rr == NULL );
+ TEST_ASSERT( m.rep.mont.mm == 0 );
+ }
exit:
/* It should be safe to call an mbedtls free several times */
mbedtls_mpi_mod_modulus_free( &m );
diff --git a/tests/suites/test_suite_bignum_mod_raw.function b/tests/suites/test_suite_bignum_mod_raw.function
index 88b8917..7c9d5db 100644
--- a/tests/suites/test_suite_bignum_mod_raw.function
+++ b/tests/suites/test_suite_bignum_mod_raw.function
@@ -134,6 +134,7 @@
ASSERT_ALLOC( Y, limbs );
ASSERT_ALLOC( buff_m, limbs );
+ memset( buff_m, 0xFF, copy_bytes );
TEST_ASSERT( mbedtls_mpi_mod_modulus_setup(
&m, buff_m, copy_limbs,
MBEDTLS_MPI_MOD_EXT_REP_BE,
@@ -214,6 +215,7 @@
ASSERT_ALLOC( tmp_Y, limbs );
ASSERT_ALLOC( buff_m, copy_limbs );
+ memset( buff_m, 0xFF, copy_bytes );
TEST_ASSERT( mbedtls_mpi_mod_modulus_setup(
&m, buff_m, copy_limbs,
MBEDTLS_MPI_MOD_EXT_REP_BE,
diff --git a/tests/suites/test_suite_ecdsa.data b/tests/suites/test_suite_ecdsa.data
index 9bb35c5..50411e9 100644
--- a/tests/suites/test_suite_ecdsa.data
+++ b/tests/suites/test_suite_ecdsa.data
@@ -361,3 +361,63 @@
ECDSA private parameter greater than n p521
depends_on:MBEDTLS_ECP_DP_SECP521R1_ENABLED
ecdsa_prim_test_vectors:MBEDTLS_ECP_DP_SECP521R1:"0065FDA3409451DCAB0A0EAD45495112A3D813C17BFD34BDF8C1209D7DF5849120597779060A7FF9D704ADF78B570FFAD6F062E95C7E0C5D5481C5B153B48B375FA11":"0151518F1AF0F563517EDD5485190DF95A4BF57B5CBA4CF2A9A3F6474725A35F7AFE0A6DDEB8BEDBCD6A197E592D40188901CECD650699C9B5E456AEA5ADD19052A8":"006F3B142EA1BFFF7E2837AD44C9E4FF6D2D34C73184BBAD90026DD5E6E85317D9DF45CAD7803C6C20035B2F3FF63AFF4E1BA64D1C077577DA3F4286C58F0AEAE643":"00C1C2B305419F5A41344D7E4359933D734096F556197A9B244342B8B62F46F9373778F9DE6B6497B1EF825FF24F42F9B4A4BD7382CFC3378A540B1B7F0C1B956C2F":"DDAF35A193617ABACC417349AE20413112E6FA4E89A97EA20A9EEEE64B55D39A2192992A274FC1A836BA3C23A3FEEBBD454D4423643CE80E2A9AC94FA54CA49F":"0154FD3836AF92D0DCA57DD5341D3053988534FDE8318FC6AAAAB68E2E6F4339B19F2F281A7E0B22C269D93CF8794A9278880ED7DBB8D9362CAEACEE544320552251":"017705A7030290D1CEB605A9A1BB03FF9CDD521E87A696EC926C8C10C8362DF4975367101F67D1CF9BCCBF2F3D239534FA509E70AAC851AE01AAC68D62F866472660":MBEDTLS_ERR_ECP_INVALID_KEY
+
+ECDSA verify invalid pub key (not on curve), zero bytes of data
+depends_on:MBEDTLS_ECP_DP_SECP256K1_ENABLED
+ecdsa_verify:MBEDTLS_ECP_DP_SECP256K1:"1":"2":"1":"1":"":MBEDTLS_ERR_ECP_INVALID_KEY
+
+ECDSA verify invalid pub key (not on curve), one byte of data
+depends_on:MBEDTLS_ECP_DP_SECP256K1_ENABLED
+ecdsa_verify:MBEDTLS_ECP_DP_SECP256K1:"1":"2":"1":"1":"00":MBEDTLS_ERR_ECP_INVALID_KEY
+
+ECDSA verify invalid pub key (not on curve), r=1, s=1
+depends_on:MBEDTLS_ECP_DP_SECP256K1_ENABLED
+ecdsa_verify:MBEDTLS_ECP_DP_SECP256K1:"1":"2":"1":"1":"0000000000000000000000000000000000000000000000000000000000000000":MBEDTLS_ERR_ECP_INVALID_KEY
+
+ECDSA verify invalid pub key (also not on curve), r=1, s=1
+depends_on:MBEDTLS_ECP_DP_SECP256K1_ENABLED
+ecdsa_verify:MBEDTLS_ECP_DP_SECP256K1:"1":"12345":"1":"1":"0000000000000000000000000000000000000000000000000000000000000000":MBEDTLS_ERR_ECP_INVALID_KEY
+
+ECDSA verify invalid pub key (not on curve), r=12345, s=1
+depends_on:MBEDTLS_ECP_DP_SECP256K1_ENABLED
+ecdsa_verify:MBEDTLS_ECP_DP_SECP256K1:"1":"2":"12345":"1":"0000000000000000000000000000000000000000000000000000000000000000":MBEDTLS_ERR_ECP_INVALID_KEY
+
+ECDSA verify invalid pub key (not on curve), r=1, s=12345
+depends_on:MBEDTLS_ECP_DP_SECP256K1_ENABLED
+ecdsa_verify:MBEDTLS_ECP_DP_SECP256K1:"1":"2":"1":"12345":"0000000000000000000000000000000000000000000000000000000000000000":MBEDTLS_ERR_ECP_INVALID_KEY
+
+ECDSA verify valid pub key, invalid sig (r=0), 0 bytes of data
+depends_on:MBEDTLS_ECP_DP_SECP256K1_ENABLED
+ecdsa_verify:MBEDTLS_ECP_DP_SECP256K1:"79BE667EF9DCBBAC55A06295CE870B07029BFCDB2DCE28D959F2815B16F81798":"483ADA7726A3C4655DA4FBFC0E1108A8FD17B448A68554199C47D08FFB10D4B8":"0":"1":"":MBEDTLS_ERR_ECP_VERIFY_FAILED
+
+ECDSA verify valid pub key, invalid sig (r=0), 1 byte of data
+depends_on:MBEDTLS_ECP_DP_SECP256K1_ENABLED
+ecdsa_verify:MBEDTLS_ECP_DP_SECP256K1:"79BE667EF9DCBBAC55A06295CE870B07029BFCDB2DCE28D959F2815B16F81798":"483ADA7726A3C4655DA4FBFC0E1108A8FD17B448A68554199C47D08FFB10D4B8":"0":"1":"00":MBEDTLS_ERR_ECP_VERIFY_FAILED
+
+ECDSA verify valid pub key, invalid sig (r>n-1), 32 bytes of data
+depends_on:MBEDTLS_ECP_DP_SECP256K1_ENABLED
+ecdsa_verify:MBEDTLS_ECP_DP_SECP256K1:"79BE667EF9DCBBAC55A06295CE870B07029BFCDB2DCE28D959F2815B16F81798":"483ADA7726A3C4655DA4FBFC0E1108A8FD17B448A68554199C47D08FFB10D4B8":"FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141":"12":"0000000000000000000000000000000000000000000000000000000000000000":MBEDTLS_ERR_ECP_VERIFY_FAILED
+
+ECDSA verify valid pub key, valid/incorrect sig, 0 bytes of data
+depends_on:MBEDTLS_ECP_DP_SECP256K1_ENABLED
+ecdsa_verify:MBEDTLS_ECP_DP_SECP256K1:"79BE667EF9DCBBAC55A06295CE870B07029BFCDB2DCE28D959F2815B16F81798":"483ADA7726A3C4655DA4FBFC0E1108A8FD17B448A68554199C47D08FFB10D4B8":"ed3bace23c5e17652e174c835fb72bf53ee306b3406a26890221b4cef7500f88":"84eead3fb3cdbdac882412af64cc125b6784690bebf575f1c32162ab65080037":"":MBEDTLS_ERR_ECP_VERIFY_FAILED
+
+ECDSA verify valid pub key, valid/incorrect sig, 1 byte of data
+depends_on:MBEDTLS_ECP_DP_SECP256K1_ENABLED
+ecdsa_verify:MBEDTLS_ECP_DP_SECP256K1:"79BE667EF9DCBBAC55A06295CE870B07029BFCDB2DCE28D959F2815B16F81798":"483ADA7726A3C4655DA4FBFC0E1108A8FD17B448A68554199C47D08FFB10D4B8":"ed3bace23c5e17652e174c835fb72bf53ee306b3406a26890221b4cef7500f88":"84eead3fb3cdbdac882412af64cc125b6784690bebf575f1c32162ab65080037":"00":MBEDTLS_ERR_ECP_VERIFY_FAILED
+
+ECDSA verify valid pub key, valid/incorrect sig, 32 bytes of data
+depends_on:MBEDTLS_ECP_DP_SECP256K1_ENABLED
+ecdsa_verify:MBEDTLS_ECP_DP_SECP256K1:"79BE667EF9DCBBAC55A06295CE870B07029BFCDB2DCE28D959F2815B16F81798":"483ADA7726A3C4655DA4FBFC0E1108A8FD17B448A68554199C47D08FFB10D4B8":"ed3bace23c5e17652e174c835fb72bf53ee306b3406a26890221b4cef7500f88":"84eead3fb3cdbdac882412af64cc125b6784690bebf575f1c32162ab65080037":"0000000000000000000000000000000000000000000000000000000000000000":MBEDTLS_ERR_ECP_VERIFY_FAILED
+
+ECDSA verify valid public key, correct sig, 0 bytes of data
+depends_on:MBEDTLS_ECP_DP_SECP256K1_ENABLED
+ecdsa_verify:MBEDTLS_ECP_DP_SECP256K1:"79BE667EF9DCBBAC55A06295CE870B07029BFCDB2DCE28D959F2815B16F81798":"483ADA7726A3C4655DA4FBFC0E1108A8FD17B448A68554199C47D08FFB10D4B8":"ed3bace23c5e17652e174c835fb72bf53ee306b3406a26890221b4cef7500f88":"c9cc1ba95156bc103055a5d7946f3a3ae7f0657d1e53f1d5c2c9782950aa69b":"":0
+
+ECDSA verify valid pub key, correct sig, 1 byte of data
+depends_on:MBEDTLS_ECP_DP_SECP256K1_ENABLED
+ecdsa_verify:MBEDTLS_ECP_DP_SECP256K1:"79BE667EF9DCBBAC55A06295CE870B07029BFCDB2DCE28D959F2815B16F81798":"483ADA7726A3C4655DA4FBFC0E1108A8FD17B448A68554199C47D08FFB10D4B8":"ed3bace23c5e17652e174c835fb72bf53ee306b3406a26890221b4cef7500f88":"c9cc1ba95156bc103055a5d7946f3a3ae7f0657d1e53f1d5c2c9782950aa69b":"00":0
+
+ECDSA verify valid pub key, correct sig, 32 bytes of data
+depends_on:MBEDTLS_ECP_DP_SECP256K1_ENABLED
+ecdsa_verify:MBEDTLS_ECP_DP_SECP256K1:"79BE667EF9DCBBAC55A06295CE870B07029BFCDB2DCE28D959F2815B16F81798":"483ADA7726A3C4655DA4FBFC0E1108A8FD17B448A68554199C47D08FFB10D4B8":"ed3bace23c5e17652e174c835fb72bf53ee306b3406a26890221b4cef7500f88":"c9cc1ba95156bc103055a5d7946f3a3ae7f0657d1e53f1d5c2c9782950aa69b":"0000000000000000000000000000000000000000000000000000000000000000":0
diff --git a/tests/suites/test_suite_ecdsa.function b/tests/suites/test_suite_ecdsa.function
index e82f39d..0e2ac92 100644
--- a/tests/suites/test_suite_ecdsa.function
+++ b/tests/suites/test_suite_ecdsa.function
@@ -466,3 +466,40 @@
mbedtls_ecdsa_free( &ctx );
}
/* END_CASE */
+
+/* BEGIN_CASE */
+void ecdsa_verify( int grp_id, char * x, char * y, char * r, char * s, data_t * content, int expected )
+{
+ mbedtls_ecdsa_context ctx;
+ mbedtls_mpi sig_r, sig_s;
+
+ mbedtls_ecdsa_init( &ctx );
+ mbedtls_mpi_init( &sig_r );
+ mbedtls_mpi_init( &sig_s );
+
+ /* Prepare ECP group context */
+ TEST_EQUAL( mbedtls_ecp_group_load( &ctx.grp, grp_id ), 0 );
+
+ /* Prepare public key */
+ TEST_EQUAL( mbedtls_test_read_mpi( &ctx.Q.X, x ), 0 );
+ TEST_EQUAL( mbedtls_test_read_mpi( &ctx.Q.Y, y ), 0 );
+ TEST_EQUAL( mbedtls_mpi_lset( &ctx.Q.Z, 1 ), 0 );
+
+ /* Prepare signature R & S */
+ TEST_EQUAL( mbedtls_test_read_mpi( &sig_r, r ), 0 );
+ TEST_EQUAL( mbedtls_test_read_mpi( &sig_s, s ), 0 );
+
+ /* Test whether public key has expected validity */
+ TEST_EQUAL( mbedtls_ecp_check_pubkey( &ctx.grp, &ctx.Q ),
+ expected == MBEDTLS_ERR_ECP_INVALID_KEY ? MBEDTLS_ERR_ECP_INVALID_KEY : 0 );
+
+ /* Verification */
+ int result = mbedtls_ecdsa_verify( &ctx.grp, content->x, content->len, &ctx.Q, &sig_r, &sig_s );
+
+ TEST_EQUAL( result, expected );
+exit:
+ mbedtls_ecdsa_free( &ctx );
+ mbedtls_mpi_free( &sig_r );
+ mbedtls_mpi_free( &sig_s );
+}
+/* END_CASE */