DTLS decryption threaded #8284
DTLS decryption threaded #8284
Conversation
SparkiDev
force-pushed
the
dtls_decrypt_threaded
branch
from
3b76118 to
4fdda82
Compare
Add support for decryption in threads for DTLS.
SparkiDev
force-pushed
the
dtls_decrypt_threaded
branch
3 times, most recently
from
a68f776 to
dbb5103
Compare
Split out encryption in software for TLSv13. Call software encryption in async encrypt. Support ChaCha20-Poly1305.
SparkiDev
force-pushed
the
dtls_decrypt_threaded
branch
7 times, most recently
from
97e745f to
ae9ce19
Compare
Split out decryption in software for TLSv13. Call software decryption in async decrypt. Support ChaCha20-Poly1305.
SparkiDev
force-pushed
the
dtls_decrypt_threaded
branch
from
ae9ce19 to
ed2606e
Compare
| if (ret == 0 || ret == WC_NO_ERR_TRACE(WANT_WRITE)) | ||
| if (ret == 0 || ret == WC_NO_ERR_TRACE(WANT_WRITE)) { | ||
| #ifdef WOLFSSL_RW_THREADED | ||
| int lockRet = wc_LockMutex(&ssl->dtls13Rtx.mutex); | ||
| if (lockRet < 0) { | ||
| return lockRet; | ||
| } | ||
| #endif | ||
| Dtls13RtxAddRecord(&ssl->dtls13Rtx, rtxRecord); | ||
| #ifdef WOLFSSL_RW_THREADED | ||
| wc_UnLockMutex(&ssl->dtls13Rtx.mutex); | ||
| #endif | ||
| } |
There was a problem hiding this comment.
I still think its better to use a high level lock. That will avoid any other possible races as well as handle other post-hs messages.
| static WC_INLINE void BuildTls13Nonce(WOLFSSL* ssl, byte* nonce, const byte* iv, | ||
| int order) | ||
| #ifndef WOLFSSL_THREADED_CRYPT | ||
| static WC_INLINE | ||
| #endif | ||
| void BuildTls13Nonce(WOLFSSL* ssl, byte* nonce, const byte* iv, int order) |
There was a problem hiding this comment.
Why not just make this never static?
rizlik
force-pushed
the
dtls_decrypt_threaded
branch
2 times, most recently
from
548f028 to
ed2606e
Compare
|
sorry, force pushed by mistake while reviewing, restored to the original commit |
There was a problem hiding this comment.
This includes the review of commit e4a661f as this PR is strictly related.
These are some high level comments:
-
Thread Keys Not Updated After Key Update
Thread keys are set inSendDatathe first time a thread is used but are never updated after a key update. This desynchronization leads to errors during decryption.
I created a small unit test here: https://github.com/rizlik/wolfssl/blob/68d5cfbed6f2bd18a88b2ee5654015bb6598519c/tests/api/test_dtls.c#L604 that should reproduce the issue. -
Changed Return Value Semantic of
wolfSSL_write
The return value semantics ofwolfSSL_writechanged, returning a size even if no data is sent, also the size returned is not equal to the plaintext size. This may apply to decryption side as well. As the application already needs to do some work to setup the threads, it can be an idea to use a separate API for threaded encryption/decryption. -
Unit Test Failures with
WOLFSSL_THREADED_CRYPT
EnablingWOLFSSL_THREADED_CRYPTcauses multiple unit tests to fail. -
Lack of Testing
These are two very complex features (RW_THREADED and THREADED_CRYPT), but no tests have been added and the current tests are failing. -
Lingering Data Sending from Threads
After the application calls wolfSSL_write(), threads run and encrypt the data. How should the application trigger the sending of the data when ready? Should it use a 0-sized write? If so, it's better to document it. Overall, it might be beneficial to add a new API. -
Unclear Feature Scope (DTLS vs. TLS 1.3)
It's unclear if the feature is only for DTLS or also for TLS 1.3. The code should be modularized better if it's only for DTLS. If it can be used also for TLS 1.3, then the sending and encryption order needs to be maintained AFAIU. -
Handling Busy Threads
If all threads are busy, the current implementation might loop indefinitely. Suggestion: ReturnWANT_WRITEinstead. -
Signal Field in
ThreadCryptStruct
There is a signal field perThreadCryptstruct, but only a global signal can be set. Suggestion: Either drop the field or allow enabling each thread individually. -
Return Values of
wolfSSL_Async*API
ThewolfSSL_Async*API should returnWOLFSSL_SUCCESSinstead of zero. -
Lingering Messages in
WOLFSSL_RW_THREADED
There are problems with stale messages if the application is not continuously callingwolfSSL_read()/wolfSSL_write(). Post-handshake messages may remain stale and not be sent.
DTLS Decryption Threaded
This PR adds support for DTLS decryption in separate threads, allowing parallel processing of encrypted packets.
Link to Devin run: https://app.devin.ai/sessions/47c568f4eabb4feea977a16db6ec8682