srtp: locate the MKI correctly for RCC mode 1 packets without a tag - #821
Merged
Conversation
With RFC 4771 RCC mode 1 and an MKI, every packet that did not carry the ROC failed srtp_unprotect with bad_mki: srtp_protect appends no authentication tag to those packets, but the key lookup still stepped a full tag length back from the packet end before reading the MKI. srtp_get_session_keys_for_rtp_packet now takes the packet's rcc_carry state and derives what trails the MKI from it: the 4-octet ROC of a mode 3 ROC-carrying AES-GCM packet, nothing for a mode 1 packet without the ROC, the full tag otherwise. The ad-hoc mode 3 subtraction in srtp_unprotect folds into that one place. Non-RCC streams compute exactly what they did before. rcc_mode1_rate4_mki_untagged pins it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
Author
|
AI code review rather than testing. I build and run an automated review loop, so I am constantly pointing it at real repositories to evaluate and improve the method, and libsrtp came up because the RCC and MKI mechanics are exactly the kind of state dependent layout I have been working to make it better at. It flagged the key lookup stepping back a full tag length on a mode 1 packet that srtp_protect never tagged. I confirmed it, wrote the test that fails on main with bad_mki, and checked the derivation against RFC 4771 before filing. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
With RFC 4771 RCC mode 1 and an MKI, every packet that does not carry the ROC failed
srtp_unprotectwithbad_mki:srtp_protectappends no authentication tag to those packets, but the key lookup still stepped a full tag length back from the packet end before reading the MKI.srtp_get_session_keys_for_rtp_packetnow takes the packet'srcc_carrystate and derives what trails the MKI from it: the 4-octet ROC of a mode 3 ROC-carrying AES-GCM packet, nothing for a mode 1 packet without the ROC, the full tag otherwise. The ad-hoc mode 3 subtraction insrtp_unprotectfolds into that one place. Non-RCC streams compute exactly what they did before.The new test
rcc_mode1_rate4_mki_untaggedfails on main withbad_mkiand passes here;format.shis clean.