FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

tls: expose SSL_export_keying_material by simllll · Pull Request #31814 · nodejs/node · GitHub

/ node Public

tls: expose SSL_export_keying_material - #31814

Closed
simllll wants to merge 1 commit into
nodejs:masterfrom
simllll:feat/expose-SSL_export_keying_material
ConversationCommits (1)ChecksFiles changed
Closed

tls: expose SSL_export_keying_material#31814
simllll wants to merge 1 commit into
nodejs:masterfrom
simllll:feat/expose-SSL_export_keying_material

Conversation

simllll commented Feb 15, 2020

Copy link
Copy Markdown
Contributor

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-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
simllll mentioned this pull request Feb 15, 2020
Closed
simllll force-pushed the feat/expose-SSL_export_keying_material branch from 886fb45 to e46860b Compare February 15, 2020 18:37
simllll changed the title tls-socket: expose SSL_export_keying_material tls: expose SSL_export_keying_material Feb 15, 2020
simllll requested a review from sam-github February 15, 2020 18:38
simllll force-pushed the feat/expose-SSL_export_keying_material branch from e46860b to e44ec54 Compare February 15, 2020 18:40
addaleax reviewed Feb 15, 2020
View reviewed changes
Comment thread doc/api/tls.md Outdated
Comment thread doc/api/tls.md Outdated
Comment thread src/node_crypto.cc Outdated
Comment thread src/node_crypto.cc Outdated
addaleax 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
simllll force-pushed the feat/expose-SSL_export_keying_material branch from e44ec54 to 367dcc0 Compare February 15, 2020 19:42

addaleax commented Feb 15, 2020

Copy link
Copy Markdown
Member

@nodejs/crypto

nodejs-github-bot commented Feb 15, 2020

Copy link
Copy Markdown
Collaborator

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

bnoordhuis reviewed Feb 15, 2020
View reviewed changes
Comment thread doc/api/tls.md Outdated
Comment thread src/node_crypto.cc Outdated
Comment thread src/node_crypto.cc Outdated
Comment thread src/node_crypto.cc Outdated
Comment thread src/node_crypto.cc Outdated
simllll force-pushed the feat/expose-SSL_export_keying_material branch 2 times, most recently from 0a9170a to 94f7fce Compare February 17, 2020 08:14

simllll commented Feb 17, 2020

Copy link
Copy Markdown
Contributor Author

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 reviewed Feb 17, 2020
View reviewed changes

bnoordhuis left a comment

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

Comment thread lib/_tls_wrap.js Outdated
Comment thread lib/_tls_wrap.js Outdated
Comment thread src/node_crypto.cc Outdated
simllll force-pushed the feat/expose-SSL_export_keying_material branch from 94f7fce to 337becf Compare February 17, 2020 11:25
jasnell reviewed Feb 17, 2020
View reviewed changes
Comment thread doc/api/tls.md Outdated
jasnell reviewed Feb 17, 2020
View reviewed changes
Comment thread doc/api/tls.md Outdated
jasnell reviewed Feb 17, 2020
View reviewed changes
Comment thread doc/api/tls.md Outdated
jasnell reviewed Feb 17, 2020
View reviewed changes
Comment thread lib/internal/errors.js Outdated
jasnell reviewed Feb 17, 2020
View reviewed changes
Comment thread test/parallel/test-tls-exportkeyingmaterial.js Outdated
jasnell reviewed Feb 17, 2020
View reviewed changes
Comment thread test/parallel/test-tls-exportkeyingmaterial.js Outdated
jasnell reviewed Feb 17, 2020
View reviewed changes
Comment thread test/parallel/test-tls-exportkeyingmaterial.js Outdated
jasnell reviewed Feb 17, 2020
View reviewed changes
Comment thread test/parallel/test-tls-exportkeyingmaterial.js Outdated
simllll force-pushed the feat/expose-SSL_export_keying_material branch from 337becf to c98c1b5 Compare February 17, 2020 15:03
simllll mentioned this pull request Feb 20, 2020
Closed
tniessen requested changes Feb 22, 2020
View reviewed changes

tniessen left a comment

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

simllll force-pushed the feat/expose-SSL_export_keying_material branch from a37a144 to 119ae43 Compare February 22, 2020 19:31
simllll requested a review from tniessen February 22, 2020 22:23
tniessen approved these changes Feb 23, 2020
View reviewed changes

tniessen left a comment

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks, LGTM.

Comment thread doc/api/tls.md Outdated
Comment thread doc/api/tls.md Outdated

This comment has been minimized.

Sign in to view
simllll force-pushed the feat/expose-SSL_export_keying_material branch from 119ae43 to 176f4e6 Compare February 23, 2020 00:45
simllll force-pushed the feat/expose-SSL_export_keying_material branch from 176f4e6 to c2b23f9 Compare February 23, 2020 00:51

nodejs-github-bot commented Feb 23, 2020

Copy link
Copy Markdown
Collaborator

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

tniessen pushed a commit that referenced this pull request Feb 23, 2020
tls: expose SSL_export_keying_material
341c06f 
Fixes: #31802

PR-URL: #31814
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>

tniessen commented Feb 23, 2020

Copy link
Copy Markdown
Member

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

tniessen closed this Feb 23, 2020
tniessen removed the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Feb 23, 2020

simllll commented Feb 23, 2020

Copy link
Copy Markdown
Contributor Author

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! 😍

simllll 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
tls: expose SSL_export_keying_material
acb3aff 
Fixes: #31802

PR-URL: #31814
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
codebytere mentioned this pull request Feb 29, 2020
Merged
codebytere added a commit that referenced this pull request Mar 1, 2020
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
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
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
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
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-net mentioned this pull request Mar 11, 2020
Closed
targos 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
tls: expose SSL_export_keying_material
b4c431e 
Fixes: nodejs#31802

PR-URL: nodejs#31814
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
targos pushed a commit that referenced this pull request Apr 28, 2020
tls: expose SSL_export_keying_material
6348fae 
Fixes: #31802

PR-URL: #31814
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
targos mentioned this pull request May 2, 2020
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

bnoordhuis bnoordhuis approved these changes
jasnell jasnell approved these changes
addaleax addaleax approved these changes
tniessen tniessen approved these changes
sam-github Awaiting 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-minor PRs that contain new features and should be released in the next minor version. tls Issues 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

Back | FazBrowse Home | New Git URL