Misleading macro names in cipher.h

[gilles-peskine-arm]

A number of identifers in cipher.h don't have CIPHER or cipher in their name, and some are misleading because they look like they would apply more generally:

• Macros MBEDTLS_MAX_IV_LENGTH, MBEDTLS_MAX_BLOCK_LENGTH, MBEDTLS_MAX_KEY_LENGTH — very misleading for “key”, somewhat misleading for “block”.
• Enum constants in mbedtls_cipher_mode_t: MBEDTLS_MODE_xxx — not really misleading because all the modes except NONE (which is 0) have a name that's a block cipher mode.
• Enum constants in mbedtls_cipher_padding_t: MBEDTLS_PADDING_xxx — not really misleading because all the modes except NONE have a name that's a name that's about a block cipher padding mode. (NONE is not 0, does this matter?)
• Type mbedtls_operation_t and its enum constants: MBEDTLS_OPERATION_NONE, MBEDTLS_ENCRYPT, MBEDTLS_DECRYPT — could be misleading if some other API uses different numerical values for encrypt/decrypt, but I don't think that's the case (RSA and pk have separate functions).
• Constants in an anonymous enum: MBEDTLS_KEY_LENGTH_NONE, MBEDTLS_KEY_LENGTH_DES, MBEDTLS_KEY_LENGTH_DES_EDE, MBEDTLS_KEY_LENGTH_DES_EDE3 — not misleading due to the lack of “cipher” in the name, but misleading in that their values are in bits, whereas elsewhere in cipher.h “length” is in bytes unless it follows the word “bit” (mbedtls_cipher_xxx_bitlen).

I think we really should rename MBEDTLS_MAX_KEY_LENGTH because it's very misleading, and we should give MBEDTLS_MAX_IV_LENGTH and MBEDTLS_MAX_BLOCK_LENGTH names following the same pattern.

I think we should rename the MBEDTLS_KEY_LENGTH_xxx constants. Or maybe just deprecate them? I don't understand why they're in the API at all. You can get the value from a function call mbedtls_cipher_info_get_key_bitlen. Having a preprocessor constant means you can use the size as an array size, but then why only for DES? And why bits which you'd have to first convert to bytes?

The other enum names aren't really harmful and I propose not to do anything about them.

Of course the old names need to stay (as \deprecated constants) for backward compatibility.

[yanesca]

If the old macros stay as deprecated, then we don't need to it in 4.0, they can wait until 4.x, right?

[gilles-peskine-arm]

Right, as long as we keep the old names, we can add new ones at any time.

There's a good chance that we'll remove cipher.h in 4.0 anyway, in which case this is moot, except that we might want to add a few warnings to the documentation in 3.6.

[gilles-peskine-arm]

For Mbed TLS 3.x, it's too late: there's only the 3.6 LTS branch and we aren't going to do these changes in an LTS branch.

In the next major release, we'll remove cipher.h.

So this issue is no longer relevant.