Skip to content
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.

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

AES ciphering APIs (setup operation, load key/iv, process ciphering) #3

Closed
wants to merge 3 commits into from

Conversation

etienne-lms
Copy link
Contributor

This change provides an example of a AES ciphering operation from a
trusted application (TA) using the GPD TEE Core Internal API.

AES TA implements the basics for setting a AES/CTR ciphering session
using the TEE Core Internal API:

  • Opening a session toward the AES TA creates AES ciphering session.
  • A TA command allows to setup and allocate the ciphering resources.
  • A TA command allows to load the AES key.
  • A TA command allows to reset the initial vector.
  • A TA command allows to cipher an input buffer into an output buffer.

The sample application create a AES128-CTR session and encodes some
data, reloading the key every 2000 bytes and resetting the AES sequence
initial vector every 500 bytes. Then the equivalent sequence is done for
decoding the data. Finally we check clear and decoded data are the same.

aes/host/main.c Outdated
/* To the the UUID (found the the TA's h-file(s)) */
#include <aes_ta.h>

#ifndef MIN
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't we always use lib/libutils/ext/include/util.h and thereby safely get rid of this re-definition?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, didn't pay attention here ... this is user space NW, so ignore my comment.

aes/host/main.c Outdated
printf("Prepare session with the TA\n");
prepare_tee_session(&ctx);

/*
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, even though this is all good, I think it is unnecessary to do things in chunks etc here since the "examples git" should serve with as easy as possible examples. I.e., a straight forward "set key, set iv and cipher" should be sufficient.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. I designed it this way to clarify the several steps: prepare operation / prepare key / reset IV / cipher something, each requiring one or more calls to GP APIs.
If you believe this bring just more complexity, I will move to a simpler setup: load key, setup and process ciphering from a single REE command.

Copy link
Contributor

@jbech-linaro jbech-linaro Oct 11, 2017

Choose a reason for hiding this comment

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

No, that is not what I'm saying, I'm just saying that the loop just adds complexity. For a newcomer I believe it is easier to just see a couple of commands.

// Psuedo
set_key(key)
set_iv(iv)
encrypt(buffer)

I think it is a good idea to split it up to show how this can be done in different calls. We could add another example too where we do everything in one go, but then you need to hardcode the key and IV in the TA instead.

aes/ta/aes_ta.c Outdated
#define AES256_KEY_BYTE_SIZE (AES256_KEY_BIT_SIZE / 8)

/*
* Cipehring context: each opened session relates to a cipehring operation.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/cipering/cipher/

aes/ta/aes_ta.c Outdated
* Cipehring context: each opened session relates to a cipehring operation.
* - configure the AES flavour from a command.
* - load key from a command (here the key is provided by the REE)
* - reset init vector (here iv is provided by the REE)
Copy link
Contributor

Choose a reason for hiding this comment

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

s/iv/IV/

aes/ta/aes_ta.c Outdated
uint32_t algo; /* AES flavour */
uint32_t mode; /* Encode or decode */
size_t key_size; /* AES key size in byte */
TEE_OperationHandle op_handle; /* aes ciphering operation */
Copy link
Contributor

Choose a reason for hiding this comment

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

s/aes/AES/

sess->key_handle = TEE_HANDLE_NULL;
sess->op_handle = TEE_HANDLE_NULL;

*session = (void *)sess;
Copy link
Contributor

@jbech-linaro jbech-linaro Oct 11, 2017

Choose a reason for hiding this comment

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

This works and I guess it's OK here. But in a real use cases I think we should avoid giving back information like this to NW (even though that address should be inaccessible).

* dummy key in the operation so that operation can be reset
* when updating the key.
*/
key = TEE_Malloc(sess->key_size, 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the dummy really necessary? Wouldn't it be enough to set

sess->key_handle = TEE_HANDLE_NULL;

and then in set_aes_key() check

if (sess->key_handle != TEE_HANDLE_NULL)
    TEE_ResetTransientObject(sess->key_handle);

That would seem more natural to me. Am I missing something?

@jbech-linaro
Copy link
Contributor

I think I'm done with the review. This example contains as you wrote not only a pure encryption it also contains session handling. I think keeping a clean encrypt only and a separate example for "how to work with sessions" would be easier for newcomers. But, in general I'm OK with this. I think as written right now adds a bit more complexity than necessary, but this is much better than nothing. So with the minor fixes fixed, you have my R-B.
Reviewed-by: Joakim Bech <joakim.bech@linaro.org>

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
@etienne-lms
Copy link
Contributor Author

Rebased on top of #8. Please wait it is merge before merging this.
Tag applied.

Prefix test applications for the examples with 'optee_example_'.
Gitignore the generate test applications.
Few extra minor changes as make verbosity and source coding style.
STR_TRACE_USER_TA is deprecated.
Update README.

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
This change provides an example of a AES ciphering operation from a
trusted application (TA) using the GPD TEE Core Internal API.

AES TA implements the basics for setting a AES/CTR ciphering session
using the TEE Core Internal API:
- Opening a session toward the AES TA creates AES ciphering session.
- A TA command allows to setup and allocate the ciphering resources.
- A TA command allows to load the AES key.
- A TA command allows to reset the initial vector.
- A TA command allows to cipher an input buffer into an output buffer.

The sample application creates an AES128-CTR encryption session,
provides the AES key and and initial vector then request encryption of
a data buffer. Then it sets up a decryption session, provide the same
key and initial vector and request decryption of the encrypted content.
Finally it checks clear and decoded data are the same.

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
Reviewed-by: Joakim Bech <joakim.bech@linaro.org>
@etienne-lms
Copy link
Contributor Author

Latest proposal superseded by #9. Will close if no objection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants