Skip to content

Commit

Permalink
Merge pull request #718 from pabuhler/remove-get-tag
Browse files Browse the repository at this point in the history 
change srtp_cipher_encrypt to append the tag generated
  • Loading branch information
@pabuhler
pabuhler authored Jul 8, 2024
2 parents 5dea791 + 3515b82 commit 88a9aa1
Show file tree
Hide file tree
Showing 20 changed files with 397 additions and 382 deletions.
2 changes: 1 addition & 1 deletion CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion Makefile.in
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
53 changes: 18 additions & 35 deletions crypto/cipher/aes_gcm_mbedtls.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,50 +290,27 @@ 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) {
return (srtp_err_status_bad_param);
if (c->dir != srtp_direction_encrypt) {
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) {
debug_print(srtp_mod_aes_gcm, "mbedtls error code: %d", errCode);
return srtp_err_status_bad_param;
}

*dst_len = src_len;
*dst_len = src_len + c->tag_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_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;
}

/*
Expand All @@ -354,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)) {
Expand All @@ -370,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;
}

/*
Expand All @@ -379,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;
}

/*
Expand All @@ -393,6 +374,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,
Expand All @@ -401,15 +383,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,
Expand All @@ -418,8 +401,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 */
84 changes: 23 additions & 61 deletions crypto/cipher/aes_gcm_nss.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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, &param, 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, &param, 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;
Expand All @@ -319,11 +339,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
Expand All @@ -335,58 +350,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);
}

/*
Expand Down Expand Up @@ -442,7 +406,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
Expand All @@ -461,7 +424,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
Expand Down
Loading

0 comments on commit 88a9aa1

Please sign in to comment.