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

Node application on k8s fails with Assertion (slen) > (0)' failed.` #29202

Closed
aartek opened this issue Aug 19, 2019 · 8 comments
Closed

Node application on k8s fails with Assertion (slen) > (0)' failed.` #29202

aartek opened this issue Aug 19, 2019 · 8 comments
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. openssl Issues and PRs related to the OpenSSL dependency.

Comments

@aartek
Copy link

aartek commented Aug 19, 2019

Hi, our node application run on kubernetes recently started failing from a weird reason. It's run on Node 10.16.3 (the same was happening on 10.15.3) and it fails with message:

node[18]: ../src/node_crypto.cc:1925:static void node::crypto::SSLWrap<Base>::GetSession(const v8::FunctionCallbackInfo<v8::Value>&) [with Base = node::TLSWrap]: Assertion `(slen) > (0)' failed.

The problem seems to be random. Application works normally and suddenly the issue occurs and it fails.

Upgrade from 10.15.3 to 10.16.3 didn't help, so I decided to ask here. Maybe you have any idea of what can be the reason of such error and how can we deal with it?

@LEQADA
Copy link

LEQADA commented Aug 19, 2019

A minimal, reproducible example would be helpful.

@Fishrock123 Fishrock123 added the c++ Issues and PRs that require attention from people who are familiar with C++. label Aug 19, 2019
@aartek
Copy link
Author

aartek commented Aug 20, 2019

It would be, but I'd need to send attach a whole app probably. This is a simple express app which periodically does some http requests, being more specific, it's a smoke-test which performs some CRUD operations on other services.

We have the same app deployed in other aws availability zones, but failures occurs only in one of them. There are over 30 restarts since I created this issue.

@bnoordhuis bnoordhuis added the openssl Issues and PRs related to the OpenSSL dependency. label Aug 20, 2019
@bnoordhuis
Copy link
Member

If it is caused by something in v10.16.3, then it's almost certainly caused by the upgrade from openssl 1.1.0j to 1.1.1c.

I went over the relevant code in node.js and openssl and I don't see any changes that really stand out but our handling of i2d_SSL_SESSION()'s return value looks like it could be improved anyway.

I'll open a pull request shortly. It would be nice if you could test it out.

bnoordhuis added a commit to bnoordhuis/io.js that referenced this issue Aug 20, 2019
i2d_SSL_SESSION() can return a value <= 0 when the session is malformed
or otherwise invalid. Handle that case.

This change comes without a regression test because I couldn't figure
out a good way to generate an existing but invalid session in a timely
fashion.

Fixes: nodejs#29202
@aartek
Copy link
Author

aartek commented Aug 21, 2019

@bnoordhuis I've applied these changes to 10.x tree, built the docker image, deployed, and seems that the issue disappeared. No crashes since 4h.

Are you going to make a PR to the 10.x tree?

@bnoordhuis
Copy link
Member

Great to hear, thanks for testing. It'll be back-ported automatically to v10.x after it's been in master for a while (unless it doesn't cherry-pick cleanly, in which case I'll do it manually.)

@Trott Trott closed this as completed in ceace1f Aug 23, 2019
targos pushed a commit that referenced this issue Aug 26, 2019
i2d_SSL_SESSION() can return a value <= 0 when the session is malformed
or otherwise invalid. Handle that case.

This change comes without a regression test because I couldn't figure
out a good way to generate an existing but invalid session in a timely
fashion.

Fixes: #29202

PR-URL: #29225
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
@aartek
Copy link
Author

aartek commented Sep 2, 2019

@bnoordhuis, I wanted to ask if there is any progress in propagating it to v10.x? Could I get any estimated date when I could expect it to be released?

@bnoordhuis
Copy link
Member

@aartek I can't really provide you with one, it depends on when the release team thinks it's safe to include.

The usual order of things is fix goes into master -> current (v12.x) -> lts (v10.x). LTS releases are naturally conservative and slower paced so you should expect at least one or two months from fix to release.

@richardlau
Copy link
Member

@bnoordhuis, I wanted to ask if there is any progress in propagating it to v10.x? Could I get any estimated date when I could expect it to be released?

The current release plan is https://github.com/nodejs/Release/wiki. All dates are approximate and as mentioned above the LTS releases are slower paced so changes might not get into the next scheduled release.

cc @nodejs/lts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. openssl Issues and PRs related to the OpenSSL dependency.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants