From 2eaf98bd2b37dd5c2eddeaad05d546b6003f7c10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pascal=20B=C3=BChler?= Date: Wed, 26 Jun 2024 23:31:45 +0200 Subject: [PATCH] change srtp_cipher_encrypt to append the tag generated This makes it symmetric with the srtp_cipher_decrypt function that will remove the tag. Currently most of the backends would have cached the tag internally and returned it in the srtp_cipher_get_tag function, this removes that extra complexity. --- crypto/cipher/aes_gcm_mbedtls.c | 39 ++++------------- crypto/cipher/aes_gcm_nss.c | 60 +------------------------ crypto/cipher/aes_gcm_ossl.c | 56 ++++++++++-------------- crypto/cipher/aes_gcm_wssl.c | 77 ++++++++++++++------------------- crypto/cipher/aes_icm.c | 2 - crypto/cipher/aes_icm_mbedtls.c | 3 -- crypto/cipher/aes_icm_nss.c | 3 -- crypto/cipher/aes_icm_ossl.c | 3 -- crypto/cipher/aes_icm_wssl.c | 3 -- crypto/cipher/cipher.c | 56 +++--------------------- crypto/cipher/null_cipher.c | 1 - crypto/include/aes_gcm.h | 3 -- crypto/include/cipher.h | 12 ----- crypto/test/cipher_driver.c | 10 ++++- srtp.def | 1 - srtp/srtp.c | 63 ++++++--------------------- 16 files changed, 92 insertions(+), 300 deletions(-) diff --git a/crypto/cipher/aes_gcm_mbedtls.c b/crypto/cipher/aes_gcm_mbedtls.c index 5b190963b..4a6da1cfb 100644 --- a/crypto/cipher/aes_gcm_mbedtls.c +++ b/crypto/cipher/aes_gcm_mbedtls.c @@ -291,16 +291,16 @@ static srtp_err_status_t srtp_aes_gcm_mbedtls_encrypt(void *cv, int errCode = 0; if (c->dir != srtp_direction_encrypt && c->dir != srtp_direction_decrypt) { - return (srtp_err_status_bad_param); + return srtp_err_status_bad_param; } - if (*dst_len < src_len) { + if (*dst_len < src_len + c->tag_len) { return srtp_err_status_buffer_small; } errCode = mbedtls_gcm_crypt_and_tag(c->ctx, MBEDTLS_GCM_ENCRYPT, src_len, c->iv, c->iv_len, c->aad, c->aad_size, - src, dst, c->tag_len, c->tag); + src, dst, c->tag_len, dst + src_len); c->aad_size = 0; if (errCode != 0) { @@ -308,32 +308,9 @@ static srtp_err_status_t srtp_aes_gcm_mbedtls_encrypt(void *cv, return srtp_err_status_bad_param; } - *dst_len = src_len; - - return (srtp_err_status_ok); -} + *dst_len = src_len + c->tag_len; -/* - * This function calculates and returns the GCM tag for a given context. - * This should be called after encrypting the data. The *len value - * is increased by the tag size. The caller must ensure that *buf has - * enough room to accept the appended tag. - * - * Parameters: - * c Crypto context - * buf data to encrypt - * len length of encrypt buffer - */ -static srtp_err_status_t srtp_aes_gcm_mbedtls_get_tag(void *cv, - uint8_t *buf, - size_t *len) -{ - FUNC_ENTRY(); - srtp_aes_gcm_ctx_t *c = (srtp_aes_gcm_ctx_t *)cv; - debug_print(srtp_mod_aes_gcm, "appended tag size: %zu", c->tag_len); - *len = c->tag_len; - memcpy(buf, c->tag, c->tag_len); - return (srtp_err_status_ok); + return srtp_err_status_ok; } /* @@ -393,6 +370,7 @@ static const char srtp_aes_gcm_256_mbedtls_description[] = /* * This is the vector function table for this crypto engine. */ +/* clang-format off */ const srtp_cipher_type_t srtp_aes_gcm_128 = { srtp_aes_gcm_mbedtls_alloc, srtp_aes_gcm_mbedtls_dealloc, @@ -401,15 +379,16 @@ const srtp_cipher_type_t srtp_aes_gcm_128 = { srtp_aes_gcm_mbedtls_encrypt, srtp_aes_gcm_mbedtls_decrypt, srtp_aes_gcm_mbedtls_set_iv, - srtp_aes_gcm_mbedtls_get_tag, srtp_aes_gcm_128_mbedtls_description, &srtp_aes_gcm_128_test_case_0, SRTP_AES_GCM_128 }; +/* clang-format on */ /* * This is the vector function table for this crypto engine. */ +/* clang-format off */ const srtp_cipher_type_t srtp_aes_gcm_256 = { srtp_aes_gcm_mbedtls_alloc, srtp_aes_gcm_mbedtls_dealloc, @@ -418,8 +397,8 @@ const srtp_cipher_type_t srtp_aes_gcm_256 = { srtp_aes_gcm_mbedtls_encrypt, srtp_aes_gcm_mbedtls_decrypt, srtp_aes_gcm_mbedtls_set_iv, - srtp_aes_gcm_mbedtls_get_tag, srtp_aes_gcm_256_mbedtls_description, &srtp_aes_gcm_256_test_case_0, SRTP_AES_GCM_256 }; +/* clang-format on */ diff --git a/crypto/cipher/aes_gcm_nss.c b/crypto/cipher/aes_gcm_nss.c index db17b918b..6ecabf069 100644 --- a/crypto/cipher/aes_gcm_nss.c +++ b/crypto/cipher/aes_gcm_nss.c @@ -319,11 +319,6 @@ static srtp_err_status_t srtp_aes_gcm_nss_do_crypto(void *cv, /* * This function encrypts a buffer using AES GCM mode * - * XXX(rlb@ipv.sx): We're required to break off and cache the tag - * here, because the get_tag() method is separate and the tests expect - * encrypt() not to change the size of the plaintext. It might be - * good to update the calling API so that this is cleaner. - * * Parameters: * c Crypto context * buf data to encrypt @@ -335,58 +330,7 @@ static srtp_err_status_t srtp_aes_gcm_nss_encrypt(void *cv, uint8_t *dst, size_t *dst_len) { - srtp_aes_gcm_ctx_t *c = (srtp_aes_gcm_ctx_t *)cv; - - // When we get a non-NULL src buffer, we know that the caller is - // prepared to also take the tag. When we get a NULL src buffer, - // even though there's no data, we need to give NSS a buffer - // where it can write the tag. We can't just use c->tag because - // memcpy has undefined behavior on overlapping ranges. - uint8_t tagbuf[16]; - const uint8_t *non_null_buf = src; - uint8_t *non_null_dst_buf = dst; - if (!non_null_buf && (src_len == 0)) { - non_null_buf = tagbuf; - non_null_dst_buf = tagbuf; - *dst_len = sizeof(tagbuf); - } else if (!non_null_buf) { - return srtp_err_status_bad_param; - } - - srtp_err_status_t status = srtp_aes_gcm_nss_do_crypto( - cv, true, non_null_buf, src_len, non_null_dst_buf, dst_len); - if (status != srtp_err_status_ok) { - return status; - } - - if (*dst_len < c->tag_size) { - return srtp_err_status_bad_param; - } - - memcpy(c->tag, non_null_dst_buf + (*dst_len - c->tag_size), c->tag_size); - *dst_len -= c->tag_size; - return srtp_err_status_ok; -} - -/* - * This function calculates and returns the GCM tag for a given context. - * This should be called after encrypting the data. The *len value - * is increased by the tag size. The caller must ensure that *buf has - * enough room to accept the appended tag. - * - * Parameters: - * c Crypto context - * buf data to encrypt - * len length of encrypt buffer - */ -static srtp_err_status_t srtp_aes_gcm_nss_get_tag(void *cv, - uint8_t *buf, - size_t *len) -{ - srtp_aes_gcm_ctx_t *c = (srtp_aes_gcm_ctx_t *)cv; - *len = c->tag_size; - memcpy(buf, c->tag, c->tag_size); - return (srtp_err_status_ok); + return srtp_aes_gcm_nss_do_crypto(cv, true, src, src_len, dst, dst_len); } /* @@ -442,7 +386,6 @@ const srtp_cipher_type_t srtp_aes_gcm_128 = { srtp_aes_gcm_nss_encrypt, srtp_aes_gcm_nss_decrypt, srtp_aes_gcm_nss_set_iv, - srtp_aes_gcm_nss_get_tag, srtp_aes_gcm_128_nss_description, &srtp_aes_gcm_128_test_case_0, SRTP_AES_GCM_128 @@ -461,7 +404,6 @@ const srtp_cipher_type_t srtp_aes_gcm_256 = { srtp_aes_gcm_nss_encrypt, srtp_aes_gcm_nss_decrypt, srtp_aes_gcm_nss_set_iv, - srtp_aes_gcm_nss_get_tag, srtp_aes_gcm_256_nss_description, &srtp_aes_gcm_256_test_case_0, SRTP_AES_GCM_256 diff --git a/crypto/cipher/aes_gcm_ossl.c b/crypto/cipher/aes_gcm_ossl.c index 692ab8957..3704eb848 100644 --- a/crypto/cipher/aes_gcm_ossl.c +++ b/crypto/cipher/aes_gcm_ossl.c @@ -299,51 +299,34 @@ static srtp_err_status_t srtp_aes_gcm_openssl_encrypt(void *cv, size_t *dst_len) { srtp_aes_gcm_ctx_t *c = (srtp_aes_gcm_ctx_t *)cv; + if (c->dir != srtp_direction_encrypt && c->dir != srtp_direction_decrypt) { - return (srtp_err_status_bad_param); + return srtp_err_status_bad_param; + } + + if (*dst_len < src_len + c->tag_len) { + return srtp_err_status_buffer_small; } /* * Encrypt the data */ EVP_Cipher(c->ctx, dst, src, src_len); - *dst_len = src_len; - return (srtp_err_status_ok); -} - -/* - * This function calculates and returns the GCM tag for a given context. - * This should be called after encrypting the data. The *len value - * is increased by the tag size. The caller must ensure that *buf has - * enough room to accept the appended tag. - * - * Parameters: - * c Crypto context - * buf data to encrypt - * len length of encrypt buffer - */ -static srtp_err_status_t srtp_aes_gcm_openssl_get_tag(void *cv, - uint8_t *buf, - size_t *len) -{ - srtp_aes_gcm_ctx_t *c = (srtp_aes_gcm_ctx_t *)cv; /* * Calculate the tag */ EVP_Cipher(c->ctx, NULL, NULL, 0); /* - * Retreive the tag + * Retrieve the tag */ - if (!EVP_CIPHER_CTX_ctrl(c->ctx, EVP_CTRL_GCM_GET_TAG, c->tag_len, buf)) { - return (srtp_err_status_algo_fail); + if (!EVP_CIPHER_CTX_ctrl(c->ctx, EVP_CTRL_GCM_GET_TAG, c->tag_len, + dst + src_len)) { + return srtp_err_status_algo_fail; } - /* - * Increase encryption length by desired tag size - */ - *len = c->tag_len; + *dst_len = src_len + c->tag_len; return (srtp_err_status_ok); } @@ -363,8 +346,13 @@ static srtp_err_status_t srtp_aes_gcm_openssl_decrypt(void *cv, size_t *dst_len) { srtp_aes_gcm_ctx_t *c = (srtp_aes_gcm_ctx_t *)cv; + if (c->dir != srtp_direction_encrypt && c->dir != srtp_direction_decrypt) { - return (srtp_err_status_bad_param); + return srtp_err_status_bad_param; + } + + if (*dst_len < src_len - c->tag_len) { + return srtp_err_status_buffer_small; } /* @@ -375,7 +363,7 @@ static srtp_err_status_t srtp_aes_gcm_openssl_decrypt(void *cv, if (!EVP_CIPHER_CTX_ctrl( c->ctx, EVP_CTRL_GCM_SET_TAG, c->tag_len, (void *)(uintptr_t)(src + (src_len - c->tag_len)))) { - return (srtp_err_status_auth_fail); + return srtp_err_status_auth_fail; } EVP_Cipher(c->ctx, dst, src, src_len - c->tag_len); @@ -383,7 +371,7 @@ static srtp_err_status_t srtp_aes_gcm_openssl_decrypt(void *cv, * Check the tag */ if (EVP_Cipher(c->ctx, NULL, NULL, 0)) { - return (srtp_err_status_auth_fail); + return srtp_err_status_auth_fail; } /* @@ -406,6 +394,7 @@ static const char srtp_aes_gcm_256_openssl_description[] = /* * This is the vector function table for this crypto engine. */ +/* clang-format off */ const srtp_cipher_type_t srtp_aes_gcm_128 = { srtp_aes_gcm_openssl_alloc, srtp_aes_gcm_openssl_dealloc, @@ -414,15 +403,16 @@ const srtp_cipher_type_t srtp_aes_gcm_128 = { srtp_aes_gcm_openssl_encrypt, srtp_aes_gcm_openssl_decrypt, srtp_aes_gcm_openssl_set_iv, - srtp_aes_gcm_openssl_get_tag, srtp_aes_gcm_128_openssl_description, &srtp_aes_gcm_128_test_case_0, SRTP_AES_GCM_128 }; +/* clang-format on */ /* * This is the vector function table for this crypto engine. */ +/* clang-format off */ const srtp_cipher_type_t srtp_aes_gcm_256 = { srtp_aes_gcm_openssl_alloc, srtp_aes_gcm_openssl_dealloc, @@ -431,8 +421,8 @@ const srtp_cipher_type_t srtp_aes_gcm_256 = { srtp_aes_gcm_openssl_encrypt, srtp_aes_gcm_openssl_decrypt, srtp_aes_gcm_openssl_set_iv, - srtp_aes_gcm_openssl_get_tag, srtp_aes_gcm_256_openssl_description, &srtp_aes_gcm_256_test_case_0, SRTP_AES_GCM_256 }; +/* clang-format on */ diff --git a/crypto/cipher/aes_gcm_wssl.c b/crypto/cipher/aes_gcm_wssl.c index 85819791e..fec34d6c6 100644 --- a/crypto/cipher/aes_gcm_wssl.c +++ b/crypto/cipher/aes_gcm_wssl.c @@ -331,59 +331,40 @@ static srtp_err_status_t srtp_aes_gcm_wolfssl_encrypt(void *cv, int err; if (c->dir != srtp_direction_encrypt && c->dir != srtp_direction_decrypt) { - return (srtp_err_status_bad_param); + return srtp_err_status_bad_param; } -#ifndef WOLFSSL_AESGCM_STREAM - err = wc_AesGcmEncrypt(c->ctx, dst, src, src_len, c->iv, c->iv_len, c->tag, - c->tag_len, c->aad, c->aad_size); + if (*dst_len < src_len + c->tag_len) { + return srtp_err_status_buffer_small; + } +#ifndef WOLFSSL_AESGCM_STREAM + // tag must always be 16 bytes when passed to wc_AesGcmEncrypt, can truncate + // to c->tag_len after + uint8_t tag[GCM_AUTH_TAG_LEN]; + err = wc_AesGcmEncrypt(c->ctx, dst, src, src_len, c->iv, c->iv_len, tag, + sizeof(tag), c->aad, c->aad_size); c->aad_size = 0; + if (err == 0) { + memcpy(dst + src_len, tag, c->tag_len); + } #else err = wc_AesGcmEncryptUpdate(c->ctx, dst, src, src_len, NULL, 0); -#endif if (err < 0) { debug_print(srtp_mod_aes_gcm, "wolfSSL error code: %d", err); - return srtp_err_status_bad_param; + return srtp_err_status_algo_fail; } - *dst_len = src_len; - - return (srtp_err_status_ok); -} - -/* - * This function calculates and returns the GCM tag for a given context. - * This should be called after encrypting the data. The *len value - * is increased by the tag size. The caller must ensure that *buf has - * enough room to accept the appended tag. - * - * Parameters: - * c Crypto context - * buf data to encrypt - * len length of encrypt buffer - */ -static srtp_err_status_t srtp_aes_gcm_wolfssl_get_tag(void *cv, - uint8_t *buf, - size_t *len) -{ - FUNC_ENTRY(); - srtp_aes_gcm_ctx_t *c = (srtp_aes_gcm_ctx_t *)cv; -#ifdef WOLFSSL_AESGCM_STREAM - int err; + err = wc_AesGcmEncryptFinal(c->ctx, dst + src_len, c->tag_len); #endif - - debug_print(srtp_mod_aes_gcm, "appended tag size: %d", c->tag_len); - *len = c->tag_len; -#ifndef WOLFSSL_AESGCM_STREAM - memcpy(buf, c->tag, c->tag_len); -#else - err = wc_AesGcmEncryptFinal(c->ctx, buf, c->tag_len); if (err < 0) { debug_print(srtp_mod_aes_gcm, "wolfSSL error code: %d", err); + printf("wolfSSL error code: %d\n", err); return srtp_err_status_algo_fail; } -#endif - return (srtp_err_status_ok); + + *dst_len = src_len + c->tag_len; + + return srtp_err_status_ok; } /* @@ -405,7 +386,11 @@ static srtp_err_status_t srtp_aes_gcm_wolfssl_decrypt(void *cv, int err; if (c->dir != srtp_direction_encrypt && c->dir != srtp_direction_decrypt) { - return (srtp_err_status_bad_param); + return srtp_err_status_bad_param; + } + + if (*dst_len < src_len - c->tag_len) { + return srtp_err_status_buffer_small; } #ifndef WOLFSSL_AESGCM_STREAM @@ -421,14 +406,14 @@ static srtp_err_status_t srtp_aes_gcm_wolfssl_decrypt(void *cv, 0); if (err < 0) { debug_print(srtp_mod_aes_gcm, "wolfSSL error code: %d", err); - return (srtp_err_status_algo_fail); + return srtp_err_status_algo_fail; } err = wc_AesGcmDecryptFinal(c->ctx, src + (src_len - c->tag_len), c->tag_len); #endif if (err < 0) { debug_print(srtp_mod_aes_gcm, "wolfSSL error code: %d", err); - return (srtp_err_status_auth_fail); + return srtp_err_status_auth_fail; } /* @@ -437,7 +422,7 @@ static srtp_err_status_t srtp_aes_gcm_wolfssl_decrypt(void *cv, */ *dst_len = src_len -= c->tag_len; - return (srtp_err_status_ok); + return srtp_err_status_ok; } /* @@ -451,6 +436,7 @@ static const char srtp_aes_gcm_256_wolfssl_description[] = /* * This is the vector function table for this crypto engine. */ +/* clang-format off */ const srtp_cipher_type_t srtp_aes_gcm_128 = { srtp_aes_gcm_wolfssl_alloc, srtp_aes_gcm_wolfssl_dealloc, @@ -459,15 +445,16 @@ const srtp_cipher_type_t srtp_aes_gcm_128 = { srtp_aes_gcm_wolfssl_encrypt, srtp_aes_gcm_wolfssl_decrypt, srtp_aes_gcm_wolfssl_set_iv, - srtp_aes_gcm_wolfssl_get_tag, srtp_aes_gcm_128_wolfssl_description, &srtp_aes_gcm_128_test_case_0, SRTP_AES_GCM_128 }; +/* clang-format on */ /* * This is the vector function table for this crypto engine. */ +/* clang-format off */ const srtp_cipher_type_t srtp_aes_gcm_256 = { srtp_aes_gcm_wolfssl_alloc, srtp_aes_gcm_wolfssl_dealloc, @@ -476,8 +463,8 @@ const srtp_cipher_type_t srtp_aes_gcm_256 = { srtp_aes_gcm_wolfssl_encrypt, srtp_aes_gcm_wolfssl_decrypt, srtp_aes_gcm_wolfssl_set_iv, - srtp_aes_gcm_wolfssl_get_tag, srtp_aes_gcm_256_wolfssl_description, &srtp_aes_gcm_256_test_case_0, SRTP_AES_GCM_256 }; +/* clang-format on */ diff --git a/crypto/cipher/aes_icm.c b/crypto/cipher/aes_icm.c index fea79f019..f56fd76ac 100644 --- a/crypto/cipher/aes_icm.c +++ b/crypto/cipher/aes_icm.c @@ -430,7 +430,6 @@ const srtp_cipher_type_t srtp_aes_icm_128 = { srtp_aes_icm_encrypt, /* */ srtp_aes_icm_encrypt, /* */ srtp_aes_icm_set_iv, /* */ - 0, /* get_tag */ srtp_aes_icm_128_description, /* */ &srtp_aes_icm_128_test_case_0, /* */ SRTP_AES_ICM_128 /* */ @@ -444,7 +443,6 @@ const srtp_cipher_type_t srtp_aes_icm_256 = { srtp_aes_icm_encrypt, /* */ srtp_aes_icm_encrypt, /* */ srtp_aes_icm_set_iv, /* */ - 0, /* get_tag */ srtp_aes_icm_256_description, /* */ &srtp_aes_icm_256_test_case_0, /* */ SRTP_AES_ICM_256 /* */ diff --git a/crypto/cipher/aes_icm_mbedtls.c b/crypto/cipher/aes_icm_mbedtls.c index e2aa98968..83879bd0b 100644 --- a/crypto/cipher/aes_icm_mbedtls.c +++ b/crypto/cipher/aes_icm_mbedtls.c @@ -339,7 +339,6 @@ const srtp_cipher_type_t srtp_aes_icm_128 = { srtp_aes_icm_mbedtls_encrypt, /* */ srtp_aes_icm_mbedtls_encrypt, /* */ srtp_aes_icm_mbedtls_set_iv, /* */ - 0, /* get_tag */ srtp_aes_icm_128_mbedtls_description, /* */ &srtp_aes_icm_128_test_case_0, /* */ SRTP_AES_ICM_128 /* */ @@ -357,7 +356,6 @@ const srtp_cipher_type_t srtp_aes_icm_192 = { srtp_aes_icm_mbedtls_encrypt, /* */ srtp_aes_icm_mbedtls_encrypt, /* */ srtp_aes_icm_mbedtls_set_iv, /* */ - 0, /* get_tag */ srtp_aes_icm_192_mbedtls_description, /* */ &srtp_aes_icm_192_test_case_0, /* */ SRTP_AES_ICM_192 /* */ @@ -375,7 +373,6 @@ const srtp_cipher_type_t srtp_aes_icm_256 = { srtp_aes_icm_mbedtls_encrypt, /* */ srtp_aes_icm_mbedtls_encrypt, /* */ srtp_aes_icm_mbedtls_set_iv, /* */ - 0, /* get_tag */ srtp_aes_icm_256_mbedtls_description, /* */ &srtp_aes_icm_256_test_case_0, /* */ SRTP_AES_ICM_256 /* */ diff --git a/crypto/cipher/aes_icm_nss.c b/crypto/cipher/aes_icm_nss.c index 25611bea9..ef141755e 100644 --- a/crypto/cipher/aes_icm_nss.c +++ b/crypto/cipher/aes_icm_nss.c @@ -367,7 +367,6 @@ const srtp_cipher_type_t srtp_aes_icm_128 = { srtp_aes_icm_nss_encrypt, /* */ srtp_aes_icm_nss_encrypt, /* */ srtp_aes_icm_nss_set_iv, /* */ - 0, /* get_tag */ srtp_aes_icm_128_nss_description, /* */ &srtp_aes_icm_128_test_case_0, /* */ SRTP_AES_ICM_128 /* */ @@ -385,7 +384,6 @@ const srtp_cipher_type_t srtp_aes_icm_192 = { srtp_aes_icm_nss_encrypt, /* */ srtp_aes_icm_nss_encrypt, /* */ srtp_aes_icm_nss_set_iv, /* */ - 0, /* get_tag */ srtp_aes_icm_192_nss_description, /* */ &srtp_aes_icm_192_test_case_0, /* */ SRTP_AES_ICM_192 /* */ @@ -403,7 +401,6 @@ const srtp_cipher_type_t srtp_aes_icm_256 = { srtp_aes_icm_nss_encrypt, /* */ srtp_aes_icm_nss_encrypt, /* */ srtp_aes_icm_nss_set_iv, /* */ - 0, /* get_tag */ srtp_aes_icm_256_nss_description, /* */ &srtp_aes_icm_256_test_case_0, /* */ SRTP_AES_ICM_256 /* */ diff --git a/crypto/cipher/aes_icm_ossl.c b/crypto/cipher/aes_icm_ossl.c index 287642d7a..81a5de7e3 100644 --- a/crypto/cipher/aes_icm_ossl.c +++ b/crypto/cipher/aes_icm_ossl.c @@ -343,7 +343,6 @@ const srtp_cipher_type_t srtp_aes_icm_128 = { srtp_aes_icm_openssl_encrypt, /* */ srtp_aes_icm_openssl_encrypt, /* */ srtp_aes_icm_openssl_set_iv, /* */ - 0, /* get_tag */ srtp_aes_icm_128_openssl_description, /* */ &srtp_aes_icm_128_test_case_0, /* */ SRTP_AES_ICM_128 /* */ @@ -361,7 +360,6 @@ const srtp_cipher_type_t srtp_aes_icm_192 = { srtp_aes_icm_openssl_encrypt, /* */ srtp_aes_icm_openssl_encrypt, /* */ srtp_aes_icm_openssl_set_iv, /* */ - 0, /* get_tag */ srtp_aes_icm_192_openssl_description, /* */ &srtp_aes_icm_192_test_case_0, /* */ SRTP_AES_ICM_192 /* */ @@ -379,7 +377,6 @@ const srtp_cipher_type_t srtp_aes_icm_256 = { srtp_aes_icm_openssl_encrypt, /* */ srtp_aes_icm_openssl_encrypt, /* */ srtp_aes_icm_openssl_set_iv, /* */ - 0, /* get_tag */ srtp_aes_icm_256_openssl_description, /* */ &srtp_aes_icm_256_test_case_0, /* */ SRTP_AES_ICM_256 /* */ diff --git a/crypto/cipher/aes_icm_wssl.c b/crypto/cipher/aes_icm_wssl.c index 4ccd3512e..a8f640bdc 100644 --- a/crypto/cipher/aes_icm_wssl.c +++ b/crypto/cipher/aes_icm_wssl.c @@ -350,7 +350,6 @@ const srtp_cipher_type_t srtp_aes_icm_128 = { srtp_aes_icm_wolfssl_encrypt, /* */ srtp_aes_icm_wolfssl_encrypt, /* */ srtp_aes_icm_wolfssl_set_iv, /* */ - 0, /* get_tag */ srtp_aes_icm_128_wolfssl_description, /* */ &srtp_aes_icm_128_test_case_0, /* */ SRTP_AES_ICM_128 /* */ @@ -368,7 +367,6 @@ const srtp_cipher_type_t srtp_aes_icm_192 = { srtp_aes_icm_wolfssl_encrypt, /* */ srtp_aes_icm_wolfssl_encrypt, /* */ srtp_aes_icm_wolfssl_set_iv, /* */ - 0, /* get_tag */ srtp_aes_icm_192_wolfssl_description, /* */ &srtp_aes_icm_192_test_case_0, /* */ SRTP_AES_ICM_192 /* */ @@ -386,7 +384,6 @@ const srtp_cipher_type_t srtp_aes_icm_256 = { srtp_aes_icm_wolfssl_encrypt, /* */ srtp_aes_icm_wolfssl_encrypt, /* */ srtp_aes_icm_wolfssl_set_iv, /* */ - 0, /* get_tag */ srtp_aes_icm_256_wolfssl_description, /* */ &srtp_aes_icm_256_test_case_0, /* */ SRTP_AES_ICM_256 /* */ diff --git a/crypto/cipher/cipher.c b/crypto/cipher/cipher.c index 80776b14c..3cc596186 100644 --- a/crypto/cipher/cipher.c +++ b/crypto/cipher/cipher.c @@ -137,20 +137,6 @@ srtp_err_status_t srtp_cipher_decrypt(srtp_cipher_t *c, return (((c)->type)->decrypt(((c)->state), src, src_len, dst, dst_len)); } -srtp_err_status_t srtp_cipher_get_tag(srtp_cipher_t *c, - uint8_t *buffer, - size_t *tag_len) -{ - if (!c || !c->type || !c->state) { - return (srtp_err_status_bad_param); - } - if (!((c)->type)->get_tag) { - return (srtp_err_status_no_such_op); - } - - return (((c)->type)->get_tag(((c)->state), buffer, tag_len)); -} - srtp_err_status_t srtp_cipher_set_aad(srtp_cipher_t *c, const uint8_t *aad, size_t aad_len) @@ -218,7 +204,6 @@ srtp_err_status_t srtp_cipher_type_test( srtp_err_status_t status; uint8_t buffer[SELF_TEST_BUF_OCTETS]; uint8_t buffer2[SELF_TEST_BUF_OCTETS]; - size_t tag_len; size_t len; size_t case_num = 0; @@ -305,19 +290,6 @@ srtp_err_status_t srtp_cipher_type_test( return status; } - if (c->algorithm == SRTP_AES_GCM_128 || - c->algorithm == SRTP_AES_GCM_256) { - /* - * Get the GCM tag - */ - status = srtp_cipher_get_tag(c, buffer + len, &tag_len); - if (status) { - srtp_cipher_dealloc(c); - return status; - } - len += tag_len; - } - debug_print(srtp_mod_cipher, "ciphertext: %s", srtp_octet_string_hex_string( buffer, test_case->ciphertext_length_octets)); @@ -406,7 +378,7 @@ srtp_err_status_t srtp_cipher_type_test( return status; } - debug_print(srtp_mod_cipher, "plaintext: %s", + debug_print(srtp_mod_cipher, "plaintext: %s", srtp_octet_string_hex_string( buffer, test_case->plaintext_length_octets)); @@ -530,18 +502,7 @@ srtp_err_status_t srtp_cipher_type_test( srtp_cipher_dealloc(c); return status; } - if (c->algorithm == SRTP_AES_GCM_128 || - c->algorithm == SRTP_AES_GCM_256) { - /* - * Get the GCM tag - */ - status = srtp_cipher_get_tag(c, buffer + encrypted_len, &tag_len); - if (status) { - srtp_cipher_dealloc(c); - return status; - } - encrypted_len += tag_len; - } + debug_print(srtp_mod_cipher, "ciphertext: %s", srtp_octet_string_hex_string(buffer, encrypted_len)); @@ -641,6 +602,7 @@ uint64_t srtp_cipher_bits_per_second(srtp_cipher_t *c, clock_t timer; uint8_t *enc_buf; size_t len = octets_in_buffer; + size_t out_len; size_t tag_len = SRTP_MAX_TAG_LEN; uint8_t aad[4] = { 0, 0, 0, 0 }; size_t aad_len = 4; @@ -669,20 +631,12 @@ uint64_t srtp_cipher_bits_per_second(srtp_cipher_t *c, } // Encrypt the buffer - if (srtp_cipher_encrypt(c, enc_buf, len, enc_buf, &len) != + out_len = octets_in_buffer + tag_len; + if (srtp_cipher_encrypt(c, enc_buf, len, enc_buf, &out_len) != srtp_err_status_ok) { srtp_crypto_free(enc_buf); return 0; } - - // Get tag if supported by the cipher - if (c->type->get_tag) { - if (srtp_cipher_get_tag(c, enc_buf + len, &tag_len) != - srtp_err_status_ok) { - srtp_crypto_free(enc_buf); - return 0; - } - } } timer = clock() - timer; diff --git a/crypto/cipher/null_cipher.c b/crypto/cipher/null_cipher.c index 58870e8a4..6337c34eb 100644 --- a/crypto/cipher/null_cipher.c +++ b/crypto/cipher/null_cipher.c @@ -160,7 +160,6 @@ const srtp_cipher_type_t srtp_null_cipher = { srtp_null_cipher_encrypt, /* */ srtp_null_cipher_encrypt, /* */ srtp_null_cipher_set_iv, /* */ - 0, /* get_tag */ srtp_null_cipher_description, /* */ &srtp_null_cipher_test_0, /* */ SRTP_NULL_CIPHER /* */ diff --git a/crypto/include/aes_gcm.h b/crypto/include/aes_gcm.h index de5423a32..5117ad122 100644 --- a/crypto/include/aes_gcm.h +++ b/crypto/include/aes_gcm.h @@ -82,7 +82,6 @@ typedef struct { int aad_size; int iv_len; uint8_t iv[GCM_NONCE_MID_SZ]; - uint8_t tag[AES_BLOCK_SIZE]; uint8_t aad[MAX_AD_SIZE]; #endif Aes *ctx; @@ -102,7 +101,6 @@ typedef struct { size_t aad_size; size_t iv_len; uint8_t iv[12]; - uint8_t tag[16]; uint8_t aad[MAX_AD_SIZE]; mbedtls_gcm_context *ctx; srtp_cipher_direction_t dir; @@ -129,7 +127,6 @@ typedef struct { uint8_t aad[MAX_AD_SIZE]; size_t aad_size; CK_GCM_PARAMS params; - uint8_t tag[16]; } srtp_aes_gcm_ctx_t; #endif /* NSS */ diff --git a/crypto/include/cipher.h b/crypto/include/cipher.h index 995cfcb0a..86ce9d3fc 100644 --- a/crypto/include/cipher.h +++ b/crypto/include/cipher.h @@ -118,14 +118,6 @@ typedef srtp_err_status_t (*srtp_cipher_set_iv_func_t)( uint8_t *iv, srtp_cipher_direction_t direction); -/* - * a cipher_get_tag_func_t function is used to get the authentication - * tag that was calculated by an AEAD cipher. - */ -typedef srtp_err_status_t (*srtp_cipher_get_tag_func_t)(void *state, - uint8_t *tag, - size_t *len); - /* * srtp_cipher_test_case_t is a (list of) key, salt, plaintext, ciphertext, * and aad values that are known to be correct for a @@ -157,7 +149,6 @@ typedef struct srtp_cipher_type_t { srtp_cipher_encrypt_func_t encrypt; srtp_cipher_decrypt_func_t decrypt; srtp_cipher_set_iv_func_t set_iv; - srtp_cipher_get_tag_func_t get_tag; const char *description; const srtp_cipher_test_case_t *test_data; srtp_cipher_type_id_t id; @@ -230,9 +221,6 @@ srtp_err_status_t srtp_cipher_decrypt(srtp_cipher_t *c, size_t src_len, uint8_t *dst, size_t *dst_len); -srtp_err_status_t srtp_cipher_get_tag(srtp_cipher_t *c, - uint8_t *buffer, - size_t *tag_len); srtp_err_status_t srtp_cipher_set_aad(srtp_cipher_t *c, const uint8_t *aad, size_t aad_len); diff --git a/crypto/test/cipher_driver.c b/crypto/test/cipher_driver.c index de648c0a3..a17fc220f 100644 --- a/crypto/test/cipher_driver.c +++ b/crypto/test/cipher_driver.c @@ -337,8 +337,14 @@ void cipher_driver_test_throughput(srtp_cipher_t *c) c->key_len); fflush(stdout); for (size_t i = min_enc_len; i <= max_enc_len; i = i * 2) { + uint64_t bits_per_second = + srtp_cipher_bits_per_second(c, i, num_trials); + if (bits_per_second == 0) { + printf("error: throughput test failed\n"); + exit(1); + } printf("msg len: %zu\tgigabits per second: %f\n", i, - srtp_cipher_bits_per_second(c, i, num_trials) / 1e9); + bits_per_second / 1e9); } } @@ -409,7 +415,7 @@ srtp_err_status_t cipher_driver_test_api(srtp_cipher_type_t *ct, int key_len) /* * cipher_driver_test_buffering(ct) tests the cipher's output - * buffering for correctness by checking the consistency of succesive + * buffering for correctness by checking the consistency of successive * calls */ diff --git a/srtp.def b/srtp.def index 7c4d83812..bf3142775 100644 --- a/srtp.def +++ b/srtp.def @@ -56,7 +56,6 @@ srtp_cipher_set_iv srtp_cipher_output srtp_cipher_encrypt srtp_cipher_decrypt -srtp_cipher_get_tag srtp_cipher_set_aad srtp_replace_cipher_type srtp_auth_get_key_length diff --git a/srtp/srtp.c b/srtp/srtp.c index da5e6e7d2..eb101543a 100644 --- a/srtp/srtp.c +++ b/srtp/srtp.c @@ -1895,7 +1895,7 @@ static srtp_err_status_t srtp_protect_aead(srtp_ctx_t *ctx, enc_octet_len = rtp_len - enc_start; /* check output length */ - if (*srtp_len < rtp_len + stream->mki_size + tag_len) { + if (*srtp_len < rtp_len + tag_len + stream->mki_size) { return srtp_err_status_buffer_small; } @@ -1979,26 +1979,14 @@ static srtp_err_status_t srtp_protect_aead(srtp_ctx_t *ctx, if (status) { return srtp_err_status_cipher_fail; } - /* - * If we're doing GCM, we need to get the tag - * and append that to the output - */ - status = srtp_cipher_get_tag(session_keys->rtp_cipher, - srtp + enc_start + enc_octet_len, &tag_len); - if (status) { - return (srtp_err_status_cipher_fail); - } if (stream->use_mki) { - srtp_inject_mki(srtp + enc_start + enc_octet_len + tag_len, - session_keys, stream->mki_size); + srtp_inject_mki(srtp + enc_start + enc_octet_len, session_keys, + stream->mki_size); } *srtp_len = enc_start + enc_octet_len; - /* increase the packet length by the length of the auth tag */ - *srtp_len += tag_len; - /* increase the packet length by the length of the mki_size */ *srtp_len += stream->mki_size; @@ -3626,7 +3614,6 @@ static srtp_err_status_t srtp_protect_rtcp_aead( uint8_t *trailer_p; /* pointer to start of trailer */ uint32_t trailer; /* trailer value */ size_t enc_octet_len = 0; /* number of octets in encrypted portion */ - uint8_t *auth_tag = NULL; /* location of auth_tag within packet */ srtp_err_status_t status; size_t tag_len; uint32_t seq_num; @@ -3661,7 +3648,7 @@ static srtp_err_status_t srtp_protect_rtcp_aead( if (stream->rtcp_services & sec_serv_conf) { trailer = htonl(SRTCP_E_BIT); /* set encrypt bit */ } else { - /* 0 is network-order independant */ + /* 0 is network-order independent */ trailer = 0x00000000; /* set encrypt bit */ } @@ -3670,14 +3657,6 @@ static srtp_err_status_t srtp_protect_rtcp_aead( session_keys, stream->mki_size); } - /* - * set the auth_tag pointer to the proper location, which is after - * the payload, but before the trailer - * (note that srtcp *always* provides authentication, unlike srtp) - */ - /* Note: This would need to change for optional mikey data */ - auth_tag = srtcp + rtcp_len; - /* * check sequence number for overruns, and copy it into the packet * if its value isn't too big @@ -3740,22 +3719,14 @@ static srtp_err_status_t srtp_protect_rtcp_aead( /* if we're encrypting, exor keystream into the message */ if (stream->rtcp_services & sec_serv_conf) { - size_t outlen = *srtcp_len - enc_start; + size_t out_len = *srtcp_len - enc_start; status = srtp_cipher_encrypt(session_keys->rtcp_cipher, rtcp + enc_start, - enc_octet_len, srtcp + enc_start, &outlen); - enc_octet_len = outlen; + enc_octet_len, srtcp + enc_start, &out_len); + enc_octet_len = out_len; if (status) { return srtp_err_status_cipher_fail; } - /* - * Get the tag and append that to the output - */ - status = - srtp_cipher_get_tag(session_keys->rtcp_cipher, auth_tag, &tag_len); - if (status) { - return (srtp_err_status_cipher_fail); - } } else { /* if no encryption and not-inplace then need to copy rest of packet */ if (rtcp != srtcp) { @@ -3766,26 +3737,20 @@ static srtp_err_status_t srtp_protect_rtcp_aead( * Even though we're not encrypting the payload, we need * to run the cipher to get the auth tag. */ - size_t nolen = 0; - status = srtp_cipher_encrypt(session_keys->rtcp_cipher, NULL, 0, NULL, - &nolen); + uint8_t *auth_tag = srtcp + enc_start + enc_octet_len; + size_t out_len = *srtcp_len - enc_start - enc_octet_len; + status = srtp_cipher_encrypt(session_keys->rtcp_cipher, NULL, 0, + auth_tag, &out_len); if (status) { return srtp_err_status_cipher_fail; } - /* - * Get the tag and append that to the output - */ - status = - srtp_cipher_get_tag(session_keys->rtcp_cipher, auth_tag, &tag_len); - if (status) { - return (srtp_err_status_cipher_fail); - } + enc_octet_len += out_len; } *srtcp_len = octets_in_rtcp_header + enc_octet_len; - /* increase the packet length by the length of the auth tag and seq_num*/ - *srtcp_len += (tag_len + sizeof(srtcp_trailer_t)); + /* increase the packet length by the length of the seq_num*/ + *srtcp_len += sizeof(srtcp_trailer_t); /* increase the packet by the mki_size */ *srtcp_len += stream->mki_size;