Merge commit from fork

Add `switch_stun_packet_verify_integrity()`, an HMAC-SHA1
MESSAGE-INTEGRITY verifier that is const and non-mutating: it runs
over a private copy of the pristine network-order packet, so the
caller's buffer and byte order stay untouched, and walks attributes
with its own unsigned bounded helper `stun_wire_attr_bounds()`
instead of the host-order iterator macros. A trailing
MESSAGE-INTEGRITY-SHA256 or FINGERPRINT after MESSAGE-INTEGRITY is
tolerated; any other trailing attribute is rejected.

Gate it in `handle_ice()` behind `ice->verify_integrity`: verify
before any ICE state is touched, keyed by message type (local
`ice->pass` for a request, remote `ice->rpass` for a response or
error response), and drop on failure. Keepalive indications carry no
MESSAGE-INTEGRITY and are ignored.

`ice->verify_integrity` is read from the `ice_verify_message_integrity`
channel variable in `switch_rtp_activate_ice()` and defaults off, so
receive-path behavior is unchanged unless it is enabled. Adds unit
tests in `tests/unit/switch_stun.c`.
This commit is contained in:
Dmitry Verenitsin
2026-08-08 17:03:47 +03:00
committed by GitHub
parent c1bb5c6ab3
commit 68ec688eb6
5 changed files with 302 additions and 0 deletions
+4
View File
@@ -426,6 +426,10 @@
<X-PRE-PROCESS cmd="set" data="rtp_liberal_dtmf=true"/>
<!-- Helps with WebRTC Audio -->
<!-- Require valid MESSAGE-INTEGRITY on inbound ICE (STUN) connectivity checks; unverified checks are
dropped. Default off. Uncomment and change Z- prefix to X- to enable. -->
<!--<Z-PRE-PROCESS cmd="set" data="ice_verify_message_integrity=true"/>-->
<!-- Stock Video Avatars -->
<X-PRE-PROCESS cmd="set" data="video_mute_png=$${images_dir}/default-mute.png"/>
<X-PRE-PROCESS cmd="set" data="video_no_avatar_png=$${images_dir}/default-avatar.png"/>
+14
View File
@@ -79,6 +79,7 @@ typedef enum {
SWITCH_STUN_ATTR_DESTINATION_ADDRESS = 0x0011, /* Address */
SWITCH_STUN_ATTR_SOURCE_ADDRESS2 = 0x0012, /* Address */
SWITCH_STUN_ATTR_DATA = 0x0013, /* ByteString */
SWITCH_STUN_ATTR_MESSAGE_INTEGRITY_SHA256 = 0x001c, /* ByteString, 16-32 bytes (RFC 8489) */
SWITCH_STUN_ATTR_OPTIONS = 0x8001, /* UInt32 */
SWITCH_STUN_ATTR_XOR_MAPPED_ADDRESS = 0x0020, /* Address */
@@ -246,6 +247,19 @@ SWITCH_DECLARE(uint8_t) switch_stun_packet_attribute_add_software(switch_stun_pa
SWITCH_DECLARE(uint8_t) switch_stun_packet_attribute_add_binded_address(switch_stun_packet_t *packet, char *ipstr, uint16_t port, int family);
SWITCH_DECLARE(uint8_t) switch_stun_packet_attribute_add_xor_binded_address(switch_stun_packet_t *packet, char *ipstr, uint16_t port, int family);
SWITCH_DECLARE(uint8_t) switch_stun_packet_attribute_add_integrity(switch_stun_packet_t *packet, const char *pass);
/*!
\brief Verify the MESSAGE-INTEGRITY (HMAC-SHA1) of a received STUN packet
\param pkt the raw, unmodified (network byte order) packet bytes as received off the wire
\param len the number of valid bytes in pkt
\param pass the key (ICE password) the sender is expected to have used
\return SWITCH_STATUS_SUCCESS if a MESSAGE-INTEGRITY attribute is present and its HMAC matches,
SWITCH_STATUS_NOTFOUND if no MESSAGE-INTEGRITY attribute is present,
SWITCH_STATUS_FALSE on a mismatch or a malformed/out-of-bounds packet
\note pkt must be the pristine wire bytes; switch_stun_packet_parse() rewrites fields to host
byte order in place, so a parsed buffer cannot be verified.
*/
SWITCH_DECLARE(switch_status_t) switch_stun_packet_verify_integrity(const uint8_t *pkt, uint32_t len, const char *pass);
SWITCH_DECLARE(uint32_t) switch_crc32_8bytes(const void* data, size_t length);
SWITCH_DECLARE(uint8_t) switch_stun_packet_attribute_add_fingerprint(switch_stun_packet_t *packet);
SWITCH_DECLARE(uint8_t) switch_stun_packet_attribute_add_use_candidate(switch_stun_packet_t *packet);
+36
View File
@@ -263,6 +263,7 @@ typedef struct {
char last_sent_id[13];
switch_time_t last_ok;
uint8_t cand_responsive;
uint8_t verify_integrity;
} switch_rtp_ice_t;
struct switch_rtp;
@@ -1010,6 +1011,36 @@ static void handle_ice(switch_rtp_t *rtp_session, switch_rtp_ice_t *ice, void *d
}
if ((ice->type & ICE_VANILLA) && ice->verify_integrity) {
/* Verify before any ICE state is touched, over the pristine wire bytes (not the byte-swapped
host-order buf). Key by type: request with our local password, response/error-response with
the remote password. Indications carry no MESSAGE-INTEGRITY and drive no state, so drop them. */
const char *ikey = NULL;
switch (packet->header.type) {
case SWITCH_STUN_BINDING_REQUEST:
ikey = ice->pass;
break;
case SWITCH_STUN_BINDING_RESPONSE:
case SWITCH_STUN_BINDING_ERROR_RESPONSE:
ikey = ice->rpass;
break;
default:
switch_log_printf(SWITCH_CHANNEL_SESSION_LOG(rtp_session->session), SWITCH_LOG_DEBUG8,
"%s ignoring unauthenticated STUN %s from %s:%d\n", rtp_type(rtp_session),
switch_stun_value_to_name(SWITCH_STUN_TYPE_PACKET_TYPE, packet->header.type), from_host, from_port);
goto end;
}
if (switch_stun_packet_verify_integrity((const uint8_t *)data, (uint32_t)cpylen, ikey) != SWITCH_STATUS_SUCCESS) {
switch_log_printf(SWITCH_CHANNEL_SESSION_LOG(rtp_session->session), SWITCH_LOG_WARNING,
"%s STUN MESSAGE-INTEGRITY verification failed; dropping %s from %s:%d\n",
rtp_type(rtp_session),
switch_stun_value_to_name(SWITCH_STUN_TYPE_PACKET_TYPE, packet->header.type), from_host, from_port);
goto end;
}
}
rtp_session->last_stun = switch_micro_time_now();
if (!rtp_session->first_stun) {
@@ -4981,6 +5012,7 @@ SWITCH_DECLARE(switch_status_t) switch_rtp_activate_ice(switch_rtp_t *rtp_sessio
ice->ice_params = ice_params;
ice->pass = "";
ice->rpass = "";
ice->verify_integrity = 0;
ice->next_run = switch_micro_time_now();
ice->initializing = 1;
@@ -4992,6 +5024,10 @@ SWITCH_DECLARE(switch_status_t) switch_rtp_activate_ice(switch_rtp_t *rtp_sessio
ice->rpass = switch_core_strdup(rtp_session->pool, rpassword);
}
if ((type & ICE_VANILLA) && switch_channel_var_true(switch_core_session_get_channel(rtp_session->session), "ice_verify_message_integrity")) {
ice->verify_integrity = 1;
}
if ((ice->type & ICE_VANILLA) && ice->ice_params) {
host = ice->ice_params->cands[ice->ice_params->chosen[ice->proto]][ice->proto].con_addr;
port = ice->ice_params->cands[ice->ice_params->chosen[ice->proto]][ice->proto].con_port;
+101
View File
@@ -35,6 +35,7 @@
#include <switch_stun.h>
#include <openssl/sha.h>
#include <openssl/hmac.h>
#include <openssl/crypto.h>
struct value_mapping {
const uint32_t value;
@@ -705,6 +706,106 @@ SWITCH_DECLARE(uint8_t) switch_stun_packet_attribute_add_password(switch_stun_pa
return 1;
}
/* True if the attribute at `attr` fits within `end`, writing its padded value length to *padded.
Unsigned (not the int16_t macros) so a high-bit length can't go negative and walk backward. */
static switch_bool_t stun_wire_attr_bounds(const switch_stun_packet_attribute_t *attr, const uint8_t *end, uint32_t *padded)
{
uint32_t plen;
if ((const uint8_t *) (attr + 1) > end || !attr->type) {
return SWITCH_FALSE;
}
plen = ntohs(attr->length);
/* STUN pads each attribute value to a 4-byte boundary; round the declared length up to match. */
plen = (plen & 0x3) ? (plen & ~0x3u) + 4 : plen;
if (plen > (uint32_t) (end - (const uint8_t *) attr->value)) {
return SWITCH_FALSE;
}
*padded = plen;
return SWITCH_TRUE;
}
SWITCH_DECLARE(switch_status_t) switch_stun_packet_verify_integrity(const uint8_t *pkt, uint32_t len, const char *pass)
{
uint8_t copy[1500];
switch_stun_packet_t *packet;
switch_stun_packet_attribute_t *attr;
switch_stun_packet_attribute_t *mi = NULL;
uint8_t *end;
uint32_t declared;
uint32_t mi_off;
uint32_t padded;
uint16_t hashed_length;
unsigned char digest[SHA_DIGEST_LENGTH];
if (!pkt || zstr(pass) || len < SWITCH_STUN_PACKET_MIN_LEN || len > sizeof(copy)) {
return SWITCH_STATUS_FALSE;
}
/* Work on a private copy so the caller's wire bytes stay untouched and we can safely
rewrite the length field for the HMAC input. Input is network byte order. */
memcpy(copy, pkt, len);
packet = (switch_stun_packet_t *) copy;
/* Bound the attribute walk to the declared message length, clamped to what we actually
received, so trailing bytes past the message are never treated as attributes. */
declared = SWITCH_STUN_PACKET_MIN_LEN + ntohs(packet->header.length);
end = copy + (declared < len ? declared : len);
/* Walk attributes (network byte order) to locate MESSAGE-INTEGRITY; stun_wire_attr_bounds
bounds-checks each attribute, including the first. */
switch_stun_packet_first_attribute(packet, attr);
while (stun_wire_attr_bounds(attr, end, &padded)) {
if (attr->type == htons(SWITCH_STUN_ATTR_MESSAGE_INTEGRITY)) {
mi = attr;
break;
}
attr = (switch_stun_packet_attribute_t *) ((uint8_t *) attr->value + padded);
}
if (!mi) {
return SWITCH_STATUS_NOTFOUND;
}
/* MESSAGE-INTEGRITY always carries a 20-byte HMAC-SHA1 value. */
if (ntohs(mi->length) != SHA_DIGEST_LENGTH) {
return SWITCH_STATUS_FALSE;
}
mi_off = (uint32_t) ((uint8_t *) mi - copy);
/* Only MESSAGE-INTEGRITY-SHA256 and FINGERPRINT may follow MESSAGE-INTEGRITY; both sit outside this
HMAC's coverage. Reject any other trailing attribute rather than act on it unauthenticated. */
attr = (switch_stun_packet_attribute_t *) (mi->value + SHA_DIGEST_LENGTH);
while (stun_wire_attr_bounds(attr, end, &padded)) {
if (attr->type != htons(SWITCH_STUN_ATTR_FINGERPRINT) &&
attr->type != htons(SWITCH_STUN_ATTR_MESSAGE_INTEGRITY_SHA256)) {
return SWITCH_STATUS_FALSE;
}
attr = (switch_stun_packet_attribute_t *) ((uint8_t *) attr->value + padded);
}
/* Reproduce the sender's HMAC input: header.length must read as if the message ended right
after MESSAGE-INTEGRITY (so a trailing FINGERPRINT is excluded), and the HMAC covers the
message prefix up to but not including the MESSAGE-INTEGRITY attribute. */
hashed_length = (uint16_t) (mi_off - sizeof(switch_stun_packet_header_t) + sizeof(switch_stun_packet_attribute_t) + SHA_DIGEST_LENGTH);
packet->header.length = htons(hashed_length);
HMAC(EVP_sha1(), (const unsigned char *) pass, (int) strlen(pass), copy, mi_off, digest, NULL);
if (CRYPTO_memcmp(digest, mi->value, SHA_DIGEST_LENGTH) != 0) {
return SWITCH_STATUS_FALSE;
}
return SWITCH_STATUS_SUCCESS;
}
SWITCH_DECLARE(char *) switch_stun_host_lookup(const char *host, switch_memory_pool_t *pool)
{
switch_sockaddr_t *addr = NULL;
+147
View File
@@ -165,6 +165,153 @@ FST_TEARDOWN_END()
fst_check_string_equals(out_ip, ipv4_str);
}
FST_TEST_END()
FST_TEST_BEGIN(test_stun_verify_integrity_accepts_valid_hmac)
{
/* A packet signed with add_integrity under a given key must verify against that same key,
including when a leading attribute precedes MESSAGE-INTEGRITY (it is part of the HMAC input). */
uint8_t buf[512] = { 0 };
char software[] = "sw";
switch_stun_packet_t *packet;
uint32_t len;
packet = switch_stun_packet_build_header(SWITCH_STUN_BINDING_REQUEST, NULL, buf);
switch_stun_packet_attribute_add_software(packet, software, (uint16_t)strlen(software));
switch_stun_packet_attribute_add_integrity(packet, "secret");
len = (uint32_t)switch_stun_packet_length(packet);
fst_xcheck(switch_stun_packet_verify_integrity(buf, len, "secret") == SWITCH_STATUS_SUCCESS,
"valid MESSAGE-INTEGRITY verifies against the signing key");
fst_xcheck(switch_stun_packet_verify_integrity(buf, len, "wrong") == SWITCH_STATUS_FALSE,
"MESSAGE-INTEGRITY does not verify against a different key");
}
FST_TEST_END()
FST_TEST_BEGIN(test_stun_verify_integrity_rejects_zeroed_hmac)
{
/* An all-zero MESSAGE-INTEGRITY value must not verify: this is the shape a sender produces when
it fills the field with zeros instead of computing the HMAC. MI is the first attribute, so its
20-byte value sits at offset 24 (20-byte header plus 4-byte attribute header). */
uint8_t buf[512] = { 0 };
switch_stun_packet_t *packet;
uint32_t len;
packet = switch_stun_packet_build_header(SWITCH_STUN_BINDING_REQUEST, NULL, buf);
switch_stun_packet_attribute_add_integrity(packet, "secret");
len = (uint32_t)switch_stun_packet_length(packet);
memset(buf + SWITCH_STUN_PACKET_MIN_LEN + 4, 0, 20);
fst_xcheck(switch_stun_packet_verify_integrity(buf, len, "secret") == SWITCH_STATUS_FALSE,
"a zeroed MESSAGE-INTEGRITY value is rejected");
}
FST_TEST_END()
FST_TEST_BEGIN(test_stun_verify_integrity_trailing_fingerprint)
{
/* MESSAGE-INTEGRITY followed by FINGERPRINT (the layout our own responses use) must still verify:
the HMAC input's length field reads as if the message ended right after MESSAGE-INTEGRITY, so the
trailing FINGERPRINT is excluded from the computation. */
uint8_t buf[512] = { 0 };
switch_stun_packet_t *packet;
uint32_t len;
packet = switch_stun_packet_build_header(SWITCH_STUN_BINDING_RESPONSE, NULL, buf);
switch_stun_packet_attribute_add_integrity(packet, "secret");
switch_stun_packet_attribute_add_fingerprint(packet);
len = (uint32_t)switch_stun_packet_length(packet);
fst_xcheck(switch_stun_packet_verify_integrity(buf, len, "secret") == SWITCH_STATUS_SUCCESS,
"MESSAGE-INTEGRITY verifies with a trailing FINGERPRINT present");
}
FST_TEST_END()
FST_TEST_BEGIN(test_stun_verify_integrity_absent)
{
/* A packet with no MESSAGE-INTEGRITY attribute reports NOTFOUND, distinct from a mismatch, so the
caller can apply its own present-or-absent policy. */
uint8_t buf[512] = { 0 };
char software[] = "sw";
switch_stun_packet_t *packet;
uint32_t len;
packet = switch_stun_packet_build_header(SWITCH_STUN_BINDING_REQUEST, NULL, buf);
switch_stun_packet_attribute_add_software(packet, software, (uint16_t)strlen(software));
len = (uint32_t)switch_stun_packet_length(packet);
fst_xcheck(switch_stun_packet_verify_integrity(buf, len, "secret") == SWITCH_STATUS_NOTFOUND,
"a packet with no MESSAGE-INTEGRITY reports NOTFOUND");
}
FST_TEST_END()
FST_TEST_BEGIN(test_stun_verify_integrity_rejects_attr_after_mi)
{
/* Only MESSAGE-INTEGRITY-SHA256 and FINGERPRINT may follow MESSAGE-INTEGRITY. A USE-CANDIDATE
appended after MI leaves the HMAC prefix - and therefore the signature - valid, so it must be
rejected outright rather than verified and then acted on. */
uint8_t buf[512] = { 0 };
switch_stun_packet_t *packet;
uint32_t len;
packet = switch_stun_packet_build_header(SWITCH_STUN_BINDING_REQUEST, NULL, buf);
switch_stun_packet_attribute_add_integrity(packet, "secret");
switch_stun_packet_attribute_add_use_candidate(packet); /* lands after MESSAGE-INTEGRITY */
len = (uint32_t)switch_stun_packet_length(packet);
fst_xcheck(switch_stun_packet_verify_integrity(buf, len, "secret") == SWITCH_STATUS_FALSE,
"a non-FINGERPRINT attribute after MESSAGE-INTEGRITY is rejected");
}
FST_TEST_END()
FST_TEST_BEGIN(test_stun_verify_integrity_oversized_attr_length)
{
/* An attribute length with the high bit set must not walk the cursor backward or read out of
bounds: the walk treats the padded length as unsigned and stops once it exceeds the bytes that
remain. With a leading oversized attribute, MESSAGE-INTEGRITY is never reached (NOTFOUND) and no
out-of-bounds access occurs (ASAN would catch a regression here). */
uint8_t buf[64] = { 0 };
switch_stun_packet_t *packet;
switch_stun_packet_attribute_t *attr;
uint32_t len;
packet = switch_stun_packet_build_header(SWITCH_STUN_BINDING_REQUEST, NULL, buf);
attr = (switch_stun_packet_attribute_t *)packet->first_attribute;
attr->type = htons(SWITCH_STUN_ATTR_USERNAME);
attr->length = htons(0x8000);
packet->header.length = htons(4); /* declare just the 4-byte attribute header */
len = SWITCH_STUN_PACKET_MIN_LEN + 4;
fst_xcheck(switch_stun_packet_verify_integrity(buf, len, "secret") == SWITCH_STATUS_NOTFOUND,
"an attribute length with the high bit set stops the walk without an out-of-bounds read");
}
FST_TEST_END()
FST_TEST_BEGIN(test_stun_verify_integrity_allows_sha256_after_mi)
{
/* RFC 8489 permits MESSAGE-INTEGRITY-SHA256 to follow MESSAGE-INTEGRITY. It is inert (carries no
ICE state) and outside the SHA-1 HMAC's coverage, so a dual-hash sender's packet must still
verify on its SHA-1 MESSAGE-INTEGRITY rather than be rejected as a disallowed trailing attribute. */
uint8_t buf[512] = { 0 };
switch_stun_packet_t *packet;
switch_stun_packet_attribute_t *sha256;
uint32_t off;
uint32_t len;
packet = switch_stun_packet_build_header(SWITCH_STUN_BINDING_REQUEST, NULL, buf);
switch_stun_packet_attribute_add_integrity(packet, "secret");
/* Append a MESSAGE-INTEGRITY-SHA256 attribute (32-byte value, left zero: the SHA-1 verifier does
not inspect it) immediately after MESSAGE-INTEGRITY. */
off = SWITCH_STUN_PACKET_MIN_LEN + ntohs(packet->header.length);
sha256 = (switch_stun_packet_attribute_t *)(buf + off);
sha256->type = htons(SWITCH_STUN_ATTR_MESSAGE_INTEGRITY_SHA256);
sha256->length = htons(32);
packet->header.length = htons((uint16_t)(ntohs(packet->header.length) + 4 + 32));
len = (uint32_t)switch_stun_packet_length(packet);
fst_xcheck(switch_stun_packet_verify_integrity(buf, len, "secret") == SWITCH_STATUS_SUCCESS,
"a MESSAGE-INTEGRITY-SHA256 attribute after MESSAGE-INTEGRITY is tolerated");
}
FST_TEST_END()
}
FST_SUITE_END()
}