Unify error codes

[gilles-peskine-arm]

PolarSSL was designed with each module having its own error codes. So we have, for example, many different MBEDTLS_ERR_xxx_ALLOC_FAILED errors, which tells you which module had a failed allocation. This doesn't actually help in debugging, and makes error handling harder. It also increases the code size because some modules convert errors from lower-level modules into their own errors.

Error codes also have structure: an error code can have a low-level error, a high-level error or a combination of the two. Modules designated as low-level (e.g. individual symmetric algorithms, bignum) raise low-level errors. Modules designates as high-level (e.g. cipher, RSA, X.509, TLS) raise high-level errors possibly combined with a low-level error. This tends to increase the code size and doesn't really help with debugging. (It might be somewhat useful with debugging if an error code contained a detailed trace, but you'd need far more than two levels for that.) Combinations of error codes should be done with MBEDTLS_ERROR_ADD, but it's likely that there are a few places that use a plain +.

PSA has its own set of error codes, which is shared by all PSA APIs.

Minimum goals for this issue:

• Unify similar error codes across modules. For example, there should be a single error code for “memory allocation failed”.
• Use PSA error codes where applicable. For example, all failed allocations should raise PSA_ERROR_INSUFFICIENT_MEMORY.
• Where there are no applicable PSA codes, use values in the API-specific range, so that an Mbed TLS error value will never be equal to a PSA error value (even if it's a PSA error value that TF-PSA-Crypto doesn't use).
• Error codes consists of a single integer, enum-like. There are no more calls to MBEDTLS_ERROR_ADD or manual additions.

In 4.0, we may still define legacy aliases to facilitate the transition, e.g.

#define MBEDTLS_ERR_RSA_ALLOC_FAILED PSA_ERROR_INSUFFICIENT_MEMORY

Note that even with these aliases, this is an API break since compilers will no longer accept code like the following (“duplicate case value”):

switch (ret) {
    case MBEDTLS_ERR_RSA_ALLOC_FAILED:
    case MBEDTLS_ERR_ECP_ALLOC_FAILED:
        printf("out of memory");
}

Optional parts:

• Have all mbedtls_xxx functions return psa_status_t instead of int? It's formally a distinct type, but it's defined as int32_t, so it's identical to int on many implementations. Thus, changing a function from returning int to returning psa_status_t is an API change, but we don't need to do that to just return a psa_status_t value in a function that returns int (or vice versa).
• An error.c equivalent covering psa_status_t.

[mpg]

PolarSSL was designed with each module having its own error codes. So we have, for example, many different MBEDTLS_ERR_xxx_ALLOC_FAILED errors, which tells you which module had a failed allocation. This doesn't actually help in debugging, and makes error handling harder.

I think (but that's just an educated guess) that the intent was not so much helping with debugging as making each module self-contained. For a long time it was a goal that people could take just aes.c + aes.h and no other file an things would work. We've decided some time ago this was no longer something we wanted to support (in large part because it tends to result in code duplication, often with a negative impact on code size), so clearly that's no longer an obstacle to sharing/unifying error codes across modules.

[gilles-peskine-arm]

For a long time it was a goal that people could take just aes.c + aes.h and no other file an things would work

We discussed that as a goal around 2017. Code size didn't feature much in those discussions. Already by that time, it was not the largely case anymore (not even in 1.3): this had been untested for a long time and people had kept including more headers when they felt like it.

Of course it completely went out of the window when we added platform_util.c, later constant_time.c etc.

[mpg]

We discussed that as a goal around 2017.

It was an explicit goal back in the PolarSSL days. (I can remember discussions with Paul where I wanted to introduce shared things and was told "no" for this reason.) I agree that already in 2017 we were largely no longer meeting that goal when we finally made the explicit decision to drop it. All I'm saying is at the time it was still a goal, it probably influenced the decision that each low-level module had its own set of error codes. Fortunately this is no longer an obstacle.