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.

Sign up for GitHub

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

Add the option to check the source address of ClientHello message on DTLS-SRTP #4261

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
+214 −14
Open

Add the option to check the source address of ClientHello message on DTLS-SRTP #4261

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7be1f21
Check the source address of ClientHello message on DTLS-SRTP
trengginas Jan 22, 2025
dfe04eb
Change the return status
trengginas Jan 22, 2025
54d7f60
Add check if source adress is available
trengginas Jan 23, 2025
458c845
Modification based on comments
trengginas Jan 28, 2025
e590ecf
Modification based on comments
trengginas Jan 28, 2025
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
10 changes: 10 additions & 0 deletions pjmedia/include/pjmedia/config.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1100,6 +1100,16 @@
# define PJMEDIA_SRTP_DTLS_OSSL_CIPHERS "DEFAULT"
#endif

/**
* Enabled this to check the source address of ClientHello message coming
* from a valid address.
*
* Default value: 0
*/
#ifndef PJMEDIA_SRTP_DTLS_CHECK_HELLO_ADDR
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need a compile-time setting? and why is the default 0 if it may cause a security issue?

Copy link
Member Author

Choose a reason for hiding this comment

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

It might add a slight delay in starting the handshake (wait for the transport address and RTP source address to be available) and possible issues when using aggressive nomination.

Copy link
Member

Choose a reason for hiding this comment

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

After reading the pdf, I believe the address check should be made mandatory for ICE, and the address should match one of the verified (valid?) candidates obtained during ICE check, so there should be no delay because ICE check has been done.

For no-ICE, then you can use the compile-time setting, because the best we can do is compare it with the SDP address, which may not be accurate.

# define PJMEDIA_SRTP_DTLS_CHECK_HELLO_ADDR 0
#endif


/**
* Maximum number of SRTP cryptos.
Expand Down
46 changes: 46 additions & 0 deletions pjmedia/src/pjmedia/transport_srtp_dtls.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1361,6 +1361,52 @@ static pj_status_t dtls_on_recv(pjmedia_transport *tp, unsigned idx,
(ds->setup == DTLS_SETUP_ACTPASS || ds->setup == DTLS_SETUP_PASSIVE))
{
pj_status_t status;

#if defined(PJMEDIA_SRTP_DTLS_CHECK_HELLO_ADDR) && PJMEDIA_SRTP_DTLS_CHECK_HELLO_ADDR!=0
pjmedia_transport_info info;
pj_sockaddr src_addr, rem_addr;
pj_bool_t rem_addr_avail = PJ_TRUE;
pjmedia_transport_get_info(ds->srtp->member_tp, &info);
if (idx == RTP_CHANNEL) {
pj_sockaddr_cp(&src_addr, &info.src_rtp_name);
if (!pj_sockaddr_has_addr(&ds->rem_addr)) {
rem_addr_avail = PJ_FALSE;
} else {
pj_sockaddr_cp(&rem_addr, &ds->rem_addr);
}
} else {
pj_sockaddr_cp(&src_addr, &info.src_rtcp_name);
if (!pj_sockaddr_has_addr(&ds->rem_rtcp)) {
rem_addr_avail = PJ_FALSE;
} else {
pj_sockaddr_cp(&rem_addr, &ds->rem_rtcp);
}
}
if (!rem_addr_avail) {
char addr[PJ_INET6_ADDRSTRLEN];

PJ_LOG(2, (ds->base.name, "DTLS-SRTP %s ignoring %lu bytes "
"from [%s], remote address not available",
CHANNEL_TO_STRING(idx), (unsigned long)size,
pj_sockaddr_print(&src_addr, addr, sizeof(addr), 3)));

DTLS_UNLOCK(ds);
return PJ_SUCCESS;
}

if (pj_sockaddr_cmp(&src_addr, &rem_addr) != 0) {
char addr[PJ_INET6_ADDRSTRLEN], raddr[PJ_INET6_ADDRSTRLEN];

PJ_LOG(2, (ds->base.name, "DTLS-SRTP %s ignoring message from [%s],"
"expecting from [%s]",
CHANNEL_TO_STRING(idx),
pj_sockaddr_print(&src_addr, addr, sizeof(addr), 3),
pj_sockaddr_print(&rem_addr, raddr, sizeof(raddr), 3)));

DTLS_UNLOCK(ds);
return PJ_SUCCESS;
}
#endif
ds->setup = DTLS_SETUP_PASSIVE;
status = ssl_handshake_channel(ds, idx);
if (status != PJ_SUCCESS) {
Expand Down
Loading