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

nrf_security: Add Oberon PSA configurations in Kconfig #11676

Merged
merged 3 commits into from
Sep 7, 2023

Conversation

Vge0rge
Copy link
Contributor

@Vge0rge Vge0rge commented Jul 3, 2023

Change the nrf_security configurations to follow the configuration scheme from the Oberon PSA core.

Signed-off-by: Georgios Vasilakis georgios.vasilakis@nordicsemi.no

test-sdk-nrf: geva-11676

@github-actions github-actions bot added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Jul 3, 2023
@NordicBuilder
Copy link
Contributor

NordicBuilder commented Jul 3, 2023

Test specification

CI/Jenkins/NRF

  • Integration Platforms

CI/Jenkins/integration

Test Module File based changes Manually selected West overwrite
test-fw-nrfconnect-chip X
test-fw-nrfconnect-nrf_crypto X
test-fw-nrfconnect-tfm X
test-sdk-homekit X

Detailed information of selected test modules

Note: This message is automatically posted and updated by the CI

@NordicBuilder
Copy link
Contributor

You can find the documentation preview for this PR at this link. It will be updated about 10 minutes after the documentation build succeeds.

Note: This comment is automatically posted by the Documentation Publishing GitHub Action.

@NordicBuilder
Copy link
Contributor

NordicBuilder commented Jul 5, 2023

The following west manifest projects have been modified in this Pull Request:

Name Old Revision New Revision Diff
zephyr nrfconnect/sdk-zephyr@582837b (main) nrfconnect/sdk-zephyr#1227 nrfconnect/sdk-zephyr#1227/files

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@Vge0rge Vge0rge force-pushed the oberon_kconfigs branch 4 times, most recently from 45eec76 to 9df6d43 Compare July 5, 2023 13:11
@Vge0rge Vge0rge force-pushed the oberon_kconfigs branch 2 times, most recently from 2890801 to a954331 Compare July 7, 2023 12:27
@Vge0rge Vge0rge requested a review from rlubos as a code owner July 7, 2023 12:27
doc/nrf/libraries/nrf_security/doc/driver_config.rst Outdated Show resolved Hide resolved
doc/nrf/libraries/nrf_security/doc/driver_config.rst Outdated Show resolved Hide resolved

The option :kconfig:option:`CONFIG_PSA_USE_CC3XX_SIGNATURE_DRIVER` enables the driver :ref:`nrf_security_drivers_cc3xx` for the RSA PKCS#1 v1.5 signing algorithm.

The option :kconfig:option:`CONFIG_PSA_USE_CC3XX_ASYMMETRIC_DRIVER` enables the driver :ref:`nrf_security_drivers_cc3xx` for RSA PKCS#1 v1.5 and RSA OAEP encryption.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The option :kconfig:option:`CONFIG_PSA_USE_CC3XX_ASYMMETRIC_DRIVER` enables the driver :ref:`nrf_security_drivers_cc3xx` for RSA PKCS#1 v1.5 and RSA OAEP encryption.
The option :kconfig:option:`CONFIG_PSA_USE_CC3XX_ASYMMETRIC_DRIVER` enables the driver :ref:`nrf_security_drivers_cc3xx` for RSA PKCS#1 v1.5 and RSA OAEP encryption.

+-----------------------+--------------------------------------------------------------------------+--------------------------------------------------------------------------+
| RSA PSS | Not supported | :kconfig:option:`CONFIG_PSA_CRYPTO_DRIVER_ALG_RSA_PSS_OBERON` |
+-----------------------+--------------------------------------------------------------------------+--------------------------------------------------------------------------+
Configuration of the :ref:`nrf_security_drivers_oberon` driver is automatically generated based on the user enabled algorithms in `RSA configurations`_.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Configuration of the :ref:`nrf_security_drivers_oberon` driver is automatically generated based on the user enabled algorithms in `RSA configurations`_.
Configuration of the :ref:`nrf_security_drivers_oberon` driver is automatically generated based on the user-enabled algorithms in `RSA configurations`_.

Copy link
Contributor

Choose a reason for hiding this comment

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

search replaced user enabled with user-enabled in the file.

| RIPEMD-160 | Not supported | Not supported |
+-----------------------+---------------------------------------------------------------+---------------------------------------------------------------+

You can use the following table to check the Hash algorithm support of each driver:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
You can use the following table to check the Hash algorithm support of each driver:
Use the following table to check the Hash algorithm support of each driver:

Please apply this change to other instances of this, where we changed 'you can use the following configurations' to 'you can check the table'.

Copy link
Contributor

Choose a reason for hiding this comment

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

Search-replaced "You can use" with "Use" in the file


The option :kconfig:option:`CONFIG_PSA_USE_CC3XX_HASH_DRIVER` enables the driver :ref:`nrf_security_drivers_cc3xx` for all the supported algorithms.

The driver configuration of the :ref:`nrf_security_drivers_oberon` driver is automatically generated based on the user enabled algorithms in `HASH configurations`_.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The driver configuration of the :ref:`nrf_security_drivers_oberon` driver is automatically generated based on the user enabled algorithms in `HASH configurations`_.
The configuration of the :ref:`nrf_security_drivers_oberon` driver is automatically generated based on the user enabled algorithms in `HASH configurations`_.

Copy link
Contributor

Choose a reason for hiding this comment

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

Search-replaced "The driver configuration" with "The configuration" in the file

| SRP | Not supported | Supported |
+-----------------------+--------------------------+---------------------------+

Configuration of the :ref:`nrf_security_drivers_oberon` driver is automatically generated based on the user enabled algorithms in `Password-authenticated key agreement configurations`_.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Configuration of the :ref:`nrf_security_drivers_oberon` driver is automatically generated based on the user enabled algorithms in `Password-authenticated key agreement configurations`_.
Configuration of the :ref:`nrf_security_drivers_oberon` driver is automatically generated based on the user-enabled algorithms in `Password-authenticated key agreement configurations`_.

@melwee01
Copy link
Contributor

melwee01 commented Aug 2, 2023

Other notes:

Notes are rendering on the page with bullet points:
image
I couldn't find other instances of this formatting in clicking around other config sections in library docs.

Adding: Bartek's opinion, which I think is reasonable, is that bullet points are fine where there are two or more points per note, but we could take them out for notes where there is only one bullet point.

+-----------------------+---------------------------------------------------------------------+----------------------------------------------------------------------+
| Stream cipher | :kconfig:option:`CONFIG_PSA_CRYPTO_DRIVER_ALG_STREAM_CIPHER_CC3XX` | :kconfig:option:`CONFIG_PSA_CRYPTO_DRIVER_ALG_STREAM_CIPHER_OBERON` |
+-----------------------+---------------------------------------------------------------------+----------------------------------------------------------------------+
Cipher algorithm support for each driver:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Cipher algorithm support for each driver:
The following table shows cipher algorithm support for each driver:

Copy link
Contributor

Choose a reason for hiding this comment

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

While I personally prefer your more straightforward way of introducing tables, it seems that 'the following table shows' is a standard stylistic choice throughout NCS. Can you apply these throughout, or would you like me to add a suggestion for each?

@@ -272,17 +300,24 @@ The ECC algorithm support is dependent on one or more Kconfig options enabling c
ECC driver configurations
=========================

You can use the following Kconfig options for fine-grained control over which drivers provide ECC support:
Use the following table to check the ECC algorithm support of each driver:
Copy link
Contributor

Choose a reason for hiding this comment

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

Think we should follow the same form for these as well. 'The following table shows ECC algorithim support of each driver'...

+--------------------+-----------------------------------------------------+

.. note::
* All RSA key size configurations are introduced by :ref:`nrf_security` and are not described by the PSA Crypto specification.
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't remember if it was in reference to this or another PR, but I checked this formatting with Bartek. He said that notes should have bullet points only if there are multiple items in the list.

Please remove bullet points for notes with only one item.

@Vge0rge Vge0rge force-pushed the oberon_kconfigs branch 6 times, most recently from c453f78 to 282dfa1 Compare September 6, 2023 18:45
Vge0rge and others added 3 commits September 7, 2023 09:31
The Oberon core uses a configuration scheme where the user
chooses the PSA_WANT and the PSA_USE options and the core itself
derives the PSA_NEED configurations which are used internally.

This change adapts this new configuration scheme in nrf_security.

With this change the user still have the option to explicitely
enable/disable drivers with the previous options:
PSA_CRYPTO_DRIVER_OBERON
PSA_CRYPTO_DRIVER_CC3XX

Signed-off-by: Georgios Vasilakis <georgios.vasilakis@nordicsemi.no>
Update the documentation files for the NRF security
configurations of the PSA APIs to follow the
Oberon PSA configuration scheme.

Signed-off-by: Georgios Vasilakis <georgios.vasilakis@nordicsemi.no>
Update the psa_tls sample to follow the new configuration scheme
from Oberon PSA core.

Signed-off-by: Georgios Vasilakis <georgios.vasilakis@nordicsemi.no>
Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
Copy link
Contributor

@frkv frkv left a comment

Choose a reason for hiding this comment

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

LGTM

@doublemis1
Copy link
Contributor

Matter/Chip plans failed on nRF7002 while connecting to the wifi:

I: 18108 [DL]Wi-Fi scan done (0)
E: ***** BUS FAULT *****
E:   Precise data bus error
E:   BFAR Address: 0x26902bb
E: r0/a1:  0x026902bb  r1/a2:  0x000e44a2  r2/a3:  0x000000a0
E: r3/a4:  0x00000077 r12/ip:  0x00000000 r14/lr:  0x00099c2f
E:  xpsr:  0x21100000
E: s[ 0]:  0x00000000  s[ 1]:  0x00000000  s[ 2]:  0x00000000  s[ 3]:  0x00000000
E: s[ 4]:  0x00000000  s[ 5]:  0x00000000  s[ 6]:  0x00000000  s[ 7]:  0x00000000
E: s[ 8]:  0x00000000  s[ 9]:  0x00000000  s[10]:  0x00000000  s[11]:  0x00000000
E: s[12]:  0x00000000  s[13]:  0x00000000  s[14]:  0x00000000  s[15]:  0x00000000
E: fpscr:  0x20018b24
E: Faulting instruction address (r15/pc): 0x0000f648
E: >>> ZEPHYR FATAL ERROR 25: Unknown error on CPU 0
E: Current thread: 0x2000e160 (CHIP)
E: Halting system

@Vge0rge
Copy link
Contributor Author

Vge0rge commented Sep 7, 2023

Matter/Chip plans failed on nRF7002 while connecting to the wifi:

I: 18108 [DL]Wi-Fi scan done (0)
E: ***** BUS FAULT *****
E:   Precise data bus error
E:   BFAR Address: 0x26902bb
E: r0/a1:  0x026902bb  r1/a2:  0x000e44a2  r2/a3:  0x000000a0
E: r3/a4:  0x00000077 r12/ip:  0x00000000 r14/lr:  0x00099c2f
E:  xpsr:  0x21100000
E: s[ 0]:  0x00000000  s[ 1]:  0x00000000  s[ 2]:  0x00000000  s[ 3]:  0x00000000
E: s[ 4]:  0x00000000  s[ 5]:  0x00000000  s[ 6]:  0x00000000  s[ 7]:  0x00000000
E: s[ 8]:  0x00000000  s[ 9]:  0x00000000  s[10]:  0x00000000  s[11]:  0x00000000
E: s[12]:  0x00000000  s[13]:  0x00000000  s[14]:  0x00000000  s[15]:  0x00000000
E: fpscr:  0x20018b24
E: Faulting instruction address (r15/pc): 0x0000f648
E: >>> ZEPHYR FATAL ERROR 25: Unknown error on CPU 0
E: Current thread: 0x2000e160 (CHIP)
E: Halting system

Two runs ago CHIP was green. (Check run 90). It seems to me like a CI issue CHIP/MATTER.

@doublemis1
Copy link
Contributor

Matter/Chip plans failed on nRF7002 while connecting to the wifi:

I: 18108 [DL]Wi-Fi scan done (0)
E: ***** BUS FAULT *****
E:   Precise data bus error
E:   BFAR Address: 0x26902bb
E: r0/a1:  0x026902bb  r1/a2:  0x000e44a2  r2/a3:  0x000000a0
E: r3/a4:  0x00000077 r12/ip:  0x00000000 r14/lr:  0x00099c2f
E:  xpsr:  0x21100000
E: s[ 0]:  0x00000000  s[ 1]:  0x00000000  s[ 2]:  0x00000000  s[ 3]:  0x00000000
E: s[ 4]:  0x00000000  s[ 5]:  0x00000000  s[ 6]:  0x00000000  s[ 7]:  0x00000000
E: s[ 8]:  0x00000000  s[ 9]:  0x00000000  s[10]:  0x00000000  s[11]:  0x00000000
E: s[12]:  0x00000000  s[13]:  0x00000000  s[14]:  0x00000000  s[15]:  0x00000000
E: fpscr:  0x20018b24
E: Faulting instruction address (r15/pc): 0x0000f648
E: >>> ZEPHYR FATAL ERROR 25: Unknown error on CPU 0
E: Current thread: 0x2000e160 (CHIP)
E: Halting system

Two runs ago CHIP was green. (Check run 90). It seems to me like a CI issue CHIP/MATTER.

Thanks for info

@Vge0rge Vge0rge removed the DNM label Sep 7, 2023
@nordicjm nordicjm merged commit bff4553 into nrfconnect:main Sep 7, 2023
17 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. doc-required PR must not be merged without tech writer approval. manifest-nrfxlib manifest-zephyr
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants