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

Fix corePKCS11 pal implementation (CA-204) #107

Open
wants to merge 1 commit into
base: release/beta
Choose a base branch
from

Conversation

chinglee-iot
Copy link

  • Use nvs_get_blob instead of nvs_get_str

* Use nvs_get_blob instead of nvs_get_str
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@github-actions github-actions bot changed the title Fix corePKCS11 pal implementation Fix corePKCS11 pal implementation (CA-204) Mar 22, 2022
@Steven35700
Copy link

Hi,

I don't understand why you want to replace "nvs_get_blob" with "nvs_get_str" in "PKCS11_PAL_FindObject" function.

In the "PKCS11_PAL_FindObject" function you modified, it allows to check the presence of a string. In the "PKCS11_PAL_GetObjectValue" function, we retrieve the value of this string. For me the whole problem comes rather from the "PKCS11_PAL_SaveObject" function which writes a blob ???

Also in the OTA example, in the CSV partition file, P11_CSK is encrypted as String.

@chinglee-iot
Copy link
Author

Hi Steven,

I uses the nvs_get_blob to replace nvs_get_str fro the following reasons.

  1. PKCS11_PAL_SaveObject uses nvs_set_blob. There will be error when I use nvs_get_str.
  2. The main branch use nvs_get_blob for the implementation.

@paulbartell
Copy link

paulbartell commented Jul 14, 2023

@Steven35700 corePKCS11 stores objects in DER format by default, which may contain null bytes. For this reason, nvs_get_blob is more appropriate.

@idea--list
Copy link

Is there any reason not to merge this PR? I mean it is waiting for over a year now.

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

5 participants