Do not add error codes #167
Conversation
Let the low-level error code win because that is usually the right choice, e.g. to propagate out-of-memory errors through semantic errors, or to propagate an ASN.1 parsing error through certificate verification. We mostly use MBEDTLS_ERROR_ADD with both a low-level and a high-level error, but we also occasionally use it with a low-level code that can be 0. Ensure that the value is nonzero whenever either value is nonzero. Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
gilles-peskine-arm
added
size-s
Signed-off-by: Gilles Peskine <[email protected]>
gilles-peskine-arm
force-pushed
the
error-codes-add-force-low-crypto
branch
from
6d1c757 to
0799704
Compare
gilles-peskine-arm
added
needs-review
Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
| /** This is a bug in the library */ | ||
| #define MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED -0x006E | ||
| /* Generic error */ | ||
| #define MBEDTLS_ERR_ERROR_GENERIC_ERROR PSA_ERROR_GENERIC_ERROR |
There was a problem hiding this comment.
Should we have a changelog entry to call out:
- when a specific
MBEDTLS_ERR_xxxconstant becomes private? - when a specific
MBEDTLS_ERR_xxxconstant remains public, but becomes an alias of another value?
There was a problem hiding this comment.
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.
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.
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.
I've gone ahead and created a directory for migration document fragments, and written a fragment about error codes.
Signed-off-by: Gilles Peskine <[email protected]>
Signed-off-by: Gilles Peskine <[email protected]>
Never add a low-level error code to a high-level error code. Just use the low-level error code. Resolves Mbed-TLS/mbedtls#9619, a step towards having unified PSA and mbedtls error codes.
In this pull request, we keep calls to
MBEDTLS_ERROR_ADD. The idea is to keep them around for a while, but not to use addition in new code. ThusMBEDTLS_ERROR_ADD(MBEDTLS_ERR_HIGH, xxx)wherexxxis never 0.Follow-up to Mbed-TLS/mbedtls#9566, which added enforcement that we do use
MBEDTLS_ERROR_ADDwhen constructing high+low error codes, and not just directly the plus operator.PR checklist