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 1/2] 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; From 3515b821866efcbe14609b1f63732c3628b56be7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pascal=20B=C3=BChler?= Date: Sun, 7 Jul 2024 23:15:22 +0200 Subject: [PATCH 2/2] add cipher test for buffer lengths Use CHECK_XXX functions in cipher_drive, extend CHECK_XXX to improve testing. --- CMakeLists.txt | 2 +- Makefile.in | 2 +- crypto/cipher/aes_gcm_mbedtls.c | 14 ++- crypto/cipher/aes_gcm_nss.c | 24 +++- crypto/cipher/aes_gcm_ossl.c | 8 +- crypto/cipher/aes_gcm_wssl.c | 8 +- crypto/cipher/aes_icm_nss.c | 12 +- crypto/cipher/aes_icm_ossl.c | 8 ++ crypto/cipher/aes_icm_wssl.c | 8 ++ crypto/include/aes_gcm.h | 8 +- crypto/test/cipher_driver.c | 210 +++++++++++++++++++++++--------- test/util.c | 76 +++++++++++- test/util.h | 7 ++ 13 files changed, 305 insertions(+), 82 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index aa896e33a..7f7fa027b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -386,7 +386,7 @@ if(LIBSRTP_TEST_APPS) target_link_libraries(datatypes_driver srtp3) add_test(datatypes_driver datatypes_driver -v) - add_executable(cipher_driver crypto/test/cipher_driver.c test/getopt_s.c) + add_executable(cipher_driver crypto/test/cipher_driver.c test/getopt_s.c test/util.c) target_set_warnings( TARGET cipher_driver diff --git a/Makefile.in b/Makefile.in index 33bc66b03..2eae17e60 100644 --- a/Makefile.in +++ b/Makefile.in @@ -229,7 +229,7 @@ test/roc_driver$(EXE): test/roc_driver.c test/ut_sim.c test/replay_driver$(EXE): test/replay_driver.c test/ut_sim.c $(COMPILE) -I$(srcdir)/test $(LDFLAGS) -o $@ $^ $(LIBS) $(SRTPLIB) -crypto/test/cipher_driver$(EXE): crypto/test/cipher_driver.c test/getopt_s.c +crypto/test/cipher_driver$(EXE): crypto/test/cipher_driver.c test/getopt_s.c test/util.c $(COMPILE) -I$(srcdir)/test $(LDFLAGS) -o $@ $^ $(LIBS) $(SRTPLIB) crypto/test/kernel_driver$(EXE): crypto/test/kernel_driver.c test/getopt_s.c diff --git a/crypto/cipher/aes_gcm_mbedtls.c b/crypto/cipher/aes_gcm_mbedtls.c index 4a6da1cfb..854d72b95 100644 --- a/crypto/cipher/aes_gcm_mbedtls.c +++ b/crypto/cipher/aes_gcm_mbedtls.c @@ -290,7 +290,7 @@ static srtp_err_status_t srtp_aes_gcm_mbedtls_encrypt(void *cv, srtp_aes_gcm_ctx_t *c = (srtp_aes_gcm_ctx_t *)cv; int errCode = 0; - if (c->dir != srtp_direction_encrypt && c->dir != srtp_direction_decrypt) { + if (c->dir != srtp_direction_encrypt) { return srtp_err_status_bad_param; } @@ -331,8 +331,12 @@ static srtp_err_status_t srtp_aes_gcm_mbedtls_decrypt(void *cv, srtp_aes_gcm_ctx_t *c = (srtp_aes_gcm_ctx_t *)cv; int errCode = 0; - if (c->dir != srtp_direction_encrypt && c->dir != srtp_direction_decrypt) { - return (srtp_err_status_bad_param); + if (c->dir != srtp_direction_decrypt) { + return srtp_err_status_bad_param; + } + + if (src_len < c->tag_len) { + return srtp_err_status_bad_param; } if (*dst_len < (src_len - c->tag_len)) { @@ -347,7 +351,7 @@ static srtp_err_status_t srtp_aes_gcm_mbedtls_decrypt(void *cv, src + (src_len - c->tag_len), c->tag_len, src, dst); c->aad_size = 0; if (errCode != 0) { - return (srtp_err_status_auth_fail); + return srtp_err_status_auth_fail; } /* @@ -356,7 +360,7 @@ static srtp_err_status_t srtp_aes_gcm_mbedtls_decrypt(void *cv, */ *dst_len = (src_len - c->tag_len); - return (srtp_err_status_ok); + return srtp_err_status_ok; } /* diff --git a/crypto/cipher/aes_gcm_nss.c b/crypto/cipher/aes_gcm_nss.c index 6ecabf069..d70386d0f 100644 --- a/crypto/cipher/aes_gcm_nss.c +++ b/crypto/cipher/aes_gcm_nss.c @@ -301,16 +301,36 @@ static srtp_err_status_t srtp_aes_gcm_nss_do_crypto(void *cv, SECItem param = { siBuffer, (unsigned char *)&c->params, sizeof(CK_GCM_PARAMS) }; if (encrypt) { + if (c->dir != srtp_direction_encrypt) { + return srtp_err_status_bad_param; + } + + if (*dst_len < src_len + c->tag_size) { + return srtp_err_status_buffer_small; + } + rv = PK11_Encrypt(c->key, CKM_AES_GCM, ¶m, dst, &out_len, *dst_len, src, src_len); } else { + if (c->dir != srtp_direction_decrypt) { + return srtp_err_status_bad_param; + } + + if (src_len < c->tag_size) { + return srtp_err_status_bad_param; + } + + if (*dst_len < src_len - c->tag_size) { + return srtp_err_status_buffer_small; + } + rv = PK11_Decrypt(c->key, CKM_AES_GCM, ¶m, dst, &out_len, *dst_len, src, src_len); } *dst_len = out_len; - srtp_err_status_t status = (srtp_err_status_ok); + srtp_err_status_t status = srtp_err_status_ok; if (rv != SECSuccess) { - status = (srtp_err_status_cipher_fail); + status = srtp_err_status_cipher_fail; } return status; diff --git a/crypto/cipher/aes_gcm_ossl.c b/crypto/cipher/aes_gcm_ossl.c index 3704eb848..6432abf44 100644 --- a/crypto/cipher/aes_gcm_ossl.c +++ b/crypto/cipher/aes_gcm_ossl.c @@ -300,7 +300,7 @@ static srtp_err_status_t srtp_aes_gcm_openssl_encrypt(void *cv, { srtp_aes_gcm_ctx_t *c = (srtp_aes_gcm_ctx_t *)cv; - if (c->dir != srtp_direction_encrypt && c->dir != srtp_direction_decrypt) { + if (c->dir != srtp_direction_encrypt) { return srtp_err_status_bad_param; } @@ -347,7 +347,11 @@ static srtp_err_status_t srtp_aes_gcm_openssl_decrypt(void *cv, { srtp_aes_gcm_ctx_t *c = (srtp_aes_gcm_ctx_t *)cv; - if (c->dir != srtp_direction_encrypt && c->dir != srtp_direction_decrypt) { + if (c->dir != srtp_direction_decrypt) { + return srtp_err_status_bad_param; + } + + if (src_len < c->tag_len) { return srtp_err_status_bad_param; } diff --git a/crypto/cipher/aes_gcm_wssl.c b/crypto/cipher/aes_gcm_wssl.c index fec34d6c6..8f4699e20 100644 --- a/crypto/cipher/aes_gcm_wssl.c +++ b/crypto/cipher/aes_gcm_wssl.c @@ -330,7 +330,7 @@ static srtp_err_status_t srtp_aes_gcm_wolfssl_encrypt(void *cv, srtp_aes_gcm_ctx_t *c = (srtp_aes_gcm_ctx_t *)cv; int err; - if (c->dir != srtp_direction_encrypt && c->dir != srtp_direction_decrypt) { + if (c->dir != srtp_direction_encrypt) { return srtp_err_status_bad_param; } @@ -385,7 +385,11 @@ static srtp_err_status_t srtp_aes_gcm_wolfssl_decrypt(void *cv, srtp_aes_gcm_ctx_t *c = (srtp_aes_gcm_ctx_t *)cv; int err; - if (c->dir != srtp_direction_encrypt && c->dir != srtp_direction_decrypt) { + if (c->dir != srtp_direction_decrypt) { + return srtp_err_status_bad_param; + } + + if (src_len < c->tag_len) { return srtp_err_status_bad_param; } diff --git a/crypto/cipher/aes_icm_nss.c b/crypto/cipher/aes_icm_nss.c index ef141755e..5d6dc07b7 100644 --- a/crypto/cipher/aes_icm_nss.c +++ b/crypto/cipher/aes_icm_nss.c @@ -334,12 +334,20 @@ static srtp_err_status_t srtp_aes_icm_nss_encrypt(void *cv, return srtp_err_status_bad_param; } + if (dst_len == NULL) { + return srtp_err_status_bad_param; + } + + if (*dst_len < src_len) { + return srtp_err_status_buffer_small; + } + int out_len = 0; int rv = PK11_CipherOp(c->ctx, dst, &out_len, *dst_len, src, src_len); *dst_len = out_len; - srtp_err_status_t status = (srtp_err_status_ok); + srtp_err_status_t status = srtp_err_status_ok; if (rv != SECSuccess) { - status = (srtp_err_status_cipher_fail); + status = srtp_err_status_cipher_fail; } return status; diff --git a/crypto/cipher/aes_icm_ossl.c b/crypto/cipher/aes_icm_ossl.c index 81a5de7e3..82e7d9632 100644 --- a/crypto/cipher/aes_icm_ossl.c +++ b/crypto/cipher/aes_icm_ossl.c @@ -308,6 +308,14 @@ static srtp_err_status_t srtp_aes_icm_openssl_encrypt(void *cv, debug_print(srtp_mod_aes_icm, "rs0: %s", v128_hex_string(&c->counter)); + if (dst_len == NULL) { + return srtp_err_status_bad_param; + } + + if (*dst_len < src_len) { + return srtp_err_status_buffer_small; + } + if (!EVP_EncryptUpdate(c->ctx, dst, &len, src, src_len)) { return srtp_err_status_cipher_fail; } diff --git a/crypto/cipher/aes_icm_wssl.c b/crypto/cipher/aes_icm_wssl.c index a8f640bdc..41cb38eef 100644 --- a/crypto/cipher/aes_icm_wssl.c +++ b/crypto/cipher/aes_icm_wssl.c @@ -318,6 +318,14 @@ static srtp_err_status_t srtp_aes_icm_wolfssl_encrypt(void *cv, int err; debug_print(srtp_mod_aes_icm, "rs0: %s", v128_hex_string(&c->counter)); + if (dst_len == NULL) { + return srtp_err_status_bad_param; + } + + if (*dst_len < src_len) { + return srtp_err_status_buffer_small; + } + err = wc_AesCtrEncrypt(c->ctx, dst, src, src_len); if (err < 0) { debug_print(srtp_mod_aes_icm, "wolfSSL encrypt error: %d", err); diff --git a/crypto/include/aes_gcm.h b/crypto/include/aes_gcm.h index 5117ad122..d6705a460 100644 --- a/crypto/include/aes_gcm.h +++ b/crypto/include/aes_gcm.h @@ -76,11 +76,11 @@ typedef struct { #include typedef struct { - int key_size; - int tag_len; + size_t key_size; + size_t tag_len; #ifndef WOLFSSL_AESGCM_STREAM - int aad_size; - int iv_len; + size_t aad_size; + size_t iv_len; uint8_t iv[GCM_NONCE_MID_SZ]; uint8_t aad[MAX_AD_SIZE]; #endif diff --git a/crypto/test/cipher_driver.c b/crypto/test/cipher_driver.c index a17fc220f..a61ff19b1 100644 --- a/crypto/test/cipher_driver.c +++ b/crypto/test/cipher_driver.c @@ -52,8 +52,9 @@ #include "cipher_priv.h" #include "datatypes.h" #include "alloc.h" +#include "util.h" -#include /* for printf() */ +#include #include #define PRINT_DEBUG 0 @@ -62,7 +63,9 @@ void cipher_driver_test_throughput(srtp_cipher_t *c); srtp_err_status_t cipher_driver_self_test(srtp_cipher_type_t *ct); -srtp_err_status_t cipher_driver_test_api(srtp_cipher_type_t *ct, int key_len); +srtp_err_status_t cipher_driver_test_api(srtp_cipher_type_t *ct, + size_t key_len, + size_t tag_len); /* * cipher_driver_test_buffering(ct) tests the cipher's output @@ -100,15 +103,6 @@ void usage(char *prog_name) exit(255); } -void check_status(srtp_err_status_t s) -{ - if (s) { - printf("error (code %d)\n", s); - exit(s); - } - return; -} - /* * null_cipher and srtp_aes_icm are the cipher meta-objects * defined in the files in crypto/cipher subdirectory. these are @@ -172,7 +166,7 @@ int main(int argc, char *argv[]) usage(argv[0]); } - /* arry timing (cache thrash) test */ + /* array timing (cache thrash) test */ if (do_array_timing_test) { size_t max_num_cipher = 1 << 16; /* number of ciphers in cipher_array */ size_t num_cipher; @@ -220,25 +214,29 @@ int main(int argc, char *argv[]) cipher_driver_self_test(&srtp_aes_gcm_256); #endif cipher_driver_test_api(&srtp_aes_icm_128, - SRTP_AES_ICM_128_KEY_LEN_WSALT); + SRTP_AES_ICM_128_KEY_LEN_WSALT, 0); +#ifdef GCM + cipher_driver_test_api(&srtp_aes_gcm_128, + SRTP_AES_GCM_128_KEY_LEN_WSALT, 16); +#endif } /* do timing and/or buffer_test on srtp_null_cipher */ status = srtp_cipher_type_alloc(&srtp_null_cipher, &c, 0, 0); - check_status(status); + CHECK_OK(status); status = srtp_cipher_init(c, NULL); - check_status(status); + CHECK_OK(status); if (do_timing_test) { cipher_driver_test_throughput(c); } if (do_validation) { status = cipher_driver_test_buffering(c); - check_status(status); + CHECK_OK(status); } status = srtp_cipher_dealloc(c); - check_status(status); + CHECK_OK(status); /* run the throughput test on the aes_icm cipher (128-bit key) */ status = srtp_cipher_type_alloc(&srtp_aes_icm_128, &c, @@ -249,7 +247,7 @@ int main(int argc, char *argv[]) } status = srtp_cipher_init(c, test_key); - check_status(status); + CHECK_OK(status); if (do_timing_test) { cipher_driver_test_throughput(c); @@ -257,11 +255,11 @@ int main(int argc, char *argv[]) if (do_validation) { status = cipher_driver_test_buffering(c); - check_status(status); + CHECK_OK(status); } status = srtp_cipher_dealloc(c); - check_status(status); + CHECK_OK(status); /* repeat the tests with 256-bit keys */ status = srtp_cipher_type_alloc(&srtp_aes_icm_256, &c, @@ -272,7 +270,7 @@ int main(int argc, char *argv[]) } status = srtp_cipher_init(c, test_key); - check_status(status); + CHECK_OK(status); if (do_timing_test) { cipher_driver_test_throughput(c); @@ -280,11 +278,11 @@ int main(int argc, char *argv[]) if (do_validation) { status = cipher_driver_test_buffering(c); - check_status(status); + CHECK_OK(status); } status = srtp_cipher_dealloc(c); - check_status(status); + CHECK_OK(status); #ifdef GCM /* run the throughput test on the aes_gcm_128 cipher */ @@ -295,7 +293,7 @@ int main(int argc, char *argv[]) exit(status); } status = srtp_cipher_init(c, test_key); - check_status(status); + CHECK_OK(status); if (do_timing_test) { cipher_driver_test_throughput(c); } @@ -303,7 +301,7 @@ int main(int argc, char *argv[]) // GCM ciphers don't do buffering; they're "one shot" status = srtp_cipher_dealloc(c); - check_status(status); + CHECK_OK(status); /* run the throughput test on the aes_gcm_256 cipher */ status = srtp_cipher_type_alloc(&srtp_aes_gcm_256, &c, @@ -313,7 +311,7 @@ int main(int argc, char *argv[]) exit(status); } status = srtp_cipher_init(c, test_key); - check_status(status); + CHECK_OK(status); if (do_timing_test) { cipher_driver_test_throughput(c); } @@ -321,7 +319,7 @@ int main(int argc, char *argv[]) // GCM ciphers don't do buffering; they're "one shot" status = srtp_cipher_dealloc(c); - check_status(status); + CHECK_OK(status); #endif return 0; @@ -354,16 +352,27 @@ srtp_err_status_t cipher_driver_self_test(srtp_cipher_type_t *ct) printf("running cipher self-test for %s...", ct->description); status = srtp_cipher_type_self_test(ct); - if (status) { - printf("failed with error code %d\n", status); - exit(status); - } + CHECK_OK(status); printf("passed\n"); return srtp_err_status_ok; } -srtp_err_status_t cipher_driver_test_api(srtp_cipher_type_t *ct, int key_len) +void reint_cipher(srtp_cipher_t *c, + uint8_t *test_key, + uint8_t *iv, + srtp_cipher_direction_t direction) +{ + srtp_err_status_t status = srtp_cipher_init(c, test_key); + CHECK_OK(status); + + status = srtp_cipher_set_iv(c, iv, direction); + CHECK_OK(status); +} + +srtp_err_status_t cipher_driver_test_api(srtp_cipher_type_t *ct, + size_t key_len, + size_t tag_len) { srtp_err_status_t status; srtp_cipher_t *c = NULL; @@ -380,33 +389,131 @@ srtp_err_status_t cipher_driver_test_api(srtp_cipher_type_t *ct, int key_len) uint8_t iv[16] = { 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f}; + uint8_t plaintext[64] = { + 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, + 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, + 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, + 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, + 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, + 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, + 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, + 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, + }; + uint8_t encrypted[96] = {0}; + uint8_t decrypted[96] = {0}; /* clang-format on */ printf("testing cipher api for %s...", ct->description); + fflush(stdout); - if (key_len > (int)sizeof(test_key)) { + if (key_len > sizeof(test_key)) { return srtp_err_status_bad_param; } - status = srtp_cipher_type_alloc(&srtp_aes_icm_256, &c, key_len, 0); - if (status) { - return status; - } + status = srtp_cipher_type_alloc(ct, &c, key_len, tag_len); + CHECK_OK(status); status = srtp_cipher_init(c, test_key); - if (status) { - return status; - } + CHECK_OK(status); status = srtp_cipher_set_iv(c, iv, srtp_direction_encrypt); - if (status) { - return status; + CHECK_OK(status); + + size_t src_len; + size_t dst_len; + + // test dst len zero + src_len = sizeof(plaintext); + dst_len = 0; + status = srtp_cipher_encrypt(c, plaintext, src_len, encrypted, &dst_len); + CHECK_RETURN(status, srtp_err_status_buffer_small); + + reint_cipher(c, test_key, iv, srtp_direction_encrypt); + + // test dst len smaller than expected + src_len = sizeof(plaintext); + dst_len = src_len + tag_len - 1; + status = srtp_cipher_encrypt(c, plaintext, src_len, encrypted, &dst_len); + CHECK_RETURN(status, srtp_err_status_buffer_small); + + reint_cipher(c, test_key, iv, srtp_direction_encrypt); + + // test dst len exact size + src_len = sizeof(plaintext); + dst_len = src_len + tag_len; + status = srtp_cipher_encrypt(c, plaintext, src_len, encrypted, &dst_len); + CHECK_OK(status); + CHECK(dst_len == src_len + tag_len); + + reint_cipher(c, test_key, iv, srtp_direction_encrypt); + + // dst len larger than src len + src_len = sizeof(plaintext); + dst_len = sizeof(encrypted); + status = srtp_cipher_encrypt(c, plaintext, src_len, encrypted, &dst_len); + CHECK_OK(status); + CHECK(dst_len == src_len + tag_len); + + reint_cipher(c, test_key, iv, srtp_direction_encrypt); + + size_t encrypted_len = dst_len; + + // init for decrypt + status = srtp_cipher_init(c, test_key); + CHECK_OK(status); + + status = srtp_cipher_set_iv(c, iv, srtp_direction_decrypt); + CHECK_OK(status); + + if (tag_len != 0) { + // test src less than tag len + src_len = tag_len - 1; + dst_len = sizeof(decrypted); + status = + srtp_cipher_decrypt(c, encrypted, src_len, decrypted, &dst_len); + CHECK_RETURN(status, srtp_err_status_bad_param); + + reint_cipher(c, test_key, iv, srtp_direction_decrypt); } + // test dst len zero + src_len = encrypted_len; + dst_len = 0; + status = srtp_cipher_decrypt(c, encrypted, src_len, decrypted, &dst_len); + CHECK_RETURN(status, srtp_err_status_buffer_small); + + reint_cipher(c, test_key, iv, srtp_direction_decrypt); + + // test dst len smaller than expected + src_len = encrypted_len; + dst_len = src_len - tag_len - 1; + status = srtp_cipher_decrypt(c, encrypted, src_len, decrypted, &dst_len); + CHECK_RETURN(status, srtp_err_status_buffer_small); + + reint_cipher(c, test_key, iv, srtp_direction_decrypt); + + // test dst len exact + src_len = encrypted_len; + dst_len = src_len - tag_len; + status = srtp_cipher_decrypt(c, encrypted, src_len, decrypted, &dst_len); + CHECK_OK(status); + CHECK(dst_len == sizeof(plaintext)); + CHECK_BUFFER_EQUAL(plaintext, decrypted, sizeof(plaintext)); + + reint_cipher(c, test_key, iv, srtp_direction_decrypt); + + // dst len larger than src len + src_len = encrypted_len; + dst_len = sizeof(decrypted); + status = srtp_cipher_decrypt(c, encrypted, src_len, decrypted, &dst_len); + CHECK_OK(status); + CHECK(dst_len == sizeof(plaintext)); + CHECK_BUFFER_EQUAL(plaintext, decrypted, sizeof(plaintext)); + + reint_cipher(c, test_key, iv, srtp_direction_decrypt); + status = srtp_cipher_dealloc(c); - if (status) { - return status; - } + CHECK_OK(status); printf("passed\n"); @@ -482,18 +589,7 @@ srtp_err_status_t cipher_driver_test_buffering(srtp_cipher_t *c) } /* compare buffers */ - for (size_t j = 0; j < buflen; j++) { - if (buffer0[j] != buffer1[j]) { -#if PRINT_DEBUG - printf("test case %zd failed at byte %zu\n", i, j); - printf("computed: %s\n", - octet_string_hex_string(buffer1, buflen)); - printf("expected: %s\n", - octet_string_hex_string(buffer0, buflen)); -#endif - return srtp_err_status_algo_fail; - } - } + CHECK_BUFFER_EQUAL(buffer0, buffer1, buflen); } printf("passed\n"); diff --git a/test/util.c b/test/util.c index 4385b507e..16b678c7d 100644 --- a/test/util.c +++ b/test/util.c @@ -52,11 +52,52 @@ /* include space for null terminator */ static char bit_string[MAX_PRINT_STRING_LEN + 1]; +#define ERR_STATUS_STRING(STATUS) \ + case srtp_err_status_##STATUS: \ + return #STATUS + +const char *err_status_string(srtp_err_status_t status) +{ + switch (status) { + ERR_STATUS_STRING(ok); + ERR_STATUS_STRING(fail); + ERR_STATUS_STRING(bad_param); + ERR_STATUS_STRING(alloc_fail); + ERR_STATUS_STRING(dealloc_fail); + ERR_STATUS_STRING(init_fail); + ERR_STATUS_STRING(terminus); + ERR_STATUS_STRING(auth_fail); + ERR_STATUS_STRING(cipher_fail); + ERR_STATUS_STRING(replay_fail); + ERR_STATUS_STRING(replay_old); + ERR_STATUS_STRING(algo_fail); + ERR_STATUS_STRING(no_such_op); + ERR_STATUS_STRING(no_ctx); + ERR_STATUS_STRING(cant_check); + ERR_STATUS_STRING(key_expired); + ERR_STATUS_STRING(socket_err); + ERR_STATUS_STRING(signal_err); + ERR_STATUS_STRING(nonce_bad); + ERR_STATUS_STRING(read_fail); + ERR_STATUS_STRING(write_fail); + ERR_STATUS_STRING(parse_err); + ERR_STATUS_STRING(encode_err); + ERR_STATUS_STRING(semaphore_err); + ERR_STATUS_STRING(pfkey_err); + ERR_STATUS_STRING(bad_mki); + ERR_STATUS_STRING(pkt_idx_old); + ERR_STATUS_STRING(pkt_idx_adv); + ERR_STATUS_STRING(buffer_small); + } + return "unkown srtp_err_status"; +} + void check_ok_impl(srtp_err_status_t status, const char *file, int line) { if (status != srtp_err_status_ok) { - fprintf(stderr, "error at %s:%d, unexpected srtp failure (code %d)\n", - file, line, status); + fprintf(stderr, + "\nerror at %s:%d, unexpected srtp failure: %d (\"%s\")\n", + file, line, status, err_status_string(status)); exit(1); } } @@ -68,8 +109,10 @@ void check_return_impl(srtp_err_status_t status, { if (status != expected) { fprintf(stderr, - "error at %s:%d, unexpected srtp status (code %d != %d)\n", - file, line, status, expected); + "\nerror at %s:%d, unexpected srtp status: %d != %d (\"%s\" != " + "\"%s\")\n", + file, line, status, expected, err_status_string(status), + err_status_string(expected)); exit(1); } } @@ -80,7 +123,7 @@ void check_impl(bool condition, const char *condition_str) { if (!condition) { - fprintf(stderr, "error at %s:%d, %s)\n", file, line, condition_str); + fprintf(stderr, "\nerror at %s:%d, %s)\n", file, line, condition_str); exit(1); } } @@ -92,6 +135,27 @@ void overrun_check_prepare(uint8_t *buffer, size_t offset, size_t buffer_len) memset(buffer + offset, OVERRUN_CHECK_BYTE, buffer_len - offset); } +void check_buffer_equal_impl(const uint8_t *buffer1, + const uint8_t *buffer2, + size_t buffer_length, + const char *file, + int line) +{ + for (size_t i = 0; i < buffer_length; i++) { + if (buffer1[i] != buffer2[i]) { + fprintf(stderr, + "\nerror at %s:%d, buffer1 != buffer2 at index: %zu (%x != " + "%x)\n", + file, line, i, buffer1[i], buffer2[i]); + fprintf(stderr, "buffer1 = %s\n", + octet_string_hex_string(buffer1, buffer_length)); + fprintf(stderr, "buffer2 = %s\n", + octet_string_hex_string(buffer2, buffer_length)); + exit(1); + } + } +} + void check_overrun_impl(const uint8_t *buffer, size_t offset, size_t buffer_length, @@ -100,7 +164,7 @@ void check_overrun_impl(const uint8_t *buffer, { for (size_t i = offset; i < buffer_length; i++) { if (buffer[i] != OVERRUN_CHECK_BYTE) { - printf("error at %s:%d, overrun detected in buffer at index %zu " + printf("\nerror at %s:%d, overrun detected in buffer at index %zu " "(expected %x, found %x)\n", file, line, i, OVERRUN_CHECK_BYTE, buffer[i]); exit(1); diff --git a/test/util.h b/test/util.h index 43926bcff..4f86552b9 100644 --- a/test/util.h +++ b/test/util.h @@ -56,6 +56,11 @@ void check_impl(bool condition, const char *file, int line, const char *condition_str); +void check_buffer_equal_impl(const uint8_t *buffer1, + const uint8_t *buffer2, + size_t buffer_length, + const char *file, + int line); void check_overrun_impl(const uint8_t *buffer, size_t offset, size_t buffer_length, @@ -67,6 +72,8 @@ void overrun_check_prepare(uint8_t *buffer, size_t offset, size_t buffer_len); #define CHECK_RETURN(status, expected) \ check_return_impl((status), (expected), __FILE__, __LINE__) #define CHECK(condition) check_impl((condition), __FILE__, __LINE__, #condition) +#define CHECK_BUFFER_EQUAL(buffer1, buffer2, length) \ + check_buffer_equal_impl((buffer1), (buffer2), (length), __FILE__, __LINE__) #define CHECK_OVERRUN(buffer, offset, length) \ check_overrun_impl((buffer), (offset), (length), __FILE__, __LINE__)