pkcs11: Add TEE Identity based authentication support#4222
pkcs11: Add TEE Identity based authentication support#4222jforissier merged 3 commits intoOP-TEE:masterfrom
Conversation
etienne-lms
left a comment
etienne-lms
left a comment
There was a problem hiding this comment.
Thank you @vesajaaskelainen for the proposal.
See some comments.
I wonder if such feature should be under a CFG_PKCS11_TA_AUTH_TEE_IDENTITY=y.
I think the ID strings passed by client ("public", "user", "group") should be formalized in the TA API, that is in pkcs11_ta.h, maybe some PKCS11_AUTH_TEE_IDENTITY_PUBLIC/_USER/_GROUP macros.
etienne-lms
left a comment
etienne-lms
left a comment
There was a problem hiding this comment.
For commit "pkcs11: set_pin: use token shortcut like in other pin functions", I don't think this change adds much, but yet LGTM.
Idea was to keep logic in sync so that if you compare them it is easy to see what is difference. |
vesajaaskelainen
force-pushed
the
token-acl
branch
from
3cc59c5 to
2d87fa0
Compare
Updated the PR. There is now commit "WIP: merge or drop?" which demoes that. I kinda prefer it without the defines -- but if you want it there then just say and I merge the commit. |
About this. As we are now in embedded device context. For me now as there is another option than having PINs hard coded or randomized per device I do not see so much practical use for PINs. PINs are somewhat cumbersome and unsecure in such context as that cannot be verified in kernel to restrict possible access form different apps and protecting it from exposure can be challenging. With TEE Identity base authentication one can trust that kernel will verify that calling process is member in group or that user is correct and those cannot be faked from user space and as a bonus there is no PIN maintenance burden. As the presented mechanism here also allows one to have one token with PINs and another token with TEE Identity based authentication I see that one can do whatever is wanted without having a conflict between those two so they could co-exists. What we did is we have one SO admin group and then specific token groups for different tokens and then calling application needs to be member of such group in order to use the token. Should it be always available it could ease other applications to integrate for this feature by default and not as special configuration option. |
There was a problem hiding this comment.
The API used to set SO and User PINs should be stated somewhere. Maybe as inline comments in pkcs11_ta.h. From my understanding:
- SO decides (
C_InitToken()) whether login SO/User authent. is based on PIN or TEE client identity. SO can switch between the two by callingC_InitToken(). - (when TEE client authent.) SO uses
C_InitPIN()with PIN values"public"/"user:..."/group:..."to bind User authentication data to a TEE Client identity. - (when TEE client authent.) User uses
C_SetPIN()to change the authentication data and bind to another user-id/group-id (or make the identity public). In this case, User callsC_SetPIN()with a new PIN value like"public","user:..."or"group:...".
As this scheme is quite specific, i think it deserves a nickname/config.
|
@etienne-lms In meanwhile we have been performing end-to-end tests with TLS 1.2 & 1.3 with RSA and ECDSA in the real device. Private keys and certificates has been stored in custody of PKCS11 TA (we have some extra changes to make all that happen). We have been also trying to execute all kind of tests and running PKCS11 test suites to figure out what kind of state we are in. In this testing we have noticed two features:
Example of certutil's 'Press Enter': |
vesajaaskelainen
force-pushed
the
token-acl
branch
from
2d87fa0 to
539504f
Compare
|
@etienne-lms I believe I have addressed all your comments. |
etienne-lms
left a comment
etienne-lms
left a comment
There was a problem hiding this comment.
thanks for the update.
| $(call force,CFG_CORE_MBEDTLS_MPI,y) | ||
|
|
||
| # Enable PKCS#11 TA's TEE Identity based authentication support | ||
| CFG_PKCS11_TA_AUTH_TEE_IDENTITY ?= y |
| assert(token->db_main->flags & PKCS11_CKFT_TOKEN_INITIALIZED); | ||
|
|
||
| #if defined(CFG_PKCS11_TA_AUTH_TEE_IDENTITY) | ||
| if (token->db_main->flags & PKCS11_CKFT_PROTECTED_AUTHENTICATION_PATH) |
There was a problem hiding this comment.
Here and other occurences in this change: s/#if defined/IS_ENABLED/ inside function blocks:
#include <config.h>
if (IS_ENABLED(CFG_PKCS11_TA_AUTH_TEE_IDENTITY) &&
token->db_main->flags & PKCS11_CKFT_PROTECTED_AUTHENTICATION_PATH)
return verify_identity_auth(token, PKCS11_CKU_SO);- a
static inlinestub in header file when cfg disabled.
There was a problem hiding this comment.
Changed for the parts that felt it was good for.
| uint8_t so_pin_hash[TEE_MAX_HASH_SIZE]; | ||
| union { | ||
| uint8_t so_pin_hash[TEE_MAX_HASH_SIZE]; | ||
| #if defined(CFG_PKCS11_TA_AUTH_TEE_IDENTITY) |
There was a problem hiding this comment.
I think we don't really need the #if pragma here. I would remove it (despite it legitimate).
There was a problem hiding this comment.
I feel better if we remote the guard here so that storage would be always same what ever build is there.
so -> OK!
vesajaaskelainen
force-pushed
the
token-acl
branch
from
539504f to
30d2578
Compare
|
Note: will prefix commit subjects with "ta: " in next round of updates. |
vesajaaskelainen
force-pushed
the
token-acl
branch
from
30d2578 to
123d9b8
Compare
|
I believe I have addressed all comments. Also rebased on top of master. |
There was a problem hiding this comment.
Reviewed-by: Etienne Carriere <[email protected]>.
@ruchi393, @jenswi-linaro, @ricardosalveti, are you ok with this feature and how it is handled?
Sum-up:
-
TA side:
The PKCS11 commands used to set a new PIN can instead enable authentication of linux ACL client identity (user/group). Archived by using a empty new PIN value in the PKCS##11 API functions.
TA feature is enabled thruCFG_PKCS11_TA_AUTH_TEE_IDENTITY=y. -
Client side:
At libckteec init, use env variablesCKTEEC_LOGIN_TYPE(possiblyCKTEEC_LOGIN_GID) to determine the PKCS11 token authentication scheme (public (default), user, group) to be used when client uses a empty PIN to set the token credentials. OP-TEE/optee_client#236.
(edited)
vesajaaskelainen
force-pushed
the
token-acl
branch
from
123d9b8 to
e47eb4d
Compare
|
@etienne-lms thanks for the review. Applied tags. |
|
yes, I did a high level review of the patches. The concept is good. Acked-by: Ruchika Gupta [email protected] |
If successful token init has been performed and new PIN is set then reset all pin change flags. Call update_persistent_db() only once as a last step during the execution. Acked-by: Ruchika Gupta <[email protected]> Reviewed-by: Etienne Carriere <[email protected]> Signed-off-by: Vesa Jääskeläinen <[email protected]>
Use common shortcut variable 'token' as in check_so_pin and check_user_pin. Acked-by: Ruchika Gupta <[email protected]> Reviewed-by: Etienne Carriere <[email protected]> Signed-off-by: Vesa Jääskeläinen <[email protected]>
In C_InitToken() if PIN is NULL_PTR then it will activate TEE Identity based authentication support for token. Once activated: - When ever PIN is required client's TEE Identity will be used for authentication - PIN failure counters are disabled - If new PIN is given as input it is in form of PIN ACL string - It can be disabled with C_InitToken with non-zero PIN Internally protected authentication path will be used for mode determination. Acked-by: Ruchika Gupta <[email protected]> Reviewed-by: Etienne Carriere <[email protected]> Signed-off-by: Vesa Jääskeläinen <[email protected]>
vesajaaskelainen
force-pushed
the
token-acl
branch
from
e47eb4d to
50edfb9
Compare
|
Applied tags. |
TEE Identity-based authentication provides functionality to log in without a pin but using a User or Group identity. The feature is valuable for embedded devices where there is no user interaction. With the TEE Identity authentication, the pin should be empty. The use case is: CKTEEC_LOGIN_TYPE=user ssh-add -s /usr/lib/libckteec.so.0 For TEE Identity-based auth pin should be provided as an empty string. But in the current implementation, if a pin is empty the message structure will not be populated with the pin(see sshbuf_put_string). As a result, the error: "pin required". As a solution add a new line character. The details about the TEE Identity-based authentication: OP-TEE/optee_os#4222 Signed-off-by: Valerii Chubar <[email protected]>
4 participants
In C_InitToken() if PIN is NULL_PTR then it will activate TEE Identity
based authentication support for token.
Once activated:
Internally protected authentication path will be used for mode determination.
Implements:
#3946
Testing this without having other PKCS11 features like RSA, EC, objects lists and so needs a bit imagination how it would eventually work but I add comments what happens in the test sequence.
Actual functionality has also been verified with "full blown" features in real hardware with NXP CPU.
Testing in here happened in our custom Yocto build with Xilinx ZynqMPSoC ZCU102 QEMU image.
Testing with normal PIN usage with slot 2:
Please note that now the token flags also behave more logically than before too.
Now initializing token 1 with SO being root user and USER being group membership for 1000.
What to notice from output is that now in token flags there is "PIN pad present" which means that protected authentication path is activated.
Second thing to notice is
CKTEEC_LOGIN_TYPE=userwhich was set before token 1 initialization. This told libckteec to use TEE_LOGIN_USER when logging in. As PIN was empty string it told PKCS11 TA to enter into protected authentication path mode AND record current login credentials (gpd.client.identity) for SO.Third thing to notice is that "foobar" was given as a pin and that was ignored as token is now in TEE Identity authentication mode and actual SO PIN entry is ignored and only OPTEE TEE Client's identity is being used to validate access.
Fourth thing to notice is that user pin is has now a special format that gets activated only when in protected authentication path mode (
group:0ece43ed-001d-51bd-bab7-480c8954c3cf).Above means here that "group" is connected to TEE Login type TEE_LOGIN_GROUP and presented UUID is TEE client ID that is populated by Linux kernel.
In here the client ID is calculated with uuidgen tool. (Note: Ubuntu bionic has broken uuidgen tool for namespace operations -- where as focal has already fixed uuidgen).
Client ID calculation MUST match what kernel is generating. An example:
One thing to be aware -- PIN counters are now disabled to make sure that token does not get locked while the device is out there (as that is not practically recoverable) and if access would be wrong then it would be more or less instantly locked as there is not human participant in here.
Now let's switch to user which is member of group 1000.
Testing that ACL works.
First thing that is verified above is that token 2 still accepts PIN.
In first try invalid PIN is given where as in second try valid PIN is given and system just responds back that listing objects is not supported.
Second thing to notice is that when testing token 1 access we first get incorrect PIN error as we tried to login as "TEE_LOGIN_PUBLIC" (being default).
Then CKTEEC_LOGIN_TYPE is configured to "group" to make TEE login to be "group" and CKTEEC_LOGIN_GID specified group identifier to be passed to kernel so that it verifies membership and can then form the client ID.
Now that login is correct it responds back that listing objects is not supported like it was with normal PIN mode.
When compiled with CFG_PKCS11_TA_AUTH_TEE_IDENTITY=n and performing initialization steps:
From above output one can see that normal PIN usage is working and then trying to do ACL token fails because SO PIN is invalid (empty and too short).