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

[Hans Leidekker]

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.

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.