Skip to content

Use md5 hashing for OpenSSL 3#924

Merged
nicolo-ribaudo merged 4 commits intobabel:mainfrom
pathmapper:md5_openssl_3
Mar 22, 2022
Merged

Use md5 hashing for OpenSSL 3#924
nicolo-ribaudo merged 4 commits intobabel:mainfrom
pathmapper:md5_openssl_3

Conversation

@pathmapper
Copy link
Copy Markdown
Contributor

@pathmapper pathmapper commented Jan 1, 2022

Please check if the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior? (You can also link to an open issue here)

#923

What is the new behavior?

Check process.versions.openssl instead of process.versions.node.

Does this PR introduce a breaking change?

  • Yes
  • No

JLHwung
JLHwung reviewed Jan 1, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@JLHwung JLHwung left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I think it'd better do a top level feature detection. We can try create to create an md4 hasher and switch to md5 if it throws.

OpenSSL 1.1.1 can disable md4 via --no-md4 building options. [1]

OpenSSL 3.0 can enable md4 via legacy provider config option. [2]

And node.js can be built with LibreSSL, too.

@pathmapper
Copy link
Copy Markdown
Contributor Author

pathmapper commented Jan 5, 2022

@JLHwung is ba92115 what you are thinking about?

JLHwung
JLHwung reviewed Jan 5, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@JLHwung JLHwung left a comment

Choose a reason for hiding this comment

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

Can you move the feature detection to the top level? So we don't have to detect it on every request.

@pathmapper
Copy link
Copy Markdown
Contributor Author

pathmapper commented Jan 7, 2022

move the feature detection to the top level

done

@pathmapper
Copy link
Copy Markdown
Contributor Author

pathmapper commented Feb 16, 2022

Any chance to get this merged?

@sirin2639
Copy link
Copy Markdown

sirin2639 commented Feb 18, 2022 •
edited
Loading

Has anyone tested this change on RH Linux? RH Linux enables FIPS and md5 isn't allowed on a FIPS compliant system.

Updated: RHEL8 allows SHA256.

@JLHwung
Copy link
Copy Markdown
Contributor

JLHwung commented Feb 18, 2022

Before this PR babel-loader uses md4 which is not FIPS compliant, too. We can open a new issue tracking how we should handle FIPS compliance.

@sonnyp sonnyp mentioned this pull request Mar 15, 2022
Closed
@sonnyp
Copy link
Copy Markdown

sonnyp commented Mar 15, 2022

babel-loader is currently unusable on Fedora 36. The beta is due to release this month.

I added some links in #934 about webpack approach in case of interest.

@nicolo-ribaudo nicolo-ribaudo merged commit 4bb9e21 into babel:main Mar 22, 2022
@pathmapper pathmapper deleted the md5_openssl_3 branch March 22, 2022 17:42
@pathmapper pathmapper mentioned this pull request Mar 22, 2022
Closed
@danielbachhuber danielbachhuber mentioned this pull request Mar 23, 2022
Merged
@DavidePastore DavidePastore mentioned this pull request Oct 13, 2022
Merged
@funnycoderstar funnycoderstar mentioned this pull request Jan 17, 2023
Open
This was referenced May 19, 2024
Open
Open
Open
Open
Open
Open
@Sheitak Sheitak mentioned this pull request May 26, 2024
Open
This was referenced Sep 15, 2024
Open
Open
@andreaeliasc andreaeliasc mentioned this pull request Sep 17, 2024
Open
@Sheitak Sheitak mentioned this pull request Sep 17, 2024
Open
@richyhoopd richyhoopd mentioned this pull request Sep 17, 2024
Open
@Sheitak Sheitak mentioned this pull request Sep 18, 2024
Open
@andreaeliasc andreaeliasc mentioned this pull request Sep 19, 2024
Open
@Sheitak Sheitak mentioned this pull request Sep 19, 2024
Open
@richyhoopd richyhoopd mentioned this pull request Sep 19, 2024
Open
@andreaeliasc andreaeliasc mentioned this pull request Sep 20, 2024
Open
This was referenced Sep 20, 2024
Open
Open
@richyhoopd richyhoopd mentioned this pull request Sep 21, 2024
Open
This was referenced Sep 22, 2024
Open
Open
@richyhoopd richyhoopd mentioned this pull request Sep 23, 2024
Open
This was referenced Sep 24, 2024
Open
Open
Open
Open
Open
Open
Open
@jonkiky jonkiky mentioned this pull request Oct 1, 2024
Closed
This was referenced Oct 1, 2024
Open
Open
Open
Open
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JLHwung JLHwung JLHwung approved these changes

@nicolo-ribaudo nicolo-ribaudo nicolo-ribaudo approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@pathmapper @sirin2639 @JLHwung @sonnyp @nicolo-ribaudo