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

Improve ECDSA verify validation #6190

Merged
merged 14 commits into from
Oct 31, 2022
Merged

Improve ECDSA verify validation #6190

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
1fdb8e8
Add unit tests for ECDSA with invalid public key
daverodgman Aug 10, 2022
c947751
Fix ECDSA signature verification edge-case
daverodgman Aug 10, 2022
56cfb31
Add missing newlines
daverodgman Aug 10, 2022
66bcadb
Fix duplicate test names
daverodgman Aug 10, 2022
d106308
Changelog for ECDSA verify fix
daverodgman Aug 10, 2022
b44c728
Minor tidy-up
daverodgman Aug 11, 2022
cd57a61
Paramaterise ecdsa_verify test case more clearly
daverodgman Aug 18, 2022
b153c6c
Improve test descriptions
daverodgman Aug 18, 2022
34ff6a7
Correct documentation for mbedtls_ecdsa_verify
daverodgman Aug 19, 2022
5d7a235
Simplify test for ecdsa_verify
daverodgman Aug 19, 2022
683850b
Update tests/suites/test_suite_ecdsa.function
daverodgman Oct 27, 2022
5d13e5e
Improve changelog for ECDSA verify fix
daverodgman Oct 27, 2022
ce48c92
Credit Cryptofuzz in the changelog
daverodgman Oct 27, 2022
f00466e
Build fix - remove line of dead code
daverodgman Oct 28, 2022
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
5 changes: 5 additions & 0 deletions ChangeLog.d/ecdsa-verify-fixes.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Bugfix
* Fix ECDSA verification, where it was failing to validate the public key
for signatures where R == 1 and S == 1. This bug meant that it was
gilles-peskine-arm marked this conversation as resolved.
Show resolved Hide resolved
possible to verify such signatures with an invalid public key, in some
cases.
gilles-peskine-arm marked this conversation as resolved.
Show resolved Hide resolved
4 changes: 1 addition & 3 deletions include/mbedtls/ecdsa.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,10 +245,8 @@ int mbedtls_ecdsa_sign_det_ext( mbedtls_ecp_group *grp, mbedtls_mpi *r,
* 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,
Expand Down
3 changes: 3 additions & 0 deletions library/ecp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2666,14 +2666,17 @@ static int mbedtls_ecp_mul_shortcuts( mbedtls_ecp_group *grp,

if( mbedtls_mpi_cmp_int( m, 0 ) == 0 )
{
MBEDTLS_MPI_CHK( mbedtls_ecp_check_pubkey( grp, P ) );
tom-cosgrove-arm marked this conversation as resolved.
Show resolved Hide resolved
gilles-peskine-arm marked this conversation as resolved.
Show resolved Hide resolved
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 );
}
Expand Down
60 changes: 60 additions & 0 deletions tests/suites/test_suite_ecdsa.data
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,3 +361,63 @@ ecdsa_prim_test_vectors:MBEDTLS_ECP_DP_SECP521R1:"0":"0151518F1AF0F563517EDD5485
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
40 changes: 40 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 @@ -466,3 +466,43 @@ exit:
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;
const mbedtls_ecp_curve_info *curve_info;
tom-cosgrove-arm marked this conversation as resolved.
Show resolved Hide resolved

mbedtls_ecdsa_init( &ctx );
mbedtls_mpi_init( &sig_r );
mbedtls_mpi_init( &sig_s );

/* Prepare ECP group context */
curve_info = mbedtls_ecp_curve_info_from_grp_id( grp_id );
TEST_ASSERT( curve_info != NULL );
TEST_EQUAL( mbedtls_ecp_group_load( &ctx.grp, curve_info->grp_id ), 0 );
daverodgman marked this conversation as resolved.
Show resolved Hide resolved

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