Skip to content

tls: expose SSL_export_keying_material#31814

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.

Sign up for GitHub

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

Jump to bottom
Closed
simllll wants to merge 1 commit into from
Closed

tls: expose SSL_export_keying_material #31814

simllll wants to merge 1 commit into from

Conversation

@simllll
Copy link
Contributor

@simllllsimllll commented Feb 15, 2020

closes#31802

I added one test, that just checks if the function is available and returns the requested amount of data.
Please double check if I have done the memory allocation right.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows [commit guidelines]

@nodejs-github-botnodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. labels Feb 15, 2020
@simllllsimllll mentioned this pull request Feb 15, 2020
Closed
@simllllsimllllforce-pushed the feat/expose-SSL_export_keying_material branch from 886fb45 to e46860bCompareFebruary 15, 2020 18:37
@simllllsimllll changed the title tls-socket: expose SSL_export_keying_materialtls: expose SSL_export_keying_materialFeb 15, 2020
@simllllsimllllforce-pushed the feat/expose-SSL_export_keying_material branch from e46860b to e44ec54CompareFebruary 15, 2020 18:40
addaleax
addaleax reviewed Feb 15, 2020
View reviewed changes
@addaleaxaddaleax added semver-minor PRs that contain new features and should be released in the next minor version. tls Issues and PRs related to the tls subsystem. and removed lib / src Issues and PRs related to general changes in the lib or src directory. labels Feb 15, 2020
@simllllsimllllforce-pushed the feat/expose-SSL_export_keying_material branch from e44ec54 to 367dcc0CompareFebruary 15, 2020 19:42
@addaleax
Copy link
Member

addaleax commented Feb 15, 2020

@nodejs/crypto

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Feb 15, 2020

CI: https://ci.nodejs.org/job/node-test-pull-request/29160/

bnoordhuis
bnoordhuis reviewed Feb 15, 2020
View reviewed changes
@simllllsimllllforce-pushed the feat/expose-SSL_export_keying_material branch 2 times, most recently from 0a9170a to 94f7fceCompareFebruary 17, 2020 08:14
@simllll
Copy link
ContributorAuthor

simllll commented Feb 17, 2020

I've adressed all comments now, thanks for it! Also linted js and c++ code correctly :)

Is there anything else todo? e.g. how gets (in the good case this gets merged ;)) the ts type definition added?

bnoordhuis
bnoordhuis reviewed Feb 17, 2020
View reviewed changes
Copy link
Member

@bnoordhuisbnoordhuis left a comment

Choose a reason for hiding this comment

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

Some minor issues but LGTM when those are fixed. Thanks.

@simllllsimllllforce-pushed the feat/expose-SSL_export_keying_material branch from 94f7fce to 337becfCompareFebruary 17, 2020 11:25
jasnell
jasnell reviewed Feb 17, 2020
View reviewed changes
jasnell
jasnell reviewed Feb 17, 2020
View reviewed changes
jasnell
jasnell reviewed Feb 17, 2020
View reviewed changes
jasnell
jasnell reviewed Feb 17, 2020
View reviewed changes
jasnell
jasnell reviewed Feb 17, 2020
View reviewed changes
jasnell
jasnell reviewed Feb 17, 2020
View reviewed changes
jasnell
jasnell reviewed Feb 17, 2020
View reviewed changes
jasnell
jasnell reviewed Feb 17, 2020
View reviewed changes
@simllllsimllllforce-pushed the feat/expose-SSL_export_keying_material branch from 337becf to c98c1b5CompareFebruary 17, 2020 15:03
@simllllsimllll mentioned this pull request Feb 20, 2020
Closed
tniessen
tniessen requested changes Feb 22, 2020
View reviewed changes
Copy link
Member

@tniessentniessen left a comment

Choose a reason for hiding this comment

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

Sorry, I forgot about one thing: Could you please document the new error code in doc/api/errors.md?

@simllllsimllllforce-pushed the feat/expose-SSL_export_keying_material branch from a37a144 to 119ae43CompareFebruary 22, 2020 19:31
tniessen
tniessen approved these changes Feb 23, 2020
View reviewed changes
Copy link
Member

@tniessentniessen left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM.

@nodejs-github-bot

This comment has been minimized.

Sign in to view
@simllllsimllllforce-pushed the feat/expose-SSL_export_keying_material branch from 119ae43 to 176f4e6CompareFebruary 23, 2020 00:45
@simllllsimllllforce-pushed the feat/expose-SSL_export_keying_material branch from 176f4e6 to c2b23f9CompareFebruary 23, 2020 00:51
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Feb 23, 2020

CI: https://ci.nodejs.org/job/node-test-pull-request/29289/

tniessen pushed a commit that referenced this pull request Feb 23, 2020
@simllll@tniessen
tls: expose SSL_export_keying_material
341c06f
Fixes: #31802 PR-URL: #31814 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
@tniessen
Copy link
Member

tniessen commented Feb 23, 2020

Landed in 341c06f, thank you, @simllll, and congratulations on becoming a contributor! 🎉

@tniessentniessen removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Feb 23, 2020
@simllll
Copy link
ContributorAuthor

simllll commented Feb 23, 2020

Thanks from my side for the great input on this PR and for the quick and easy process :-) absolutely love the team behind node js! Great work! 😍

@simllllsimllll deleted the feat/expose-SSL_export_keying_material branch February 23, 2020 12:21
codebytere pushed a commit that referenced this pull request Feb 27, 2020
@simllll@codebytere
tls: expose SSL_export_keying_material
acb3aff
Fixes: #31802 PR-URL: #31814 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
@codebyterecodebytere mentioned this pull request Feb 29, 2020
Merged
codebytere added a commit that referenced this pull request Mar 1, 2020
@codebytere
2020-03-03 Version 13.10.0 (Current)
165ecaa
Notable changes: * async_hooks * introduce async-context API (vdeturckheim) #26540 * stream * support passing generator functions into pipeline() (Robert Nagy) #31223 * tls * expose SSL\_export\_keying\_material (simon) #31814 * vm * implement vm.measureMemory() for per-context memory measurement (Joyee Cheung) #31824 PR-URL: #32027
codebytere added a commit that referenced this pull request Mar 3, 2020
@codebytere
2020-03-03 Version 13.10.0 (Current)
e39b006
Notable changes: * async_hooks * introduce async-context API (vdeturckheim) #26540 * stream * support passing generator functions into pipeline() (Robert Nagy) #31223 * tls * expose SSL\_export\_keying\_material (simon) #31814 * vm * implement vm.measureMemory() for per-context memory measurement (Joyee Cheung) #31824 PR-URL: #32027
codebytere added a commit that referenced this pull request Mar 3, 2020
@codebytere
2020-03-03 Version 13.10.0 (Current)
471590e
Notable changes: * async_hooks * introduce async-context API (vdeturckheim) #26540 * stream * support passing generator functions into pipeline() (Robert Nagy) #31223 * tls * expose SSL\_export\_keying\_material (simon) #31814 * vm * implement vm.measureMemory() for per-context memory measurement (Joyee Cheung) #31824 PR-URL: #32027
codebytere added a commit that referenced this pull request Mar 3, 2020
@codebytere
2020-03-04 Version 13.10.0 (Current)
f6ffdc2
Notable changes: * async_hooks * introduce async-context API (vdeturckheim) #26540 * stream * support passing generator functions into pipeline() (Robert Nagy) #31223 * tls * expose SSL\_export\_keying\_material (simon) #31814 * vm * implement vm.measureMemory() for per-context memory measurement (Joyee Cheung) #31824 PR-URL: #32027
codebytere added a commit that referenced this pull request Mar 4, 2020
@codebytere
2020-03-04 Version 13.10.0 (Current)
7546686
Notable changes: * async_hooks * introduce async-context API (vdeturckheim) #26540 * stream * support passing generator functions into pipeline() (Robert Nagy) #31223 * tls * expose SSL\_export\_keying\_material (simon) #31814 * vm * implement vm.measureMemory() for per-context memory measurement (Joyee Cheung) #31824 PR-URL: #32027
@developher-netdevelopher-net mentioned this pull request Mar 11, 2020
Closed
@targostargos mentioned this pull request Apr 20, 2020
Closed
2 tasks
targos pushed a commit to targos/node that referenced this pull request Apr 25, 2020
@simllll@targos
tls: expose SSL_export_keying_material
b4c431e
Fixes: nodejs#31802 PR-URL: nodejs#31814 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
targos pushed a commit that referenced this pull request Apr 28, 2020
@simllll@targos
tls: expose SSL_export_keying_material
6348fae
Fixes: #31802 PR-URL: #31814 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
@targostargos mentioned this pull request May 2, 2020
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bnoordhuisbnoordhuisbnoordhuis approved these changes

@jasnelljasnelljasnell approved these changes

@addaleaxaddaleaxaddaleax approved these changes

@tniessentniessentniessen approved these changes

@sam-githubsam-githubAwaiting requested review from sam-github

Assignees

No one assigned

Labels

c++Issues and PRs that require attention from people who are familiar with C++.semver-minorPRs that contain new features and should be released in the next minor version.tlsIssues and PRs related to the tls subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

expose SSL_export_keying_material via Node API (e.g. like SSL_get_shared_sigalgs)

6 participants

@simllll@addaleax@nodejs-github-bot@tniessen@bnoordhuis@jasnell