Documentation of rsa_rsassa_pss_sign_no_mode_check() could do with a comment better defining an 'initialised context'

[paul-elliott-arm]

Summary

Coverity issue 446768 found a potential integer underflow in an extreme edge case, which in turn may result in undefined behaviour.

Basically if mbedtls_mpi_bitlen() of the public modulus (in the context) returns 0, we then subtract 1 one off this (unsigned) result, and then use it. (as of time of writing, this is line 2212/2213, rsa.c)

The only way this can happen is if the context has the hash set, but no key is imported or there was a partial import and no _complete was called. The latter is clear violation of the API and the former is nonsensical too, but complies with the letter of the documentation.

The impact is undefined behaviour, but if this doesn't crash in some way thenmbedtls_rsa_private() will error out in the end anyway. In principle we have limited parameter validation to save code size however I am not sure if we would want to add a check for this. It might, however merit updating the documentation to elaborate a bit on what we mean by initialised context.

[gilles-peskine-arm]

if mbedtls_mpi_bitlen() of the public modulus (in the context) returns 0

If mbedtls_mpi_bitlen(&ctx->N) == 0 then mbedtls_mpi_size(&ctx->N) == 0. The field ctx->len caches mbedtls_mpi_size(&ctx->N), and I don't see a way for it to be wrong unless application code fiddles with private context fields manually, at which point we can't expect anything.

If ctx->len == 0 then a check above (either olen < hlen + min_slen + 2 or saltlen + hlen + 2 > olen) causes the function to return MBEDTLS_ERR_RSA_BAD_INPUT_DATA before reaching the call to mbedtls_mpi_bitlen. So I don't see how this edge case is even reachable.