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

core: add interface for user TA loading and decryption/authentication #1498

Closed
wants to merge 1 commit into from

Conversation

jforissier
Copy link
Contributor

Currently, TA load, parse and verification are closely tied together
and hard-coded into the TEE core. However, some use cases may require
custom load and verification methods.

Introduce struct tee_ta_load_ops and struct tee_ta_decryption_ops to
allow for customization. The default implementation is identical to the
previous code, except for memory allocation.

Signed-off-by: Zeng Tao prime.zeng@hisilicon.com
Tested-by: Zeng Tao prime.zeng@hisilicon.com (Hi3798cv200Dmb, xtest)
[jf: cosmetic changes, cleanup includes, update changelog]
Signed-off-by: Jerome Forissier jerome.forissier@linaro.org

Currently, TA load, parse and verification are closely tied together
and hard-coded into the TEE core. However, some use cases may require
custom load and verification methods.

Introduce struct tee_ta_load_ops and struct tee_ta_decryption_ops to
allow for customization. The default implementation is identical to the
previous code, except for memory allocation.

Signed-off-by: Zeng Tao <prime.zeng@hisilicon.com>
Tested-by: Zeng Tao <prime.zeng@hisilicon.com> (Hi3798cv200Dmb, xtest)
[jf: cosmetic changes, cleanup includes, update changelog]
Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
@jforissier
Copy link
Contributor Author

jforissier commented Apr 26, 2017

Creating this PR on behalf of Zengtao (@prime-zeng), from a patch he sent me privately ; thanks for the contribution!

I have made a couple of adjustments already. In addition I will address the review comments raised here.

I am slightly concerned by the change in the way memory is allocated. If I'm not mistaken, the new method has an overhead of twice the size of the TA binary, and I think this could easily be fixed by letting .load_file() (and possibly .verify_and_decrypt()) handle the allocations instead of having them do memcpy(). Thoughts?

@jenswi-linaro
Copy link
Contributor

I've been thinking of a completely different approach, something along:
A TA of more or less the current format (optionally encrypted too) is supplied to a pseudo TA which verifies (and optionally decrypts) the TA into a special secure storage file.

TAs can only be instantiated if they already are installed in secure storage. Loading of TAs will probably be a bit quicker as there's no need to do RSA verifications any longer.

Different formats of supplied TAs can easily be handled by just using a different pseudo TA which would in turn use some internal interface to register and store the TA in secure storage.

@jforissier
Copy link
Contributor Author

@jenswi-linaro that's an interesting approach, but the point of this PR is only to introduce some flexibility in the low-level handling of user TA binaries (load, optionally decrypt, and authenticate a TA). So I think it is useful on its own. I see your proposal (as well as the current TA mechanism) as one possible implementation. We could even support multiple methods and try them in sequence. We must consider that manufacturers may have unique requirements regarding the location where the binaries are stored (REE fs, dedicated flash storage maybe?) and the way the keys are obtained.

@jenswi-linaro
Copy link
Contributor

@jforissier, I understand that, but sooner or later we'll need to deal with how TAs are deployed and then it's good if this is a step in the right direction. The flexibility you'd like to introduce could be achieved with an interface like this:

struct tee_ta_store_handle;
struct tee_ta_store_ops {
        TEE_Result (*open)(const TEE_UUID *uuid,
                           struct tee_ta_store_handle **h);
        TEE_Result (*read)(struct tee_ta_store_handle *h, void *data,
                           size_t *len)
        void (*close)(struct tee_ta_store_handle *h);
};

which should be able to accommodate reading from secure storage too when needed. Note that open() doesn't really need to know if the TA is there or not, that's OK to discover later in read().
elf_load_init() we be declared like:

TEE_Result elf_load_init(const struct tee_ta_store_ops *ops,
                         struct tee_ta_store_handle *h,
                         struct elf_load_state **state);

Where the handle already has be created by load_elf(), which also is the one closing the handle.

Another nice side effect is that the TA can be loaded in increments which would reduce the amount of required shared memory (very large TAs are already causing problems from time to time).

@jforissier
Copy link
Contributor Author

@jenswi-linaro please check #1513. Also @prime-zeng let me know if you're OK with this new version.
Closing this PR.

@jforissier jforissier closed this May 5, 2017
@jforissier jforissier deleted the custom-ta-load branch November 13, 2018 13:13
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.

3 participants