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 noup] tls: Use MBEDTLS_PK_USE_PSA_EC_DATA #33

Open
wants to merge 95 commits into
base: main
Choose a base branch
from

Conversation

frkv
Copy link
Contributor

@frkv frkv commented Mar 8, 2024

-This is a noup since Mbed TLS still favors using legacy types
for ECC even though PSA Crypto is enabled. Please see pk.h
search for the define MBEDTLS_PK_USE_PSA_EC_DATA

ref: NCSDK-26412

Signed-off-by: Frank Audun Kvamtrø frank.kvamtro@nordicsemi.no

frkv and others added 30 commits May 26, 2023 13:08
-To remove unnecessary build warnings for define being redefined
 we need to undefine the MBEDTLS_PSA_CRYPTO_CLIENT in the
 crypto_types header file

Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
(cherry picked from commit 63b9b11)
(cherry picked from commit e28066d)
-This removes the redefinition of the define
 PSA_VENDOR_ECC_MAX_CURVE_BITS which we will set
 in our configuration file.

 Ref: NCSDK-12898

Signed-off-by: Georgios Vasilakis <georgios.vasilakis@nordicsemi.no>
Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
(cherry picked from commit 1292885)
(cherry picked from commit 79f38c8)
-The ECP_ALT file sets this variable so we
  will have a multiple definition if we enable
  it in ecp.h.

Signed-off-by: Georgios Vasilakis <georgios.vasilakis@nordicsemi.no>
Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
(cherry picked from commit 11023cd)
(cherry picked from commit 6c70849)
-This checks if GCM_C is enabled in gcm.h
 before including the functions. This was
 causing build issues when the GCM is disabled
 but GCM_ALT is enabled.

Signed-off-by: Georgios Vasilakis <georgios.vasilakis@nordicsemi.no>
Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
(cherry picked from commit 08e9148)
(cherry picked from commit 125633d)
-This adds a psa_driver_wrapper call for the PSA
 psa_key_derivation_key_agreement API.
 Without this patch the psa_key_derivation APIs
 will never reach the PSA drivers (Oberon, CC3XX)
 and they will only support the software
 implementation. This patch is a noupp because
 we expect the Mbed TLS project to add this
 change in a later release.

Ref: NCSDK-13564

Signed-off-by: Georgios Vasilakis <georgios.vasilakis@nordicsemi.no>
Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
(cherry picked from commit 7e0b0ec)
(cherry picked from commit afeb7c3)
-Enable use of SNI without x509 by testing for
 MBEDTLS_SSL_SERVER_NAME_INDICATION

ref: NCSDK-15193

Signed-off-by: Pete Skeggs <peter.skeggs@nordicsemi.no>
Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
(cherry picked from commit a80889e)
(cherry picked from commit 4bf3986)
-Enable more TLS/DTLS types being auto-generated in documentation
 in Mbed TLS.

Note that these are not in use in nRF Connect SDK documentation
generation at the moment, this commit currently has no effect

ref: NCSDK-15193

This one conflicted because PREDEFINED was removed in the doxyfile.
Check if this commit can be dropped.
Conflict resolution is to bring back the old defines.

Signed-off-by: Pete Skeggs <peter.skeggs@nordicsemi.no>
Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
(cherry picked from commit b4e0e5c)
(cherry picked from commit 6ef9f19)
-Change from PSA_ALG_ECDSA_ANY to a version that provides
 the correct conversion of md_alg => psa_md_alg

ref: NCSDK-14199

Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
(cherry picked from commit c3884bd)
(cherry picked from commit 3ff651a)
-Disabling this prevents in-field devices from returning errors
 when non ECJPAKE PSK is used for OpenThread devices.

ref: NCSDK-14629

Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
(cherry picked from commit 231620d)
(cherry picked from commit 14c0a29)
-The current version is HMAC-oriented and assumes the key type has
 PSA_KEY_USAGE_SIGN_HASH and PSA_KEY_USAGE_VERIFY_HASH, while
 PSA_KEY_USAGE_SIGN_MESSAGE and PSA_KEY_USAGE_VERIFY_MESSAGE
 is more apt for CMAC.

ref: NCSDK-14656

Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
(cherry picked from commit 8c1dff4)
(cherry picked from commit 7a26f05)
-Since the CC3XX driver can call Oberon to do the
 hashing it needs to be aware of the context size
 of the Oberon operation. This adds this size in
 the build.
-Setting the alignment for the oberon type as it is using
 uint64_t as the first tipe and hence will be 8 byte aligned.

ref: NCSDK-13860
ref: NCSDK-13857

Signed-off-by: Georgios Vasilakis <georgios.vasilakis@nordicsemi.no>
Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
(cherry picked from commit bec3de0)
(cherry picked from commit e422c8d)
-The runtime library expects key-bits to be set when it is not
 for cipher and ECDSA, this is fixed here. This may be an issue
 either in Mbed TLS or in nrf_cc3xx v0.9.14. Hence setting as a
 noup

ref: NCSDK-13857

Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
(cherry picked from commit 67cb08f)
(cherry picked from commit 1707e93)
This makes sure that the content of the mutex
inside the mbedtls_entropy_context is zeroed.
This is a workaround because the CryptoCell
runtime library will generate a fault if
the mutex is not zeroed. This workaround
will be reverted later when NCSDK-17004
is fixed. There is no reason to upstream this
since it is a limitation in our CryptoCell
runtime library and not an upstream limitation.

Ref: NCSDK-8075

Signed-off-by: Georgios Vasilakis <georgios.vasilakis@nordicsemi.no>
Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
(cherry picked from commit 73337db)
(cherry picked from commit 10d43b5)
-There is an inconsistency between PSA Crypto API specification in
 Mbed TLS and in the interface exposed by TF-M for key representation
 where an additional type has been added to hold information about owner.
 This functionality is controlled by setting the configuration
 MBEDTLS_PSA_CRYPTO_KEY_ID_ENCODES_OWNER which configures the type
 Mbed TLS internal type mbedtls_svc_key_id_t to a structure type of two
 words and not as a single word compatible with the PSA Crypto API
 type psk_key_id_t.

 This commit adds a reserved word in psa_core_key_attributes_t after
 the instance of mbedtls_svc_key_id_t to ensure that this structure
 is binary compatible with PSA Crypto drivers that are built with
 MBEDTLS_PSA_CRYPTO_KEY_ID_ENCODES_OWNER.

 This is a [noup] commit as this problem for our pre-built PSA
 crypto drivers which is required to be compiled with the configuration
 MBEDTLS_PSA_CRYPTO_KEY_ID_ENCODES_OWNER enabled to ensure support
 with and without TF-M using the same library.

ref: NCSDK-17464

Signed-off-by: Markus Swarowsky <markus.swarowsky@nordicsemi.no>
Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
(cherry picked from commit 3f5ebf6)
(cherry picked from commit 1b0e9ea)
Fix RSA (MBEDTLS_RSA_C) dependency on PK write (MBEDTLS_PK_WRITE_C)
when enabling PSA Crypto (MBEDTLS_PSA_CRYPTO_C).
This should instead depend on MBedTLS using PSA crypto
(MBEDTLS_USE_PSA_CRYPTO).

Raised as issue to MbedTLS project:
Mbed-TLS/mbedtls#7126

Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
(cherry picked from commit 65311cf)
(cherry picked from commit 702f824)
…derivation_abort

Fix compilation error in psa_key_derivation_abort when
MBEDTLS_PSA_BUILTIN_ALG_TLS12_PRF is defined, but
MBEDTLS_PSA_BUILTIN_ALG_TLS12_PSK_TO_MS is not defined.

In function 'psa_key_derivation_abort':
error: 'psa_tls12_prf_key_derivation_t'
{aka 'struct psa_tls12_prf_key_derivation_s'} has no member named
'other_secret'

Upstream PR:
Mbed-TLS/mbedtls#7125

Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
(cherry picked from commit 87acf5b)
(cherry picked from commit adea381)
…out builtin keys

Fix psa_key_derivation_output_key not being able to derive ECC keys
without MBEDTLS_BUILTIN ECC key types enabled.
The PSA crypto drivers can generate these keys without requiring the
builtin key types.

Upstream PR:
Mbed-TLS/mbedtls#7192

Signed-off-by: Joakim Andersson <joakim.andersson@nordicsemi.no>
(cherry picked from commit de1b3f5)
(cherry picked from commit 5881d82)
`mbedtls_ssl_set_hs_ecjpake_password()` sets psa roles as client / server.

PSA crypto API doesn’t allow setting roles `pake_set_role()` for ECJPAKE.
Although it allows setting user and peer ID with `psa_pake_set_user()`
and `psa_pake_set_peer()`.

The issue is already documented in:
ARM-software/psa-api#45 and
Mbed-TLS/mbedtls#6961,
but in mbedtls 3.3.0 it blocks OpenThread’s TLS/DTLS using PSA crypto API.

This commit adds necessary workaround for mbedtls 3.3.0
It additionally fixes status checking after `psa_pake_set_password_key()`.

This is a noup commit because the upstream fix has too many conflicts,
This change should be reverted when updating to version 3.4.0 or newer.

ref: NCSDK-23631

Signed-off-by: Maciej Baczmanski <maciej.baczmanski@nordicsemi.no>
Recreated from: commit c8df898
With the following information:

"Fix a buffer overflow in TLS 1.2 ClientKeyExchange parsing. When
MBEDTLS_USE_PSA_CRYPTO is enabled, the length of the public key in an ECDH
or ECDHE key exchange was not validated. This could result in an overflow
of handshake->xxdh_psa_peerkey, overwriting further data in the handshake
structure or further on the heap."

This commit is "noup" since Mbed TLS TLS/DTLS has gone through refactoring
meaning its content had to be recreated.

Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
Recreated from commit 12c5aaa
which provides the following information:

"Fix a buffer overflow in TLS 1.3 ServerHello and ClientHello parsing. The
length of the public key in an ECDH- or FFDH-based key exchange was not
validated. This could result in an overflow of handshake->xxdh_psa_peerkey,
overwriting further data in the handshake structure or further on the
heap."

This commit is "noup" since it TLS/DTLS is undergoing refactoring and
the content of the commit had to be recreated.

Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
Recreated from commit faf0b86
which provides the following information

"With stream ciphers, add a check that there's enough room to read a MAC
in the record. Without this check, subtracting the MAC length from the
data length resulted in an integer underflow, causing the MAC calculation
to try reading (SIZE_MAX + 1 - maclen) bytes of input, which is a buffer
overread."

This commit is a "noup" since TLS/DTLS is undergoing refactoring and
the content of the commit had to be recreated.

Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
This reverts commit acea48f.

Signed-off-by: Markus Swarowsky <markus.swarowsky@nordicsemi.no>
…sing"

This reverts commit 4a204f2.

Signed-off-by: Markus Swarowsky <markus.swarowsky@nordicsemi.no>
…arsing"

This reverts commit 2d81092.

Signed-off-by: Markus Swarowsky <markus.swarowsky@nordicsemi.no>
…OpenThread"

This reverts commit b573773.

Signed-off-by: Markus Swarowsky <markus.swarowsky@nordicsemi.no>
…ECC without builtin keys"

This reverts commit 829e3ed.

Signed-off-by: Markus Swarowsky <markus.swarowsky@nordicsemi.no>
…psa_key_derivation_abort"

This reverts commit a58396e.

Signed-off-by: Markus Swarowsky <markus.swarowsky@nordicsemi.no>
…rypto"

This reverts commit 45374c0.

Signed-off-by: Markus Swarowsky <markus.swarowsky@nordicsemi.no>
This reverts commit 7322ffa.

Signed-off-by: Markus Swarowsky <markus.swarowsky@nordicsemi.no>
This reverts commit c56a2ae.

Signed-off-by: Markus Swarowsky <markus.swarowsky@nordicsemi.no>
jwinzig-at-hilscher and others added 27 commits January 29, 2024 15:13
….data

Signed-off-by: Jonathan Winzig <jwinzig@hilscher.com>
(cherry picked from commit 93f5240)
Signed-off-by: Markus Swarowsky <markus.swarowsky@nordicsemi.no>
Co-authored-by: David Horstmann <david.horstmann@arm.com>
Signed-off-by: Jonathan Winzig <jwinzig@hilscher.com>
(cherry picked from commit 144bfde)
Signed-off-by: Markus Swarowsky <markus.swarowsky@nordicsemi.no>
Signed-off-by: Jonathan Winzig <jwinzig@hilscher.com>
(cherry picked from commit acd35a5)
Signed-off-by: Markus Swarowsky <markus.swarowsky@nordicsemi.no>
Co-authored-by: Paul Elliott <62069445+paul-elliott-arm@users.noreply.github.com>
Signed-off-by: Jonathan Winzig <jwinzig@hilscher.com>
(cherry picked from commit af553bf)
Signed-off-by: Markus Swarowsky <markus.swarowsky@nordicsemi.no>
Signed-off-by: Paul Elliott <paul.elliott@arm.com>
(cherry picked from commit 968a928)
Signed-off-by: Markus Swarowsky <markus.swarowsky@nordicsemi.no>
Signed-off-by: Janos Follath <janos.follath@arm.com>
(cherry picked from commit d6b0965)
Signed-off-by: Markus Swarowsky <markus.swarowsky@nordicsemi.no>
Signed-off-by: Janos Follath <janos.follath@arm.com>
(cherry picked from commit 6bcbc92)
Signed-off-by: Markus Swarowsky <markus.swarowsky@nordicsemi.no>
Signed-off-by: Janos Follath <janos.follath@arm.com>
(cherry picked from commit a865fc9)
Signed-off-by: Markus Swarowsky <markus.swarowsky@nordicsemi.no>
Signed-off-by: Janos Follath <janos.follath@arm.com>
(cherry picked from commit 100dcdd)
Signed-off-by: Markus Swarowsky <markus.swarowsky@nordicsemi.no>
Signed-off-by: Janos Follath <janos.follath@arm.com>
(cherry picked from commit a62a554)
Signed-off-by: Markus Swarowsky <markus.swarowsky@nordicsemi.no>
Signed-off-by: Janos Follath <janos.follath@arm.com>
(cherry picked from commit e6750b2)
Signed-off-by: Markus Swarowsky <markus.swarowsky@nordicsemi.no>
Signed-off-by: Janos Follath <janos.follath@arm.com>
(cherry picked from commit 47ee770)
Signed-off-by: Markus Swarowsky <markus.swarowsky@nordicsemi.no>
Signed-off-by: Janos Follath <janos.follath@arm.com>
(cherry picked from commit b4b8f3d)
Signed-off-by: Markus Swarowsky <markus.swarowsky@nordicsemi.no>
Signed-off-by: Janos Follath <janos.follath@arm.com>
(cherry picked from commit 16ab76b)
Signed-off-by: Markus Swarowsky <markus.swarowsky@nordicsemi.no>
Any timing variance dependant on the output of this function enables a
Bleichenbacher attack. It is extremely difficult to use safely.

In the Marvin attack paper
(https://people.redhat.com/~hkario/marvin/marvin-attack-paper.pdf) the
author suggests that implementations of PKCS 1.5 decryption that don't
include a countermeasure should be considered inherently dangerous.

They suggest that all libraries implement the same countermeasure, as
implementing different countermeasures across libraries enables the
Bleichenbacher attack as well.

This is extremely fragile and therefore we don't implement it. The use
of PKCS 1.5 in Mbed TLS implements the countermeasures recommended in
the TLS standard (7.4.7.1 of RFC 5246) and is not vulnerable.

Add a warning to PKCS 1.5 decryption to warn users about this.

Signed-off-by: Janos Follath <janos.follath@arm.com>
(cherry picked from commit 393df9c)
Signed-off-by: Markus Swarowsky <markus.swarowsky@nordicsemi.no>
Upon further consideration we think that a remote attacker close to the
victim might be able to have precise enough timing information to
exploit the side channel as well. Update the Changelog to reflect this.

Signed-off-by: Janos Follath <janos.follath@arm.com>
(cherry picked from commit 0d57f10)
Signed-off-by: Markus Swarowsky <markus.swarowsky@nordicsemi.no>
Signed-off-by: Dave Rodgman <dave.rodgman@arm.com>
(cherry picked from commit 6ba4169)
Signed-off-by: Markus Swarowsky <markus.swarowsky@nordicsemi.no>
./scripts/bump_version.sh --version 3.5.1

Signed-off-by: Dave Rodgman <dave.rodgman@arm.com>
(cherry picked from commit e23d647)
Signed-off-by: Markus Swarowsky <markus.swarowsky@nordicsemi.no>
Signed-off-by: Dave Rodgman <dave.rodgman@arm.com>
(cherry picked from commit daca7a3)
Signed-off-by: Markus Swarowsky <markus.swarowsky@nordicsemi.no>
…CDSA_SIGN_ALT

When ECDSA_SIGN_ALT but not ECDSA_VERIFY_ALT, mbedtls_ecdsa_can_do was not being defined causing mbedtls_ecdsa_verify_restartable to always fail

Signed-off-by: JonathanWitthoeft <jonw@gridconnect.com>
Replace MBEDTLS_ACCEL symbols with the equivalent
PSA_WANT symbols since we don't use the MBEDTLS_ACCEL
symbols in sdk-nrf.

Signed-off-by: Georgios Vasilakis <georgios.vasilakis@nordicsemi.no>
The CryptoCell runtime libraries use the
mbedtls_md APIs for hashing. Since the CryptoCell
libraries are binaries and the mbedtls_md APIs are
compiled from source at build time, they need to have
the same view of the mbedtls_md context.

This change makes the mbedtls_md contexts to have
fixed sizes.

It also makes the context sizes static for AES, cipher and the
md_wrap contexts.

Signed-off-by: Georgios Vasilakis <georgios.vasilakis@nordicsemi.no>
The Oberon PSA core has some replacement headers but it also
uses headers from the library folder. Since the headers and the
C files in this folder perform inclusion with double quotes this
can create issues since the Oberon PSA core may use headrs from
this repo that it shouldn't.

This moves the headers that the Oberon PSA core replaces with
to another folder so that they don't accidentaly included from the
Oberon PSA core.

Signed-off-by: Georgios Vasilakis <georgios.vasilakis@nordicsemi.no>
This is temporary fix until: NCSDK-26077 is fixed

The given hash algorithm for an ecdsa verify operation was just omitted
by setting the algorithm for psa_verify_hash to PSA_ALG_ECDSA_ANY.
As the PSA spec states:
This is the same signature scheme as PSA_ALG_ECDSA(), but without
specifying a hash algorithm, and skipping the message hashing operation.

This algorithm is only recommended to sign or verify a sequence of bytes
that are an already-calculated hash. Note that the input is padded with
zeros on the left or truncated on the right as required to fit the curve
size.

So the input should be hashed but thats not the case for
ecdsa_verify_psa therefore changing it to PSA_ALG_ECDSA(hash_alg)

Upstream PR: Mbed-TLS/mbedtls#8834

Signed-off-by: Markus Swarowsky <markus.swarowsky@nordicsemi.no>
This is a temporary noup as the mbed TLS PSA core hasn't adapted the
final PSA PAKE APIS from the 1.2 spec.
Once that is done this can be removed.

Check the signature of psa_pake_setup and if psa_pake_get_implicit_key
is removed and replaced with psa_pake_get_shared_key

Signed-off-by: Markus Swarowsky <markus.swarowsky@nordicsemi.no>
-This is a noup since Mbed TLS still favors using legacy types
 for ECC even though PSA Crypto is enabled. Please see pk.h
 search for the define MBEDTLS_PK_USE_PSA_EC_DATA

ref: NCSDK-26412

Signed-off-by: Frank Audun Kvamtrø <frank.kvamtro@nordicsemi.no>
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.