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

pkcs11: remove NUL byte from SO PIN and use ASCII characters #658

Merged
merged 3 commits into from
Apr 4, 2023

Conversation

jforissier
Copy link
Contributor

@jforissier jforissier commented Mar 24, 2023

[Edited]

PKCS#11 PINs are UTF-8 character strings. The default SO PIN used in the
test suite contains a NUL, which causes issues with for example
pkcs11-tool which interprets the NUL byte as the end of the string, thus
making the slot impossible to use from the command line.

Therefore, and for better compatibility with command-line tools after
xtest has been run, use simpler (ASCII) values for both the SO and user
PINs.

Signed-off-by: Jerome Forissier jerome.forissier@linaro.org

@jforissier
Copy link
Contributor Author

I shall probably provide a small C program to change the SO PIN to the new one on a test board that has already run xtest -t pkcs11. I'll look into it.

@jforissier
Copy link
Contributor Author

Force-pushed to reword the commit message after @etienne-lms comments. No code change.

@jforissier jforissier changed the title pkcs11: remove invalid null bytes from SO PIN and use valid UTF-8 pkcs11: remove NUL byte from SO PIN and use ASCII characters Mar 24, 2023
Copy link
Contributor

@etienne-lms etienne-lms left a comment

Choose a reason for hiding this comment

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

Acked-by: Etienne Carriere <etienne.carriere@linaro.org>

@vesajaaskelainen
Copy link
Contributor

Acked-by: Vesa Jääskeläinen <vesa.jaaskelainen@vaisala.com>

PKCS#11 PINs are UTF-8 character strings. The default SO PIN used in the
test suite contains a NUL, which causes issues with for example
pkcs11-tool which interprets the NUL byte as the end of the string, thus
making the slot impossible to use from the command line.

Therefore, and for better compatibility with command-line tools after
xtest has been run, use simpler (ASCII) values for both the SO and user
PINs.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Acked-by: Etienne Carriere <etienne.carriere@linaro.org>
Acked-by: Vesa Jääskeläinen <vesa.jaaskelainen@vaisala.com>
@jforissier jforissier force-pushed the pkcs11-pins branch 3 times, most recently from eba2f67 to b1b3e05 Compare March 28, 2023 10:02
@jforissier
Copy link
Contributor Author

I am updating this PR with two more commits:

  • A cosmetic fix
  • A compatibility patch to be able to run the newer xtest on systems that have the older SO PIN without causing any error.

(and sorry for the multiple pushes)

Copy link
Contributor

@etienne-lms etienne-lms 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: Etienne Carriere <etienne.carriere@linaro.org>
for commits "xtest: pkcs11: fix subcase name mismatch"
and "xtest: pkcs11: update SO PIN automatically if needed".

xtest pkcs11_1003 shows the following warning:

 o pkcs11_1003.3 Test C_Login()/C_Logout() with PIN based authentication
 Do_ADBG_EndSubCase: Active SubCase "Test C_Login()/C_Logout() with PIN
 based authentication" doesn't match supplied title
 "Test C_Login()/C_Logout() with PIN based authorization"

Fix the second label.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org>
If the system under test is using the now deprecated binary SO PIN
('0x00', '0x01', '0x02' etc), change it to the new one automatically.
This is a compatibility patch to avoid failing any test on older
systems.

Tested on QEMUv8 by running "xtest -t pkcs11" once with optee_test at
commit c0a6172 ("regression 1013: lower number of loops when pager
is constrained"), in other words: before the SO PIN was changed, then a
second time with a build issued from this commit.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org>
@jforissier jforissier merged commit ac0f210 into OP-TEE:master Apr 4, 2023
1 check passed
@jforissier jforissier deleted the pkcs11-pins branch April 4, 2023 12:33
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.

None yet

3 participants