commit 88fca3ef0e497a64d39ee40766e8b780beaba914
author Manuel Pégourié-Gonnard <mpg@elzevir.fr> Fri Mar 27 15:06:07 2015 +0100
committer Manuel Pégourié-Gonnard <mpg@elzevir.fr> Fri Mar 27 15:12:05 2015 +0100
tree 91b2dd78f031816f964956654e6ba96bf53b40cb
parent 39ead3ef2f7513352c415136737c2c8a00ccb4ac

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.

library/rsa.c

diff --git a/library/rsa.c b/library/rsa.c
index 0d71ad0..f45b234 100644
--- a/library/rsa.c
+++ b/library/rsa.c
@@ -282,11 +282,18 @@
         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 @@
         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 @@
     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 );