Skip to content

Commit

Permalink
Merge pull request #731 from pabuhler/update-openssl-gcm
Browse files Browse the repository at this point in the history
update openssl gcm code
  • Loading branch information
pabuhler authored Dec 5, 2024
2 parents 28d1735 + bd6f343 commit bf992dc
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 54 deletions.
4 changes: 2 additions & 2 deletions Makefile.in
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ endif

runtest-valgrind: test
@echo "running libsrtp3 test applications... (valgrind)"
valgrind --error-exitcode=1 --leak-check=full test/test_srtp$(EXE) -v >/dev/null
valgrind --error-exitcode=1 --leak-check=full test/srtp_driver$(EXE) -v >/dev/null
valgrind --error-exitcode=1 --leak-check=full --suppressions=./valgrind.supp test/test_srtp$(EXE) -v >/dev/null
valgrind --error-exitcode=1 --leak-check=full --suppressions=./valgrind.supp test/srtp_driver$(EXE) -v >/dev/null
@echo "libsrtp3 test applications passed. (valgrind)"

# makefile variables
Expand Down
104 changes: 52 additions & 52 deletions crypto/cipher/aes_gcm_ossl.c
Original file line number Diff line number Diff line change
Expand Up @@ -197,14 +197,14 @@ static srtp_err_status_t srtp_aes_gcm_openssl_context_init(void *cv,
EVP_CIPHER_CTX_reset(c->ctx);

if (!EVP_CipherInit_ex(c->ctx, evp, NULL, key, NULL, 0)) {
return (srtp_err_status_init_fail);
return srtp_err_status_init_fail;
}

if (!EVP_CIPHER_CTX_ctrl(c->ctx, EVP_CTRL_GCM_SET_IVLEN, 12, 0)) {
return (srtp_err_status_init_fail);
return srtp_err_status_init_fail;
}

return (srtp_err_status_ok);
return srtp_err_status_ok;
}

/*
Expand All @@ -227,12 +227,17 @@ static srtp_err_status_t srtp_aes_gcm_openssl_set_iv(
debug_print(srtp_mod_aes_gcm, "setting iv: %s",
srtp_octet_string_hex_string(iv, 12));

if (!EVP_CipherInit_ex(c->ctx, NULL, NULL, NULL, iv,
(c->dir == srtp_direction_encrypt ? 1 : 0))) {
return (srtp_err_status_init_fail);
if (c->dir == srtp_direction_encrypt) {
if (EVP_EncryptInit_ex(c->ctx, NULL, NULL, NULL, iv) != 1) {
return srtp_err_status_init_fail;
}
} else {
if (EVP_DecryptInit_ex(c->ctx, NULL, NULL, NULL, iv) != 1) {
return srtp_err_status_init_fail;
}
}

return (srtp_err_status_ok);
return srtp_err_status_ok;
}

/*
Expand All @@ -248,40 +253,26 @@ static srtp_err_status_t srtp_aes_gcm_openssl_set_aad(void *cv,
size_t aad_len)
{
srtp_aes_gcm_ctx_t *c = (srtp_aes_gcm_ctx_t *)cv;
int rv;
int len = 0;

debug_print(srtp_mod_aes_gcm, "setting AAD: %s",
srtp_octet_string_hex_string(aad, aad_len));

/*
* EVP_CTRL_GCM_SET_TAG can only be used when decrypting
*/
if (c->dir == srtp_direction_decrypt) {
/*
* Set dummy tag, OpenSSL requires the Tag to be set before
* processing AAD
*/

/*
* OpenSSL never write to address pointed by the last parameter of
* EVP_CIPHER_CTX_ctrl while EVP_CTRL_GCM_SET_TAG (in reality,
* OpenSSL copy its content to the context), so we can make
* aad read-only in this function and all its wrappers.
*/
uint8_t dummy_tag[GCM_AUTH_TAG_LEN];
memset(dummy_tag, 0x0, GCM_AUTH_TAG_LEN);
if (!EVP_CIPHER_CTX_ctrl(c->ctx, EVP_CTRL_GCM_SET_TAG, c->tag_len,
&dummy_tag)) {
return (srtp_err_status_algo_fail);
if (c->dir == srtp_direction_encrypt) {
if (EVP_EncryptUpdate(c->ctx, NULL, &len, aad, aad_len) != 1) {
return srtp_err_status_algo_fail;
}
} else {
if (EVP_DecryptUpdate(c->ctx, NULL, &len, aad, aad_len) != 1) {
return srtp_err_status_algo_fail;
}
}

rv = EVP_Cipher(c->ctx, NULL, aad, aad_len);
if (rv < 0 || (uint32_t)rv != aad_len) {
return (srtp_err_status_algo_fail);
} else {
return (srtp_err_status_ok);
if (len != (int)aad_len) {
return srtp_err_status_algo_fail;
}

return srtp_err_status_ok;
}

/*
Expand All @@ -299,6 +290,7 @@ 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;
int len = 0;

if (c->dir != srtp_direction_encrypt) {
return srtp_err_status_bad_param;
Expand All @@ -311,24 +303,29 @@ static srtp_err_status_t srtp_aes_gcm_openssl_encrypt(void *cv,
/*
* Encrypt the data
*/
EVP_Cipher(c->ctx, dst, src, src_len);
if (EVP_EncryptUpdate(c->ctx, dst, &len, src, src_len) != 1) {
return srtp_err_status_algo_fail;
}
*dst_len = len;

/*
* Calculate the tag
*/
EVP_Cipher(c->ctx, NULL, NULL, 0);
if (EVP_EncryptFinal_ex(c->ctx, dst + len, &len) != 1) {
return srtp_err_status_algo_fail;
}
*dst_len += len;

/*
* Retrieve the tag
*/
if (!EVP_CIPHER_CTX_ctrl(c->ctx, EVP_CTRL_GCM_GET_TAG, c->tag_len,
dst + src_len)) {
if (EVP_CIPHER_CTX_ctrl(c->ctx, EVP_CTRL_GCM_GET_TAG, c->tag_len,
dst + *dst_len) != 1) {
return srtp_err_status_algo_fail;
}
*dst_len += c->tag_len;

*dst_len = src_len + c->tag_len;

return (srtp_err_status_ok);
return srtp_err_status_ok;
}

/*
Expand All @@ -346,6 +343,7 @@ 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;
int len = 0;

if (c->dir != srtp_direction_decrypt) {
return srtp_err_status_bad_param;
Expand All @@ -359,32 +357,34 @@ static srtp_err_status_t srtp_aes_gcm_openssl_decrypt(void *cv,
return srtp_err_status_buffer_small;
}

/*
* Decrypt the data
*/
if (EVP_DecryptUpdate(c->ctx, dst, &len, src, src_len - c->tag_len) != 1) {
return srtp_err_status_algo_fail;
}
*dst_len = len;

/*
* Set the tag before decrypting
*
* explicitly cast away const of src
*/
if (!EVP_CIPHER_CTX_ctrl(
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;
(void *)(uintptr_t)(src + (src_len - c->tag_len))) != 1) {
return srtp_err_status_algo_fail;
}
EVP_Cipher(c->ctx, dst, src, src_len - c->tag_len);

/*
* Check the tag
*/
if (EVP_Cipher(c->ctx, NULL, NULL, 0)) {
if (EVP_DecryptFinal_ex(c->ctx, dst + *dst_len, &len) != 1) {
return srtp_err_status_auth_fail;
}
*dst_len += len;

/*
* Reduce the buffer size by the tag length since the tag
* is not part of the original payload
*/
*dst_len = src_len -= c->tag_len;

return (srtp_err_status_ok);
return srtp_err_status_ok;
}

/*
Expand Down
9 changes: 9 additions & 0 deletions valgrind.supp
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
https://github.com/openssl/openssl/issues/19719
Memcheck:Cond
obj:*libcrypto.so.3
obj:*libcrypto.so.3
fun:EVP_DecryptFinal_ex
fun:srtp_aes_gcm_openssl_decrypt
...
}

0 comments on commit bf992dc

Please sign in to comment.