Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TLS Cipher 4: clean up #5207

Closed
2 tasks done
mpg opened this issue Nov 22, 2021 · 4 comments · Fixed by #5434
Closed
2 tasks done

TLS Cipher 4: clean up #5207

mpg opened this issue Nov 22, 2021 · 4 comments · Fixed by #5434
Assignees
Labels
enhancement size-s Estimated task size: small (~2d)

Comments

@mpg
Copy link
Contributor

mpg commented Nov 22, 2021

Complete the replacement that was started in #5204.

  • In ssl_tls12_populate_transform() and mbedtls_ssl_tls13_populate_transform(), remove calls to mbedtls_cipher_setkey() and mbedtls_cipher_setup_psa()/mbedtls_cipher_setup() when MBEDTLS_USE_PSA_CRYPTO is enabled
  • In struct mbedtls_transform, remove the filedscipher_ctx_enc/cipher_ctx_dec and do the corresponding adaptations in mbedtls_ssl_transform_init and mbedtls_ssl_transform_free() when MBEDTLS_USE_PSA_CRYPTO is enabled

Depends on: #5205 and #5206

@mprse
Copy link
Contributor

mprse commented Jan 18, 2022

I see potential problem with removing cipher_ctx_enc/cipher_ctx_dec when PSA is enabled.
At least in mbedtls_ssl_encrypt_buf()/mbedtls_ssl_decrypt_buf() mode is determined as follows (using cipher_ctx_enc/cipher_ctx_dec):
mode = mbedtls_cipher_get_cipher_mode( &transform->cipher_ctx_enc );

Maybe for PSA we need also mode field?

@gilles-peskine-arm
Copy link
Contributor

A psa_algorithm_t encodes the same information as a mbedtls_cipher_mode_t. PSA doesn't need a mode field. It needs an alg field if there isn't already one.

@mpg
Copy link
Contributor Author

mpg commented Jan 19, 2022

That's also mentioned in the description of #5205 (first bullet point), on which this issue depends, precisely for this reason :) Since you seem to be tackling the whole cipher series you probably want to assign yourself 5205 too.

@mpg
Copy link
Contributor Author

mpg commented Jan 19, 2022

Btw, see also programs/psa/aead_cipher_psa.c introduced in #5436 regarding how data is represented in PSA vs Cipher: unless I missed something, with PSA the key + alg contains all the information that with Cipher was represented as context + tag length (see the _info() functions in that program in particular).

@mprse mprse linked a pull request Jan 21, 2022 that will close this issue
@mpg mpg closed this as completed in #5434 Feb 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement size-s Estimated task size: small (~2d)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants