Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ECDSA failing when the hash is all-bits-zero #4199

Merged
merged 7 commits into from
Apr 19, 2021
Merged

Fix ECDSA failing when the hash is all-bits-zero #4199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9edff74
Fix EC J-PAKE failing when the payload is all-bits-zero
TRodziewicz Mar 4, 2021
782a7ea
ecjpake_zkp_read() now returns ...BAD_INPUT_DATA when r len == 0 and …
tomasz-rodziewicz-mobica Mar 17, 2021
5feb670
Fix the Changelog and extend tests to cover the hash of all-bits zero
tomasz-rodziewicz-mobica Apr 6, 2021
20ad475
Remove trailing spaces
tomasz-rodziewicz-mobica Apr 7, 2021
40de3c9
Fix Changelog, add separate test functions for hash of all-zero bits
tomasz-rodziewicz-mobica Apr 7, 2021
611f043
Correct the new tests names
tomasz-rodziewicz-mobica Apr 7, 2021
0594205
Remove debug statement
tomasz-rodziewicz-mobica Apr 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions ChangeLog.d/issue1792.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Bugfix
* Fix a bug in ECDSA that would cause it to fail when the hash is all-bits
zero. Fixes #1792
2 changes: 1 addition & 1 deletion library/ecjpake.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ static int ecjpake_zkp_read( const mbedtls_md_info_t *md_info,

r_len = *(*p)++;

if( end < *p || (size_t)( end - *p ) < r_len )
if( end < *p || (size_t)( end - *p ) < r_len || r_len == 0 )
{
ret = MBEDTLS_ERR_ECP_BAD_INPUT_DATA;
goto cleanup;
Expand Down
8 changes: 6 additions & 2 deletions library/ecp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2795,7 +2795,7 @@ static int ecp_check_pubkey_sw( const mbedtls_ecp_group *grp, const mbedtls_ecp_

#if defined(MBEDTLS_ECP_SHORT_WEIERSTRASS_ENABLED)
/*
* R = m * P with shortcuts for m == 1 and m == -1
* R = m * P with shortcuts for m == 0, m == 1 and m == -1
* NOT constant-time - ONLY for short Weierstrass!
*/
static int mbedtls_ecp_mul_shortcuts( mbedtls_ecp_group *grp,
Expand All @@ -2806,7 +2806,11 @@ static int mbedtls_ecp_mul_shortcuts( mbedtls_ecp_group *grp,
{
int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;

if( mbedtls_mpi_cmp_int( m, 1 ) == 0 )
if( mbedtls_mpi_cmp_int( m, 0 ) == 0 )
{
MBEDTLS_MPI_CHK( mbedtls_ecp_set_zero( R ) );
}
else if( mbedtls_mpi_cmp_int( m, 1 ) == 0 )
{
MBEDTLS_MPI_CHK( mbedtls_ecp_copy( R, P ) );
}
Expand Down
40 changes: 40 additions & 0 deletions tests/suites/test_suite_ecdsa.data
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,26 @@
ECDSA Parameter validation
ecdsa_invalid_param:

ECDSA primitive hash zero #1
depends_on:MBEDTLS_ECP_DP_SECP192R1_ENABLED
ecdsa_prim_zero:MBEDTLS_ECP_DP_SECP192R1

ECDSA primitive hash zero #2
depends_on:MBEDTLS_ECP_DP_SECP224R1_ENABLED
ecdsa_prim_zero:MBEDTLS_ECP_DP_SECP224R1

ECDSA primitive hash zero #3
depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED
ecdsa_prim_zero:MBEDTLS_ECP_DP_SECP256R1

ECDSA primitive hash zero #4
depends_on:MBEDTLS_ECP_DP_SECP384R1_ENABLED
ecdsa_prim_zero:MBEDTLS_ECP_DP_SECP384R1

ECDSA primitive hash zero #5
depends_on:MBEDTLS_ECP_DP_SECP521R1_ENABLED
ecdsa_prim_zero:MBEDTLS_ECP_DP_SECP521R1

ECDSA primitive random #1
depends_on:MBEDTLS_ECP_DP_SECP192R1_ENABLED
ecdsa_prim_random:MBEDTLS_ECP_DP_SECP192R1
Expand Down Expand Up @@ -33,6 +53,26 @@ ECDSA primitive rfc 4754 p521
depends_on:MBEDTLS_ECP_DP_SECP521R1_ENABLED
ecdsa_prim_test_vectors:MBEDTLS_ECP_DP_SECP521R1:"0065FDA3409451DCAB0A0EAD45495112A3D813C17BFD34BDF8C1209D7DF5849120597779060A7FF9D704ADF78B570FFAD6F062E95C7E0C5D5481C5B153B48B375FA1":"0151518F1AF0F563517EDD5485190DF95A4BF57B5CBA4CF2A9A3F6474725A35F7AFE0A6DDEB8BEDBCD6A197E592D40188901CECD650699C9B5E456AEA5ADD19052A8":"006F3B142EA1BFFF7E2837AD44C9E4FF6D2D34C73184BBAD90026DD5E6E85317D9DF45CAD7803C6C20035B2F3FF63AFF4E1BA64D1C077577DA3F4286C58F0AEAE643":"00C1C2B305419F5A41344D7E4359933D734096F556197A9B244342B8B62F46F9373778F9DE6B6497B1EF825FF24F42F9B4A4BD7382CFC3378A540B1B7F0C1B956C2F":"DDAF35A193617ABACC417349AE20413112E6FA4E89A97EA20A9EEEE64B55D39A2192992A274FC1A836BA3C23A3FEEBBD454D4423643CE80E2A9AC94FA54CA49F":"0154FD3836AF92D0DCA57DD5341D3053988534FDE8318FC6AAAAB68E2E6F4339B19F2F281A7E0B22C269D93CF8794A9278880ED7DBB8D9362CAEACEE544320552251":"017705A7030290D1CEB605A9A1BB03FF9CDD521E87A696EC926C8C10C8362DF4975367101F67D1CF9BCCBF2F3D239534FA509E70AAC851AE01AAC68D62F866472660":0

ECDSA write-read hash zero #1
depends_on:MBEDTLS_ECP_DP_SECP192R1_ENABLED
ecdsa_write_read_zero:MBEDTLS_ECP_DP_SECP192R1

ECDSA write-read hash zero #2
depends_on:MBEDTLS_ECP_DP_SECP224R1_ENABLED
ecdsa_write_read_zero:MBEDTLS_ECP_DP_SECP224R1

ECDSA write-read hash zero #3
depends_on:MBEDTLS_ECP_DP_SECP256R1_ENABLED
ecdsa_write_read_zero:MBEDTLS_ECP_DP_SECP256R1

ECDSA write-read hash zero #4
depends_on:MBEDTLS_ECP_DP_SECP384R1_ENABLED
ecdsa_write_read_zero:MBEDTLS_ECP_DP_SECP384R1

ECDSA write-read hash zero #5
depends_on:MBEDTLS_ECP_DP_SECP521R1_ENABLED
ecdsa_write_read_zero:MBEDTLS_ECP_DP_SECP521R1

ECDSA write-read random #1
depends_on:MBEDTLS_ECP_DP_SECP192R1_ENABLED
ecdsa_write_read_random:MBEDTLS_ECP_DP_SECP192R1
Expand Down
92 changes: 92 additions & 0 deletions tests/suites/test_suite_ecdsa.function
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,38 @@ exit:
}
/* END_CASE */

/* BEGIN_CASE */
void ecdsa_prim_zero( int id )
{
mbedtls_ecp_group grp;
mbedtls_ecp_point Q;
mbedtls_mpi d, r, s;
mbedtls_test_rnd_pseudo_info rnd_info;
unsigned char buf[MBEDTLS_MD_MAX_SIZE];

mbedtls_ecp_group_init( &grp );
mbedtls_ecp_point_init( &Q );
mbedtls_mpi_init( &d ); mbedtls_mpi_init( &r ); mbedtls_mpi_init( &s );
memset( &rnd_info, 0x00, sizeof( mbedtls_test_rnd_pseudo_info ) );
memset( buf, 0, sizeof( buf ) );

TEST_ASSERT( mbedtls_ecp_group_load( &grp, id ) == 0 );
TEST_ASSERT( mbedtls_ecp_gen_keypair( &grp, &d, &Q,
&mbedtls_test_rnd_pseudo_rand,
&rnd_info ) == 0 );

TEST_ASSERT( mbedtls_ecdsa_sign( &grp, &r, &s, &d, buf, sizeof( buf ),
&mbedtls_test_rnd_pseudo_rand,
&rnd_info ) == 0 );
TEST_ASSERT( mbedtls_ecdsa_verify( &grp, buf, sizeof( buf ), &Q, &r, &s ) == 0 );

exit:
mbedtls_ecp_group_free( &grp );
mbedtls_ecp_point_free( &Q );
mbedtls_mpi_free( &d ); mbedtls_mpi_free( &r ); mbedtls_mpi_free( &s );
}
/* END_CASE */

/* BEGIN_CASE */
void ecdsa_prim_random( int id )
{
Expand Down Expand Up @@ -346,6 +378,66 @@ exit:
}
/* END_CASE */

/* BEGIN_CASE depends_on:MBEDTLS_SHA256_C */
void ecdsa_write_read_zero( int id )
{
mbedtls_ecdsa_context ctx;
mbedtls_test_rnd_pseudo_info rnd_info;
unsigned char hash[32];
unsigned char sig[200];
size_t sig_len, i;

mbedtls_ecdsa_init( &ctx );
memset( &rnd_info, 0x00, sizeof( mbedtls_test_rnd_pseudo_info ) );
memset( hash, 0, sizeof( hash ) );
memset( sig, 0x2a, sizeof( sig ) );

/* generate signing key */
TEST_ASSERT( mbedtls_ecdsa_genkey( &ctx, id,
&mbedtls_test_rnd_pseudo_rand,
&rnd_info ) == 0 );

/* generate and write signature, then read and verify it */
TEST_ASSERT( mbedtls_ecdsa_write_signature( &ctx, MBEDTLS_MD_SHA256,
hash, sizeof( hash ),
sig, &sig_len, &mbedtls_test_rnd_pseudo_rand,
&rnd_info ) == 0 );
TEST_ASSERT( mbedtls_ecdsa_read_signature( &ctx, hash, sizeof( hash ),
sig, sig_len ) == 0 );

/* check we didn't write past the announced length */
for( i = sig_len; i < sizeof( sig ); i++ )
TEST_ASSERT( sig[i] == 0x2a );

/* try verification with invalid length */
TEST_ASSERT( mbedtls_ecdsa_read_signature( &ctx, hash, sizeof( hash ),
sig, sig_len - 1 ) != 0 );
TEST_ASSERT( mbedtls_ecdsa_read_signature( &ctx, hash, sizeof( hash ),
sig, sig_len + 1 ) != 0 );

/* try invalid sequence tag */
sig[0]++;
TEST_ASSERT( mbedtls_ecdsa_read_signature( &ctx, hash, sizeof( hash ),
sig, sig_len ) != 0 );
sig[0]--;

/* try modifying r */
sig[10]++;
TEST_ASSERT( mbedtls_ecdsa_read_signature( &ctx, hash, sizeof( hash ),
sig, sig_len ) == MBEDTLS_ERR_ECP_VERIFY_FAILED );
sig[10]--;

/* try modifying s */
sig[sig_len - 1]++;
TEST_ASSERT( mbedtls_ecdsa_read_signature( &ctx, hash, sizeof( hash ),
sig, sig_len ) == MBEDTLS_ERR_ECP_VERIFY_FAILED );
sig[sig_len - 1]--;

exit:
mbedtls_ecdsa_free( &ctx );
}
/* END_CASE */

/* BEGIN_CASE depends_on:MBEDTLS_SHA256_C */
void ecdsa_write_read_random( int id )
{
Expand Down
8 changes: 4 additions & 4 deletions tests/suites/test_suite_ecjpake.data
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ ECJPAKE round one: KKP1: nothing after second point
read_round_one:MBEDTLS_ECJPAKE_CLIENT:"41047ea6e3a4487037a9e0dbd79262b2cc273e779930fc18409ac5361c5fe669d702e147790aeb4ce7fd6575ab0f6c7fd1c335939aa863ba37ec91b7e32bb013bb2b410409f85b3d20ebd7885ce464c08d056d6428fe4dd9287aa365f131f4360ff386d846898bc4b41583c2a5197f65d78742746c12a5ec0a4ffe2f270a750a1d8fb516":MBEDTLS_ERR_ECP_BAD_INPUT_DATA

ECJPAKE round one: KKP1: zero-length r
read_round_one:MBEDTLS_ECJPAKE_CLIENT:"41047ea6e3a4487037a9e0dbd79262b2cc273e779930fc18409ac5361c5fe669d702e147790aeb4ce7fd6575ab0f6c7fd1c335939aa863ba37ec91b7e32bb013bb2b410409f85b3d20ebd7885ce464c08d056d6428fe4dd9287aa365f131f4360ff386d846898bc4b41583c2a5197f65d78742746c12a5ec0a4ffe2f270a750a1d8fb51600":MBEDTLS_ERR_ECP_INVALID_KEY
read_round_one:MBEDTLS_ECJPAKE_CLIENT:"41047ea6e3a4487037a9e0dbd79262b2cc273e779930fc18409ac5361c5fe669d702e147790aeb4ce7fd6575ab0f6c7fd1c335939aa863ba37ec91b7e32bb013bb2b410409f85b3d20ebd7885ce464c08d056d6428fe4dd9287aa365f131f4360ff386d846898bc4b41583c2a5197f65d78742746c12a5ec0a4ffe2f270a750a1d8fb51600":MBEDTLS_ERR_ECP_BAD_INPUT_DATA

ECJPAKE round one: KKP1: no data for r
read_round_one:MBEDTLS_ECJPAKE_CLIENT:"41047ea6e3a4487037a9e0dbd79262b2cc273e779930fc18409ac5361c5fe669d702e147790aeb4ce7fd6575ab0f6c7fd1c335939aa863ba37ec91b7e32bb013bb2b410409f85b3d20ebd7885ce464c08d056d6428fe4dd9287aa365f131f4360ff386d846898bc4b41583c2a5197f65d78742746c12a5ec0a4ffe2f270a750a1d8fb51601":MBEDTLS_ERR_ECP_BAD_INPUT_DATA
Expand Down Expand Up @@ -104,7 +104,7 @@ ECJPAKE round one: KKP2: nothing after second point
read_round_one:MBEDTLS_ECJPAKE_CLIENT:"4104190a07700ffa4be6ae1d79ee0f06aeb544cd5addaabedf70f8623321332c54f355f0fbfec783ed359e5d0bf7377a0fc4ea7ace473c9c112b41ccd41ac56a56124104360a1cea33fce641156458e0a4eac219e96831e6aebc88b3f3752f93a0281d1bf1fb106051db9694a8d6e862a5ef1324a3d9e27894f1ee4f7c59199965a8dd4a2091847d2d22df3ee55faa2a3fb33fd2d1e055a07a7c61ecfb8d80ec00c2c9eb1241047ea6e3a4487037a9e0dbd79262b2cc273e779930fc18409ac5361c5fe669d702e147790aeb4ce7fd6575ab0f6c7fd1c335939aa863ba37ec91b7e32bb013bb2b410409f85b3d20ebd7885ce464c08d056d6428fe4dd9287aa365f131f4360ff386d846898bc4b41583c2a5197f65d78742746c12a5ec0a4ffe2f270a750a1d8fb516":MBEDTLS_ERR_ECP_BAD_INPUT_DATA

ECJPAKE round one: KKP2: zero-length r
read_round_one:MBEDTLS_ECJPAKE_CLIENT:"4104190a07700ffa4be6ae1d79ee0f06aeb544cd5addaabedf70f8623321332c54f355f0fbfec783ed359e5d0bf7377a0fc4ea7ace473c9c112b41ccd41ac56a56124104360a1cea33fce641156458e0a4eac219e96831e6aebc88b3f3752f93a0281d1bf1fb106051db9694a8d6e862a5ef1324a3d9e27894f1ee4f7c59199965a8dd4a2091847d2d22df3ee55faa2a3fb33fd2d1e055a07a7c61ecfb8d80ec00c2c9eb1241047ea6e3a4487037a9e0dbd79262b2cc273e779930fc18409ac5361c5fe669d702e147790aeb4ce7fd6575ab0f6c7fd1c335939aa863ba37ec91b7e32bb013bb2b410409f85b3d20ebd7885ce464c08d056d6428fe4dd9287aa365f131f4360ff386d846898bc4b41583c2a5197f65d78742746c12a5ec0a4ffe2f270a750a1d8fb51600":MBEDTLS_ERR_ECP_INVALID_KEY
read_round_one:MBEDTLS_ECJPAKE_CLIENT:"4104190a07700ffa4be6ae1d79ee0f06aeb544cd5addaabedf70f8623321332c54f355f0fbfec783ed359e5d0bf7377a0fc4ea7ace473c9c112b41ccd41ac56a56124104360a1cea33fce641156458e0a4eac219e96831e6aebc88b3f3752f93a0281d1bf1fb106051db9694a8d6e862a5ef1324a3d9e27894f1ee4f7c59199965a8dd4a2091847d2d22df3ee55faa2a3fb33fd2d1e055a07a7c61ecfb8d80ec00c2c9eb1241047ea6e3a4487037a9e0dbd79262b2cc273e779930fc18409ac5361c5fe669d702e147790aeb4ce7fd6575ab0f6c7fd1c335939aa863ba37ec91b7e32bb013bb2b410409f85b3d20ebd7885ce464c08d056d6428fe4dd9287aa365f131f4360ff386d846898bc4b41583c2a5197f65d78742746c12a5ec0a4ffe2f270a750a1d8fb51600":MBEDTLS_ERR_ECP_BAD_INPUT_DATA

ECJPAKE round one: KKP2: no data for r
read_round_one:MBEDTLS_ECJPAKE_CLIENT:"4104190a07700ffa4be6ae1d79ee0f06aeb544cd5addaabedf70f8623321332c54f355f0fbfec783ed359e5d0bf7377a0fc4ea7ace473c9c112b41ccd41ac56a56124104360a1cea33fce641156458e0a4eac219e96831e6aebc88b3f3752f93a0281d1bf1fb106051db9694a8d6e862a5ef1324a3d9e27894f1ee4f7c59199965a8dd4a2091847d2d22df3ee55faa2a3fb33fd2d1e055a07a7c61ecfb8d80ec00c2c9eb1241047ea6e3a4487037a9e0dbd79262b2cc273e779930fc18409ac5361c5fe669d702e147790aeb4ce7fd6575ab0f6c7fd1c335939aa863ba37ec91b7e32bb013bb2b410409f85b3d20ebd7885ce464c08d056d6428fe4dd9287aa365f131f4360ff386d846898bc4b41583c2a5197f65d78742746c12a5ec0a4ffe2f270a750a1d8fb51601":MBEDTLS_ERR_ECP_BAD_INPUT_DATA
Expand Down Expand Up @@ -170,7 +170,7 @@ ECJPAKE round two client: nothing after second point
read_round_two_cli:"03001741040fb22b1d5d1123e0ef9feb9d8a2e590a1f4d7ced2c2b06586e8f2a16d4eb2fda4328a20b07d8fd667654ca18c54e32a333a0845451e926ee8804fd7af0aaa7a641045516ea3e54a0d5d8b2ce786b38d383370029a5dbe4459c9dd601b408a24ae6465c8ac905b9eb03b5d3691c139ef83f1cd4200f6c9cd4ec392218a59ed243d3c8":MBEDTLS_ERR_ECP_BAD_INPUT_DATA

ECJPAKE round two client: zero-length r
read_round_two_cli:"03001741040fb22b1d5d1123e0ef9feb9d8a2e590a1f4d7ced2c2b06586e8f2a16d4eb2fda4328a20b07d8fd667654ca18c54e32a333a0845451e926ee8804fd7af0aaa7a641045516ea3e54a0d5d8b2ce786b38d383370029a5dbe4459c9dd601b408a24ae6465c8ac905b9eb03b5d3691c139ef83f1cd4200f6c9cd4ec392218a59ed243d3c800":MBEDTLS_ERR_ECP_INVALID_KEY
read_round_two_cli:"03001741040fb22b1d5d1123e0ef9feb9d8a2e590a1f4d7ced2c2b06586e8f2a16d4eb2fda4328a20b07d8fd667654ca18c54e32a333a0845451e926ee8804fd7af0aaa7a641045516ea3e54a0d5d8b2ce786b38d383370029a5dbe4459c9dd601b408a24ae6465c8ac905b9eb03b5d3691c139ef83f1cd4200f6c9cd4ec392218a59ed243d3c800":MBEDTLS_ERR_ECP_BAD_INPUT_DATA

ECJPAKE round two client: no data for r
read_round_two_cli:"03001741040fb22b1d5d1123e0ef9feb9d8a2e590a1f4d7ced2c2b06586e8f2a16d4eb2fda4328a20b07d8fd667654ca18c54e32a333a0845451e926ee8804fd7af0aaa7a641045516ea3e54a0d5d8b2ce786b38d383370029a5dbe4459c9dd601b408a24ae6465c8ac905b9eb03b5d3691c139ef83f1cd4200f6c9cd4ec392218a59ed243d3c801":MBEDTLS_ERR_ECP_BAD_INPUT_DATA
Expand Down Expand Up @@ -224,7 +224,7 @@ ECJPAKE round two server: nothing after second point
read_round_two_srv:"410469d54ee85e90ce3f1246742de507e939e81d1dc1c5cb988b58c310c9fdd9524d93720b45541c83ee8841191da7ced86e3312d43623c1d63e74989aba4affd1ee4104077e8c31e20e6bedb760c13593e69f15be85c27d68cd09ccb8c4183608917c5c3d409fac39fefee82f7292d36f0d23e055913f45a52b85dd8a2052e9e129bb4d":MBEDTLS_ERR_ECP_BAD_INPUT_DATA

ECJPAKE round two server: zero-length r
read_round_two_srv:"410469d54ee85e90ce3f1246742de507e939e81d1dc1c5cb988b58c310c9fdd9524d93720b45541c83ee8841191da7ced86e3312d43623c1d63e74989aba4affd1ee4104077e8c31e20e6bedb760c13593e69f15be85c27d68cd09ccb8c4183608917c5c3d409fac39fefee82f7292d36f0d23e055913f45a52b85dd8a2052e9e129bb4d00":MBEDTLS_ERR_ECP_INVALID_KEY
read_round_two_srv:"410469d54ee85e90ce3f1246742de507e939e81d1dc1c5cb988b58c310c9fdd9524d93720b45541c83ee8841191da7ced86e3312d43623c1d63e74989aba4affd1ee4104077e8c31e20e6bedb760c13593e69f15be85c27d68cd09ccb8c4183608917c5c3d409fac39fefee82f7292d36f0d23e055913f45a52b85dd8a2052e9e129bb4d00":MBEDTLS_ERR_ECP_BAD_INPUT_DATA

ECJPAKE round two server: no data for r
read_round_two_srv:"410469d54ee85e90ce3f1246742de507e939e81d1dc1c5cb988b58c310c9fdd9524d93720b45541c83ee8841191da7ced86e3312d43623c1d63e74989aba4affd1ee4104077e8c31e20e6bedb760c13593e69f15be85c27d68cd09ccb8c4183608917c5c3d409fac39fefee82f7292d36f0d23e055913f45a52b85dd8a2052e9e129bb4d20":MBEDTLS_ERR_ECP_BAD_INPUT_DATA
Expand Down