Remove cipher.h in 4.0

[mpg]

This issue is meant as a place to discuss what we want to do with Cipher in 4.0.

There are two main options:

1. Keep it as part of the public API until 5.0, but aim to make it a thin wrapper around PSA Crypto (in 4.0 or 4.x).
2. Remove it from the public API in 4.0, and eventually remove it entirely (in 4.0 or 4.x).

Note: Cipher currently provides functionality similar to two PSA Crypto families: psa_cipher_xxx() and psa_aead_xxx(), plus it also wraps NIST-KW (which is not covered by PSA so far, and treated as an AEAD by Cipher). It provides both streaming and one-shot API for each family.

Note: for low-level cipher/AEAD modules (aes.h, gcm.h etc) I think the plan is to remove them from the public API, regardless of what we do with Cipher - the only exception being nist_kw.h.

Rationale for option 1 (keep)

This gives users more time to move to PSA. People who were using low-level cipher/AEAD modules need to migrate immediately, but those who were using the generic Cipher/AEAD API can migrate at a convenient time between now and 5.0.

There is no PSA alternative for NIST_KW. One is coming, but we might not have it by the time 4.0 comes out.

Work needed for option 1 (keep)

In 4.0:

• We may still want to remove some parts:
  • strings: mbedtls_cipher_info_from_string(), mbedtls_cipher_get_name() - see #8133 item 1
  • probably mbedtls_cipher_list() as well (not used in the library)
  • support for NIST-KW - see #8133 as well
  • the deprecated function mbedtls_cipher_setup_psa()
• We might want to remove mbedtls_cipher_info_t from the API in order to make it slicker but that would require users to change their code, so it runs against the stated goal. (Alternatively, we can keep the structure but later make it trivial as it has no public field.)
• We need some investigation (probably including a prototype) to make sure the current API can be implemented efficiently as a thin wrapper around PSA.
  • On the positive side, there is partial support already in the form of mbedtls_cipher_setup_psa() - which we're not keeping, but at least it serves as a proof of concept for AES with ECB, CBC, GCM, CCM, and for ChachaPoly - reducing the risk of big API mismatch that can't be worked around.
  • On the negative side, psa_crypto_cipher.c currently heavily relies on cipher.c so we'll need to change that in order to turn cipher.c into a thin wrapper. That's something we need to change anyway, but with option 2 it can be left for 4.x, while with option 1 we'll probably want to prototype it before 4.0.

In 4.0 or 4.x:

• Actually make it a thin wrapper around PSA crypto.
• Migrate all internal users (library and tests) to use PSA Crypto directly (note: common with option 2).

Rationale for option 2 (remove)

This leaves us with only one cipher/AEAD API to maintain, document and test. For new users, this also gives more clarity.

I'll note the current cipher.h API is not very well designed, and often not documented and tested enough. (Examples that pop to mind: can I/O buffers overlap? At one point the documentation said no but TLS did it anyway. What's the default padding mode if not explicitly selected? What are the minimum sizes of the output buffers - yes, some of them are implicit...) Which I think makes it a more attractive candidate for removal than other parts of the code.

Work needed for option 2 (remove)

In 4.0:

• Move cipher.h to an internal location and adapt all files that #include it.
• Change or remove all example programs that used it.
• Implement the PSA key wrapping interface and make it support NIST_KW.
• Implement XTS in the PSA interface.
• Provide a migration guide. (Note: we already have a pair of example programs (psa/aead_demo and cipher/cipher_aead_demo.c going in that direction for AEAD.)

In 4.0 or 4.x:

• Refactor psa_crypto_cipher.c so that it no longer
• Migrate all remaining internal users (library and tests) to use PSA Crypto directly (note: common with option 2).
• Then remove Cipher from the code base entirely.

Other options

• We could go for a mix: keep one part (cipher or AEAD) but remove the other. I'm not really seeing reasons to do this, but mentioning it anyway for the sake of completeness.
• Other?

[gilles-peskine-arm]

The general wisdom for API transitions is to have one full major version cycle where the new API is stable and the old API is present but deprecated. In Mbed TLS 3, we're at an intermediate stage where the new API is stable but old API is not deprecated.

There are features of cipher that are not provided through the PSA API, but that we want to keep in the library: XTS, NIST KW. If we remove cipher, we really need to provide them through PSA, and that's a nontrivial amount of work. We're still working on an API that would be suitable for NIST KW and I'm not convinced we'll ship that in 4.0.

There are nontrivial discrepancies between the legacy cipher API and the PSA cipher/AEAD APIs. So implementing even a useful subset of cipher on top of PSA is not easy. In addition to XTS and KW:

• Metadata is represented very differently.
• You can reset a legacy operation with the same key. You can't do that with PSA.
• For AEAD, the legacy API separates ciphertext and tag. The PSA API doesn't.

So I'm torn. From a user's perspective, we really should keep the cipher interface as deprecated. But it's a significant amount of work.

[mpg]

I think there's an other option I forgot, which may be useful in cases like this (that is, where implementing the old API as a thin wrapper around PSA is not easy): keep cipher.h public, keep its implementation mostly unchanged (don't make it a wrapper around PSA), and then just migrate internal users to PSA.

People who want to keep using the Cipher API pay a higher price in code size (since Cipher's not a wrapper around PSA, there's some duplication between its implementation and PSA's cipher/aead implementation), but people who are using PSA only are not paying that price.

For us, that's less work in that we don't have to re-implement Cipher on PSA, but OTOH that means we need to prioritize migrating internal users away from Cipher in order to improve code size - but unless I'm mistaken that's something we need to do anyway to optimize code size for people who only use PSA.

[mpg]

There are nontrivial discrepancies between the legacy cipher API and the PSA cipher/AEAD APIs. So implementing even a useful subset of cipher on top of PSA is not easy.

And yet, unless I'm mistaken, we've done it with setup_psa(). (Though we seem to be missing a test for reset() with the same key.) Of course the problem is that it's not a thin wrapper: there is still a lot of code in cipher.c to handle the discrepancies you mention.

[mpg]

Note (just for reference, I don't think it influences the decision to keep Cipher or not): currently (with USE_PSA), internal users of Cipher are:

• ccm.c, gcm.c, nist_kw.c, cmac.c
• pkcs5.c, pkcs12.c (note: pem.c uses low-level AES directly)
• psa_crypto_aead.c - direct uses are being removed as we speak, then we'll only have indirect use via ccm and gcm.
• psa_crypto_cipher.c - directly used all over the place
• psa_crypto_mac.c - both direct uses (cipher_setup()) and indirect with cmac.

[mpg]

Aw, I think I forgot a work item for "option 2 (remove)": what about uses of types from cipher.h in other APIs?

• In TLS: mbedtls_ssl_ticket_setup() take an argument of type mbedtls_cipher_type_t.

Actually, I say that's a work item for option 2, but that's something we probably still want to do with option 1 anyway.

[yanesca]

keep cipher.h public, keep its implementation mostly unchanged (don't make it a wrapper around PSA), and then just migrate internal users to PSA.

This would mean that we still need to maintain a parallel implementation in the lifetime of 4.x. This could be quite expensive as we probably will want to adjust the low level APIs. If we still have the old cipher module implementation (I mean as opposed to being a thin wrapper) around when we do that, we will have additional constraints and update more code.

Making cipher a thin wrapper around PSA would make this problem go away, but it is expensive and risky on its own.

I think the cleanest solution here is removal (option 2). (From the user perspective in essence this is just like any other API break since we provide a stable alternative. It might only look like a removal because the stable alternative is already present.)

[mpg]

The problem is the parts of cipher for which there isn't a stable alternative yet: XTS and NIST KW as Gilles pointed out.

[gilles-peskine-arm]

My current thinking is:

• XTS has a PSA spec. We implement it. (About 2×S because of the key type weirdness.)
• NIST_KW doesn't have a PSA spec yet. We keep it as a public module, with an API that takes a PSA key type rather than a cipher key type.
• Bye-byte cipher.h, we won't miss you.

[mpg]

• NIST_KW doesn't have a PSA spec yet. We keep it as a public module, with an API that takes a PSA key type rather than a cipher key type.

For reference, the two changes we need to make are:

• in mbedtls_nist_kw_setkey() change the type of a parameter from mbedtls_cipher_id_t to its PSA equivalent;
• in struct mbedtls_nist_kw_context change the type of a filed from mbedtls_cipher_context_t to a PSA equivalent.

The 2nd one is likely to require us to rework the module's implementation as well to work on top of PSA APIs, but that's desirable anyway, and in the past such transitions were found to be a reasonable amount of work.

[gilles-peskine-arm]

Decision: we're removing cipher.h as a public interface in TF-PSA-Crypto 1.0 (and thus Mbed TLS 4.0).

This is a minor loss of functionality (e.g. cipher names, some exotic padding modes, ...).

We'll need to adapt a few things. A detailed investigation is needed. At least:

• Adapt nist_kw.h
• Implement XTS through PSA

[yanesca]

We decided to go for Option 3: keep but offer no interface stability. After the repo-split the remaining task here is to document this fact clearly.

[gilles-peskine-arm]

To clarify, we will not keep cipher.h as part of the public API of TF-PSA-Crypto. We will keep cipher.h as an unstable interface solely for the benefit of Mbed TLS code, until we finish removing uses of this interface internally in Mbed TLS. We will also keep mbedtls_cipher_xxx types as unstable interfaces of the built-in driver in TF-PSA-Crypto as long as it's convenient.

Thus, for TF-PSA-Crypto 1.0 and Mbed TLS 4.0, the minimum we need to do is to remove places where mbedtls_cipher_xxx is exposed through 1.0/4.0 APIs. This requires the following work:

• Adapt nist_kw.h — needed for NIST_KW to be available in TF-PSA-Crypto.
• Adapt ssl_ticket.h — needed for ssl_ticket.h to be usable in Mbed TLS.
• Implement XTS through PSA — needed for XTS to be available in TF-PSA-Crypto. (Currently planned for after 1.0.)

Since we will make legacy crypto APIs internal by default, including cipher.h, no work on cipher.h itself is needed as part of this issue. Given that all the work is now filed in other issues, I am closing this high-level ticket.