Skip to content

PSA Crypto Service - multipart operation memory fixes #10232

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Apr 11, 2019
Merged

PSA Crypto Service - multipart operation memory fixes #10232

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
1b26e0d
Fix crypto service abort functionality - cipher
Mar 20, 2019
da999c6
Fix crypto service abort functionality - hash
Mar 20, 2019
545e669
Fix crypto service abort functionality - mac
Mar 20, 2019
8044f6d
Fix crypto service abort functionality - generators
Mar 24, 2019
7b35e76
Do not allocate zero sized buffers - mac
Mar 27, 2019
979ca1e
Do not allocate zero sized buffers - hash
Mar 27, 2019
18feb26
Do not allocate zero sized buffers - asymmetric
Mar 27, 2019
19952d9
Do not allocate zero sized buffers - aead
Mar 27, 2019
f3294ef
Do not allocate zero sized buffers - cipher
Mar 27, 2019
19b8381
Do not allocate zero sized buffers - keys
Mar 27, 2019
abd9c66
Do not allocate zero sized buffers - entropy
Mar 27, 2019
749b1e3
Do not allocate zero sized buffers - rng
Mar 27, 2019
b26890e
Do not allocate zero sized buffers - generators
Mar 27, 2019
a446ee6
Rename internal function destroy_hash_clone to clear_hash_clone
Mar 27, 2019
2b81588
Add message context cleanup helper function
Mar 28, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 64 additions & 17 deletions components/TARGET_PSA/services/crypto/COMPONENT_PSA_SRV_IPC/psa_crypto_spm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,10 @@ static psa_status_t psa_mac_setup(psa_mac_operation_t *operation,
psa_algorithm_t alg,
psa_sec_function_t func)
{
if (operation->handle != PSA_NULL_HANDLE) {
return (PSA_ERROR_BAD_STATE);
}

psa_crypto_ipc_t psa_crypto_ipc = {
.func = func,
.handle = key_handle,
Expand All @@ -133,6 +137,9 @@ static psa_status_t psa_mac_setup(psa_mac_operation_t *operation,
return (status);
}
status = ipc_call(&operation->handle, &in_vec, 1, NULL, 0, false);
if (status != PSA_SUCCESS) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ipc_call is a wrapper by itself.
i suggest for the same idea of code being clear and refactoring.
to put the
if (status != PSA_SUCCESS) {
ipc_close(&operation->handle);

inside ipc_call in case of failure.
with only one important note, if and only if we always want to close connections on call failure

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not always true, e.g. generators functions like psa_get_generator_capacity etc. don't close in case of error, so leaving it as is.

Copy link
Contributor

@alekshex alekshex Apr 1, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why they don't close?
maybe we want to consider having a flag passed whether to close or not.
and still hide the close inside? there are too many of the same "close"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think having another flag makes much of a difference regarding code size or readability, if you think it's important I'll make another change.

ipc_close(&operation->handle);
}
return (status);
}

Expand Down Expand Up @@ -168,6 +175,9 @@ psa_status_t psa_mac_update(psa_mac_operation_t *operation,
};

psa_status_t status = ipc_call(&operation->handle, in_vec, 2, NULL, 0, false);
if (status != PSA_SUCCESS) {
ipc_close(&operation->handle);
}
return (status);
}

Expand Down Expand Up @@ -240,6 +250,10 @@ psa_status_t psa_hash_abort(psa_hash_operation_t *operation)
psa_status_t psa_hash_setup(psa_hash_operation_t *operation,
psa_algorithm_t alg)
{
if (operation->handle != PSA_NULL_HANDLE) {
return (PSA_ERROR_BAD_STATE);
}

psa_crypto_ipc_t psa_crypto_ipc = {
.func = PSA_HASH_SETUP,
.handle = 0,
Expand All @@ -253,6 +267,9 @@ psa_status_t psa_hash_setup(psa_hash_operation_t *operation,
return (status);
}
status = ipc_call(&operation->handle, &in_vec, 1, NULL, 0, false);
if (status != PSA_SUCCESS) {
ipc_close(&operation->handle);
}
return (status);
}

Expand All @@ -272,6 +289,9 @@ psa_status_t psa_hash_update(psa_hash_operation_t *operation,
};

psa_status_t status = ipc_call(&operation->handle, in_vec, 2, NULL, 0, false);
if (status != PSA_SUCCESS) {
ipc_close(&operation->handle);
}
return (status);
}

Expand Down Expand Up @@ -986,6 +1006,10 @@ psa_status_t psa_key_derivation(psa_crypto_generator_t *generator,
size_t label_length,
size_t capacity)
{
if (generator->handle != PSA_NULL_HANDLE) {
return (PSA_ERROR_BAD_STATE);
}

psa_crypto_derivation_ipc_t psa_crypto_ipc = {
.func = PSA_KEY_DERIVATION,
.handle = key_handle,
Expand All @@ -1004,6 +1028,9 @@ psa_status_t psa_key_derivation(psa_crypto_generator_t *generator,
return (status);
}
status = ipc_call(&generator->handle, in_vec, 3, NULL, 0, false);
if (status != PSA_SUCCESS) {
ipc_close(&generator->handle);
}
return (status);
}

Expand All @@ -1013,6 +1040,10 @@ psa_status_t psa_key_agreement(psa_crypto_generator_t *generator,
size_t peer_key_length,
psa_algorithm_t alg)
{
if (generator->handle != PSA_NULL_HANDLE) {
return (PSA_ERROR_BAD_STATE);
}

psa_crypto_derivation_ipc_t psa_crypto_ipc = {
.func = PSA_KEY_AGREEMENT,
.handle = private_key_handle,
Expand All @@ -1030,6 +1061,9 @@ psa_status_t psa_key_agreement(psa_crypto_generator_t *generator,
return (status);
}
status = ipc_call(&generator->handle, in_vec, 2, NULL, 0, false);
if (status != PSA_SUCCESS) {
ipc_close(&generator->handle);
}
return (status);
}

Expand All @@ -1055,12 +1089,17 @@ psa_status_t psa_generator_abort(psa_crypto_generator_t *generator)
/****************************************************************/
/* SYMMETRIC */
/****************************************************************/
psa_status_t psa_cipher_encrypt_setup(psa_cipher_operation_t *operation,
psa_key_handle_t key_handle,
psa_algorithm_t alg)
static psa_status_t psa_cipher_setup(psa_cipher_operation_t *operation,
psa_key_handle_t key_handle,
psa_algorithm_t alg,
psa_sec_function_t func)
{
if (operation->handle != PSA_NULL_HANDLE) {
return (PSA_ERROR_BAD_STATE);
}

psa_crypto_ipc_t psa_crypto_ipc = {
.func = PSA_CIPHER_ENCRYPT_SETUP,
.func = func,
.handle = key_handle,
.alg = alg
};
Expand All @@ -1072,26 +1111,25 @@ psa_status_t psa_cipher_encrypt_setup(psa_cipher_operation_t *operation,
return (status);
}
status = ipc_call(&operation->handle, &in_vec, 1, NULL, 0, false);
if (status != PSA_SUCCESS) {
ipc_close(&operation->handle);
}
return (status);
}

psa_status_t psa_cipher_decrypt_setup(psa_cipher_operation_t *operation,
psa_status_t psa_cipher_encrypt_setup(psa_cipher_operation_t *operation,
psa_key_handle_t key_handle,
psa_algorithm_t alg)
{
psa_crypto_ipc_t psa_crypto_ipc = {
.func = PSA_CIPHER_DECRYPT_SETUP,
.handle = key_handle,
.alg = alg
};

psa_invec in_vec = { &psa_crypto_ipc, sizeof(psa_crypto_ipc) };
psa_status_t status = psa_cipher_setup(operation, key_handle, alg, PSA_CIPHER_ENCRYPT_SETUP);
return (status);
}

psa_status_t status = ipc_connect(PSA_SYMMETRIC_ID, &operation->handle);
if (status != PSA_SUCCESS) {
return (status);
}
status = ipc_call(&operation->handle, &in_vec, 1, NULL, 0, false);
psa_status_t psa_cipher_decrypt_setup(psa_cipher_operation_t *operation,
psa_key_handle_t key_handle,
psa_algorithm_t alg)
{
psa_status_t status = psa_cipher_setup(operation, key_handle, alg, PSA_CIPHER_DECRYPT_SETUP);
return (status);
}

Expand All @@ -1114,6 +1152,9 @@ psa_status_t psa_cipher_generate_iv(psa_cipher_operation_t *operation,
};

psa_status_t status = ipc_call(&operation->handle, &in_vec, 1, out_vec, 2, false);
if (status != PSA_SUCCESS) {
ipc_close(&operation->handle);
}
return (status);
}

Expand All @@ -1133,6 +1174,9 @@ psa_status_t psa_cipher_set_iv(psa_cipher_operation_t *operation,
};

psa_status_t status = ipc_call(&operation->handle, in_vec, 2, NULL, 0, false);
if (status != PSA_SUCCESS) {
ipc_close(&operation->handle);
}
return (status);
}

Expand Down Expand Up @@ -1160,6 +1204,9 @@ psa_status_t psa_cipher_update(psa_cipher_operation_t *operation,
};

psa_status_t status = ipc_call(&operation->handle, in_vec, 2, out_vec, 2, false);
if (status != PSA_SUCCESS) {
ipc_close(&operation->handle);
}
return (status);
}

Expand Down
Loading