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

Use a full-length key even with null ciphers #559

Merged
merged 5 commits into from
Sep 9, 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
44 changes: 37 additions & 7 deletions srtp/srtp.c
Original file line number Diff line number Diff line change
Expand Up @@ -757,22 +757,40 @@ static inline int base_key_length(const srtp_cipher_type_t *cipher,
int key_length)
{
switch (cipher->id) {
case SRTP_NULL_CIPHER:
return 0;
case SRTP_AES_ICM_128:
case SRTP_AES_ICM_192:
case SRTP_AES_ICM_256:
/* The legacy modes are derived from
* the configured key length on the policy */
return key_length - SRTP_SALT_LEN;
break;
case SRTP_AES_GCM_128:
return key_length - SRTP_AEAD_SALT_LEN;
break;
case SRTP_AES_GCM_256:
return key_length - SRTP_AEAD_SALT_LEN;
break;
default:
return key_length;
break;
}
}

/* Get the key length that the application should supply for the given cipher */
static inline int full_key_length(const srtp_cipher_type_t *cipher)
{
switch (cipher->id) {
case SRTP_NULL_CIPHER:
case SRTP_AES_ICM_128:
return SRTP_AES_ICM_128_KEY_LEN_WSALT;
case SRTP_AES_ICM_192:
return SRTP_AES_ICM_192_KEY_LEN_WSALT;
case SRTP_AES_ICM_256:
return SRTP_AES_ICM_256_KEY_LEN_WSALT;
case SRTP_AES_GCM_128:
return SRTP_AES_GCM_128_KEY_LEN_WSALT;
case SRTP_AES_GCM_256:
return SRTP_AES_ICM_256_KEY_LEN_WSALT;
Copy link
Contributor

@lminiero lminiero Sep 13, 2021

Choose a reason for hiding this comment

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

Shouldn't this be SRTP_AES_GCM_256_KEY_LEN_WSALT? SRTP_AES_ICM_256_KEY_LEN_WSALT is 32+14 while SRTP_AES_GCM_256_KEY_LEN_WSALT is 32+12. This is causing SRTP/SRTCP errors in our WebRTC servers when GCM-256 is used.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ack, thanks, I created a mini-PR in #562 that addresses the typo.

default:
return 0;
}
}

Expand Down Expand Up @@ -870,6 +888,7 @@ srtp_err_status_t srtp_stream_init_keys(srtp_stream_ctx_t *srtp,
srtp_err_status_t stat;
srtp_kdf_t kdf;
uint8_t tmp_key[MAX_SRTP_KEY_LEN];
int input_keylen, input_keylen_rtcp;
int kdf_keylen = 30, rtp_keylen, rtcp_keylen;
int rtp_base_key_len, rtp_salt_len;
int rtcp_base_key_len, rtcp_salt_len;
Expand Down Expand Up @@ -906,6 +925,12 @@ srtp_err_status_t srtp_stream_init_keys(srtp_stream_ctx_t *srtp,

session_keys->mki_size = master_key->mki_size;

input_keylen = full_key_length(session_keys->rtp_cipher->type);
input_keylen_rtcp = full_key_length(session_keys->rtcp_cipher->type);
if (input_keylen_rtcp > input_keylen) {
input_keylen = input_keylen_rtcp;
}

rtp_keylen = srtp_cipher_get_key_length(session_keys->rtp_cipher);
rtcp_keylen = srtp_cipher_get_key_length(session_keys->rtcp_cipher);
rtp_base_key_len =
Expand All @@ -920,6 +945,11 @@ srtp_err_status_t srtp_stream_init_keys(srtp_stream_ctx_t *srtp,
kdf_keylen = 46; /* AES-CTR mode is always used for KDF */
}

if (input_keylen > kdf_keylen) {
kdf_keylen = 46; /* AES-CTR mode is always used for KDF */
}

debug_print(mod_srtp, "input key len: %d", input_keylen);
debug_print(mod_srtp, "srtp key len: %d", rtp_keylen);
debug_print(mod_srtp, "srtcp key len: %d", rtcp_keylen);
debug_print(mod_srtp, "base key len: %d", rtp_base_key_len);
Expand All @@ -932,7 +962,7 @@ srtp_err_status_t srtp_stream_init_keys(srtp_stream_ctx_t *srtp,
* the legacy CTR mode KDF, which uses a 112 bit master SALT.
*/
memset(tmp_key, 0x0, MAX_SRTP_KEY_LEN);
memcpy(tmp_key, key, (rtp_base_key_len + rtp_salt_len));
memcpy(tmp_key, key, input_keylen);

/* initialize KDF state */
#if defined(OPENSSL) && defined(OPENSSL_KDF)
Expand Down Expand Up @@ -3247,7 +3277,7 @@ void srtp_crypto_policy_set_null_cipher_hmac_sha1_80(srtp_crypto_policy_t *p)
*/

p->cipher_type = SRTP_NULL_CIPHER;
p->cipher_key_len = 0;
p->cipher_key_len = 16;
p->auth_type = SRTP_HMAC_SHA1;
p->auth_key_len = 20;
p->auth_tag_len = 10;
Expand All @@ -3261,7 +3291,7 @@ void srtp_crypto_policy_set_null_cipher_hmac_null(srtp_crypto_policy_t *p)
*/

p->cipher_type = SRTP_NULL_CIPHER;
p->cipher_key_len = 0;
p->cipher_key_len = 16;
p->auth_type = SRTP_NULL_AUTH;
p->auth_key_len = 0;
p->auth_tag_len = 0;
Expand Down
171 changes: 171 additions & 0 deletions test/srtp_driver.c
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@

srtp_err_status_t srtp_validate(void);

srtp_err_status_t srtp_validate_null(void);

#ifdef GCM
srtp_err_status_t srtp_validate_gcm(void);
#endif
Expand Down Expand Up @@ -442,6 +444,15 @@ int main(int argc, char *argv[])
exit(1);
}

printf("testing srtp_protect and srtp_unprotect against "
"reference packet using null cipher and HMAC\n");
if (srtp_validate_null() == srtp_err_status_ok) {
printf("passed\n\n");
} else {
printf("failed\n");
exit(1);
}

#ifdef GCM
printf("testing srtp_protect and srtp_unprotect against "
"reference packet using GCM\n");
Expand Down Expand Up @@ -1806,6 +1817,166 @@ srtp_err_status_t srtp_validate()
return srtp_err_status_ok;
}

/*
* srtp_validate_null() verifies the correctness of libsrtp by comparing
* some computed packets against some pre-computed reference values.
* These packets were made with a policy that applies null encryption
* and HMAC authentication.
*/

srtp_err_status_t srtp_validate_null()
{
// clang-format off
uint8_t srtp_plaintext_ref[28] = {
0x80, 0x0f, 0x12, 0x34, 0xde, 0xca, 0xfb, 0xad,
0xca, 0xfe, 0xba, 0xbe, 0xab, 0xab, 0xab, 0xab,
0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab,
0xab, 0xab, 0xab, 0xab
};
uint8_t srtp_plaintext[38] = {
0x80, 0x0f, 0x12, 0x34, 0xde, 0xca, 0xfb, 0xad,
0xca, 0xfe, 0xba, 0xbe, 0xab, 0xab, 0xab, 0xab,
0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab,
0xab, 0xab, 0xab, 0xab, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00
};
uint8_t srtp_ciphertext[38] = {
0x80, 0x0f, 0x12, 0x34, 0xde, 0xca, 0xfb, 0xad,
0xca, 0xfe, 0xba, 0xbe, 0xab, 0xab, 0xab, 0xab,
0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab,
0xab, 0xab, 0xab, 0xab, 0xab, 0xa1, 0x36, 0x27,
0x0b, 0x67, 0x91, 0x34, 0xce, 0x9b
};
uint8_t rtcp_plaintext_ref[24] = {
0x81, 0xc8, 0x00, 0x0b, 0xca, 0xfe, 0xba, 0xbe,
0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab,
0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab,
};
uint8_t rtcp_plaintext[38] = {
0x81, 0xc8, 0x00, 0x0b, 0xca, 0xfe, 0xba, 0xbe,
0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab,
0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00
};
uint8_t srtcp_ciphertext[38] = {
0x81, 0xc8, 0x00, 0x0b, 0xca, 0xfe, 0xba, 0xbe,
0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab,
0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab, 0xab,
0x00, 0x00, 0x00, 0x01, 0xfe, 0x88, 0xc7, 0xfd,
0xfd, 0x37, 0xeb, 0xce, 0x61, 0x5d,
};
// clang-format on

srtp_t srtp_snd, srtp_recv;
srtp_err_status_t status;
int len;
srtp_policy_t policy;

/*
* create a session with a single stream using the default srtp
* policy and with the SSRC value 0xcafebabe
*/
memset(&policy, 0, sizeof(policy));
srtp_crypto_policy_set_null_cipher_hmac_sha1_80(&policy.rtp);
srtp_crypto_policy_set_null_cipher_hmac_sha1_80(&policy.rtcp);
policy.ssrc.type = ssrc_specific;
policy.ssrc.value = 0xcafebabe;
policy.key = test_key;
policy.deprecated_ekt = NULL;
policy.window_size = 128;
policy.allow_repeat_tx = 0;
policy.next = NULL;

status = srtp_create(&srtp_snd, &policy);
if (status) {
return status;
}

/*
* protect plaintext, then compare with ciphertext
*/
len = 28;
status = srtp_protect(srtp_snd, srtp_plaintext, &len);
if (status || (len != 38)) {
return srtp_err_status_fail;
}

debug_print(mod_driver, "ciphertext:\n %s",
octet_string_hex_string(srtp_plaintext, len));
debug_print(mod_driver, "ciphertext reference:\n %s",
octet_string_hex_string(srtp_ciphertext, len));

if (srtp_octet_string_is_eq(srtp_plaintext, srtp_ciphertext, len)) {
return srtp_err_status_fail;
}

/*
* protect plaintext rtcp, then compare with srtcp ciphertext
*/
len = 24;
status = srtp_protect_rtcp(srtp_snd, rtcp_plaintext, &len);
if (status || (len != 38)) {
return srtp_err_status_fail;
}

debug_print(mod_driver, "srtcp ciphertext:\n %s",
octet_string_hex_string(rtcp_plaintext, len));
debug_print(mod_driver, "srtcp ciphertext reference:\n %s",
octet_string_hex_string(srtcp_ciphertext, len));

if (srtp_octet_string_is_eq(rtcp_plaintext, srtcp_ciphertext, len)) {
return srtp_err_status_fail;
}

/*
* create a receiver session context comparable to the one created
* above - we need to do this so that the replay checking doesn't
* complain
*/
status = srtp_create(&srtp_recv, &policy);
if (status) {
return status;
}

/*
* unprotect ciphertext, then compare with plaintext
*/
status = srtp_unprotect(srtp_recv, srtp_ciphertext, &len);
if (status || (len != 28)) {
return status;
}

if (srtp_octet_string_is_eq(srtp_ciphertext, srtp_plaintext_ref, len)) {
return srtp_err_status_fail;
}

/*
* unprotect srtcp ciphertext, then compare with rtcp plaintext
*/
len = 38;
status = srtp_unprotect_rtcp(srtp_recv, srtcp_ciphertext, &len);
if (status || (len != 24)) {
return status;
}

if (srtp_octet_string_is_eq(srtcp_ciphertext, rtcp_plaintext_ref, len)) {
return srtp_err_status_fail;
}

status = srtp_dealloc(srtp_snd);
if (status) {
return status;
}

status = srtp_dealloc(srtp_recv);
if (status) {
return status;
}

return srtp_err_status_ok;
}

#ifdef GCM
/*
* srtp_validate_gcm() verifies the correctness of libsrtp by comparing
Expand Down