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

xtest: regression 41xx target Secure Key Services tests #281

Merged
merged 1 commit into from
Feb 3, 2020

Conversation

etienne-lms
Copy link
Contributor

@etienne-lms etienne-lms commented May 22, 2018

Introduce regression test 41xx for PKCS#11 and SKS TA testing.

edited: this P-R depends on RFCs OP-TEE/optee_os#2732 and OP-TEE/optee_client#138.

host/xtest/Makefile Outdated Show resolved Hide resolved
host/xtest/CMakeLists.txt Outdated Show resolved Hide resolved
host/xtest/regression_4100.c Outdated Show resolved Hide resolved
host/xtest/regression_4100.c Outdated Show resolved Hide resolved
@etienne-lms
Copy link
Contributor Author

comments addressed.
need to rebase...

@etienne-lms
Copy link
Contributor Author

rebased.

@etienne-lms etienne-lms force-pushed the sks-merge branch 2 times, most recently from 1fc6476 to e9dc8f8 Compare June 15, 2018 14:07
@etienne-lms
Copy link
Contributor Author

Updated according to recent changes (library renaming) in OP-TEE/optee_client#121.

@etienne-lms
Copy link
Contributor Author

Rebase on latest master.
PKCS#11/SKS tests have moved to regression_42xx since regression_41xx are already assigned.

This P-R is still a first step for libsks to propose a PKCS#11 client API. This P-R introduces a regression test series. The sole test done here is calling the library initialization;

@@ -77,6 +77,14 @@ ifeq ($(CFG_SECURE_DATA_PATH),y)
srcs += sdp_basic.c
endif

ifeq ($(CFG_SECURE_KEY_SERVICES),y)
srcs += regression_4200.c
OPTEE_SKS_HEADERS ?= ../../../optee_client/libsks/include
Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't reach outside of this git into some other git for internal .h files. Whatever .h files are needed should be installed somewhere where we can find them.

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 will remove this line and CFLAGS += -I$(OPTEE_SKS_HEADERS) below.
The lib headers are already exported in $(OPTEE_CLIENT_EXPORT)/include.

Copy link
Contributor

@wamserma wamserma left a comment

Choose a reason for hiding this comment

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

LGTM, but add a test for SKS_CMD_PING.

@jenswi-linaro
Copy link
Contributor

Please replace the SKS acronym and Secure Key Services with pkcs11 since that's what it's about.
Add this as a separate suit in xtest, pkcs11 for instance.
+1 to a test for SKS_CMD_PING.

@etienne-lms
Copy link
Contributor Author

With this latest update, pkcs11 TA tests are no more in the regression tests and the TA is no more default embedded within Qemu OP-TEE core.
To test the pkcs11 TA:

######################################################
#
# pkcs11
#
######################################################
* pkcs11_1000 Initialize and close Cryptoki library
  pkcs11_1000 OK
+-----------------------------------------------------
Result of testsuite pkcs11:
pkcs11_1000 OK
+-----------------------------------------------------
5 subtests of which 0 failed
1 test case of which 0 failed
0 test cases were skipped
TEE test application done!

@etienne-lms
Copy link
Contributor Author

For info, IBART fails because it needs libckteec from OP-TEE/optee_client#138:

 32693:  /home/optee/devel/reference/toolchains83_201903/aarch64/bin/../lib/gcc/aarch64-linux-gnu/8.3.0/../../../../aarch64-linux-gnu/bin/ld: cannot find -lckteec

@jbech-linaro
Copy link
Contributor

For info, IBART fails because it needs libckteec from OP-TEE/optee_client#138:

Let me know if I need to update the build config in IBART ... add flags etc.

Copy link
Contributor

@jenswi-linaro jenswi-linaro left a comment

Choose a reason for hiding this comment

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

Reviewed-by: Jens Wiklander <[email protected]>

@etienne-lms
Copy link
Contributor Author

Let me know if I need to update the build config in IBART ... add flags etc.

Do you want IBART always run the pkcs11 tests ? now and in the future?

@etienne-lms
Copy link
Contributor Author

For info, the pkcs11 TA gets pinged when client opens (initializes) the cryptoki library, i.e. from test pkcs11_1000 in the P-R.

Introduce pkcs11 tests for PKCS#11 services through the PKCS11 TA.

Signed-off-by: Etienne Carriere <[email protected]>
Signed-off-by: Victor Chong <[email protected]>
Reviewed-by: Jens Wiklander <[email protected]>
@etienne-lms
Copy link
Contributor Author

R-B tag applied.
Depends on OP-TEE/optee_os#2733 and OP-TEE/optee_client#138.

Maybe add the pkcs11 to IBART.
Need build with CFG_PKCS11_TA=y and embedding the TA image, i.e. adding TA to early TAs: CFG_IN_TREE_EARLY_TAS="avb/023f8f1a-292a-432b-8fc4-de8471358067 pkcs11/fd02c9da-306c-48c7-a49c-bbd827ae86ee".

@jbech-linaro
Copy link
Contributor

Maybe add the pkcs11 to IBART.

Maybe? Give me a 'yes' or 'no' instead and I'll do what you think makes sense :)

@jforissier
Copy link
Contributor

jforissier commented Jan 31, 2020

Maybe add the pkcs11 to IBART.

Maybe? Give me a 'yes' or 'no' instead and I'll do what you think makes sense :)

😄

I have just merged the optee_client and optee_os PRs, so we're good from a dependency point of view. And I agree it would be nice to have this checked by IBART.

Edit: I see IBART is failing with ...ld: cannot find -lckteec, perhaps it had started before I have merged the client PR, and needs to be restarted.

@etienne-lms
Copy link
Contributor Author

@jbech-linaro, yes, add to IBART please.

@jbech-linaro
Copy link
Contributor

@jbech-linaro, yes, add to IBART please.

That was clear! 😄

So, I'll add

make ... CFG_PKCS11_TA=y CFG_IN_TREE_EARLY_TAS="avb/023f8f1a-292a-432b-8fc4-de8471358067 pkcs11/fd02c9da-306c-48c7-a49c-bbd827ae86ee"

and we're good to go?

@etienne-lms
Copy link
Contributor Author

Should be, yes.
You should see a trace pkcs11_1001 OK in xtest logs.

@jforissier jforissier merged commit 109c1d7 into OP-TEE:master Feb 3, 2020
@etienne-lms
Copy link
Contributor Author

thanks

@etienne-lms etienne-lms deleted the sks-merge branch February 4, 2020 19:12
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.

5 participants