Git Product home page Git Product logo

Comments (13)

etienne-lms avatar etienne-lms commented on May 28, 2024 1

PINs are passed in a serialized manner, so client does not provide a NULL or non NULL PIN pointer to the TA, it rather passes a PIN size and possibly appended PIN data. When pin size is 0, deserialization of the pin data (with serialargs_get_ptr()) in the TA returns a NULL pin pointer and a NUL pin size.
https://github.com/OP-TEE/optee_os/blob/3.20.0/ta/pkcs11/src/serializer.c#L89-L92
So testing pin is sufficient.

from optee_test.

vesajaaskelainen avatar vesajaaskelainen commented on May 28, 2024 1

Here is the proposed change:
OP-TEE/optee_os#5914

from optee_test.

jforissier avatar jforissier commented on May 28, 2024

#658

from optee_test.

etienne-lms avatar etienne-lms commented on May 28, 2024

I think you're right on the pin codes used in xtest, they should be char values '0' '1' ..., not decimal values, so that we can connect a pkcs11 token with standard tools after the tocke has been initialized with xtest.

Regarding the validity of the UTF8 chars used in a pin, the spec says:

This standard allows PIN values to contain any valid UTF8 character, but the token may impose subset restrictions.

Value 0 is a valid UTF8 char, no? It represents NUL and it not printable in ASCII, but still a valid UTF8 character.

Regarding CFG_PKCS11_TA_AUTH_TEE_IDENTITY=y, it relies on a null PIN, not a PIN starting with a NUL byte. UTF8 does not enforce UTF8 strings to end on NUL byte and not include NUL bytes before UTF8 string end, AFAICT.
Note the TA enforces PIN size is at least 4 bytes (whatever they are): PKCS11_TOKEN_PIN_SIZE_MIN.

from optee_test.

jforissier avatar jforissier commented on May 28, 2024

Value 0 is a valid UTF8 char, no? It represents NUL and it not printable in ASCII, but still a valid UTF8 character.

Technically, yes. Then perhaps I should re-word my patch in #658 to just mention compatibility with pkcs11-tool.

Regarding CFG_PKCS11_TA_AUTH_TEE_IDENTITY=y, it relies on a null PIN, not a PIN starting with a NUL byte.

Indeed the codeif (!pin) here so it is this comment that is wrong: https://github.com/OP-TEE/optee_os/blob/3.20.0/ta/pkcs11/include/pkcs11_ta.h#L895-L898

Thanks!

from optee_test.

etienne-lms avatar etienne-lms commented on May 28, 2024

Technically, yes. Then perhaps I should re-word my patch in #658 to just mention compatibility with pkcs11-tool.

Maybe the interface library (libckteec) could reject PINs that contain NUL byte since some tools treat such bytes as PIN string termination. Or this could is a restriction implemented in the TA itself.

Indeed the codeif (!pin) here so it is this comment that is wrong: https://github.com/OP-TEE/optee_os/blob/3.20.0/ta/pkcs11/include/pkcs11_ta.h#L895-L898

An empty PIN remains a PIN with no bytes, not a PIN containing a NUL byte. So I think the pointed comment is right.

from optee_test.

jforissier avatar jforissier commented on May 28, 2024

An empty PIN remains a PIN with no bytes, not a PIN containing a NUL byte. So I think the pointed comment is right.

Wouldn't an empty pin verify pin && !pin_size? In which case the test should be if (!pin || !pin_size) to reflect the comment.

from optee_test.

vesajaaskelainen avatar vesajaaskelainen commented on May 28, 2024

Hmm... If I understood correctly what is being discussed in here:

  1. xtest uses a bit silly tests for testing PIN based logins which @jforissier is changing with the referred PR
  2. There was confusion about serialization layer that is now sorted out

Was there something that I missed 😃 ?

from optee_test.

jforissier avatar jforissier commented on May 28, 2024

Hmm... If I understood correctly what is being discussed in here:

1. `xtest` uses a bit silly tests for testing PIN based logins which @jforissier is changing with the referred PR

2. There was confusion about serialization layer that is now sorted out

Correct.

Was there something that I missed 😃 ?

Yes, the fact that a slot can be re-initialized and put into "TEE identity authentication" mode without requiring the previous SO pin.
Test case, with some debug output with CFG_TEE_TA_LOG_LEVEL=4 (some of the debug traces are mine):

# alias p11="pkcs11-tool --module /usr/lib/libckteec.so.0"
# p11 --init-token --so-pin 12345678 --slot-index 2 --label tests --init-pin --pin 12345
# p11 --login --token-label tests --keypairgen --key-type rsa:1024 --label testkey --pin 12345
# p11 --login --token-label tests --keypairgen --key-type rsa:1024 --label testkey --pin 12346
error: PKCS11 function C_Login failed: rv = CKR_PIN_INCORRECT (0xa0)
Aborting.
>>> All good, user PIN authentication works

# p11 --init-token --so-pin '' --slot-index 2 --label tests --init-pin --pin 12345
D/TA:  TA_InvokeCommandEntryPoint:143 PKCS11_CMD_PING p#0 4@0x4001753c, p#1 --- 0@0x0, p#2 out 12@0x40016548
D/TA:  TA_InvokeCommandEntryPoint:364 PKCS11_CMD_PING rc 0/OK
D/TA:  TA_InvokeCommandEntryPoint:143 PKCS11_CMD_SLOT_LIST p#0 4@0x40016454, p#1 --- 0@0x0, p#2 out 0@0x40017000
D/TA:  TA_InvokeCommandEntryPoint:364 PKCS11_CMD_SLOT_LIST rc 0x150/BUFFER_TOO_SMALL
D/TA:  TA_InvokeCommandEntryPoint:143 PKCS11_CMD_SLOT_LIST p#0 4@0x40017454, p#1 --- 0@0x0, p#2 out 12@0x40016000
D/TA:  TA_InvokeCommandEntryPoint:364 PKCS11_CMD_SLOT_LIST rc 0/OK
Using slot with index 2 (0x2)
D/TA:  TA_InvokeCommandEntryPoint:143 PKCS11_CMD_TOKEN_INFO p#0 4@0x40017000, p#1 --- 0@0x0, p#2 out 160@0x40016000
D/TA:  TA_InvokeCommandEntryPoint:364 PKCS11_CMD_TOKEN_INFO rc 0/OK
D/TA:  TA_InvokeCommandEntryPoint:143 PKCS11_CMD_INIT_TOKEN p#0 40@0x40016000, p#1 --- 0@0x0, p#2 --- 0@0x0
D/TA:  entry_ck_token_initialize:823
D/TA:  entry_ck_token_initialize:829
D/TA:  entry_ck_token_initialize:834
D/TA:  entry_ck_token_initialize:839
D/TA:  entry_ck_token_initialize:844
D/TA:  entry_ck_token_initialize:849
D/TA:  entry_ck_token_initialize:853
D/TA:  entry_ck_token_initialize:858
D/TA:  entry_ck_token_initialize:864
D/TA:  entry_ck_token_initialize:871
D/TA:  entry_ck_token_initialize:873
D/TA:  entry_ck_token_initialize:881
D/TA:  entry_ck_token_initialize:885
D/TA:  entry_ck_token_initialize:890
I/TA: PKCS11 token 2: initialized
D/TA:  TA_InvokeCommandEntryPoint:364 PKCS11_CMD_INIT_TOKEN rc 0/OK
Token successfully initialized
D/TA:  TA_InvokeCommandEntryPoint:143 PKCS11_CMD_OPEN_SESSION p#0 8@0x40016000, p#1 --- 0@0x0, p#2 out 4@0x40017000
D/TA:  entry_ck_open_session:669 Open PKCS11 session 1
D/TA:  TA_InvokeCommandEntryPoint:364 PKCS11_CMD_OPEN_SESSION rc 0/OK
D/TA:  TA_InvokeCommandEntryPoint:143 PKCS11_CMD_TOKEN_INFO p#0 4@0x40016000, p#1 --- 0@0x0, p#2 out 160@0x40017000
D/TA:  TA_InvokeCommandEntryPoint:364 PKCS11_CMD_TOKEN_INFO rc 0/OK
D/TA:  TA_InvokeCommandEntryPoint:143 PKCS11_CMD_LOGIN p#0 12@0x40016000, p#1 --- 0@0x0, p#2 --- 0@0x0
I/TA: PKCS11 session 1: login
D/TA:  TA_InvokeCommandEntryPoint:364 PKCS11_CMD_LOGIN rc 0/OK
D/TA:  TA_InvokeCommandEntryPoint:143 PKCS11_CMD_TOKEN_INFO p#0 4@0x40017000, p#1 --- 0@0x0, p#2 out 160@0x40016000
D/TA:  TA_InvokeCommandEntryPoint:364 PKCS11_CMD_TOKEN_INFO rc 0/OK
D/TA:  TA_InvokeCommandEntryPoint:143 PKCS11_CMD_INIT_PIN p#0 13@0x40016000, p#1 --- 0@0x0, p#2 --- 0@0x0
I/TA: PKCS11 session 1: init PIN
E/TA:  setup_identity_auth_from_pin:197 Invalid PIN ACL string - login
D/TA:  TA_InvokeCommandEntryPoint:364 PKCS11_CMD_INIT_PIN rc 0xa1/PIN_INVALID
D/TA:  close_ck_session:691 Close PKCS11 session 1
error: PKCS11 function C_InitPIN failed: rv = CKR_PIN_INVALID (0xa1)
Aborting.
>>> This seems to be trying to enable "TEE identity authentication", user PIN is rejected because not the expected format

root@trs-qemuarm64:~# p11 --init-token --so-pin '' --slot-index 2 --label tests --init-pin --pin public
D/TA:  TA_InvokeCommandEntryPoint:143 PKCS11_CMD_PING p#0 4@0x4001691c, p#1 --- 0@0x0, p#2 out 12@0x40017928
D/TA:  TA_InvokeCommandEntryPoint:364 PKCS11_CMD_PING rc 0/OK
D/TA:  TA_InvokeCommandEntryPoint:143 PKCS11_CMD_SLOT_LIST p#0 4@0x40017834, p#1 --- 0@0x0, p#2 out 0@0x40016000
D/TA:  TA_InvokeCommandEntryPoint:364 PKCS11_CMD_SLOT_LIST rc 0x150/BUFFER_TOO_SMALL
D/TA:  TA_InvokeCommandEntryPoint:143 PKCS11_CMD_SLOT_LIST p#0 4@0x40016834, p#1 --- 0@0x0, p#2 out 12@0x40017000
D/TA:  TA_InvokeCommandEntryPoint:364 PKCS11_CMD_SLOT_LIST rc 0/OK
Using slot with index 2 (0x2)
D/TA:  TA_InvokeCommandEntryPoint:143 PKCS11_CMD_TOKEN_INFO p#0 4@0x40016000, p#1 --- 0@0x0, p#2 out 160@0x40017000
D/TA:  TA_InvokeCommandEntryPoint:364 PKCS11_CMD_TOKEN_INFO rc 0/OK
D/TA:  TA_InvokeCommandEntryPoint:143 PKCS11_CMD_INIT_TOKEN p#0 40@0x40016000, p#1 --- 0@0x0, p#2 --- 0@0x0
D/TA:  entry_ck_token_initialize:823
D/TA:  entry_ck_token_initialize:829
D/TA:  entry_ck_token_initialize:834
D/TA:  entry_ck_token_initialize:839
D/TA:  entry_ck_token_initialize:844
D/TA:  entry_ck_token_initialize:849
D/TA:  entry_ck_token_initialize:853
D/TA:  entry_ck_token_initialize:858
D/TA:  entry_ck_token_initialize:864
D/TA:  entry_ck_token_initialize:871
D/TA:  entry_ck_token_initialize:873
D/TA:  entry_ck_token_initialize:876
D/TA:  entry_ck_token_initialize:881
D/TA:  entry_ck_token_initialize:885
D/TA:  entry_ck_token_initialize:890
I/TA: PKCS11 token 2: initialized
D/TA:  TA_InvokeCommandEntryPoint:364 PKCS11_CMD_INIT_TOKEN rc 0/OK
>>> "TEE identity authentication" succesfully enabled with "public" user authentication?

# p11 --login --token-label tests --keypairgen --key-type rsa:1024 --label testkey --pin 12345
# p11 --login --token-label tests --keypairgen --key-type rsa:1024 --label testkey --pin 12346
>>> Indeed, the slot can be used to generate keys with any user PIN

from optee_test.

vesajaaskelainen avatar vesajaaskelainen commented on May 28, 2024

Ok. Thanks for the test case.

Let's see.

from optee_test.

vesajaaskelainen avatar vesajaaskelainen commented on May 28, 2024

Just updated the dev environment to latest versions and were able to verify the behavior.

There is slight problem with the fix however.

In standard it is specified that once token is initialized then SO PIN must be provided for the function for authentication purposes. It also specifies that old SO PIN will be new SO PIN for empty token.

We do not support using both PIN authentication and TEE identity authentication independently for different users.
During token initialization PKCS11_CKFT_PROTECTED_AUTHENTICATION_PATH flag gets set/cleared and is used for selecting the authentication mode for all users.

In theory we could support going from TEE Identity mode to PIN mode but not backwards -- where as I would probably select that then you are stuck with your initial selection to limit miss use.

Specification leaves lot open for protected authentication path -- like SO PIN recovery is out of scope. So in theory we could have another tool to allow full reset of the token(s). There was some discussion on this end some time ago for tests.

from optee_test.

vesajaaskelainen avatar vesajaaskelainen commented on May 28, 2024

Tried out PIN change trick with pkcs11-tool but it does not allow empty pin to be entered from prompt but from command line arguments it allows it to happen.

So if agreeable then we could still retain the authentication mode change.

I will share the changes tomorrow.

from optee_test.

github-actions avatar github-actions commented on May 28, 2024

This issue has been marked as a stale issue because it has been open (more than) 30 days with no activity. Remove the stale label or add a comment, otherwise this issue will automatically be closed in 5 days. Note that you can always re-open a closed issue at any time.

from optee_test.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.