Skip to content
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

C++: Update libsrtp #1313

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog

### NEXT

- C++: Update libsrtp to pre-3.0.

### 3.13.16

- Node: Add new `worker.on('subprocessclose')` event ([PR #1307](https://github.com/versatica/mediasoup/pull/1307)).
Expand Down
10 changes: 5 additions & 5 deletions worker/include/RTC/SrtpSession.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,13 @@ namespace RTC
~SrtpSession();

public:
bool EncryptRtp(const uint8_t** data, int* len);
bool DecryptSrtp(uint8_t* data, int* len);
bool EncryptRtcp(const uint8_t** data, int* len);
bool DecryptSrtcp(uint8_t* data, int* len);
bool EncryptRtp(const uint8_t** data, size_t* len);
bool DecryptSrtp(uint8_t* data, size_t* len);
bool EncryptRtcp(const uint8_t** data, size_t* len);
bool DecryptSrtcp(uint8_t* data, size_t* len);
void RemoveStream(uint32_t ssrc)
{
srtp_remove_stream(this->session, uint32_t{ htonl(ssrc) });
srtp_stream_remove(this->session, uint32_t{ htonl(ssrc) });
}

private:
Expand Down
32 changes: 11 additions & 21 deletions worker/src/RTC/PipeTransport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -484,9 +484,9 @@ namespace RTC
}

const uint8_t* data = packet->GetData();
auto intLen = static_cast<int>(packet->GetSize());
auto len = packet->GetSize();

if (HasSrtp() && !this->srtpSendSession->EncryptRtp(&data, &intLen))
if (HasSrtp() && !this->srtpSendSession->EncryptRtp(&data, &len))
{
if (cb)
{
Expand All @@ -497,8 +497,6 @@ namespace RTC
return;
}

auto len = static_cast<size_t>(intLen);

this->tuple->Send(data, len, cb);

// Increase send transmission.
Expand All @@ -515,15 +513,13 @@ namespace RTC
}

const uint8_t* data = packet->GetData();
auto intLen = static_cast<int>(packet->GetSize());
auto len = packet->GetSize();

if (HasSrtp() && !this->srtpSendSession->EncryptRtcp(&data, &intLen))
if (HasSrtp() && !this->srtpSendSession->EncryptRtcp(&data, &len))
{
return;
}

auto len = static_cast<size_t>(intLen);

this->tuple->Send(data, len);

// Increase send transmission.
Expand All @@ -542,15 +538,13 @@ namespace RTC
packet->Serialize(RTC::RTCP::Buffer);

const uint8_t* data = packet->GetData();
auto intLen = static_cast<int>(packet->GetSize());
auto len = packet->GetSize();

if (HasSrtp() && !this->srtpSendSession->EncryptRtcp(&data, &intLen))
if (HasSrtp() && !this->srtpSendSession->EncryptRtcp(&data, &len))
{
return;
}

auto len = static_cast<size_t>(intLen);

this->tuple->Send(data, len);

// Increase send transmission.
Expand Down Expand Up @@ -638,11 +632,9 @@ namespace RTC
}

// Decrypt the SRTP packet.
auto intLen = static_cast<int>(len);

if (HasSrtp() && !this->srtpRecvSession->DecryptSrtp(const_cast<uint8_t*>(data), &intLen))
if (HasSrtp() && !this->srtpRecvSession->DecryptSrtp(const_cast<uint8_t*>(data), &len))
{
RTC::RtpPacket* packet = RTC::RtpPacket::Parse(data, static_cast<size_t>(intLen));
RTC::RtpPacket* packet = RTC::RtpPacket::Parse(data, len);

if (!packet)
{
Expand All @@ -663,7 +655,7 @@ namespace RTC
return;
}

RTC::RtpPacket* packet = RTC::RtpPacket::Parse(data, static_cast<size_t>(intLen));
RTC::RtpPacket* packet = RTC::RtpPacket::Parse(data, len);

if (!packet)
{
Expand Down Expand Up @@ -700,9 +692,7 @@ namespace RTC
}

// Decrypt the SRTCP packet.
auto intLen = static_cast<int>(len);

if (HasSrtp() && !this->srtpRecvSession->DecryptSrtcp(const_cast<uint8_t*>(data), &intLen))
if (HasSrtp() && !this->srtpRecvSession->DecryptSrtcp(const_cast<uint8_t*>(data), &len))
{
return;
}
Expand All @@ -715,7 +705,7 @@ namespace RTC
return;
}

RTC::RTCP::Packet* packet = RTC::RTCP::Packet::Parse(data, static_cast<size_t>(intLen));
RTC::RTCP::Packet* packet = RTC::RTCP::Packet::Parse(data, len);

if (!packet)
{
Expand Down
32 changes: 11 additions & 21 deletions worker/src/RTC/PlainTransport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -764,9 +764,9 @@ namespace RTC
}

const uint8_t* data = packet->GetData();
auto intLen = static_cast<int>(packet->GetSize());
auto len = packet->GetSize();

if (HasSrtp() && !this->srtpSendSession->EncryptRtp(&data, &intLen))
if (HasSrtp() && !this->srtpSendSession->EncryptRtp(&data, &len))
{
if (cb)
{
Expand All @@ -777,8 +777,6 @@ namespace RTC
return;
}

auto len = static_cast<size_t>(intLen);

this->tuple->Send(data, len, cb);

// Increase send transmission.
Expand All @@ -795,15 +793,13 @@ namespace RTC
}

const uint8_t* data = packet->GetData();
auto intLen = static_cast<int>(packet->GetSize());
auto len = packet->GetSize();

if (HasSrtp() && !this->srtpSendSession->EncryptRtcp(&data, &intLen))
if (HasSrtp() && !this->srtpSendSession->EncryptRtcp(&data, &len))
{
return;
}

auto len = static_cast<size_t>(intLen);

if (this->rtcpMux)
{
this->tuple->Send(data, len);
Expand All @@ -829,15 +825,13 @@ namespace RTC
packet->Serialize(RTC::RTCP::Buffer);

const uint8_t* data = packet->GetData();
auto intLen = static_cast<int>(packet->GetSize());
auto len = packet->GetSize();

if (HasSrtp() && !this->srtpSendSession->EncryptRtcp(&data, &intLen))
if (HasSrtp() && !this->srtpSendSession->EncryptRtcp(&data, &len))
{
return;
}

auto len = static_cast<size_t>(intLen);

if (this->rtcpMux)
{
this->tuple->Send(data, len);
Expand Down Expand Up @@ -933,11 +927,9 @@ namespace RTC
}

// Decrypt the SRTP packet.
auto intLen = static_cast<int>(len);

if (HasSrtp() && !this->srtpRecvSession->DecryptSrtp(const_cast<uint8_t*>(data), &intLen))
if (HasSrtp() && !this->srtpRecvSession->DecryptSrtp(const_cast<uint8_t*>(data), &len))
{
RTC::RtpPacket* packet = RTC::RtpPacket::Parse(data, static_cast<size_t>(intLen));
RTC::RtpPacket* packet = RTC::RtpPacket::Parse(data, len);

if (!packet)
{
Expand All @@ -958,7 +950,7 @@ namespace RTC
return;
}

RTC::RtpPacket* packet = RTC::RtpPacket::Parse(data, static_cast<size_t>(intLen));
RTC::RtpPacket* packet = RTC::RtpPacket::Parse(data, len);

if (!packet)
{
Expand Down Expand Up @@ -1031,9 +1023,7 @@ namespace RTC
}

// Decrypt the SRTCP packet.
auto intLen = static_cast<int>(len);

if (HasSrtp() && !this->srtpRecvSession->DecryptSrtcp(const_cast<uint8_t*>(data), &intLen))
if (HasSrtp() && !this->srtpRecvSession->DecryptSrtcp(const_cast<uint8_t*>(data), &len))
{
return;
}
Expand Down Expand Up @@ -1107,7 +1097,7 @@ namespace RTC
return;
}

RTC::RTCP::Packet* packet = RTC::RTCP::Packet::Parse(data, static_cast<size_t>(intLen));
RTC::RTCP::Packet* packet = RTC::RTCP::Packet::Parse(data, len);

if (!packet)
{
Expand Down
12 changes: 6 additions & 6 deletions worker/src/RTC/SrtpSession.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -189,12 +189,12 @@ namespace RTC
}
}

bool SrtpSession::EncryptRtp(const uint8_t** data, int* len)
bool SrtpSession::EncryptRtp(const uint8_t** data, size_t* len)
{
MS_TRACE();

// Ensure that the resulting SRTP packet fits into the encrypt buffer.
if (static_cast<size_t>(*len) + SRTP_MAX_TRAILER_LEN > EncryptBufferSize)
if (*len + SRTP_MAX_TRAILER_LEN > EncryptBufferSize)
{
MS_WARN_TAG(srtp, "cannot encrypt RTP packet, size too big (%i bytes)", *len);

Expand Down Expand Up @@ -239,7 +239,7 @@ namespace RTC
return true;
}

bool SrtpSession::DecryptSrtp(uint8_t* data, int* len)
bool SrtpSession::DecryptSrtp(uint8_t* data, size_t* len)
{
MS_TRACE();

Expand All @@ -255,12 +255,12 @@ namespace RTC
return true;
}

bool SrtpSession::EncryptRtcp(const uint8_t** data, int* len)
bool SrtpSession::EncryptRtcp(const uint8_t** data, size_t* len)
{
MS_TRACE();

// Ensure that the resulting SRTCP packet fits into the encrypt buffer.
if (static_cast<size_t>(*len) + SRTP_MAX_TRAILER_LEN > EncryptBufferSize)
if (*len + SRTP_MAX_TRAILER_LEN > EncryptBufferSize)
{
MS_WARN_TAG(srtp, "cannot encrypt RTCP packet, size too big (%i bytes)", *len);

Expand All @@ -285,7 +285,7 @@ namespace RTC
return true;
}

bool SrtpSession::DecryptSrtcp(uint8_t* data, int* len)
bool SrtpSession::DecryptSrtcp(uint8_t* data, size_t* len)
{
MS_TRACE();

Expand Down
32 changes: 11 additions & 21 deletions worker/src/RTC/WebRtcTransport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -708,9 +708,9 @@ namespace RTC
}

const uint8_t* data = packet->GetData();
auto intLen = static_cast<int>(packet->GetSize());
auto len = packet->GetSize();

if (!this->srtpSendSession->EncryptRtp(&data, &intLen))
if (!this->srtpSendSession->EncryptRtp(&data, &len))
{
if (cb)
{
Expand All @@ -721,8 +721,6 @@ namespace RTC
return;
}

auto len = static_cast<size_t>(intLen);

this->iceServer->GetSelectedTuple()->Send(data, len, cb);

// Increase send transmission.
Expand All @@ -739,7 +737,7 @@ namespace RTC
}

const uint8_t* data = packet->GetData();
auto intLen = static_cast<int>(packet->GetSize());
auto len = packet->GetSize();

// Ensure there is sending SRTP session.
if (!this->srtpSendSession)
Expand All @@ -749,13 +747,11 @@ namespace RTC
return;
}

if (!this->srtpSendSession->EncryptRtcp(&data, &intLen))
if (!this->srtpSendSession->EncryptRtcp(&data, &len))
{
return;
}

auto len = static_cast<size_t>(intLen);

this->iceServer->GetSelectedTuple()->Send(data, len);

// Increase send transmission.
Expand All @@ -774,7 +770,7 @@ namespace RTC
packet->Serialize(RTC::RTCP::Buffer);

const uint8_t* data = packet->GetData();
auto intLen = static_cast<int>(packet->GetSize());
auto len = packet->GetSize();

// Ensure there is sending SRTP session.
if (!this->srtpSendSession)
Expand All @@ -784,13 +780,11 @@ namespace RTC
return;
}

if (!this->srtpSendSession->EncryptRtcp(&data, &intLen))
if (!this->srtpSendSession->EncryptRtcp(&data, &len))
{
return;
}

auto len = static_cast<size_t>(intLen);

this->iceServer->GetSelectedTuple()->Send(data, len);

// Increase send transmission.
Expand Down Expand Up @@ -957,11 +951,9 @@ namespace RTC
}

// Decrypt the SRTP packet.
auto intLen = static_cast<int>(len);

if (!this->srtpRecvSession->DecryptSrtp(const_cast<uint8_t*>(data), &intLen))
if (!this->srtpRecvSession->DecryptSrtp(const_cast<uint8_t*>(data), &len))
{
RTC::RtpPacket* packet = RTC::RtpPacket::Parse(data, static_cast<size_t>(intLen));
RTC::RtpPacket* packet = RTC::RtpPacket::Parse(data, len);

if (!packet)
{
Expand All @@ -982,7 +974,7 @@ namespace RTC
return;
}

RTC::RtpPacket* packet = RTC::RtpPacket::Parse(data, static_cast<size_t>(intLen));
RTC::RtpPacket* packet = RTC::RtpPacket::Parse(data, len);

if (!packet)
{
Expand Down Expand Up @@ -1028,14 +1020,12 @@ namespace RTC
}

// Decrypt the SRTCP packet.
auto intLen = static_cast<int>(len);

if (!this->srtpRecvSession->DecryptSrtcp(const_cast<uint8_t*>(data), &intLen))
if (!this->srtpRecvSession->DecryptSrtcp(const_cast<uint8_t*>(data), &len))
{
return;
}

RTC::RTCP::Packet* packet = RTC::RTCP::Packet::Parse(data, static_cast<size_t>(intLen));
RTC::RTCP::Packet* packet = RTC::RTCP::Packet::Parse(data, len);

if (!packet)
{
Expand Down
8 changes: 4 additions & 4 deletions worker/subprojects/libsrtp2.wrap
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
[wrap-file]
directory = libsrtp-4c9f0956f2933ac0650208d69c8d897625ba6301
source_url = https://github.com/versatica/libsrtp/archive/4c9f0956f2933ac0650208d69c8d897625ba6301.zip
source_filename = libsrtp-4c9f0956f2933ac0650208d69c8d897625ba6301.zip
source_hash = 4f3af61e26df398569605fc4bcf377587ca2d8bd34b2b4bf9cdb9590cadbd662
directory = libsrtp-37d88ecac103f4e2a5336099c64b155ba4ca3b0c
source_url = https://github.com/versatica/libsrtp/archive/37d88ecac103f4e2a5336099c64b155ba4ca3b0c.zip
source_filename = libsrtp-37d88ecac103f4e2a5336099c64b155ba4ca3b0c.zip
source_hash = edf8b2c7a27ed6b6aac64c8d6a165379d3a0042bd420706b4a1825acc182441d
Comment on lines +2 to +5
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why pulling from a fork rather than upstream?

Also FYI GitHub doesn't guarantee the hash of ZIP files from random revisions to not change over time. It is only guaranteed for releases.

Copy link
Member Author

Choose a reason for hiding this comment

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

Why pulling from a fork rather than upstream?

This is a mistake, I thought I was taking it from libsrtp repo directly.

Also FYI GitHub doesn't guarantee the hash of ZIP files from random revisions to not change over time. It is only guaranteed for releases.

That's right.., libsrtp version 3 is to be released sometime this year.., I'll ask for the timeline of the 2.5 release. I may create a PR for the enhancement to be included in 2.5.

Copy link
Member

Choose a reason for hiding this comment

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

Why don't we create a GH relaase in our libsrtp fork and point to it in the wrap file?

Copy link
Collaborator

Choose a reason for hiding this comment

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

That would be a solution indeed

Copy link
Member Author

@jmillan jmillan Jan 16, 2024

Choose a reason for hiding this comment

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

Also FYI GitHub doesn't guarantee the hash of ZIP files from random revisions to not change over time. It is only guaranteed for releases.

I can't read where it says that a zip file for a certain commit hash may change. Indeed the documentation says it won't.

https://docs.github.com/en/repositories/working-with-files/using-files/downloading-source-code-archives#stability-of-source-code-archives

Copy link
Collaborator

Choose a reason for hiding this comment

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

An archive of a commit ID will always have the same file contents whenever it's requested, assuming the commit ID is still in the repository and the repository's name has not changed.

Only contents because:

The exact compression settings used to generate a zipball or tarball may change over time. The extracted contents won't change if the branch or tag doesn't change, but the outer compressed archive may have a different byte layout. GitHub will give at least six months' notice before changing compression settings.

Copy link
Member

Choose a reason for hiding this comment

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

Over a year ago GH changed the compression settings and half of the projects (including mediasoup) got into problems, so this is a real thing. However GH took the problem into consideration hence that new policy. Probably we are good whatever we do here but the only reliable way to go is by having a GH release.

Copy link
Collaborator

Choose a reason for hiding this comment

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

They have a note about 6 months warning, but I can imagine downstream projects might rely in older mediasoup releases that should ideally continue to compile in the future too.

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice, I'll do a release from our fork then as suggested 👍


[provide]
libsrtp2 = libsrtp2_dep
Loading