crypto/cipher: NewCTR is too expensive for small runs

[jech]

Hi,

SRTP, the security mechanism used by WebRTC, encrypts each packet individually with a different IV. Current implementations use AES in CTR mode, with the IV derived from the packet sequence number, the track's identifier (SSRC), and the phase of the moon. The Pion WebRTC library implements this by calling cipher.NewCTR for each packet. This turns out to be one of he main sources of memory allocations in the library.

Pull request pion/srtp#76 works around the problem by implementing a function that performs the equivalent of cipher.NewCTR followed by XORKeyStream without allocating the Stream structure. However, the Pion maintainers appear to believe that the client library is not the right place to perform this kind of manipulation.

I humbly suggest that the library should include a function

func findAGoodName(block cipher.Block, iv []byte, dst, src []byte)

which performs the equivalent of NewCTR followed by XORKeyStream. An alternative would be to add a function

func (ctr *ctr) Reset(block Block, iv []byte)

which resets a Stream to its initial state without reallocating the buffer. Of course, due to backwards compatibility, this would not be added to the existing Stream interface, the client would need to check for it explicitly.

[ianlancetaylor]

CC @FiloSottile

[ernado]

The AES-IGE mode is used in telegram:

cipher, err := aes.NewCipher(key[:]) // allocation
if err != nil {
	return nil, err
}
plaintext := make([]byte, len(data))
d := ige.NewIGEDecrypter(cipher, iv[:])
d.CryptBlocks(plaintext, data)

Telegram uses different iv and key for each message.
We can't reuse Cipher because there are no Reset method.

Should I create another issue for this?

[jech]

@FiloSottile Any chance you could consider this, now that the tree is open again?

[starius]

I think, that CTR mode is slow on small runs, because it precalculates 512 bytes:
https://github.com/golang/go/blob/go1.22.0/src/crypto/cipher/ctr.go#L28

Can the size of precalculated buffer be set to BlockSize? Method refill() fills the buffer with BlockSize runs of Encrypt() method anyway. If streamBufferSize is always set to BlockSize, if would improve performance of small runs (e.g. 16 byte) significantly, but should not damage performance of large runs.

[starius]

I made an experiment: reduced streamBufferSize from 512 to 32 bytes. It caused ~5% slowdown.

diff --git a/src/crypto/cipher/ctr.go b/src/crypto/cipher/ctr.go
index eac8e266cf..9b93f43c69 100644
--- a/src/crypto/cipher/ctr.go
+++ b/src/crypto/cipher/ctr.go
@@ -25,7 +25,7 @@ type ctr struct {
        outUsed int
 }
 
-const streamBufferSize = 512
+const streamBufferSize = 32
 
 // ctrAble is an interface implemented by ciphers that have a specific optimized
 // implementation of CTR, like crypto/aes. NewCTR will check for this interface

Comparison of benckmarks:

$ benchstat /tmp/old.txt /tmp/new.txt
goos: linux
goarch: amd64
pkg: crypto/cipher
cpu: AMD EPYC Processor (with IBPB)
           │ /tmp/old.txt │            /tmp/new.txt             │
           │    sec/op    │   sec/op     vs base                │
AESCTR1K-3    1.485µ ± 0%   1.559µ ± 1%  +4.98% (p=0.000 n=100)
AESCTR8K-3    11.84µ ± 0%   12.46µ ± 1%  +5.25% (p=0.000 n=100)
geomean       4.193µ        4.407µ       +5.11%

           │ /tmp/old.txt │         /tmp/new.txt         │
           │     B/s      │     B/s       vs base        │
AESCTR1K-3   654.2Mi ± 0%   623.4Mi ± 1%  -4.72% (n=100)
AESCTR8K-3   659.6Mi ± 0%   626.7Mi ± 1%  -4.99% (n=100)
geomean      656.9Mi        625.0Mi       -4.85%

[jech]

Now that 57d0551 has landed, it's fairly easy to work around the issue. Please see https://github.com/pion/srtp/blob/master/crypto.go.

[gopherbot]

Change https://go.dev/cl/621958 mentions this issue: crypto/aes: speedup CTR mode on AMD64 and ARM64

[gopherbot]

Change https://go.dev/cl/621957 mentions this issue: crypto/cipher: add small CTR benchmark, remove CFB/OFB benchmarks