[RFC] libsks for Pkcs#11 services through OP-TEE SKS TA

[etienne-lms]

This change introduces an implementation of the Pkcs#11 Cryptoki library that interface a specific trusted application in the secure side provided by the optee_os source tree.

This P-R implements the basics for the client library. The effective implementation of the services will come later. However this P-R still introduces a first TA command for this initial P-R to at least invoke the SKS TA to check it is available when library is initialized.

This P-R does not introduce the full TA API itself that is being reviewed through (edited: OP-TEE/optee_os#2732). The client library will integrate the released TA API once its is approved and merged in optee_os mainline.

This P-R replaces #121.

edited, url of TA API P-R and below
#139 proposes the first commits of this RFC.

[wamserma]

a bit more review

[wamserma]

LGTM in general, possible errors in the function parameters can still be weeded out when the functions are actually implemented.

[etienne-lms]

Minor question but to be clarified asap if possible: since removing the SKS meaningless acronym, I'm looking for a nice name for the client library. Not too small, not too long, to address a cryptoki integration ina GTD TEE env (as OP-TEE is).
I am thinking of libcryptokioptee, libckteec, libckoptee. Any other artistic idea?

[etienne-lms]

Refer to OP-TEE/optee_test#281 (comment) for running the basic test.

[jenswi-linaro]

I'd prefer libckteec, but I don't have a strong opinion on this.

[etienne-lms]

let's go for libckteec. short and still refers to both libteec and cryptoki/ck.

[jenswi-linaro]

Comments for the commit "libckteec: Introduce PKCS#11 API (2.40-e01) header"

[jenswi-linaro]

Comments for the commit "libckteec: fully stubbed cryptoki API"

[jenswi-linaro]

Comment for the commit "libckteec: local utilities":

There's no ASSERT defined in this commit, please update the commit message.

[jenswi-linaro]

For the commit "libckteec: implement C_GetFunctionList()":
Acked-by: Jens Wiklander <[email protected]>

[jenswi-linaro]

Comment for "libckteec: introduce minimal PKCS11 TA API"

[jenswi-linaro]

Comment for the commit "libckteec: helpers for Cryptoki/PKCS11 TA IDs conversion"

[jenswi-linaro]

Comment for the commit "libckteec: debug helpers for PKCS#11 IDs as strings"

[jenswi-linaro]

Comments for commit "libckteec: generic invocation of the PKCS11 TA"

[jenswi-linaro]

Comments for "libckteec: sanity on APIs regarding library initialization"

[etienne-lms]

Hello,
I will submit soon an update of this P-R. Syncing with TA API exported from OP-TEE/optee_os#2733, I must discard few commits about ID conversions originally in this optee_client series. Also I will fully rewrite commit 'libckteec: generic invocation of the PKCS11 TA", not append fixup commit on top of it, since it needed lot of changes.

[jenswi-linaro]

For commit "libckteec: local utilities" + fixup:
Reviewed-by: Jens Wiklander <[email protected]>

[jenswi-linaro]

For commit "libckteec: generic invocation of the PKCS11 TA":
Reviewed-by: Jens Wiklander <[email protected]>

[jenswi-linaro]

For commit "libckteec: sanity on APIs regarding library initialization":
Reviewed-by: Jens Wiklander <[email protected]>

[etienne-lms]

Added late fixup on top of the series for an issue reported by Travis.
Also saw few issues (missing #include <> and few commit messages to be updated).
Here is the new series with tags applied where applicable.

[jenswi-linaro]

Please rebase on master to include checkpatch in the review.

[etienne-lms]

Update for checkpatch compliance. I squashed all fixup related to checkpatch complains in "libckteec: Introduce PKCS#11 API (2.40-e01) header …".

I see there are still failures. Most I'll fix. But there ar esome I don't understand what wring with:

• ARRAY_SIZE. What wrong with the label?

total: 0 errors, 0 warnings, 1 checks, 96 lines checked
47f5399 libckteec: local utilities
WARNING: Prefer ARRAY_SIZE(array)
#29: FILE: libckteec/src/local_utils.h:9:
+#define ARRAY_SIZE(array)	(sizeof(array) / sizeof(array[0]))

• BIT() macro. There is no standard header file for BIT() hence I prefer to not rely on BIT in the user client API header file.

total: 0 errors, 0 warnings, 3 checks, 1020 lines checked
a86e8c1 libckteec: Introduce PKCS#11 API (2.40-e01) header
CHECK: Prefer using the BIT macro
#158: FILE: libckteec/include/pkcs11.h:93:
+#define CKF_ARRAY_ATTRIBUTE		(1U << 30)

[jenswi-linaro]

I'm fine with ignoring the BIT() and ARRAY_SIZE() warnings.

[jenswi-linaro]

Let me know when you're done with the current updates and I'll go through it once more.

[etienne-lms]

Another fixup commit for "libckteec: implement C_GetFunctionList()" (R-b tagged). b16330b simplifies the code the comply with checkpatch.
I will squash the 2 fixup commits (b16330b and a9bc912) if you agree.

[etienne-lms]

I am done with the updates.

[jenswi-linaro]

Looks good to me.
Acked-by: Jens Wiklander <[email protected]>

[etienne-lms]

squashed. tags applied.

[jbech-linaro]

FYI, I've noticed that this PR breaks the make optee-client-common in build.git. Haven't checked if it is just a missing flag or if it's actually broken. Looks like CMake files have been updated, but not the GNU make files.

$ cd build && make -j`nproc` optee-client-common                                         
make -C /home/jbech/devel/optee_projects/qemu/build/../optee_client CROSS_COMPILE="/usr/bin/ccache /home/jbech/devel/optee_projects/qemu/build/../toolchains/aarch32/bin/arm-linux-gnueabihf-
" CFG_TEE_BENCHMARK=n CFG_TA_TEST_PATH=y          
make[1]: Entering directory '/home/jbech/devel/optee_projects/qemu/optee_client'
Building libteec.so                                                     
Building libckteec.so                 
  CC      src/ck_helpers.c                                     
  CC      src/pkcs11_api.c                                 
  CC      src/invoke_ta.c
  CC      src/tee_client_api.c                                          
  CC      src/teec_trace.c                                                
  LINK    /home/jbech/devel/optee_projects/qemu/optee_client/libckteec/../out/libckteec/libckteec.so.0.1.0
  AR      /home/jbech/devel/optee_projects/qemu/optee_client/libckteec/../out/libckteec/libckteec.a
/home/jbech/devel/optee_projects/reference/toolchains/aarch32/bin/../lib/gcc/arm-linux-gnueabihf/8.3.0/../../../../arm-linux-gnueabihf/bin/ld: cannot find -lteec
collect2: error: ld returned 1 exit status                            
Makefile:47: recipe for target '/home/jbech/devel/optee_projects/qemu/optee_client/libckteec/../out/libckteec/libckteec.so.0.1.0' failed
make[2]: *** [/home/jbech/devel/optee_projects/qemu/optee_client/libckteec/../out/libckteec/libckteec.so.0.1.0] Error 1
Makefile:40: recipe for target 'build-libckteec' failed                  
make[1]: *** [build-libckteec] Error 2     
make[1]: *** Waiting for unfinished jobs....                     
  AR      /home/jbech/devel/optee_projects/qemu/optee_client/libteec/../out/libteec/libteec.a
  LINK    /home/jbech/devel/optee_projects/qemu/optee_client/libteec/../out/libteec/libteec.so.1.0.0
                                                       
make[1]: Leaving directory '/home/jbech/devel/optee_projects/qemu/optee_client'
common.mk:413: recipe for target 'optee-client-common' failed
make: *** [optee-client-common] Error 2

[jforissier]

@jbech-linaro I've noticed that too, does not happen every time. Now I have double checked, it's just a missing dependency. I'll create a PR.

[jforissier]

#183