From 88fca3ef0e497a64d39ee40766e8b780beaba914 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 27 Mar 2015 15:06:07 +0100 Subject: [PATCH] Fix thread safety issue in RSA operations The race was due to mpi_exp_mod storing a Montgomery coefficient in the context (RM, RP, RQ). The fix was verified with -fsanitize-thread using ssl_pthread_server and two concurrent clients. A more fine-grained fix should be possible, locking just enough time to check if those values are OK and set them if not, rather than locking for the whole mpi_exp_mod() operation, but it will be for later. --- ChangeLog | 1 + library/rsa.c | 14 +++++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index c9f2e1005..2c6869695 100644 --- a/ChangeLog +++ b/ChangeLog @@ -25,6 +25,7 @@ Features errors on use of deprecated functions. Bugfix + * Fix thread safety bug in RSA operations (found by Fredrik Axelsson). * Fix hardclock() (only used in the benchmarking program) with some versions of mingw64 (found by kxjhlele). * Fix warnings from mingw64 in timing.c (found by kxjklele). diff --git a/library/rsa.c b/library/rsa.c index 0d71ad0c8..f45b23444 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -282,11 +282,18 @@ int rsa_public( rsa_context *ctx, return( POLARSSL_ERR_RSA_BAD_INPUT_DATA ); } +#if defined(POLARSSL_THREADING_C) + polarssl_mutex_lock( &ctx->mutex ); +#endif + olen = ctx->len; MPI_CHK( mpi_exp_mod( &T, &T, &ctx->E, &ctx->N, &ctx->RN ) ); MPI_CHK( mpi_write_binary( &T, output, olen ) ); cleanup: +#if defined(POLARSSL_THREADING_C) + polarssl_mutex_unlock( &ctx->mutex ); +#endif mpi_free( &T ); @@ -400,6 +407,10 @@ int rsa_private( rsa_context *ctx, MPI_CHK( mpi_mod_mpi( &T, &T, &ctx->N ) ); } +#if defined(POLARSSL_THREADING_C) + polarssl_mutex_lock( &ctx->mutex ); +#endif + #if defined(POLARSSL_RSA_NO_CRT) MPI_CHK( mpi_exp_mod( &T, &T, &ctx->D, &ctx->N, &ctx->RN ) ); #else @@ -440,10 +451,11 @@ int rsa_private( rsa_context *ctx, MPI_CHK( mpi_write_binary( &T, output, olen ) ); cleanup: - mpi_free( &T ); mpi_free( &T1 ); mpi_free( &T2 ); #if defined(POLARSSL_THREADING_C) + polarssl_mutex_unlock( &ctx->mutex ); mpi_free( &Vi_copy ); mpi_free( &Vf_copy ); #endif + mpi_free( &T ); mpi_free( &T1 ); mpi_free( &T2 ); if( ret != 0 ) return( POLARSSL_ERR_RSA_PRIVATE_FAILED + ret );