powered by Jive Software

Bug - Concurrency issues with blowfish implementation

When multiple threads perform encryption/decryption on the same Blowfish object (as is the case with the Blowfish object supplied by the AuthFactory), the results may come out wrong.

The issue rises from the fact that the Blowfish encryption and decryption aren’'t atomic operations and thus threads may interfere with each other, causing unexpected results.

The problem with this implementation is that access to the m_bfish member isn’'t synchronized so that threads may alter its internal state while encryption/decryption is being performed on another thread.

This code in the Blowfish’'s encStr method demonstrates the issue:


.

.

.

// create the encryptor

m_bfish.setCBCIV(lNewCBCIV);

// Another thread may execute here, altering the CBCIV and affecting the result of this
// thread’'s encryption operation. // encrypt the buffer

m_bfish.encrypt(buf);

.

.

.


This issue also applies to the decryptString method.

One solution to this problem could be to synchronize the access to the m_bfish object during encryption/decryption:


synchronized (m_bfish)

{

// create the encryptor

m_bfish.setCBCIV(lNewCBCIV);

// encrypt the buffer

m_bfish.encrypt(buf);

}


as well as declaring all methods of the BlowfishCBC class that alter the CBCIV (the m_lCBCIV member) synchronized.

You could also synchronize on a higher level, like the Blowfish’‘s encryptString/decryptString methods, or even in the AuthFactory’'s

encryptPassword/decryptPassword methods.

Please note that this is a very serious issue with the encrypted passwords which prevents it to work as expected and should be fixed a.s.a.p.

Is there a more efficient way to report a defect / issue instead of this forum?

Hey Daniel,

Thanks for the bug report. I created JM-1035 and checked in a fix for this problem. In fact, a also made some changes that could potentially improve performance too.

Thanks,

– Gato

Thank you for your fast action!