Skip to content

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

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

Merged
merged 5 commits into from
Sep 9, 2021
Merged

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

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
89a5f70
Use a full-length key even with null ciphers
Sep 8, 2021
15e64b9
Accommodate shorter GCM keys
Sep 8, 2021
f383031
Formatting
Sep 8, 2021
c4a5455
Set full-size key length in crypto policies
Sep 9, 2021
812a683
Set full-size key length in crypto policies
Sep 9, 2021
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
34 changes: 33 additions & 1 deletion srtp/srtp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -776,6 +776,26 @@ static inline int base_key_length(const srtp_cipher_type_t *cipher,
}
}

/* 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 •
edited
Loading

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

@paulej paulej Sep 13, 2021

Choose a reason for hiding this comment

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

Yes.

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.

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

default:
return 0;
}
}

unsigned int srtp_validate_policy_master_keys(const srtp_policy_t *policy)
{
unsigned long i = 0;
Expand Down Expand Up @@ -870,6 +890,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 +927,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 +947,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);
Copy link
Contributor

@paulej paulej Sep 9, 2021

Choose a reason for hiding this comment

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

extra ; before :

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 +964,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
171 changes: 171 additions & 0 deletions test/srtp_driver.c
View file
Open in desktop
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