Skip to content

Fix bidirectional keystream/nonce reuse in HSv5 encryption - #3387

Open
cl-ment wants to merge 6 commits into
Haivision:masterfrom
cl-ment:fix/bidir-keystream-reuse
Open

cl-ment wants to merge 6 commits into
Haivision:masterfrom
cl-ment:fix/bidir-keystream-reuse

Conversation

@cl-ment

@cl-ment cl-ment commented Sep 29, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

In HSv5, both directions of a connection used the same SEK and salt. The responder cloned its RX context into TX, and the initiator applied its own TX key to RX. Both directions also started from the same ISN, and the IV is derived only from  (salt, packet seqno) . Packets with the same sequence number in each direction were therefore encrypted with the same keystream:  C_A ⊕ C_B = P_A ⊕ P_B . This breaks confidentiality in both AES-CTR and AES-GCM, and in GCM it also weakens authentication. The issue affects any application sending data in both directions (e.g.  srt-tunnel ).

Solution: a separate key per direction

• New capability  SRT_OPT_SECDIST  (bit 8 of the HSREQ/HSRSP SRT flags). The KM message format is unchanged.
• Both peers support it. The responder generates its own TX key and salt, and returns its own KM in the KMRSP instead of echoing the KMREQ. The initiator unwraps that KM with the shared passphrase and installs it as its RX key.
• A KMRSP that cannot be unwrapped yields  SRT_KM_S_BADSECRET .
• A KMRSP that is not an echo, from a peer that did not declare the capability, is rejected.
• Repeated handshake responses are recognized and ignored.
• Peer is an older version (no  SRT_OPT_SECDIST ). The newer side immediately performs a forced key refresh after the connection: new key in the alternate (odd/even) slot, sent via KMREQ.
• Data submitted by the application is still accepted into the sender buffer, but no data packet is sent until the peer acknowledges the new key with a KMRSP.
• Packets are encrypted with the new key only when sent. They are never sent in clear, nor with the shared key.
• Only a full sender buffer blocks  srt_sendmsg , or returns  SRT_EASYNCSND  in non-blocking mode, as usual.
• If the peer does not acknowledge within  SRTO_CONNTIMEO , the connection is broken.
• Old ↔ old behavior is unchanged.

Implementation notes

•  haicrypt : new  HaiCrypt_Tx_ForceRefresh()  and  HaiCrypt_Tx_ForceSwitch() , which prepare a key in the alternate slot and activate it immediately.
•  CCryptoControl : a forced-refresh state machine ( NONE / NEEDED / PENDING / DONE / FAILED ), driven from  checkSndTimers() . Data sending stays gated in  NEEDED ,  PENDING  and  FAILED .
•  dev  only. Data is encrypted when it is added to the sender buffer, so packets added while the refresh is pending are stored in clear. They are flagged  m_bEncryptPending  and encrypted when extracted for sending. On  master , encryption already happens at send time.

Tests

• New  test/test_crypto_bidir.cpp :
• Unit tests for independent keys, older initiator/responder, bad secret, and a non-echo KMRSP without the capability, for both CTR and GCM.
• End-to-end caller/listener transmission through a UDP relay that can hide the capability. Both sides send in non-blocking mode right after connecting. The test checks that no sequence number shares a keystream between the two directions, and that data flows without  SRT_EASYNCSND .
• Negative check: disabling the fix makes the tests fail with identical ciphertexts.
• Full unit test suite passes with bonding enabled.
• Interop against v1.5.7 passes in every combination (new/old × caller/listener, blocking and non-blocking).
• On  dev , the rendezvous cases are disabled. Bidirectional rendezvous transfer currently breaks there with  SEQUENCE DISCREPANCY , even without encryption; that is unrelated to this change.

Documentation

•  docs/features/encryption.md : key separation and backward-compatibility behavior.
•  docs/features/handshake.md :  SRT_OPT_SECDIST  flag.

Compatibility

• No wire format change.
• New ↔ new peers use separate keys directly from the handshake.
• New ↔ old peers pay about one extra RTT before the first data packet leaves.
• Old ↔ old peers are unchanged and remain vulnerable. Applications that need bidirectional data with older peers can use two unidirectional connections with different passphrases.

@codecov

codecov Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.76845% with 52 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.13%. Comparing base (0976f76) to head (e4530c0).
⚠️ Report is 70 commits behind head on master.

Files with missing lines Patch % Lines
srtcore/crypto.cpp 66.66% 17 Missing and 13 partials ⚠️
test/test_crypto_bidir.cpp 95.01% 7 Missing and 6 partials ⚠️
haicrypt/hcrypt_tx.c 79.31% 2 Missing and 4 partials ⚠️
srtcore/core.cpp 57.14% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3387      +/-   ##
==========================================
- Coverage   67.88%   65.13%   -2.76%     
==========================================
  Files         101      104       +3     
  Lines       18489    20588    +2099     
  Branches        0     3025    +3025     
==========================================
+ Hits        12551    13409     +858     
+ Misses       5938     5821     -117     
- Partials        0     1358    +1358     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread srtcore/core.cpp
if (m_pCryptoControl)
{
m_pCryptoControl->sendKeysToPeer(this, SRTT());

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please mark the equivalent SSS_* flag for 1.6.0 in the comment.

@ethouris ethouris added Type: Maintenance Work required to maintain or clean up the code [core] Area: Changes in SRT library core labels Oct 7, 2026
@ethouris ethouris added this to the v1.5.8 milestone Oct 7, 2026
ethouris added a commit that referenced this pull request Oct 23, 2026
* Adapt PR 3387 for independent encryption keys

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@cl-ment
cl-ment requested a review from ethouris November 5, 2026 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[core] Area: Changes in SRT library core Type: Maintenance Work required to maintain or clean up the code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants