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

tlsServer.setTicketKeys results in an abort #38305

Closed
zyscoder opened this issue Apr 20, 2021 · 3 comments · Fixed by #38308
Closed

tlsServer.setTicketKeys results in an abort #38305

zyscoder opened this issue Apr 20, 2021 · 3 comments · Fixed by #38308
Labels
confirmed-bug Issues with confirmed bugs. tls Issues and PRs related to the tls subsystem.

Comments

@zyscoder
Copy link

What steps will reproduce the bug?

Setup a node instance,

» node

and run the following javascript code.

tls = require('tls');tlsServer = new tls.Server();
tlsServer.setTicketKeys(1);

Then the node instance occurs an abort.

How often does it reproduce? Is there a required condition?

This abort can always be triggered following the steps above.

What is the expected behavior?

If any error occurs, an exception or other similar error-reporting stuff should be thrown. There is no reason to abort the whole node process.

What do you see instead?

» node
Welcome to Node.js v16.0.0-pre.
Type ".help" for more information.
> tls = require('tls');tlsServer = new tls.Server();
Server {
...
}
> tlsServer.setTicketKeys(1);
/home/zys/Toolchains/node/node[24181]: ../src/crypto/crypto_context.cc:1059:static void node::crypto::SecureContext::SetTicketKeys(const v8::FunctionCallbackInfo<v8::Value>&): Assertion `args[0]->IsArrayBufferView()' failed.
 1: 0x56281bfeaaf4 node::Abort() [/home/zys/Toolchains/node/node]
 2: 0x56281bfeab88  [/home/zys/Toolchains/node/node]
 3: 0x56281c15a1ec node::crypto::SecureContext::SetTicketKeys(v8::FunctionCallbackInfo<v8::Value> const&) [/home/zys/Toolchains/node/node]
 4: 0x56281c269167 v8::internal::FunctionCallbackArguments::Call(v8::internal::CallHandlerInfo) [/home/zys/Toolchains/node/node]
 5: 0x56281c269f10  [/home/zys/Toolchains/node/node]
 6: 0x56281c26a507  [/home/zys/Toolchains/node/node]
 7: 0x56281c26a79a v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) [/home/zys/Toolchains/node/node]
 8: 0x56281cc08259  [/home/zys/Toolchains/node/node]
[1]    24181 abort (core dumped)  /home/zys/Toolchains/node/node                                                                                                                                                                                               

Additional information

@Ayase-252 Ayase-252 added the tls Issues and PRs related to the tls subsystem. label Apr 20, 2021
@Ayase-252
Copy link
Member

reproducible in master branch

@Ayase-252 Ayase-252 added the confirmed-bug Issues with confirmed bugs. label Apr 20, 2021
@Ayase-252
Copy link
Member

Ayase-252 commented Apr 20, 2021

Code in question is here. It may need more user-friendly argument type checking (e.g. throw an type error to JS)

void SecureContext::SetTicketKeys(const FunctionCallbackInfo<Value>& args) {

@EladKeyshawn
Copy link

Code in question is here. It may need more user-friendly argument type checking (e.g. throw an type error to JS)

void SecureContext::SetTicketKeys(const FunctionCallbackInfo<Value>& args) {

@Ayase-252 sounds reasonable I'm going to work on this alright ?

aduh95 added a commit to aduh95/node that referenced this issue Apr 20, 2021
@aduh95 aduh95 closed this as completed in e151e90 Apr 23, 2021
targos pushed a commit that referenced this issue Apr 29, 2021
Fixes: #38305

PR-URL: #38308
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
targos pushed a commit that referenced this issue May 30, 2021
Fixes: #38305

PR-URL: #38308
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
targos pushed a commit that referenced this issue Jun 5, 2021
Fixes: #38305

PR-URL: #38308
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
targos pushed a commit that referenced this issue Jun 11, 2021
Fixes: #38305

PR-URL: #38308
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. tls Issues and PRs related to the tls subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants