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

feat: expose blowfish cipher family #32356

Merged
merged 7 commits into from Jan 12, 2022
Merged

feat: expose blowfish cipher family #32356

merged 7 commits into from Jan 12, 2022

Conversation

nornagon
Copy link
Member

@nornagon nornagon commented Jan 5, 2022

Description of Change

Adds support for bf-{cbc,cfb,ecb} to the nodejs crypto api.

These ciphers are obsolete, but some applications still depend on them, and
boringssl has the code, so we might as well expose them.

Closes #20238.

Checklist

Release Notes

Notes: Support obsolete blowfish ciphers bf-{cbc,cfb,ecb} through the Node.js crypto API.

@nornagon nornagon requested a review from a team as a code owner January 5, 2022 21:13
@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Jan 5, 2022
@nornagon nornagon added the semver/minor backwards-compatible functionality label Jan 5, 2022
Copy link
Member

@codebytere codebytere left a comment

Choose a reason for hiding this comment

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

API LGTM though i think you need to fix an undefined symbol issue.

@codebytere
Copy link
Member

@nornagon some of the Node.js tests still need to be updated it looks like - they expect undefined but we're now returning the bf data:

+ {
+   blockSize: 1,
+   ivLength: 8,
+   keyLength: 16,
+   mode: 'cfb',
+   name: 'bf-cfb',
+   nid: 93
+ }

Copy link
Contributor

@jkleinsc jkleinsc left a comment

Choose a reason for hiding this comment

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

API LGTM

@nornagon nornagon changed the title feat: expose blowfish cipher family feat: expose blowfish cipher family Jan 12, 2022
@nornagon nornagon changed the title feat: expose blowfish cipher family feat: expose blowfish cipher family Jan 12, 2022
@nornagon nornagon merged commit 0a7bc4f into main Jan 12, 2022
@nornagon nornagon deleted the expose-blowfish branch January 12, 2022 18:44
@release-clerk
Copy link

release-clerk bot commented Jan 12, 2022

Release Notes Persisted

Support obsolete blowfish ciphers bf-{cbc,cfb,ecb} through the Node.js crypto API.

trop bot pushed a commit that referenced this pull request Jan 12, 2022
@trop
Copy link
Contributor

trop bot commented Jan 12, 2022

I have automatically backported this PR to "17-x-y", please check out #32439

@trop trop bot removed the in-flight/17-x-y label Jan 18, 2022
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Jan 18, 2022
t57ser pushed a commit to t57ser/electron that referenced this pull request Jan 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-review/requested 🗳 semver/minor backwards-compatible functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for bf-cbc cipher
3 participants