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.
This commit is contained in:
Manuel Pégourié-Gonnard 2015-03-27 15:06:07 +01:00
parent 39ead3ef2f
commit 88fca3ef0e
2 changed files with 14 additions and 1 deletions

View file

@ -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).

View file

@ -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 );