crypto: document which cipher.Block implementations are safe for concurrent use

[FiloSottile]

Many of them are, but since it's undocumented it might change from one architecture to the next.

This came up in https://go-review.googlesource.com/c/crypto/+/118535 because x/crypto/xts effectively assumes they all are.

[nodo]

Hey @FiloSottile ! Are you OK if I help with this issue? Do you have any pointers?

[FiloSottile]

@nodo Absolutely! It's a matter of going through the implementations (mainly AES and DES) and checking that they don't write to object state in Encrypt / Decrypt. You can also use the race detector to help, but some implementations are in assembly, so it will not work everywhere.

[nodo]

Hey @FiloSottile,

I was checking the packages, would it be enough to verify the files below?

• https://github.com/golang/go/blob/master/src/crypto/aes/gcm_amd64.s
• https://github.com/golang/go/blob/master/src/crypto/aes/block.go
• https://github.com/golang/go/blob/master/src/crypto/des/block.go

[FiloSottile]

There are a few other implementations for other platforms in those packages that you'll have to check. By the way, this is about cipher.Block, so you don't have to check the GCM mode, which is a cipher.AEAD implementation.

[anitgandhi]

I think the same should apply to some of the other interfaces and implementations. For example, cipher.BlockMode, and specifically the CBCEncrypter/Decrypter types, have iv as a shared state, so it's not concurrent safe. This is equivalent to the XTS tweak situation.

I can open a separate issue for this if it makes more sense to do so.

[FiloSottile]

In general nothing is concurrent safe until explicitly marked. cipher.Block was confusing because they mostly (all) are and that assumption had made its way into places. If you think it would be worth explicitly documenting other implementations, feel free to open another issue, though.

[nodo]

@FiloSottile Apologies for the delay, I have spent some time trying to figure out how things are connected together.

I have had a look at AES in pure go. It looks to me that https://github.com/golang/go/blob/master/src/crypto/aes/block.go#L40 and https://github.com/golang/go/blob/master/src/crypto/aes/block.go#L85 do not access shared state and are safe for concurrent use.

I have also written a quick test to be run with the race detector: https://gist.github.com/nodo/e7beb7dfa1b5495310c27f607d72cdc5

Do you think this is an acceptable proof that aes in pure go is safe for concurrent use? If that is the case I can use the same approach for other cipher.Block implementations.