Hi folks,
last week I described a problem with RSA blinding and its locking[1].
Coincidentally another user ran into the same problem in a totally different
scenario, the work-around of disabling the blinding fixed the issue for
him[2].
So here's a patch to fix this issue.
The issue is that the rsa_get_blinding method checks whether there is
concurrent access to the blinding and tells its caller whether locking needs
to be done. The check is done by comparing thread/process IDs. If they differ
locking needs to be done. The problem is that locking is not done if the IDs
are same. It is assumed that the creator doesn't need to do locking which is
correct for a single-threaded application but is wrong in a multi-
threaded/processed environment once another process has accessed the blinding.
As a fix I've introduced a new flag for struct rsa_st,
RSA_FLAG_BLINDING_NEEDS_LOCKING. Once it has been noticed that locking has to
be done the flag is set and from then on we always do locking, whether we are
the creator or not.
However, I'm scratching my head why this fixes the issue given the fact that
there are actually two blindings: one when doing no locking (rsa->blinding)
and another one when doing locking (rsa->mt_blinding). Yet our load test
indicates that this really does fix the issue: without it we get about a dozen
Bad Record MAC per day due to the blinding. Since I've implemented the
attached patch none has occured (which is about 1.5 days of load test now).
Bye,
Marc
[1] - http://marc.info/?l=openssl-dev&m=123754568501758&w=2
[2] - http://marc.info/?l=openssl-users&m=123788810006272&w=2
--
Marc Haisenko
Team Leader and Senior Developer
Comdasys AG
Rüdesheimer Str. 7
80686 München
Germany
Tel.: +49 (0)89 548 433 321
diff -purd openssl-0.9.8k.orig/crypto/rsa/rsa_eay.c openssl-0.9.8k/crypto/rsa/rsa_eay.c
--- openssl-0.9.8k.orig/crypto/rsa/rsa_eay.c 2008-09-16 16:55:26.000000000 +0200
+++ openssl-0.9.8k/crypto/rsa/rsa_eay.c 2009-03-30 12:46:02.000000000 +0200
@@ -273,9 +273,16 @@
if (ret == NULL)
goto err;
- if (BN_BLINDING_get_thread_id(ret) == CRYPTO_thread_id())
+ if (rsa->flags & RSA_FLAG_BLINDING_NEEDS_LOCK)
{
- /* rsa->blinding is ours! */
+ /* We already noticed that we have to do locking. */
+ *local = 0;
+ }
+ else if (BN_BLINDING_get_thread_id(ret) == CRYPTO_thread_id())
+ {
+ /* rsa->blinding is ours and there haven't been any other
+ * threads trying to access the blinding
+ */
*local = 1;
}
@@ -289,6 +296,13 @@ static BN_BLINDING *rsa_get_blinding(RSA
* stored outside the BN_BLINDING
*/
+ /* Set flag so we don't need to do this test any more. */
+ rsa->flags |= RSA_FLAG_BLINDING_NEEDS_LOCK;
+ }
+
+ if (!(*local))
+ {
+ /* Set up the shared blinding. */
if (rsa->mt_blinding == NULL)
{
if (!got_write_lock)
diff -purd openssl-0.9.8k.orig/crypto/rsa/rsa.h openssl-0.9.8k/crypto/rsa/rsa.h
--- openssl-0.9.8k.orig/crypto/rsa/rsa.h 2008-09-16 16:55:26.000000000 +0200
+++ openssl-0.9.8k/crypto/rsa/rsa.h 2009-03-30 12:37:28.000000000 +0200
@@ -236,6 +236,10 @@
* be used for all exponents.
*/
#endif
+#define RSA_FLAG_BLINDING_NEEDS_LOCK 0x0200 /* new with 0.9.8l; once we notice that the
+ * blinding needs to have a lock we set this
+ * flag to always use locking.
+ */
#define RSA_PKCS1_PADDING 1
