Skip to content

Fix srtp_unprotect_rtcp_mki when RTP auth != RTCP #733

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 11, 2024

Conversation

vopatek
Copy link
Contributor

@vopatek vopatek commented Dec 6, 2024

srtp_get_session_keys, which is used by both srtp_unprotect_mki and srtp_unprotect_rtcp_mki, determines the tag len from rtp_auth.

This fails when rtp_auth differ from rtcp_auth. E.g. when SRTP is used with weak authentication but SRTCP must not (RFC 3711).

This commit splits the function in two:
srtp_get_session_keys_rtp
srtp_get_session_keys_rtcp

And adds a short auth policy test to test/srtp_driver.

srtp_get_session_keys, which is used by both srtp_unprotect_mki and
srtp_unprotect_rtcp_mki, determines the tag len from rtp_auth.

This fails when rtp_auth differ from rtcp_auth. E.g. when SRTP is used
with weak authentication but SRTCP must not (RFC 3711).

This commit splits the function in two:
srtp_get_session_keys_rtp
srtp_get_session_keys_rtcp

And adds a short auth policy test to test/srtp_driver.
@pabuhler
Copy link
Member

Thanks @vopatek this does seam to be an issue, I will fist fix up the CI build problem and then we can try to merge.

@pabuhler pabuhler merged commit 596e30e into cisco:2_x_dev Dec 11, 2024
32 of 33 checks passed
@pabuhler
Copy link
Member

Hi @vopatek ,

I have now merged this into the 2_x_dev branch and created a pr #737 that cherry picks this into main. Great if you could take a look and leave a comment if it looks ok.

Thanks!

mwalbeck pushed a commit to mwalbeck/docker-janus-gateway that referenced this pull request Aug 17, 2025
This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [cisco/libsrtp](https://github.com/cisco/libsrtp) | minor | `v2.6.0` -> `v2.7.0` |

---

> ⚠️ **Warning**
>
> Some dependencies could not be looked up. Check the Dependency Dashboard for more information.

---

### Release Notes

<details>
<summary>cisco/libsrtp (cisco/libsrtp)</summary>

### [`v2.7.0`](https://github.com/cisco/libsrtp/releases/tag/v2.7.0): libSRTP 2.7.0

[Compare Source](cisco/libsrtp@v2.6.0...v2.7.0)

#### What's Changed

- explicitly look for mbedcrypto dependency by [@&#8203;pabuhler](https://github.com/pabuhler) in cisco/libsrtp#736
- Fix srtp\_unprotect\_rtcp\_mki when RTP auth != RTCP by [@&#8203;vopatek](https://github.com/vopatek) in cisco/libsrtp#733
- update formatting scripts to  clang-format-14 by [@&#8203;pabuhler](https://github.com/pabuhler) in cisco/libsrtp#747
- update upload-artifact version in cifuzz task by [@&#8203;pabuhler](https://github.com/pabuhler) in cisco/libsrtp#748
- Fix function signature mismatch for `srtp_remove_stream` by [@&#8203;vikramdattu](https://github.com/vikramdattu) in cisco/libsrtp#746

**Full Changelog**: cisco/libsrtp@v2.6...v2.7.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOS4yMDUuMSIsInVwZGF0ZWRJblZlciI6IjQxLjQzLjUiLCJ0YXJnZXRCcmFuY2giOiJtYWluIiwibGFiZWxzIjpbXX0=-->

Reviewed-on: https://git.walbeck.it/walbeck-it/docker-janus-gateway/pulls/170
Co-authored-by: renovate-bot <bot@walbeck.it>
Co-committed-by: renovate-bot <bot@walbeck.it>
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.

2 participants