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

Update nrfxlib to be compatible with jan.5 Zephyr #613

Merged
merged 19 commits into from
Jan 26, 2022

Conversation

SebastianBoe
Copy link
Contributor

@SebastianBoe SebastianBoe commented Jan 3, 2022

Changes required for compatibility with new Zephyr revision.

See commit messages for details

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

@SebastianBoe SebastianBoe changed the title crypto: Port to new power management API Update nrfxlib to be compatible with jan.5 Zephyr Jan 4, 2022
Copy link
Contributor

@tejlmand tejlmand left a comment

Choose a reason for hiding this comment

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

lgtm.

minor nit noticed.

# warning.
zephyr_library_compile_options(-Wno-address-of-packed-member)


Copy link
Contributor

Choose a reason for hiding this comment

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

minor nit: why the extra empty line ?

Suggested change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Accidental.

Would prefer to not do any git paperwork for this, but will fix if I have to touch the code any way.

@SebastianBoe SebastianBoe force-pushed the zephyr_jan_5_upmerge branch 4 times, most recently from b52b1d1 to c330404 Compare January 17, 2022 12:24
@lmaciejonczyk
Copy link
Contributor

lmaciejonczyk commented Jan 17, 2022

From OpenThread perspective, what have changed and makes the build failure is:

  • missing MBEDTLS_SSL_EXPORT_KEYS in nrf-config.h
  • now CONFIG_MBEDTLS_SSL_PROTO_DTLS=y needs to be explicitly enabled
  • MBEDTLS_ENTROPY_C is disabled by default and cannot be y enabled because it's a promptless option

@SebastianBoe
Copy link
Contributor Author

@lmaciejonczyk : I believe MBEDTLS_SSL_EXPORT_KEYS has been fixed.

See 7142b6e#diff-5381c07ad9e799f8bbf6967323abc7876f592fd24f9dab1f93822531f7b0136bR1618

@frkv
Copy link
Contributor

frkv commented Jan 18, 2022

I would set the CONFIG_MBEDTLS_SSL_PROTO_DTLS either with a defconfig or by using the openthread security profile. Not everybody is using DTLS and it is uses more RAM and FLASH.

I don't know why you needed to set MBEDTLS_ENTROPY_C specifically. It is on a list of configurations that most likely will change due to PSA APIs becoming the standard one. If you are building with TF-M then the configuration is not applicable at all...

@lmaciejonczyk
Copy link
Contributor

@lmaciejonczyk : I believe MBEDTLS_SSL_EXPORT_KEYS has been fixed.

See 7142b6e#diff-5381c07ad9e799f8bbf6967323abc7876f592fd24f9dab1f93822531f7b0136bR1618

I believe we are still missing:
kconfig_check_and_set_base(MBEDTLS_SSL_EXPORT_KEYS) in legacy_crypto_config.cmake

@SebastianBoe
Copy link
Contributor Author

@lmaciejonczyk :

I believe we are still missing:
kconfig_check_and_set_base(MBEDTLS_SSL_EXPORT_KEYS) in legacy_crypto_config.cmake

I've now pushed this patch, good catch.

@lmaciejonczyk
Copy link
Contributor

lmaciejonczyk commented Jan 18, 2022

I don't know why you needed to set MBEDTLS_ENTROPY_C specifically. It is on a list of configurations that most likely will change due to PSA APIs becoming the standard one. If you are building with TF-M then the configuration is not applicable at all...

OpenThread currently uses MBEDTLS_ENTROPY_C related functions (mbedtls_entropy_init, mbedtls_entropy_func) for initializing random number generator.

@frkv
Copy link
Contributor

frkv commented Jan 18, 2022

I don't know why you needed to set MBEDTLS_ENTROPY_C specifically. It is on a list of configurations that most likely will change due to PSA APIs becoming the standard one. If you are building with TF-M then the configuration is not applicable at all...

OpenThread currently uses MBEDTLS_ENTROPY_C related functions (mbedtls_entropy_init, mbedtls_entropy_func) for initializing random number generator.

This topic will be handled in the meeting at 14.00

@b-gent b-gent self-requested a review January 19, 2022 09:44
crypto/nrf_oberon/include/ocrypto_aes_ccm.h Outdated Show resolved Hide resolved
crypto/nrf_oberon/include/ocrypto_aes_ccm.h Outdated Show resolved Hide resolved
crypto/nrf_oberon/include/ocrypto_curve_p224.h Outdated Show resolved Hide resolved
crypto/nrf_oberon/include/ocrypto_ecdh_p256.h Outdated Show resolved Hide resolved
crypto/nrf_oberon/include/ocrypto_rsa.h Outdated Show resolved Hide resolved
@SebastianBoe SebastianBoe force-pushed the zephyr_jan_5_upmerge branch 3 times, most recently from e05cdab to 768d0a3 Compare January 24, 2022 09:05
@SebastianBoe SebastianBoe force-pushed the zephyr_jan_5_upmerge branch 2 times, most recently from 2efd76f to b144eb4 Compare January 25, 2022 11:08
power/reboot.h has been moved to sys/reboot.h.

Signed-off-by: Sebastian Bøe <sebastian.boe@nordicsemi.no>
SebastianBoe and others added 18 commits January 26, 2022 11:32
nrfxlib/zboss/src/zcl/zcl_s_wwah.c and others are producing the
warning -Waddress-of-packed-member. e.g.

650 | (req)->cluster_id = (zb_uint16_t*)(&(src_ptr->cluster_id));
                                        ~^~~~~~~~~~~~~~~~~~~~~~~

It is suspected that this should be fixed in the source code. Until it
is we temporarily suppress the warning.

Signed-off-by: Sebastian Bøe <sebastian.boe@nordicsemi.no>
-Adding library nrf_oberon v3.0.9
-Updating some APR.rst grouping

NOTE: Not setting to use the new version as it is built against mbed
TLS 3.0.0 which is incompatible for a some APIs

Ref: NCSDK-13189

Signed-off-by: Georgios Vasilakis <georgios.vasilakis@nordicsemi.no>
Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
-Added as single commit for easier readability, not made use of yet
-Adding faux nrf_platform_cc3xx driver as well

ref: NCSDK-11689

Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
-Added as single commit for easier readability, not made use of yet

ref: NCSDK-11689

Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
-Added as single commit for easier readability, not made use of yet
-Note that builtin driver is a "construct". Builtin in mbed TLS
 project is defacto SW implementation that is given as-is and is
 handled like a "fallback" and part of the core support.

ref: NCSDK-11689

Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
-Added as single commit for easier readability
-Not put in use yet

ref: NCSDK-11689

Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
-Not in use yet, added as single commit for readability
-This generates configurations for PSA APIs
-Added CMake logic to convert Kconfigs to defines and to generate
 MBEDTLS_CONFIG_FILE and MBEDTLS_USER_CONFIG_FILE
-Added template file psa_crypto-config.h.template that is filled
 using #cmakedefine

ref: NCSDK-11689

Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
-Not in use yet, added as single commit for readability
-This generates configurations for legacy mbed TLS APIs
-Added CMake logic to convert Kconfigs to defines and to generate
 MBEDTLS_CONFIG_FILE and MBEDTLS_USER_CONFIG_FILE
-Added template file legacy_crypto_config.h.template that is filled
 using #cmakedefine

ref: NCSDK-11689

Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
-Not in use yet, added for better readability
-Adds functionality to pass configurations to TF-M build

ref: NCSDK-11689

Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
-Not in use yet. Added in single commit for readability
-Adds Kconfig.legacy used for legacy mbed TLS configurations
-Adds Kconfig.tls used for TLS/DTLS configurations

ref: NCSDK-11689

Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
-Adds replacement of psa_crypto_driver_wrappers.c from mbed TLS
-This implementation supports nrf_cc3xx and nrf_oberon
-This file may be moved into sdk-mbedtls in due time

ref: NCSDK-11689

Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
Signed-off-by: Georgios Vasilakis <georgios.vasilakis@nordicsemi.no>
-Implements mbedtls_psa_external_get_random which is used
 instead of using ctr_drbg.c or hmac_drbg.c
-This uses nrf_cc3xx_platform_ctr_drbg API to generate random numbers

ref: NCSDK-11689

Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
Signed-off-by: Georgios Vasilakis <georgios.vasilakis@nordicsemi.no>
-Adding support for mbed TLS 3.0.0 which includes
   -Some updated APIs in legacy mbed TLS
   -Adds the concept of PSA Crypto Drivers which we have implemented
    for nrf_cc3xx and nrf_oberon
-Removed old "glue"-functionality. There is no longer any symbol
 manipulation and recombinations in libraries.
-Kconfig refactoring:
   -Adding NRF_SECURITY config for PSA usage while NRF_SECURITY_BACKEND
    is prevent older code breaking.
   -Setting NORDIC_SECURITY_BACKEND enables legacy mbed TLS APIs
   -Removing glue/backend concepts
   -Refactoring PSA into Kconfig.psa
   -Make use of Kconfig.tls and Kconfig.legacy
-Removal of macros in extensions:
   -Remove all functions used for stripping and gluing
-General refactoring
-Moving source-files to legacy folder

ref: NCSDK-11689

Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
-As its own commit

Ref: NCSDK-13189

Signed-off-by: Georgios Vasilakis <georgios.vasilakis@nordicsemi.no>
Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
-Removed functionality to extract, rename and combine libraries
 used by the old glue-functionality
-No longer needed

ref: NCSDK-11689

Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
-Usable only in mbed TLS 2.x

ref: NCSDK-11689

Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
fix docs

Signed-off-by: Sebastian Bøe <sebastian.boe@nordicsemi.no>
Add workaround for liboberon PSA API prefix usage. When mbedcrypto
has been compiled with a prefix for the PSA APIs.
When building with TF-M the PSA APIs needs to be replaced with a prefix
in order to separate it from the TF-M frontend versions of these APIs.

Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
int
range 0 16384
default 900 if OPENTHREAD_NRF_SECURITY
default 16380

Choose a reason for hiding this comment

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

This constant (16380) leads to mbedtls not working after this fix nrfconnect/sdk-mbedtls@668b31f

(unless of course server sends less)

Glad this config and wrong constant is removed altogether now.

But with SDK-NRF 2.4.2 it's not working...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DNM Do not merge doc-required PR must not be merged without tech writer approval.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants