From 88fed8e700399f55053d520d54d5ff9f7eaf7081 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Tue, 12 Apr 2022 09:03:22 +0100 Subject: [PATCH 1/7] Rewrite ecdh_curve25519 program Rewrite the example ECDH x25519 program using the high-level ECDH API. Signed-off-by: Thomas Daubney --- programs/pkey/ecdh_curve25519.c | 128 ++++++++++++++------------------ 1 file changed, 56 insertions(+), 72 deletions(-) diff --git a/programs/pkey/ecdh_curve25519.c b/programs/pkey/ecdh_curve25519.c index ca046fd241..e435b03501 100644 --- a/programs/pkey/ecdh_curve25519.c +++ b/programs/pkey/ecdh_curve25519.c @@ -30,12 +30,12 @@ #define MBEDTLS_EXIT_FAILURE EXIT_FAILURE #endif /* MBEDTLS_PLATFORM_C */ -#if !defined(MBEDTLS_ECDH_C) || !defined(MBEDTLS_ECDH_LEGACY_CONTEXT) || \ +#if !defined(MBEDTLS_ECDH_C) || \ !defined(MBEDTLS_ECP_DP_CURVE25519_ENABLED) || \ !defined(MBEDTLS_ENTROPY_C) || !defined(MBEDTLS_CTR_DRBG_C) int main( void ) { - mbedtls_printf( "MBEDTLS_ECDH_C and/or MBEDTLS_ECDH_LEGACY_CONTEXT and/or " + mbedtls_printf( "MBEDTLS_ECDH_C and/or " "MBEDTLS_ECP_DP_CURVE25519_ENABLED and/or " "MBEDTLS_ENTROPY_C and/or MBEDTLS_CTR_DRBG_C " "not defined\n" ); @@ -47,6 +47,8 @@ int main( void ) #include "mbedtls/ctr_drbg.h" #include "mbedtls/ecdh.h" +#include + int main( int argc, char *argv[] ) { @@ -55,8 +57,13 @@ int main( int argc, char *argv[] ) mbedtls_ecdh_context ctx_cli, ctx_srv; mbedtls_entropy_context entropy; mbedtls_ctr_drbg_context ctr_drbg; - unsigned char cli_to_srv[32], srv_to_cli[32]; + unsigned char cli_to_srv[36], srv_to_cli[33]; const char pers[] = "ecdh"; + + size_t olen; + unsigned char secret_cli[32], secret_srv[32]; + const unsigned char *p_cli_to_srv = cli_to_srv; + ((void) argc); ((void) argv); @@ -67,15 +74,17 @@ int main( int argc, char *argv[] ) /* * Initialize random number generation */ - mbedtls_printf( " . Seeding the random number generator..." ); + mbedtls_printf( " . Seed the random number generator..." ); fflush( stdout ); mbedtls_entropy_init( &entropy ); - if( ( ret = mbedtls_ctr_drbg_seed( &ctr_drbg, mbedtls_entropy_func, &entropy, - (const unsigned char *) pers, - sizeof pers ) ) != 0 ) + if( ( ret = mbedtls_ctr_drbg_seed( &ctr_drbg, mbedtls_entropy_func, + &entropy, + (const unsigned char *) pers, + sizeof pers ) ) != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_ctr_drbg_seed returned %d\n", ret ); + mbedtls_printf( " failed\n ! mbedtls_ctr_drbg_seed returned %d\n", + ret ); goto exit; } @@ -84,28 +93,23 @@ int main( int argc, char *argv[] ) /* * Client: initialize context and generate keypair */ - mbedtls_printf( " . Setting up client context..." ); + mbedtls_printf( " . Set up client context, generate EC key pair..." ); fflush( stdout ); - ret = mbedtls_ecp_group_load( &ctx_cli.MBEDTLS_PRIVATE(grp), MBEDTLS_ECP_DP_CURVE25519 ); + ret = mbedtls_ecdh_setup( &ctx_cli, MBEDTLS_ECP_DP_CURVE25519 ); if( ret != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_ecp_group_load returned %d\n", ret ); + mbedtls_printf( " failed\n ! mbedtls_ecdh_setup returned %d\n", ret ); goto exit; } - ret = mbedtls_ecdh_gen_public( &ctx_cli.MBEDTLS_PRIVATE(grp), &ctx_cli.MBEDTLS_PRIVATE(d), &ctx_cli.MBEDTLS_PRIVATE(Q), - mbedtls_ctr_drbg_random, &ctr_drbg ); + ret = mbedtls_ecdh_make_params( &ctx_cli, &olen, cli_to_srv, + sizeof( cli_to_srv ), + mbedtls_ctr_drbg_random, &ctr_drbg ); if( ret != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_ecdh_gen_public returned %d\n", ret ); - goto exit; - } - - ret = mbedtls_mpi_write_binary( &ctx_cli.MBEDTLS_PRIVATE(Q).MBEDTLS_PRIVATE(X), cli_to_srv, 32 ); - if( ret != 0 ) - { - mbedtls_printf( " failed\n ! mbedtls_mpi_write_binary returned %d\n", ret ); + mbedtls_printf( " failed\n ! mbedtls_ecdh_make_params returned %d\n", + ret ); goto exit; } @@ -114,90 +118,70 @@ int main( int argc, char *argv[] ) /* * Server: initialize context and generate keypair */ - mbedtls_printf( " . Setting up server context..." ); + mbedtls_printf( " . Server: read params, generate public key..." ); fflush( stdout ); - ret = mbedtls_ecp_group_load( &ctx_srv.MBEDTLS_PRIVATE(grp), MBEDTLS_ECP_DP_CURVE25519 ); + ret = mbedtls_ecdh_read_params( &ctx_srv, &p_cli_to_srv, + p_cli_to_srv + sizeof( cli_to_srv ) ); if( ret != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_ecp_group_load returned %d\n", ret ); + mbedtls_printf( " failed\n ! mbedtls_ecdh_read_params returned %d\n", + ret ); goto exit; } - ret = mbedtls_ecdh_gen_public( &ctx_srv.MBEDTLS_PRIVATE(grp), &ctx_srv.MBEDTLS_PRIVATE(d), &ctx_srv.MBEDTLS_PRIVATE(Q), - mbedtls_ctr_drbg_random, &ctr_drbg ); + ret = mbedtls_ecdh_make_public( &ctx_srv, &olen, srv_to_cli, + sizeof( srv_to_cli ), + mbedtls_ctr_drbg_random, &ctr_drbg ); if( ret != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_ecdh_gen_public returned %d\n", ret ); - goto exit; - } - - ret = mbedtls_mpi_write_binary( &ctx_srv.MBEDTLS_PRIVATE(Q).MBEDTLS_PRIVATE(X), srv_to_cli, 32 ); - if( ret != 0 ) - { - mbedtls_printf( " failed\n ! mbedtls_mpi_write_binary returned %d\n", ret ); + mbedtls_printf( " failed\n ! mbedtls_ecdh_make_params returned %d\n", + ret ); goto exit; } mbedtls_printf( " ok\n" ); /* - * Server: read peer's key and generate shared secret + * Client: read public key */ - mbedtls_printf( " . Server reading client key and computing secret..." ); + mbedtls_printf( " . Client: read public key..." ); fflush( stdout ); - ret = mbedtls_mpi_lset( &ctx_srv.MBEDTLS_PRIVATE(Qp).MBEDTLS_PRIVATE(Z), 1 ); + ret = mbedtls_ecdh_read_public( &ctx_cli, srv_to_cli, + sizeof( srv_to_cli ) ); if( ret != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_mpi_lset returned %d\n", ret ); - goto exit; - } - - ret = mbedtls_mpi_read_binary( &ctx_srv.MBEDTLS_PRIVATE(Qp).MBEDTLS_PRIVATE(X), cli_to_srv, 32 ); - if( ret != 0 ) - { - mbedtls_printf( " failed\n ! mbedtls_mpi_read_binary returned %d\n", ret ); - goto exit; - } - - ret = mbedtls_ecdh_compute_shared( &ctx_srv.MBEDTLS_PRIVATE(grp), &ctx_srv.MBEDTLS_PRIVATE(z), - &ctx_srv.MBEDTLS_PRIVATE(Qp), &ctx_srv.MBEDTLS_PRIVATE(d), - mbedtls_ctr_drbg_random, &ctr_drbg ); - if( ret != 0 ) - { - mbedtls_printf( " failed\n ! mbedtls_ecdh_compute_shared returned %d\n", ret ); + mbedtls_printf( " failed\n ! mbedtls_ecdh_read_public returned %d\n", + ret ); goto exit; } mbedtls_printf( " ok\n" ); /* - * Client: read peer's key and generate shared secret + * Calculate secrets */ - mbedtls_printf( " . Client reading server key and computing secret..." ); + mbedtls_printf( " . Calculate secrets..." ); fflush( stdout ); - ret = mbedtls_mpi_lset( &ctx_cli.MBEDTLS_PRIVATE(Qp).MBEDTLS_PRIVATE(Z), 1 ); + ret = mbedtls_ecdh_calc_secret( &ctx_cli, &olen, secret_cli, + sizeof( secret_cli ), + mbedtls_ctr_drbg_random, &ctr_drbg ); if( ret != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_mpi_lset returned %d\n", ret ); + mbedtls_printf( " failed\n ! mbedtls_ecdh_calc_secret returned %d\n", + ret ); goto exit; } - ret = mbedtls_mpi_read_binary( &ctx_cli.MBEDTLS_PRIVATE(Qp).MBEDTLS_PRIVATE(X), srv_to_cli, 32 ); + ret = mbedtls_ecdh_calc_secret( &ctx_srv, &olen, secret_srv, + sizeof( secret_srv ), + mbedtls_ctr_drbg_random, &ctr_drbg ); if( ret != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_mpi_read_binary returned %d\n", ret ); - goto exit; - } - - ret = mbedtls_ecdh_compute_shared( &ctx_cli.MBEDTLS_PRIVATE(grp), &ctx_cli.MBEDTLS_PRIVATE(z), - &ctx_cli.MBEDTLS_PRIVATE(Qp), &ctx_cli.MBEDTLS_PRIVATE(d), - mbedtls_ctr_drbg_random, &ctr_drbg ); - if( ret != 0 ) - { - mbedtls_printf( " failed\n ! mbedtls_ecdh_compute_shared returned %d\n", ret ); + mbedtls_printf( " failed\n ! mbedtls_ecdh_calc_secret returned %d\n", + ret ); goto exit; } @@ -206,13 +190,13 @@ int main( int argc, char *argv[] ) /* * Verification: are the computed secrets equal? */ - mbedtls_printf( " . Checking if both computed secrets are equal..." ); + mbedtls_printf( " . Check if both calculated secrets are equal..." ); fflush( stdout ); - ret = mbedtls_mpi_cmp_mpi( &ctx_cli.MBEDTLS_PRIVATE(z), &ctx_srv.MBEDTLS_PRIVATE(z) ); + ret = memcmp( secret_srv, secret_cli, sizeof( secret_srv ) ); if( ret != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_ecdh_compute_shared returned %d\n", ret ); + mbedtls_printf( " failed\n ! Shared secrets not equal.\n" ); goto exit; } From 64042b8d3d6c03a8110fafe2a1836ad5fe7084c8 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Wed, 18 May 2022 09:59:55 +0100 Subject: [PATCH 2/7] Fix typo Fix typo that was caught during review. Signed-off-by: Thomas Daubney --- programs/pkey/ecdh_curve25519.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/programs/pkey/ecdh_curve25519.c b/programs/pkey/ecdh_curve25519.c index e435b03501..690f9687f7 100644 --- a/programs/pkey/ecdh_curve25519.c +++ b/programs/pkey/ecdh_curve25519.c @@ -135,7 +135,7 @@ int main( int argc, char *argv[] ) mbedtls_ctr_drbg_random, &ctr_drbg ); if( ret != 0 ) { - mbedtls_printf( " failed\n ! mbedtls_ecdh_make_params returned %d\n", + mbedtls_printf( " failed\n ! mbedtls_ecdh_make_public returned %d\n", ret ); goto exit; } From ec2ec42828b001eacdb615692eef7b921bf73f8d Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Wed, 18 May 2022 10:23:20 +0100 Subject: [PATCH 3/7] Fix formatting Line up function parameters. Signed-off-by: Thomas Daubney --- programs/pkey/ecdh_curve25519.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/programs/pkey/ecdh_curve25519.c b/programs/pkey/ecdh_curve25519.c index 690f9687f7..017d01cded 100644 --- a/programs/pkey/ecdh_curve25519.c +++ b/programs/pkey/ecdh_curve25519.c @@ -79,9 +79,9 @@ int main( int argc, char *argv[] ) mbedtls_entropy_init( &entropy ); if( ( ret = mbedtls_ctr_drbg_seed( &ctr_drbg, mbedtls_entropy_func, - &entropy, - (const unsigned char *) pers, - sizeof pers ) ) != 0 ) + &entropy, + (const unsigned char *) pers, + sizeof pers ) ) != 0 ) { mbedtls_printf( " failed\n ! mbedtls_ctr_drbg_seed returned %d\n", ret ); From 306a89094a12237ec26e74a72e3e64e2314c8a0a Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Wed, 18 May 2022 14:22:08 +0100 Subject: [PATCH 4/7] Add additional error checking Initialise client and server secret buffers and check their lengths. Signed-off-by: Thomas Daubney --- programs/pkey/ecdh_curve25519.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/programs/pkey/ecdh_curve25519.c b/programs/pkey/ecdh_curve25519.c index 017d01cded..555a2a0bb7 100644 --- a/programs/pkey/ecdh_curve25519.c +++ b/programs/pkey/ecdh_curve25519.c @@ -61,7 +61,8 @@ int main( int argc, char *argv[] ) const char pers[] = "ecdh"; size_t olen; - unsigned char secret_cli[32], secret_srv[32]; + unsigned char secret_cli[32] = { 0 }; + unsigned char secret_srv[32] = { 0 }; const unsigned char *p_cli_to_srv = cli_to_srv; ((void) argc); @@ -175,6 +176,8 @@ int main( int argc, char *argv[] ) goto exit; } + size_t secret_cli_olen = olen; + ret = mbedtls_ecdh_calc_secret( &ctx_srv, &olen, secret_srv, sizeof( secret_srv ), mbedtls_ctr_drbg_random, &ctr_drbg ); @@ -185,6 +188,8 @@ int main( int argc, char *argv[] ) goto exit; } + size_t secret_srv_olen = olen; + mbedtls_printf( " ok\n" ); /* @@ -193,8 +198,8 @@ int main( int argc, char *argv[] ) mbedtls_printf( " . Check if both calculated secrets are equal..." ); fflush( stdout ); - ret = memcmp( secret_srv, secret_cli, sizeof( secret_srv ) ); - if( ret != 0 ) + ret = memcmp( secret_srv, secret_cli, sizeof( secret_srv_olen ) ); + if( ret != 0 || ( secret_cli_olen != secret_srv_olen ) ) { mbedtls_printf( " failed\n ! Shared secrets not equal.\n" ); goto exit; From eff0f3f5be519ca9bb7244fdec33d5784c2e76d7 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Wed, 18 May 2022 14:36:45 +0100 Subject: [PATCH 5/7] Add changelog entry Add changelog entry for bug fix in sample program. Signed-off-by: Thomas Daubney --- ChangeLog.d/fix-x25519-program.txt | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 ChangeLog.d/fix-x25519-program.txt diff --git a/ChangeLog.d/fix-x25519-program.txt b/ChangeLog.d/fix-x25519-program.txt new file mode 100644 index 0000000000..af60465b54 --- /dev/null +++ b/ChangeLog.d/fix-x25519-program.txt @@ -0,0 +1,4 @@ +Bugfix + * Fix a bug in x25519 example program where the removal of + MBEDTLS_ECDH_LEGACY_CONTEXT caused the program not to run. Fixes #4901 and + #3191. From 70c008823941dd4313b5c2649838173bcd2994ea Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Fri, 20 May 2022 18:43:09 +0100 Subject: [PATCH 6/7] Change use of olen variables Removed olen variable in favour of storing olens for client and server separately. Signed-off-by: Thomas Daubney --- programs/pkey/ecdh_curve25519.c | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/programs/pkey/ecdh_curve25519.c b/programs/pkey/ecdh_curve25519.c index 555a2a0bb7..50c4ad85e2 100644 --- a/programs/pkey/ecdh_curve25519.c +++ b/programs/pkey/ecdh_curve25519.c @@ -60,7 +60,8 @@ int main( int argc, char *argv[] ) unsigned char cli_to_srv[36], srv_to_cli[33]; const char pers[] = "ecdh"; - size_t olen; + size_t srv_olen; + size_t cli_olen; unsigned char secret_cli[32] = { 0 }; unsigned char secret_srv[32] = { 0 }; const unsigned char *p_cli_to_srv = cli_to_srv; @@ -104,7 +105,7 @@ int main( int argc, char *argv[] ) goto exit; } - ret = mbedtls_ecdh_make_params( &ctx_cli, &olen, cli_to_srv, + ret = mbedtls_ecdh_make_params( &ctx_cli, &cli_olen, cli_to_srv, sizeof( cli_to_srv ), mbedtls_ctr_drbg_random, &ctr_drbg ); if( ret != 0 ) @@ -131,7 +132,7 @@ int main( int argc, char *argv[] ) goto exit; } - ret = mbedtls_ecdh_make_public( &ctx_srv, &olen, srv_to_cli, + ret = mbedtls_ecdh_make_public( &ctx_srv, &srv_olen, srv_to_cli, sizeof( srv_to_cli ), mbedtls_ctr_drbg_random, &ctr_drbg ); if( ret != 0 ) @@ -166,7 +167,7 @@ int main( int argc, char *argv[] ) mbedtls_printf( " . Calculate secrets..." ); fflush( stdout ); - ret = mbedtls_ecdh_calc_secret( &ctx_cli, &olen, secret_cli, + ret = mbedtls_ecdh_calc_secret( &ctx_cli, &cli_olen, secret_cli, sizeof( secret_cli ), mbedtls_ctr_drbg_random, &ctr_drbg ); if( ret != 0 ) @@ -176,9 +177,7 @@ int main( int argc, char *argv[] ) goto exit; } - size_t secret_cli_olen = olen; - - ret = mbedtls_ecdh_calc_secret( &ctx_srv, &olen, secret_srv, + ret = mbedtls_ecdh_calc_secret( &ctx_srv, &srv_olen, secret_srv, sizeof( secret_srv ), mbedtls_ctr_drbg_random, &ctr_drbg ); if( ret != 0 ) @@ -188,8 +187,6 @@ int main( int argc, char *argv[] ) goto exit; } - size_t secret_srv_olen = olen; - mbedtls_printf( " ok\n" ); /* @@ -198,8 +195,8 @@ int main( int argc, char *argv[] ) mbedtls_printf( " . Check if both calculated secrets are equal..." ); fflush( stdout ); - ret = memcmp( secret_srv, secret_cli, sizeof( secret_srv_olen ) ); - if( ret != 0 || ( secret_cli_olen != secret_srv_olen ) ) + ret = memcmp( secret_srv, secret_cli, sizeof( srv_olen ) ); + if( ret != 0 || ( cli_olen != srv_olen ) ) { mbedtls_printf( " failed\n ! Shared secrets not equal.\n" ); goto exit; From 413550c5298af349627597689e4f95b622dd6c9d Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Mon, 23 May 2022 16:11:31 +0100 Subject: [PATCH 7/7] Change memcmp call Previous call used sizeof() function which is not needed. Signed-off-by: Thomas Daubney --- programs/pkey/ecdh_curve25519.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/programs/pkey/ecdh_curve25519.c b/programs/pkey/ecdh_curve25519.c index 50c4ad85e2..6230981229 100644 --- a/programs/pkey/ecdh_curve25519.c +++ b/programs/pkey/ecdh_curve25519.c @@ -195,7 +195,7 @@ int main( int argc, char *argv[] ) mbedtls_printf( " . Check if both calculated secrets are equal..." ); fflush( stdout ); - ret = memcmp( secret_srv, secret_cli, sizeof( srv_olen ) ); + ret = memcmp( secret_srv, secret_cli, srv_olen ); if( ret != 0 || ( cli_olen != srv_olen ) ) { mbedtls_printf( " failed\n ! Shared secrets not equal.\n" );