gh-Mbed-TLS/TF-PSA-Crypto
Watch
1
0
Fork
You've already forked TF-PSA-Crypto
 0
Code Issues 167 Pull requests 92 Releases 5 Activity

Finalize pk.h #468

Merged
mpg merged 31 commits from finalize-pk.h into development 2025-09-26 11:38:15 -05:00
Conversation 22 Commits 31 Files changed 7 +211 -219
mpg commented 2025-09-23 04:37:40 -05:00 (Migrated from github.com)

Description

Resolve #347 and resolve #205

Depends on: https://github.com/Mbed-TLS/mbedtls/pull/10418 for a clean CI (sigh).

PR checklist

  • changelog provided
  • framework PR not required
  • mbedtls development PR not required because: crypto only
  • mbedtls 3.6 PR not required because: 1.0 only
  • tests not required because: docs only
## Description Resolve #347 and resolve #205 **Depends on:** https://github.com/Mbed-TLS/mbedtls/pull/10418 for a clean CI (sigh). ## PR checklist - [x] **changelog** provided - [x] **framework PR** not required - [x] **mbedtls development PR** not required because: crypto only - [x] **mbedtls 3.6 PR** not required because: 1.0 only - **tests** not required because: docs only
gilles-peskine-arm (Migrated from github.com) requested changes 2025-09-24 08:35:36 -05:00
gilles-peskine-arm (Migrated from github.com) left a comment

I have reviewed for correctness (with the loss of distinction between v1.5 and v2.1 RSA keys that we discussed on Slack and that will be implemented in https://github.com/Mbed-TLS/TF-PSA-Crypto/pull/471).

I haven't reviewed for completeness yet, I'll do another pass for that but it may have to wait until tomorrow.

I have reviewed for correctness (with the loss of distinction between v1.5 and v2.1 RSA keys that we discussed on Slack and that will be implemented in https://github.com/Mbed-TLS/TF-PSA-Crypto/pull/471). I haven't reviewed for completeness yet, I'll do another pass for that but it may have to wait until tomorrow.
include/mbedtls/pk.h
gilles-peskine-arm (Migrated from github.com) commented 2025-09-24 08:05:33 -05:00

the permitted algorithm is the same as the original PSA key.

The thing is, a PSA key does not in general have “the permitted algorithm”. It could be the same permitted algorithm policy (including the enrollment algorithm), or it could be the PSA key's primary algorithm (which is what it was in 3.6, and AFAIK this hasn't changed).

> the permitted algorithm is the same as the original PSA key. The thing is, a PSA key does not in general have “the permitted algorithm”. It could be the same permitted algorithm _policy_ (including the enrollment algorithm), or it could be the PSA key's primary algorithm (which is what it was in 3.6, and AFAIK this hasn't changed).
include/mbedtls/pk.h
@ -450,0 +420,4 @@
* For keys created with \c mbedtls_pk_wrap_psa(), the
* primary algorithm is the same as the original PSA key.
* Otherwise, it is determined as follows:
* - For RSA keys:
gilles-peskine-arm (Migrated from github.com) commented 2025-09-24 08:02:19 -05:00

Is verification supported with ECDSA? I can't find that in the test code.

Is verification supported with ECDSA? I can't find that in the test code.
include/mbedtls/pk.h
gilles-peskine-arm (Migrated from github.com) commented 2025-09-24 08:09:09 -05:00

Not necessarily a key pair, it can be a public key.

Not necessarily a key pair, it can be a public key.
include/mbedtls/pk.h
@ -578,3 +548,3 @@
/**
* \brief Verify signature (including padding if relevant).
* \brief Verify signature.
*
gilles-peskine-arm (Migrated from github.com) commented 2025-09-24 08:14:51 -05:00

Given that this function is only useful for ECDSA, and there is only one ECDSA verification algorithm, I think this whole note is useless. For RSA, there's already a sentence above that says it's the same as mbedtls_pk_verify.

Given that this function is only useful for ECDSA, and there is only one ECDSA verification algorithm, I think this whole note is useless. For RSA, there's already a sentence above that says it's the same as `mbedtls_pk_verify`.
include/mbedtls/pk.h
@ -606,20 +578,20 @@ int mbedtls_pk_verify(mbedtls_pk_context *ctx, mbedtls_md_type_t md_alg,
*
* \note Performs the same job as \c mbedtls_pk_verify(), but can
gilles-peskine-arm (Migrated from github.com) commented 2025-09-24 08:16:41 -05:00

“includes verification of the padding” is an obsolete concept from back when textbook RSA was considered a cryptographic algorithm rather than a low-level building block. Please remove this clause while you're at it.

“includes verification of the padding” is an obsolete concept from back when textbook RSA was considered a cryptographic algorithm rather than a low-level building block. Please remove this clause while you're at it.
include/mbedtls/pk.h
@ -665,0 +631,4 @@
* \note The signature algorithm used will be the one that would be
* selected by \c mbedtls_pk_get_psa_attributes() called with a
* usage of #PSA_KEY_USAGE_SIGN_HASH - see that function's
* documentation for details.
gilles-peskine-arm (Migrated from github.com) commented 2025-09-24 08:28:49 -05:00

Given that wrapped PSA keys aren't supported (which should be mentioned in this function's description, btw), this simplifies to “this function always uses MBEDTLS_PK_ALG_ECDSA” (plus being equivalent to mbedtls_pk_sign for RSA keys as stated just above).

Given that wrapped PSA keys aren't supported (which should be mentioned in this function's description, btw), this simplifies to “this function always uses `MBEDTLS_PK_ALG_ECDSA`” (plus being equivalent to `mbedtls_pk_sign` for RSA keys as stated just above).
include/mbedtls/pk.h
@ -793,9 +764,8 @@ int mbedtls_pk_check_pair(const mbedtls_pk_context *pub,
* \param pwdlen Size of the password in bytes.
gilles-peskine-arm (Migrated from github.com) commented 2025-09-24 08:11:19 -05:00

This information is still relevant. And for RSA keys, we should state that PKCS#1v1.5 signature (PSA_ALG_RSA_PKCS1V15_SIGN(hash_alg) or PSA_ALG_RSA_PKCS1V15_SIGN_RAW) will be used.

I do think this should be mentioned explicitly, because it can be somewhat surprising: you don't necessarily end up with the same algorithm with mbedtls_pk_wrap_psa.

Also applies to mbedtls_pk_copy_public_from_psa.

This information is still relevant. And for RSA keys, we should state that PKCS#1v1.5 signature (`PSA_ALG_RSA_PKCS1V15_SIGN(hash_alg)` or `PSA_ALG_RSA_PKCS1V15_SIGN_RAW`) will be used. I do think this should be mentioned explicitly, because it can be somewhat surprising: you don't necessarily end up with the same algorithm with `mbedtls_pk_wrap_psa`. Also applies to `mbedtls_pk_copy_public_from_psa`.
valeriosetti (Migrated from github.com) reviewed 2025-09-24 16:14:19 -05:00
include/mbedtls/pk.h
@ -450,0 +420,4 @@
* For keys created with \c mbedtls_pk_wrap_psa(), the
* primary algorithm is the same as the original PSA key.
* Otherwise, it is determined as follows:
* - For RSA keys:
valeriosetti (Migrated from github.com) commented 2025-09-24 16:14:19 -05:00

Yes, IMO it's supported and I think it's also tested

Yes, IMO it's [supported](https://github.com/Mbed-TLS/TF-PSA-Crypto/blob/779296b0fb7165d7f36f436ca34085afff48c02c/drivers/builtin/src/pk_wrap.c#L715) and I think it's [also tested](https://github.com/Mbed-TLS/TF-PSA-Crypto/blob/779296b0fb7165d7f36f436ca34085afff48c02c/tests/suites/test_suite_pk.function#L1440)
valeriosetti (Migrated from github.com) requested changes 2025-09-24 16:39:30 -05:00
include/mbedtls/pk.h
@ -537,1 +497,3 @@
* otherwise PKCS1 v1.5 is set.
*
* \note This function only copies the key material but discards
* policy information entirely. See \c
valeriosetti (Migrated from github.com) commented 2025-09-24 16:27:14 -05:00 
 *
```suggestion * ```
include/mbedtls/pk.h
valeriosetti (Migrated from github.com) commented 2025-09-24 16:34:20 -05:00

I'm not sure this is consistent with what will be mbedtls_pk_get_psa_attributes after https://github.com/Mbed-TLS/TF-PSA-Crypto/pull/471. For example for an RSA key mbedtls_pk_get_psa_attributes will always return PKCS v1.5 algorithm, whereas rsa_verify_wrap picks the proper padding mode depending on pk->rsa_padding.
Will this be updated after https://github.com/Mbed-TLS/TF-PSA-Crypto/pull/471?

I'm not sure this is consistent with what will be `mbedtls_pk_get_psa_attributes` after https://github.com/Mbed-TLS/TF-PSA-Crypto/pull/471. For example for an RSA key `mbedtls_pk_get_psa_attributes` will always return PKCS v1.5 algorithm, whereas `rsa_verify_wrap` picks the proper padding mode depending on `pk->rsa_padding`. Will this be updated after https://github.com/Mbed-TLS/TF-PSA-Crypto/pull/471?
mpg (Migrated from github.com) reviewed 2025-09-25 02:22:52 -05:00
include/mbedtls/pk.h
@ -450,0 +420,4 @@
* For keys created with \c mbedtls_pk_wrap_psa(), the
* primary algorithm is the same as the original PSA key.
* Otherwise, it is determined as follows:
* - For RSA keys:
mpg (Migrated from github.com) commented 2025-09-25 02:22:52 -05:00

I had not checked tests and has just looked at the info structure fro which pointers are not NULL. Thank you Valerio for locating tests.

I had not checked tests and has just looked at the info structure fro which pointers are not NULL. Thank you Valerio for locating tests.
mpg (Migrated from github.com) reviewed 2025-09-25 02:27:16 -05:00
include/mbedtls/pk.h
@ -793,9 +764,8 @@ int mbedtls_pk_check_pair(const mbedtls_pk_context *pub,
* \param pwdlen Size of the password in bytes.
mpg (Migrated from github.com) commented 2025-09-25 02:27:16 -05:00

This information is now centralized in the documentation of mbedtls_pk_get_psa_attributes(), in the "else" part of: "if the key was create with wrap_psa(), else ...." I guess I should add a pointer here?

The thing is, there is not public concept of an algorithm associated with a key, as we don't say how the key will be stored. So, the only observables is what get_psa_attributes() will return, and what pk_sign() and pk_verify() will do, which is now defined to be the same.

This information is now centralized in the documentation of `mbedtls_pk_get_psa_attributes()`, in the "else" part of: "if the key was create with `wrap_psa()`, else ...." I guess I should add a pointer here? The thing is, there is not public concept of an algorithm associated with a key, as we don't say how the key will be stored. So, the only observables is what `get_psa_attributes()` will return, and what `pk_sign()` and `pk_verify()` will do, which is now defined to be the same.
mpg (Migrated from github.com) reviewed 2025-09-25 02:28:25 -05:00
include/mbedtls/pk.h
@ -578,3 +548,3 @@
/**
* \brief Verify signature (including padding if relevant).
* \brief Verify signature.
*
mpg (Migrated from github.com) commented 2025-09-25 02:28:25 -05:00

Ah, right, I did the same as for sign but did not thing that there is no distinction when verifying. Thanks!

Ah, right, I did the same as for sign but did not thing that there is no distinction when verifying. Thanks!
mpg (Migrated from github.com) reviewed 2025-09-25 02:28:58 -05:00
include/mbedtls/pk.h
@ -606,20 +578,20 @@ int mbedtls_pk_verify(mbedtls_pk_context *ctx, mbedtls_md_type_t md_alg,
*
* \note Performs the same job as \c mbedtls_pk_verify(), but can
mpg (Migrated from github.com) commented 2025-09-25 02:28:58 -05:00

Fully agreed. I kept that out of habit but I don't like it either.

Fully agreed. I kept that out of habit but I don't like it either.
mpg (Migrated from github.com) reviewed 2025-09-25 02:42:44 -05:00
include/mbedtls/pk.h
mpg (Migrated from github.com) commented 2025-09-25 02:42:44 -05:00

whereas rsa_verify_wrap picks the proper padding mode depending on pk->rsa_padding.

Unless I'm missing something, the value of pk->rsa_padding will always be v1.5 at this point because:

  • that's the default set by init()
  • after 471 none of the 5 public function for setting up a key modify it
  • there is no public method for modifying it.

Specifically, in 471, mbedtls_pk_set_rsa_(padding|hash)() are used only in tests, not in the library (and not in mbedtls).

In fact, I think we should remove that field (and rsa_hash) entirely. I didn't initially suggest doing that in 471 in order to keep it minimal, but that's something I'd like to do right after it. But perhaps it makes more sense to do it in 471 in order to be more confident about the fact that they're not used? (The same reason why I removed pk_context::pk_ctx here.) Wdyt?

As Gilles said previously, a guiding principle of PK for 1.0 is that it doesn't do policy - that's PSA's job. According to this principle, we shouldn't have those fields in pk_context. (Again, it makes sense that you introduced them in the initial refactoring of how RSA keys are held, because it's better to not change existing behaviour when refactoring, but I think there should have been a 2nd step where we change the behaviour to simplify and align with the new principles.)

> whereas `rsa_verify_wrap` picks the proper padding mode depending on `pk->rsa_padding`. Unless I'm missing something, the value of `pk->rsa_padding` will always be v1.5 at this point because: - that's the default set by `init()` - after 471 none of the 5 public function for setting up a key modify it - there is no public method for modifying it. Specifically, in 471, `mbedtls_pk_set_rsa_(padding|hash)()` are used only in tests, not in the library (and not in mbedtls). In fact, I think we should remove that field (and `rsa_hash`) entirely. I didn't initially suggest doing that in 471 in order to keep it minimal, but that's something I'd like to do right after it. But perhaps it makes more sense to do it in 471 in order to be more confident about the fact that they're not used? (The same reason why I removed `pk_context::pk_ctx` here.) Wdyt? As Gilles said previously, a guiding principle of PK for 1.0 is that it doesn't do policy - that's PSA's job. According to this principle, we shouldn't have those fields in `pk_context`. (Again, it makes sense that you introduced them in the initial refactoring of how RSA keys are held, because it's better to not change existing behaviour when refactoring, but I think there should have been a 2nd step where we change the behaviour to simplify and align with the new principles.)
valeriosetti (Migrated from github.com) reviewed 2025-09-25 02:54:26 -05:00
include/mbedtls/pk.h
valeriosetti (Migrated from github.com) commented 2025-09-25 02:54:25 -05:00

Unless I'm missing something, the value of pk->rsa_padding will always be v1.5 at this point because:

Oh, you are perfectly right, sorry.

In fact, I think we should remove that field (and rsa_hash) entirely. I didn't initially suggest doing that in 471 in order to keep it minimal, but that's something I'd like to do right after it. But perhaps it makes more sense to do it in 471 in order to be more confident about the fact that they're not used?

Well, I can open a follow-up PR so that we can both have 471 merged soon (if it's OK) and then keep other changes in the follow-up

> Unless I'm missing something, the value of pk->rsa_padding will always be v1.5 at this point because: Oh, you are perfectly right, sorry. > In fact, I think we should remove that field (and rsa_hash) entirely. I didn't initially suggest doing that in 471 in order to keep it minimal, but that's something I'd like to do right after it. But perhaps it makes more sense to do it in 471 in order to be more confident about the fact that they're not used? Well, I can open a follow-up PR so that we can both have 471 merged soon (if it's OK) and then keep other changes in the follow-up
gilles-peskine-arm (Migrated from github.com) reviewed 2025-09-25 03:56:32 -05:00
include/mbedtls/pk.h
@ -793,9 +764,8 @@ int mbedtls_pk_check_pair(const mbedtls_pk_context *pub,
* \param pwdlen Size of the password in bytes.
gilles-peskine-arm (Migrated from github.com) commented 2025-09-25 03:56:32 -05:00

there is not public concept of an algorithm associated with a key

We know that, but users don't necessarily know that. An interface like pk could reasonably have a specific algorithm associated with each key, and could reasonably treat keys as pure mathematical objects with no policy. It's not obvious that we went for the latter. It's even less obvious for users who have used previous versions of Mbed TLS where a key did have partial policy information.

This is why I think this should be mentioned explicitly in the documentation of copy_from_psa, not just by reference to the documentation of other functions. We don't necessarily have to have the specifics here, but at least we should state the general principle that this is functionally equivalent to export+import and that no policy information carries over.

> there is not public concept of an algorithm associated with a key We know that, but users don't necessarily know that. An interface like pk could reasonably have a specific algorithm associated with each key, and could reasonably treat keys as pure mathematical objects with no policy. It's not obvious that we went for the latter. It's even less obvious for users who have used previous versions of Mbed TLS where a key did have partial policy information. This is why I think this should be mentioned explicitly in the documentation of `copy_from_psa`, not just by reference to the documentation of other functions. We don't necessarily have to have the specifics here, but at least we should state the general principle that this is functionally equivalent to export+import and that no policy information carries over.
mpg (Migrated from github.com) reviewed 2025-09-25 06:00:31 -05:00
include/mbedtls/pk.h
mpg (Migrated from github.com) commented 2025-09-25 06:00:31 -05:00

Duh, it says right below "key pair or public key". What was I thinking.

Duh, it says right below "key pair or public key". What was I thinking.
valeriosetti commented 2025-09-25 07:17:45 -05:00 (Migrated from github.com)
- there still seem to be usages of `mbedtls_pk_[rsa|ec]()` that make the CI unhappy - commit https://github.com/Mbed-TLS/TF-PSA-Crypto/pull/468/commits/e832c5213d31abe37943b44d1275fec7b667eff7 lacks `signed-off-by`
👍 1
gilles-peskine-arm (Migrated from github.com) requested changes 2025-09-25 07:52:39 -05:00
gilles-peskine-arm (Migrated from github.com) left a comment

Mostly LGTM now, just a couple of things left.

I also had a high-level look at pk.h to look for other things to change, and the only things that stood out were:

Mostly LGTM now, just a couple of things left. I also had a high-level look at `pk.h` to look for other things to change, and the only things that stood out were: * The [“set up” terminology](https://github.com/Mbed-TLS/TF-PSA-Crypto/pull/468#discussion_r2378928212). * [The salt length for `mbedtls_pk_verify_ext()`](https://github.com/Mbed-TLS/TF-PSA-Crypto/pull/468/files#r2379347862).
ChangeLog.d/pk-simplify-rsa-padding.txt
@ -0,0 +1,7 @@
API changes
gilles-peskine-arm (Migrated from github.com) commented 2025-09-25 07:17:48 -05:00

Copyediting:

   * When a PK context for an RSA key is created with
     mbedtls_pk_copy_from_psa() or mbedtls_pk_copy_public_from_psa(), the
     associated padding mode will be PKCS#1 v1.5.
   * If mbedtls_pk_get_psa_attributes() is called on a PK context containing
     an RSA key, the returned algorithms is
     PSA_ALG_RSA_PKCS1V15_SIGN(PSA_ALG_ANY_HASH) for a sign/verify usage and
     PSA_ALG_RSA_PKCS1V15_CRYPT when the usage is encrypt/decrypt,
     unless the key was created with mbedtls_pk_psa_wrap().
Copyediting: ```suggestion * When a PK context for an RSA key is created with mbedtls_pk_copy_from_psa() or mbedtls_pk_copy_public_from_psa(), the associated padding mode will be PKCS#1 v1.5. * If mbedtls_pk_get_psa_attributes() is called on a PK context containing an RSA key, the returned algorithms is PSA_ALG_RSA_PKCS1V15_SIGN(PSA_ALG_ANY_HASH) for a sign/verify usage and PSA_ALG_RSA_PKCS1V15_CRYPT when the usage is encrypt/decrypt, unless the key was created with mbedtls_pk_psa_wrap(). ```
gilles-peskine-arm (Migrated from github.com) commented 2025-09-25 07:21:29 -05:00

These changes overlap a lot, and I'm not sure it makes sense to list them separately. I think it would be easier to understand with a single entry that goes something like this:

A PK context no longer associates specific algorithms with the key, except when wrapping a PSA key. In particular, after mbedtls_pk_copy_from_psa() or mbedtls_pk_copy_public_from_psa() on an RSA key, the functions mbedtls_pk_get_psa_attributes(), mbedtls_pk_sign() and mbedtls_pk_verify() will use PKCS#1v1.5 signature or encryption, regardless of the original key's policy.

These changes overlap a lot, and I'm not sure it makes sense to list them separately. I think it would be easier to understand with a single entry that goes something like this: > A PK context no longer associates specific algorithms with the key, except when wrapping a PSA key. In particular, after mbedtls_pk_copy_from_psa() or mbedtls_pk_copy_public_from_psa() on an RSA key, the functions mbedtls_pk_get_psa_attributes(), mbedtls_pk_sign() and mbedtls_pk_verify() will use PKCS#1v1.5 signature or encryption, regardless of the original key's policy.
include/mbedtls/pk.h
gilles-peskine-arm (Migrated from github.com) commented 2025-09-25 07:25:54 -05:00

I don't think “freshly initialized” is quite right, because that seems to forbid reusing a context after calling mbedtls_pk_free() on it.

I don't think “freshly initialized” is quite right, because that seems to forbid reusing a context after calling `mbedtls_pk_free()` on it.
include/mbedtls/pk.h
@ -306,3 +279,3 @@
* \param ctx The context to initialize. It must be empty (type NONE).
* \param ctx The context to populate. It must be empty.
* \param key The PSA key to wrap, which must hold an ECC or RSA key pair.
*
gilles-peskine-arm (Migrated from github.com) commented 2025-09-25 07:31:23 -05:00

We no longer have anything that's called “setting up” a context. This was already a defect in the documentation in 3.x: we were using “set up” without defining it, so a reasonable understanding was that this meant calling mbedtls_pk_setup() or mbedtls_pk_setup_xxx(). But parsing a key also counted as “set up”, even though that was neither documented as such nor obvious from the name.

I think now we should completely stop using “set up”, and instead say something like “it must have been filled with key material” or “it must contain key material” or some such.

Mind you, we don't always need to call this out explicitly. In any function that consumes a PK object as a key, it's obvious that the object must, in fact, consume a key. And if the object doesn't contain a key, there is always a well-defined error for an object that doesn't contain key material (even if we don't document exactly which error code is used).

What is important to state explicitly is that for all functions that populate a PK object, the object must be initialized, and the object must not already have material in it.

We no longer have anything that's called “setting up” a context. This was already a defect in the documentation in 3.x: we were using “set up” without defining it, so a reasonable understanding was that this meant calling `mbedtls_pk_setup()` or `mbedtls_pk_setup_xxx()`. But parsing a key also counted as “set up”, even though that was neither documented as such nor obvious from the name. I think now we should completely stop using “set up”, and instead say something like “it must have been filled with key material” or “it must contain key material” or some such. Mind you, we don't always need to call this out explicitly. In any function that consumes a PK object as a key, it's obvious that the object must, in fact, consume a key. And if the object doesn't contain a key, there is always a well-defined error for an object that doesn't contain key material (even if we don't document exactly which error code is used). What is important to state explicitly is that for all functions that populate a PK object, the object must be initialized, and the object must not already have material in it.
include/mbedtls/pk.h
gilles-peskine-arm (Migrated from github.com) commented 2025-09-25 07:10:25 -05:00

Based on https://github.com/Mbed-TLS/TF-PSA-Crypto/pull/468#discussion_r2375723065 isn't this the other way round? It works with wrapped ECC keys but not wrapped RSA keys.

Based on https://github.com/Mbed-TLS/TF-PSA-Crypto/pull/468#discussion_r2375723065 isn't this the other way round? It works with wrapped ECC keys but not wrapped RSA keys.
include/mbedtls/pk.h
@ -665,0 +631,4 @@
* \note The signature algorithm used will be the one that would be
* selected by \c mbedtls_pk_get_psa_attributes() called with a
* usage of #PSA_KEY_USAGE_SIGN_HASH - see that function's
* documentation for details.
gilles-peskine-arm (Migrated from github.com) commented 2025-09-25 07:04:34 -05:00

This is still imprecise: if there is an enrollment algorithm, is it copied or ignored?

This is still imprecise: if there is an enrollment algorithm, is it copied or ignored?
gilles-peskine-arm (Migrated from github.com) reviewed 2025-09-25 09:24:09 -05:00
include/mbedtls/pk.h
gilles-peskine-arm (Migrated from github.com) commented 2025-09-25 09:24:09 -05:00

One more thing about mbedtls_pk_verify_ext: with the removal of options, we lost the information of what this function does about the salt length. It allows any salt length.

One more thing about `mbedtls_pk_verify_ext`: with the removal of `options`, we lost the information of what this function does about the salt length. It allows any salt length.
mpg commented 2025-09-26 02:35:20 -05:00 (Migrated from github.com)
  • there still seem to be usages of mbedtls_pk_[rsa|ec]() that make the CI unhappy

Aw did a trial build, and even built Mbed TLS, but didn't build the programs...

> * there still seem to be usages of `mbedtls_pk_[rsa|ec]()` that make the CI unhappy Aw did a trial build, and even built Mbed TLS, but didn't build the programs...
mpg (Migrated from github.com) reviewed 2025-09-26 02:58:50 -05:00
include/mbedtls/pk.h
mpg (Migrated from github.com) commented 2025-09-26 02:58:50 -05:00

Aw, copy-pasta I guess, thanks!

Aw, copy-pasta I guess, thanks!
mpg (Migrated from github.com) reviewed 2025-09-26 03:25:43 -05:00
include/mbedtls/pk.h
@ -306,3 +279,3 @@
* \param ctx The context to initialize. It must be empty (type NONE).
* \param ctx The context to populate. It must be empty.
* \param key The PSA key to wrap, which must hold an ECC or RSA key pair.
*
mpg (Migrated from github.com) commented 2025-09-26 03:25:43 -05:00

Yeah, this was on my list at some point, then I removed it as perhaps not needed right now. Well, let's do it after all.

Yeah, this was on my list at some point, then I removed it as perhaps not needed right now. Well, let's do it after all.
gilles-peskine-arm (Migrated from github.com) approved these changes 2025-09-26 04:50:50 -05:00
gilles-peskine-arm (Migrated from github.com) left a comment

I'm happy with 97a417cc59 but the CI isn't.

I'm happy with 97a417cc5939f8a057921bf8e2eac45baa9a2185 but the CI isn't.
valeriosetti commented 2025-09-26 05:13:13 -05:00 (Migrated from github.com)

I'm happy with 97a417c but the CI isn't.

Yeah, the failure in build_no_std_function seems to be related to making stdlib.h always included in udp_proxy.

> I'm happy with [97a417c](https://github.com/Mbed-TLS/TF-PSA-Crypto/commit/97a417cc5939f8a057921bf8e2eac45baa9a2185) but the CI isn't. Yeah, the failure in `build_no_std_function` seems to be related to making `stdlib.h` always included in `udp_proxy`.
valeriosetti (Migrated from github.com) approved these changes 2025-09-26 05:13:27 -05:00
mpg commented 2025-09-26 05:14:10 -05:00 (Migrated from github.com)

Yes, I'm preparing an mbedtls PR to fix that. Just checking if there were other failures hidden by this one in the same component before I push.

Yes, I'm preparing an mbedtls PR to fix that. Just checking if there were other failures hidden by this one in the same component before I push.
👍 1
mpg commented 2025-09-26 05:20:04 -05:00 (Migrated from github.com)

https://github.com/Mbed-TLS/mbedtls/pull/10418

https://github.com/Mbed-TLS/mbedtls/pull/10418
Sign in to join this conversation.
Reviewers
No reviewers
gilles-peskine-arm
valeriosetti
Labels
Clear labels   
api-break
This issue/PR breaks the API and must wait for a new major version

  
approved
Design and code approved - may be waiting for CI or backports

  
bug
Something isn't working

  
component-docs
Docs / website issues filed here for tracking

  
component-platform
Portability layer and build scripts

  
component-test
Test framework and CI scripts

  
dependencies
Pull requests that update a dependency file

  
deprecation
needs deprecation announcement

  
DO-NOT-MERGE

   
enhancement
New feature or request

  
good first issue
Good for newcomers

  
help wanted
This issue is not being actively worked on, but PRs welcome.

  
needs-adoption
stalled PR that someone should pick up and complete

  
needs-backports
Backports are missing or are pending review and approval.

  
needs-ci
Needs to pass CI tests

  
needs-design-approval
Needs design discussion / approval

  
needs-info
An issue or PR which needs further info from the reporter / author

  
needs-preceding-pr
Requires another PR to be merged first

  
needs-review
Every commit must be reviewed by at least two team members

  
needs-reviewer
This PR needs someone to pick it up for review

  
needs-work

   
priority-high
High priority - will be reviewed soon

  
priority-low
Low priority - this may not receive review soon

  
priority-medium
Medium priority - this can be reviewed as time permits

  
priority-scheduled
This PR is big - it will require time to be scheduled for review

  
priority-very-high
Highest priority - prioritise this over other review work

  
resolved-after-1.1
The issue is resolved in development, but still present in 1.1 LTS.

  
single-reviewer
This PR qualifies for having only one reviewer

  
size-l
Estimated task size: large (2w+)

  
size-m
Estimated task size: medium (~1w)

  
size-optimisation

   
size-s
Estimated task size: small (~2d)

  
size-xs
Estimated task size: extra small (a few hours at most)

No labels
api-break
approved
bug
component-docs
component-platform
component-test
dependencies
deprecation
DO-NOT-MERGE
enhancement
good first issue
help wanted
needs-adoption
needs-backports
needs-ci
needs-design-approval
needs-info
needs-preceding-pr
needs-review
needs-reviewer
needs-work
priority-high
priority-low
priority-medium
priority-scheduled
priority-very-high
resolved-after-1.1
single-reviewer
size-l
size-m
size-optimisation
size-s
size-xs
Milestone
Clear milestone
No items
No milestone
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gh-Mbed-TLS/TF-PSA-Crypto!468
No description provided.