Commit 86b10358e2 for freeswitch.com
commit 86b10358e2a191e2daff33d97c32757d43c8f92f
Author: Dmitry Verenitsin <morbit85@gmail.com>
Date: Sat Aug 8 21:47:47 2026 +0500
Merge commit from fork
`switch_core_media_add_crypto()` parses the SDP `a=crypto` keysalt and
decodes it into a fixed-size key buffer. Enforce the buffer contract at
the call site:
- Reject a zero-length, negative, or over-long keysalt (the length
check now has both a lower bound and an upper bound against the copy
buffer).
- Copy the keysalt token into a NUL-terminated buffer and decode from
that, so `switch_b64_decode` stops at the token instead of reading on
into the following key material (it consumes input to the NUL and
skips non-base64 bytes).
- Pass the destination buffer size as the decode bound rather than the
parsed token length.
- Require the decoded length to cover the crypto suite's key+salt so the
subsequent copy cannot read past the decoded bytes.
Add `test_add_crypto_keysalt_bounds`, a table-driven test covering the
accepted and rejected keysalt shapes across suites (AES-128/192/256),
including RFC 4568 lifetime/MKI and multi-key lines.
diff --git a/src/switch_core_media.c b/src/switch_core_media.c
index 51a4d32604..6a8e06342c 100644
--- a/src/switch_core_media.c
+++ b/src/switch_core_media.c
@@ -1240,6 +1240,7 @@ static const char* switch_core_media_crypto_find_key_material_candidate_end(cons
SWITCH_DECLARE(switch_status_t) switch_core_media_add_crypto(switch_core_session_t *session, switch_secure_settings_t *ssec, switch_rtp_crypto_direction_t direction)
{
unsigned char key[SWITCH_RTP_MAX_CRYPTO_LEN];
+ char keysalt_b64[SWITCH_RTP_MAX_CRYPTO_LEN + 1]; /* NUL-terminated copy of one key's base64, isolated from following key material */
switch_rtp_crypto_key_type_t type;
const char *p, *delimit;
@@ -1247,6 +1248,7 @@ SWITCH_DECLARE(switch_status_t) switch_core_media_add_crypto(switch_core_session
const char *key_material_end = NULL; /* begin and end of the current key material candidate */
int method_len;
int keysalt_len;
+ switch_size_t decoded_len;
const char *opts;
uint32_t opt_field; /* LIFETIME or MKI */
@@ -1341,11 +1343,24 @@ SWITCH_DECLARE(switch_status_t) switch_core_media_add_crypto(switch_core_session
keysalt_len = key_material_end - p;
}
- if (keysalt_len > sizeof(key)) {
+ if (keysalt_len <= 0 || (size_t) keysalt_len >= sizeof(keysalt_b64)) {
goto bad_keysalt_len;
}
- switch_b64_decode(p, (char *) key, keysalt_len);
+ /* switch_b64_decode consumes its input up to the NUL terminator and ignores
+ non-base64 bytes, so isolate this key's base64 in a terminated buffer to keep
+ the decode from spilling into the following key material. */
+ memcpy(keysalt_b64, p, keysalt_len);
+ keysalt_b64[keysalt_len] = '\0';
+
+ decoded_len = switch_b64_decode(keysalt_b64, (char *) key, sizeof(key));
+
+ /* switch_b64_decode returns the decoded byte count plus the trailing NUL it appends.
+ Require at least the suite's key+salt length so the copy below cannot read past the
+ decoded bytes into uninitialized stack. */
+ if (decoded_len <= (switch_size_t) SUITES[type].keysalt_len) {
+ goto bad_keysalt_len;
+ }
if (!multiple_keys) { /* First key becomes default (used in case no MKI is found). */
if (direction == SWITCH_RTP_CRYPTO_SEND) {
diff --git a/tests/unit/Makefile.am b/tests/unit/Makefile.am
index fed97bb535..a3c59059b3 100644
--- a/tests/unit/Makefile.am
+++ b/tests/unit/Makefile.am
@@ -6,6 +6,7 @@ noinst_PROGRAMS += switch_core_video switch_core_db switch_vad switch_packetizer
noinst_PROGRAMS += switch_stun
noinst_PROGRAMS += test_tts_format
noinst_PROGRAMS+= switch_hold switch_sip
+noinst_PROGRAMS += switch_core_media
noinst_PROGRAMS += test_mod_verto
noinst_PROGRAMS += test_mod_event_socket
diff --git a/tests/unit/switch_core_media.c b/tests/unit/switch_core_media.c
new file mode 100644
index 0000000000..b3a8bf2dbd
--- /dev/null
+++ b/tests/unit/switch_core_media.c
@@ -0,0 +1,144 @@
+/*
+ * FreeSWITCH Modular Media Switching Software Library / Soft-Switch Application
+ * Copyright (C) 2005-2021, Anthony Minessale II <anthm@freeswitch.org>
+ *
+ * Version: MPL 1.1
+ *
+ * The contents of this file are subject to the Mozilla Public License Version
+ * 1.1 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ * http://www.mozilla.org/MPL/
+ *
+ * Software distributed under the License is distributed on an "AS IS" basis,
+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+ * for the specific language governing rights and limitations under the
+ * License.
+ *
+ * The Original Code is FreeSWITCH Modular Media Switching Software Library / Soft-Switch Application
+ *
+ * The Initial Developer of the Original Code is
+ * Anthony Minessale II <anthm@freeswitch.org>
+ * Portions created by the Initial Developer are Copyright (C)
+ * the Initial Developer. All Rights Reserved.
+ *
+ * switch_core_media.c -- tests for the core media layer.
+ */
+
+#include <switch.h>
+#include <test/switch_test.h>
+
+FST_CORE_BEGIN("./conf")
+{
+ FST_SUITE_BEGIN(switch_core_media)
+ {
+ FST_SETUP_BEGIN()
+ {
+ fst_requires_module("mod_loopback");
+ }
+ FST_SETUP_END()
+
+ FST_TEARDOWN_BEGIN()
+ {
+ }
+ FST_TEARDOWN_END()
+
+ FST_TEST_BEGIN(test_add_crypto_keysalt_bounds)
+ {
+ switch_core_session_t *session = NULL;
+ switch_status_t status;
+ switch_call_cause_t cause;
+ switch_secure_settings_t ssec;
+ int i;
+
+ /* Writable buffers: switch_core_media_add_crypto() strips spaces in place. Each key||salt is a
+ 40-char base64 that decodes to the 30-byte key+salt of AES_CM_128_HMAC_SHA1_80. RFC 4568 crypto
+ line: "<tag> <suite> inline:<key||salt> ["|" lifetime] ["|" MKI ":" length]". rfc_full and
+ rfc_mki_only are the RFC's two verbatim examples; a multi-key attribute requires every key to
+ carry an equal-length MKI (RFC 4568 section 6.1). '/' (base64 63) decodes to 0xFF bytes. */
+ char valid_crypto[] = "1 AES_CM_128_HMAC_SHA1_80 inline:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA";
+ char nonzero_crypto[] = "1 AES_CM_128_HMAC_SHA1_80 inline:////////////////////////////////////////";
+ char rfc_full[] = "1 AES_CM_128_HMAC_SHA1_80 inline:d0RmdmcmVCspeEc3QGZiNWpVLFJhQX1cfHAwJSoj|2^20|1:4";
+ char rfc_mki_only[] = "1 AES_CM_128_HMAC_SHA1_80 inline:YUJDZGVmZ2hpSktMbW9QUXJzVHVWd3l6MTIzNDU2|1066:4";
+ char rfc_multikey[] = "1 AES_CM_128_HMAC_SHA1_80 inline:d0RmdmcmVCspeEc3QGZiNWpVLFJhQX1cfHAwJSoj|2^20|1:4 inline:YUJDZGVmZ2hpSktMbW9QUXJzVHVWd3l6MTIzNDU2|2^20|2:4";
+ char zero_len_space[] = "1 AES_CM_128_HMAC_SHA1_80 inline: AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA";
+ char zero_len_pipe[] = "1 AES_CM_128_HMAC_SHA1_80 inline:|2^20|1:4";
+ char short_keysalt[] = "1 AES_CM_128_HMAC_SHA1_80 inline:QUJD";
+ char overlong_keysalt[] = "1 AES_CM_128_HMAC_SHA1_80 inline:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA";
+ char zero_len_tail[] = "1 AES_CM_128_HMAC_SHA1_80 inline: AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA";
+ /* Other suites carry longer key+salts (RFC 6188): AES_192 is 38 bytes (52 base64 chars),
+ AES_256 is 46 bytes (64 chars - the largest keysalt that fits the copy buffer). Both
+ decode to all-0xFF bytes. aes256_short is a 128-length keysalt (30 bytes) offered under
+ the 256 suite. */
+ char aes192_crypto[] = "1 AES_192_CM_HMAC_SHA1_80 inline://////////////////////////////////////////////////8=";
+ char aes256_crypto[] = "1 AES_256_CM_HMAC_SHA1_80 inline://///////////////////////////////////////////////////////////w==";
+ char aes256_short[] = "1 AES_256_CM_HMAC_SHA1_80 inline:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA";
+
+ /* Independent base64 reference decode (not the parser's) of RFC 4568's first example key||salt. */
+ unsigned char rfc_full_raw[30] = {
+ 0x77, 0x44, 0x66, 0x76, 0x67, 0x26, 0x54, 0x2b, 0x29, 0x78,
+ 0x47, 0x37, 0x40, 0x66, 0x62, 0x35, 0x6a, 0x55, 0x2c, 0x52,
+ 0x61, 0x41, 0x7d, 0x5c, 0x7c, 0x70, 0x30, 0x25, 0x2a, 0x23 };
+ unsigned char ff_raw[46]; /* all-'/' keysalts decode to 0xFF bytes; raw_len of them compared per suite */
+
+ /* One row per crypto line: expected add_crypto() result and, for accepted lines, the expected
+ decoded key+salt over raw_len bytes (raw == NULL skips the content check). Covers: valid AES-128;
+ byte-exact decode; RFC lifetime+MKI, MKI-only (lifetime/MKI disambiguation) and multi-key
+ (loop + buffer reuse); the AES-192 (38-byte) and AES-256 (46-byte, max-length) suite key+salts;
+ a keysalt too short for its suite; both zero-length rejects (space and '|' opts branch); short
+ decode; over-long. */
+ struct {
+ const char *desc;
+ char *crypto;
+ switch_status_t expect;
+ const unsigned char *raw;
+ size_t raw_len;
+ } cases[] = {
+ { "well-formed keysalt (AES-128)", valid_crypto, SWITCH_STATUS_SUCCESS, NULL, 0 },
+ { "byte-exact decode (AES-128)", nonzero_crypto, SWITCH_STATUS_SUCCESS, ff_raw, 30 },
+ { "RFC lifetime+MKI key", rfc_full, SWITCH_STATUS_SUCCESS, rfc_full_raw, 30 },
+ { "RFC MKI-only key", rfc_mki_only, SWITCH_STATUS_SUCCESS, NULL, 0 },
+ { "RFC multi-key attribute", rfc_multikey, SWITCH_STATUS_SUCCESS, rfc_full_raw, 30 },
+ { "AES-192 keysalt (38-byte)", aes192_crypto, SWITCH_STATUS_SUCCESS, ff_raw, 38 },
+ { "AES-256 keysalt (46-byte)", aes256_crypto, SWITCH_STATUS_SUCCESS, ff_raw, 46 },
+ { "keysalt too short for AES-256", aes256_short, SWITCH_STATUS_FALSE, NULL, 0 },
+ { "zero-length keysalt (space)", zero_len_space, SWITCH_STATUS_FALSE, NULL, 0 },
+ { "zero-length keysalt (pipe)", zero_len_pipe, SWITCH_STATUS_FALSE, NULL, 0 },
+ { "short keysalt", short_keysalt, SWITCH_STATUS_FALSE, NULL, 0 },
+ { "over-long keysalt", overlong_keysalt, SWITCH_STATUS_FALSE, NULL, 0 },
+ { "zero-length, long tail", zero_len_tail, SWITCH_STATUS_FALSE, NULL, 0 },
+ };
+
+ memset(ff_raw, 0xFF, sizeof(ff_raw));
+
+ status = switch_ivr_originate(NULL, &session, &cause, "null/+15553334444", 2, NULL, NULL, NULL, NULL, NULL, SOF_NONE, NULL, NULL);
+ if (!session) {
+ fst_fail("failed to originate session");
+ goto add_crypto_bounds_done;
+ }
+ fst_xcheck(status == SWITCH_STATUS_SUCCESS, "originate must succeed");
+
+ for (i = 0; i < (int)(sizeof(cases) / sizeof(cases[0])); i++) {
+ memset(&ssec, 0, sizeof(ssec));
+ ssec.remote_crypto_key = cases[i].crypto;
+
+ status = switch_core_media_add_crypto(session, &ssec, SWITCH_RTP_CRYPTO_RECV);
+ fst_xcheck(status == cases[i].expect,
+ switch_core_sprintf(fst_pool, "add_crypto(%s): unexpected status", cases[i].desc));
+
+ if (cases[i].raw) {
+ fst_xcheck(memcmp(ssec.remote_raw_key, cases[i].raw, cases[i].raw_len) == 0,
+ switch_core_sprintf(fst_pool, "add_crypto(%s): decoded key+salt mismatch", cases[i].desc));
+ }
+ }
+
+ add_crypto_bounds_done:
+ if (session) {
+ switch_channel_hangup(switch_core_session_get_channel(session), SWITCH_CAUSE_NORMAL_CLEARING);
+ switch_core_session_rwunlock(session);
+ }
+ }
+ FST_TEST_END()
+ }
+ FST_SUITE_END()
+}
+FST_CORE_END()