From 9f58c4b6e5cfaab2fcd67ef517c5398512a008fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 25 Jun 2020 12:34:58 +0200 Subject: [PATCH 1/7] DHM: make drawing of blinding value a function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the next commit, we'll need to draw a second random value, in order to blind modular inversion. Having a function for that will avoid repetition. Signed-off-by: Manuel Pégourié-Gonnard --- library/dhm.c | 41 ++++++++++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/library/dhm.c b/library/dhm.c index 392ed0c15..6df40f13b 100644 --- a/library/dhm.c +++ b/library/dhm.c @@ -324,6 +324,32 @@ cleanup: return( 0 ); } +/* + * Pick a random R in the range [2, M) for blinding purposes + */ +static int dhm_random_below( mbedtls_mpi *R, const mbedtls_mpi *M, + int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ) +{ + int ret, count; + + count = 0; + do + { + MBEDTLS_MPI_CHK( mbedtls_mpi_fill_random( R, mbedtls_mpi_size( M ), f_rng, p_rng ) ); + + while( mbedtls_mpi_cmp_mpi( R, M ) >= 0 ) + MBEDTLS_MPI_CHK( mbedtls_mpi_shift_r( R, 1 ) ); + + if( count++ > 10 ) + return( MBEDTLS_ERR_MPI_NOT_ACCEPTABLE ); + } + while( mbedtls_mpi_cmp_int( R, 1 ) <= 0 ); + +cleanup: + return( ret ); +} + + /* * Use the blinding method and optimisation suggested in section 10 of: * KOCHER, Paul C. Timing attacks on implementations of Diffie-Hellman, RSA, @@ -333,7 +359,7 @@ cleanup: static int dhm_update_blinding( mbedtls_dhm_context *ctx, int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ) { - int ret, count; + int ret; /* * Don't use any blinding the first time a particular X is used, @@ -368,18 +394,7 @@ static int dhm_update_blinding( mbedtls_dhm_context *ctx, */ /* Vi = random( 2, P-1 ) */ - count = 0; - do - { - MBEDTLS_MPI_CHK( mbedtls_mpi_fill_random( &ctx->Vi, mbedtls_mpi_size( &ctx->P ), f_rng, p_rng ) ); - - while( mbedtls_mpi_cmp_mpi( &ctx->Vi, &ctx->P ) >= 0 ) - MBEDTLS_MPI_CHK( mbedtls_mpi_shift_r( &ctx->Vi, 1 ) ); - - if( count++ > 10 ) - return( MBEDTLS_ERR_MPI_NOT_ACCEPTABLE ); - } - while( mbedtls_mpi_cmp_int( &ctx->Vi, 1 ) <= 0 ); + MBEDTLS_MPI_CHK( dhm_random_below( &ctx->Vi, &ctx->P, f_rng, p_rng ) ); /* Vf = Vi^-X mod P */ MBEDTLS_MPI_CHK( mbedtls_mpi_inv_mod( &ctx->Vf, &ctx->Vi, &ctx->P ) ); From af72167f40a97958a6157418cbd1fda29119b3f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 25 Jun 2020 12:47:22 +0200 Subject: [PATCH 2/7] DHM: blind call to mpi_inv_mod() on secret value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/dhm.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/library/dhm.c b/library/dhm.c index 6df40f13b..f9c55892a 100644 --- a/library/dhm.c +++ b/library/dhm.c @@ -360,6 +360,9 @@ static int dhm_update_blinding( mbedtls_dhm_context *ctx, int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ) { int ret; + mbedtls_mpi R; + + mbedtls_mpi_init( &R ); /* * Don't use any blinding the first time a particular X is used, @@ -396,11 +399,21 @@ static int dhm_update_blinding( mbedtls_dhm_context *ctx, /* Vi = random( 2, P-1 ) */ MBEDTLS_MPI_CHK( dhm_random_below( &ctx->Vi, &ctx->P, f_rng, p_rng ) ); - /* Vf = Vi^-X mod P */ - MBEDTLS_MPI_CHK( mbedtls_mpi_inv_mod( &ctx->Vf, &ctx->Vi, &ctx->P ) ); + /* Vf = Vi^-X mod P + * First compute Vi^-1 = R * (R Vi)^-1, (avoiding leaks from inv_mod), + * then elevate to the Xth power. */ + MBEDTLS_MPI_CHK( dhm_random_below( &R, &ctx->P, f_rng, p_rng ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( &ctx->Vf, &ctx->Vi, &R ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &ctx->Vf, &ctx->Vf, &ctx->P ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_inv_mod( &ctx->Vf, &ctx->Vf, &ctx->P ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( &ctx->Vf, &ctx->Vf, &R ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &ctx->Vf, &ctx->Vf, &ctx->P ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_exp_mod( &ctx->Vf, &ctx->Vf, &ctx->X, &ctx->P, &ctx->RP ) ); cleanup: + mbedtls_mpi_free( &R ); + return( ret ); } From b3e3d79e1a2524b2d66d8c3c251ca099f2ce7abe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 26 Jun 2020 11:03:19 +0200 Subject: [PATCH 3/7] RSA: remove redundant GCD call in prepare_blinding() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit inv_mod() already returns a specific error code if the value is not invertible, so no need to check in advance that it is. Also, this is a preparation for blinding the call to inv_mod(), which is made easier by avoiding the redundancy (otherwise the call to gcd() would need to be blinded too). Signed-off-by: Manuel Pégourié-Gonnard --- library/rsa.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/library/rsa.c b/library/rsa.c index 6c457468e..00b379c2b 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -800,11 +800,14 @@ static int rsa_prepare_blinding( mbedtls_rsa_context *ctx, return( MBEDTLS_ERR_RSA_RNG_FAILED ); MBEDTLS_MPI_CHK( mbedtls_mpi_fill_random( &ctx->Vf, ctx->len - 1, f_rng, p_rng ) ); - MBEDTLS_MPI_CHK( mbedtls_mpi_gcd( &ctx->Vi, &ctx->Vf, &ctx->N ) ); - } while( mbedtls_mpi_cmp_int( &ctx->Vi, 1 ) != 0 ); + + ret = mbedtls_mpi_inv_mod( &ctx->Vi, &ctx->Vf, &ctx->N ); + if( ret != 0 && ret != MBEDTLS_ERR_MPI_NOT_ACCEPTABLE ) + goto cleanup; + + } while( ret == MBEDTLS_ERR_MPI_NOT_ACCEPTABLE ) /* Blinding value: Vi = Vf^(-e) mod N */ - MBEDTLS_MPI_CHK( mbedtls_mpi_inv_mod( &ctx->Vi, &ctx->Vf, &ctx->N ) ); MBEDTLS_MPI_CHK( mbedtls_mpi_exp_mod( &ctx->Vi, &ctx->Vi, &ctx->E, &ctx->N, &ctx->RN ) ); From 750d3c76cbc1071774a7efd75d578c4b798d7cb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 26 Jun 2020 11:19:12 +0200 Subject: [PATCH 4/7] RSA: blind call to mpi_inv_mod() on secret value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/rsa.c | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/library/rsa.c b/library/rsa.c index 00b379c2b..782df036b 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -782,6 +782,9 @@ static int rsa_prepare_blinding( mbedtls_rsa_context *ctx, int (*f_rng)(void *, unsigned char *, size_t), void *p_rng ) { int ret, count = 0; + mbedtls_mpi R; + + mbedtls_mpi_init( &R ); if( ctx->Vf.p != NULL ) { @@ -801,17 +804,32 @@ static int rsa_prepare_blinding( mbedtls_rsa_context *ctx, MBEDTLS_MPI_CHK( mbedtls_mpi_fill_random( &ctx->Vf, ctx->len - 1, f_rng, p_rng ) ); - ret = mbedtls_mpi_inv_mod( &ctx->Vi, &ctx->Vf, &ctx->N ); - if( ret != 0 && ret != MBEDTLS_ERR_MPI_NOT_ACCEPTABLE ) + /* Compute the Vf^1 as R * (R Vf)^-1 to avoid leaks from inv_mod. + * There's a negligible but non-zero probability that R is not + * invertible mod N, in that case we'd just loop one more time, + * just as if Vf itself wasn't invertible - no need to distinguish. */ + MBEDTLS_MPI_CHK( mbedtls_mpi_fill_random( &R, ctx->len - 1, f_rng, p_rng ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( &ctx->Vi, &ctx->Vf, &R ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &ctx->Vi, &ctx->Vi, &ctx->N ) ); + + ret = mbedtls_mpi_inv_mod( &ctx->Vi, &ctx->Vi, &ctx->N ); + if( ret == MBEDTLS_ERR_MPI_NOT_ACCEPTABLE ) + continue; + if( ret != 0 ) goto cleanup; - } while( ret == MBEDTLS_ERR_MPI_NOT_ACCEPTABLE ) + MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( &ctx->Vi, &ctx->Vi, &R ) ); + MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &ctx->Vi, &ctx->Vi, &ctx->N ) ); + } while( 0 ); - /* Blinding value: Vi = Vf^(-e) mod N */ + /* Blinding value: Vi = Vf^(-e) mod N + * (Vi already contains Vf^-1 at this point) */ MBEDTLS_MPI_CHK( mbedtls_mpi_exp_mod( &ctx->Vi, &ctx->Vi, &ctx->E, &ctx->N, &ctx->RN ) ); cleanup: + mbedtls_mpi_free( &R ); + return( ret ); } From 34c1e7d0696a52f71801d6f196f75a6ac2bc8e6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 26 Jun 2020 11:33:41 +0200 Subject: [PATCH 5/7] Add ChangeLog entry for base blinding protection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- ChangeLog.d/protect-base-blinding.txt | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 ChangeLog.d/protect-base-blinding.txt diff --git a/ChangeLog.d/protect-base-blinding.txt b/ChangeLog.d/protect-base-blinding.txt new file mode 100644 index 000000000..ca0600cee --- /dev/null +++ b/ChangeLog.d/protect-base-blinding.txt @@ -0,0 +1,6 @@ +Security + * Fix side channel in RSA private key operations and static (finite-field) + Diffie-Hellman. An adversary with precise enough timing and memory access + information (typically an untrusted operating system attacking a secure + enclave) could bypass an existing counter-measure (base blinding) and + potentially fully recover the private key. From e288ec06515c80ef4c8d9048683d8d296d97a99d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 16 Jul 2020 09:23:30 +0200 Subject: [PATCH 6/7] Fix memory leak on error path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/rsa.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/library/rsa.c b/library/rsa.c index 782df036b..1f53708b2 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -800,7 +800,10 @@ static int rsa_prepare_blinding( mbedtls_rsa_context *ctx, /* Unblinding value: Vf = random number, invertible mod N */ do { if( count++ > 10 ) - return( MBEDTLS_ERR_RSA_RNG_FAILED ); + { + ret = MBEDTLS_ERR_RSA_RNG_FAILED; + goto cleanup; + } MBEDTLS_MPI_CHK( mbedtls_mpi_fill_random( &ctx->Vf, ctx->len - 1, f_rng, p_rng ) ); From 7868396e78fdf761f0e7fce231aff4d4c30899a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 16 Jul 2020 09:48:54 +0200 Subject: [PATCH 7/7] Clarify some comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/rsa.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/library/rsa.c b/library/rsa.c index 1f53708b2..e6f863b00 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -807,25 +807,27 @@ static int rsa_prepare_blinding( mbedtls_rsa_context *ctx, MBEDTLS_MPI_CHK( mbedtls_mpi_fill_random( &ctx->Vf, ctx->len - 1, f_rng, p_rng ) ); - /* Compute the Vf^1 as R * (R Vf)^-1 to avoid leaks from inv_mod. - * There's a negligible but non-zero probability that R is not - * invertible mod N, in that case we'd just loop one more time, - * just as if Vf itself wasn't invertible - no need to distinguish. */ + /* Compute Vf^-1 as R * (R Vf)^-1 to avoid leaks from inv_mod. */ MBEDTLS_MPI_CHK( mbedtls_mpi_fill_random( &R, ctx->len - 1, f_rng, p_rng ) ); MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( &ctx->Vi, &ctx->Vf, &R ) ); MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &ctx->Vi, &ctx->Vi, &ctx->N ) ); + /* At this point, Vi is invertible mod N if and only if both Vf and R + * are invertible mod N. If one of them isn't, we don't need to know + * which one, we just loop and choose new values for both of them. + * (Each iteration succeeds with overwhelming probability.) */ ret = mbedtls_mpi_inv_mod( &ctx->Vi, &ctx->Vi, &ctx->N ); if( ret == MBEDTLS_ERR_MPI_NOT_ACCEPTABLE ) continue; if( ret != 0 ) goto cleanup; + /* Finish the computation of Vf^-1 = R * (R Vf)^-1 */ MBEDTLS_MPI_CHK( mbedtls_mpi_mul_mpi( &ctx->Vi, &ctx->Vi, &R ) ); MBEDTLS_MPI_CHK( mbedtls_mpi_mod_mpi( &ctx->Vi, &ctx->Vi, &ctx->N ) ); } while( 0 ); - /* Blinding value: Vi = Vf^(-e) mod N + /* Blinding value: Vi = Vf^(-e) mod N * (Vi already contains Vf^-1 at this point) */ MBEDTLS_MPI_CHK( mbedtls_mpi_exp_mod( &ctx->Vi, &ctx->Vi, &ctx->E, &ctx->N, &ctx->RN ) );