[PATCH 1/2] bcrypt: Synchronize symmetric key access for encrypt and decrypt.

[Paul Gofman]

On 4/20/21 17:54, Hans Leidekker wrote:
> On Tue, 2021-04-20 at 16:19 +0300, Paul Gofman wrote:
>> On 4/20/21 16:13, Hans Leidekker wrote:
>>> On Tue, 2021-04-20 at 13:48 +0300, Paul Gofman wrote:
>>>> Signed-off-by: Paul Gofman <pgofman at codeweavers.com>
>>>> ---
>>>>     Forza Horizon 4 crashes during entering multiplayer mode due to BCryptEncrypt and
>>>>     BCryptDecrypt being used from different threads with the same key using MODE_ID_GCM.
>>>>     Most of the time that is either glinc free() assertion or segfault inside gnutls function.
>>>>
>>>>     While bcrypt functions are probably not thread safe in general on Windows, I suppose
>>>>     the native implementation does not modify any key data in encrypt and decrypt
>>>>     in this mode.
>>> Are you referring to the late initialization of the GnuTLS cipher or
>>> does this happen entirely within GnuTLS code?
>>>
>>>
>> The exact race manifestation might be different, wgat I observed in the
>> real game happens from key_symmetric_set_vector(). Where is can be
>> either working with key->u.s.vector in key_symmetric_set_vector itself
>> (I think that is what is most readily reproduce by my test in the next
>> patch, which always gets the wrong results and only sometimes crashes),
>> or race in gnutls_cipher_deinit() called from 
>> key_symmetric_vector_reset (exhibiting itself as a segfault inside
>> gnutls code or assertion from glibc's free). But I think the whole path
>> with the (temporary) key update is racy, including
>> key_symmetric_set_auth_data().
> Unfortunately we can't avoid reinitializing the cipher.

That's what I assumed and thus suggested to just sync the access in such
scenario.


>
> It turns out there's support for decryption with asymmetric keys but we
> failed to remove the FIXME and error return from BCryptDecrypt. I fixed
> that and rebased your patch.
>
>
Thanks.