net/srtp: teardown salt hygiene and fail-closed cipher checks - #445
Merged
Conversation
The per-session salt is derived from the master key; zero it when a stream state is torn down rather than leaving it in freed heap, matching how the key-derivation scratch is already cleared. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The RTP and RTCP encrypt/decrypt paths ignored the return of r_crypto_cipher_encrypt / _decrypt. A cipher that cannot service the call — for instance an AEAD cipher reaching this stream-cipher path — returns without touching the buffer, which would emit or accept an unprotected payload. Treat a non-OK result as R_SRTP_ERROR_INTERNAL so the transform produces no output instead, matching how the other internal errors on these paths are handled. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
1 participant
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.
Two small SRTP hardening fixes surfaced by the security audit of the RFC 6904 header-extension work (#444). Independent of each other; one commit each.
Clear the session salt on teardown
The per-session salt is derived from the master key, but
r_srtp_state_clearfreed the stream state without zeroing it. Clear it at teardown so it is not left in freed heap, matching how the key-derivation scratch is already wiped.Fail closed if the payload cipher rejects the request
The RTP and RTCP encrypt/decrypt paths ignored the return value of
r_crypto_cipher_encrypt/r_crypto_cipher_decrypt. A cipher that cannot service the call — for example an AEAD cipher reaching this stream-cipher path — returns without touching the buffer, which would emit or accept an unprotected payload. All four call sites now treat a non-OK result asR_SRTP_ERROR_INTERNALand produce no output, matching how the other internal errors on these paths are handled.No current cipher suite triggers this (the defined suites are all counter-mode and return OK), so the guards are latent by design and cannot be exercised by a unit test without a synthetic failing cipher; the existing tests confirm they do not disturb the working path.
Testing
Full suite green on the linux and ASan tiers; both tiers compile warning-free.