Skip to content

[nrf fromlist] Fix ECDSA secp521r1 padding issue in oberon driver#34

Open
frkv wants to merge 29 commits intonrfconnect:mainfrom
frkv:secp521r1_padding_fix
Open

[nrf fromlist] Fix ECDSA secp521r1 padding issue in oberon driver#34
frkv wants to merge 29 commits intonrfconnect:mainfrom
frkv:secp521r1_padding_fix

Conversation

@frkv
Copy link
Contributor

@frkv frkv commented Nov 17, 2025

-This is a [nrf fromlist] as this is based on a patch we have
received from Oberon ahead of introducing this in a coming
version of the Oberon PSA crypto deliverable.
-There was an issue found in test on deterministic ECDSA using NIST
secp521r1 curve type for the Oberon PSA crypto driver. The issue
was caused by an arithmetic issue with the buffer used to handle
padding of ECDSA sign and verify operations. The buffer length is
set according to the key size (modulo size) of the ECC curve
and when secp521r1 is enabled (then this buffer is 66 bytes long).
The padding is used when digests are smaller than the buffer and
since secp521r1 uses SHA-512 (normally, 64 bytes long digest) the
system handled this as a normal padding. ocrypto_ APIs expects 64
bytes input, and was handed a digest with invalid bytes.
-Signing: The API to do signature generation uses hash + 2 to get the
right offset for the digest
-Verification: The API has special handling for secp521r1 to get the
proper offset and uses hash + 2 for the right offset for the digest

ref: NCSDK-36329

frkv and others added 29 commits July 27, 2025 23:10
@frkv @Vge0rge
[nrf noup] Remove duplicated legacy Mbed TLS header files
a565151 
-Remove legacy header files from oberon-psa-crypto that are duplicates
 of files from standard Mbed TLS distribution (modules/crypto/)

Signed-off-by: Sebastian Bøe <sebastian.boe@nordicsemi.no>
Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
@Vge0rge
[nrf toup] library: Add _Static_assert in common.h
a1ae168 
GCC supports _Static_assert from version 4.6 as a builtin feature
even when you compile with std=99.

Use this _Static_assert for the implementation of
MBEDTLS_STATIC_ASSERT if there is no defined static_assert.

I opened a PR in the MbedTLS PSA core to include that:
Mbed-TLS/TF-PSA-Crypto#190

Hopefully they will accept that and then we can remove
this commit in the next release.

Signed-off-by: Georgios Vasilakis <georgios.vasilakis@nordicsemi.no>
@Vge0rge
[nrf noup] Remove redefinition ECC_MAX_CURVE_BITS
64f7610 
-This removes the redefinition of the define
 PSA_VENDOR_ECC_MAX_CURVE_BITS which we allow to
 be set in our configuration file from nrf security.

Signed-off-by: Georgios Vasilakis <georgios.vasilakis@nordicsemi.no>
@Vge0rge
[nrf noup] Add missing brainpool key sizes
9d56fa9 
The core doesn't include some brainpool key
sizes, this change adds these bit sizes:
160
192
224
320

Signed-off-by: Georgios Vasilakis <georgios.vasilakis@nordicsemi.no>
@Vge0rge
[nrf noup] Remove oberon_config.h inclusion
8200edd 
We don't use the file in our system, we use the
generated headers (nrf-config.h and nrf-config-user.h)
from nrf_security.

Signed-off-by: Georgios Vasilakis <georgios.vasilakis@nordicsemi.no>
@SebastianBoe @Vge0rge
[nrf noup] Turn the repo into a Zephyr module
846f2fe 
Turn the repo into a Zephyr module so we can use the
ZEPHYR_bla_bla_MODULE_DIR CMake variable.

Signed-off-by: Sebastian Bøe <sebastian.boe@nordicsemi.no>
@vlilleboe @Vge0rge
[nrf toup] Support builtin keys with CMAC KDF
0f3b46c 
Patch to enable more key types for KDF.

Signed-off-by: Vidar Lillebø <vidar.lillebo@nordicsemi.no>
Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
@mswarowsky @Vge0rge
[nrf noup] keys: Add plausibility checks for ECC keys
6fff796 
Add checks for the key attributes, key sizes and a zero check
for ecc keys for importing keys.

Also a attribute check for psa_generate_key.

This was [nrf fromlist] and is now [nrf noup]

Upstream PR:oberon-psa-crypto-nrf/pull/15 (closed)

Ref: NCSDK-24820

Signed-off-by: Markus Swarowsky <markus.swarowsky@nordicsemi.no>
Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
@Vge0rge
[nrf noup] library: psa_crypto_storage.c error
d0898f6 
The psa_crypto_storage_store retuns DATA_INVALID for
all the error codes coming from psa_its_set.
This causes some issues for our testing infanstructure
since it doesn't follow the PSA spec recomendation
for return codes.

This was a [nrf fromlist] but it has become a [nrf noup]

Upstream PR: pull/14 (Closed)

Signed-off-by: Georgios Vasilakis <georgios.vasilakis@nordicsemi.no>
Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
@stephen-nordic @Vge0rge
[nrf noup] testspec: Add test spec to run crypto and TF-M tests
ccb3a47 
Run crypto and TF-M tests on sdk-nrf downstream

Signed-off-by: Stephen Kingston <stephen.kingston@nordicsemi.no>
Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
@vlilleboe @Vge0rge
[nrf noup] Adjust range for builtin keys
f83e081 
Opens the full range of vendor keys to be used via builtin key
mechanism.

This change makes the version of the Oberon PSA core 1.3.4 incompatible
with the volatile keys when MBEDTLS_PSA_STATIC_KEY_SLOTS is NOT enabled.

Since we decided to set the starting range of the builtin keys to
be the first section of the vendor key id range (PSA_KEY_ID_VENDOR_*)
these now overlaped with the volatile keys. So to fix this we
move the volatile key ids after the builtin key ids.

Signed-off-by: Vidar Lillebø <vidar.lillebo@nordicsemi.no>
Signed-off-by: Georgios Vasilakis <georgios.vasilakis@nordicsemi.no>
@vlilleboe @Vge0rge
[nrf noup] Allow import and destroy of builtin keys
4bbc0e1 
Adds the capability of import and destroying persistent keys
that are "builtin" i.e. handled fully by PSA driver implementation.

Signed-off-by: Vidar Lillebø <vidar.lillebo@nordicsemi.no>
Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
@vlilleboe @Vge0rge
[nrf noup] Don't ignore error code
69028a8 
Signed-off-by: Vidar Lillebø <vidar.lillebo@nordicsemi.no>
Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
@frkv @Vge0rge
[nrf noup] PSA key attribute ABI compliance
6210742 
Patch to make server and client attributes compatible.
Oberon will support this in the future.

Reworked from commit 5bfe42c
Reworded due to psa_core_key_attributes_t being removed in Mbed TLS 3.6.0

The psa_core_key_attributes_t declaration is now integrated into
psa_key_attributes_s and the macro PSA_CORE_KEY_ATTRIBUTES_INIT
has been replaced with PSA_KEY_ATTRIBUTES_INIT.

Signed-off-by: Vidar Lillebø <vidar.lillebo@nordicsemi.no>
Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
@frkv @Vge0rge
[nrf noup] psa: Using simpler initialization for operation structs
34b2895 
-This simplifies PSA crypto operation struct initialization to
 better tailor to the SSF use-case. This simplifies the initializers
 to only be empty brackets, assuming zero-initialized structs
 without the added complexities of writing every sub-type out.
 This is done to prevent compilation warnings for PSA crypto builds.

Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
@krish2718 @Vge0rge
[nrf noup] Do not use generic header names
5a19781 
Zephyr has a global list of include directories that is applicable for
all modules unless specifically excluded by using "targets", and using a
generic named header can cause issues if there are duplicate headers
files and depending on the inclusion order.

So, rename the header with module prefix to avoid naming collision.

Signed-off-by: Chaitanya Tata <Chaitanya.Tata@nordicsemi.no>
@PFnord @Vge0rge
[nrf noup] added skip in ecdsa verify if eddsaph
323a133
@Vge0rge
[nrf noup] ed448 key size 456 bits
1251218 
RFC8032 mentions the length of a key for ed448 and ed448ph to be 57 bytes (456 bits).

Using 448 bits as a max length creates a buffer overflow warning when building with ED448 enabled.

Signed-off-by: Georgios Vasilakis <georgios.vasilakis@nordicsemi.no>
@Vge0rge
[nrf noup] Update unsupported algs header
57bea3f 
This updates the oberon_check_unsupported.h
file:

1) The inclusion of the crypto_driver_config.h is removed
   since we don't use that with nrf_security
2) All the PSA_ACCEL and PSA_WANT symbols are prefixed
   with CONFIG_ so that we can use them directly from
   the Zephyr autconf.

Signed-off-by: Georgios Vasilakis <georgios.vasilakis@nordicsemi.no>
@Vge0rge
[nrf noup] Add mbedtls config headers build_info.h
04490a4 
The Oberon PSA v1.3.4 removed the inclusion of the MbedTLS headers
which do adjustments when building the SSL library in build_info.h

Add these headers back since they are required when nrf_security
is build with PSA and SSL enabled.

Signed-off-by: Georgios Vasilakis <georgios.vasilakis@nordicsemi.no>
@tomi-font @Vge0rge
[nrf toup] fix unused variable warning
2127b5f 
Fix an unused variable warning that happens when building with the LLVM
toolchain.
Adding the `unused` attribute to the variable to mark it as potentially
unused is the simplest as it's used in different `#ifdef` scenarios.

This should be communicated to Oberon and fixed upstream.

Signed-off-by: Tomi Fontanilles <tomi.fontanilles@nordicsemi.no>
@tomi-font @Vge0rge
[nrf toup] fix unused function warning
975ee63 
Fix unused function warning for ecdsa_sign_hash() by guarding it with
the same conditions as those under which the function is used.

This is communicated with Oberon and accepted by them. So this
should be dropped in the next update.

Signed-off-by: Tomi Fontanilles <tomi.fontanilles@nordicsemi.no>
@DatGizmo @tomi-font
[nrf noup] ci: Adding yamllint aciton
93c0e6e 
* The intention is to catch yaml files that are not parsable
* [yamllint action with all the rules definition](https://github.com/nrfconnect/action-yamllint/blob/main/action.yml)

Signed-off-by: Sebastian Wezel <sebastian.wezel@nordicsemi.no>
@DatGizmo @tomi-font
[nrf noup] ci: fix yaml files
bd4fd82 
* Fix yaml files based on yamllint

Signed-off-by: Sebastian Wezel <sebastian.wezel@nordicsemi.no>
@Vge0rge @tomi-font
[nrf toup] drivers: Align RSA OAEP with the Cracen driver
d682b30 
Oberon was informed and will follow this methodology in the
next release.

Signed-off-by: Georgios Vasilakis <georgios.vasilakis@nordicsemi.no>
@AntonZma @tomi-font
[nrf noup] library: fix psa_destroy_key
180fa1b 
psa_destroy_persistent_key() improperly handled built-in keys.
This resulted in ITS call attempted to delete the key that had never
been created by it.
Conditions for key creation and destruction are now aligned.

This is a fixup for the following commit:
[nrf noup] Allow import and destroy of builtin keys

Signed-off-by: Anton Zyma <anton.zyma@nordicsemi.no>
@tomi-font
[nrf toup] don't try to find volatile/builtin keys from wrong sources
c79b47b 
When not finding a given key in memory, the implementation would try
to find it from the persistent keys regardless of the actual key type
(volatile/builtin/persistent).

Don't try to find inexistent volatile/builtin keys from persistent
ones.

In addition to the calls being superflous, the problem that was
happening here is that the ITS implementation (Secure Storage
subsystem) returns `PSA_ERROR_INVALID_ARGUMENT` because the ID is not
in the persistent key range, and because it doesn't return
`PSA_ERROR_DOES_NOT_EXIST` then the wrong error code is propagated
back to the caller.

`toup` as the issue has been communicated to Mbed TLS and should be
fixed there.
See:
- Mbed-TLS/TF-PSA-Crypto#488
- Mbed-TLS/TF-PSA-Crypto#492

Signed-off-by: Tomi Fontanilles <tomi.fontanilles@nordicsemi.no>
@AntonZma @tomi-font
[nrf noup] drivers: key_derivation: Fix ifdefs for WPA3-SAE
ee0cdb0 
Fixes build issues due to the missing checks for
PSA_NEED_OBERON_WPA3_SAE_H2E macro.

This change is already taken in and will be
a part of the future version of Oberon PSA Crypto.

Signed-off-by: Anton Zyma <anton.zyma@nordicsemi.no>
@frkv
[nrf fromlist] Fix ECDSA secp521r1 padding issue in oberon driver
4e6a8b9 
-This is a [nrf fromlist] as this is based on a patch we have
 received from Oberon ahead of introducing this in a coming
 version of the Oberon PSA crypto deliverable.
-There was an issue found in test on deterministic ECDSA using NIST
 secp521r1 curve type for the Oberon PSA crypto driver. The issue
 was caused by an arithmetic issue with the buffer used to handle
 padding of ECDSA sign and verify operations. The buffer length is
 set according to the key size (modulo size) of the ECC curve
 and when secp521r1 is enabled (then this buffer is 66 bytes long).
 The padding is used when digests are smaller than the buffer and
 since secp521r1 uses SHA-512 (normally, 64 bytes long digest) the
 system handled this as a normal padding. ocrypto_ APIs expects 64
 bytes input, and was handed a digest with invalid bytes.
-Signing: The API to do signature generation uses hash + 2 to get the
 right offset for the digest
-Verification: The API has special handling for secp521r1 to get the
 proper offset and uses hash + 2 for the right offset for the digest

ref: NCSDK-36329

Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
@NordicBuilder NordicBuilder mentioned this pull request Nov 20, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Vge0rge Vge0rge Vge0rge approved these changes

@tomi-font tomi-font tomi-font approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.