search

eduvpn / apple

app for iOS and macOS
Other
61 stars 18 forks source link

TunnelKit: Queue the data packets to avoid crypto errors #484

Closed roop closed 2 years ago

roop commented 2 years ago

Fixes #481.

When sending data packets, the encryption currently happens in the NEPacketTunnelFlow's queue. The OpenVPN ping packets are sent from the OpenVPNSession object's queue. This results in encryptions happening in parallel, especially when sending lots of packets in a short time. Since both kinds of packets use the same crypto context, this results in the same crypto context being accessed from two different threads at the same time.

Consider the code for encryptBytes. If two threads are simultaneously executing that method, and both thread have finished EVP_CipherInit, then after one thread exits EVP_CipherFinal_ex, the other thread's EVP_CipherFinal_ex call fails because the same IV can't be reused. (EVP_CipherFinal_ex() calls aes_gcm_cipher() here when using AES-256-GCM cipher). The OpenSSL API is not designed to be used from different threads on the same context.

So the fix is to make the data packets' encryption also happen in the OpenVPNSession object's queue.

roop commented 2 years ago

@jeroenleenarts Thanks for reviewing it while it was a draft. I've added info on how it's related to the crypto error in the description now.