ta: pkcs11: Add implementation for random number generation #4383
Conversation
ta/pkcs11/src/pkcs11_token.c
Outdated
| if (!buffer) | ||
| return PKCS11_CKR_DEVICE_MEMORY; | ||
|
|
||
| TEE_GenerateRandom(buffer, out->memref.size); |
There was a problem hiding this comment.
I did not understand why trusted_keys's TEE_GenerateRandom() doesn't require TEE_Malloc():
https://github.com/OP-TEE/optee_os/blob/master/ta/trusted_keys/entry.c#L45
Tried to observe from GlobalPlatform specification and did not notice why there is required to have ACL check in TEE_GenerateRandom():
https://github.com/OP-TEE/optee_os/blob/master/core/tee/tee_svc_cryp.c#L3200
So should that be relaxed a bit or does trusted_keys also need fixing?
There was a problem hiding this comment.
TEE_GenerateRandom() has a [out] annotation on the randomBuffer pointer which means (among other things) that it must reside in shared memory, in other words it must be private to the TA.
So yes I would think trusted_keys needs fixing. How is it tested? It seems not :-/
There was a problem hiding this comment.
OK found the annotation description.
There was a problem hiding this comment.
Just a note about trusted_keys -- Linux kernel has optee-rng driver which when configured should provide additional seed to Linux kernel's already good RNG -- so I don't understand why there is separate entry point for this -- but I leave this to others to resolve.
There was a problem hiding this comment.
So yes I would think trusted_keys needs fixing. How is it tested? It seems not :-/
Its tested but I don't think its tested after this patch [1] which has switched to stricter memory requirements for randomBuffer to reside in TA private memory only. So will fix it using TEE_Malloc() instead.
[1] e12c9f6
@vesajaaskelainen Trusted Keys in Linux are meant to be generated, sealed and unsealed via trust source only. See this [2] discussion for reference.
There was a problem hiding this comment.
@b49020 thanks for following up on this. Note, when I wrote it seemed not tested I didn't mean to say you tested poorly 😉 but that we don't have a systematic test in optee_test for instance. Which would be nice to have once all the dependencies are stable.
There was a problem hiding this comment.
I agree to have a systematic test in optee_test. See related discussion here [1].
vesajaaskelainen
force-pushed
the
pkcs11-rng
branch
from
634cad1 to
16dab17
Compare
|
Updated with the comments. |
|
Thanks @vesajaaskelainen . Patches LGTM. |
vesajaaskelainen
force-pushed
the
pkcs11-rng
branch
from
16dab17 to
52f0b75
Compare
|
Updated with the review comments. |
|
|
|
|
Allocate command IDs for C_SeedRandom() and C_GenerateRandom() functionality. Reviewed-by: Ruchika Gupta <[email protected]> Reviewed-by: Etienne Carriere <[email protected]> Signed-off-by: Vesa Jääskeläinen <[email protected]>
Add code for handling C_SeedRandom() and C_GenerateRandom() functionality. Reviewed-by: Ruchika Gupta <[email protected]> Reviewed-by: Etienne Carriere <[email protected]> Signed-off-by: Vesa Jääskeläinen <[email protected]>
vesajaaskelainen
force-pushed
the
pkcs11-rng
branch
from
52f0b75 to
be6159b
Compare
|
Thanks! Tags applied. |
Add code for handling C_SeedRandom() and C_GenerateRandom() functionality.
Relates to:
#4283
Goes with:
OP-TEE/optee_client#256
OP-TEE/optee_test#487