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

[core] Fixed cookie contest #1517

Merged
merged 1 commit into from
May 4, 2021
Merged
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
21 changes: 20 additions & 1 deletion docs/features/handshake.md
Original file line number Diff line number Diff line change
Expand Up @@ -697,7 +697,26 @@ connection will not be made until new, unique cookies are generated (after a
delay of up to one minute). In the case of an application "connecting to itself",
the cookies will always be identical, and so the connection will never be made.

When one party's cookie value is greater than its peer's, it wins the cookie
```c++
// Here m_ConnReq.m_iCookie is a local cookie value sent in connection request to the peer.
// m_ConnRes.m_iCookie is a cookie value sent by the peer in its connection request.
const int64_t contest = int64_t(m_ConnReq.m_iCookie) - int64_t(m_ConnRes.m_iCookie);

if ((contest & 0xFFFFFFFF) == 0)
{
return HSD_DRAW;
}

if (contest & 0x80000000)
{
return HSD_RESPONDER;
}

return HSD_INITIATOR;
```

When one party's cookie value is greater than its peer's (based on 32-bit subtraction of both
with potential overflow), it wins the cookie
contest and becomes Initiator (the other party becomes the Responder).

At this point there are two "handshake flows" possible (at least theoretically):
Expand Down
50 changes: 32 additions & 18 deletions srtcore/core.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3758,8 +3758,10 @@ void CUDT::cookieContest()
if (m_SrtHsSide != HSD_DRAW)
return;

HLOGC(cnlog.Debug, log << "cookieContest: agent=" << m_ConnReq.m_iCookie << " peer=" << m_ConnRes.m_iCookie);
LOGC(cnlog.Error, log << "cookieContest: agent=" << m_ConnReq.m_iCookie << " peer=" << m_ConnRes.m_iCookie);

// Here m_ConnReq.m_iCookie is a local cookie value sent in connection request to the peer.
// m_ConnRes.m_iCookie is a cookie value sent by the peer in its connection request.
if (m_ConnReq.m_iCookie == 0 || m_ConnRes.m_iCookie == 0)
{
// Note that it's virtually impossible that Agent's cookie is not ready, this
Expand All @@ -3772,31 +3774,43 @@ void CUDT::cookieContest()
//
// The cookie contest must be repeated every time because it
// may change the state at some point.
int better_cookie = m_ConnReq.m_iCookie - m_ConnRes.m_iCookie;

if (better_cookie > 0)
{
m_SrtHsSide = HSD_INITIATOR;
//
// In SRT v1.4.3 and prior the below subtraction was performed in 32-bit arithmetic.
// The result of subtraction can overflow 32-bits.
// Example
// m_ConnReq.m_iCookie = -1480577720;
// m_ConnRes.m_iCookie = 811599203;
// int64_t llBetterCookie = -1480577720 - 811599203 = -2292176923 (FFFF FFFF 7760 27E5);
// int32_t iBetterCookie = 2002790373 (7760 27E5);
//
// Now 64-bit arithmetic is used to calculate the actual result of subtraction.
// The 31-st bit is then checked to check if the resulting is negative in 32-bit aritmetics.
// This way the old contest behavior is preserved, and potential compiler optimisations are avoided.
const int64_t contest = int64_t(m_ConnReq.m_iCookie) - int64_t(m_ConnRes.m_iCookie);

if ((contest & 0xFFFFFFFF) == 0)
{
// DRAW! The only way to continue would be to force the
// cookies to be regenerated and to start over. But it's
// not worth a shot - this is an extremely rare case.
// This can simply do reject so that it can be started again.

// Pretend then that the cookie contest wasn't done so that
// it's done again. Cookies are baked every time anew, however
// the successful initial contest remains valid no matter how
// cookies will change.

m_SrtHsSide = HSD_DRAW;
return;
}

if (better_cookie < 0)
if (contest & 0x80000000)
{
m_SrtHsSide = HSD_RESPONDER;
return;
}

// DRAW! The only way to continue would be to force the
// cookies to be regenerated and to start over. But it's
// not worth a shot - this is an extremely rare case.
// This can simply do reject so that it can be started again.

// Pretend then that the cookie contest wasn't done so that
// it's done again. Cookies are baked every time anew, however
// the successful initial contest remains valid no matter how
// cookies will change.

m_SrtHsSide = HSD_DRAW;
m_SrtHsSide = HSD_INITIATOR;
}

// This function should complete the data for KMX needed for an out-of-band
Expand Down