Skip to content

fix: re-enable HKDF crypto functionality - #34767

Merged
codebytere merged 2 commits into
mainfrom
hkdf-works-now
Jun 29, 2022
Merged

fix: re-enable HKDF crypto functionality#34767
codebytere merged 2 commits into
mainfrom
hkdf-works-now

Conversation

@codebytere

@codebytere codebytere commented Jun 28, 2022

Copy link
Copy Markdown
Member

Description of Change

Closes #31634.

Enables HKDF support in embedded Node.js as a result of https://boringssl-review.googlesource.com/c/boringssl/+/52829.

Checklist

Release Notes

Notes: Enable missing HKDF support in Node.js crypto.

@codebytere codebytere added semver/patch backwards-compatible bug fixes no-backport labels Jun 28, 2022
@codebytere
codebytere requested review from a team as code owners June 28, 2022 07:54
@electron-cation electron-cation Bot added the new-pr 🌱 PR opened recently label Jun 28, 2022
@electron-cation electron-cation Bot removed the new-pr 🌱 PR opened recently label Jun 29, 2022
['sha256', 'secret', 'salt', 'info', 10],
['sha512', 'secret', 'salt', '', 15],
];
-if (!common.hasOpenSSL3)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we expose a property common.openSSLisBoringSSL based on the build time constant OPENSSL_IS_BORINGSSL that is already available in upstream Node.js. It could simplify the patch in a upstream friendly way.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@deepak1556 it's stalled but i actually started that ages ago: nodejs/node#38928

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh cool, btw agree with the concern in nodejs/node#38928 (review), shouldn't the change be

#ifdef OPENSSL_IS_BORINGSSL
    #define IS_BORINGSSL 1
    NODE_DEFINE_CONSTANT(target, IS_BORINGSSL);
   #undef IS_BORINGSSL
#endif

@codebytere
codebytere merged commit ad2b1fe into main Jun 29, 2022
@codebytere
codebytere deleted the hkdf-works-now branch June 29, 2022 12:53
@release-clerk

release-clerk Bot commented Jun 29, 2022

Copy link
Copy Markdown

Release Notes Persisted

Enable missing HKDF support in Node.js crypto.

schetle pushed a commit to schetle/electron that referenced this pull request Nov 3, 2022
* fix: re-enable HKDF crypto functionality

* chore: update patches

Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
khalwa pushed a commit to solarwindscloud/electron that referenced this pull request Feb 22, 2023
* fix: re-enable HKDF crypto functionality

* chore: update patches

Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-backport semver/patch backwards-compatible bug fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: crypto.hkdf "Deriving bits failed"

2 participants