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

Replace MBEDTLS_MD_CAN_SHA256 with PSA_WANT_ALG_SHA_256 #9173

Merged

Conversation

eleuzi01
Copy link
Contributor

@eleuzi01 eleuzi01 commented May 23, 2024

Description

Issue #9112

PR checklist

Please tick as appropriate and edit the reasons (e.g.: "backport: not needed because this is a new feature")

  • changelog not required, internal changes
  • 3.6 backport not required, needed for 4.0
  • 2.28 backport not required, needed for 4.0
  • tests not required

Notes for the submitter

Please refer to the contributing guidelines, especially the
checklist for PR contributors.

Help make review efficient:

  • Multiple simple commits
    • please structure your PR into a series of small commits, each of which does one thing
  • Avoid force-push
    • please do not force-push to update your PR - just add new commit(s)
  • See our Guidelines for Contributors for more details about the review process.

@eleuzi01 eleuzi01 added enhancement component-crypto Crypto primitives and low-level interfaces labels May 23, 2024
@eleuzi01 eleuzi01 self-assigned this May 23, 2024
@eleuzi01 eleuzi01 added the size-m Estimated task size: medium (~1w) label May 31, 2024
@gabor-mezei-arm gabor-mezei-arm linked an issue Jun 5, 2024 that may be closed by this pull request
@gabor-mezei-arm gabor-mezei-arm added the needs-preceding-pr Requires another PR to be merged first label Jun 5, 2024
@eleuzi01 eleuzi01 added needs-ci Needs to pass CI tests priority-high High priority - will be reviewed soon labels Jun 13, 2024
@eleuzi01 eleuzi01 force-pushed the replace-mbedtls-md-can-sha256 branch from b609f2c to 2080c46 Compare June 20, 2024 15:45
@eleuzi01 eleuzi01 added needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review and removed needs-preceding-pr Requires another PR to be merged first labels Jun 20, 2024
@gabor-mezei-arm gabor-mezei-arm self-requested a review June 20, 2024 16:07
@eleuzi01 eleuzi01 removed the needs-ci Needs to pass CI tests label Jun 24, 2024
@eleuzi01 eleuzi01 force-pushed the replace-mbedtls-md-can-sha256 branch from 414bce4 to a181126 Compare June 26, 2024 12:40
@eleuzi01 eleuzi01 added the needs-ci Needs to pass CI tests label Jun 26, 2024
@eleuzi01 eleuzi01 force-pushed the replace-mbedtls-md-can-sha256 branch from a181126 to 3a91df9 Compare June 26, 2024 15:00
@eleuzi01 eleuzi01 removed the needs-ci Needs to pass CI tests label Jun 27, 2024
Copy link
Contributor

@gabor-mezei-arm gabor-mezei-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

gabor-mezei-arm
gabor-mezei-arm previously approved these changes Jul 2, 2024
Copy link
Contributor

@gabor-mezei-arm gabor-mezei-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tom-daubney-arm tom-daubney-arm self-requested a review July 2, 2024 19:05
@tom-daubney-arm tom-daubney-arm removed the needs-reviewer This PR needs someone to pick it up for review label Jul 2, 2024
tom-daubney-arm
tom-daubney-arm previously approved these changes Jul 9, 2024
Copy link
Contributor

@tom-daubney-arm tom-daubney-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - thanks!

@tom-daubney-arm tom-daubney-arm added the approved Design and code approved - may be waiting for CI or backports label Jul 9, 2024
@eleuzi01 eleuzi01 dismissed stale reviews from tom-daubney-arm and gabor-mezei-arm via eb7c0c1 July 9, 2024 14:58
@eleuzi01 eleuzi01 force-pushed the replace-mbedtls-md-can-sha256 branch from e61443a to eb7c0c1 Compare July 9, 2024 14:58
@eleuzi01 eleuzi01 requested a review from gabor-mezei-arm July 10, 2024 08:38
Copy link
Contributor

@gabor-mezei-arm gabor-mezei-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@tom-daubney-arm tom-daubney-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current rebase LGTM but this PR is already in need of another rebase what with all the other PRs that have gone in today.

@eleuzi01 if you could ping me as soon as you have rebased again and the CI passes that would be great, I will then re-re-review ASAP and hopefully we can get this in without further bitrotting.

@eleuzi01 eleuzi01 dismissed stale reviews from tom-daubney-arm and gabor-mezei-arm via 4e6a368 July 11, 2024 08:14
@eleuzi01 eleuzi01 force-pushed the replace-mbedtls-md-can-sha256 branch from eb7c0c1 to 4e6a368 Compare July 11, 2024 08:14
Copy link
Contributor

@tom-daubney-arm tom-daubney-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - thanks!

Copy link
Contributor

@gabor-mezei-arm gabor-mezei-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@paul-elliott-arm paul-elliott-arm added this pull request to the merge queue Jul 11, 2024
Merged via the queue into Mbed-TLS:development with commit 095fa2c Jul 11, 2024
6 checks passed
@irwir
Copy link
Contributor

irwir commented Jul 11, 2024

Still using MBEDTLS_MD_CAN_SHA256 macro: check_config.h and config_adjust_legacy_crypto.h.
Should these files be updated too?

@tom-daubney-arm
Copy link
Contributor

Still using MBEDTLS_MD_CAN_SHA256 macro: check_config.h and config_adjust_legacy_crypto.h. Should these files be updated too?

Not for now, my understanding is these will be updated later however @gabor-mezei-arm knows more so could weigh in here.

@gabor-mezei-arm
Copy link
Contributor

It will be removed in #9396.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Design and code approved - may be waiting for CI or backports component-crypto Crypto primitives and low-level interfaces enhancement priority-high High priority - will be reviewed soon size-m Estimated task size: medium (~1w)
Projects
Development

Successfully merging this pull request may close these issues.

Replace MBEDTLS_MD_CAN_SHA256 with its PSA_WANT counterpartart
6 participants