This commit resolves issue #46. The GCM mode was using the wrong master SALT length. The master SALT should be 96 bits instead of 112 bits. Note, GCM mode uses the legacy CTR mode for the KDF. The legagacy CTR mode cipher implementations assume a 112 bit SALT. Changes to the cipher abstraction layer API are required to provide the ability to specify the SALT length. For now this commit modifies the SRTP layer to ensure the SALT is zero-appended before initializing the KDF. This commit also provides public definitions for the GCM cipher suite master key sizes to avoid confusion for application developers.
diff --git a/crypto/cipher/aes_gcm_ossl.c b/crypto/cipher/aes_gcm_ossl.c
index f36ce9d..f281025 100644
--- a/crypto/cipher/aes_gcm_ossl.c
+++ b/crypto/cipher/aes_gcm_ossl.c
@@ -73,7 +73,7 @@
/*
* This function allocates a new instance of this crypto engine.
- * The key_len parameter should be one of 30 or 46 for
+ * The key_len parameter should be one of 28 or 44 for
* AES-128-GCM or AES-256-GCM respectively. Note that the
* key length includes the 14 byte salt value that is used when
* initializing the KDF.
@@ -89,8 +89,8 @@
/*
* Verify the key_len is valid for one of: AES-128/256
*/
- if (key_len != AES_128_KEYSIZE_WSALT &&
- key_len != AES_256_KEYSIZE_WSALT) {
+ if (key_len != AES_128_GCM_KEYSIZE_WSALT &&
+ key_len != AES_256_GCM_KEYSIZE_WSALT) {
return (err_status_bad_param);
}
@@ -108,14 +108,14 @@
/* increment ref_count */
switch (key_len) {
- case AES_128_KEYSIZE_WSALT:
+ case AES_128_GCM_KEYSIZE_WSALT:
(*c)->type = &aes_gcm_128_openssl;
(*c)->algorithm = AES_128_GCM;
aes_gcm_128_openssl.ref_count++;
((aes_gcm_ctx_t*)(*c)->state)->key_size = AES_128_KEYSIZE;
((aes_gcm_ctx_t*)(*c)->state)->tag_len = GCM_AUTH_TAG_LEN;
break;
- case AES_256_KEYSIZE_WSALT:
+ case AES_256_GCM_KEYSIZE_WSALT:
(*c)->type = &aes_gcm_256_openssl;
(*c)->algorithm = AES_256_GCM;
aes_gcm_256_openssl.ref_count++;
@@ -376,11 +376,11 @@
* values we're derived from independent test code
* using OpenSSL.
*/
-uint8_t aes_gcm_test_case_0_key[AES_128_KEYSIZE_WSALT] = {
+uint8_t aes_gcm_test_case_0_key[AES_128_GCM_KEYSIZE_WSALT] = {
0xfe, 0xff, 0xe9, 0x92, 0x86, 0x65, 0x73, 0x1c,
0x6d, 0x6a, 0x8f, 0x94, 0x67, 0x30, 0x83, 0x08,
0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08,
- 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e,
+ 0x09, 0x0a, 0x0b, 0x0c,
};
uint8_t aes_gcm_test_case_0_iv[12] = {
@@ -419,7 +419,7 @@
};
cipher_test_case_t aes_gcm_test_case_0 = {
- AES_128_KEYSIZE_WSALT, /* octets in key */
+ AES_128_GCM_KEYSIZE_WSALT, /* octets in key */
aes_gcm_test_case_0_key, /* key */
aes_gcm_test_case_0_iv, /* packet index */
60, /* octets in plaintext */
@@ -431,13 +431,13 @@
NULL /* pointer to next testcase */
};
-uint8_t aes_gcm_test_case_1_key[AES_256_KEYSIZE_WSALT] = {
+uint8_t aes_gcm_test_case_1_key[AES_256_GCM_KEYSIZE_WSALT] = {
0xfe, 0xff, 0xe9, 0x92, 0x86, 0x65, 0x73, 0x1c,
0xa5, 0x59, 0x09, 0xc5, 0x54, 0x66, 0x93, 0x1c,
0xaf, 0xf5, 0x26, 0x9a, 0x21, 0xd5, 0x14, 0xb2,
0x6d, 0x6a, 0x8f, 0x94, 0x67, 0x30, 0x83, 0x08,
0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08,
- 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e,
+ 0x09, 0x0a, 0x0b, 0x0c,
};
@@ -477,7 +477,7 @@
};
cipher_test_case_t aes_gcm_test_case_1 = {
- AES_256_KEYSIZE_WSALT, /* octets in key */
+ AES_256_GCM_KEYSIZE_WSALT, /* octets in key */
aes_gcm_test_case_1_key, /* key */
aes_gcm_test_case_1_iv, /* packet index */
60, /* octets in plaintext */
diff --git a/crypto/include/aes_gcm_ossl.h b/crypto/include/aes_gcm_ossl.h
index 87b3e9f..8e7711d 100644
--- a/crypto/include/aes_gcm_ossl.h
+++ b/crypto/include/aes_gcm_ossl.h
@@ -47,6 +47,7 @@
#define AES_GCM_OSSL_H
#include "cipher.h"
+#include "srtp.h"
#include <openssl/evp.h>
#include <openssl/aes.h>
diff --git a/crypto/test/cipher_driver.c b/crypto/test/cipher_driver.c
index c4b4d66..26992e1 100644
--- a/crypto/test/cipher_driver.c
+++ b/crypto/test/cipher_driver.c
@@ -200,11 +200,11 @@
cipher_driver_test_array_throughput(&aes_icm_256, 46, num_cipher);
for (num_cipher=1; num_cipher < max_num_cipher; num_cipher *=8) {
- cipher_driver_test_array_throughput(&aes_gcm_128_openssl, 30, num_cipher);
+ cipher_driver_test_array_throughput(&aes_gcm_128_openssl, AES_128_GCM_KEYSIZE_WSALT, num_cipher);
}
for (num_cipher=1; num_cipher < max_num_cipher; num_cipher *=8) {
- cipher_driver_test_array_throughput(&aes_gcm_256_openssl, 46, num_cipher);
+ cipher_driver_test_array_throughput(&aes_gcm_256_openssl, AES_256_GCM_KEYSIZE_WSALT, num_cipher);
}
#endif
}
@@ -287,7 +287,7 @@
#ifdef OPENSSL
/* run the throughput test on the aes_gcm_128_openssl cipher */
- status = cipher_type_alloc(&aes_gcm_128_openssl, &c, 30);
+ status = cipher_type_alloc(&aes_gcm_128_openssl, &c, AES_128_GCM_KEYSIZE_WSALT);
if (status) {
fprintf(stderr, "error: can't allocate GCM 128 cipher\n");
exit(status);
@@ -306,7 +306,7 @@
check_status(status);
/* run the throughput test on the aes_gcm_256_openssl cipher */
- status = cipher_type_alloc(&aes_gcm_256_openssl, &c, 46);
+ status = cipher_type_alloc(&aes_gcm_256_openssl, &c, AES_256_GCM_KEYSIZE_WSALT);
if (status) {
fprintf(stderr, "error: can't allocate GCM 256 cipher\n");
exit(status);
diff --git a/crypto/test/stat_driver.c b/crypto/test/stat_driver.c
index b2738e8..8c11a16 100644
--- a/crypto/test/stat_driver.c
+++ b/crypto/test/stat_driver.c
@@ -12,6 +12,7 @@
#include "err.h"
#include "stat.h"
+#include "srtp.h"
#include "cipher.h"
@@ -141,7 +142,7 @@
for (i=0; i < 2500; i++) {
buffer[i] = 0;
}
- err_check(cipher_type_alloc(&aes_gcm_128_openssl, &c, 30));
+ err_check(cipher_type_alloc(&aes_gcm_128_openssl, &c, AES_128_GCM_KEYSIZE_WSALT));
err_check(cipher_init(c, key));
err_check(cipher_set_iv(c, &nonce, direction_encrypt));
err_check(cipher_encrypt(c, buffer, &buf_len));
@@ -170,7 +171,7 @@
for (i=0; i < 2500; i++) {
buffer[i] = 0;
}
- err_check(cipher_type_alloc(&aes_gcm_256_openssl, &c, 46));
+ err_check(cipher_type_alloc(&aes_gcm_256_openssl, &c, AES_256_GCM_KEYSIZE_WSALT));
err_check(cipher_init(c, key));
err_check(cipher_set_iv(c, &nonce, direction_encrypt));
err_check(cipher_encrypt(c, buffer, &buf_len));
diff --git a/include/srtp.h b/include/srtp.h
index a9c73d4..d64f946 100644
--- a/include/srtp.h
+++ b/include/srtp.h
@@ -94,6 +94,10 @@
* as part of the IV formation logic applied to each RTP packet.
*/
#define SRTP_AEAD_SALT_LEN 12
+#define AES_128_GCM_KEYSIZE_WSALT SRTP_AEAD_SALT_LEN + 16
+#define AES_192_GCM_KEYSIZE_WSALT SRTP_AEAD_SALT_LEN + 24
+#define AES_256_GCM_KEYSIZE_WSALT SRTP_AEAD_SALT_LEN + 32
+
/*
* nota bene: since libSRTP doesn't support the use of the MKI, the
diff --git a/srtp/srtp.c b/srtp/srtp.c
index 0144dcb..328319f 100644
--- a/srtp/srtp.c
+++ b/srtp/srtp.c
@@ -46,6 +46,9 @@
#include "srtp.h"
#include "ekt.h" /* for SRTP Encrypted Key Transport */
#include "alloc.h" /* for crypto_alloc() */
+#ifdef OPENSSL
+#include "aes_gcm_ossl.h" /* for AES GCM mode */
+#endif
#ifndef SRTP_KERNEL
# include <limits.h>
@@ -451,24 +454,37 @@
/* TODO: kdf algorithm, master key length, and master salt length should
* be part of srtp_policy_t. */
rtp_keylen = cipher_get_key_length(srtp->rtp_cipher);
- if (rtp_keylen > kdf_keylen)
- kdf_keylen = rtp_keylen;
-
rtcp_keylen = cipher_get_key_length(srtp->rtcp_cipher);
- if (rtcp_keylen > kdf_keylen)
- kdf_keylen = rtcp_keylen;
+ rtp_base_key_len = base_key_length(srtp->rtp_cipher->type, rtp_keylen);
+ rtp_salt_len = rtp_keylen - rtp_base_key_len;
+
+ if (rtp_keylen > kdf_keylen) {
+ kdf_keylen = 46; /* AES-CTR mode is always used for KDF */
+ }
+
+ if (rtcp_keylen > kdf_keylen) {
+ kdf_keylen = 46; /* AES-CTR mode is always used for KDF */
+ }
+
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);
+ debug_print(mod_srtp, "kdf key len: %d", kdf_keylen);
+ debug_print(mod_srtp, "rtp salt len: %d", rtp_salt_len);
+
+ /*
+ * Make sure the key given to us is 'zero' appended. GCM
+ * mode uses a shorter master SALT (96 bits), but still relies on
+ * 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));
/* initialize KDF state */
- stat = srtp_kdf_init(&kdf, AES_ICM, (const uint8_t *)key, kdf_keylen);
+ stat = srtp_kdf_init(&kdf, AES_ICM, (const uint8_t *)tmp_key, kdf_keylen);
if (stat) {
return err_status_init_fail;
}
-
- rtp_base_key_len = base_key_length(srtp->rtp_cipher->type, rtp_keylen);
- rtp_salt_len = rtp_keylen - rtp_base_key_len;
- debug_print(mod_srtp, "rtp salt len: %d", rtp_salt_len);
/* generate encryption key */
stat = srtp_kdf_generate(&kdf, label_rtp_encryption,
@@ -478,6 +494,8 @@
octet_string_set_to_zero(tmp_key, MAX_SRTP_KEY_LEN);
return err_status_init_fail;
}
+ debug_print(mod_srtp, "cipher key: %s",
+ octet_string_hex_string(tmp_key, rtp_base_key_len));
/*
* if the cipher in the srtp context uses a salt, then we need
@@ -496,8 +514,6 @@
}
memcpy(srtp->salt, tmp_key + rtp_base_key_len, SRTP_AEAD_SALT_LEN);
}
- debug_print(mod_srtp, "cipher key: %s",
- octet_string_hex_string(tmp_key, rtp_base_key_len));
if (rtp_salt_len > 0) {
debug_print(mod_srtp, "cipher salt: %s",
octet_string_hex_string(tmp_key + rtp_base_key_len, rtp_salt_len));
@@ -2044,7 +2060,7 @@
void
crypto_policy_set_aes_gcm_128_8_auth(crypto_policy_t *p) {
p->cipher_type = AES_128_GCM;
- p->cipher_key_len = AES_128_KEYSIZE_WSALT;
+ p->cipher_key_len = AES_128_GCM_KEYSIZE_WSALT;
p->auth_type = NULL_AUTH; /* GCM handles the auth for us */
p->auth_key_len = 0;
p->auth_tag_len = 8; /* 8 octet tag length */
@@ -2057,7 +2073,7 @@
void
crypto_policy_set_aes_gcm_256_8_auth(crypto_policy_t *p) {
p->cipher_type = AES_256_GCM;
- p->cipher_key_len = AES_256_KEYSIZE_WSALT;
+ p->cipher_key_len = AES_256_GCM_KEYSIZE_WSALT;
p->auth_type = NULL_AUTH; /* GCM handles the auth for us */
p->auth_key_len = 0;
p->auth_tag_len = 8; /* 8 octet tag length */
@@ -2070,7 +2086,7 @@
void
crypto_policy_set_aes_gcm_128_8_only_auth(crypto_policy_t *p) {
p->cipher_type = AES_128_GCM;
- p->cipher_key_len = AES_128_KEYSIZE_WSALT;
+ p->cipher_key_len = AES_128_GCM_KEYSIZE_WSALT;
p->auth_type = NULL_AUTH; /* GCM handles the auth for us */
p->auth_key_len = 0;
p->auth_tag_len = 8; /* 8 octet tag length */
@@ -2083,7 +2099,7 @@
void
crypto_policy_set_aes_gcm_256_8_only_auth(crypto_policy_t *p) {
p->cipher_type = AES_256_GCM;
- p->cipher_key_len = AES_256_KEYSIZE_WSALT;
+ p->cipher_key_len = AES_256_GCM_KEYSIZE_WSALT;
p->auth_type = NULL_AUTH; /* GCM handles the auth for us */
p->auth_key_len = 0;
p->auth_tag_len = 8; /* 8 octet tag length */
diff --git a/test/rtpw_test_gcm.sh b/test/rtpw_test_gcm.sh
index dfac335..b34d60d 100755
--- a/test/rtpw_test_gcm.sh
+++ b/test/rtpw_test_gcm.sh
@@ -18,7 +18,7 @@
if test -x $RTPW; then
-GCMARGS128="-k 012345678901234567890123456789012345678901234567890123456789 -g -e 128"
+GCMARGS128="-k 01234567890123456789012345678901234567890123456789012345 -g -e 128"
echo $0 ": starting GCM mode 128-bit rtpw receiver process... "
exec $RTPW $* $GCMARGS128 -r 127.0.0.1 $DEST_PORT &
@@ -62,7 +62,7 @@
-GCMARGS256="-k 01234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901 -g -e 256"
+GCMARGS256="-k 0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567 -g -e 256"
echo $0 ": starting GCM mode 256-bit rtpw receiver process... "
exec $RTPW $* $GCMARGS256 -r 127.0.0.1 $DEST_PORT &
diff --git a/test/srtp_driver.c b/test/srtp_driver.c
index 783d518..8872971 100644
--- a/test/srtp_driver.c
+++ b/test/srtp_driver.c
@@ -1670,7 +1670,7 @@
{ ssrc_any_outbound, 0 }, /* SSRC */
{ /* SRTP policy */
AES_128_GCM, /* cipher type */
- 30, /* cipher key length in octets */
+ AES_128_GCM_KEYSIZE_WSALT, /* cipher key length in octets */
NULL_AUTH, /* authentication func type */
0, /* auth key length in octets */
8, /* auth tag length in octets */
@@ -1678,7 +1678,7 @@
},
{ /* SRTCP policy */
AES_128_GCM, /* cipher type */
- 30, /* cipher key length in octets */
+ AES_128_GCM_KEYSIZE_WSALT, /* cipher key length in octets */
NULL_AUTH, /* authentication func type */
0, /* auth key length in octets */
8, /* auth tag length in octets */
@@ -1695,7 +1695,7 @@
{ ssrc_any_outbound, 0 }, /* SSRC */
{ /* SRTP policy */
AES_128_GCM, /* cipher type */
- 30, /* cipher key length in octets */
+ AES_128_GCM_KEYSIZE_WSALT, /* cipher key length in octets */
NULL_AUTH, /* authentication func type */
0, /* auth key length in octets */
8, /* auth tag length in octets */
@@ -1703,7 +1703,7 @@
},
{ /* SRTCP policy */
AES_128_GCM, /* cipher type */
- 30, /* cipher key length in octets */
+ AES_128_GCM_KEYSIZE_WSALT, /* cipher key length in octets */
NULL_AUTH, /* authentication func type */
0, /* auth key length in octets */
8, /* auth tag length in octets */
@@ -1720,7 +1720,7 @@
{ ssrc_any_outbound, 0 }, /* SSRC */
{ /* SRTP policy */
AES_256_GCM, /* cipher type */
- 46, /* cipher key length in octets */
+ AES_256_GCM_KEYSIZE_WSALT, /* cipher key length in octets */
NULL_AUTH, /* authentication func type */
0, /* auth key length in octets */
8, /* auth tag length in octets */
@@ -1728,7 +1728,7 @@
},
{ /* SRTCP policy */
AES_256_GCM, /* cipher type */
- 46, /* cipher key length in octets */
+ AES_256_GCM_KEYSIZE_WSALT, /* cipher key length in octets */
NULL_AUTH, /* authentication func type */
0, /* auth key length in octets */
8, /* auth tag length in octets */
@@ -1745,7 +1745,7 @@
{ ssrc_any_outbound, 0 }, /* SSRC */
{ /* SRTP policy */
AES_256_GCM, /* cipher type */
- 46, /* cipher key length in octets */
+ AES_256_GCM_KEYSIZE_WSALT, /* cipher key length in octets */
NULL_AUTH, /* authentication func type */
0, /* auth key length in octets */
8, /* auth tag length in octets */
@@ -1753,7 +1753,7 @@
},
{ /* SRTCP policy */
AES_256_GCM, /* cipher type */
- 46, /* cipher key length in octets */
+ AES_256_GCM_KEYSIZE_WSALT, /* cipher key length in octets */
NULL_AUTH, /* authentication func type */
0, /* auth key length in octets */
8, /* auth tag length in octets */