-
Notifications
You must be signed in to change notification settings - Fork 29
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
Do not add error codes #167
Merged
mpg
merged 10 commits into
Mbed-TLS:development
from
gilles-peskine-arm:error-codes-add-force-low-crypto
Feb 20, 2025
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
4a71260
Have a low-level error code override any high-level one
gilles-peskine-arm 7168431
Make it possible to include psa/crypto_values.h independently
gilles-peskine-arm 3b6557b
Remove obsolete error code layout
gilles-peskine-arm f916903
error_common.h: turn error code constants into aliases for PSA codes
gilles-peskine-arm 0799704
Changelog entry for error code space unification
gilles-peskine-arm a81889a
Don't allow psa_xxx() to return MBEDTLS_ERR_XXX
gilles-peskine-arm e08cefe
Finish sentence
gilles-peskine-arm 05e4856
Add migration guide directory
gilles-peskine-arm f6f7355
Migration guide for error codes
gilles-peskine-arm 1bbe98b
Merge remote-tracking branch 'development' into error-codes-add-force…
gilles-peskine-arm File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
API changes | ||
* The PSA and Mbed TLS error spaces are now unified. mbedtls_xxx() | ||
functions can now return PSA_ERROR_xxx values. | ||
This will not affect most applications since the error values are | ||
between -32767 and -1 as before. | ||
|
||
Removals | ||
* Remove many MBEDTLS_ERR_xxx error codes, superseded by PSA_ERROR_xxx. | ||
See the 4.0 migration guide for details. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
This directory contains sections intended for the migration guide | ||
from Mbed TLS 3.6 to TF-PSA-Crypto 1.0 + Mbed TLS 4.0. | ||
|
||
The files in this document should be markdown files with the .md | ||
extension. They should generally contain level-2 sections (## title). | ||
The files will be consolidated into a single cohesive document or | ||
document set shortly before the .0 releases. | ||
|
||
The reason to have multiple files is that each strand of work can create | ||
one file, without worrying about where to fit in the document structure. | ||
We'll worry about the general structure once we have the content. | ||
Furthermore, placing different strands of work in separate files will | ||
avoid merge conflicts (as would happen e.g. if parallel strands of | ||
work append to the same file in concurrent pull requests). | ||
|
||
Files can be placed in the docs/4.0-migration-guide indifferently in | ||
mbedtls or TF-PSA-Crypto, as convenient. | ||
|
||
Documentation that is specifically about migrating from the legacy | ||
crypto API to the PSA crypto API should be added to the existing | ||
docs/psa-transition.md instead. | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
## Error codes | ||
|
||
### Unified error code space | ||
|
||
The convention still applies that functions return 0 for success and a negative value between -32767 and -1 on error. PSA functions (`psa_xxx()` or `mbedtls_psa_xxx()`) still return a `PSA_ERROR_xxx` error codes. Non-PSA functions (`mbedtls_xxx()` excluding `mbedtls_psa_xxx()`) can return either `PSA_ERROR_xxx` or `MBEDTLS_ERR_xxx` error codes. | ||
|
||
There may be cases where an `MBEDTLS_ERR_xxx` constant has the same numerical value as a `PSA_ERROR_xxx`. In such cases, they have the same meaning: they are different names for the same error condition. | ||
|
||
### Simplified legacy error codes | ||
|
||
All values returned by a function to indicate an error now have a defined constant named `MBEDTLS_ERR_xxx` or `PSA_ERROR_xxx`. Functions no longer return the sum of a “low-level” and a “high-level” error code. | ||
|
||
Generally, functions that used to return the sum of two error codes now return the low-level code. However, as before, the exact error code returned in a given scenario can change without notice unless the condition is specifically described in the function's documentation and no other condition is applicable. | ||
|
||
As a consequence, the functions `mbedtls_low_level_sterr()` and `mbedtls_high_level_strerr()` no longer exist. | ||
|
||
### Removed error code names | ||
|
||
Many legacy error codes have been removed in favor of PSA error codes. Generally, functions that returned a legacy error code in the table below in Mbed TLS 3.6 now return the PSA error code listed on the same row. Similarly, callbacks should apply the same changes to error code, unless there has been a relevant change to the callback's interface. | ||
|
||
| Legacy constant (Mbed TLS 3.6) | PSA constant (Mbed TLS 4.0, TF-PSA-Crypto 1.0) | | ||
| ------------------------------ | ---------------------------------------------- | | ||
| `MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED` | `PSA_ERROR_CORRUPTION_DETECTED` | | ||
| `MBEDTLS_ERR_ERROR_GENERIC_ERROR` | `PSA_ERROR_GENERIC_ERROR` | | ||
| `MBEDTLS_ERR_PLATFORM_FEATURE_UNSUPPORTED` | `PSA_ERROR_NOT_SUPPORTED` | | ||
| `MBEDTLS_ERR_PLATFORM_HW_ACCEL_FAILED` | `PSA_ERROR_HARDWARE_FAILURE` | |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we have a changelog entry to call out:
MBEDTLS_ERR_xxx
constant becomes private?MBEDTLS_ERR_xxx
constant remains public, but becomes an alias of another value?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we shoud in both cases. This is a user facing error, and people will look at the changelog first when they see something not matching what they expect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed.
On the practical side, this may make the ChangeLog pretty verbose, so perhaps in the end we'll just want to have a generic entry like "Some MBEDTLS_ERR_ constants have become private, and others have become aliases to other values. For details, see in the migration guide." But in the meantime it's probably better to create a ChangeLog entry for every change we make, so we have a comprehensive list (while avoiding conflicts) and we can consolidate/edit in the late stages before the release.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I propose to start the migration document right away. Maybe multiple files that we'll merge at the end, similar to changelog.d, to avoid conflicts during the work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've gone ahead and created a directory for migration document fragments, and written a fragment about error codes.