Skip to content

fix(media): skip SRTP encryption when remote peer has no crypto#130

Merged
emiago merged 2 commits into
emiago:mainfrom
HectorMalot:fix/sdes-srtp-plaintext-peer
Feb 27, 2026
Merged

fix(media): skip SRTP encryption when remote peer has no crypto#130
emiago merged 2 commits into
emiago:mainfrom
HectorMalot:fix/sdes-srtp-plaintext-peer

Conversation

@HectorMalot
Copy link
Copy Markdown
Contributor

Fixes #129

When SecureRTP=1 (SDES), LocalSDP() unconditionally created an SRTP context causing encrypted outbound RTP toward peers expecting plaintext.

  • Add hasRemoteSDP flag so answerer only includes crypto when the offerer actually offered SRTP
  • Clear localCtxSRTP in offerer path when remote answer has no crypto

Also added a small unrelated fix in the same code:

  • Fix TrimLeft→TrimPrefix for crypto tag parsing

…o#129

When SecureRTP=1 (SDES), LocalSDP() unconditionally created an SRTP
context causing encrypted outbound RTP toward peers expecting plaintext.

- Add hasRemoteSDP flag so answerer only includes crypto when the
  offerer actually offered SRTP
- Clear localCtxSRTP in offerer path when remote answer has no crypto
- Fix TrimLeft→TrimPrefix for crypto tag parsing

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment thread media/media_session.go Outdated
masterKey, masterSalt := keysalt[:keyLen], keysalt[keyLen:]
// RFC 4568/8643: only include crypto when offering (no remote SDP yet)
// or when the peer actually offered SRTP
if !s.hasRemoteSDP || s.remoteCtxSRTP != nil {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Hi @HectorMalot . Simpler change would be just check would be like if s.RemoteAddr != nil and not have additional hasRemoteSDP flag. Generally this is safest as remoteadd is only known after getting offer.
You could move this under this anonymous func and return nil, instead shifting. This is just my request not to have breaks with future code comming, but not required really.

@emiago
Copy link
Copy Markdown
Owner

emiago commented Feb 25, 2026

Hi @HectorMalot . Please check comments.

@HectorMalot
Copy link
Copy Markdown
Contributor Author

HectorMalot commented Feb 26, 2026

Thanks @emiago , I've updated to checking s.RemoteAddr != nil.

@emiago emiago merged commit 3789060 into emiago:main Feb 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SDES SRTP: outbound RTP encrypted even when remote peer offers no crypto

2 participants