Skip to content

Commit

Permalink
cryp: prevent direct calls to update and final functions
Browse files Browse the repository at this point in the history
With inconsistent or malformed data it has been possible to call
"update" and "final" crypto functions directly. Using a fuzzer tool [1]
we have seen that this results in asserts, i.e., a crash that
potentially could leak sensitive information.

By setting the state (initialized) in the crypto context (i.e., the
tee_cryp_state) at the end of all syscall_*_init functions and then add
a check of the state at the beginning of all update and final functions,
  we prevent direct entrance to the "update" and "final" functions.

[1] https://github.com/MartijnB/optee_fuzzer

Fixes: OP-TEE-2019-0021

Signed-off-by: Joakim Bech <joakim.bech@linaro.org>
Reported-by: Martijn Bogaard <bogaard@riscure.com>
Acked-by: Jerome Forissier <jerome.forissier@linaro.org>
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
  • Loading branch information
jbech-linaro authored and jforissier committed Oct 8, 2019
1 parent 28aa35f commit 34a08be
Showing 1 changed file with 36 additions and 0 deletions.
36 changes: 36 additions & 0 deletions core/tee/tee_svc_cryp.c
Expand Up @@ -31,6 +31,11 @@
#include <tee/tee_cryp_pbkdf2.h>
#endif

enum cryp_state {
CRYP_STATE_INITIALIZED = 0,
CRYP_STATE_UNINITIALIZED
};

typedef void (*tee_cryp_ctx_finalize_func_t) (void *ctx, uint32_t algo);
struct tee_cryp_state {
TAILQ_ENTRY(tee_cryp_state) link;
Expand All @@ -40,6 +45,7 @@ struct tee_cryp_state {
vaddr_t key2;
void *ctx;
tee_cryp_ctx_finalize_func_t ctx_finalize;
enum cryp_state state;
};

struct tee_cryp_obj_secret {
Expand Down Expand Up @@ -2056,6 +2062,7 @@ TEE_Result syscall_cryp_state_alloc(unsigned long algo, unsigned long mode,
TAILQ_INSERT_TAIL(&utc->cryp_states, cs, link);
cs->algo = algo;
cs->mode = mode;
cs->state = CRYP_STATE_UNINITIALIZED;

switch (TEE_ALG_GET_CLASS(algo)) {
case TEE_OPERATION_EXTENSION:
Expand Down Expand Up @@ -2179,6 +2186,8 @@ TEE_Result syscall_cryp_state_copy(unsigned long dst, unsigned long src)
return TEE_ERROR_BAD_STATE;
}

cs_dst->state = cs_src->state;

return TEE_SUCCESS;
}

Expand Down Expand Up @@ -2253,6 +2262,8 @@ TEE_Result syscall_hash_init(unsigned long state,
return TEE_ERROR_BAD_PARAMETERS;
}

cs->state = CRYP_STATE_INITIALIZED;

return TEE_SUCCESS;
}

Expand Down Expand Up @@ -2286,6 +2297,9 @@ TEE_Result syscall_hash_update(unsigned long state, const void *chunk,
if (res != TEE_SUCCESS)
return res;

if (cs->state != CRYP_STATE_INITIALIZED)
return TEE_ERROR_BAD_STATE;

switch (TEE_ALG_GET_CLASS(cs->algo)) {
case TEE_OPERATION_DIGEST:
res = crypto_hash_update(cs->ctx, cs->algo, chunk, chunk_size);
Expand Down Expand Up @@ -2344,6 +2358,9 @@ TEE_Result syscall_hash_final(unsigned long state, const void *chunk,
if (res != TEE_SUCCESS)
return res;

if (cs->state != CRYP_STATE_INITIALIZED)
return TEE_ERROR_BAD_STATE;

switch (TEE_ALG_GET_CLASS(cs->algo)) {
case TEE_OPERATION_DIGEST:
res = tee_hash_get_digest_size(cs->algo, &hash_size);
Expand Down Expand Up @@ -2453,6 +2470,8 @@ TEE_Result syscall_cipher_init(unsigned long state, const void *iv,
return res;

cs->ctx_finalize = crypto_cipher_final;
cs->state = CRYP_STATE_INITIALIZED;

return TEE_SUCCESS;
}

Expand All @@ -2473,6 +2492,9 @@ static TEE_Result tee_svc_cipher_update_helper(unsigned long state,
if (res != TEE_SUCCESS)
return res;

if (cs->state != CRYP_STATE_INITIALIZED)
return TEE_ERROR_BAD_STATE;

res = tee_mmu_check_access_rights(to_user_ta_ctx(sess->ctx),
TEE_MEMORY_ACCESS_READ |
TEE_MEMORY_ACCESS_ANY_OWNER,
Expand Down Expand Up @@ -2987,6 +3009,8 @@ TEE_Result syscall_authenc_init(unsigned long state, const void *nonce,
return res;

cs->ctx_finalize = (tee_cryp_ctx_finalize_func_t)crypto_authenc_final;
cs->state = CRYP_STATE_INITIALIZED;

return TEE_SUCCESS;
}

Expand All @@ -3013,6 +3037,9 @@ TEE_Result syscall_authenc_update_aad(unsigned long state,
if (res != TEE_SUCCESS)
return res;

if (cs->state != CRYP_STATE_INITIALIZED)
return TEE_ERROR_BAD_STATE;

if (TEE_ALG_GET_CLASS(cs->algo) != TEE_OPERATION_AE)
return TEE_ERROR_BAD_STATE;

Expand Down Expand Up @@ -3041,6 +3068,9 @@ TEE_Result syscall_authenc_update_payload(unsigned long state,
if (res != TEE_SUCCESS)
return res;

if (cs->state != CRYP_STATE_INITIALIZED)
return TEE_ERROR_BAD_STATE;

if (TEE_ALG_GET_CLASS(cs->algo) != TEE_OPERATION_AE)
return TEE_ERROR_BAD_STATE;

Expand Down Expand Up @@ -3100,6 +3130,9 @@ TEE_Result syscall_authenc_enc_final(unsigned long state,
if (res != TEE_SUCCESS)
return res;

if (cs->state != CRYP_STATE_INITIALIZED)
return TEE_ERROR_BAD_STATE;

if (cs->mode != TEE_MODE_ENCRYPT)
return TEE_ERROR_BAD_PARAMETERS;

Expand Down Expand Up @@ -3184,6 +3217,9 @@ TEE_Result syscall_authenc_dec_final(unsigned long state,
if (res != TEE_SUCCESS)
return res;

if (cs->state != CRYP_STATE_INITIALIZED)
return TEE_ERROR_BAD_STATE;

if (cs->mode != TEE_MODE_DECRYPT)
return TEE_ERROR_BAD_PARAMETERS;

Expand Down

0 comments on commit 34a08be

Please sign in to comment.