mbedtls icon indicating copy to clipboard operation
mbedtls copied to clipboard

Make legacy crypto APIs internal

Open mpg opened this issue 1 year ago • 3 comments

As part of making PSA Crypto the main crypto API in 4.0, we're making (most of) the legacy crypto API internal (that is, headers would be visible to other crypto modules but not to applications).

This issue is an attempt at summarizing the consensus that was reached last time we discussed this.

Categorized list of crypto headers

Cipher & related:

  • internalize: aes.h, aria.h, block_cipher.h, camellia.h, ccm.h, chacha20.h, chachapoly.h, cmac.h, des.h, gcm.h, poly1305.h,
  • under discussion: cipher.h #8451
  • keep for now: nist_kw.h (note: includes cipher.h! If we remove cipher.h we'll need to change types in nist_kw.h - only mbedtls_cipher_id_t is used AFAICS).

Hashes & related:

  • internalize: md5.h, ripemd160.h, sha1.h, sha256.h, sha3.h, sha512.h,
  • under discussion: md.h #8450
  • remove hkdf.h

Randomness:

  • internalize: entropy.h, ctr_drbg.h, hmac_drbg.h - require #8150 to avoid functionality loss

Asymmetric crypto:

  • internalize: bignum.h, dhm.h, ecdh.h, ecdsa.h, ecjpake.h, ecp.h, rsa.h - note: potential feature loss by removing ecp.h and rsa.h (and bignum.h) but came to an agreement accepting it. Note: important feature loss (restartable/interruptible) unless #7292, #7293, #7294 and resulting EPIC(s) are done first. Note impact on fuzzers.
  • under discussion: pk.h #8452
  • keep for now: lms.h (note: does not include other headers)

Supporting modules:

  • pkcs12.h, pkcs5.h -> internalize (used by PK parse and soon extended PSA key import)
  • base64.h -> internalize (used by PEM only)
  • pem.h -> semi-internal? (used by X.509, PK parse/write and soon extended PSA key import/export)
  • asn1.h, asn1write.h -> semi-internal? (used by X.509, PK parse/write and soon extended PSA key import/export, but also basic RSA import/export)
  • oid.h -> can't remember: split (X.509 vs crypto) and internalize?
  • platform.h, platform_time.h, platform_util.h, threading.h -> keep?
  • constant_time.h -> ?? (only one function now)
  • build_info.h error.h, version.h, -> keep but split crypto vs non-crypto I guess?
  • psa_util.h -> keep if it helps the transition, or internalize
  • private_access.h -> keep
  • memory_buffer_alloc.h -> ??
  • timing.h -> ??

Notes

Above, by "crypto headers" I mean all headers that are part of libmbedcrypto - that includes things that are does not actually provide crypto functionality, like platform support.

There seems to be a need to a special "semi-internal" status for header that should be visible to both crypto modules and X.509, but perhaps not to applications (pem.h, asn1.h, asn1write.h).

This issue it mainly to record what we've discussed, and host further discussion if needed. For execution, we probably want to split this in a series of task, for example per area as listed above.

There will probably be interactions with splitting out PSA crypto to its own repo too.

mpg avatar Dec 28 '23 09:12 mpg

Ping @dave-rodgman - I think you took notes during the meeting, please correct my list if it's not in line with those notes.

mpg avatar Dec 28 '23 09:12 mpg

Note: some legacy modules, when made internal, can actually be removed as well - when the PSA implementation of that feature doesn't actually use the legacy module.

For example for FFDH, when making dhm.h internal, we can remove dhm.c entirely, as psa_crypto_ffdh.c does not depend on it. We can also move the definition of the constants for the domain parameters to psa_crypto_ffdh.c (currently taken from dhm.h) and then remove dhm.h as well.

Similar considerations might apply to other modules, I've not tried making a complete list yet.

(Note: currently psa_crypto_ecp.c calls functions from ecdh.c but I don't think there's a good reason for that, IMO it should be calling mbedtls_ecp_mul() directly instead.)

mpg avatar Jan 25 '24 09:01 mpg

(Note: currently psa_crypto_ecp.c calls functions from ecdh.c but I don't think there's a good reason for that, IMO it should be calling mbedtls_ecp_mul() directly instead.)

Ah, actually there's a reason, it's that Everest dispatch is implemented in ecdh.c not ecp.c. IMO that's not a good reason in the long term: Everest should be a PSA driver like everything else instead of having its own special dispatch mechanism in ecdh.c.

mpg avatar Jan 25 '24 10:01 mpg