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 record HMAC 3: remove non-PSA keys #5179

Closed
3 tasks done
mpg opened this issue Nov 16, 2021 · 3 comments · Fixed by #5573
Closed
3 tasks done

TLS record HMAC 3: remove non-PSA keys #5179

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

Comments

@mpg
Copy link
Contributor

mpg commented Nov 16, 2021

This is the conclusion of a series starting with #5176 and continuing with #5177 and #5178. Now that all computations use PSA, we can remove the legacy contexts from the transform when MBEDTLS_USE_PSA_CRYPTO is defined.

  • In struct mbedtls_ssl_transform, remove md_ctx_enc/md_ctx_dec when MBEDTLS_USE_PSA_CRYPTO is defined.
  • In mbedtls_ssl_transform_init() and mbedtls_ssl_transform_free(), remove related code when MBEDTLS_USE_PSA_CRYPTO is defined.
  • In ssl_tls12_populate_transform(), code using those fields when MBEDTLS_USE_PSA_CRYPTO is defined.

Depends on: #5177 and #5178 - for the legacy context to be no longer needed.

@superna9999
Copy link
Contributor

While #5573 implements this, I think it could go further to a complete removal of md_info in ssl_tls12_populate_transform(), should I create a new issue for that or put a new PR when #5573 is merged ?

@mpg
Copy link
Contributor Author

mpg commented Mar 11, 2022

Hmm, good question. Strictly speaking, I think that step further would be part of #5210 - remove the remaining dependencies on MD - which we weren't planning on working on right now. OTOH, it's always better to strike the iron while it's hot :) How much work do you think it would be? If it's just one or two smallish commits, perhaps just do it as part of 5573? Otherwise, I'd say open a PR after 5573 is merged. Or you could also just leave a note in 5210 with your thoughts. Up to you!

@superna9999
Copy link
Contributor

It's a small commit, I'll open a PR after #5573 is merged, not need to add more complexity to it.

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.

2 participants